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 291359 6b710da110000b8d4b0116281fa319ab78879fbe
parent 291358 3eb3406f08b594fe2b54926c6649eb8c5347104d
child 291360 d03f089bbab80cd36c4b0e82cbbaa1eebdb52528
push id74545
push userkwierso@gmail.com
push dateFri, 01 Apr 2016 23:05:42 +0000
treeherdermozilla-inbound@c410d4e20586 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjryans
bugs1247203
milestone48.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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");