Bug 1317101 - Part 7c: Run extension popups in a remote browser. r=aswan

MozReview-Commit-ID: CATeESBwj1J

--HG--
extra : rebase_source : c83dbcabbafbe2884746d282de78d23f5e0edff6
extra : source : b7892d3fb0ca5268a252377ecbb44dfb1d289500

browser/components/extensions/ext-utils.js

@@ -123,11 +123,13 @@ class BasePopup {
     this.destroyed = true;
     this.browserLoadedDeferred.reject(new Error("Popup destroyed"));
     return this.browserReady.then(() => {
-      this.destroyBrowser(this.browser);
+      this.destroyBrowser(this.browser, true);
       this.browser.remove();
 
+      if (this.viewNode) {
         this.viewNode.removeEventListener(this.DESTROY_EVENT, this);
         this.viewNode.style.maxHeight = "";
+      }
 
       if (this.panel) {
         this.panel.style.removeProperty("--arrowpanel-background");
@@ -141,16 +143,19 @@ class BasePopup {
     });
   }
 
-  destroyBrowser(browser) {
+  destroyBrowser(browser, finalize = false) {
     let mm = browser.messageManager;
     // If the browser has already been removed from the document, because the
-    // popup was closed externally, there will be no message manager here.
+    // popup was closed externally, there will be no message manager here, so
+    // just replace our receiveMessage method with a stub.
     if (mm) {
       mm.removeMessageListener("DOMTitleChanged", this);
       mm.removeMessageListener("Extension:BrowserBackgroundChanged", this);
       mm.removeMessageListener("Extension:BrowserContentLoaded", this);
       mm.removeMessageListener("Extension:BrowserResized", this);
       mm.removeMessageListener("Extension:DOMWindowClose", this);
+    } else if (finalize) {
+      this.receiveMessage = () => {};
     }
   }
 
@@ -213,53 +218,73 @@ class BasePopup {
   handleEvent(event) {
     switch (event.type) {
       case this.DESTROY_EVENT:
+        if (!this.destroyed) {
           this.destroy();
+        }
         break;
     }
   }
 
   createBrowser(viewNode, popupURL = null) {
     let document = viewNode.ownerDocument;
-    this.browser = document.createElementNS(XUL_NS, "browser");
-    this.browser.setAttribute("type", "content");
-    this.browser.setAttribute("disableglobalhistory", "true");
-    this.browser.setAttribute("transparent", "true");
-    this.browser.setAttribute("class", "webextension-popup-browser");
-    this.browser.setAttribute("webextension-view-type", "popup");
-    this.browser.setAttribute("tooltip", "aHTMLTooltip");
+    let browser = document.createElementNS(XUL_NS, "browser");
+    browser.setAttribute("type", "content");
+    browser.setAttribute("disableglobalhistory", "true");
+    browser.setAttribute("transparent", "true");
+    browser.setAttribute("class", "webextension-popup-browser");
+    browser.setAttribute("webextension-view-type", "popup");
+    browser.setAttribute("tooltip", "aHTMLTooltip");
+
+    if (this.extension.remote) {
+      browser.setAttribute("remote", "true");
+    }
 
     // We only need flex sizing for the sake of the slide-in sub-views of the
     // main menu panel, so that the browser occupies the full width of the view,
     // and also takes up any extra height that's available to it.
-    this.browser.setAttribute("flex", "1");
+    browser.setAttribute("flex", "1");
 
     // Note: When using noautohide panels, the popup manager will add width and
     // height attributes to the panel, breaking our resize code, if the browser
     // starts out smaller than 30px by 10px. This isn't an issue now, but it
     // will be if and when we popup debugging.
 
-    viewNode.appendChild(this.browser);
+    this.browser = browser;
 
-    extensions.emit("extension-browser-inserted", this.browser);
+    let readyPromise;
+    if (this.extension.remote) {
+      readyPromise = promiseEvent(browser, "XULFrameLoaderCreated");
+    } else {
+      readyPromise = promiseEvent(browser, "load");
+    }
 
-    let initBrowser = browser => {
+    viewNode.appendChild(browser);
+
+    extensions.emit("extension-browser-inserted", browser);
+
+    let setupBrowser = browser => {
       let mm = browser.messageManager;
       mm.addMessageListener("DOMTitleChanged", this);
       mm.addMessageListener("Extension:BrowserBackgroundChanged", this);
       mm.addMessageListener("Extension:BrowserContentLoaded", this);
       mm.addMessageListener("Extension:BrowserResized", this);
       mm.addMessageListener("Extension:DOMWindowClose", this, true);
+      return browser;
     };
 
     if (!popupURL) {
-      initBrowser(this.browser);
-      return this.browser;
+      // For remote browsers, we can't do any setup until the frame loader is
+      // created. Non-remote browsers get a message manager immediately, so
+      // there's no need to wait for the load event.
+      if (this.extension.remote) {
+        return readyPromise.then(() => setupBrowser(browser));
+      }
+      return setupBrowser(browser);
     }
 
-    return promiseEvent(this.browser, "load").then(() => {
-      initBrowser(this.browser);
-
-      let mm = this.browser.messageManager;
+    return readyPromise.then(() => {
+      setupBrowser(browser);
+      let mm = browser.messageManager;
 
       mm.loadFrameScript(
         "chrome://extensions/content/ext-browser-content.js", false);
@@ -272,7 +297,7 @@ class BasePopup {
         stylesheets: this.STYLESHEETS,
       });
 
-      this.browser.loadURI(popupURL);
+      browser.loadURI(popupURL);
     });
   }
 
@@ -363,12 +388,13 @@ class PanelPopup extends BasePopup {
   destroy() {
     super.destroy();
     this.viewNode.remove();
+    this.viewNode = null;
   }
 
   closePopup() {
     promisePopupShown(this.viewNode).then(() => {
-      // Make sure we're not already destroyed.
-      if (this.viewNode) {
+      // Make sure we're not already destroyed, or removed from the DOM.
+      if (this.viewNode && this.viewNode.hidePopup) {
         this.viewNode.hidePopup();
       }
     });
@@ -391,6 +417,7 @@ class ViewPopup extends BasePopup {
     this.ignoreResizes = true;
 
     this.attached = false;
+    this.shown = false;
     this.tempPanel = panel;
 
     this.browser.classList.add("webextension-preload-browser");
@@ -432,11 +459,13 @@ class ViewPopup extends BasePopup {
       }
 
       if (this.destroyed) {
+        CustomizableUI.hidePanelForNode(viewNode);
         return false;
       }
 
       this.attached = true;
 
+
       // Store the initial height of the view, so that we never resize menu panel
       // sub-views smaller than the initial height of the menu.
       this.viewHeight = this.viewNode.boxObject.height;
@@ -457,7 +486,7 @@ class ViewPopup extends BasePopup {
 
       // Create a new browser in the real popup.
       let browser = this.browser;
-      this.createBrowser(this.viewNode);
+      yield this.createBrowser(this.viewNode);
 
       this.browser.swapDocShells(browser);
       this.destroyBrowser(browser);
@@ -470,6 +499,14 @@ class ViewPopup extends BasePopup {
       this.tempPanel.remove();
       this.tempPanel = null;
 
+      this.shown = true;
+
+      if (this.destroyed) {
+        this.closePopup();
+        this.destroy();
+        return false;
+      }
+
       let event = new this.window.CustomEvent("WebExtPopupLoaded", {
         bubbles: true,
         detail: {extension: this.extension},
@@ -494,8 +531,10 @@ class ViewPopup extends BasePopup {
   }
 
   closePopup() {
-    if (this.attached) {
+    if (this.shown) {
       CustomizableUI.hidePanelForNode(this.viewNode);
+    } else if (this.attached) {
+      this.destroyed = true;
     } else {
       this.destroy();
     }

browser/components/extensions/test/browser/browser_ext_browserAction_popup.js

@@ -82,7 +82,7 @@ function* testInArea(area) {
           () => {
             browser.test.log(`Set popup to "c" and click browser action. Expect popup "c".`);
             browser.browserAction.setPopup({popup: "popup-c.html"});
-            sendClick({expectEvent: false, expectPopup: "c", closePopup: false});
+            sendClick({expectEvent: false, expectPopup: "c", waitUntilClosed: true});
           },
           () => {
             browser.test.log(`Set popup to "b" and click browser action. Expect popup "b".`);
@@ -120,7 +120,7 @@ function* testInArea(area) {
         let expect = {};
         sendClick = ({expectEvent, expectPopup, runNextTest, waitUntilClosed, closePopup}, message = "send-click") => {
           if (closePopup == undefined) {
-            closePopup = true;
+            closePopup = !expectEvent;
           }
 
           expect = {event: expectEvent, popup: expectPopup, runNextTest, waitUntilClosed, closePopup};
@@ -190,8 +190,11 @@ function* testInArea(area) {
       CustomizableUI.addWidgetToArea(widget.id, area);
     }
     if (expecting.waitUntilClosed) {
+      yield new Promise(resolve => setTimeout(resolve, 0));
+
       let panel = getBrowserActionPopup(extension);
       if (panel && panel.state != "closed") {
+        info("Popup is open. Waiting for close");
         yield promisePopupHidden(panel);
       }
     } else if (expecting.closePopupUsingWindow) {
@@ -204,9 +207,16 @@ function* testInArea(area) {
       yield promisePopupHidden(panel);
       ok(true, "Panel is closed");
     } else if (expecting.closePopup) {
+      if (!getBrowserActionPopup(extension)) {
+        info("Waiting for panel");
+        yield awaitExtensionPanel(extension);
+      }
+
+      info("Closing for panel");
       yield closeBrowserAction(extension);
     }
 
+    info("Starting next test");
     extension.sendMessage("next-test");
   }));
 

browser/components/extensions/test/browser/browser_ext_commands_execute_browser_action.js

@@ -82,6 +82,10 @@ function* testExecuteBrowserActionWithOptions(options = {}) {
 
   if (options.withPopup) {
     yield extension.awaitFinish("execute-browser-action-popup-opened");
+
+    if (!getBrowserActionPopup(extension)) {
+      yield awaitExtensionPanel(extension);
+    }
     yield closeBrowserAction(extension);
   } else {
     yield extension.awaitFinish("execute-browser-action-on-clicked-fired");