Bug 1501375 - Account for shutdown of bg page during extension start-up r=rpl,aswan
authorRob Wu <rob@robwu.nl>
Wed, 20 Feb 2019 21:53:01 +0000
changeset 518753 2090ee542b58d0610623717a02a1a3dea1ceb1e0
parent 518752 2def2f8d49e7f0f13fcb5b15f9b3d586c963c875
child 518754 3a5c6e81192dbdb0a8adfb20e00181d3de5540f7
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrpl, aswan
bugs1501375
milestone67.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 1501375 - Account for shutdown of bg page during extension start-up r=rpl,aswan Ensure that the windowless browser is only closed when its dummy XUL page has completely loaded, and that HiddenExtensionPage's browser is really cleared if the extension was shut down during initialization. Depends on D9959 Differential Revision: https://phabricator.services.mozilla.com/D10954
toolkit/components/extensions/ExtensionParent.jsm
toolkit/components/extensions/parent/ext-backgroundPage.js
--- a/toolkit/components/extensions/ExtensionParent.jsm
+++ b/toolkit/components/extensions/ExtensionParent.jsm
@@ -1052,81 +1052,91 @@ ParentAPIManager.init();
  * ExtensionDebuggingUtils to contain the browser elements used by the
  * addon debugger to connect to the devtools actors running in the same
  * process of the target extension (and be able to stay connected across
  *  the addon reloads).
  */
 class HiddenXULWindow {
   constructor() {
     this._windowlessBrowser = null;
+    this.unloaded = false;
     this.waitInitialized = this.initWindowlessBrowser();
   }
 
   shutdown() {
     if (this.unloaded) {
       throw new Error("Unable to shutdown an unloaded HiddenXULWindow instance");
     }
 
     this.unloaded = true;
 
-    this.chromeShell = null;
     this.waitInitialized = null;
 
+    if (!this._windowlessBrowser) {
+      Cu.reportError("HiddenXULWindow was shut down while it was loading.");
+      // initWindowlessBrowser will close windowlessBrowser when possible.
+      return;
+    }
+
     this._windowlessBrowser.close();
     this._windowlessBrowser = null;
   }
 
   get chromeDocument() {
     return this._windowlessBrowser.document;
   }
 
   /**
    * Private helper that create a XULDocument in a windowless browser.
    *
-   * @returns {Promise<XULDocument>}
-   *          A promise which resolves to the newly created XULDocument.
+   * @returns {Promise<void>}
+   *          A promise which resolves when the windowless browser is ready.
    */
   async initWindowlessBrowser() {
     if (this.waitInitialized) {
       throw new Error("HiddenXULWindow already initialized");
     }
 
     // The invisible page is currently wrapped in a XUL window to fix an issue
     // with using the canvas API from a background page (See Bug 1274775).
     let windowlessBrowser = Services.appShell.createWindowlessBrowser(true);
-    this._windowlessBrowser = windowlessBrowser;
 
     // The windowless browser is a thin wrapper around a docShell that keeps
     // its related resources alive. It implements nsIWebNavigation and
     // forwards its methods to the underlying docShell, but cannot act as a
     // docShell itself.  Getting .docShell gives us the
     // underlying docShell, and `QueryInterface(nsIWebNavigation)` gives us
     // access to the webNav methods that are already available on the
     // windowless browser, but contrary to appearances, they are not the same
     // object.
-    this.chromeShell = this._windowlessBrowser.docShell
-                           .QueryInterface(Ci.nsIWebNavigation);
+    let chromeShell = windowlessBrowser.docShell
+                                       .QueryInterface(Ci.nsIWebNavigation);
 
     if (PrivateBrowsingUtils.permanentPrivateBrowsing) {
-      let attrs = this.chromeShell.getOriginAttributes();
+      let attrs = chromeShell.getOriginAttributes();
       attrs.privateBrowsingId = 1;
-      this.chromeShell.setOriginAttributes(attrs);
+      chromeShell.setOriginAttributes(attrs);
     }
 
     let system = Services.scriptSecurityManager.getSystemPrincipal();
-    this.chromeShell.createAboutBlankContentViewer(system);
-    this.chromeShell.useGlobalHistory = false;
+    chromeShell.createAboutBlankContentViewer(system);
+    chromeShell.useGlobalHistory = false;
     let loadURIOptions = {
       triggeringPrincipal: system,
     };
-    this.chromeShell.loadURI("chrome://extensions/content/dummy.xul", loadURIOptions);
+    chromeShell.loadURI("chrome://extensions/content/dummy.xul", loadURIOptions);
 
     await promiseObserved("chrome-document-global-created",
-                          win => win.document == this.chromeShell.document);
-    return promiseDocumentLoaded(windowlessBrowser.document);
+                          win => win.document == chromeShell.document);
+    await promiseDocumentLoaded(windowlessBrowser.document);
+    if (this.unloaded) {
+      windowlessBrowser.close();
+      return;
+    }
+    this._windowlessBrowser = windowlessBrowser;
   }
 
   /**
    * Creates the browser XUL element that will contain the WebExtension Page.
    *
    * @param {Object} xulAttributes
    *        An object that contains the xul attributes to set of the newly
    *        created browser XUL element.
@@ -1218,51 +1228,68 @@ class HiddenExtensionPage {
   constructor(extension, viewType) {
     if (!extension || !viewType) {
       throw new Error("extension and viewType parameters are mandatory");
     }
 
     this.extension = extension;
     this.viewType = viewType;
     this.browser = null;
+    this.unloaded = false;
   }
 
   /**
    * Destroy the created parent document.
    */
   shutdown() {
     if (this.unloaded) {
       throw new Error("Unable to shutdown an unloaded HiddenExtensionPage instance");
     }
 
+    this.unloaded = true;
+
     if (this.browser) {
-      this.browser.remove();
-      this.browser = null;
-      SharedWindow.release();
+      this._releaseBrowser();
     }
   }
 
+  _releaseBrowser() {
+    this.browser.remove();
+    this.browser = null;
+    SharedWindow.release();
+  }
+
   /**
    * Creates the browser XUL element that will contain the WebExtension Page.
    *
    * @returns {Promise<XULElement>}
    *          A Promise which resolves to the newly created browser XUL element.
    */
   async createBrowserElement() {
     if (this.browser) {
       throw new Error("createBrowserElement called twice");
     }
 
     let window = SharedWindow.acquire();
-    this.browser = await window.createBrowserElement({
-      "webextension-view-type": this.viewType,
-      "remote": this.extension.remote ? "true" : null,
-      "remoteType": this.extension.remote ?
-        E10SUtils.EXTENSION_REMOTE_TYPE : null,
-    }, this.extension.groupFrameLoader);
+    try {
+      this.browser = await window.createBrowserElement({
+        "webextension-view-type": this.viewType,
+        "remote": this.extension.remote ? "true" : null,
+        "remoteType": this.extension.remote ?
+          E10SUtils.EXTENSION_REMOTE_TYPE : null,
+      }, this.extension.groupFrameLoader);
+    } catch (e) {
+      SharedWindow.release();
+      throw e;
+    }
+
+    if (this.unloaded) {
+      this._releaseBrowser();
+      throw new Error("Extension shut down before browser element was created");
+    }
 
     return this.browser;
   }
 }
 
 /**
  * This object provides utility functions needed by the devtools actors to
  * be able to connect and debug an extension (which can run in the main or in
--- a/toolkit/components/extensions/parent/ext-backgroundPage.js
+++ b/toolkit/components/extensions/parent/ext-backgroundPage.js
@@ -28,29 +28,33 @@ class BackgroundPage extends HiddenExten
     }
   }
 
   async build() {
     const {extension} = this;
 
     ExtensionTelemetry.backgroundPageLoad.stopwatchStart(extension, this);
 
-    await this.createBrowserElement();
-    extension._backgroundPageFrameLoader = this.browser.frameLoader;
-
-    extensions.emit("extension-browser-inserted", this.browser);
-
-    let contextPromise = promiseExtensionViewLoaded(this.browser);
-    this.browser.loadURI(this.url, {triggeringPrincipal: extension.principal});
-
     let context;
     try {
+      await this.createBrowserElement();
+      if (!this.browser) {
+        throw new Error("Extension shut down before the background page was created");
+      }
+      extension._backgroundPageFrameLoader = this.browser.frameLoader;
+
+      extensions.emit("extension-browser-inserted", this.browser);
+
+      let contextPromise = promiseExtensionViewLoaded(this.browser);
+      this.browser.loadURI(this.url, {triggeringPrincipal: extension.principal});
+
       context = await contextPromise;
     } catch (e) {
       // Extension was down before the background page has loaded.
+      Cu.reportError(e);
       ExtensionTelemetry.backgroundPageLoad.stopwatchCancel(extension, this);
       return;
     }
 
     ExtensionTelemetry.backgroundPageLoad.stopwatchFinish(extension, this);
 
     if (context) {
       // Wait until all event listeners registered by the script so far