Bug 1645541, use outerWindowId from the windowGlobal instead of using the Browser:Init message, Also makes the BrowserTab actor specific to the main browser, r=mconley,remote-protocol-reviewers,maja_zf
authorNeil Deakin <neil@mozilla.com>
Wed, 17 Jun 2020 15:01:28 +0000
changeset 536127 660432a1429dd333d2cf4bdde6c025a140051118
parent 536126 758831c2e0976b544eb4ecdd04262c115f10cf17
child 536128 73823aa5538bfe0f3eff6807d8773740b0a7fd21
push id119251
push userneil@mozilla.com
push dateWed, 17 Jun 2020 16:06:24 +0000
treeherderautoland@660432a1429d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmconley, remote-protocol-reviewers, maja_zf
bugs1645541
milestone79.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 1645541, use outerWindowId from the windowGlobal instead of using the Browser:Init message, Also makes the BrowserTab actor specific to the main browser, r=mconley,remote-protocol-reviewers,maja_zf Differential Revision: https://phabricator.services.mozilla.com/D79609
browser/actors/BrowserTabParent.jsm
browser/base/content/tabbrowser.js
browser/components/BrowserGlue.jsm
browser/modules/BrowserWindowTracker.jsm
browser/modules/NewTabPagePreloading.jsm
remote/Sync.jsm
remote/targets/TargetList.jsm
toolkit/content/browser-child.js
toolkit/content/widgets/browser-custom-element.js
--- a/browser/actors/BrowserTabParent.jsm
+++ b/browser/actors/BrowserTabParent.jsm
@@ -1,49 +1,36 @@
 /* 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/. */
 "use strict";
 
 var EXPORTED_SYMBOLS = ["BrowserTabParent"];
 
+const { BrowserWindowTracker } = ChromeUtils.import(
+  "resource:///modules/BrowserWindowTracker.jsm"
+);
+
 class BrowserTabParent extends JSWindowActorParent {
   receiveMessage(message) {
     let browser = this.manager.browsingContext.embedderElement;
     if (!browser) {
       return; // Can happen sometimes if browser is being destroyed
     }
 
     if (browser.outerBrowser) {
       browser = browser.outerBrowser; // handle RDM mode
     }
 
     let gBrowser = browser.ownerGlobal.gBrowser;
 
-    if (!gBrowser) {
-      // Note: gBrowser might be null because this message might be received
-      // from the extension process. There's still an embedderElement involved,
-      // but it's the one coming from dummy.xul.
-      // This should probably be fixed by adding support to specifying "group: 'browsers"
-      // in the registerWindowActor options/. See bug 1557118.
-      return;
-    }
-
-    if (!gBrowser.tabpanels || !gBrowser.tabpanels.contains(browser)) {
-      // Note: This is ignoring browser elements related to extension pages that are not loaded
-      // as a browser tab (like sidebars, devtools panels and options pages embedded into
-      // about:addons, browserAction and pageAction popup panels.
-      // (Ideally we could call gBrowser.getTabForBrowser, but it returns undefined early in
-      // the tab browser creation and we would ignore browsers related to newly created tabs).
-      return;
-    }
-
     switch (message.name) {
       case "Browser:WindowCreated": {
         gBrowser.announceWindowCreated(browser, message.data.userContextId);
+        BrowserWindowTracker.windowCreated(browser);
         break;
       }
 
       case "Browser:FirstPaint": {
         browser.ownerGlobal.gBrowserInit._firstBrowserPaintDeferred.resolve();
         break;
       }
 
--- a/browser/base/content/tabbrowser.js
+++ b/browser/base/content/tabbrowser.js
@@ -47,20 +47,16 @@
         Services.prefs.getIntPref("browser.display.document_color_use") == 2
       ) {
         this.tabpanels.style.backgroundColor = Services.prefs.getCharPref(
           "browser.display.background_color"
         );
       }
 
       let messageManager = window.getGroupMessageManager("browsers");
-
-      if (gMultiProcessBrowser) {
-        messageManager.addMessageListener("Browser:Init", this);
-      }
       messageManager.addMessageListener("RefreshBlocker:Blocked", this);
 
       this._setFindbarData();
 
       XPCOMUtils.defineLazyModuleGetters(this, {
         E10SUtils: "resource://gre/modules/E10SUtils.jsm",
       });
 
@@ -4017,23 +4013,16 @@
         aOtherBrowser.docShellIsActive = this.shouldActivateDocShell(
           ourBrowser
         );
       }
 
       // Swap the docshells
       ourBrowser.swapDocShells(aOtherBrowser);
 
-      if (ourBrowser.isRemoteBrowser) {
-        // Switch outerWindowIDs for remote browsers.
-        let ourOuterWindowID = ourBrowser._outerWindowID;
-        ourBrowser._outerWindowID = aOtherBrowser._outerWindowID;
-        aOtherBrowser._outerWindowID = ourOuterWindowID;
-      }
-
       // Swap permanentKey properties.
       let ourPermanentKey = ourBrowser.permanentKey;
       ourBrowser.permanentKey = aOtherBrowser.permanentKey;
       aOtherBrowser.permanentKey = ourPermanentKey;
       aOurTab.permanentKey = ourBrowser.permanentKey;
       if (remoteBrowser) {
         let otherTab = remoteBrowser.getTabForBrowser(aOtherBrowser);
         if (otherTab) {
@@ -4066,19 +4055,27 @@
       }
       if (tmp) {
         aOtherBrowser.registeredOpenURI = tmp;
       }
     },
 
     announceWindowCreated(browser, userContextId) {
       let tab = this.getTabForBrowser(browser);
-      if (tab && userContextId) {
-        ContextualIdentityService.telemetry(userContextId);
-        tab.setUserContextId(userContextId);
+      if (tab) {
+        if (userContextId) {
+          ContextualIdentityService.telemetry(userContextId);
+          tab.setUserContextId(userContextId);
+        }
+
+        browser.sendMessageToActor(
+          "Browser:AppTab",
+          { isAppTab: tab.pinned },
+          "BrowserTab"
+        );
       }
 
       // We don't want to update the container icon and identifier if
       // this is not the selected browser.
       if (browser == gBrowser.selectedBrowser) {
         updateUserContextUIIndicator();
       }
     },
@@ -5109,29 +5106,16 @@
       }
     },
 
     receiveMessage(aMessage) {
       let data = aMessage.data;
       let browser = aMessage.target;
 
       switch (aMessage.name) {
-        case "Browser:Init": {
-          let tab = this.getTabForBrowser(browser);
-          if (!tab) {
-            return undefined;
-          }
-
-          browser.sendMessageToActor(
-            "Browser:AppTab",
-            { isAppTab: tab.pinned },
-            "BrowserTab"
-          );
-          break;
-        }
         case "RefreshBlocker:Blocked": {
           // The data object is expected to contain the following properties:
           //  - URI (string)
           //     The URI that a page is attempting to refresh or redirect to.
           //  - delay (int)
           //     The delay (in milliseconds) before the page was going to
           //     reload or redirect.
           //  - sameURI (bool)
--- a/browser/components/BrowserGlue.jsm
+++ b/browser/components/BrowserGlue.jsm
@@ -254,16 +254,18 @@ let JSWINDOWACTORS = {
 
       events: {
         DOMWindowCreated: {},
         MozAfterPaint: {},
         "MozDOMPointerLock:Entered": {},
         "MozDOMPointerLock:Exited": {},
       },
     },
+
+    messageManagerGroups: ["browsers"],
   },
 
   ClickHandler: {
     parent: {
       moduleURI: "resource:///actors/ClickHandlerParent.jsm",
     },
     child: {
       moduleURI: "resource:///actors/ClickHandlerChild.jsm",
--- a/browser/modules/BrowserWindowTracker.jsm
+++ b/browser/modules/BrowserWindowTracker.jsm
@@ -79,26 +79,16 @@ function _handleEvent(event) {
       WindowHelper.onActivate(event.target);
       break;
     case "unload":
       WindowHelper.removeWindow(event.currentTarget);
       break;
   }
 }
 
-function _handleMessage(message) {
-  let browser = message.target;
-  if (
-    message.name === "Browser:Init" &&
-    browser === browser.ownerGlobal.gBrowser.selectedBrowser
-  ) {
-    _updateCurrentContentOuterWindowID(browser);
-  }
-}
-
 function _trackWindowOrder(window) {
   if (window.windowState == window.STATE_MINIMIZED) {
     let firstMinimizedWindow = _trackedWindows.findIndex(
       w => w.windowState == w.STATE_MINIMIZED
     );
     if (firstMinimizedWindow == -1) {
       firstMinimizedWindow = _trackedWindows.length;
     }
@@ -121,38 +111,32 @@ var WindowHelper = {
     // Add event listeners
     TAB_EVENTS.forEach(function(event) {
       window.gBrowser.tabContainer.addEventListener(event, _handleEvent);
     });
     WINDOW_EVENTS.forEach(function(event) {
       window.addEventListener(event, _handleEvent);
     });
 
-    let messageManager = window.getGroupMessageManager("browsers");
-    messageManager.addMessageListener("Browser:Init", _handleMessage);
-
     _trackWindowOrder(window);
 
     // Update the selected tab's content outer window ID.
     _updateCurrentContentOuterWindowID(window.gBrowser.selectedBrowser);
   },
 
   removeWindow(window) {
     _untrackWindowOrder(window);
 
     // Remove the event listeners
     TAB_EVENTS.forEach(function(event) {
       window.gBrowser.tabContainer.removeEventListener(event, _handleEvent);
     });
     WINDOW_EVENTS.forEach(function(event) {
       window.removeEventListener(event, _handleEvent);
     });
-
-    let messageManager = window.getGroupMessageManager("browsers");
-    messageManager.removeMessageListener("Browser:Init", _handleMessage);
   },
 
   onActivate(window) {
     // If this window was the last focused window, we don't need to do anything
     if (window == _trackedWindows[0]) {
       return;
     }
 
@@ -183,16 +167,22 @@ this.BrowserWindowTracker = {
           PrivateBrowsingUtils.isWindowPrivate(win) == options.private)
       ) {
         return win;
       }
     }
     return null;
   },
 
+  windowCreated(browser) {
+    if (browser === browser.ownerGlobal.gBrowser.selectedBrowser) {
+      _updateCurrentContentOuterWindowID(browser);
+    }
+  },
+
   /**
    * Number of currently open browser windows.
    */
   get windowCount() {
     return _trackedWindows.length;
   },
 
   /**
--- a/browser/modules/NewTabPagePreloading.jsm
+++ b/browser/modules/NewTabPagePreloading.jsm
@@ -89,20 +89,16 @@ let NewTabPagePreloading = {
     // Don't call getPreloadedBrowser because it'll consume the browser:
     let oldBrowser = oldWin.gBrowser.preloadedBrowser;
     oldWin.gBrowser.preloadedBrowser = null;
 
     let newBrowser = this._createBrowser(window);
 
     oldBrowser.swapBrowsers(newBrowser);
 
-    // Switch outerWindowIDs for remote browsers.
-    if (newBrowser.isRemoteBrowser) {
-      newBrowser._outerWindowID = oldBrowser._outerWindowID;
-    }
     newBrowser.permanentKey = oldBrowser.permanentKey;
 
     oldWin.gBrowser.getPanel(oldBrowser).remove();
     return newBrowser;
   },
 
   maybeCreatePreloadedBrowser(window) {
     // If we're not enabled, have already got one, or are in a popup window,
--- a/remote/Sync.jsm
+++ b/remote/Sync.jsm
@@ -1,20 +1,15 @@
 /* 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/. */
 
 "use strict";
 
-var EXPORTED_SYMBOLS = [
-  "EventPromise",
-  "executeSoon",
-  "MessagePromise",
-  "PollPromise",
-];
+var EXPORTED_SYMBOLS = ["EventPromise", "executeSoon", "PollPromise"];
 
 const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
 
 const { TYPE_REPEATING_SLACK } = Ci.nsITimer;
 
 /**
  * Wait for a single event to be fired on a specific EventListener.
  *
@@ -93,41 +88,16 @@ function executeSoon(fn) {
   if (typeof fn != "function") {
     throw new TypeError();
   }
 
   Services.tm.dispatchToMainThread(fn);
 }
 
 /**
- * Awaits a single IPC message.
- *
- * @param {nsIMessageSender} target
- * @param {string} name
- *
- * @return {Promise}
- *
- * @throws {TypeError}
- *     If target is not an nsIMessageSender.
- */
-function MessagePromise(target, name) {
-  if (!(target instanceof Ci.nsIMessageSender)) {
-    throw new TypeError();
-  }
-
-  return new Promise(resolve => {
-    const onMessage = (...args) => {
-      target.removeMessageListener(name, onMessage);
-      resolve(...args);
-    };
-    target.addMessageListener(name, onMessage);
-  });
-}
-
-/**
  * Runs a Promise-like function off the main thread until it is resolved
  * through ``resolve`` or ``rejected`` callbacks.  The function is
  * guaranteed to be run at least once, irregardless of the timeout.
  *
  * The ``func`` is evaluated every ``interval`` for as long as its
  * runtime duration does not exceed ``interval``.  Evaluations occur
  * sequentially, meaning that evaluations of ``func`` are queued if
  * the runtime evaluation duration of ``func`` is greater than ``interval``.
--- a/remote/targets/TargetList.jsm
+++ b/remote/targets/TargetList.jsm
@@ -4,19 +4,16 @@
 
 "use strict";
 
 var EXPORTED_SYMBOLS = ["TargetList"];
 
 const { EventEmitter } = ChromeUtils.import(
   "resource://gre/modules/EventEmitter.jsm"
 );
-const { MessagePromise } = ChromeUtils.import(
-  "chrome://remote/content/Sync.jsm"
-);
 const { TabTarget } = ChromeUtils.import(
   "chrome://remote/content/targets/TabTarget.jsm"
 );
 const { MainProcessTarget } = ChromeUtils.import(
   "chrome://remote/content/targets/MainProcessTarget.jsm"
 );
 const { TabObserver } = ChromeUtils.import(
   "chrome://remote/content/observers/TargetObserver.jsm"
@@ -47,25 +44,17 @@ class TargetList {
    * each of them.
    */
   async watchForTabs() {
     if (this.tabObserver) {
       throw new Error("Targets is already watching for new tabs");
     }
     this.tabObserver = new TabObserver({ registerExisting: true });
     this.tabObserver.on("open", async (eventName, tab) => {
-      const browser = tab.linkedBrowser;
-      // The tab may just have been created and not fully initialized yet.
-      // Target class expects BrowserElement.browsingContext to be defined
-      // whereas it is asynchronously set by the custom element class.
-      // At least ensure that this property is set before instantiating the target.
-      if (!browser.browsingContext) {
-        await new MessagePromise(browser.messageManager, "Browser:Init");
-      }
-      const target = new TabTarget(this, browser);
+      const target = new TabTarget(this, tab.linkedBrowser);
       this.registerTarget(target);
     });
     this.tabObserver.on("close", (eventName, tab) => {
       const browser = tab.linkedBrowser;
       // Ignore the browsers that haven't had time to initialize.
       if (!browser.browsingContext) {
         return;
       }
--- a/toolkit/content/browser-child.js
+++ b/toolkit/content/browser-child.js
@@ -41,14 +41,8 @@ addMessageListener("BrowserElement:Creat
     content.document.nodePrincipal
   );
   partitionedPrincipal = BrowserUtils.principalWithMatchingOA(
     partitionedPrincipal,
     content.document.partitionedPrincipal
   );
   docShell.createAboutBlankContentViewer(principal, partitionedPrincipal);
 });
-
-// We may not get any responses to Browser:Init if the browser element
-// is torn down too quickly.
-var outerWindowID = docShell.outerWindowID;
-var browsingContextId = docShell.browsingContext.id;
-sendAsyncMessage("Browser:Init", { outerWindowID, browsingContextId });
--- a/toolkit/content/widgets/browser-custom-element.js
+++ b/toolkit/content/widgets/browser-custom-element.js
@@ -225,18 +225,16 @@
       this._webBrowserFind = null;
 
       this._finder = null;
 
       this._remoteFinder = null;
 
       this._fastFind = null;
 
-      this._outerWindowID = null;
-
       this._innerWindowID = null;
 
       this._lastSearchString = null;
 
       this._remoteWebNavigation = null;
 
       this._remoteWebProgress = null;
 
@@ -578,20 +576,17 @@
           Ci.nsITypeAheadFind
         );
         this._fastFind.init(this.docShell);
       }
       return this._fastFind;
     }
 
     get outerWindowID() {
-      if (this.isRemoteBrowser) {
-        return this._outerWindowID;
-      }
-      return this.docShell.outerWindowID;
+      return this.browsingContext?.currentWindowGlobal?.outerWindowId;
     }
 
     get innerWindowID() {
       if (this.isRemoteBrowser) {
         return this._innerWindowID;
       }
       try {
         return this.contentWindow.windowUtils.currentInnerWindowID;
@@ -1097,18 +1092,16 @@
         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.
         this._csp = null;
 
-        this.messageManager.addMessageListener("Browser:Init", this);
-
         let jsm = "resource://gre/modules/RemoteWebProgress.jsm";
         let { RemoteWebProgressManager } = ChromeUtils.import(jsm, {});
 
         let oldManager = this._remoteWebProgressManager;
         this._remoteWebProgressManager = new RemoteWebProgressManager(this);
         if (oldManager) {
           // We're transitioning from one remote type to another. This means that
           // the RemoteWebProgress listener is listening to the old message manager,
@@ -1211,29 +1204,16 @@
       this.mInitialized = false;
       this.lastURI = null;
 
       if (!this.isRemoteBrowser) {
         this.removeEventListener("pagehide", this.onPageHide, true);
       }
     }
 
-    receiveMessage(aMessage) {
-      if (this.isRemoteBrowser) {
-        const data = aMessage.data;
-        switch (aMessage.name) {
-          case "Browser:Init":
-            this._outerWindowID = data.outerWindowID;
-            break;
-          default:
-            break;
-        }
-      }
-    }
-
     get remoteWebProgressManager() {
       return this._remoteWebProgressManager;
     }
 
     updateForStateChange(aCharset, aDocumentURI, aContentType) {
       if (this.isRemoteBrowser && this.messageManager) {
         if (aCharset != null) {
           this._characterSet = aCharset;