Bug 896267 - Only create one walker actor per tab. r=harth
authorDave Camp <dcamp@mozilla.com>
Fri, 19 Jul 2013 11:21:40 -0700
changeset 152075 6f7b49cd25124fa1100d0d9c18dece4c3d2c16d7
parent 152074 fb7d20306b6777ba8159d1d570f447d509373e7b
child 152076 d7f60ad11f4851a08be6e54840f8469eb2f4118c
push id2859
push userakeybl@mozilla.com
push dateMon, 16 Sep 2013 19:14:59 +0000
treeherdermozilla-beta@87d3c51cd2bf [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersharth
bugs896267
milestone25.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 896267 - Only create one walker actor per tab. r=harth
browser/devtools/inspector/inspector-panel.js
toolkit/devtools/server/actors/inspector.js
toolkit/devtools/server/tests/mochitest/Makefile.in
toolkit/devtools/server/tests/mochitest/inspector-helpers.js
toolkit/devtools/server/tests/mochitest/test_inspector-reload.html
--- a/browser/devtools/inspector/inspector-panel.js
+++ b/browser/devtools/inspector/inspector-panel.js
@@ -151,24 +151,27 @@ InspectorPanel.prototype = {
     this.setupSidebar();
 
     return deferred.promise;
   },
 
   /**
    * 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;
       }
       return this.walker.documentElement(this.walker.rootNode);
     }).then(node => {
       if (walker !== this.walker) {
         promise.reject(null);
       }
@@ -280,43 +283,31 @@ InspectorPanel.prototype = {
     this.sidebar.show();
   },
 
   /**
    * Reset the inspector on navigate away.
    */
   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();
       });
     });
   },
 
   /**
    * When a new node is selected.
    */
   onNewSelection: function InspectorPanel_onNewSelection() {
--- a/toolkit/devtools/server/actors/inspector.js
+++ b/toolkit/devtools/server/actors/inspector.js
@@ -1576,16 +1576,24 @@ var WalkerActor = protocol.ActorClass({
         mutation.added = addedActors;
       }
       this.queueMutation(mutation);
     }
   },
 
   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;
     }
 
     this.queueMutation({
       type: "frameLoad",
       target: frameActor.actorID,
@@ -1636,16 +1644,21 @@ var WalkerActor = protocol.ActorClass({
     }
 
     let doc = window.document;
     let documentActor = this._refMap.get(doc);
     if (!documentActor) {
       return;
     }
 
+    if (this.rootDoc === doc) {
+      this.rootDoc = null;
+      this.rootNode = null;
+    }
+
     this.queueMutation({
       type: "documentUnload",
       target: documentActor.actorID
     });
 
     let walker = documentWalker(doc);
     let parentNode = walker.parentNode();
     if (parentNode) {
@@ -1668,29 +1681,41 @@ var WalkerActor = protocol.ActorClass({
 /**
  * Client side of the DOM walker.
  */
 var WalkerFront = exports.WalkerFront = protocol.FrontClass(WalkerActor, {
   // Set to true if cleanup should be requested after every mutation list.
   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();
   },
 
   destroy: function() {
     protocol.Front.prototype.destroy.call(this);
   },
 
   // 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;
   },
 
   /**
    * When reading an actor form off the wire, we want to hook it up to its
    * parent front.  The protocol guarantees that the parent will be seen
    * by the client in either a previous or the current request.
    * So if we've already seen this parent return it, otherwise create
    * a bare-bones stand-in node.  The stand-in node will be updated
@@ -1788,18 +1813,29 @@ var WalkerFront = exports.WalkerFront = 
   /**
    * Get any unprocessed mutation records and process them.
    */
   getMutations: protocol.custom(function(options={}) {
     return this._getMutations(options).then(mutations => {
       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;
         }
 
         let emittedMutation = object.merge(change, { target: targetFront });
 
         if (change.type === "childList") {
@@ -1843,16 +1879,21 @@ var WalkerFront = exports.WalkerFront = 
           // document children, because we should have gotten a documentUnload
           // first.
           for (let child of targetFront.treeChildren()) {
             if (child.nodeType === Ci.nsIDOMNode.DOCUMENT_NODE) {
               console.trace("Got an unexpected frameLoad in the inspector, please file a bug on bugzilla.mozilla.org!");
             }
           }
         } 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;
           emittedMutation.targetParent = targetFront.parentNode();
 
           // Release the document node and all of its children, even retained.
           this._releaseFront(targetFront, true);
         } else if (change.type === "unretained") {
@@ -1986,33 +2027,38 @@ var InspectorActor = protocol.ActorClass
       return tabActor.browser;
     } else if (tabActor.browser instanceof Ci.nsIDOMElement) {
       return tabActor.browser.contentWindow;
     }
     return null;
   },
 
   getWalker: method(function(options={}) {
+    if (this._walkerPromise) {
+      return this._walkerPromise;
+    }
+
     let deferred = promise.defer();
+    this._walkerPromise = deferred.promise;
 
     let window = this.window;
 
     var domReady = () => {
       let tabActor = this.tabActor;
       window.removeEventListener("DOMContentLoaded", domReady, true);
       deferred.resolve(WalkerActor(this.conn, window.document, tabActor._tabbrowser, options));
     };
 
     if (window.document.readyState === "loading") {
       window.addEventListener("DOMContentLoaded", domReady, true);
     } else {
       domReady();
     }
 
-    return deferred.promise;
+    return this._walkerPromise;
   }, {
     request: {},
     response: {
       walker: RetVal("domwalker")
     }
   })
 });
 
--- a/toolkit/devtools/server/tests/mochitest/Makefile.in
+++ b/toolkit/devtools/server/tests/mochitest/Makefile.in
@@ -18,16 +18,17 @@ MOCHITEST_CHROME_FILES	= \
 	test_inspector-changevalue.html \
 	test_inspector-insert.html \
 	test_inspector-mutations-attr.html \
 	test_inspector-mutations-childlist.html \
 	test_inspector-mutations-frameload.html \
 	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 \
 	test_unsafeDereference.html \
 	nonchrome_unsafeDereference.html \
 	$(NULL)
 
 include $(topsrcdir)/config/rules.mk
--- a/toolkit/devtools/server/tests/mochitest/inspector-helpers.js
+++ b/toolkit/devtools/server/tests/mochitest/inspector-helpers.js
@@ -230,16 +230,20 @@ function isFrameLoad(change) {
 function isUnretained(change) {
   return change.type === "unretained";
 }
 
 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) {
   return assertAndStrip(mutations, "Should have had an iframe source change.", isSrcChange);
 }
 
 // Make sure there's an unload in the mutation list and strip
 // that mutation out of the list
new file mode 100644
--- /dev/null
+++ b/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>