Bug 886848 - Chain promises on debugger initialization/destruction to remove some redundancy, r=past
authorVictor Porof <vporof@mozilla.com>
Fri, 13 Sep 2013 16:23:11 +0300
changeset 147061 114177e01bda58bfa6c8c4f4c30412c9f6d0d4c7
parent 147060 e7c825ddd5408f05e1072c2edf882ea609ecd833
child 147062 5ce1bf575b280ac3a05253b9224abc73e4e44453
push idunknown
push userunknown
push dateunknown
reviewerspast
bugs886848
milestone26.0a1
Bug 886848 - Chain promises on debugger initialization/destruction to remove some redundancy, r=past
browser/devtools/debugger/debugger-controller.js
browser/devtools/debugger/debugger-panel.js
browser/devtools/debugger/debugger-view.js
--- a/browser/devtools/debugger/debugger-controller.js
+++ b/browser/devtools/debugger/debugger-controller.js
@@ -72,97 +72,86 @@ let DebuggerController = {
 
   /**
    * Initializes the view.
    *
    * @return object
    *         A promise that is resolved when the debugger finishes startup.
    */
   startupDebugger: function() {
-    if (this._isInitialized) {
-      return this._startup.promise;
+    if (this._startup) {
+      return this._startup;
     }
-    this._isInitialized = true;
 
     // Chrome debugging lives in a different process and needs to handle
     // debugger startup by itself.
     if (window._isChromeDebugger) {
       window.removeEventListener("DOMContentLoaded", this.startupDebugger, true);
     }
 
-    let deferred = this._startup = promise.defer();
-
-    DebuggerView.initialize(() => {
-      DebuggerView._isInitialized = true;
-
+    return this._startup = DebuggerView.initialize().then(() => {
       // Chrome debugging needs to initiate the connection by itself.
       if (window._isChromeDebugger) {
-        this.connect().then(deferred.resolve);
+        return this.connect();
       } else {
-        deferred.resolve();
+        return promise.resolve(null); // Done.
       }
     });
-
-    return deferred.promise;
   },
 
   /**
    * Destroys the view and disconnects the debugger client from the server.
    *
    * @return object
    *         A promise that is resolved when the debugger finishes shutdown.
    */
   shutdownDebugger: function() {
-    if (this._isDestroyed) {
-      return this._shutdown.promise;
+    if (this._shutdown) {
+      return this._shutdown;
     }
-    this._isDestroyed = true;
-    this._startup = null;
 
     // Chrome debugging lives in a different process and needs to handle
     // debugger shutdown by itself.
     if (window._isChromeDebugger) {
       window.removeEventListener("unload", this.shutdownDebugger, true);
     }
 
-    let deferred = this._shutdown = promise.defer();
-
-    DebuggerView.destroy(() => {
-      DebuggerView._isDestroyed = true;
-
+    return this._shutdown = DebuggerView.destroy().then(() => {
+      DebuggerView.destroy();
       this.SourceScripts.disconnect();
       this.StackFrames.disconnect();
       this.ThreadState.disconnect();
-
       this.disconnect();
-      deferred.resolve();
 
       // Chrome debugging needs to close its parent process on shutdown.
-      window._isChromeDebugger && this._quitApp();
+      if (window._isChromeDebugger) {
+        return this._quitApp();
+      } else {
+        return promise.resolve(null); // Done.
+      }
     });
-
-    return deferred.promise;
   },
 
   /**
    * Initiates remote or chrome debugging based on the current target,
    * wiring event handlers as necessary.
    *
    * In case of a chrome debugger living in a different process, a socket
    * connection pipe is opened as well.
    *
    * @return object
    *         A promise that is resolved when the debugger finishes connecting.
    */
   connect: function() {
     if (this._connection) {
-      return this._connection.promise;
+      return this._connection;
     }
 
-    let deferred = this._connection = promise.defer();
+    let deferred = promise.defer();
+    this._connection = deferred.promise;
 
     if (!window._isChromeDebugger) {
       let target = this._target;
       let { client, form, threadActor } = target;
       target.on("close", this._onTabDetached);
       target.on("navigate", this._onTabNavigated);
       target.on("will-navigate", this._onTabNavigated);
 
@@ -177,17 +166,16 @@ let DebuggerController = {
 
     // Chrome debugging needs to make its own connection to the debuggee.
     let transport = debuggerSocketConnect(
       Prefs.chromeDebuggingHost, Prefs.chromeDebuggingPort);
 
     let client = new DebuggerClient(transport);
     client.addListener("tabNavigated", this._onTabNavigated);
     client.addListener("tabDetached", this._onTabDetached);
-
     client.connect((aType, aTraits) => {
       client.listTabs((aResponse) => {
         this._startChromeDebugging(client, aResponse.chromeDebugger, deferred.resolve);
       });
     });
 
     return deferred.promise;
   },
@@ -344,32 +332,41 @@ let DebuggerController = {
       // Update the stack frame list.
       this.activeThread._clearFrames();
       this.activeThread.fillFrames(CALL_STACK_PAGE_SIZE);
     });
   },
 
   /**
    * Attempts to quit the current process if allowed.
+   *
+   * @return object
+   *         A promise that is resolved if the app will quit successfully.
    */
   _quitApp: function() {
-    let canceled = Cc["@mozilla.org/supports-PRBool;1"]
-      .createInstance(Ci.nsISupportsPRBool);
+    let deferred = promise.defer();
 
-    Services.obs.notifyObservers(canceled, "quit-application-requested", null);
+    // Quitting the app is synchronous. Give the returned promise consumers
+    // a chance to do their thing before killing the process.
+    Services.tm.currentThread.dispatch({ run: () => {
+      let quit = Cc["@mozilla.org/supports-PRBool;1"].createInstance(Ci.nsISupportsPRBool);
+      Services.obs.notifyObservers(quit, "quit-application-requested", null);
 
-    // Somebody canceled our quit request.
-    if (canceled.data) {
-      return;
-    }
-    Services.startup.quit(Ci.nsIAppStartup.eAttemptQuit);
+      // Somebody canceled our quit request.
+      if (quit.data) {
+        deferred.reject(quit.data);
+      } else {
+        deferred.resolve(quit.data);
+        Services.startup.quit(Ci.nsIAppStartup.eForceQuit);
+      }
+    }}, 0);
+
+    return deferred.promise;
   },
 
-  _isInitialized: false,
-  _isDestroyed: false,
   _startup: null,
   _shutdown: null,
   _connection: null,
   client: null,
   activeThread: null
 };
 
 /**
@@ -1147,17 +1144,17 @@ SourceScripts.prototype = {
    * @return object
    *         A promise that is resolved after source texts have been fetched.
    */
   getTextForSources: function(aUrls) {
     let deferred = promise.defer();
     let pending = new Set(aUrls);
     let fetched = [];
 
-    // Can't use Promise.all, because if one fetch operation is rejected, then
+    // Can't use promise.all, because if one fetch operation is rejected, then
     // everything is considered rejected, thus no other subsequent source will
     // be getting fetched. We don't want that. Something like Q's allSettled
     // would work like a charm here.
 
     // Try to fetch as many sources as possible.
     for (let url of aUrls) {
       let sourceItem = DebuggerView.Sources.getItemByValue(url);
       let sourceClient = sourceItem.attachment.source;
--- a/browser/devtools/debugger/debugger-panel.js
+++ b/browser/devtools/debugger/debugger-panel.js
@@ -7,16 +7,17 @@
 
 const { Cc, Ci, Cu, Cr } = require("chrome");
 const EventEmitter = require("devtools/shared/event-emitter");
 const promise = require("sdk/core/promise");
 
 function DebuggerPanel(iframeWindow, toolbox) {
   this.panelWin = iframeWindow;
   this._toolbox = toolbox;
+  this._destroyer = null;
 
   this._view = this.panelWin.DebuggerView;
   this._controller = this.panelWin.DebuggerController;
   this._controller._target = this.target;
   this._bkp = this._controller.Breakpoints;
 
   this.highlightWhenPaused = this.highlightWhenPaused.bind(this);
   this.unhighlightWhenResumed = this.unhighlightWhenResumed.bind(this);
@@ -27,17 +28,17 @@ exports.DebuggerPanel = DebuggerPanel;
 
 DebuggerPanel.prototype = {
   /**
    * Open is effectively an asynchronous constructor.
    *
    * @return object
    *         A promise that is resolved when the Debugger completes opening.
    */
-  open: function DebuggerPanel_open() {
+  open: function() {
     let targetPromise;
 
     // Local debugging needs to make the target remote.
     if (!this.target.isRemote) {
       targetPromise = this.target.makeRemote();
     } else {
       targetPromise = promise.resolve(this.target);
     }
@@ -49,28 +50,35 @@ DebuggerPanel.prototype = {
         this.target.on("thread-paused", this.highlightWhenPaused);
         this.target.on("thread-resumed", this.unhighlightWhenResumed);
         this.isReady = true;
         this.emit("ready");
         return this;
       })
       .then(null, function onError(aReason) {
         Cu.reportError("DebuggerPanel open failed. " +
-                       reason.error + ": " + reason.message);
+                       aReason.error + ": " + aReason.message);
       });
   },
 
   // DevToolPanel API
   get target() this._toolbox.target,
 
   destroy: function() {
+    // Make sure this panel is not already destroyed.
+    if (this._destroyer) {
+      return this._destroyer;
+    }
+
     this.target.off("thread-paused", this.highlightWhenPaused);
     this.target.off("thread-resumed", this.unhighlightWhenResumed);
-    this.emit("destroyed");
-    return promise.resolve(null);
+
+    return this._destroyer = this._controller.shutdownDebugger().then(() => {
+      this.emit("destroyed");
+    });
   },
 
   // DebuggerPanel API
 
   addBreakpoint: function() {
     this._bkp.addBreakpoint.apply(this._bkp, arguments);
   },
 
--- a/browser/devtools/debugger/debugger-view.js
+++ b/browser/devtools/debugger/debugger-view.js
@@ -32,63 +32,72 @@ Cu.import("resource://gre/modules/devtoo
 
 /**
  * Object defining the debugger view components.
  */
 let DebuggerView = {
   /**
    * Initializes the debugger view.
    *
-   * @param function aCallback
-   *        Called after the view finishes initializing.
+   * @return object
+   *         A promise that is resolved when the view finishes initializing.
    */
-  initialize: function(aCallback) {
-    dumpn("Initializing the DebuggerView");
+  initialize: function() {
+    if (this._startup) {
+      return this._startup;
+    }
+
+    let deferred = promise.defer();
+    this._startup = deferred.promise;
 
     this._initializePanes();
-
     this.Toolbar.initialize();
     this.Options.initialize();
     this.Filtering.initialize();
     this.FilteredSources.initialize();
     this.FilteredFunctions.initialize();
     this.ChromeGlobals.initialize();
     this.StackFrames.initialize();
     this.Sources.initialize();
     this.WatchExpressions.initialize();
     this.GlobalSearch.initialize();
+    this._initializeVariablesView();
+    this._initializeEditor(deferred.resolve);
 
-    this._initializeVariablesView();
-    this._initializeEditor(aCallback);
+    return deferred.promise;
   },
 
   /**
    * Destroys the debugger view.
    *
-   * @param function aCallback
-   *        Called after the view finishes destroying.
+   * @return object
+   *         A promise that is resolved when the view finishes destroying.
    */
-  destroy: function(aCallback) {
-    dumpn("Destroying the DebuggerView");
+  destroy: function() {
+    if (this._shutdown) {
+      return this._shutdown;
+    }
+
+    let deferred = promise.defer();
+    this._shutdown = deferred.promise;
 
     this.Toolbar.destroy();
     this.Options.destroy();
     this.Filtering.destroy();
     this.FilteredSources.destroy();
     this.FilteredFunctions.destroy();
     this.ChromeGlobals.destroy();
     this.StackFrames.destroy();
     this.Sources.destroy();
     this.WatchExpressions.destroy();
     this.GlobalSearch.destroy();
+    this._destroyPanes();
+    this._destroyEditor(deferred.resolve);
 
-    this._destroyPanes();
-    this._destroyEditor();
-
-    aCallback();
+    return deferred.promise;
   },
 
   /**
    * Initializes the UI for all the displayed panes.
    */
   _initializePanes: function() {
     dumpn("Initializing the DebuggerView panes");
 
@@ -179,17 +188,16 @@ let DebuggerView = {
    * The load event handler for the source editor, also executing any necessary
    * post-load operations.
    */
   _onEditorLoad: function() {
     dumpn("Finished loading the DebuggerView editor");
 
     DebuggerController.Breakpoints.initialize();
     window.dispatchEvent(document, "Debugger:EditorLoaded", this.editor);
-    this.editor.focus();
   },
 
   /**
    * Destroys the SourceEditor instance and also executes any necessary
    * post-unload operations.
    */
   _destroyEditor: function() {
     dumpn("Destroying the DebuggerView editor");
@@ -462,16 +470,18 @@ let DebuggerView = {
 
     if (this.editor) {
       this.editor.setText("");
       this.editor.focus();
       this._editorSource = null;
     }
   },
 
+  _startup: null,
+  _shutdown: null,
   Toolbar: null,
   Options: null,
   Filtering: null,
   FilteredSources: null,
   FilteredFunctions: null,
   GlobalSearch: null,
   ChromeGlobals: null,
   StackFrames: null,
@@ -481,18 +491,16 @@ let DebuggerView = {
   _editor: null,
   _editorSource: null,
   _loadingText: "",
   _sourcesPane: null,
   _instrumentsPane: null,
   _instrumentsPaneToggleButton: null,
   _collapsePaneString: "",
   _expandPaneString: "",
-  _isInitialized: false,
-  _isDestroyed: false
 };
 
 /**
  * A stacked list of items, compatible with WidgetMethods instances, used for
  * displaying views like the watch expressions, filtering or search results etc.
  *
  * You should never need to access these methods directly, use the wrapped
  * WidgetMethods instead.