Bug 1248600 - Remove registerBrowserWindow/forgetBrowserWindow calls from /browser/. r=jryans

browser/base/content/browser.js

@@ -1291,9 +1291,6 @@ var gBrowserInit = {
 
     gBrowserThumbnails.init();
 
-    // Add Devtools menuitems and listeners
-    gDevToolsBrowser.registerBrowserWindow(window);
-
     gMenuButtonBadgeManager.init();
 
     gMenuButtonUpdateBadge.init();
@@ -1407,8 +1404,6 @@ var gBrowserInit = {
     if (!this._loadHandled)
       return;
 
-    gDevToolsBrowser.forgetBrowserWindow(window);
-
     let desc = Object.getOwnPropertyDescriptor(window, "DeveloperToolbar");
     if (desc && !desc.get) {
       DeveloperToolbar.destroy();

browser/installer/package-manifest.in

@@ -375,8 +375,8 @@
 @RESPATH@/browser/components/nsBrowserGlue.js
 @RESPATH@/browser/components/nsSetDefaultBrowser.manifest
 @RESPATH@/browser/components/nsSetDefaultBrowser.js
-@RESPATH@/browser/components/devtools-clhandler.manifest
-@RESPATH@/browser/components/devtools-clhandler.js
+@RESPATH@/browser/components/devtools-startup.manifest
+@RESPATH@/browser/components/devtools-startup.js
 @RESPATH@/browser/components/webideCli.js
 @RESPATH@/browser/components/webideComponents.manifest
 @RESPATH@/browser/components/Experiments.manifest

devtools/client/devtools-startup.js

@@ -6,6 +6,15 @@
    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.
+ *
+ * Be careful to lazy load dependencies as much as possible.
+ **/
+
 "use strict";
 
 const { interfaces: Ci, utils: Cu } = Components;
@@ -16,9 +25,9 @@ const kDebuggerPrefs = [
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "Services", "resource://gre/modules/Services.jsm");
 
-function devtoolsCommandlineHandler() {}
+function DevToolsStartup() {}
 
-devtoolsCommandlineHandler.prototype = {
+DevToolsStartup.prototype = {
   handle: function(cmdLine) {
     let consoleFlag = cmdLine.handleFlag("jsconsole", false);
     let debuggerFlag = cmdLine.handleFlag("jsdebugger", false);
@@ -30,9 +39,6 @@ devtoolsCommandlineHandler.prototype = {
     if (debuggerFlag) {
       this.handleDebuggerFlag(cmdLine);
     }
-    if (devtoolsFlag) {
-      this.handleDevToolsFlag();
-    }
     let debuggerServerFlag;
     try {
       debuggerServerFlag =
@@ -45,19 +51,36 @@ devtoolsCommandlineHandler.prototype = {
     if (debuggerServerFlag) {
       this.handleDebuggerServerFlag(cmdLine, debuggerServerFlag);
     }
+
+    let onStartup = function(window) {
+      Services.obs.removeObserver(onStartup,
+                                  "browser-delayed-startup-finished");
+      // Ensure loading core module once firefox is ready
+      this.initDevTools();
+
+      if (devtoolsFlag) {
+        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", {});
+    // 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");
   },
 
   handleConsoleFlag: function(cmdLine) {
     let window = Services.wm.getMostRecentWindow("devtools:webconsole");
     if (!window) {
-      let { require } = 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.
-      // Bug 1247203 should ease handling this.
-      require("devtools/client/framework/devtools-browser");
+      this.initDevTools();
 
+      let { require } = Cu.import("resource://devtools/shared/Loader.jsm", {});
       let hudservice = require("devtools/client/webconsole/hudservice");
       let { console } = Cu.import("resource://gre/modules/Console.jsm", {});
       hudservice.toggleBrowserConsole().then(null, console.error);
@@ -72,16 +95,12 @@ devtoolsCommandlineHandler.prototype = {
   },
 
   // Open the toolbox on the selected tab once the browser starts up.
-  handleDevToolsFlag: function() {
-    Services.obs.addObserver(function onStartup(window) {
-      Services.obs.removeObserver(onStartup,
-                                  "browser-delayed-startup-finished");
-      const {require} = Cu.import("resource://devtools/shared/Loader.jsm", {});
-      const {gDevTools} = require("devtools/client/framework/devtools");
-      const {TargetFactory} = require("devtools/client/framework/target");
-      let target = TargetFactory.forTab(window.gBrowser.selectedTab);
-      gDevTools.showToolbox(target);
-    }, "browser-delayed-startup-finished", false);
+  handleDevToolsFlag: function(window) {
+    const {require} = Cu.import("resource://devtools/shared/Loader.jsm", {});
+    const {gDevTools} = require("devtools/client/framework/devtools");
+    const {TargetFactory} = require("devtools/client/framework/target");
+    let target = TargetFactory.forTab(window.gBrowser.selectedTab);
+    gDevTools.showToolbox(target);
   },
 
   _isRemoteDebuggingEnabled() {
@@ -168,4 +187,4 @@ devtoolsCommandlineHandler.prototype = {
 };
 
 this.NSGetFactory = XPCOMUtils.generateNSGetFactory(
-  [devtoolsCommandlineHandler]);
+  [DevToolsStartup]);

devtools/client/devtools-startup.manifest

@@ -1,2 +1,2 @@
-component {9e9a9283-0ce9-4e4a-8f1c-ba129a032c32} devtools-clhandler.js
+component {9e9a9283-0ce9-4e4a-8f1c-ba129a032c32} devtools-startup.js
 contract @mozilla.org/toolkit/console-clh;1 {9e9a9283-0ce9-4e4a-8f1c-ba129a032c32}

devtools/client/framework/devtools-browser.js

@@ -4,6 +4,14 @@
 
 "use strict";
 
+/**
+ * This is the main module loaded in Firefox desktop that handles browser
+ * windows and coordinates devtools around each window.
+ *
+ * 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");
@@ -116,10 +124,17 @@ var gDevToolsBrowser = exports.gDevToolsBrowser = {
   },
 
   observe: function(subject, topic, prefName) {
-    if (prefName.endsWith("enabled")) {
-      for (let win of this._trackedBrowserWindows) {
-        this.updateCommandAvailability(win);
-      }
+    switch (topic) {
+      case "browser-delayed-startup-finished":
+        this._registerBrowserWindow(subject);
+        break;
+      case "nsPref:changed":
+        if (prefName.endsWith("enabled")) {
+          for (let win of this._trackedBrowserWindows) {
+            this.updateCommandAvailability(win);
+          }
+        }
+        break;
     }
   },
 
@@ -319,11 +334,11 @@ var gDevToolsBrowser = exports.gDevToolsBrowser = {
    * @param {XULDocument} doc
    *        The document to which menuitems and handlers are to be added
    */
-  // Used by browser.js
-  registerBrowserWindow: function DT_registerBrowserWindow(win) {
+  _registerBrowserWindow: function(win) {
     this.updateCommandAvailability(win);
     this.ensurePrefObserver();
     gDevToolsBrowser._trackedBrowserWindows.add(win);
+    win.addEventListener("unload", this);
     gDevToolsBrowser._addAllToolsToMenu(win.document);
 
     if (this._isFirebugInstalled()) {
@@ -749,8 +764,9 @@ var gDevToolsBrowser = exports.gDevToolsBrowser = {
    * @param  {XULWindow} win
    *         The window containing the menu entry
    */
-  forgetBrowserWindow: function DT_forgetBrowserWindow(win) {
+  _forgetBrowserWindow: function(win) {
     gDevToolsBrowser._trackedBrowserWindows.delete(win);
+    win.removeEventListener("unload", this);
 
     // Destroy toolboxes for closed window
     for (let [target, toolbox] of gDevTools._toolboxes) {
@@ -792,6 +808,11 @@ var gDevToolsBrowser = exports.gDevToolsBrowser = {
       break;
       case "TabSelect":
         gDevToolsBrowser._updateMenuCheckbox();
+      break;
+      case "unload":
+        // top-level browser window unload
+        gDevToolsBrowser._forgetBrowserWindow(event.target.defaultView);
+      break;
     }
   },
 
@@ -800,7 +821,12 @@ var gDevToolsBrowser = exports.gDevToolsBrowser = {
    */
   destroy: function() {
     Services.prefs.removeObserver("devtools.", gDevToolsBrowser);
+    Services.obs.removeObserver(gDevToolsBrowser, "browser-delayed-startup-finished");
     Services.obs.removeObserver(gDevToolsBrowser.destroy, "quit-application");
+
+    for (let win of gDevToolsBrowser._trackedBrowserWindows) {
+      gDevToolsBrowser._forgetBrowserWindow(win);
+    }
   },
 }
 
@@ -820,6 +846,16 @@ 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

devtools/client/moz.build

@@ -44,8 +44,8 @@ if CONFIG['MOZ_BUILD_APP'] == 'browser':
     DIRS += ['themes/shims']
 
 EXTRA_COMPONENTS += [
-    'devtools-clhandler.js',
-    'devtools-clhandler.manifest',
+    'devtools-startup.js',
+    'devtools-startup.manifest',
 ]
 
 JAR_MANIFESTS += ['jar.mn']