Bug 1463016: Part 3 - Remove (mostly unnecessary) instanceof nsIDOMWindow checks. r?nika draft
authorKris Maglione <maglione.k@gmail.com>
Sun, 20 May 2018 16:28:59 -0700
changeset 797627 baee8869ccc4089c9354d4d97ef381c2282551c8
parent 797626 6070f579bf03cc842e594a11016d5b58921c1248
child 797628 a48b502edb6d62e28579a41ae921ec72eef81a53
push id110518
push usermaglione.k@gmail.com
push dateMon, 21 May 2018 02:57:23 +0000
reviewersnika
bugs1463016
milestone62.0a1
Bug 1463016: Part 3 - Remove (mostly unnecessary) instanceof nsIDOMWindow checks. r?nika Most of these checks are there for the side-effect of calling QueryInterface on the object, and making sure it's actually queried to nsIDOMWindow. That's never necessary for WebIDL objects, so those can easily be removed. A few of the others seemed over-cautious, so I removed those as well. For instance, if someone dispatches a domwindowopened observer notification with a non-Window object, it's better that we fail loudly when we try to use it as a window, rather than return silently. The rest actually seem to have some purpose, so I switched most of them to use Window.isInstance. MozReview-Commit-ID: IBglPGKZJ0u
browser/base/content/browser.js
browser/base/content/test/general/browser_tabfocus.js
browser/components/downloads/DownloadsCommon.jsm
browser/components/feeds/WebContentConverter.js
browser/components/uitour/test/browser_UITour_resetProfile.js
browser/modules/BrowserUsageTelemetry.jsm
devtools/server/actors/addon.js
devtools/server/actors/object/previewers.js
devtools/server/actors/source.js
devtools/server/actors/webconsole.js
devtools/server/actors/webconsole/utils.js
devtools/server/actors/webextension.js
devtools/shared/builtin-modules.js
devtools/shared/layout/utils.js
toolkit/components/downloads/DownloadCore.jsm
toolkit/content/browser-content.js
toolkit/modules/addons/WebRequestContent.js
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -6840,21 +6840,21 @@ var CanvasPermissionPromptHelper = {
   // aSubject is an nsIBrowser (e10s) or an nsIDOMWindow (non-e10s).
   // aData is an URL string.
   observe(aSubject, aTopic, aData) {
     if (aTopic != this._permissionsPrompt) {
       return;
     }
 
     let browser;
-    if (aSubject instanceof Ci.nsIDOMWindow) {
+    if (aSubject instanceof Ci.nsIBrowser) {
+      browser = aSubject;
+    } else {
       let contentWindow = aSubject;
       browser = gBrowser.getBrowserForContentWindow(contentWindow);
-    } else {
-      browser = aSubject.QueryInterface(Ci.nsIBrowser);
     }
 
     let uri = Services.io.newURI(aData);
     if (gBrowser.selectedBrowser !== browser) {
       // Must belong to some other window.
       return;
     }
 
--- a/browser/base/content/test/general/browser_tabfocus.js
+++ b/browser/base/content/test/general/browser_tabfocus.js
@@ -67,19 +67,19 @@ function focusInChild() {
     return (String(target.location).includes("1")) ? "window1" : "window2";
   }
 
   function eventListener(event) {
     // Stop the shim code from seeing this event process.
     event.stopImmediatePropagation();
 
     var id;
-    if (event.target instanceof Ci.nsIDOMWindow)
+    if (Window.isInstance(event.target))
       id = getWindowDocId(event.originalTarget) + "-window";
-    else if (event.target instanceof Ci.nsIDOMDocument)
+    else if (Document.isInstance(event.target))
       id = getWindowDocId(event.originalTarget) + "-document";
     else
       id = event.originalTarget.id;
     sendSyncMessage("Browser:FocusChanged", { details: event.type + ": " + id });
   }
 
   addEventListener("focus", eventListener, true);
   addEventListener("blur", eventListener, true);
--- a/browser/components/downloads/DownloadsCommon.jsm
+++ b/browser/components/downloads/DownloadsCommon.jsm
@@ -51,16 +51,18 @@ XPCOMUtils.defineLazyGetter(this, "Downl
   let { ConsoleAPI } = ChromeUtils.import("resource://gre/modules/Console.jsm", {});
   let consoleOptions = {
     maxLogLevelPref: "browser.download.loglevel",
     prefix: "Downloads"
   };
   return new ConsoleAPI(consoleOptions);
 });
 
+Cu.importGlobalProperties(["Window"]);
+
 const kDownloadsStringBundleUrl =
   "chrome://browser/locale/downloads/downloads.properties";
 
 const kDownloadsStringsRequiringFormatting = {
   sizeWithUnits: true,
   statusSeparator: true,
   statusSeparatorBeforeNumber: true,
   fileExecutableSecurityWarning: true
@@ -404,17 +406,17 @@ var DownloadsCommon = {
    */
   openDownloadedFile(aFile, aMimeInfo, aOwnerWindow) {
     if (!(aFile instanceof Ci.nsIFile)) {
       throw new Error("aFile must be a nsIFile object");
     }
     if (aMimeInfo && !(aMimeInfo instanceof Ci.nsIMIMEInfo)) {
       throw new Error("Invalid value passed for aMimeInfo");
     }
-    if (!(aOwnerWindow instanceof Ci.nsIDOMWindow)) {
+    if (!Window.isInstance(aOwnerWindow)) {
       throw new Error("aOwnerWindow must be a dom-window object");
     }
 
     let isWindowsExe = AppConstants.platform == "win" &&
       aFile.leafName.toLowerCase().endsWith(".exe");
 
     let promiseShouldLaunch;
     // Don't prompt on Windows for .exe since there will be a native prompt.
@@ -577,17 +579,17 @@ var DownloadsCommon = {
         break;
       default: // Assume Downloads.Error.BLOCK_VERDICT_MALWARE
         message = s.unblockTypeMalware;
         break;
     }
     message += "\n\n" + s.unblockTip2;
 
     Services.ww.registerNotification(function onOpen(subj, topic) {
-      if (topic == "domwindowopened" && subj instanceof Ci.nsIDOMWindow) {
+      if (topic == "domwindowopened") {
         // Make sure to listen for "DOMContentLoaded" because it is fired
         // before the "load" event.
         subj.addEventListener("DOMContentLoaded", function() {
           if (subj.document.documentURI ==
               "chrome://global/content/commonDialog.xul") {
             Services.ww.unregisterNotification(onOpen);
             let dialog = subj.document.getElementById("commonDialog");
             if (dialog) {
--- a/browser/components/feeds/WebContentConverter.js
+++ b/browser/components/feeds/WebContentConverter.js
@@ -360,17 +360,17 @@ WebContentConverterRegistrar.prototype =
     return false;
   },
 
   /**
    * See nsIWebContentHandlerRegistrar
    */
   registerProtocolHandler(aProtocol, aURIString, aTitle, aBrowserOrWindow) {
     LOG("registerProtocolHandler(" + aProtocol + "," + aURIString + "," + aTitle + ")");
-    let haveWindow = (aBrowserOrWindow instanceof Ci.nsIDOMWindow);
+    let haveWindow = !(aBrowserOrWindow instanceof Ci.nsIBrowser);
     let uri;
     if (haveWindow) {
       uri = Utils.checkAndGetURI(aURIString, aBrowserOrWindow);
     } else {
       // aURIString must not be a relative URI.
       uri = Utils.makeURI(aURIString, null);
     }
 
@@ -450,17 +450,17 @@ WebContentConverterRegistrar.prototype =
    * prompt the user to confirm the registration.
    */
   registerContentHandler(aContentType, aURIString, aTitle, aWindowOrBrowser) {
     LOG("registerContentHandler(" + aContentType + "," + aURIString + "," + aTitle + ")");
 
     // Make sure to do our URL checks up front, before our content type check,
     // just like the WebContentConverterRegistrarContent does.
     let haveWindow = aWindowOrBrowser &&
-                     (aWindowOrBrowser instanceof Ci.nsIDOMWindow);
+                     !(aWindowOrBrowser instanceof Ci.nsIBrowser);
     let uri;
     if (haveWindow) {
       uri = Utils.checkAndGetURI(aURIString, aWindowOrBrowser);
     } else if (aWindowOrBrowser) {
       // uri was vetted in the content process.
       uri = Utils.makeURI(aURIString, null);
     }
 
--- a/browser/components/uitour/test/browser_UITour_resetProfile.js
+++ b/browser/components/uitour/test/browser_UITour_resetProfile.js
@@ -7,17 +7,17 @@ var gContentWindow;
 add_task(setup_UITourTest);
 
 // Test that a reset profile dialog appears when "resetFirefox" event is triggered
 add_UITour_task(async function test_resetFirefox() {
   let canReset = await getConfigurationPromise("canReset");
   ok(!canReset, "Shouldn't be able to reset from mochitest's temporary profile.");
   let dialogPromise = new Promise((resolve) => {
     Services.ww.registerNotification(function onOpen(subj, topic, data) {
-      if (topic == "domwindowopened" && subj instanceof Ci.nsIDOMWindow) {
+      if (topic == "domwindowopened") {
         subj.addEventListener("load", function() {
           if (subj.document.documentURI ==
               "chrome://global/content/resetProfile.xul") {
             Services.ww.unregisterNotification(onOpen);
             ok(true, "Observed search manager window open");
             is(subj.opener, window,
                "Reset Firefox event opened a reset profile window.");
             subj.close();
--- a/browser/modules/BrowserUsageTelemetry.jsm
+++ b/browser/modules/BrowserUsageTelemetry.jsm
@@ -646,21 +646,16 @@ let BrowserUsageTelemetry = {
     this._recordTabCount(tabCount);
   },
 
   /**
    * Tracks the window count and registers the listeners for the tab count.
    * @param{Object} win The window object.
    */
   _onWindowOpen(win) {
-    // Make sure to have a |nsIDOMWindow|.
-    if (!(win instanceof Ci.nsIDOMWindow)) {
-      return;
-    }
-
     let onLoad = () => {
       win.removeEventListener("load", onLoad);
 
       // Ignore non browser windows.
       if (win.document.documentElement.getAttribute("windowtype") != "navigator:browser") {
         return;
       }
 
--- a/devtools/server/actors/addon.js
+++ b/devtools/server/actors/addon.js
@@ -210,17 +210,17 @@ BrowserAddonActor.prototype = {
       let metadata = Cu.getSandboxMetadata(global);
       if (metadata) {
         return metadata.addonID === this.id;
       }
     } catch (e) {
       // ignore
     }
 
-    if (global instanceof Ci.nsIDOMWindow) {
+    if (Window.isInstance(global)) {
       return global.document.nodePrincipal.addonId == this.id;
     }
 
     return false;
   },
 
   /**
    * Override the eligibility check for scripts and sources to make
--- a/devtools/server/actors/object/previewers.js
+++ b/devtools/server/actors/object/previewers.js
@@ -513,22 +513,22 @@ previewers.Object = [
     };
     return true;
   },
 
   function ObjectWithURL({obj, hooks}, grip, rawObj) {
     if (isWorker || !rawObj || !(obj.class == "CSSImportRule" ||
                                  obj.class == "CSSStyleSheet" ||
                                  obj.class == "Location" ||
-                                 rawObj instanceof Ci.nsIDOMWindow)) {
+                                 Window.isInstance(rawObj))) {
       return false;
     }
 
     let url;
-    if (rawObj instanceof Ci.nsIDOMWindow && rawObj.location) {
+    if (Window.isInstance(rawObj) && rawObj.location) {
       url = rawObj.location.href;
     } else if (rawObj.href) {
       url = rawObj.href;
     } else {
       return false;
     }
 
     grip.preview = {
--- a/devtools/server/actors/source.js
+++ b/devtools/server/actors/source.js
@@ -388,17 +388,17 @@ let SourceActor = ActorClassWithSpec(sou
       // for the document.  Without this check, the cache may return stale data
       // that doesn't match the document shown in the browser.
       let loadFromCache = this.isInlineSource && this.isCacheEnabled;
 
       // Fetch the sources with the same principal as the original document
       let win = this.threadActor._parent.window;
       let principal, cacheKey;
       // On xpcshell, we don't have a window but a Sandbox
-      if (!isWorker && win instanceof Ci.nsIDOMWindow) {
+      if (!isWorker && Window.isInstance(win)) {
         let webNav = win.QueryInterface(Ci.nsIInterfaceRequestor)
                         .getInterface(Ci.nsIWebNavigation);
         let channel = webNav.currentDocumentChannel;
         principal = channel.loadInfo.loadingPrincipal;
 
         // Retrieve the cacheKey in order to load POST requests from cache
         // Note that chrome:// URLs don't support this interface.
         if (loadFromCache &&
--- a/devtools/server/actors/webconsole.js
+++ b/devtools/server/actors/webconsole.js
@@ -643,17 +643,17 @@ WebConsoleActor.prototype =
           }
           startedListeners.push(listener);
           break;
         case "FileActivity":
           // Workers don't support this message type
           if (isWorker) {
             break;
           }
-          if (this.window instanceof Ci.nsIDOMWindow) {
+          if (Window.isInstance(this.window)) {
             if (!this.consoleProgressListener) {
               this.consoleProgressListener =
                 new ConsoleProgressListener(this.window, this);
             }
             this.consoleProgressListener.startMonitor(this.consoleProgressListener
                                                       .MONITOR_FILE_ACTIVITY);
             startedListeners.push(listener);
           }
--- a/devtools/server/actors/webconsole/utils.js
+++ b/devtools/server/actors/webconsole/utils.js
@@ -469,17 +469,17 @@ WebConsoleCommands._registerOriginal("cd
   }
 
   if (typeof window == "string") {
     window = owner.window.document.querySelector(window);
   }
   if (Element.isInstance(window) && window.contentWindow) {
     window = window.contentWindow;
   }
-  if (!(window instanceof Ci.nsIDOMWindow)) {
+  if (!Window.isInstance(window)) {
     owner.helperResult = {
       type: "error",
       message: "cdFunctionInvalidArgument"
     };
     return;
   }
 
   owner.consoleActor.evalWindow = window;
--- a/devtools/server/actors/webextension.js
+++ b/devtools/server/actors/webextension.js
@@ -342,17 +342,17 @@ webExtensionChildPrototype._allowSource 
 
 /**
  * Return true if the given global is associated with this addon and should be
  * added as a debuggee, false otherwise.
  */
 webExtensionChildPrototype._shouldAddNewGlobalAsDebuggee = function(newGlobal) {
   const global = unwrapDebuggerObjectGlobal(newGlobal);
 
-  if (global instanceof Ci.nsIDOMWindow) {
+  if (Window.isInstance(global)) {
     try {
       global.document;
     } catch (e) {
       // The global might be a sandbox with a window object in its proto chain. If the
       // window navigated away since the sandbox was created, it can throw a security
       // exception during this property check as the sandbox no longer has access to
       // its own proto.
       return false;
--- a/devtools/shared/builtin-modules.js
+++ b/devtools/shared/builtin-modules.js
@@ -37,16 +37,17 @@ const {
   Event,
   FileReader,
   FormData,
   indexedDB,
   InspectorUtils,
   TextDecoder,
   TextEncoder,
   URL,
+  Window,
   XMLHttpRequest,
 } = Cu.Sandbox(CC("@mozilla.org/systemprincipal;1", "nsIPrincipal")(), {
   wantGlobalProperties: [
     "atob",
     "btoa",
     "ChromeUtils",
     "CSS",
     "CSSRule",
@@ -55,16 +56,17 @@ const {
     "Event",
     "FileReader",
     "FormData",
     "indexedDB",
     "TextDecoder",
     "TextEncoder",
     "InspectorUtils",
     "URL",
+    "Window",
     "XMLHttpRequest",
   ]
 });
 
 /**
  * Defines a getter on a specified object that will be created upon first use.
  *
  * @param object
@@ -283,16 +285,17 @@ exports.globals = {
   },
   Node: Ci.nsIDOMNode,
   reportError: Cu.reportError,
   StructuredCloneHolder,
   TextDecoder,
   TextEncoder,
   URL,
   XMLHttpRequest,
+  Window,
 };
 // DevTools loader copy globals property descriptors on each module global
 // object so that we have to memoize them from here in order to instantiate each
 // global only once.
 // `globals` is a cache object on which we put all global values
 // and we set getters on `exports.globals` returning `globals` values.
 let globals = {};
 function lazyGlobal(name, getter) {
--- a/devtools/shared/layout/utils.js
+++ b/devtools/shared/layout/utils.js
@@ -674,17 +674,17 @@ exports.getViewportDimensions = getViewp
  * @return {DOMWindow}
  */
 function getWindowFor(node) {
   if (node instanceof Ci.nsIDOMNode) {
     if (node.nodeType === node.DOCUMENT_NODE) {
       return node.defaultView;
     }
     return node.ownerDocument.defaultView;
-  } else if (node instanceof Ci.nsIDOMWindow) {
+  } else if (Window.isInstance(node)) {
     return node;
   }
   return null;
 }
 
 /**
  * Synchronously loads a style sheet from `uri` and adds it to the list of
  * additional style sheets of the document.
--- a/toolkit/components/downloads/DownloadCore.jsm
+++ b/toolkit/components/downloads/DownloadCore.jsm
@@ -44,16 +44,18 @@ XPCOMUtils.defineLazyServiceGetter(this,
            Ci.nsPIExternalAppLauncher);
 XPCOMUtils.defineLazyServiceGetter(this, "gExternalHelperAppService",
            "@mozilla.org/uriloader/external-helper-app-service;1",
            Ci.nsIExternalHelperAppService);
 XPCOMUtils.defineLazyServiceGetter(this, "gPrintSettingsService",
            "@mozilla.org/gfx/printsettings-service;1",
            Ci.nsIPrintSettingsService);
 
+Cu.importGlobalProperties(["Window"]);
+
 /* global DownloadIntegration */
 Integration.downloads.defineModuleGetter(this, "DownloadIntegration",
             "resource://gre/modules/DownloadIntegration.jsm");
 
 const BackgroundFileSaverStreamListener = Components.Constructor(
       "@mozilla.org/network/background-file-saver;1?mode=streamlistener",
       "nsIBackgroundFileSaver");
 
@@ -1307,17 +1309,17 @@ this.DownloadSource.prototype = {
  */
 this.DownloadSource.fromSerializable = function(aSerializable) {
   let source = new DownloadSource();
   if (isString(aSerializable)) {
     // Convert String objects to primitive strings at this point.
     source.url = aSerializable.toString();
   } else if (aSerializable instanceof Ci.nsIURI) {
     source.url = aSerializable.spec;
-  } else if (aSerializable instanceof Ci.nsIDOMWindow) {
+  } else if (Window.isInstance(aSerializable)) {
     source.url = aSerializable.location.href;
     source.isPrivate = PrivateBrowsingUtils.isContentWindowPrivate(aSerializable);
     source.windowRef = Cu.getWeakReference(aSerializable);
   } else {
     // Convert String objects to primitive strings at this point.
     source.url = aSerializable.url.toString();
     if ("isPrivate" in aSerializable) {
       source.isPrivate = aSerializable.isPrivate;
--- a/toolkit/content/browser-content.js
+++ b/toolkit/content/browser-content.js
@@ -186,17 +186,17 @@ var ClickEventHandler = {
     // (which we use below) is null. Autoscrolling is broken in Print
     // Preview anyways (see bug 1393494), so just don't start it at all.
     if (!content.performance)
       return;
 
     let domUtils = content.QueryInterface(Ci.nsIInterfaceRequestor)
                           .getInterface(Ci.nsIDOMWindowUtils);
     let scrollable = this._scrollable;
-    if (scrollable instanceof Ci.nsIDOMWindow) {
+    if (Window.isInstance(scrollable)) {
       // getViewId() needs an element to operate on.
       scrollable = scrollable.document.documentElement;
     }
     this._scrollId = null;
     try {
       this._scrollId = domUtils.getViewId(scrollable);
     } catch (e) {
       // No view ID - leave this._scrollId as null. Receiving side will check.
@@ -1151,22 +1151,22 @@ addMessageListener("WebChannelMessageToC
   if (e.data) {
     // e.objects.eventTarget will be defined if sending a response to
     // a WebChannelMessageToChrome event. An unsolicited send
     // may not have an eventTarget defined, in this case send to the
     // main content window.
     let eventTarget = e.objects.eventTarget || content;
 
     // Use nodePrincipal if available, otherwise fallback to document principal.
-    let targetPrincipal = eventTarget instanceof Ci.nsIDOMWindow ? eventTarget.document.nodePrincipal : eventTarget.nodePrincipal;
+    let targetPrincipal = Window.isInstance(eventTarget) ? eventTarget.document.nodePrincipal : eventTarget.nodePrincipal;
 
     if (e.principal.subsumes(targetPrincipal)) {
       // If eventTarget is a window, use it as the targetWindow, otherwise
       // find the window that owns the eventTarget.
-      let targetWindow = eventTarget instanceof Ci.nsIDOMWindow ? eventTarget : eventTarget.ownerGlobal;
+      let targetWindow = Window.isInstance(eventTarget) ? eventTarget : eventTarget.ownerGlobal;
 
       eventTarget.dispatchEvent(new targetWindow.CustomEvent("WebChannelMessageToContent", {
         detail: Cu.cloneInto({
           id: e.data.id,
           message: e.data.message,
         }, targetWindow),
       }));
     } else {
--- a/toolkit/modules/addons/WebRequestContent.js
+++ b/toolkit/modules/addons/WebRequestContent.js
@@ -133,17 +133,17 @@ var ContentPolicy = {
       // Firefox loads an iframe, it sets |node| to the <iframe>
       // element, whose window is the parent window. We adopt the
       // Chrome behavior here.
       node = node.contentWindow;
     }
 
     if (node) {
       let window;
-      if (node instanceof Ci.nsIDOMWindow) {
+      if (node.Window && node instanceof node.Window) {
         window = node;
       } else {
         let doc;
         if (node.ownerDocument) {
           doc = node.ownerDocument;
         } else {
           doc = node;
         }