Bug 1161072 - Destroy the walker actor on disconnect. r=bgrins
☠☠ backed out by 0afaff7ed957 ☠ ☠
authorAlexandre Poirot <poirot.alex@gmail.com>
Wed, 05 Aug 2015 02:59:11 -0700
changeset 287991 2672589e571e254d7354cab95d42efe887c67511
parent 287990 9900bb712ae7e1001e64af7c24216310f193841f
child 287992 8b77628e560848d307e5fe663b233f82ad88c237
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbgrins
bugs1161072
milestone42.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 1161072 - Destroy the walker actor on disconnect. r=bgrins
browser/devtools/framework/toolbox.js
toolkit/devtools/server/actors/inspector.js
--- a/browser/devtools/framework/toolbox.js
+++ b/browser/devtools/framework/toolbox.js
@@ -1738,62 +1738,62 @@ Toolbox.prototype = {
     return this._initInspector;
   },
 
   /**
    * Destroy the inspector/walker/selection fronts
    * Returns a promise that resolves when the fronts are destroyed
    */
   destroyInspector: function() {
-    if (this._destroying) {
-      return this._destroying;
-    }
-
-    if (!this._inspector) {
-      return promise.resolve();
+    if (this._destroyingInspector) {
+      return this._destroyingInspector;
     }
 
-    let outstanding = () => {
-      return Task.spawn(function*() {
-        yield this.highlighterUtils.stopPicker();
-        yield this._inspector.destroy();
-        if (this._highlighter) {
-          // Note that if the toolbox is closed, this will work fine, but will fail
-          // in case the browser is closed and will trigger a noSuchActor message.
-          // We ignore the promise that |_hideBoxModel| returns, since we should still
-          // proceed with the rest of destruction if it fails.
-          // FF42+ now does the cleanup from the actor.
-          if (!this.highlighter.traits.autoHideOnDestroy) {
-            this.highlighterUtils.unhighlight();
-          }
-          yield this._highlighter.destroy();
-        }
-        if (this._selection) {
-          this._selection.destroy();
-        }
+    return this._destroyingInspector = Task.spawn(function*() {
+      if (!this._inspector) {
+        return;
+      }
+
+      // Releasing the walker (if it has been created)
+      // This can fail, but in any case, we want to continue destroying the
+      // inspector/highlighter/selection
+      // FF42+: Inspector actor starts managing Walker actor and auto destroy it.
+      if (this._walker && !this.walker.traits.autoReleased) {
+        try {
+          yield this._walker.release();
+        } catch(e) {}
+      }
 
-        if (this.walker) {
-          this.walker.off("highlighter-ready", this._highlighterReady);
-          this.walker.off("highlighter-hide", this._highlighterHidden);
+      yield this.highlighterUtils.stopPicker();
+      yield this._inspector.destroy();
+      if (this._highlighter) {
+        // Note that if the toolbox is closed, this will work fine, but will fail
+        // in case the browser is closed and will trigger a noSuchActor message.
+        // We ignore the promise that |_hideBoxModel| returns, since we should still
+        // proceed with the rest of destruction if it fails.
+        // FF42+ now does the cleanup from the actor.
+        if (!this.highlighter.traits.autoHideOnDestroy) {
+          this.highlighterUtils.unhighlight();
         }
+        yield this._highlighter.destroy();
+      }
+      if (this._selection) {
+        this._selection.destroy();
+      }
 
-        this._inspector = null;
-        this._highlighter = null;
-        this._selection = null;
-        this._walker = null;
-      }.bind(this));
-    };
+      if (this.walker) {
+        this.walker.off("highlighter-ready", this._highlighterReady);
+        this.walker.off("highlighter-hide", this._highlighterHidden);
+      }
 
-    // Releasing the walker (if it has been created)
-    // This can fail, but in any case, we want to continue destroying the
-    // inspector/highlighter/selection
-    let walker = (this._destroying = this._walker) ?
-                 this._walker.release() :
-                 promise.resolve();
-    return walker.then(outstanding, outstanding);
+      this._inspector = null;
+      this._highlighter = null;
+      this._selection = null;
+      this._walker = null;
+    }.bind(this));
   },
 
   /**
    * Get the toolbox's notification box
    *
    * @return The notification box element.
    */
   getNotificationBox: function() {
--- a/toolkit/devtools/server/actors/inspector.js
+++ b/toolkit/devtools/server/actors/inspector.js
@@ -1293,17 +1293,23 @@ var WalkerActor = protocol.ActorClass({
     this._onReflows = this._onReflows.bind(this);
     this.reflowObserver.on("reflows", this._onReflows);
   },
 
   // Returns the JSON representation of this object over the wire.
   form: function() {
     return {
       actor: this.actorID,
-      root: this.rootNode.form()
+      root: this.rootNode.form(),
+      traits: {
+        // FF42+ Inspector starts managing the Walker, while the inspector also
+        // starts cleaning itself up automatically on client disconnection.
+        // So that there is no need to manually release the walker anymore.
+        autoReleased: true
+      }
     }
   },
 
   toString: function() {
     return "[WalkerActor " + this.actorID + "]";
   },
 
   getDocumentWalker: function(node, whatToShow) {
@@ -3103,16 +3109,18 @@ var WalkerFront = exports.WalkerFront = 
     protocol.Front.prototype.destroy.call(this);
   },
 
   // Update the object given a form representation off the wire.
   form: function(json) {
     this.actorID = json.actor;
     this.rootNode = types.getType("domnode").read(json.root, this);
     this._rootNodeDeferred.resolve(this.rootNode);
+    // FF42+ the actor starts exposing traits
+    this.traits = json.traits || {};
   },
 
   /**
    * Clients can use walker.rootNode to get the current root node of the
    * walker, but during a reload the root node might be null.  This
    * method returns a promise that will resolve to the root node when it is
    * set.
    */
@@ -3517,16 +3525,17 @@ var InspectorActor = exports.InspectorAc
     let deferred = promise.defer();
     this._walkerPromise = deferred.promise;
 
     let window = this.window;
     var domReady = () => {
       let tabActor = this.tabActor;
       window.removeEventListener("DOMContentLoaded", domReady, true);
       this.walker = WalkerActor(this.conn, tabActor, options);
+      this.manage(this.walker);
       events.once(this.walker, "destroyed", () => {
         this._walkerPromise = null;
         this._pageStylePromise = null;
       });
       deferred.resolve(this.walker);
     };
 
     if (window.document.readyState === "loading") {