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 321625 ba186717174aec9677487aa47caa84fc1430e334
parent 321624 138f7c7856b618dfd3f3faafcec9d3e164e188ec
child 321626 913b4129466e9de24eddbc730efd46b61936b354
push id5913
push userjlund@mozilla.com
push dateMon, 25 Apr 2016 16:57:49 +0000
treeherdermozilla-beta@dcaf0a6fa115 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjryans
bugs1248600
milestone47.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 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',
 )