Bug 1308912 - Simplify tool-unregistered by only accepting a string toolId. r=ochameau
authorLuca Greco <lgreco@mozilla.com>
Fri, 14 Oct 2016 17:54:56 +0200
changeset 323011 df6ef63c73e302aac8d126b844d6bcdaf33be0da
parent 323010 3478d56c55a26cf90445b5c0c077a4b20d617141
child 323012 381c0d6fd9b9d19fb8e55bf57e897fa5130502cd
push id21
push usermaklebus@msu.edu
push dateThu, 01 Dec 2016 06:22:08 +0000
reviewersochameau
bugs1308912
milestone53.0a1
Bug 1308912 - Simplify tool-unregistered by only accepting a string toolId. r=ochameau MozReview-Commit-ID: 3QjWIAwol6A
devtools/client/framework/devtools-browser.js
devtools/client/framework/devtools.js
devtools/client/framework/test/browser_toolbox_dynamic_registration.js
devtools/client/framework/test/browser_toolbox_options.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
@@ -725,19 +725,16 @@ 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);
 });
 
 gDevTools.on("tool-unregistered", function (ev, toolId) {
-  if (typeof toolId != "string") {
-    toolId = toolId.id;
-  }
   gDevToolsBrowser._removeToolFromWindows(toolId);
 });
 
 gDevTools.on("toolbox-ready", gDevToolsBrowser._updateMenuCheckbox);
 gDevTools.on("toolbox-destroyed", gDevToolsBrowser._updateMenuCheckbox);
 
 Services.obs.addObserver(gDevToolsBrowser.destroy, "quit-application", false);
 Services.obs.addObserver(gDevToolsBrowser, "browser-delayed-startup-finished", false);
--- a/devtools/client/framework/devtools.js
+++ b/devtools/client/framework/devtools.js
@@ -135,22 +135,26 @@ DevTools.prototype = {
    */
   unregisterTool: function DT_unregisterTool(tool, isQuitApplication) {
     let toolId = null;
     if (typeof tool == "string") {
       toolId = tool;
       tool = this._tools.get(tool);
     }
     else {
+      let {Deprecated} = Cu.import("resource://gre/modules/Deprecated.jsm", {});
+      Deprecated.warning("Deprecation WARNING: gDevTools.unregisterTool(tool) is deprecated. " +
+                         "You should unregister a tool using its toolId: " +
+                         "gDevTools.unregisterTool(toolId).");
       toolId = tool.id;
     }
     this._tools.delete(toolId);
 
     if (!isQuitApplication) {
-      this.emit("tool-unregistered", tool);
+      this.emit("tool-unregistered", toolId);
     }
   },
 
   /**
    * Sorting function used for sorting tools based on their ordinals.
    */
   ordinalSort: function DT_ordinalSort(d1, d2) {
     let o1 = (typeof d1.ordinal == "number") ? d1.ordinal : MAX_ORDINAL;
--- a/devtools/client/framework/test/browser_toolbox_dynamic_registration.js
+++ b/devtools/client/framework/test/browser_toolbox_dynamic_registration.js
@@ -66,19 +66,18 @@ function getAllBrowserWindows() {
 
 function testUnregister()
 {
   gDevTools.once("tool-unregistered", toolUnregistered);
 
   gDevTools.unregisterTool("test-tool");
 }
 
-function toolUnregistered(event, toolDefinition)
+function toolUnregistered(event, toolId)
 {
-  let toolId = toolDefinition.id;
   is(toolId, "test-tool", "tool-unregistered event handler sent tool id");
 
   ok(!gDevTools.getToolDefinitionMap().has(toolId), "tool removed from map");
 
   // test that it disappeared from the UI
   let doc = toolbox.doc;
   let tab = doc.getElementById("toolbox-tab-" + toolId);
   ok(!tab, "tool's tab was removed from the toolbox UI");
--- a/devtools/client/framework/test/browser_toolbox_options.js
+++ b/devtools/client/framework/test/browser_toolbox_options.js
@@ -230,17 +230,17 @@ function* toggleTool(node) {
   }
   node.scrollIntoView();
   EventUtils.synthesizeMouseAtCenter(node, {}, panelWin);
 
   yield deferred.promise;
 }
 
 function checkUnregistered(toolId, deferred, event, data) {
-  if (data.id == toolId) {
+  if (data == toolId) {
     ok(true, "Correct tool removed");
     // checking tab on the toolbox
     ok(!doc.getElementById("toolbox-tab-" + toolId),
       "Tab removed for " + toolId);
   } else {
     ok(false, "Something went wrong, " + toolId + " was not unregistered");
   }
   deferred.resolve();
--- a/devtools/client/framework/toolbox-options.js
+++ b/devtools/client/framework/toolbox-options.js
@@ -185,21 +185,17 @@ OptionsPanel.prototype = {
     let toolsNotSupportedLabel = this.panelDoc.getElementById(
       "tools-not-supported-label");
     let atleastOneToolNotSupported = false;
 
     let onCheckboxClick = function (id) {
       let toolDefinition = gDevTools._tools.get(id);
       // Set the kill switch pref boolean to true
       Services.prefs.setBoolPref(toolDefinition.visibilityswitch, this.checked);
-      if (this.checked) {
-        gDevTools.emit("tool-registered", id);
-      } else {
-        gDevTools.emit("tool-unregistered", toolDefinition);
-      }
+      gDevTools.emit(this.checked ? "tool-registered" : "tool-unregistered", id);
     };
 
     let createToolCheckbox = tool => {
       let checkboxLabel = this.panelDoc.createElement("label");
       let checkboxInput = this.panelDoc.createElement("input");
       checkboxInput.setAttribute("type", "checkbox");
       checkboxInput.setAttribute("id", tool.id);
       checkboxInput.setAttribute("title", tool.tooltip || "");
--- a/devtools/client/framework/toolbox.js
+++ b/devtools/client/framework/toolbox.js
@@ -1930,25 +1930,20 @@ Toolbox.prototype = {
     // 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|object} toolId
-   *         Definition or id of the tool that was unregistered. Passing the
-   *         tool id should be avoided as it is a temporary measure.
+   * @param  {string} toolId
+   *         id of the tool that was unregistered.
    */
   _toolUnregistered: function (event, toolId) {
-    if (typeof toolId != "string") {
-      toolId = toolId.id;
-    }
-
     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);