Bug 1374590 - Refactoring ParentDevToolsPanel class to provide create/destroyBrowserElement helper methods. draft
authorLuca Greco <lgreco@mozilla.com>
Mon, 11 Sep 2017 15:49:36 +0200
changeset 664017 a7f61d74a4221f168fae94e7582e0928c06d7baf
parent 663831 1888ec2f277f6bb26271b8808e08914a21db9efe
child 664018 2b4692446d788cf888b62be7f4124ab39e4f7304
push id79593
push userluca.greco@alcacoop.it
push dateWed, 13 Sep 2017 16:19:52 +0000
bugs1374590
milestone57.0a1
Bug 1374590 - Refactoring ParentDevToolsPanel class to provide create/destroyBrowserElement helper methods. MozReview-Commit-ID: FsGlCU0H9VG
browser/components/extensions/ext-devtools-panels.js
devtools/client/framework/toolbox.js
--- a/browser/components/extensions/ext-devtools-panels.js
+++ b/browser/components/extensions/ext-devtools-panels.js
@@ -57,22 +57,23 @@ class ParentDevToolsPanel {
     this.panelOptions = panelOptions;
 
     this.context.callOnClose(this);
 
     this.id = this.panelOptions.id;
 
     this.onToolboxPanelSelect = this.onToolboxPanelSelect.bind(this);
 
-    this.panelAdded = false;
-    this.addPanel();
-
+    this.unwatchExtensionProxyContextLoad = null;
     this.waitTopLevelContext = new Promise(resolve => {
       this._resolveTopLevelContext = resolve;
     });
+
+    this.panelAdded = false;
+    this.addPanel();
   }
 
   addPanel() {
     const {icon, title} = this.panelOptions;
     const extensionName = this.context.extension.name;
 
     this.toolbox.addAdditionalTool({
       id: this.id,
@@ -83,26 +84,85 @@ class ParentDevToolsPanel {
       invertIconForLightTheme: false,
       visibilityswitch:  `devtools.webext-${this.id}.enabled`,
       isTargetSupported: target => target.isLocalTab,
       build: (window, toolbox) => {
         if (toolbox !== this.toolbox) {
           throw new Error("Unexpected toolbox received on addAdditionalTool build property");
         }
 
-        const destroy = this.buildPanel(window, toolbox);
+        const destroy = this.buildPanel(window);
 
         return {toolbox, destroy};
       },
     });
 
     this.panelAdded = true;
   }
 
-  buildPanel(window, toolbox) {
+  buildPanel(window) {
+    const {toolbox} = this;
+
+    this.createBrowserElement(window);
+
+    toolbox.on("select", this.onToolboxPanelSelect);
+
+    // Return a cleanup method that is when the panel is destroyed, e.g.
+    // - when addon devtool panel has been disabled by the user from the toolbox preferences,
+    //   its ParentDevToolsPanel instance is still valid, but the built devtools panel is removed from
+    //   the toolbox (and re-built again if the user re-enables it from the toolbox preferences panel)
+    // - when the creator context has been destroyed, the ParentDevToolsPanel close method is called,
+    //   it removes the tool definition from the toolbox, which will call this destroy method.
+    return () => {
+      this.destroyBrowserElement();
+      toolbox.off("select", this.onToolboxPanelSelect);
+    };
+  }
+
+  async onToolboxPanelSelect(what, id) {
+    if (!this.waitTopLevelContext || !this.panelAdded) {
+      return;
+    }
+
+    // Wait that the panel is fully loaded and emit show.
+    await this.waitTopLevelContext;
+
+    if (!this.visible && id === this.id) {
+      this.visible = true;
+    } else if (this.visible && id !== this.id) {
+      this.visible = false;
+    }
+
+    const extensionMessage = `Extension:DevToolsPanel${this.visible ? "Shown" : "Hidden"}`;
+    this.context.parentMessageManager.sendAsyncMessage(extensionMessage, {
+      toolboxPanelId: this.id,
+    });
+  }
+
+  close() {
+    const {toolbox} = this;
+
+    if (!toolbox) {
+      throw new Error("Unable to destroy a closed devtools panel");
+    }
+
+    // Explicitly remove the panel if it is registered and the toolbox is not
+    // closing itself.
+    if (this.panelAdded && toolbox.isToolRegistered(this.id)) {
+      toolbox.removeAdditionalTool(this.id);
+    }
+
+    this.context = null;
+    this.toolbox = null;
+    this.waitTopLevelContext = null;
+    this._resolveTopLevelContext = null;
+  }
+
+  createBrowserElement(window) {
+    const {toolbox} = this;
     const {url} = this.panelOptions;
     const {document} = window;
 
     const browser = document.createElementNS(XUL_NS, "browser");
     browser.setAttribute("type", "content");
     browser.setAttribute("disableglobalhistory", "true");
     browser.setAttribute("style", "width: 100%; height: 100%;");
     browser.setAttribute("transparent", "true");
@@ -125,17 +185,17 @@ class ParentDevToolsPanel {
       // because of a swapFrameLoader exception (see bug 1075490).
       browser.setAttribute("type", "chrome");
       browser.setAttribute("forcemessagemanager", true);
     }
 
     let hasTopLevelContext = false;
 
     // Listening to new proxy contexts.
-    const unwatchExtensionProxyContextLoad = watchExtensionProxyContextLoad(this, context => {
+    this.unwatchExtensionProxyContextLoad = watchExtensionProxyContextLoad(this, context => {
       // Keep track of the toolbox and target associated to the context, which is
       // needed by the API methods implementation.
       context.devToolsToolbox = toolbox;
 
       if (!hasTopLevelContext) {
         hasTopLevelContext = true;
 
         // Resolve the promise when the root devtools_panel context has been created.
@@ -149,75 +209,36 @@ class ParentDevToolsPanel {
     extensions.emit("extension-browser-inserted", browser, {
       devtoolsToolboxInfo: {
         toolboxPanelId: this.id,
         inspectedWindowTabId: getTargetTabIdForToolbox(toolbox),
       },
     });
 
     browser.loadURI(url);
-
-    toolbox.on("select", this.onToolboxPanelSelect);
-
-    // Return a cleanup method that is when the panel is destroyed, e.g.
-    // - when addon devtool panel has been disabled by the user from the toolbox preferences,
-    //   its ParentDevToolsPanel instance is still valid, but the built devtools panel is removed from
-    //   the toolbox (and re-built again if the user re-enable it from the toolbox preferences panel)
-    // - when the creator context has been destroyed, the ParentDevToolsPanel close method is called,
-    //   it remove the tool definition from the toolbox, which will call this destroy method.
-    return () => {
-      unwatchExtensionProxyContextLoad();
-      browser.remove();
-      toolbox.off("select", this.onToolboxPanelSelect);
-
-      // If the panel has been disabled from the toolbox preferences,
-      // we need to re-initialize the waitTopLevelContext Promise.
-      this.waitTopLevelContext = new Promise(resolve => {
-        this._resolveTopLevelContext = resolve;
-      });
-    };
   }
 
-  onToolboxPanelSelect(what, id) {
-    if (!this.waitTopLevelContext || !this.panelAdded) {
-      return;
-    }
-    if (!this.visible && id === this.id) {
-      // Wait that the panel is fully loaded and emit show.
-      this.waitTopLevelContext.then(() => {
-        this.visible = true;
-        this.context.parentMessageManager.sendAsyncMessage("Extension:DevToolsPanelShown", {
-          toolboxPanelId: this.id,
-        });
-      });
-    } else if (this.visible && id !== this.id) {
-      this.visible = false;
-      this.context.parentMessageManager.sendAsyncMessage("Extension:DevToolsPanelHidden", {
-        toolboxPanelId: this.id,
-      });
-    }
-  }
-
-  close() {
-    const {toolbox} = this;
-
-    if (!toolbox) {
-      throw new Error("Unable to destroy a closed devtools panel");
+  destroyBrowserElement() {
+    const {browser, unwatchExtensionProxyContextLoad} = this;
+    if (unwatchExtensionProxyContextLoad) {
+      this.unwatchExtensionProxyContextLoad = null;
+      unwatchExtensionProxyContextLoad();
     }
 
-    // Explicitly remove the panel if it is registered and the toolbox is not
-    // closing itself.
-    if (this.panelAdded && toolbox.isToolRegistered(this.id) && !toolbox._destroyer) {
-      toolbox.removeAdditionalTool(this.id);
+    if (browser) {
+      browser.remove();
+      this.browser = null;
     }
 
-    this.context = null;
-    this.toolbox = null;
-    this.waitTopLevelContext = null;
-    this._resolveTopLevelContext = null;
+    // If the panel has been removed or disabled (e.g. from the toolbox preferences
+    // or during the toolbox switching between docked and undocked),
+    // we need to re-initialize the waitTopLevelContext Promise.
+    this.waitTopLevelContext = new Promise(resolve => {
+      this._resolveTopLevelContext = resolve;
+    });
   }
 }
 
 class DevToolsSelectionObserver extends EventEmitter {
   constructor(context) {
     if (!context.devToolsToolbox) {
       // This should never happen when this constructor is called with a valid
       // devtools extension context.
--- a/devtools/client/framework/toolbox.js
+++ b/devtools/client/framework/toolbox.js
@@ -1521,16 +1521,21 @@ Toolbox.prototype = {
 
   /**
    * Unregister and unload an additional tool from this particular toolbox.
    *
    * @param {string} toolId
    *        the id of the additional tool to unregister and remove.
    */
   removeAdditionalTool(toolId) {
+    // Early exit if the toolbox is already destroying itself.
+    if (this._destroyer) {
+      return;
+    }
+
     if (!this.hasAdditionalTool(toolId)) {
       throw new Error("Tool definition not registered to this toolbox: " +
                       toolId);
     }
 
     this.additionalToolDefinitions.delete(toolId);
     this.visibleAdditionalTools = this.visibleAdditionalTools
                                       .filter(id => id !== toolId);