Bug 1248600 - Remove registerBrowserWindow/forgetBrowserWindow calls from /browser/. r=jryans
authorAlexandre Poirot <poirot.alex@gmail.com>
Wed, 24 Feb 2016 02:06:42 -0800
changeset 334114 ba186717174aec9677487aa47caa84fc1430e334
parent 334113 138f7c7856b618dfd3f3faafcec9d3e164e188ec
child 334115 913b4129466e9de24eddbc730efd46b61936b354
push id11452
push userjdescottes@mozilla.com
push dateWed, 24 Feb 2016 12:47:30 +0000
reviewersjryans
bugs1248600
milestone47.0a1
Bug 1248600 - Remove registerBrowserWindow/forgetBrowserWindow calls from /browser/. r=jryans
browser/base/content/browser.js
browser/installer/package-manifest.in
devtools/client/devtools-clhandler.js
devtools/client/devtools-clhandler.manifest
devtools/client/devtools-startup.js
devtools/client/devtools-startup.manifest
devtools/client/framework/devtools-browser.js
devtools/client/moz.build
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -1286,19 +1286,16 @@ var gBrowserInit = {
     gSyncUI.init();
     gFxAccounts.init();
 
     if (AppConstants.MOZ_DATA_REPORTING)
       gDataNotificationInfoBar.init();
 
     gBrowserThumbnails.init();
 
-    // Add Devtools menuitems and listeners
-    gDevToolsBrowser.registerBrowserWindow(window);
-
     gMenuButtonBadgeManager.init();
 
     gMenuButtonUpdateBadge.init();
 
     window.addEventListener("mousemove", MousePosTracker, false);
     window.addEventListener("dragover", MousePosTracker, false);
 
     gNavToolbox.addEventListener("customizationstarting", CustomizationHandler);
@@ -1402,18 +1399,16 @@ var gBrowserInit = {
 
   onUnload: function() {
     // In certain scenarios it's possible for unload to be fired before onload,
     // (e.g. if the window is being closed after browser.js loads but before the
     // load completes). In that case, there's nothing to do here.
     if (!this._loadHandled)
       return;
 
-    gDevToolsBrowser.forgetBrowserWindow(window);
-
     let desc = Object.getOwnPropertyDescriptor(window, "DeveloperToolbar");
     if (desc && !desc.get) {
       DeveloperToolbar.destroy();
     }
 
     // First clean up services initialized in gBrowserInit.onLoad (or those whose
     // uninit methods don't depend on the services having been initialized).
 
--- a/browser/installer/package-manifest.in
+++ b/browser/installer/package-manifest.in
@@ -370,18 +370,18 @@
 @RESPATH@/browser/components/FeedConverter.js
 @RESPATH@/browser/components/FeedWriter.js
 @RESPATH@/browser/components/WebContentConverter.js
 @RESPATH@/browser/components/BrowserComponents.manifest
 @RESPATH@/browser/components/nsBrowserContentHandler.js
 @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
 @RESPATH@/browser/components/ExperimentsService.js
 @RESPATH@/browser/components/browser-newtab.xpt
 @RESPATH@/browser/components/aboutNewTabService.js
 @RESPATH@/browser/components/NewTabComponents.manifest
 @RESPATH@/components/Downloads.manifest
rename from devtools/client/devtools-clhandler.js
rename to devtools/client/devtools-startup.js
--- a/devtools/client/devtools-clhandler.js
+++ b/devtools/client/devtools-startup.js
@@ -1,92 +1,111 @@
 /* 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/. */
 
 /* 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.
+ *
+ * Be careful to lazy load dependencies as much as possible.
+ **/
+
 "use strict";
 
 const { interfaces: Ci, utils: Cu } = Components;
 const kDebuggerPrefs = [
   "devtools.debugger.remote-enabled",
   "devtools.chrome.enabled"
 ];
 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);
     let devtoolsFlag = cmdLine.handleFlag("devtools", false);
 
     if (consoleFlag) {
       this.handleConsoleFlag(cmdLine);
     }
     if (debuggerFlag) {
       this.handleDebuggerFlag(cmdLine);
     }
-    if (devtoolsFlag) {
-      this.handleDevToolsFlag();
-    }
     let debuggerServerFlag;
     try {
       debuggerServerFlag =
         cmdLine.handleFlagWithParam("start-debugger-server", false);
     } catch (e) {
       // We get an error if the option is given but not followed by a value.
       // By catching and trying again, the value is effectively optional.
       debuggerServerFlag = cmdLine.handleFlag("start-debugger-server", false);
     }
     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) {
+      this.initDevTools();
+
       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");
-
       let hudservice = require("devtools/client/webconsole/hudservice");
       let { console } = Cu.import("resource://gre/modules/Console.jsm", {});
       hudservice.toggleBrowserConsole().then(null, console.error);
     } else {
       // the Browser Console was already open
       window.focus();
     }
 
     if (cmdLine.state == Ci.nsICommandLine.STATE_REMOTE_AUTO) {
       cmdLine.preventDefault = true;
     }
   },
 
   // 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() {
     let remoteDebuggingEnabled = false;
     try {
       remoteDebuggingEnabled = kDebuggerPrefs.every(pref => {
         return Services.prefs.getBoolPref(pref);
       });
@@ -163,9 +182,9 @@ devtoolsCommandlineHandler.prototype = {
             "Start the debugger server on a TCP port or " +
             "Unix domain socket path.  Defaults to TCP port 6000.\n",
 
   classID: Components.ID("{9e9a9283-0ce9-4e4a-8f1c-ba129a032c32}"),
   QueryInterface: XPCOMUtils.generateQI([Ci.nsICommandLineHandler]),
 };
 
 this.NSGetFactory = XPCOMUtils.generateNSGetFactory(
-  [devtoolsCommandlineHandler]);
+  [DevToolsStartup]);
rename from devtools/client/devtools-clhandler.manifest
rename to devtools/client/devtools-startup.manifest
--- a/devtools/client/devtools-clhandler.manifest
+++ b/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}
--- a/devtools/client/framework/devtools-browser.js
+++ b/devtools/client/framework/devtools-browser.js
@@ -1,14 +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";
 
+/**
+ * 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");
 const {gDevTools} = require("./devtools");
 
 // 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);
@@ -111,20 +119,27 @@ var gDevToolsBrowser = exports.gDevTools
     let consoleEnabled = Services.prefs.getBoolPref("devtools.errorconsole.enabled");
     toggleCmd("Tools:ErrorConsole", consoleEnabled);
 
     // Enable DevTools connection screen, if the preference allows this.
     toggleCmd("Tools:DevToolsConnect", devtoolsRemoteEnabled);
   },
 
   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;
     }
   },
 
   _prefObserverRegistered: false,
 
   ensurePrefObserver: function() {
     if (!this._prefObserverRegistered) {
       this._prefObserverRegistered = true;
@@ -314,21 +329,21 @@ var gDevToolsBrowser = exports.gDevTools
   },
 
   /**
    * Add this DevTools's presence to a browser window's document
    *
    * @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()) {
       let broadcaster = win.document.getElementById("devtoolsMenuBroadcaster_DevToolbox");
       broadcaster.removeAttribute("key");
     }
 
     let tabContainer = win.gBrowser.tabContainer;
@@ -744,18 +759,19 @@ var gDevToolsBrowser = exports.gDevTools
 
   /**
    * Called on browser unload to remove menu entries, toolboxes and event
    * listeners from the closed browser window.
    *
    * @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) {
       if (toolbox.frame && toolbox.frame.ownerDocument.defaultView == win) {
         toolbox.destroy();
       }
     }
 
@@ -787,25 +803,35 @@ var gDevToolsBrowser = exports.gDevTools
 
         this._tabStats.histOpen.push(open);
         this._tabStats.histPinned.push(pinned);
         this._tabStats.peakOpen = Math.max(open, this._tabStats.peakOpen);
         this._tabStats.peakPinned = Math.max(pinned, this._tabStats.peakPinned);
       break;
       case "TabSelect":
         gDevToolsBrowser._updateMenuCheckbox();
+      break;
+      case "unload":
+        // top-level browser window unload
+        gDevToolsBrowser._forgetBrowserWindow(event.target.defaultView);
+      break;
     }
   },
 
   /**
    * All browser windows have been closed, tidy up remaining objects.
    */
   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);
+    }
   },
 }
 
 gDevTools.on("tool-registered", function(ev, toolId) {
   let toolDefinition = gDevTools._tools.get(toolId);
   gDevToolsBrowser._addToolToWindows(toolDefinition);
 });
 
@@ -815,14 +841,24 @@ 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");
 
--- a/devtools/client/moz.build
+++ b/devtools/client/moz.build
@@ -39,18 +39,18 @@ DIRS += [
     'webide',
 ]
 
 # Shim old theme paths used by DevTools add-ons
 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']
 
 DevToolsModules(
     'definitions.js',
     'main.js',
 )