Bug 1259093: Part 3 - Preload browserAction popups to prevent flicker during opening. r?Gijs r?jaws r?bwinton draft
authorKris Maglione <maglione.k@gmail.com>
Tue, 16 Aug 2016 18:21:53 -0700
changeset 401496 d58a2a6de72632459e3a9d9764c417bfb725fcd2
parent 401495 c07402726353cb395c827c1f66257ebcb817a1a7
child 528491 343ca4b57c70b590055e32a7e57f4b8234dce876
push id26462
push usermaglione.k@gmail.com
push dateWed, 17 Aug 2016 01:44:40 +0000
reviewersGijs, jaws, bwinton
bugs1259093
milestone51.0a1
Bug 1259093: Part 3 - Preload browserAction popups to prevent flicker during opening. r?Gijs r?jaws r?bwinton MozReview-Commit-ID: EpAKLV8VPTn
browser/components/extensions/ext-browserAction.js
browser/components/extensions/ext-utils.js
browser/components/extensions/test/browser/browser_ext_browserAction_popup.js
browser/components/extensions/test/browser/browser_ext_browserAction_popup_resize.js
browser/components/extensions/test/browser/browser_ext_browserAction_simple.js
browser/components/extensions/test/browser/browser_ext_currentWindow.js
browser/components/extensions/test/browser/browser_ext_getViews.js
browser/components/extensions/test/browser/browser_ext_popup_api_injection.js
browser/components/extensions/test/browser/browser_ext_popup_background.js
browser/components/extensions/test/browser/browser_ext_popup_corners.js
browser/components/extensions/test/browser/browser_ext_tabs_executeScript_good.js
browser/components/extensions/test/browser/head.js
toolkit/components/extensions/Extension.jsm
toolkit/components/extensions/ExtensionUtils.jsm
--- a/browser/components/extensions/ext-browserAction.js
+++ b/browser/components/extensions/ext-browserAction.js
@@ -1,42 +1,51 @@
 /* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* vim: set sts=2 sw=2 et tw=80: */
 "use strict";
 
 XPCOMUtils.defineLazyModuleGetter(this, "CustomizableUI",
                                   "resource:///modules/CustomizableUI.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "clearTimeout",
+                                  "resource://gre/modules/Timer.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "setTimeout",
+                                  "resource://gre/modules/Timer.jsm");
 
 XPCOMUtils.defineLazyGetter(this, "colorUtils", () => {
   return require("devtools/shared/css-color").colorUtils;
 });
 
 Cu.import("resource://devtools/shared/event-emitter.js");
 
 Cu.import("resource://gre/modules/ExtensionUtils.jsm");
 var {
   EventManager,
   IconDetails,
 } = ExtensionUtils;
 
+const POPUP_PRELOAD_TIMEOUT_MS = 200;
+
 const XUL_NS = "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";
 
 // WeakMap[Extension -> BrowserAction]
 var browserActionMap = new WeakMap();
 
 // Responsible for the browser_action section of the manifest as well
 // as the associated popup.
 function BrowserAction(options, extension) {
   this.extension = extension;
 
   let widgetId = makeWidgetId(extension.id);
   this.id = `${widgetId}-browser-action`;
   this.viewId = `PanelUI-webext-${widgetId}-browser-action-view`;
   this.widget = null;
 
+  this.pendingPopup = null;
+  this.pendingPopupTimeout = null;
+
   this.tabManager = TabManager.for(extension);
 
   this.defaults = {
     enabled: true,
     title: options.default_title || extension.name,
     badgeText: "",
     badgeBackgroundColor: null,
     icon: IconDetails.normalize({path: options.default_icon}, extension),
@@ -81,16 +90,18 @@ BrowserAction.prototype = {
         }
       },
 
       onCreated: node => {
         node.classList.add("badged-button");
         node.classList.add("webextension-browser-action");
         node.setAttribute("constrain-size", "true");
 
+        node.onmousedown = event => this.handleEvent(event);
+
         this.updateButton(node, this.defaults);
       },
 
       onViewShowing: event => {
         let document = event.target.ownerDocument;
         let tabbrowser = document.defaultView.gBrowser;
 
         let tab = tabbrowser.selectedTab;
@@ -98,17 +109,18 @@ BrowserAction.prototype = {
         this.tabManager.addActiveTabPermission(tab);
 
         // If the widget has a popup URL defined, we open a popup, but do not
         // dispatch a click event to the extension.
         // If it has no popup URL defined, we dispatch a click event, but do not
         // open a popup.
         if (popupURL) {
           try {
-            new ViewPopup(this.extension, event.target, popupURL, this.browserStyle);
+            let popup = this.getPopup(document.defaultView, popupURL);
+            event.detail.addBlocker(popup.attach(event.target));
           } catch (e) {
             Cu.reportError(e);
             event.preventDefault();
           }
         } else {
           // This isn't not a hack, but it seems to provide the correct behavior
           // with the fewest complications.
           event.preventDefault();
@@ -118,16 +130,110 @@ BrowserAction.prototype = {
     });
 
     this.tabContext.on("tab-select", // eslint-disable-line mozilla/balanced-listeners
                        (evt, tab) => { this.updateWindow(tab.ownerGlobal); });
 
     this.widget = widget;
   },
 
+  handleEvent(event) {
+    let button = event.target;
+    let window = button.ownerDocument.defaultView;
+
+    switch (event.type) {
+      case "mousedown":
+        if (event.button == 0) {
+          // Begin pre-loading the browser for the popup, so it's more likely to
+          // be ready by the time we get a complete click.
+          let tab = window.gBrowser.selectedTab;
+          let popupURL = this.getProperty(tab, "popup");
+
+          if (popupURL) {
+            this.pendingPopup = this.getPopup(window, popupURL);
+            window.addEventListener("mouseup", this, true);
+          } else {
+            this.clearPopup();
+          }
+        }
+        break;
+
+      case "mouseup":
+        if (event.button == 0) {
+          this.clearPopupTimeout();
+          // If we have a pending pre-loaded popup, cancel it after we've waited
+          // long enough that we can be relatively certain it won't be opening.
+          if (this.pendingPopup) {
+            if (event.target === this.widget.forWindow(window).node) {
+              this.pendingPopupTimeout = setTimeout(() => this.clearPopup(),
+                                                    POPUP_PRELOAD_TIMEOUT_MS);
+            } else {
+              this.clearPopup();
+            }
+          }
+        }
+        break;
+    }
+  },
+
+  /**
+   * Returns a potentially pre-loaded popup for the given URL in the given
+   * window. If a matching pre-load popup already exists, returns that.
+   * Otherwise, initializes a new one.
+   *
+   * If a pre-load popup exists which does not match, it is destroyed before a
+   * new one is created.
+   *
+   * @param {Window} window
+   *        The browser window in which to create the popup.
+   * @param {string} popupURL
+   *        The URL to load into the popup.
+   * @returns {ViewPopup}
+   */
+  getPopup(window, popupURL) {
+    this.clearPopupTimeout();
+    let {pendingPopup} = this;
+    this.pendingPopup = null;
+
+    if (pendingPopup) {
+      if (pendingPopup.window === window && pendingPopup.popupURL === popupURL) {
+        return pendingPopup;
+      }
+      pendingPopup.destroy();
+    }
+
+    let fixedWidth = this.widget.areaType == CustomizableUI.TYPE_MENU_PANEL;
+    return new ViewPopup(this.extension, window, popupURL, this.browserStyle, fixedWidth);
+  },
+
+  /**
+   * Clears any pending pre-loaded popup and related timeouts.
+   */
+  clearPopup() {
+    this.clearPopupTimeout();
+    if (this.pendingPopup) {
+      this.pendingPopup.destroy();
+      this.pendingPopup = null;
+    }
+  },
+
+  /**
+   * Clears any pending timeouts to clear stale, pre-loaded popups.
+   */
+  clearPopupTimeout() {
+    if (this.pendingPopup) {
+      this.pendingPopup.window.removeEventListener("mouseup", this, true);
+    }
+
+    if (this.pendingPopupTimeout) {
+      clearTimeout(this.pendingPopupTimeout);
+      this.pendingPopupTimeout = null;
+    }
+  },
+
   // Update the toolbar button |node| with the tab context data
   // in |tabData|.
   updateButton(node, tabData) {
     let title = tabData.title || this.extension.name;
     node.setAttribute("tooltiptext", title);
     node.setAttribute("label", title);
 
     if (tabData.badgeText) {
--- a/browser/components/extensions/ext-utils.js
+++ b/browser/components/extensions/ext-utils.js
@@ -3,28 +3,34 @@
 "use strict";
 
 XPCOMUtils.defineLazyModuleGetter(this, "CustomizableUI",
                                   "resource:///modules/CustomizableUI.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "NetUtil",
                                   "resource://gre/modules/NetUtil.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "PrivateBrowsingUtils",
                                   "resource://gre/modules/PrivateBrowsingUtils.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "Task",
+                                  "resource://gre/modules/Task.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "setTimeout",
+                                  "resource://gre/modules/Timer.jsm");
 
 XPCOMUtils.defineLazyServiceGetter(this, "styleSheetService",
                                    "@mozilla.org/content/style-sheet-service;1",
                                    "nsIStyleSheetService");
 
 XPCOMUtils.defineLazyGetter(this, "colorUtils", () => {
   return require("devtools/shared/css-color").colorUtils;
 });
 
 Cu.import("resource://gre/modules/ExtensionUtils.jsm");
 Cu.import("resource://gre/modules/AppConstants.jsm");
 
+const POPUP_LOAD_TIMEOUT_MS = 200;
+
 const XUL_NS = "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";
 
 // Minimum time between two resizes.
 const RESIZE_TIMEOUT = 100;
 
 var {
   EventManager,
 } = ExtensionUtils;
@@ -81,72 +87,76 @@ XPCOMUtils.defineLazyGetter(this, "stand
     let winStyleSheet = styleSheetService.preloadSheet(styleSheetURI,
                                                        styleSheetService.AGENT_SHEET);
     stylesheets.push(winStyleSheet);
   }
   return stylesheets;
 });
 
 class BasePopup {
-  constructor(extension, viewNode, popupURL, browserStyle) {
-    let popupURI = Services.io.newURI(popupURL, null, extension.baseURI);
-
-    Services.scriptSecurityManager.checkLoadURIWithPrincipal(
-      extension.principal, popupURI,
-      Services.scriptSecurityManager.DISALLOW_SCRIPT);
-
+  constructor(extension, viewNode, popupURL, browserStyle, fixedWidth = false) {
     this.extension = extension;
-    this.popupURI = popupURI;
+    this.popupURL = popupURL;
     this.viewNode = viewNode;
     this.browserStyle = browserStyle;
     this.window = viewNode.ownerGlobal;
+    this.destroyed = false;
+    this.fixedWidth = fixedWidth;
+    this.ignoreResizes = true;
 
     this.contentReady = new Promise(resolve => {
       this._resolveContentReady = resolve;
     });
 
     this.viewNode.addEventListener(this.DESTROY_EVENT, this);
 
     let doc = viewNode.ownerDocument;
     let arrowContent = doc.getAnonymousElementByAttribute(this.panel, "class", "panel-arrowcontent");
     this.borderColor = doc.defaultView.getComputedStyle(arrowContent).borderTopColor;
 
     this.browser = null;
-    this.browserReady = this.createBrowser(viewNode, popupURI);
+    this.browserLoaded = new Promise((resolve, reject) => {
+      this.browserLoadedDeferred = {resolve, reject};
+    });
+    this.browserReady = this.createBrowser(viewNode, popupURL);
   }
 
   destroy() {
-    this.browserReady.then(() => {
-      this.browser.removeEventListener("DOMWindowCreated", this, true);
-      this.browser.removeEventListener("load", this, true);
-      this.browser.removeEventListener("DOMTitleChanged", this, true);
-      this.browser.removeEventListener("DOMWindowClose", this, true);
-      this.browser.removeEventListener("MozScrolledAreaChanged", this, true);
+    this.destroyed = true;
+    this.browserLoadedDeferred.reject(new Error("Popup destroyed"));
+    return this.browserReady.then(() => {
+      this.destroyBrowser(this.browser);
+      this.browser.remove();
+
       this.viewNode.removeEventListener(this.DESTROY_EVENT, this);
       this.viewNode.style.maxHeight = "";
-      this.browser.remove();
 
       this.panel.style.setProperty("--panel-arrowcontent-background", "");
       this.panel.style.setProperty("--panel-arrow-image-vertical", "");
 
       this.browser = null;
       this.viewNode = null;
     });
   }
 
+  destroyBrowser(browser) {
+    browser.removeEventListener("DOMWindowCreated", this, true);
+    browser.removeEventListener("load", this, true);
+    browser.removeEventListener("DOMContentLoaded", this, true);
+    browser.removeEventListener("DOMTitleChanged", this, true);
+    browser.removeEventListener("DOMWindowClose", this, true);
+    browser.removeEventListener("MozScrolledAreaChanged", this, true);
+  }
+
   // Returns the name of the event fired on `viewNode` when the popup is being
   // destroyed. This must be implemented by every subclass.
   get DESTROY_EVENT() {
     throw new Error("Not implemented");
   }
 
-  get fixedWidth() {
-    return false;
-  }
-
   get panel() {
     let panel = this.viewNode;
     while (panel.localName != "panel") {
       panel = panel.parentNode;
     }
     return panel;
   }
 
@@ -181,24 +191,29 @@ class BasePopup {
           this.closePopup();
         }
         break;
 
       case "DOMTitleChanged":
         this.viewNode.setAttribute("aria-label", this.browser.contentTitle);
         break;
 
+      case "DOMContentLoaded":
+        this.browserLoadedDeferred.resolve();
+        this.resizeBrowser(true);
+        break;
+
       case "load":
         // We use a capturing listener, so we get this event earlier than any
         // load listeners in the content page. Resizing after a timeout ensures
         // that we calculate the size after the entire event cycle has completed
         // (unless someone spins the event loop, anyway), and hopefully after
         // the content has made any modifications.
         Promise.resolve().then(() => {
-          this.resizeBrowser();
+          this.resizeBrowser(true);
         });
 
         // Mutation observer to make sure the panel shrinks when the content does.
         new this.browser.contentWindow.MutationObserver(this.resizeBrowser.bind(this)).observe(
           this.browser.contentDocument.documentElement, {
             attributes: true,
             characterData: true,
             childList: true,
@@ -207,17 +222,17 @@ class BasePopup {
         break;
 
       case "MozScrolledAreaChanged":
         this.resizeBrowser();
         break;
     }
   }
 
-  createBrowser(viewNode, popupURI) {
+  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");
 
@@ -228,61 +243,78 @@ class BasePopup {
 
     // 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);
 
+    let initBrowser = browser => {
+      browser.addEventListener("DOMWindowCreated", this, true);
+      browser.addEventListener("load", this, true);
+      browser.addEventListener("DOMContentLoaded", this, true);
+      browser.addEventListener("DOMTitleChanged", this, true);
+      browser.addEventListener("DOMWindowClose", this, true);
+      browser.addEventListener("MozScrolledAreaChanged", this, true);
+    };
+
+    if (!popupURL) {
+      initBrowser(this.browser);
+      return this.browser;
+    }
+
     return new Promise(resolve => {
       // The first load event is for about:blank.
       // We can't finish setting up the browser until the binding has fully
       // initialized. Waiting for the first load event guarantees that it has.
       let loadListener = event => {
         this.browser.removeEventListener("load", loadListener, true);
         resolve();
       };
       this.browser.addEventListener("load", loadListener, true);
     }).then(() => {
+      initBrowser(this.browser);
+
       let {contentWindow} = this.browser;
 
       contentWindow.QueryInterface(Ci.nsIInterfaceRequestor)
                    .getInterface(Ci.nsIDOMWindowUtils)
                    .allowScriptsToClose();
 
-      this.browser.setAttribute("src", popupURI.spec);
-
-      this.browser.addEventListener("DOMWindowCreated", this, true);
-      this.browser.addEventListener("load", this, true);
-      this.browser.addEventListener("DOMTitleChanged", this, true);
-      this.browser.addEventListener("DOMWindowClose", this, true);
-      this.browser.addEventListener("MozScrolledAreaChanged", this, true);
+      this.browser.setAttribute("src", popupURL);
     });
   }
+
   // Resizes the browser to match the preferred size of the content (debounced).
-  resizeBrowser() {
+  resizeBrowser(ignoreThrottling = false) {
+    if (this.ignoreResizes) {
+      return;
+    }
+
+    if (ignoreThrottling && this.resizeTimeout) {
+      this.window.clearTimeout(this.resizeTimeout);
+      this.resizeTimeout = null;
+    }
+
     if (this.resizeTimeout == null) {
       this.resizeTimeout = this.window.setTimeout(() => {
         try {
           this._resizeBrowser();
         } finally {
           this.resizeTimeout = null;
         }
       }, RESIZE_TIMEOUT);
-      this._resizeBrowser(false);
+
+      this._resizeBrowser();
     }
   }
 
-  _resizeBrowser(clearTimeout = true) {
-    if (!this.browser) {
-      return;
-    }
-
-    let doc = this.browser.contentDocument;
+  _resizeBrowser() {
+    let doc = this.browser && this.browser.contentDocument;
     if (!doc || !doc.documentElement) {
       return;
     }
 
     let root = doc.documentElement;
     let body = doc.body;
     if (!body || doc.compatMode == "BackCompat") {
       // In quirks mode, the root element is used as the scroll frame, and the
@@ -388,16 +420,18 @@ global.PanelPopup = class PanelPopup ext
     panel.setAttribute("class", "browser-extension-panel");
     panel.setAttribute("type", "arrow");
     panel.setAttribute("role", "group");
 
     document.getElementById("mainPopupSet").appendChild(panel);
 
     super(extension, panel, popupURL, browserStyle);
 
+    this.ignoreResizes = false;
+
     this.contentReady.then(() => {
       panel.openPopup(imageNode, "bottomcenter topright", 0, 0, false, false);
     });
   }
 
   get DESTROY_EVENT() {
     return "popuphidden";
   }
@@ -413,48 +447,125 @@ global.PanelPopup = class PanelPopup ext
       if (this.viewNode) {
         this.viewNode.hidePopup();
       }
     });
   }
 };
 
 global.ViewPopup = class ViewPopup extends BasePopup {
-  constructor(...args) {
-    super(...args);
+  constructor(extension, window, popupURL, browserStyle, fixedWidth) {
+    let document = window.document;
+
+    // Create a temporary panel to hold the browser while it pre-loads its
+    // content. This panel will never be shown, but the browser's docShell will
+    // be swapped with the browser in the real panel when it's ready.
+    let panel = document.createElement("panel");
+    panel.setAttribute("type", "arrow");
+    document.getElementById("mainPopupSet").appendChild(panel);
+
+    super(extension, panel, popupURL, browserStyle, fixedWidth);
+
+    this.attached = false;
+    this.tempPanel = panel;
+
+    this.browser.classList.add("webextension-preload-browser");
+  }
 
-    // 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;
+  /**
+   * Attaches the pre-loaded browser to the given view node, and reserves a
+   * promise which resolves when the browser is ready.
+   *
+   * @param {Element} viewNode
+   *        The node to attach the browser to.
+   * @returns {Promise<boolean>}
+   *        Resolves when the browser is ready. Resolves to `false` if the
+   *        browser was destroyed before it was fully loaded, and the popup
+   *        should be closed, or `true` otherwise.
+   */
+  attach(viewNode) {
+    return Task.spawn(function* () {
+      this.viewNode = viewNode;
+      this.viewNode.addEventListener(this.DESTROY_EVENT, this);
+
+      // Wait until the browser element is fully initialized, and give it at least
+      // a short grace period to finish loading its initial content, if necessary.
+      //
+      // In practice, the browser that was created by the mousdown handler should
+      // nearly always be ready by this point.
+      yield Promise.all([
+        this.browserReady,
+        Promise.race([
+          // This promise may be rejected if the popup calls window.close()
+          // before it has fully loaded.
+          this.browserLoaded.catch(() => {}),
+          new Promise(resolve => setTimeout(resolve, POPUP_LOAD_TIMEOUT_MS)),
+        ]),
+      ]);
 
-    // Calculate the extra height available on the screen above and below the
-    // menu panel. Use that to calculate the how much the sub-view may grow.
-    let popupRect = this.panel.getBoundingClientRect();
+      if (this.destroyed) {
+        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;
+
+      // Calculate the extra height available on the screen above and below the
+      // menu panel. Use that to calculate the how much the sub-view may grow.
+      let popupRect = this.panel.getBoundingClientRect();
+
+      let win = this.window;
+      let popupBottom = win.mozInnerScreenY + popupRect.bottom;
+      let popupTop = win.mozInnerScreenY + popupRect.top;
+
+      let screenBottom = win.screen.availTop + win.screen.availHeight;
+      this.extraHeight = {
+        bottom: Math.max(0, screenBottom - popupBottom),
+        top:  Math.max(0, popupTop - win.screen.availTop),
+      };
 
-    let win = this.window;
-    let popupBottom = win.mozInnerScreenY + popupRect.bottom;
-    let popupTop = win.mozInnerScreenY + popupRect.top;
+      // Create a new browser in the real popup.
+      let browser = this.browser;
+      this.createBrowser(this.viewNode);
+
+      this.browser.swapDocShells(browser);
+      this.destroyBrowser(browser);
+
+      this.ignoreResizes = false;
+      this.resizeBrowser(true);
 
-    let screenBottom = win.screen.availTop + win.screen.availHeight;
-    this.extraHeight = {
-      bottom: Math.max(0, screenBottom - popupBottom),
-      top:  Math.max(0, popupTop - win.screen.availTop),
-    };
+      this.tempPanel.remove();
+      this.tempPanel = null;
+
+      return true;
+    }.bind(this));
+  }
+
+  destroy() {
+    return super.destroy().then(() => {
+      if (this.tempPanel) {
+        this.tempPanel.remove();
+        this.tempPanel = null;
+      }
+    });
   }
 
   get DESTROY_EVENT() {
     return "ViewHiding";
   }
 
-  get fixedWidth() {
-    return !this.viewNode.classList.contains("cui-widget-panelview");
-  }
-
   closePopup() {
-    CustomizableUI.hidePanelForNode(this.viewNode);
+    if (this.attached) {
+      CustomizableUI.hidePanelForNode(this.viewNode);
+    } else {
+      this.destroy();
+    }
   }
 };
 
 // Manages tab-specific context data, and dispatching tab select events
 // across all windows.
 global.TabContext = function TabContext(getDefaults, extension) {
   this.extension = extension;
   this.getDefaults = getDefaults;
--- a/browser/components/extensions/test/browser/browser_ext_browserAction_popup.js
+++ b/browser/components/extensions/test/browser/browser_ext_browserAction_popup.js
@@ -28,35 +28,54 @@ function* testInArea(area) {
           if (msg == "close-popup") {
             window.close();
           }
         });
       },
 
       "data/popup-b.html": scriptPage("popup-b.js"),
       "data/popup-b.js": function() {
-        browser.runtime.sendMessage("from-popup-b");
+        window.onload = () => {
+          browser.runtime.sendMessage("from-popup-b");
+        };
+      },
+
+      "data/popup-c.html": scriptPage("popup-c.js"),
+      "data/popup-c.js": function() {
+        // Close the popup before the document is fully-loaded to make sure that
+        // we handle this case sanely.
+        browser.runtime.sendMessage("from-popup-c");
+        window.close();
       },
 
       "data/background.html": scriptPage("background.js"),
 
       "data/background.js": function() {
         let sendClick;
         let tests = [
           () => {
+            browser.test.log("Open popup a");
             sendClick({expectEvent: false, expectPopup: "a"});
           },
           () => {
+            browser.test.log("Open popup a again");
             sendClick({expectEvent: false, expectPopup: "a"});
           },
           () => {
+            browser.test.log("Open popup c");
+            browser.browserAction.setPopup({popup: "popup-c.html"});
+            sendClick({expectEvent: false, expectPopup: "c", closePopup: false});
+          },
+          () => {
+            browser.test.log("Open popup b");
             browser.browserAction.setPopup({popup: "popup-b.html"});
             sendClick({expectEvent: false, expectPopup: "b"});
           },
           () => {
+            browser.test.log("Open popup b again");
             sendClick({expectEvent: false, expectPopup: "b"});
           },
           () => {
             browser.browserAction.setPopup({popup: ""});
             sendClick({expectEvent: true, expectPopup: null});
           },
           () => {
             sendClick({expectEvent: true, expectPopup: null});
@@ -66,18 +85,22 @@ function* testInArea(area) {
             sendClick({expectEvent: false, expectPopup: "a", runNextTest: true});
           },
           () => {
             browser.test.sendMessage("next-test", {expectClosed: true});
           },
         ];
 
         let expect = {};
-        sendClick = ({expectEvent, expectPopup, runNextTest}) => {
-          expect = {event: expectEvent, popup: expectPopup, runNextTest};
+        sendClick = ({expectEvent, expectPopup, runNextTest, closePopup}) => {
+          if (closePopup == undefined) {
+            closePopup = true;
+          }
+
+          expect = {event: expectEvent, popup: expectPopup, runNextTest, closePopup};
           browser.test.sendMessage("send-click");
         };
 
         browser.runtime.onMessage.addListener(msg => {
           if (msg == "close-popup") {
             return;
           } else if (expect.popup) {
             browser.test.assertEq(msg, `from-popup-${expect.popup}`,
@@ -86,17 +109,17 @@ function* testInArea(area) {
             browser.test.fail(`unexpected popup: ${msg}`);
           }
 
           expect.popup = null;
           if (expect.runNextTest) {
             expect.runNextTest = false;
             tests.shift()();
           } else {
-            browser.test.sendMessage("next-test");
+            browser.test.sendMessage("next-test", {closePopup: expect.closePopup});
           }
         });
 
         browser.browserAction.onClicked.addListener(() => {
           if (expect.event) {
             browser.test.succeed("expected click event received");
           } else {
             browser.test.fail("unexpected click event");
@@ -143,17 +166,17 @@ function* testInArea(area) {
       let panel = getBrowserActionPopup(extension);
       ok(panel, "Expect panel to exist");
       yield promisePopupShown(panel);
 
       extension.sendMessage("close-popup");
 
       yield promisePopupHidden(panel);
       ok(true, "Panel is closed");
-    } else {
+    } else if (expecting.closePopup) {
       yield closeBrowserAction(extension);
     }
 
     extension.sendMessage("next-test");
   }));
 
   yield Promise.all([extension.startup(), extension.awaitFinish("browseraction-tests-done")]);
 
@@ -165,8 +188,74 @@ function* testInArea(area) {
 
 add_task(function* testBrowserActionInToolbar() {
   yield testInArea(CustomizableUI.AREA_NAVBAR);
 });
 
 add_task(function* testBrowserActionInPanel() {
   yield testInArea(CustomizableUI.AREA_PANEL);
 });
+
+add_task(function* testBrowserActionClickCanceled() {
+  let extension = ExtensionTestUtils.loadExtension({
+    manifest: {
+      "browser_action": {
+        "default_popup": "popup.html",
+        "browser_style": true,
+      },
+    },
+
+    files: {
+      "popup.html": `<!DOCTYPE html><html><head><meta charset="utf-8"></head></html>`,
+    },
+  });
+
+  yield extension.startup();
+
+  const {GlobalManager, browserActionFor} = Cu.import("resource://gre/modules/Extension.jsm", {});
+
+  let ext = GlobalManager.extensionMap.get(extension.id);
+  let browserAction = browserActionFor(ext);
+
+  let widget = getBrowserActionWidget(extension).forWindow(window);
+
+  // Test canceled click.
+  EventUtils.synthesizeMouseAtCenter(widget.node, {type: "mousedown", button: 0}, window);
+
+  isnot(browserAction.pendingPopup, null, "Have pending popup");
+  is(browserAction.pendingPopup.window, window, "Have pending popup for the correct window");
+
+  is(browserAction.pendingPopupTimeout, null, "Have no pending popup timeout");
+
+  EventUtils.synthesizeMouseAtCenter(document.documentElement, {type: "mouseup", button: 0}, window);
+
+  is(browserAction.pendingPopup, null, "Pending popup was cleared");
+  is(browserAction.pendingPopupTimeout, null, "Have no pending popup timeout");
+
+  // Test completed click.
+  EventUtils.synthesizeMouseAtCenter(widget.node, {type: "mousedown", button: 0}, window);
+
+  isnot(browserAction.pendingPopup, null, "Have pending popup");
+  is(browserAction.pendingPopup.window, window, "Have pending popup for the correct window");
+
+  is(browserAction.pendingPopupTimeout, null, "Have no pending popup timeout");
+
+  // We need to do these tests during the mouseup event cycle, since the click
+  // and command events will be dispatched immediately after mouseup, and void
+  // the results.
+  let mouseUpPromise = BrowserTestUtils.waitForEvent(widget.node, "mouseup", event => {
+    isnot(browserAction.pendingPopup, null, "Pending popup was not cleared");
+    isnot(browserAction.pendingPopupTimeout, null, "Have a pending popup timeout");
+    return true;
+  });
+
+  EventUtils.synthesizeMouseAtCenter(widget.node, {type: "mouseup", button: 0}, window);
+
+  yield mouseUpPromise;
+
+  is(browserAction.pendingPopup, null, "Pending popup was cleared");
+  is(browserAction.pendingPopupTimeout, null, "Pending popup timeout was cleared");
+
+  yield promisePopupShown(getBrowserActionPopup(extension));
+  yield closeBrowserAction(extension);
+
+  yield extension.unload();
+});
--- a/browser/components/extensions/test/browser/browser_ext_browserAction_popup_resize.js
+++ b/browser/components/extensions/test/browser/browser_ext_browserAction_popup_resize.js
@@ -1,22 +1,16 @@
 /* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* vim: set sts=2 sw=2 et tw=80: */
 "use strict";
 
 function* openPanel(extension, win = window) {
   clickBrowserAction(extension, win);
 
-  let {target} = yield BrowserTestUtils.waitForEvent(win.document, "load", true, (event) => {
-    return event.target.location && event.target.location.href.endsWith("popup.html");
-  });
-
-  return target.defaultView
-               .QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIDocShell)
-               .chromeEventHandler;
+  return yield awaitExtensionPanel(extension, win);
 }
 
 function* awaitResize(browser) {
   // Debouncing code makes this a bit racy.
   // Try to skip the first, early resize, and catch the resize event we're
   // looking for, but don't wait longer than a few seconds.
 
   return Promise.race([
--- a/browser/components/extensions/test/browser/browser_ext_browserAction_simple.js
+++ b/browser/components/extensions/test/browser/browser_ext_browserAction_simple.js
@@ -15,17 +15,19 @@ add_task(function* () {
       "popup.html": `
       <!DOCTYPE html>
       <html><body>
       <script src="popup.js"></script>
       </body></html>
       `,
 
       "popup.js": function() {
-        browser.runtime.sendMessage("from-popup");
+        window.onload = () => {
+          browser.runtime.sendMessage("from-popup");
+        };
       },
     },
 
     background: function() {
       browser.runtime.onMessage.addListener(msg => {
         browser.test.assertEq(msg, "from-popup", "correct message received");
         browser.test.sendMessage("popup");
       });
--- a/browser/components/extensions/test/browser/browser_ext_currentWindow.js
+++ b/browser/components/extensions/test/browser/browser_ext_currentWindow.js
@@ -106,16 +106,17 @@ add_task(function* () {
 
   yield focusWindow(win1);
   yield checkWindow("background", winId1, "win1");
   yield focusWindow(win2);
   yield checkWindow("background", winId2, "win2");
 
   function* triggerPopup(win, callback) {
     yield clickBrowserAction(extension, win);
+    yield awaitExtensionPanel(extension, win);
 
     yield extension.awaitMessage("popup-ready");
 
     yield callback();
 
     closeBrowserAction(extension, win);
   }
 
--- a/browser/components/extensions/test/browser/browser_ext_getViews.js
+++ b/browser/components/extensions/test/browser/browser_ext_getViews.js
@@ -132,16 +132,17 @@ add_task(function* () {
   yield checkViews("tab", 1, 0, 1);
 
   yield openTab(winId2);
 
   yield checkViews("background", 2, 0, 0, winId2, 1);
 
   function* triggerPopup(win, callback) {
     yield clickBrowserAction(extension, win);
+    yield awaitExtensionPanel(extension, win);
 
     yield extension.awaitMessage("popup-ready");
 
     yield callback();
 
     closeBrowserAction(extension, win);
   }
 
--- a/browser/components/extensions/test/browser/browser_ext_popup_api_injection.js
+++ b/browser/components/extensions/test/browser/browser_ext_popup_api_injection.js
@@ -56,16 +56,17 @@ add_task(function* testPageActionPopup()
   yield extension.awaitMessage("ready");
 
 
   // Check that unprivileged documents don't get the API.
   // BrowserAction:
   let awaitMessage = promiseConsoleMessage(/WebExt Privilege Escalation: BrowserAction/);
   SimpleTest.expectUncaughtException();
   yield clickBrowserAction(extension);
+  yield promisePopupShown(getBrowserActionPopup(extension));
 
   let message = yield awaitMessage;
   ok(message.includes("WebExt Privilege Escalation: BrowserAction: typeof(browser) = undefined"),
      `No BrowserAction API injection`);
 
   yield closeBrowserAction(extension);
 
   // PageAction
@@ -84,16 +85,17 @@ add_task(function* testPageActionPopup()
 
   // Check that privileged documents *do* get the API.
   extension.sendMessage("next");
   yield extension.awaitMessage("ok");
 
 
   yield clickBrowserAction(extension);
   yield extension.awaitMessage("from-popup-a");
+  yield promisePopupShown(getBrowserActionPopup(extension));
   yield closeBrowserAction(extension);
 
   yield clickPageAction(extension);
   yield extension.awaitMessage("from-popup-b");
   yield closePageAction(extension);
 
   yield extension.unload();
 });
--- a/browser/components/extensions/test/browser/browser_ext_popup_background.js
+++ b/browser/components/extensions/test/browser/browser_ext_popup_background.js
@@ -1,22 +1,12 @@
 /* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* vim: set sts=2 sw=2 et tw=80: */
 "use strict";
 
-function* awaitPanel(extension, win = window) {
-  let {target} = yield BrowserTestUtils.waitForEvent(win.document, "load", true, (event) => {
-    return event.target.location && event.target.location.href.endsWith("popup.html");
-  });
-
-  return target.defaultView
-               .QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIDocShell)
-               .chromeEventHandler;
-}
-
 function* awaitResize(browser) {
   // Debouncing code makes this a bit racy.
   // Try to skip the first, early resize, and catch the resize event we're
   // looking for, but don't wait longer than a few seconds.
 
   return Promise.race([
     BrowserTestUtils.waitForEvent(browser, "WebExtPopupResized")
       .then(() => BrowserTestUtils.waitForEvent(browser, "WebExtPopupResized")),
@@ -112,36 +102,36 @@ add_task(function* testPopupBackground()
 
     checkArrow(null);
   }
 
   {
     info("Test stand-alone browserAction popup");
 
     clickBrowserAction(extension);
-    let browser = yield awaitPanel(extension);
+    let browser = yield awaitExtensionPanel(extension);
     yield testPanel(browser, true);
     yield closeBrowserAction(extension);
   }
 
   {
     info("Test menu panel browserAction popup");
 
     let widget = getBrowserActionWidget(extension);
     CustomizableUI.addWidgetToArea(widget.id, CustomizableUI.AREA_PANEL);
 
     clickBrowserAction(extension);
-    let browser = yield awaitPanel(extension);
+    let browser = yield awaitExtensionPanel(extension);
     yield testPanel(browser, false);
     yield closeBrowserAction(extension);
   }
 
   {
     info("Test pageAction popup");
 
     clickPageAction(extension);
-    let browser = yield awaitPanel(extension);
+    let browser = yield awaitExtensionPanel(extension);
     yield testPanel(browser, true);
     yield closePageAction(extension);
   }
 
   yield extension.unload();
 });
--- a/browser/components/extensions/test/browser/browser_ext_popup_corners.js
+++ b/browser/components/extensions/test/browser/browser_ext_popup_corners.js
@@ -1,22 +1,12 @@
 /* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* vim: set sts=2 sw=2 et tw=80: */
 "use strict";
 
-function* awaitPanel(extension, win = window) {
-  let {target} = yield BrowserTestUtils.waitForEvent(win.document, "load", true, (event) => {
-    return event.target.location && event.target.location.href.endsWith("popup.html");
-  });
-
-  return target.defaultView
-               .QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIDocShell)
-               .chromeEventHandler;
-}
-
 add_task(function* testPopupBorderRadius() {
   let extension = ExtensionTestUtils.loadExtension({
     background() {
       browser.tabs.query({active: true, currentWindow: true}, tabs => {
         browser.pageAction.show(tabs[0].id);
       });
     },
 
@@ -66,36 +56,36 @@ add_task(function* testPopupBorderRadius
       }
     }
   }
 
   {
     info("Test stand-alone browserAction popup");
 
     clickBrowserAction(extension);
-    let browser = yield awaitPanel(extension);
+    let browser = yield awaitExtensionPanel(extension);
     yield testPanel(browser);
     yield closeBrowserAction(extension);
   }
 
   {
     info("Test menu panel browserAction popup");
 
     let widget = getBrowserActionWidget(extension);
     CustomizableUI.addWidgetToArea(widget.id, CustomizableUI.AREA_PANEL);
 
     clickBrowserAction(extension);
-    let browser = yield awaitPanel(extension);
+    let browser = yield awaitExtensionPanel(extension);
     yield testPanel(browser, false);
     yield closeBrowserAction(extension);
   }
 
   {
     info("Test pageAction popup");
 
     clickPageAction(extension);
-    let browser = yield awaitPanel(extension);
+    let browser = yield awaitExtensionPanel(extension);
     yield testPanel(browser);
     yield closePageAction(extension);
   }
 
   yield extension.unload();
 });
--- a/browser/components/extensions/test/browser/browser_ext_tabs_executeScript_good.js
+++ b/browser/components/extensions/test/browser/browser_ext_tabs_executeScript_good.js
@@ -136,17 +136,21 @@ add_task(function* testGoodPermissions()
   });
 
   info("Test activeTab permission with a browser action w/popup click");
   yield testHasPermission({
     manifest: {
       "permissions": ["activeTab"],
       "browser_action": {"default_popup": "_blank.html"},
     },
-    setup: clickBrowserAction,
+    setup: extension => {
+      return clickBrowserAction(extension).then(() => {
+        return awaitExtensionPanel(extension);
+      });
+    },
     tearDown: closeBrowserAction,
   });
 
   info("Test activeTab permission with a page action w/popup click");
   yield testHasPermission({
     manifest: {
       "permissions": ["activeTab"],
       "page_action": {"default_popup": "_blank.html"},
--- a/browser/components/extensions/test/browser/head.js
+++ b/browser/components/extensions/test/browser/head.js
@@ -6,16 +6,17 @@
  *          getBrowserActionWidget
  *          clickBrowserAction clickPageAction
  *          getBrowserActionPopup getPageActionPopup
  *          closeBrowserAction closePageAction
  *          promisePopupShown promisePopupHidden
  *          openContextMenu closeContextMenu
  *          openExtensionContextMenu closeExtensionContextMenu
  *          imageBuffer getListStyleImage getPanelForNode
+ *          awaitExtensionPanel
  */
 
 var {AppConstants} = Cu.import("resource://gre/modules/AppConstants.jsm");
 var {CustomizableUI} = Cu.import("resource:///modules/CustomizableUI.jsm");
 
 // Bug 1239884: Our tests occasionally hit a long GC pause at unpredictable
 // times in debug builds, which results in intermittent timeouts. Until we have
 // a better solution, we force a GC after certain strategic tests, which tend to
@@ -85,16 +86,35 @@ function promisePopupHidden(popup) {
 
 function getPanelForNode(node) {
   while (node.localName != "panel") {
     node = node.parentNode;
   }
   return node;
 }
 
+function* awaitExtensionPanel(extension, win = window) {
+  let {target} = yield BrowserTestUtils.waitForEvent(win.document, "load", true, (event) => {
+    return event.target.location && event.target.location.href.endsWith("popup.html");
+  });
+
+  let browser = target.defaultView
+                      .QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIDocShell)
+                      .chromeEventHandler;
+
+  if (browser.matches(".webextension-preload-browser")) {
+    let event = yield BrowserTestUtils.waitForEvent(browser, "SwapDocShells");
+    browser = event.detail;
+  }
+
+  yield promisePopupShown(getPanelForNode(browser));
+
+  return browser;
+}
+
 function getBrowserActionWidget(extension) {
   return CustomizableUI.getWidget(makeWidgetId(extension.id) + "-browser-action");
 }
 
 function getBrowserActionPopup(extension, win = window) {
   let group = getBrowserActionWidget(extension);
 
   if (group.areaType == CustomizableUI.TYPE_TOOLBAR) {
--- a/toolkit/components/extensions/Extension.jsm
+++ b/toolkit/components/extensions/Extension.jsm
@@ -755,25 +755,25 @@ GlobalManager = {
     let incognito = PrivateBrowsingUtils.isContentWindowPrivate(contentWindow);
 
     let context = new ExtensionContext(extension, {type, contentWindow, uri, docShell, incognito});
     inject(extension, context);
     if (type == "background") {
       this._initializeBackgroundPage(contentWindow);
     }
 
-    let eventHandler = docShell.chromeEventHandler;
     let listener = event => {
-      if (event.target != docShell.contentViewer.DOMDocument) {
-        return;
+      if (event.target === contentWindow.document) {
+        contentWindow.removeEventListener("unload", listener, true);
+        Promise.resolve().then(() => {
+          context.unload();
+        });
       }
-      eventHandler.removeEventListener("unload", listener, true);
-      context.unload();
     };
-    eventHandler.addEventListener("unload", listener, true);
+    contentWindow.addEventListener("unload", listener, true);
   },
 
   _initializeBackgroundPage(contentWindow) {
     // Override the `alert()` method inside background windows;
     // we alias it to console.log().
     // See: https://bugzilla.mozilla.org/show_bug.cgi?id=1203394
     let alertDisplayedWarning = false;
     let alertOverwrite = text => {
--- a/toolkit/components/extensions/ExtensionUtils.jsm
+++ b/toolkit/components/extensions/ExtensionUtils.jsm
@@ -366,17 +366,17 @@ class BaseContext {
             if (this.unloaded) {
               dump(`Promise resolved after context unloaded\n`);
             } else {
               runSafe(resolve, value);
             }
           },
           value => {
             if (this.unloaded) {
-              dump(`Promise rejected after context unloaded\n`);
+              dump(`Promise rejected after context unloaded: ${value && value.message}\n`);
             } else {
               this.runSafeWithoutClone(reject, this.normalizeError(value));
             }
           });
       });
     }
   }