Bug 896267 - Only create one walker actor per tab. r=harth

browser/devtools/inspector/inspector-panel.js

@@ -156,14 +156,17 @@ InspectorPanel.prototype = {
   /**
    * Return a promise that will resolve to the default node for selection.
    */
-  _getDefaultNodeForSelection : function() {
+  _getDefaultNodeForSelection: function() {
     if (this._defaultNode) {
       return this._defaultNode;
     }
     let walker = this.walker;
+
     // if available set body node as default selected node
     // else set documentElement
-    return walker.querySelector(this.walker.rootNode, "body").then(front => {
+    return walker.getRootNode().then(rootNode => {
+      return walker.querySelector(rootNode, "body");
+    }).then(front => {
       if (front) {
         return front;
       }
@@ -285,33 +288,21 @@ InspectorPanel.prototype = {
    */
   onNavigatedAway: function InspectorPanel_onNavigatedAway(event, payload) {
     let newWindow = payload._navPayload || payload;
-    this.walker.release().then(null, console.error);
-    this.walker = null;
     this._defaultNode = null;
     this.selection.setNodeFront(null);
-    this.selection.setWalker(null);
     this._destroyMarkup();
     this.isDirty = false;
 
-    this.target.inspector.getWalker().then(walker => {
+    this._getDefaultNodeForSelection().then(defaultNode => {
       if (this._destroyPromise) {
-        walker.release().then(null, console.error);
         return;
       }
+      this.selection.setNodeFront(defaultNode, "navigateaway");
 
-      this.walker = walker;
-      this.selection.setWalker(walker);
-      this._getDefaultNodeForSelection().then(defaultNode => {
-        if (this._destroyPromise) {
-          return;
-        }
-        this.selection.setNodeFront(defaultNode, "navigateaway");
-
-        this._initMarkup();
-        this.once("markuploaded", () => {
-          this.markup.expandNode(this.selection.nodeFront);
-          this.setupSearchBox();
-        });
+      this._initMarkup();
+      this.once("markuploaded", () => {
+        this.markup.expandNode(this.selection.nodeFront);
+        this.setupSearchBox();
       });
     });
   },

toolkit/devtools/server/actors/inspector.js

@@ -1581,6 +1581,14 @@ var WalkerActor = protocol.ActorClass({
 
   onFrameLoad: function(window) {
     let frame = window.frameElement;
+    if (!frame && !this.rootDoc) {
+      this.rootDoc = window.document;
+      this.rootNode = this.document();
+      this.queueMutation({
+        type: "newRoot",
+        target: this.rootNode.form()
+      });
+    }
     let frameActor = this._refMap.get(frame);
     if (!frameActor) {
       return;
@@ -1641,6 +1649,11 @@ var WalkerActor = protocol.ActorClass({
       return;
     }
 
+    if (this.rootDoc === doc) {
+      this.rootDoc = null;
+      this.rootNode = null;
+    }
+
     this.queueMutation({
       type: "documentUnload",
       target: documentActor.actorID
@@ -1673,6 +1686,7 @@ var WalkerFront = exports.WalkerFront = protocol.FrontClass(WalkerActor, {
   autoCleanup: true,
 
   initialize: function(client, form) {
+    this._rootNodeDeferred = promise.defer();
     protocol.Front.prototype.initialize.call(this, client, form);
     this._orphaned = new Set();
     this._retainedOrphans = new Set();
@@ -1684,8 +1698,19 @@ var WalkerFront = exports.WalkerFront = protocol.FrontClass(WalkerActor, {
 
   // Update the object given a form representation off the wire.
   form: function(json) {
-    this.actorID = json.actorID;
+    this.actorID = json.actor;
     this.rootNode = types.getType("domnode").read(json.root, this);
+    this._rootNodeDeferred.resolve(this.rootNode);
+  },
+
+  /**
+   * 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.
+   */
+  getRootNode: function() {
+    return this._rootNodeDeferred.promise;
   },
 
   /**
@@ -1793,8 +1818,19 @@ var WalkerFront = exports.WalkerFront = protocol.FrontClass(WalkerActor, {
       let emitMutations = [];
       for (let change of mutations) {
         // The target is only an actorID, get the associated front.
-        let targetID = change.target;
-        let targetFront = this.get(targetID);
+        let targetID;
+        let targetFront;
+
+        if (change.type === "newRoot") {
+          this.rootNode = types.getType("domnode").read(change.target, this);
+          this._rootNodeDeferred.resolve(this.rootNode);
+          targetID = this.rootNode.actorID;
+          targetFront = this.rootNode;
+        } else {
+          targetID = change.target;
+          targetFront = this.get(targetID);
+        }
+
         if (!targetFront) {
           console.trace("Got a mutation for an unexpected actor: " + targetID + ", please file a bug on bugzilla.mozilla.org!");
           continue;
@@ -1848,6 +1884,11 @@ var WalkerFront = exports.WalkerFront = protocol.FrontClass(WalkerActor, {
             }
           }
         } else if (change.type === "documentUnload") {
+          if (targetFront === this.rootNode) {
+            this.rootNode = null;
+            this._rootNodeDeferred = promise.defer();
+          }
+
           // We try to give fronts instead of actorIDs, but these fronts need
           // to be destroyed now.
           emittedMutation.target = targetFront.actorID;
@@ -1991,7 +2032,12 @@ var InspectorActor = protocol.ActorClass({
   },
 
   getWalker: method(function(options={}) {
+    if (this._walkerPromise) {
+      return this._walkerPromise;
+    }
+
     let deferred = promise.defer();
+    this._walkerPromise = deferred.promise;
 
     let window = this.window;
 
@@ -2007,7 +2053,7 @@ var InspectorActor = protocol.ActorClass({
       domReady();
     }
 
-    return deferred.promise;
+    return this._walkerPromise;
   }, {
     request: {},
     response: {

toolkit/devtools/server/tests/mochitest/Makefile.in

@@ -23,6 +23,7 @@ MOCHITEST_CHROME_FILES	= \
 	test_inspector-mutations-value.html \
 	test_inspector-release.html \
 	test_inspector-remove.html \
+	test_inspector-reload.html \
 	test_inspector-retain.html \
 	test_inspector-pseudoclass-lock.html \
 	test_inspector-traversal.html \

toolkit/devtools/server/tests/mochitest/inspector-helpers.js

@@ -235,6 +235,10 @@ function isChildList(change) {
   return change.type === "childList";
 }
 
+function isNewRoot(change) {
+  return change.type === "newRoot";
+}
+
 // Make sure an iframe's src attribute changed and then
 // strip that mutation out of the list.
 function assertSrcChange(mutations) {

toolkit/devtools/server/tests/mochitest/test_inspector-reload.html

@@ -0,0 +1,83 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test for Bug </title>
+
+  <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css">
+  <script type="application/javascript;version=1.8" src="inspector-helpers.js"></script>
+  <script type="application/javascript;version=1.8">
+Components.utils.import("resource://gre/modules/devtools/Loader.jsm");
+const Ci = Components.interfaces;
+const promise = devtools.require("sdk/core/promise");
+const inspector = devtools.require("devtools/server/actors/inspector");
+
+window.onload = function() {
+  SimpleTest.waitForExplicitFinish();
+  runNextTest();
+}
+
+var gInspectee = null;
+var gClient = null;
+var gWalker = null;
+
+addTest(function setup() {
+  let url = document.getElementById("inspectorContent").href;
+  attachURL(url, function(err, client, tab, doc) {
+    gInspectee = doc;
+    let {InspectorFront} = devtools.require("devtools/server/actors/inspector");
+    let inspector = InspectorFront(client, tab);
+    promiseDone(inspector.getWalker().then(walker => {
+      ok(walker, "getWalker() should return an actor.");
+      gClient = client;
+      gWalker = walker;
+      return inspector.getWalker();
+    }).then(walker => {
+      dump(walker.actorID + "\n");
+      ok(walker === gWalker, "getWalker() twice should return the same walker.");
+    }).then(runNextTest));
+  });
+});
+
+addTest(function testReload() {
+  let nodeFront;
+  let oldRootID = gWalker.rootNode.actorID;
+  // Load a node to populate the tree a bit.
+  promiseDone(gWalker.querySelector(gWalker.rootNode, "#a").then(front => {
+    gInspectee.defaultView.location.reload();
+    return waitForMutation(gWalker, isNewRoot);
+  }).then(() => {
+    ok(gWalker.rootNode.actorID != oldRootID, "Root node should have changed.");
+  }).then(() => {
+    // Make sure we can still access the document
+    return gWalker.querySelector(gWalker.rootNode, "#a");
+  }).then(front => {
+    ok(front.actorID, "Got a new actor ID");
+  }).then(runNextTest));
+});
+
+addTest(function cleanup() {
+  delete gWalker;
+  delete gInspectee;
+  delete gClient;
+  runNextTest();
+});
+
+
+  </script>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=">Mozilla Bug </a>
+<a id="inspectorContent" target="_blank" href="inspector-traversal-data.html">Test Document</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+
+</div>
+<pre id="test">
+</pre>
+</body>
+</html>