Bug 862363 - Sync the killswitch upon (un)registering of a tool and respect it in Options Panel, r=jwalker

browser/devtools/framework/ToolDefinitions.jsm

@@ -201,13 +201,10 @@ this.defaultTools = [
   webConsoleDefinition,
   debuggerDefinition,
   inspectorDefinition,
+  profilerDefinition,
   netMonitorDefinition
 ];
 
-if (Services.prefs.getBoolPref("devtools.profiler.enabled")) {
-  defaultTools.push(profilerDefinition);
-}
-
 /**
  * Lookup l10n string from a string bundle.
  *

browser/devtools/framework/Toolbox.jsm

@@ -670,10 +670,15 @@ Toolbox.prototype = {
    * 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
+   * @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.
    */
   _toolUnregistered: function TBOX_toolUnregistered(event, toolId) {
+    if (typeof toolId != "string") {
+      toolId = toolId.id;
+    }
+
     if (this._toolPanels.has(toolId)) {
       let instance = this._toolPanels.get(toolId);
       instance.destroy();

browser/devtools/framework/gDevTools.jsm

@@ -83,17 +83,26 @@ DevTools.prototype = {
    * Removes all tools that match the given |toolId|
    * Needed so that add-ons can remove themselves when they are deactivated
    *
-   * @param {string} toolId
-   *        id of the tool to unregister
+   * @param {string|object} tool
+   *        Definition or the id of the tool to unregister. Passing the
+   *        tool id should be avoided as it is a temporary measure.
    * @param {boolean} isQuitApplication
    *        true to indicate that the call is due to app quit, so we should not
    *        cause a cascade of costly events
    */
-  unregisterTool: function DT_unregisterTool(toolId, isQuitApplication) {
+  unregisterTool: function DT_unregisterTool(tool, isQuitApplication) {
+    let toolId = null;
+    if (typeof tool == "string") {
+      toolId = tool;
+      tool = this._tools.get(tool);
+    }
+    else {
+      toolId = tool.id;
+    }
     this._tools.delete(toolId);
 
     if (!isQuitApplication) {
-      this.emit("tool-unregistered", toolId);
+      this.emit("tool-unregistered", tool);
     }
   },
 
@@ -382,12 +391,16 @@ let gDevToolsBrowser = {
   },
 
   /**
-   * Add the menuitem for a tool to all open browser windows.
+   * Add the menuitem for a tool to all open browser windows. Also toggles the
+   * kill switch preference of the tool.
    *
    * @param {object} toolDefinition
    *        properties of the tool to add
    */
   _addToolToWindows: function DT_addToolToWindows(toolDefinition) {
+    // Set the kill switch pref boolean to true
+    Services.prefs.setBoolPref(toolDefinition.killswitch, true);
+
     // We need to insert the new tool in the right place, which means knowing
     // the tool that comes before the tool that we're trying to add
     let allDefs = gDevTools.getToolDefinitionArray();
@@ -567,12 +580,16 @@ let gDevToolsBrowser = {
   },
 
   /**
-   * Remove the menuitem for a tool to all open browser windows.
+   * Remove the menuitem for a tool to all open browser windows. Also sets the
+   * kill switch boolean pref to false.
    *
    * @param {object} toolId
    *        id of the tool to remove
+   * @param {string} killswitch
+   *        The kill switch preference string of the tool
    */
-  _removeToolFromWindows: function DT_removeToolFromWindows(toolId) {
+  _removeToolFromWindows: function DT_removeToolFromWindows(toolId, killswitch) {
+    Services.prefs.setBoolPref(killswitch, false);
     for (let win of gDevToolsBrowser._trackedBrowserWindows) {
       gDevToolsBrowser._removeToolFromMenu(toolId, win.document);
     }
@@ -650,7 +667,15 @@ gDevTools.on("tool-registered", function(ev, toolId) {
 });
 
 gDevTools.on("tool-unregistered", function(ev, toolId) {
-  gDevToolsBrowser._removeToolFromWindows(toolId);
+  let killswitch;
+  if (typeof toolId == "string") {
+    killswitch = "devtools." + toolId + ".enabled";
+  }
+  else {
+    killswitch = toolId.killswitch;
+    toolId = toolId.id;
+  }
+  gDevToolsBrowser._removeToolFromWindows(toolId, killswitch);
 });
 
 gDevTools.on("toolbox-ready", gDevToolsBrowser._updateMenuCheckbox);

browser/devtools/framework/test/browser_toolbox_dynamic_registration.js

@@ -77,8 +77,9 @@ function testUnregister()
   gDevTools.unregisterTool("test-tool");
 }
 
-function toolUnregistered(event, toolId)
+function toolUnregistered(event, toolDefinition)
 {
+  let toolId = toolDefinition.id;
   is(toolId, "test-tool", "tool-unregistered event handler sent tool id");
 
   ok(!gDevTools.getToolDefinitionMap().has(toolId), "tool removed from map");

browser/devtools/framework/test/browser_toolbox_options.js

@@ -109,16 +109,17 @@ function toggleTools() {
 }
 
 function checkUnregistered(event, data) {
-  if (data == prefNodes[index].getAttribute("id")) {
+  if (data.id == prefNodes[index].getAttribute("id")) {
     ok(true, "Correct tool removed");
     // checking tab on the toolbox
-    ok(!doc.getElementById("toolbox-tab-" + data), "Tab removed for " + data);
+    ok(!doc.getElementById("toolbox-tab-" + data.id), "Tab removed for " +
+       data.id);
     index++;
     // Wait for the next turn of the event loop to avoid stack overflow errors.
     executeSoon(toggleTools);
     return;
   }
-  ok(false, "Something went wrong, " + data + " was not unregistered");
+  ok(false, "Something went wrong, " + data.id + " was not unregistered");
   cleanup();
 }
 

browser/devtools/framework/toolbox-options.js

@@ -32,7 +32,7 @@ function setupToolsList() {
     else {
       disabledTools.push(id);
       Services.prefs.setCharPref(DISABLED_TOOLS, JSON.stringify(disabledTools));
-      gDevTools.emit("tool-unregistered", id);
+      gDevTools.emit("tool-unregistered", gDevTools._tools.get(id));
     }
   };