Bug 1518598 - Return early if popup has already been closed r=mixedpuppy
authorRob Wu <rob@robwu.nl>
Tue, 15 Jan 2019 18:50:49 +0000
changeset 514836 d27e22f625f26ba6ae485d5da99f4b5ded52e065
parent 514835 480c8157839021de247ead74bffb60ffb5c70a8c
child 514837 9e9085aee8bcf0fd6fc9b71cb30bf02eab563478
push id1953
push userffxbld-merge
push dateMon, 11 Mar 2019 12:10:20 +0000
treeherdermozilla-release@9c35dcbaa899 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmixedpuppy
bugs1518598
milestone66.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 1518598 - Return early if popup has already been closed r=mixedpuppy Differential Revision: https://phabricator.services.mozilla.com/D16033
browser/components/extensions/ExtensionPopups.jsm
browser/components/extensions/parent/ext-browserAction.js
browser/components/extensions/test/browser/browser_ext_browserAction_popup_preload.js
--- a/browser/components/extensions/ExtensionPopups.jsm
+++ b/browser/components/extensions/ExtensionPopups.jsm
@@ -330,16 +330,19 @@ class BasePopup {
       });
 
       browser.loadURI(popupURL, {triggeringPrincipal: this.extension.principal});
     });
   }
 
   unblockParser() {
     this.browserReady.then(browser => {
+      if (this.destroyed) {
+        return;
+      }
       this.browser.messageManager.sendAsyncMessage("Extension:UnblockParser");
     });
   }
 
   resizeBrowser({width, height, detail}) {
     if (this.fixedWidth) {
       // Figure out how much extra space we have on the side of the panel
       // opposite the arrow.
@@ -484,16 +487,19 @@ class ViewPopup extends BasePopup {
    * @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.
    */
   async attach(viewNode) {
+    if (this.destroyed) {
+      return false;
+    }
     this.viewNode.removeEventListener(this.DESTROY_EVENT, this);
     this.panel.removeEventListener("popuppositioned", this, {once: true, capture: true});
 
     this.viewNode = viewNode;
     this.viewNode.addEventListener(this.DESTROY_EVENT, this);
     this.viewNode.setAttribute("closemenu", "none");
 
     this.panel.addEventListener("popuppositioned", this, {once: true, capture: true});
--- a/browser/components/extensions/parent/ext-browserAction.js
+++ b/browser/components/extensions/parent/ext-browserAction.js
@@ -196,17 +196,17 @@ this.browserAction = class extends Exten
             let popup = this.getPopup(document.defaultView, popupURL);
             let attachPromise = popup.attach(event.target);
             event.detail.addBlocker(attachPromise);
             await attachPromise;
             ExtensionTelemetry.browserActionPopupOpen.stopwatchFinish(extension, this);
             if (this.eventQueue.length) {
               ExtensionTelemetry.browserActionPreloadResult.histogramAdd({
                 category: "popupShown",
-                extension: this.extension,
+                extension,
               });
               this.eventQueue = [];
             }
           } catch (e) {
             ExtensionTelemetry.browserActionPopupOpen.stopwatchCancel(extension, this);
             Cu.reportError(e);
             event.preventDefault();
           }
--- a/browser/components/extensions/test/browser/browser_ext_browserAction_popup_preload.js
+++ b/browser/components/extensions/test/browser/browser_ext_browserAction_popup_preload.js
@@ -206,8 +206,66 @@ add_task(async function testBrowserActio
 
   EventUtils.synthesizeMouseAtCenter(widget.node, {type: "mouseup", button: 0}, win);
 
   await extension.awaitMessage("tabTitle");
 
   await extension.unload();
   await BrowserTestUtils.closeWindow(win);
 });
+
+add_task(async function testClosePopupDuringPreload() {
+  let extension = ExtensionTestUtils.loadExtension({
+    manifest: {
+      "browser_action": {
+        "default_popup": "popup.html",
+        "browser_style": true,
+      },
+    },
+
+    files: {
+      "popup.html": scriptPage("popup.js"),
+      "popup.js": function() {
+        browser.test.sendMessage("popup_loaded");
+        window.close();
+      },
+    },
+  });
+
+  // Make sure the mouse isn't hovering over the browserAction widget.
+  EventUtils.synthesizeMouseAtCenter(gURLBar, {type: "mouseover"}, window);
+
+  await extension.startup();
+
+  const {GlobalManager, Management: {global: {browserActionFor}}} = ChromeUtils.import("resource://gre/modules/Extension.jsm", {});
+
+  let ext = GlobalManager.extensionMap.get(extension.id);
+  let browserAction = browserActionFor(ext);
+
+  let widget = getBrowserActionWidget(extension).forWindow(window);
+
+  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");
+
+  await extension.awaitMessage("popup_loaded");
+  try {
+    await browserAction.pendingPopup.browserLoaded;
+  } catch (e) {
+    is(e.message, "Popup destroyed", "Popup content should have been destroyed");
+  }
+
+  let promiseViewShowing =
+    BrowserTestUtils.waitForEvent(document, "ViewShowing", false,
+                                  ev => ev.target.id === browserAction.viewId);
+  EventUtils.synthesizeMouseAtCenter(widget.node, {type: "mouseup", button: 0}, window);
+
+  // The popup panel may become visible after the ViewShowing event. Wait a bit
+  // to ensure that the popup is not shown when window.close() was used.
+  await promiseViewShowing;
+  await new Promise(resolve => setTimeout(resolve, 50));
+
+  let panel = getBrowserActionPopup(extension);
+  is(panel, null, "Popup panel should have been closed");
+
+  await extension.unload();
+});