Bug 1595927 - Remove XPCOM gunk around RemoteWebNavigation creation. r=mconley
☠☠ backed out by f7d3a6261cb3 ☠ ☠
authorKris Maglione <maglione.k@gmail.com>
Thu, 14 Nov 2019 19:09:22 +0000
changeset 502005 70304898d836c345e2ad3583ed148bb5ba0797d9
parent 502004 93deb240edb2863e03c5756341d03a64361e5e9a
child 502006 868a55980ae8e0f6bda936e9a5c0cbb2311a5717
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
toolkit/components/remotebrowserutils/RemoteWebNavigation.jsm
toolkit/components/remotebrowserutils/components.conf
toolkit/components/remotebrowserutils/moz.build
toolkit/content/widgets/browser-custom-element.js
--- 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);
         }