Bug 1595927 - Remove XPCOM gunk around RemoteWebNavigation creation. r=mconley
authorKris Maglione <maglione.k@gmail.com>
Fri, 15 Nov 2019 01:23:40 +0000
changeset 502097 cab5d681291405a636d403d0cb9bf5359f58afbb
parent 502096 c72321ba48b8af4a8a4e36ee6212121429de6ce9
child 502098 e04f7fc87005084851805cc6f78a2b277cb77a46
push id114172
push userdluca@mozilla.com
push dateTue, 19 Nov 2019 11:31:10 +0000
treeherdermozilla-inbound@b5c5ba07d3db [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmconley
bugs1595927
milestone72.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1595927 - Remove XPCOM gunk around RemoteWebNavigation creation. r=mconley It just adds a lot of unnecessary overhead and indirection. Differential Revision: https://phabricator.services.mozilla.com/D52753
devtools/client/responsive/browser/swap.js
devtools/client/responsive/browser/tunnel.js
toolkit/components/remotebrowserutils/RemoteWebNavigation.jsm
toolkit/components/remotebrowserutils/components.conf
toolkit/components/remotebrowserutils/moz.build
toolkit/content/widgets/browser-custom-element.js
--- a/devtools/client/responsive/browser/swap.js
+++ b/devtools/client/responsive/browser/swap.js
@@ -450,18 +450,18 @@ function addXULBrowserDecorations(browse
   }
 
   // It's not necessary for these to actually do anything.  These properties are
   // swapped between browsers in browser.js's `swapDocShells`, and then their
   // `swapBrowser` methods are called, so we define them here for that to work
   // without errors.  During the swap process above, these will move from the
   // the new inner browser to the original tab's browser (step 4) and then to
   // the temporary container tab's browser (step 7), which is then closed.
-  if (browser._remoteWebNavigationImpl == undefined) {
-    browser._remoteWebNavigationImpl = {
+  if (browser._remoteWebNavigation == undefined) {
+    browser._remoteWebNavigation = {
       swapBrowser() {},
     };
   }
   if (browser._remoteWebProgressManager == undefined) {
     browser._remoteWebProgressManager = {
       swapBrowser() {},
       get progressListeners() {
         return [];
--- a/devtools/client/responsive/browser/tunnel.js
+++ b/devtools/client/responsive/browser/tunnel.js
@@ -106,18 +106,18 @@ function tunnelToInnerBrowser(outer, inn
         inner._documentURI = outer._documentURI;
         inner._documentContentType = outer._documentContentType;
         inner._contentTitle = outer._contentTitle;
         inner._characterSet = outer._characterSet;
         inner._contentPrincipal = outer._contentPrincipal;
         inner._imageDocument = outer._imageDocument;
         inner._isSyntheticDocument = outer._isSyntheticDocument;
         inner._innerWindowID = outer._innerWindowID;
-        inner._remoteWebNavigationImpl._currentURI =
-          outer._remoteWebNavigationImpl._currentURI;
+        inner._remoteWebNavigation._currentURI =
+          outer._remoteWebNavigation._currentURI;
       }
     },
 
     // We do not need an onSecurityChange handler since the remote security UI
     // has been copied from the inner (remote) browser to the outer (non-remote)
     // browser and they share it.
 
     QueryInterface: ChromeUtils.generateQI([
@@ -224,19 +224,18 @@ function tunnelToInnerBrowser(outer, inn
       }
 
       // Replace the `webNavigation` object with our own version which tries to use
       // mozbrowser APIs where possible.  This replaces the webNavigation object that the
       // remote browser binding creates.  We do not care about it's original value
       // because stop() will remove the browser binding and these will no longer bee
       // used.
       const webNavigation = new BrowserElementWebNavigation(inner);
-      webNavigation.copyStateFrom(inner._remoteWebNavigationImpl);
+      webNavigation.copyStateFrom(inner._remoteWebNavigation);
       outer._remoteWebNavigation = webNavigation;
-      outer._remoteWebNavigationImpl = webNavigation;
 
       // Now that we've flipped to the remote browser mode, add `progressListener`
       // onto the remote version of `webProgress`.  Normally tabbrowser.xml does this step
       // when it creates a new browser, etc.  Since we manually changed the mode
       // above, it caused a fresh webProgress object to be created which does not have any
       // listeners added.  So, we get the listener that gBrowser is using for the tab and
       // reattach it here.
       const tab = gBrowser.getTabForBrowser(outer);
--- a/toolkit/components/remotebrowserutils/RemoteWebNavigation.jsm
+++ b/toolkit/components/remotebrowserutils/RemoteWebNavigation.jsm
@@ -9,76 +9,72 @@ ChromeUtils.defineModuleGetter(
   "resource://gre/modules/Services.jsm"
 );
 ChromeUtils.defineModuleGetter(
   this,
   "PrivateBrowsingUtils",
   "resource://gre/modules/PrivateBrowsingUtils.jsm"
 );
 
-function RemoteWebNavigation() {
-  this.wrappedJSObject = this;
-  this._cancelContentJSEpoch = 1;
-}
-
-RemoteWebNavigation.prototype = {
-  classDescription: "nsIWebNavigation for remote browsers",
-  classID: Components.ID("{4b56964e-cdf3-4bb8-830c-0e2dad3f4ebd}"),
-  contractID: "@mozilla.org/remote-web-navigation;1",
-
-  QueryInterface: ChromeUtils.generateQI([Ci.nsIWebNavigation]),
+class RemoteWebNavigation {
+  constructor(aBrowser) {
+    this._browser = aBrowser;
+    this._cancelContentJSEpoch = 1;
+    this._currentURI = null;
+    this.canGoBack = false;
+    this.canGoForward = false;
+    this.referringURI = null;
+    this.wrappedJSObject = this;
+  }
 
   swapBrowser(aBrowser) {
     this._browser = aBrowser;
-  },
+  }
 
-  canGoBack: false,
-  canGoForward: false,
   goBack() {
     let cancelContentJSEpoch = this._cancelContentJSEpoch++;
     this._browser.frameLoader.remoteTab.maybeCancelContentJSExecution(
       Ci.nsIRemoteTab.NAVIGATE_BACK,
       { epoch: cancelContentJSEpoch }
     );
     this._sendMessage("WebNavigation:GoBack", { cancelContentJSEpoch });
-  },
+  }
   goForward() {
     let cancelContentJSEpoch = this._cancelContentJSEpoch++;
     this._browser.frameLoader.remoteTab.maybeCancelContentJSExecution(
       Ci.nsIRemoteTab.NAVIGATE_FORWARD,
       { epoch: cancelContentJSEpoch }
     );
     this._sendMessage("WebNavigation:GoForward", { cancelContentJSEpoch });
-  },
+  }
   gotoIndex(aIndex) {
     let cancelContentJSEpoch = this._cancelContentJSEpoch++;
     this._browser.frameLoader.remoteTab.maybeCancelContentJSExecution(
       Ci.nsIRemoteTab.NAVIGATE_INDEX,
       { index: aIndex, epoch: cancelContentJSEpoch }
     );
     this._sendMessage("WebNavigation:GotoIndex", {
       index: aIndex,
       cancelContentJSEpoch,
     });
-  },
+  }
   loadURI(aURI, aLoadURIOptions) {
     let uri;
     try {
-      let fixup = Cc["@mozilla.org/docshell/urifixup;1"].getService();
-      let fixupFlags = fixup.webNavigationFlagsToFixupFlags(
+      let fixupFlags = Services.uriFixup.webNavigationFlagsToFixupFlags(
         aURI,
         aLoadURIOptions.loadFlags
       );
       let isBrowserPrivate = PrivateBrowsingUtils.isBrowserPrivate(
         this._browser
       );
       if (isBrowserPrivate) {
         fixupFlags |= Services.uriFixup.FIXUP_FLAG_PRIVATE_CONTEXT;
       }
-      uri = fixup.createFixupURI(aURI, fixupFlags);
+      uri = Services.uriFixup.createFixupURI(aURI, fixupFlags);
 
       // We know the url is going to be loaded, let's start requesting network
       // connection before the content process asks.
       // Note that we might have already setup the speculative connection in
       // some cases, especially when the url is from location bar or its popup
       // menu.
       if (uri.schemeIs("http") || uri.schemeIs("https")) {
         let principal = aLoadURIOptions.triggeringPrincipal;
@@ -102,64 +98,72 @@ RemoteWebNavigation.prototype = {
       // reason (such as failing to parse the URI), just ignore it.
     }
 
     let cancelContentJSEpoch = this._cancelContentJSEpoch++;
     this._browser.frameLoader.remoteTab.maybeCancelContentJSExecution(
       Ci.nsIRemoteTab.NAVIGATE_URL,
       { uri, epoch: cancelContentJSEpoch }
     );
-    aLoadURIOptions.cancelContentJSEpoch = cancelContentJSEpoch;
-    this._browser.frameLoader.browsingContext.loadURI(aURI, aLoadURIOptions);
-  },
+    this._browser.frameLoader.browsingContext.loadURI(aURI, {
+      ...aLoadURIOptions,
+      cancelContentJSEpoch,
+    });
+  }
   setOriginAttributesBeforeLoading(aOriginAttributes) {
     this._sendMessage("WebNavigation:SetOriginAttributes", {
       originAttributes: aOriginAttributes,
     });
-  },
+  }
   reload(aReloadFlags) {
     this._sendMessage("WebNavigation:Reload", { loadFlags: aReloadFlags });
-  },
+  }
   stop(aStopFlags) {
     this._sendMessage("WebNavigation:Stop", { loadFlags: aStopFlags });
-  },
+  }
 
   get document() {
     return this._browser.contentDocument;
-  },
+  }
 
-  _currentURI: null,
   get currentURI() {
     if (!this._currentURI) {
       this._currentURI = Services.io.newURI("about:blank");
     }
-
     return this._currentURI;
-  },
+  }
   set currentURI(aURI) {
     // Bug 1498600 verify usages of systemPrincipal here
     let loadURIOptions = {
       triggeringPrincipal: Services.scriptSecurityManager.getSystemPrincipal(),
     };
     this.loadURI(aURI.spec, loadURIOptions);
-  },
-
-  referringURI: null,
+  }
 
   // Bug 1233803 - accessing the sessionHistory of remote browsers should be
   // done in content scripts.
   get sessionHistory() {
-    throw Cr.NS_ERROR_NOT_IMPLEMENTED;
-  },
+    throw new Components.Exception(
+      "Not implemented",
+      Cr.NS_ERROR_NOT_IMPLEMENTED
+    );
+  }
   set sessionHistory(aValue) {
-    throw Cr.NS_ERROR_NOT_IMPLEMENTED;
-  },
+    throw new Components.Exception(
+      "Not implemented",
+      Cr.NS_ERROR_NOT_IMPLEMENTED
+    );
+  }
 
   _sendMessage(aMessage, aData) {
     try {
       this._browser.sendMessageToActor(aMessage, aData, "WebNavigation");
     } catch (e) {
       Cu.reportError(e);
     }
-  },
-};
+  }
+}
+
+RemoteWebNavigation.prototype.QueryInterface = ChromeUtils.generateQI([
+  Ci.nsIWebNavigation,
+]);
 
 var EXPORTED_SYMBOLS = ["RemoteWebNavigation"];
deleted file mode 100644
--- a/toolkit/components/remotebrowserutils/components.conf
+++ /dev/null
@@ -1,15 +0,0 @@
-# -*- Mode: python; indent-tabs-mode: nil; tab-width: 40 -*-
-# vim: set filetype=python:
-# This Source Code Form is subject to the terms of the Mozilla Public
-# License, v. 2.0. If a copy of the MPL was not distributed with this
-# file, You can obtain one at http://mozilla.org/MPL/2.0/.
-
-Classes = [
-    {
-        'cid': '{4b56964e-cdf3-4bb8-830c-0e2dad3f4ebd}',
-        'contract_ids': ['@mozilla.org/remote-web-navigation;1'],
-        'jsm': 'resource://gre/modules/RemoteWebNavigation.jsm',
-        'constructor': 'RemoteWebNavigation',
-        'processes': ProcessSelector.MAIN_PROCESS_ONLY,
-    },
-]
--- a/toolkit/components/remotebrowserutils/moz.build
+++ b/toolkit/components/remotebrowserutils/moz.build
@@ -6,13 +6,9 @@
 
 with Files('**'):
     BUG_COMPONENT = ('Core', 'DOM: Navigation')
 
 EXTRA_JS_MODULES += [
     'RemoteWebNavigation.jsm',
 ]
 
-XPCOM_MANIFESTS += [
-    'components.conf',
-]
-
 BROWSER_CHROME_MANIFESTS += ['tests/browser/browser.ini']
--- a/toolkit/content/widgets/browser-custom-element.js
+++ b/toolkit/content/widgets/browser-custom-element.js
@@ -23,16 +23,22 @@
   );
 
   ChromeUtils.defineModuleGetter(
     LazyModules,
     "E10SUtils",
     "resource://gre/modules/E10SUtils.jsm"
   );
 
+  ChromeUtils.defineModuleGetter(
+    LazyModules,
+    "RemoteWebNavigation",
+    "resource://gre/modules/RemoteWebNavigation.jsm"
+  );
+
   const elementsToDestroyOnUnload = new Set();
 
   window.addEventListener(
     "unload",
     () => {
       for (let element of elementsToDestroyOnUnload.values()) {
         element.destroy();
       }
@@ -1251,26 +1257,19 @@
       this.resetFields();
       this.mInitialized = true;
       if (this.isRemoteBrowser) {
         /*
          * Don't try to send messages from this function. The message manager for
          * the <browser> element may not be initialized yet.
          */
 
-        this._remoteWebNavigation = Cc[
-          "@mozilla.org/remote-web-navigation;1"
-        ].createInstance(Ci.nsIWebNavigation);
-        this._remoteWebNavigationImpl = this._remoteWebNavigation.wrappedJSObject;
-        this._remoteWebNavigationImpl.swapBrowser(this);
+        this._remoteWebNavigation = new LazyModules.RemoteWebNavigation(this);
 
         // Initialize contentPrincipal to the about:blank principal for this loadcontext
-        let { Services } = ChromeUtils.import(
-          "resource://gre/modules/Services.jsm"
-        );
         let aboutBlank = Services.io.newURI("about:blank");
         let ssm = Services.scriptSecurityManager;
         this._contentPrincipal = ssm.getLoadContextContentPrincipal(
           aboutBlank,
           this.loadContext
         );
         // CSP for about:blank is null; if we ever change _contentPrincipal above,
         // we should re-evaluate the CSP here.
@@ -1574,19 +1573,18 @@
         if (aContentType != null) {
           this._documentContentType = aContentType;
         }
       }
     }
 
     updateWebNavigationForLocationChange(aCanGoBack, aCanGoForward) {
       if (this.isRemoteBrowser && this.messageManager) {
-        let remoteWebNav = this._remoteWebNavigationImpl;
-        remoteWebNav.canGoBack = aCanGoBack;
-        remoteWebNav.canGoForward = aCanGoForward;
+        this._remoteWebNavigation.canGoBack = aCanGoBack;
+        this._remoteWebNavigation.canGoForward = aCanGoForward;
       }
     }
 
     updateForLocationChange(
       aLocation,
       aCharset,
       aMayEnableCharacterEncodingMenu,
       aCharsetAutodetected,
@@ -1609,17 +1607,17 @@
           this._mayEnableCharacterEncodingMenu = aMayEnableCharacterEncodingMenu;
           this._charsetAutodetected = aCharsetAutodetected;
         }
 
         if (aContentType != null) {
           this._documentContentType = aContentType;
         }
 
-        this._remoteWebNavigationImpl._currentURI = aLocation;
+        this._remoteWebNavigation._currentURI = aLocation;
         this._documentURI = aDocumentURI;
         this._contentTitle = aTitle;
         this._imageDocument = null;
         this._contentPrincipal = aContentPrincipal;
         this._contentStoragePrincipal = aContentStoragePrincipal;
         this._contentBlockingAllowListPrincipal = aContentBlockingAllowListPrincipal;
         this._csp = aCSP;
         this._referrerInfo = aReferrerInfo;
@@ -1628,18 +1626,18 @@
         this._contentRequestContextID = aHaveRequestContextID
           ? aRequestContextID
           : null;
       }
     }
 
     purgeSessionHistory() {
       if (this.isRemoteBrowser) {
-        this._remoteWebNavigationImpl.canGoBack = false;
-        this._remoteWebNavigationImpl.canGoForward = false;
+        this._remoteWebNavigation.canGoBack = false;
+        this._remoteWebNavigation.canGoForward = false;
       }
       try {
         this.sendMessageToActor(
           "Browser:PurgeSessionHistory",
           {},
           "PurgeSessionHistory",
           true
         );
@@ -1952,17 +1950,16 @@
       // because these notifications are dispatched again once the docshells
       // are swapped.
       var fieldsToSwap = ["_webBrowserFind"];
 
       if (this.isRemoteBrowser) {
         fieldsToSwap.push(
           ...[
             "_remoteWebNavigation",
-            "_remoteWebNavigationImpl",
             "_remoteWebProgressManager",
             "_remoteWebProgress",
             "_remoteFinder",
             "_securityUI",
             "_documentURI",
             "_documentContentType",
             "_contentTitle",
             "_characterSet",
@@ -2005,18 +2002,18 @@
 
       if (!this.isRemoteBrowser) {
         // Null the current nsITypeAheadFind instances so that they're
         // lazily re-created on access. We need to do this because they
         // might have attached the wrong docShell.
         this._fastFind = aOtherBrowser._fastFind = null;
       } else {
         // Rewire the remote listeners
-        this._remoteWebNavigationImpl.swapBrowser(this);
-        aOtherBrowser._remoteWebNavigationImpl.swapBrowser(aOtherBrowser);
+        this._remoteWebNavigation.swapBrowser(this);
+        aOtherBrowser._remoteWebNavigation.swapBrowser(aOtherBrowser);
 
         if (
           this._remoteWebProgressManager &&
           aOtherBrowser._remoteWebProgressManager
         ) {
           this._remoteWebProgressManager.swapBrowser(this);
           aOtherBrowser._remoteWebProgressManager.swapBrowser(aOtherBrowser);
         }