Bug 1308912 - Add support for addon tools registered to a specific DevTools toolbox. draft
authorLuca Greco <lgreco@mozilla.com>
Thu, 03 Nov 2016 18:41:26 +0100
changeset 438596 8d6e6b5327887a3acf4c82908dfcb91ee2cc4fb1
parent 438595 b4ffacea1338a604ef5bf28eaffdc4067077f8b8
child 536958 77c448195efed1d318fd5962feb3e1fbd7c0ed41
push id35773
push userluca.greco@alcacoop.it
push dateMon, 14 Nov 2016 20:52:26 +0000
bugs1308912
milestone53.0a1
Bug 1308912 - Add support for addon tools registered to a specific DevTools toolbox. MozReview-Commit-ID: 7DzyXLGOs5w
devtools/client/framework/devtools-browser.js
devtools/client/framework/test/browser.ini
devtools/client/framework/test/browser_toolbox_options.js
devtools/client/framework/test/browser_toolbox_tools_per_toolbox_registration.js
devtools/client/framework/toolbox-options.js
devtools/client/framework/toolbox.js
--- a/devtools/client/framework/devtools-browser.js
+++ b/devtools/client/framework/devtools-browser.js
@@ -721,17 +721,21 @@ var gDevToolsBrowser = exports.gDevTools
 };
 
 // Handle all already registered tools,
 gDevTools.getToolDefinitionArray()
          .forEach(def => gDevToolsBrowser._addToolToWindows(def));
 // and the new ones.
 gDevTools.on("tool-registered", function (ev, toolId) {
   let toolDefinition = gDevTools._tools.get(toolId);
-  gDevToolsBrowser._addToolToWindows(toolDefinition);
+  // If the tool has been registered globally, add to all the
+  // available windows.
+  if (toolDefinition) {
+    gDevToolsBrowser._addToolToWindows(toolDefinition);
+  }
 });
 
 gDevTools.on("tool-unregistered", function (ev, toolId) {
   gDevToolsBrowser._removeToolFromWindows(toolId);
 });
 
 gDevTools.on("toolbox-ready", gDevToolsBrowser._updateMenuCheckbox);
 gDevTools.on("toolbox-destroyed", gDevToolsBrowser._updateMenuCheckbox);
--- a/devtools/client/framework/test/browser.ini
+++ b/devtools/client/framework/test/browser.ini
@@ -73,16 +73,17 @@ skip-if = e10s # Bug 1069044 - destroyIn
 [browser_toolbox_split_console.js]
 [browser_toolbox_target.js]
 [browser_toolbox_tabsswitch_shortcuts.js]
 [browser_toolbox_textbox_context_menu.js]
 [browser_toolbox_theme_registration.js]
 [browser_toolbox_toggle.js]
 [browser_toolbox_tool_ready.js]
 [browser_toolbox_tool_remote_reopen.js]
+[browser_toolbox_tools_per_toolbox_registration.js]
 [browser_toolbox_transport_events.js]
 [browser_toolbox_view_source_01.js]
 [browser_toolbox_view_source_02.js]
 [browser_toolbox_view_source_03.js]
 [browser_toolbox_view_source_04.js]
 [browser_toolbox_window_reload_target.js]
 [browser_toolbox_window_shortcuts.js]
 skip-if = os == "mac" && os_version == "10.8" || os == "win" && os_version == "5.1" # Bug 851129 - Re-enable browser_toolbox_window_shortcuts.js test after leaks are fixed
--- a/devtools/client/framework/test/browser_toolbox_options.js
+++ b/devtools/client/framework/test/browser_toolbox_options.js
@@ -16,16 +16,17 @@ const L10N = new LocalizationHelper("dev
 add_task(function* () {
   const URL = "data:text/html;charset=utf8,test for dynamically registering " +
               "and unregistering tools";
   registerNewTool();
   let tab = yield addTab(URL);
   let target = TargetFactory.forTab(tab);
   toolbox = yield gDevTools.showToolbox(target);
   doc = toolbox.doc;
+  yield registerNewPerToolboxTool();
   yield testSelectTool();
   yield testOptionsShortcut();
   yield testOptions();
   yield testToggleTools();
   yield cleanup();
 });
 
 function registerNewTool() {
@@ -41,16 +42,41 @@ function registerNewTool() {
   ok(!gDevTools.getToolDefinitionMap().has("test-tool"),
     "The tool is not registered");
 
   gDevTools.registerTool(toolDefinition);
   ok(gDevTools.getToolDefinitionMap().has("test-tool"),
     "The tool is registered");
 }
 
+function registerNewPerToolboxTool() {
+  let toolDefinition = {
+    id: "test-pertoolbox-tool",
+    isTargetSupported: () => true,
+    visibilityswitch: "devtools.test-pertoolbox-tool.enabled",
+    url: "about:blank",
+    label: "perToolboxSomeLabel"
+  };
+
+  ok(gDevTools, "gDevTools exists");
+  ok(!gDevTools.getToolDefinitionMap().has("test-pertoolbox-tool"),
+     "The per-toolbox tool is not registered globally");
+
+  ok(toolbox, "toolbox exists");
+  ok(!toolbox.hasAdditionalTool("test-pertoolbox-tool"),
+     "The per-toolbox tool is not yet registered to the toolbox");
+
+  toolbox.addAdditionalTool(toolDefinition);
+
+  ok(!gDevTools.getToolDefinitionMap().has("test-pertoolbox-tool"),
+     "The per-toolbox tool is not registered globally");
+  ok(toolbox.hasAdditionalTool("test-pertoolbox-tool"),
+     "The per-toolbox tool has been registered to the toolbox");
+}
+
 function* testSelectTool() {
   info("Checking to make sure that the options panel can be selected.");
 
   let onceSelected = toolbox.once("options-selected");
   toolbox.selectTool("options");
   yield onceSelected;
   ok(true, "Toolbox selected via selectTool method");
 }
@@ -163,19 +189,23 @@ function* testMouseClick(node, prefValue
 }
 
 function* testToggleTools() {
   let toolNodes = panelWin.document.querySelectorAll(
     "#default-tools-box input[type=checkbox]:not([data-unsupported])," +
     "#additional-tools-box input[type=checkbox]:not([data-unsupported])");
   let enabledTools = [...toolNodes].filter(node => node.checked);
 
-  let toggleableTools = gDevTools.getDefaultTools().filter(tool => {
-    return tool.visibilityswitch;
-  }).concat(gDevTools.getAdditionalTools());
+  let toggleableTools = gDevTools.getDefaultTools()
+                                 .filter(tool => {
+                                   return tool.visibilityswitch;
+                                 })
+                                 .concat(gDevTools.getAdditionalTools())
+                                 .concat(toolbox.getAdditionalTools());
+
 
   for (let node of toolNodes) {
     let id = node.getAttribute("id");
     ok(toggleableTools.some(tool => tool.id === id),
       "There should be a toggle checkbox for: " + id);
   }
 
   // Store modified pref names so that they can be cleared on error.
copy from devtools/client/framework/test/browser_toolbox_dynamic_registration.js
copy to devtools/client/framework/test/browser_toolbox_tools_per_toolbox_registration.js
--- a/devtools/client/framework/test/browser_toolbox_dynamic_registration.js
+++ b/devtools/client/framework/test/browser_toolbox_tools_per_toolbox_registration.js
@@ -1,104 +1,139 @@
 /* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* vim: set ft=javascript ts=2 et sw=2 tw=80: */
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
-const TEST_URL = "data:text/html,test for dynamically registering and unregistering tools";
-
-var toolbox;
+const TEST_URL = `data:text/html,<!DOCTYPE html>
+  <html>
+    <head>
+      <meta charset="utf-8">
+    </head>
+    <body>
+      test for registering and unregistering tools to a specific toolbox
+    </body>
+  </html>`;
 
-function test()
-{
+const TOOL_ID = "test-toolbox-tool";
+var toolbox;
+var target;
+
+function test() {
   addTab(TEST_URL).then(tab => {
-    let target = TargetFactory.forTab(tab);
-    gDevTools.showToolbox(target).then(testRegister);
+    target = TargetFactory.forTab(tab);
+
+    gDevTools.showToolbox(target)
+      .then(toolboxRegister)
+      .then(testToolRegistered);
   });
 }
 
-function testRegister(aToolbox)
-{
+var resolveToolInstanceBuild;
+var waitForToolInstanceBuild = new Promise((resolve) => {
+  resolveToolInstanceBuild = resolve;
+});
+
+var resolveToolInstanceDestroyed;
+var waitForToolInstanceDestroyed = new Promise((resolve) => {
+  resolveToolInstanceDestroyed = resolve;
+});
+
+function toolboxRegister(aToolbox) {
   toolbox = aToolbox;
-  gDevTools.once("tool-registered", toolRegistered);
+
+  var resolveToolInstanceBuild;
 
-  gDevTools.registerTool({
-    id: "test-tool",
-    label: "Test Tool",
+  waitForToolInstanceBuild = new Promise((resolve) => {
+    resolveToolInstanceBuild = resolve;
+  });
+
+  info("add per-toolbox tool in the opened toolbox.");
+
+  toolbox.addAdditionalTool({
+    id: TOOL_ID,
+    label: "per-toolbox Test Tool",
     inMenu: true,
     isTargetSupported: () => true,
-    build: function () {},
+    build: function () {
+      info("per-toolbox tool has been built.");
+      resolveToolInstanceBuild();
+
+      return {
+        destroy: () => {
+          info("per-toolbox tool has been destroyed.");
+          resolveToolInstanceDestroyed();
+        },
+      };
+    },
     key: "t"
   });
 }
 
-function toolRegistered(event, toolId)
-{
-  is(toolId, "test-tool", "tool-registered event handler sent tool id");
+function testToolRegistered() {
+  ok(!gDevTools.getToolDefinitionMap().has(TOOL_ID), "per-toolbox tool is not registered globally");
+  ok(toolbox.hasAdditionalTool(TOOL_ID),
+     "per-toolbox tool registered to the specific toolbox");
 
-  ok(gDevTools.getToolDefinitionMap().has(toolId), "tool added to map");
-
-  // test that it appeared in the UI
+  // Test that the tool appeared in the UI.
   let doc = toolbox.doc;
-  let tab = doc.getElementById("toolbox-tab-" + toolId);
+  let tab = doc.getElementById("toolbox-tab-" + TOOL_ID);
   ok(tab, "new tool's tab exists in toolbox UI");
 
-  let panel = doc.getElementById("toolbox-panel-" + toolId);
+  let panel = doc.getElementById("toolbox-panel-" + TOOL_ID);
   ok(panel, "new tool's panel exists in toolbox UI");
 
   for (let win of getAllBrowserWindows()) {
-    let key = win.document.getElementById("key_" + toolId);
-    ok(key, "key for new tool added to every browser window");
-    let menuitem = win.document.getElementById("menuitem_" + toolId);
-    ok(menuitem, "menu item of new tool added to every browser window");
+    let key = win.document.getElementById("key_" + TOOL_ID);
+    if (win.document == doc) {
+      continue;
+    }
+    ok(!key, "key for new tool should not exists in the other browser windows");
+    let menuitem = win.document.getElementById("menuitem_" + TOOL_ID);
+    ok(!menuitem, "menu item should not exists in the other browser window");
   }
 
-  // then unregister it
-  testUnregister();
+  // Test that the tool is built once selected and then test its unregistering.
+  info("select per-toolbox tool in the opened toolbox.");
+  gDevTools.showToolbox(target, TOOL_ID)
+           .then(waitForToolInstanceBuild)
+           .then(testUnregister);
 }
 
 function getAllBrowserWindows() {
   let wins = [];
   let enumerator = Services.wm.getEnumerator("navigator:browser");
   while (enumerator.hasMoreElements()) {
     wins.push(enumerator.getNext());
   }
   return wins;
 }
 
-function testUnregister()
-{
-  gDevTools.once("tool-unregistered", toolUnregistered);
+function testUnregister() {
+  info("remove per-toolbox tool in the opened toolbox.");
+  toolbox.removeAdditionalTool(TOOL_ID);
 
-  gDevTools.unregisterTool("test-tool");
+  Promise.all([
+    waitForToolInstanceDestroyed
+  ]).then(toolboxToolUnregistered);
 }
 
-function toolUnregistered(event, toolId)
-{
-  is(toolId, "test-tool", "tool-unregistered event handler sent tool id");
-
-  ok(!gDevTools.getToolDefinitionMap().has(toolId), "tool removed from map");
+function toolboxToolUnregistered() {
+  ok(!toolbox.hasAdditionalTool(TOOL_ID),
+     "per-toolbox tool unregistered from the specific toolbox");
 
   // test that it disappeared from the UI
   let doc = toolbox.doc;
-  let tab = doc.getElementById("toolbox-tab-" + toolId);
+  let tab = doc.getElementById("toolbox-tab-" + TOOL_ID);
   ok(!tab, "tool's tab was removed from the toolbox UI");
 
-  let panel = doc.getElementById("toolbox-panel-" + toolId);
+  let panel = doc.getElementById("toolbox-panel-" + TOOL_ID);
   ok(!panel, "tool's panel was removed from toolbox UI");
 
-  for (let win of getAllBrowserWindows()) {
-    let key = win.document.getElementById("key_" + toolId);
-    ok(!key, "key removed from every browser window");
-    let menuitem = win.document.getElementById("menuitem_" + toolId);
-    ok(!menuitem, "menu item removed from every browser window");
-  }
-
   cleanup();
 }
 
-function cleanup()
-{
+function cleanup() {
   toolbox.destroy();
   toolbox = null;
   gBrowser.removeCurrentTab();
   finish();
 }
--- a/devtools/client/framework/toolbox-options.js
+++ b/devtools/client/framework/toolbox-options.js
@@ -181,18 +181,22 @@ OptionsPanel.prototype = {
   setupToolsList: function () {
     let defaultToolsBox = this.panelDoc.getElementById("default-tools-box");
     let additionalToolsBox = this.panelDoc.getElementById(
       "additional-tools-box");
     let toolsNotSupportedLabel = this.panelDoc.getElementById(
       "tools-not-supported-label");
     let atleastOneToolNotSupported = false;
 
+    const toolbox = this.toolbox;
+
+    // Signal tool registering/unregistering globally (for the tools registered
+    // globally) and per toolbox (for the tools registered to a single toolbox).
     let onCheckboxClick = function (id) {
-      let toolDefinition = gDevTools._tools.get(id);
+      let toolDefinition = gDevTools._tools.get(id) || toolbox.getToolDefinition(id);
       // Set the kill switch pref boolean to true
       Services.prefs.setBoolPref(toolDefinition.visibilityswitch, this.checked);
       gDevTools.emit(this.checked ? "tool-registered" : "tool-unregistered", id);
     };
 
     let createToolCheckbox = tool => {
       let checkboxLabel = this.panelDoc.createElement("label");
       let checkboxInput = this.panelDoc.createElement("input");
@@ -234,16 +238,23 @@ OptionsPanel.prototype = {
 
     // Populating the additional tools list that came from add-ons.
     let atleastOneAddon = false;
     for (let tool of gDevTools.getAdditionalTools()) {
       atleastOneAddon = true;
       additionalToolsBox.appendChild(createToolCheckbox(tool));
     }
 
+    // Populating the additional toolbox-specific tools list that came
+    // from WebExtension add-ons.
+    for (let tool of this.toolbox.getAdditionalTools()) {
+      atleastOneAddon = true;
+      additionalToolsBox.appendChild(createToolCheckbox(tool));
+    }
+
     if (!atleastOneAddon) {
       additionalToolsBox.style.display = "none";
     }
 
     if (!atleastOneToolNotSupported) {
       toolsNotSupportedLabel.style.display = "none";
     }
 
--- a/devtools/client/framework/toolbox.js
+++ b/devtools/client/framework/toolbox.js
@@ -890,16 +890,17 @@ Toolbox.prototype = {
       toolbarHeight
     });
   },
 
   /**
    * Add tabs to the toolbox UI for registered tools
    */
   _buildTabs: function () {
+    // Build tabs for global registered tools.
     for (let definition of gDevTools.getToolDefinitionArray()) {
       this._buildTabForTool(definition);
     }
   },
 
   /**
    * Get all dev tools tab bar focusable elements. These are visible elements
    * such as buttons or elements with tabindex.
@@ -1248,16 +1249,91 @@ Toolbox.prototype = {
         });
       }
     }
 
     this._addKeysToWindow();
   },
 
   /**
+   * Lazily created map of the additional tools registered to this toolbox.
+   *
+   * @returns {Map<string, object>}
+   *          a map of the tools definitions registered to this
+   *          particular toolbox (the key is the toolId string, the value
+   *          is the tool definition plain javascript object).
+   */
+  get additionalToolDefinitions() {
+    if (!this._additionalToolDefinitions) {
+      this._additionalToolDefinitions = new Map();
+    }
+
+    return this._additionalToolDefinitions;
+  },
+
+  /**
+   * Retrieve the array of the additional tools registered to this toolbox.
+   *
+   * @return {Array<object>}
+   *         the array of additional tool definitions registered on this toolbox.
+   */
+  getAdditionalTools() {
+    return Array.from(this.additionalToolDefinitions.values());
+  },
+
+  /**
+   * Test the existence of a additional tools registered to this toolbox by tool id.
+   *
+   * @param {string} toolId
+   *        the id of the tool to test for existence.
+   *
+   * @return {boolean}
+   *
+   */
+  hasAdditionalTool(toolId) {
+    return this.additionalToolDefinitions.has(toolId);
+  },
+
+  /**
+   * Register and load an additional tool on this particular toolbox.
+   *
+   * @param {object} definition
+   *        the additional tool definition to register and add to this toolbox.
+   */
+  addAdditionalTool(definition) {
+    if (!definition.id) {
+      throw new Error("Tool definition id is missing");
+    }
+
+    if (this.isToolRegistered(definition.id)) {
+      throw new Error("Tool definition already registered: " +
+                      definition.id);
+    }
+
+    this.additionalToolDefinitions.set(definition.id, definition);
+    this._buildTabForTool(definition);
+  },
+
+  /**
+   * 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) {
+    if (!this.hasAdditionalTool(toolId)) {
+      throw new Error("Tool definition not registered to this toolbox: " +
+                      toolId);
+    }
+
+    this.unloadTool(toolId);
+    this.additionalToolDefinitions.delete(toolId);
+  },
+
+  /**
    * Ensure the tool with the given id is loaded.
    *
    * @param {string} id
    *        The id of the tool to load.
    */
   loadTool: function (id) {
     if (id === "inspector" && !this._inspector) {
       return this.initInspector().then(() => {
@@ -1275,17 +1351,19 @@ Toolbox.prototype = {
       } else {
         this.once(id + "-ready", initializedPanel => {
           deferred.resolve(initializedPanel);
         });
       }
       return deferred.promise;
     }
 
-    let definition = gDevTools.getToolDefinition(id);
+    // Retrieve the tool definition (from the global or the per-toolbox tool maps)
+    let definition = this.getToolDefinition(id);
+
     if (!definition) {
       deferred.reject(new Error("no such tool id " + id));
       return deferred.promise;
     }
 
     iframe = this.doc.createElement("iframe");
     iframe.className = "toolbox-panel-iframe";
     iframe.id = "toolbox-panel-iframe-" + id;
@@ -1902,48 +1980,59 @@ Toolbox.prototype = {
     // host switching button. We now have to restore the focus.
     this.focusTool(this.currentToolId, true);
 
     this.emit("host-changed");
     this._telemetry.log(HOST_HISTOGRAM, this._getTelemetryHostId());
   },
 
   /**
-   * Return if the tool is available as a tab (i.e. if it's checked
-   * in the options panel). This is different from Toolbox.getPanel -
-   * a tool could be registered but not yet opened in which case
-   * isToolRegistered would return true but getPanel would return false.
+   * Test the availability of a tool (both globally registered tools and
+   * additional tools registered to this toolbox) by tool id.
+   *
+   * @param  {string} toolId
+   *         Id of the tool definition to search in the per-toolbox or globally
+   *         registered tools.
+   *
+   * @returns {bool}
+   *         Returns true if the tool is registered globally or on this toolbox.
    */
   isToolRegistered: function (toolId) {
-    return gDevTools.getToolDefinitionMap().has(toolId);
+    return !!this.getToolDefinition(toolId);
   },
 
   /**
-   * Handler for the tool-registered event.
-   * @param  {string} event
-   *         Name of the event ("tool-registered")
+   * Return the tool definition registered globally or additional tools registered
+   * to this toolbox.
+   *
    * @param  {string} toolId
-   *         Id of the tool that was registered
+   *         Id of the tool definition to retrieve for the per-toolbox and globally
+   *         registered tools.
+   *
+   * @returns {object}
+   *         The plain javascript object that represents the requested tool definition.
    */
-  _toolRegistered: function (event, toolId) {
-    let tool = gDevTools.getToolDefinition(toolId);
-    this._buildTabForTool(tool);
-    // Emit the event so tools can listen to it from the toolbox level
-    // instead of gDevTools
-    this.emit("tool-registered", toolId);
+  getToolDefinition: function (toolId) {
+    return gDevTools.getToolDefinition(toolId) ||
+      this.additionalToolDefinitions.get(toolId);
   },
 
   /**
-   * Handler for the tool-unregistered event.
-   * @param  {string} event
-   *         Name of the event ("tool-unregistered")
+   * Internal helper that removes a loaded tool from the toolbox,
+   * it removes a loaded tool panel and tab from the toolbox without removing
+   * its definition, so that it can still be listed in options and re-added later.
+   *
    * @param  {string} toolId
-   *         id of the tool that was unregistered.
+   *         Id of the tool to be removed.
    */
-  _toolUnregistered: function (event, toolId) {
+  unloadTool: function (toolId) {
+    if (typeof toolId != "string") {
+      throw new Error("Unexpected non-string toolId received.");
+    }
+
     if (this._toolPanels.has(toolId)) {
       let instance = this._toolPanels.get(toolId);
       instance.destroy();
       this._toolPanels.delete(toolId);
     }
 
     let radio = this.doc.getElementById("toolbox-tab-" + toolId);
     let panel = this.doc.getElementById("toolbox-panel-" + toolId);
@@ -1970,16 +2059,48 @@ Toolbox.prototype = {
 
     if (this.hostType == Toolbox.HostType.WINDOW) {
       let doc = this.win.parent.document;
       let key = doc.getElementById("key_" + toolId);
       if (key) {
         key.parentNode.removeChild(key);
       }
     }
+  },
+
+  /**
+   * Handler for the tool-registered event.
+   * @param  {string} event
+   *         Name of the event ("tool-registered")
+   * @param  {string} toolId
+   *         Id of the tool that was registered
+   */
+  _toolRegistered: function (event, toolId) {
+    let tool = this.getToolDefinition(toolId);
+    if (!tool) {
+      // Ignore if the tool is not found, when a per-toolbox tool
+      // has been toggle in the toolbox options view, every toolbox will receive
+      // the toolbox-register and toolbox-unregister events.
+      return;
+    }
+    this._buildTabForTool(tool);
+    // Emit the event so tools can listen to it from the toolbox level
+    // instead of gDevTools.
+    this.emit("tool-registered", toolId);
+  },
+
+  /**
+   * Handler for the tool-unregistered event.
+   * @param  {string} event
+   *         Name of the event ("tool-unregistered")
+   * @param  {string} toolId
+   *         id of the tool that was unregistered
+   */
+  _toolUnregistered: function (event, toolId) {
+    this.unloadTool(toolId);
     // Emit the event so tools can listen to it from the toolbox level
     // instead of gDevTools
     this.emit("tool-unregistered", toolId);
   },
 
   /**
    * Initialize the inspector/walker/selection/highlighter fronts.
    * Returns a promise that resolves when the fronts are initialized