Bug 1247203 - Cleanup relations between main client module and devtools-browser. r=jryans
authorAlexandre Poirot <poirot.alex@gmail.com>
Fri, 01 Apr 2016 05:48:59 -0700
changeset 291386 6b710da110000b8d4b0116281fa319ab78879fbe
parent 291385 3eb3406f08b594fe2b54926c6649eb8c5347104d
child 291387 d03f089bbab80cd36c4b0e82cbbaa1eebdb52528
push id19656
push usergwagner@mozilla.com
push dateMon, 04 Apr 2016 13:43:23 +0000
treeherderb2g-inbound@e99061fde28a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjryans
bugs1247203
milestone48.0a1
Bug 1247203 - Cleanup relations between main client module and devtools-browser. r=jryans
devtools/client/devtools-startup.js
devtools/client/framework/devtools-browser.js
devtools/client/framework/devtools.js
devtools/client/framework/toolbox-process-window.js
devtools/client/main.js
--- a/devtools/client/devtools-startup.js
+++ b/devtools/client/devtools-startup.js
@@ -4,18 +4,18 @@
 
 /* FIXME: remove this globals comment and replace with import-globals-from when
    bug 1242893 is fixed */
 /* globals BrowserToolboxProcess */
 
 /**
  * This XPCOM component is loaded very early.
  * It handles command line arguments like -jsconsole, but also ensures starting
- * core modules like devtools/devtools-browser that listen for application
- * startup.
+ * core modules like 'devtools-browser.js' that hooks the browser windows
+ * and ensure setting up tools.
  *
  * Be careful to lazy load dependencies as much as possible.
  **/
 
 "use strict";
 
 const { interfaces: Ci, utils: Cu } = Components;
 const kDebuggerPrefs = [
@@ -62,22 +62,20 @@ DevToolsStartup.prototype = {
         this.handleDevToolsFlag(window);
       }
     }.bind(this);
     Services.obs.addObserver(onStartup, "browser-delayed-startup-finished",
                              false);
   },
 
   initDevTools: function() {
-    let { require } = Cu.import("resource://devtools/shared/Loader.jsm", {});
+    let { loader } = Cu.import("resource://devtools/shared/Loader.jsm", {});
     // Ensure loading main devtools module that hooks up into browser UI
     // and initialize all devtools machinery.
-    // browser.xul or main top-level document used to load this module,
-    // but this code may be called without/before it.
-    require("devtools/client/framework/devtools-browser");
+    loader.main("devtools/client/main");
   },
 
   handleConsoleFlag: function(cmdLine) {
     let window = Services.wm.getMostRecentWindow("devtools:webconsole");
     if (!window) {
       this.initDevTools();
 
       let { require } = Cu.import("resource://devtools/shared/Loader.jsm", {});
--- a/devtools/client/framework/devtools-browser.js
+++ b/devtools/client/framework/devtools-browser.js
@@ -11,17 +11,18 @@
  * This module is loaded lazily by devtools-clhandler.js, once the first
  * browser window is ready (i.e. fired browser-delayed-startup-finished event)
  **/
 
 const {Cc, Ci, Cu} = require("chrome");
 const Services = require("Services");
 const promise = require("promise");
 const Telemetry = require("devtools/client/shared/telemetry");
-const {gDevTools} = require("./devtools");
+const { gDevTools } = require("./devtools");
+const { when: unload } = require("sdk/system/unload");
 
 // Load target and toolbox lazily as they need gDevTools to be fully initialized
 loader.lazyRequireGetter(this, "TargetFactory", "devtools/client/framework/target", true);
 loader.lazyRequireGetter(this, "Toolbox", "devtools/client/framework/toolbox", true);
 loader.lazyRequireGetter(this, "DebuggerServer", "devtools/server/main", true);
 loader.lazyRequireGetter(this, "DebuggerClient", "devtools/shared/client/main", true);
 
 loader.lazyImporter(this, "CustomizableUI", "resource:///modules/CustomizableUI.jsm");
@@ -859,16 +860,20 @@ var gDevToolsBrowser = exports.gDevTools
     gDevToolsBrowser._telemetry = null;
 
     for (let win of gDevToolsBrowser._trackedBrowserWindows) {
       gDevToolsBrowser._forgetBrowserWindow(win);
     }
   },
 }
 
+// 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);
 });
 
 gDevTools.on("tool-unregistered", function(ev, toolId) {
   if (typeof toolId != "string") {
     toolId = toolId.id;
@@ -876,23 +881,23 @@ gDevTools.on("tool-unregistered", functi
   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);
+
 // Fake end of browser window load event for all already opened windows
 // that is already fully loaded.
 let enumerator = Services.wm.getEnumerator("navigator:browser");
 while (enumerator.hasMoreElements()) {
   let win = enumerator.getNext();
   if (win.gBrowserInit && win.gBrowserInit.delayedStartupFinished) {
     gDevToolsBrowser._registerBrowserWindow(win);
   }
 }
 
-// Load the browser devtools main module as the loader's main module.
-// This is done precisely here as main.js ends up dispatching the
-// tool-registered events we are listening in this module.
-loader.main("devtools/client/main");
-
+// Watch for module loader unload. Fires when the tools are reloaded.
+unload(function () {
+  gDevToolsBrowser.destroy();
+});
--- a/devtools/client/framework/devtools.js
+++ b/devtools/client/framework/devtools.js
@@ -11,42 +11,46 @@ const promise = require("promise");
 loader.lazyRequireGetter(this, "Toolbox", "devtools/client/framework/toolbox", true);
 loader.lazyRequireGetter(this, "gDevToolsBrowser", "devtools/client/framework/devtools-browser", true);
 
 const {defaultTools: DefaultTools, defaultThemes: DefaultThemes} =
   require("devtools/client/definitions");
 const EventEmitter = require("devtools/shared/event-emitter");
 const {JsonView} = require("devtools/client/jsonview/main");
 const AboutDevTools = require("devtools/client/framework/about-devtools-toolbox");
+const {when: unload} = require("sdk/system/unload");
 
 const FORBIDDEN_IDS = new Set(["toolbox", ""]);
 const MAX_ORDINAL = 99;
 
 /**
  * DevTools is a class that represents a set of developer tools, it holds a
  * set of tools and keeps track of open toolboxes in the browser.
  */
 this.DevTools = function DevTools() {
   this._tools = new Map();     // Map<toolId, tool>
   this._themes = new Map();    // Map<themeId, theme>
   this._toolboxes = new Map(); // Map<target, toolbox>
 
   // destroy() is an observer's handler so we need to preserve context.
   this.destroy = this.destroy.bind(this);
-  this._teardown = this._teardown.bind(this);
 
   // JSON Viewer for 'application/json' documents.
   JsonView.initialize();
 
   AboutDevTools.register();
 
   EventEmitter.decorate(this);
 
-  Services.obs.addObserver(this._teardown, "devtools-unloaded", false);
   Services.obs.addObserver(this.destroy, "quit-application", false);
+
+  // This is important step in initialization codepath where we are going to
+  // start registering all default tools and themes: create menuitems, keys, emit
+  // related events.
+  this.registerDefaults();
 };
 
 DevTools.prototype = {
   registerDefaults() {
     // Ensure registering items in the sorted order (getDefault* functions
     // return sorted lists)
     this.getDefaultTools().forEach(definition => this.registerTool(definition));
     this.getDefaultThemes().forEach(definition => this.registerTheme(definition));
@@ -476,33 +480,38 @@ DevTools.prototype = {
     AboutDevTools.unregister();
   },
 
   /**
    * All browser windows have been closed, tidy up remaining objects.
    */
   destroy: function() {
     Services.obs.removeObserver(this.destroy, "quit-application");
-    Services.obs.removeObserver(this._teardown, "devtools-unloaded");
 
     for (let [key, tool] of this.getToolDefinitionMap()) {
       this.unregisterTool(key, true);
     }
 
     JsonView.destroy();
 
+    gDevTools.unregisterDefaults();
+
     // Cleaning down the toolboxes: i.e.
     //   for (let [target, toolbox] of this._toolboxes) toolbox.destroy();
     // Is taken care of by the gDevToolsBrowser.forgetBrowserWindow
   },
 
   /**
    * Iterator that yields each of the toolboxes.
    */
   *[Symbol.iterator]() {
     for (let toolbox of this._toolboxes) {
       yield toolbox;
     }
   }
 };
 
-exports.gDevTools = new DevTools();
+const gDevTools = exports.gDevTools = new DevTools();
 
+// Watch for module loader unload. Fires when the tools are reloaded.
+unload(function () {
+  gDevTools._teardown();
+});
--- a/devtools/client/framework/toolbox-process-window.js
+++ b/devtools/client/framework/toolbox-process-window.js
@@ -4,20 +4,19 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 var { classes: Cc, interfaces: Ci, utils: Cu } = Components;
 
 var { loader, require } = Cu.import("resource://devtools/shared/Loader.jsm", {});
-// Require this module just to setup things like themes and tools
-// devtools-browser is special as it loads main module
-// To be cleaned up in bug 1247203.
-require("devtools/client/framework/devtools-browser");
+// Require this module to setup core modules
+loader.main("devtools/client/main");
+
 var { gDevTools } = require("devtools/client/framework/devtools");
 var { TargetFactory } = require("devtools/client/framework/target");
 var { Toolbox } = require("devtools/client/framework/toolbox");
 var Services = require("Services");
 var { DebuggerClient } = require("devtools/shared/client/main");
 var { PrefsHelper } = require("devtools/client/shared/prefs");
 var { Task } = Cu.import("resource://gre/modules/Task.jsm", {});
 
--- a/devtools/client/main.js
+++ b/devtools/client/main.js
@@ -1,30 +1,22 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
-const Services = require("Services");
-const { gDevTools } = require("devtools/client/framework/devtools");
+/**
+ * This module could have been devtools-browser.js.
+ * But we need this wrapper in order to define precisely what we are exporting
+ * out of client module loader (Loader.jsm): only Toolbox and TargetFactory.
+ */
 
-// This is important step in initialization codepath where we are going to
-// start registering all default tools and themes: create menuitems, keys, emit
-// related events.
-gDevTools.registerDefaults();
-
+// For compatiblity reasons, exposes these symbols on "devtools":
 Object.defineProperty(exports, "Toolbox", {
   get: () => require("devtools/client/framework/toolbox").Toolbox
 });
 Object.defineProperty(exports, "TargetFactory", {
   get: () => require("devtools/client/framework/target").TargetFactory
 });
 
-const unloadObserver = {
-  observe: function(subject) {
-    if (subject.wrappedJSObject === require("@loader/unload")) {
-      Services.obs.removeObserver(unloadObserver, "sdk:loader:destroy");
-      gDevTools.unregisterDefaults();
-    }
-  }
-};
-Services.obs.addObserver(unloadObserver, "sdk:loader:destroy", false);
+// Load the main browser module
+require("devtools/client/framework/devtools-browser");