Bug 878442 - Add tree change mutations to the inspector actor. r=jwalker
authorDave Camp <dcamp@mozilla.com>
Thu, 06 Jun 2013 16:22:01 -0700
changeset 146702 cccfd3161714be699bc2dd6c3bccc289bd3eece7
parent 146701 f9b59a3ef675dae18aef928029a4457717f21dd0
child 146703 97ca769b1ed6568a5ecf7737489afaa42d95c8b7
push id2697
push userbbajaj@mozilla.com
push dateMon, 05 Aug 2013 18:49:53 +0000
treeherdermozilla-beta@dfec938c7b63 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwalker
bugs878442
milestone24.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 878442 - Add tree change mutations to the inspector actor. r=jwalker
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/inspector-traversal-data.html
toolkit/devtools/server/tests/mochitest/test_inspector-mutations-childlist.html
--- a/toolkit/devtools/server/actors/inspector.js
+++ b/toolkit/devtools/server/actors/inspector.js
@@ -235,16 +235,23 @@ let NodeFront = protocol.FrontClass(Node
       // parent node of this actor from it, creating a standin node if
       // necessary.
       let parentNodeFront = ctx.marshallPool().ensureParentFront(form.parent);
       this.reparent(parentNodeFront);
     }
   },
 
   /**
+   * Returns the parent NodeFront for this NodeFront.
+   */
+  parentNode: function() {
+    return this._parent;
+  },
+
+  /**
    * Process a mutation entry as returned from the walker's `getMutations`
    * request.  Only tries to handle changes of the node's contents
    * themselves (character data and attribute changes), the walker itself
    * will keep the ownership tree up to date.
    */
   updateMutation: function(change) {
     if (change.type === "attributes") {
       // We'll need to lazily reparse the attributes after this change.
@@ -581,16 +588,21 @@ var WalkerActor = protocol.ActorClass({
    *    The server connection.
    */
   initialize: function(conn, document, options) {
     protocol.Actor.prototype.initialize.call(this, conn);
     this.rootDoc = document;
     this._refMap = new Map();
     this._pendingMutations = [];
 
+    // Nodes which have been removed from the client's known
+    // ownership tree are considered "orphaned", and stored in
+    // this set.
+    this._orphaned = new Set();
+
     this.onMutations = this.onMutations.bind(this);
 
     // Ensure that the root document node actor is ready and
     // managed.
     this.rootNode = this.document();
   },
 
   // Returns the JSON representation of this object over the wire.
@@ -643,16 +655,17 @@ var WalkerActor = protocol.ActorClass({
   _watchDocument: function(actor) {
     let node = actor.rawNode;
     // Create the observer on the node's actor.  The node will make sure
     // the observer is cleaned up when the actor is released.
     actor.observer = actor.rawNode.defaultView.MutationObserver(this.onMutations);
     actor.observer.observe(node, {
       attributes: true,
       characterData: true,
+      childList: true,
       subtree: true
     });
   },
 
   /**
    * Return the document node that contains the given node,
    * or the root node if no node is specified.
    * @param NodeActor node
@@ -1010,30 +1023,50 @@ var WalkerActor = protocol.ActorClass({
   /**
    * Get any pending mutation records.  Must be called by the client after
    * the `new-mutations` notification is received.  Returns an array of
    * mutation records.
    *
    * Mutation records have a basic structure:
    *
    * {
-   *   type: attributes|characterData,
+   *   type: attributes|characterData|childList,
    *   target: <domnode actor ID>,
    * }
    *
    * And additional attributes based on the mutation type:
+   *
    * `attributes` type:
    *   attributeName: <string> - the attribute that changed
    *   attributeNamespace: <string> - the attribute's namespace URI, if any.
    *   newValue: <string> - The new value of the attribute, if any.
    *
    * `characterData` type:
    *   newValue: <string> - the new shortValue for the node
    *   [incompleteValue: true] - True if the shortValue was truncated.
    *
+   * `childList` type is returned when the set of children for a node
+   * has changed.  Includes extra data, which can be used by the client to
+   * maintain its ownership subtree.
+   *
+   *   added: array of <domnode actor ID> - The list of actors *previously
+   *     seen by the client* that were added to the target node.
+   *   removed: array of <domnode actor ID> The list of actors *previously
+   *     seen by the client* that were removed from the target node.
+   *
+   * Actors that are included in a MutationRecord's `removed` but
+   * not in an `added` have been removed from the client's ownership
+   * tree (either by being moved under a node the client has seen yet
+   * or by being removed from the tree entirely), and is considered
+   * 'orphaned'.
+   *
+   * Keep in mind that if a node that the client hasn't seen is moved
+   * into or out of the target node, it will not be included in the
+   * removedNodes and addedNodes list, so if the client is interested
+   * in the new set of children it needs to issue a `children` request.
    */
   getMutations: method(function() {
     let pending = this._pendingMutations || [];
     this._pendingMutations = [];
     return pending;
   }, {
     request: {},
     response: {
@@ -1069,32 +1102,66 @@ var WalkerActor = protocol.ActorClass({
         mutation.newValue = targetNode.getAttribute(mutation.attributeName);
       } else if (mutation.type === "characterData") {
         if (targetNode.nodeValue.length > gValueSummaryLength) {
           mutation.newValue = targetNode.nodeValue.substring(0, gValueSummaryLength);
           mutation.incompleteValue = true;
         } else {
           mutation.newValue = targetNode.nodeValue;
         }
+      } else if (mutation.type === "childList") {
+        // Get the list of removed and added actors that the client has seen
+        // so that it can keep its ownership tree up to date.
+        let removedActors = [];
+        let addedActors = [];
+        for (let removed of change.removedNodes) {
+          let removedActor = this._refMap.get(removed);
+          if (!removedActor) {
+            // If the client never encountered this actor we don't need to
+            // mention that it was removed.
+            continue;
+          }
+          // While removed from the tree, nodes are saved as orphaned.
+          this._orphaned.add(removedActor);
+          removedActors.push(removedActor.actorID);
+        }
+        for (let added of change.addedNodes) {
+          let addedActor = this._refMap.get(added);
+          if (!addedActor) {
+            // If the client never encounted this actor we don't need to tell
+            // it about its addition for ownership tree purposes - if the
+            // client wants to see the new nodes it can ask for children.
+            continue;
+          }
+          // The actor is reconnected to the ownership tree, unorphan
+          // it and let the client know so that its ownership tree is up
+          // to date.
+          this._orphaned.delete(addedActor);
+          addedActors.push(addedActor.actorID);
+        }
+        mutation.numChildren = change.target.childNodes.length;
+        mutation.removed = removedActors;
+        mutation.added = addedActors;
       }
       this._pendingMutations.push(mutation);
     }
     if (needEvent) {
       events.emit(this, "new-mutations");
     }
   }
 
 });
 
 /**
  * Client side of the DOM walker.
  */
 var WalkerFront = exports.WalkerFront = protocol.FrontClass(WalkerActor, {
   initialize: function(client, form) {
     protocol.Front.prototype.initialize.call(this, client, form);
+    this._orphaned = new Set();
   },
 
   destroy: function() {
     protocol.Front.prototype.destroy.call(this);
   },
 
   // Update the object given a form representation off the wire.
   form: function(json) {
@@ -1146,21 +1213,60 @@ var WalkerFront = exports.WalkerFront = 
       for (let change of mutations) {
         // The target is only an actorID, get the associated front.
         let targetID = change.target;
         let targetFront = this.get(targetID);
         if (!targetFront) {
           console.error("Got a mutation for an unexpected actor: " + targetID);
           continue;
         }
-        targetFront.updateMutation(change);
+
+        let emittedMutation = object.merge(change, { target: targetFront });
+
+        if (change.type === "childList") {
+          // Update the ownership tree according to the mutation record.
+          let addedFronts = [];
+          let removedFronts = [];
+          for (let removed of change.removed) {
+            let removedFront = this.get(removed);
+            if (!removedFront) {
+              console.error("Got a removal of an actor we didn't know about: " + removed);
+              continue;
+            }
+            // Remove from the ownership tree
+            removedFront.reparent(null);
 
-        // Emit the mutation as received, except update the target to
-        // piont at the front rather than a bare actorID.
-        emitMutations.push(object.merge(change, { target: targetFront }));
+            // This node is orphaned unless we get it in the 'added' list
+            // eventually.
+            this._orphaned.add(removedFront);
+            removedFronts.push(removedFront);
+          }
+          for (let added of change.added) {
+            let addedFront = this.get(added);
+            if (!addedFront) {
+              console.error("Got an addition of an actor we didn't know about: " + added);
+              continue;
+            }
+            addedFront.reparent(targetFront)
+
+            // The actor is reconnected to the ownership tree, unorphan
+            // it.
+            this._orphaned.delete(addedFront);
+            addedFronts.push(addedFront);
+          }
+          // Before passing to users, replace the added and removed actor
+          // ids with front in the mutation record.
+          emittedMutation.added = addedFronts;
+          emittedMutation.removed = removedFronts;
+          targetFront._form.numChildren = change.numChildren;
+        } else {
+          targetFront.updateMutation(change);
+        }
+
+        emitMutations.push(emittedMutation);
       }
       events.emit(this, "mutations", emitMutations);
     });
   }, {
     impl: "_getMutations"
   }),
 
   /**
--- a/toolkit/devtools/server/tests/mochitest/Makefile.in
+++ b/toolkit/devtools/server/tests/mochitest/Makefile.in
@@ -10,16 +10,17 @@ VPATH		= @srcdir@
 relativesrcdir	= @relativesrcdir@
 
 include $(DEPTH)/config/autoconf.mk
 
 MOCHITEST_CHROME_FILES	= \
 	inspector-helpers.js \
 	inspector-traversal-data.html \
 	test_inspector-mutations-attr.html \
+	test_inspector-mutations-childlist.html \
 	test_inspector-mutations-value.html \
 	test_inspector-release.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
@@ -17,34 +17,50 @@ const {_documentWalker} = devtools.requi
 if (!DebuggerServer.initialized) {
   DebuggerServer.init(() => true);
   DebuggerServer.addBrowserActors();
   SimpleTest.registerCleanupFunction(function() {
     DebuggerServer.destroy();
   });
 }
 
+var gAttachCleanups = [];
+
+SimpleTest.registerCleanupFunction(function() {
+  for (let cleanup of gAttachCleanups) {
+    cleanup();
+  }
+});
+
 /**
  * Open a tab, load the url, wait for it to signal its readiness,
  * find the tab with the debugger server, and call the callback.
+ *
+ * Returns a function which can be called to close the opened ta
+ * and disconnect its debugger client.
  */
 function attachURL(url, callback) {
   var win = window.open(url, "_blank");
+  var client = null;
 
-  SimpleTest.registerCleanupFunction(function() {
-    win.close();
-  });
+  let cleanup = () => {
+    if (client) {
+      client.close();
+      client = null;
+    }
+    if (win) {
+      win.close();
+      win = null;
+    }
+  };
+  gAttachCleanups.push(cleanup);
 
   window.addEventListener("message", function loadListener(event) {
     if (event.data === "ready") {
-      let client = new DebuggerClient(DebuggerServer.connectPipe());
-      SimpleTest.registerCleanupFunction(function() {
-        client.close();
-      });
-
+      client = new DebuggerClient(DebuggerServer.connectPipe());
       client.connect((applicationType, traits) => {
         client.listTabs(response => {
           for (let tab of response.tabs) {
             if (tab.url === url) {
               window.removeEventListener("message", loadListener, false);
               try {
                 callback(null, client, tab, win.document);
               } catch(ex) {
@@ -53,16 +69,18 @@ function attachURL(url, callback) {
               }
               break;
             }
           }
         });
       });
     }
   }, false);
+
+  return cleanup;
 }
 
 function sortOwnershipChildren(children) {
   return children.sort((a, b) => a.name.localeCompare(b.name));
 }
 
 function serverOwnershipSubtree(walker, node) {
   let actor = walker._refMap.get(node);
@@ -85,44 +103,50 @@ function serverOwnershipSubtree(walker, 
     children: sortOwnershipChildren(children)
   }
 }
 
 function serverOwnershipTree(walker) {
   let serverConnection = walker.conn._transport._serverConnection;
   let serverWalker = serverConnection.getActor(walker.actorID);
 
-  return serverOwnershipSubtree(serverWalker, serverWalker.rootDoc)
+  return {
+    root: serverOwnershipSubtree(serverWalker, serverWalker.rootDoc ),
+    orphaned: [serverOwnershipSubtree(serverWalker, o.rawNode) for (o of serverWalker._orphaned)]
+  };
 }
 
 function clientOwnershipSubtree(node) {
   return {
     name: node.actorID,
     children: sortOwnershipChildren([clientOwnershipSubtree(child) for (child of node.treeChildren())])
   }
 }
 
 function clientOwnershipTree(walker) {
-  return clientOwnershipSubtree(walker.rootNode);
+  return {
+    root: clientOwnershipSubtree(walker.rootNode),
+    orphaned: [clientOwnershipSubtree(o) for (o of walker._orphaned)]
+  }
 }
 
 function ownershipTreeSize(tree) {
   let size = 1;
   for (let child of tree.children) {
     size += ownershipTreeSize(child);
   }
   return size;
 }
 
 function assertOwnershipTrees(walker) {
   let serverTree = serverOwnershipTree(walker);
   let clientTree = clientOwnershipTree(walker);
   is(JSON.stringify(clientTree, null, ' '), JSON.stringify(serverTree, null, ' '), "Server and client ownership trees should match.");
 
-  return ownershipTreeSize(clientTree);
+  return ownershipTreeSize(clientTree.root);
 }
 
 // Verify that an actorID is inaccessible both from the client library and the server.
 function checkMissing(client, actorID) {
   let deferred = Promise.defer();
   let front = client.getActor(actorID);
   ok(!front, "Front shouldn't be accessible from the client for actorID: " + actorID);
 
--- a/toolkit/devtools/server/tests/mochitest/inspector-traversal-data.html
+++ b/toolkit/devtools/server/tests/mochitest/inspector-traversal-data.html
@@ -42,10 +42,13 @@
     <div id="t">t</div>
     <div id="u">u</div>
     <div id="v">v</div>
     <div id="w">w</div>
     <div id="x">x</div>
     <div id="y">y</div>
     <div id="z">z</div>
   </div>
+  <div id="longlist-sibling">
+    <div id="longlist-sibling-firstchild"></div>
+  </div>
 </body>
 </html>
\ No newline at end of file
new file mode 100644
--- /dev/null
+++ b/toolkit/devtools/server/tests/mochitest/test_inspector-mutations-childlist.html
@@ -0,0 +1,269 @@
+<!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 Promise = devtools.require("sdk/core/promise");
+const inspector = devtools.require("devtools/server/actors/inspector");
+
+window.onload = function() {
+  SimpleTest.waitForExplicitFinish();
+  runNextTest();
+}
+
+var gInspectee = null;
+var gWalker = null;
+var gClient = null;
+var gCleanupConnection = null;
+
+function setup(callback) {
+  let url = document.getElementById("inspectorContent").href;
+  gCleanupConnection = 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 => {
+      gClient = client;
+      gWalker = walker;
+    }).then(callback));
+  });
+}
+
+function teardown() {
+  gWalker = null;
+  gClient = null;
+  gInspectee = null;
+  if (gCleanupConnection) {
+    gCleanupConnection();
+    gCleanupConnection = null;
+  }
+}
+
+function assertOwnership() {
+  let num = assertOwnershipTrees(gWalker);
+}
+
+function setParent(nodeSelector, newParentSelector) {
+  let node = gInspectee.querySelector(nodeSelector);
+  if (newParentSelector) {
+    let newParent = gInspectee.querySelector(newParentSelector);
+    newParent.appendChild(node);
+  } else {
+    node.parentNode.removeChild(node);
+  }
+}
+
+function loadSelector(selector) {
+  return gWalker.querySelectorAll(gWalker.rootNode, selector).then(nodeList => {
+    return nodeList.items();
+  });
+}
+
+function loadSelectors(selectors) {
+  return Promise.all([loadSelector(sel) for (sel of selectors)]);
+}
+
+function doMoves(moves) {
+  for (let move of moves) {
+    setParent(move[0], move[1]);
+  }
+}
+
+/**
+ * Test a set of tree rearrangements and make sure they cause the expected changes.
+ */
+
+var gDummySerial = 0;
+
+function mutationTest(testSpec) {
+  return function() {
+    setup(() => {
+      promiseDone(loadSelectors(testSpec.load || ["html"]).then(() => {
+        if (testSpec.preCheck) {
+          testSpec.preCheck();
+        }
+        doMoves(testSpec.moves || []);
+
+        // Some of these moves will trigger no mutation events,
+        // so do a dummy change to the root node to trigger
+        // a mutation event anyway.
+        gInspectee.documentElement.setAttribute("data-dummy", gDummySerial++);
+
+        gWalker.once("mutations", (mutations) => {
+          // Filter out our dummy mutation.
+          let mutations = mutations.filter(change => {
+            if (change.type == "attributes" &&
+                change.attributeName == "data-dummy") {
+              return false;
+            }
+            return true;
+          });
+          assertOwnership();
+          if (testSpec.postCheck) {
+            testSpec.postCheck(mutations);
+          }
+          teardown();
+          runNextTest();
+        });
+      }));
+    })
+  }
+}
+
+// Verify that our dummy mutation works.
+addTest(mutationTest({
+  postCheck: function(mutations) {
+    is(mutations.length, 0, "Dummy mutation is filtered out.");
+  }
+}));
+
+// Test a simple move to a different location in the sibling list for the same
+// parent.
+addTest(mutationTest({
+  load: ["#longlist div"],
+  moves: [
+    ["#a", "#longlist"]
+  ],
+  postCheck: function(mutations) {
+    let remove = mutations[0];
+    is(remove.type, "childList", "First mutation should be a childList.")
+    ok(remove.removed.length > 0, "First mutation should be a removal.")
+    let add = mutations[1];
+    is(add.type, "childList", "Second mutation should be a childList removal.")
+    ok(add.added.length > 0, "Second mutation should be an addition.")
+    let a = add.added[0];
+    is(a.id, "a", "Added node should be #a");
+    is(a.parentNode(), remove.target, "Should still be a child of longlist.");
+    is(remove.target, add.target, "First and second mutations should be against the same node.");
+  }
+}));
+
+// Test a move to another location that is within our ownership tree.
+addTest(mutationTest({
+  load: ["#longlist div", "#longlist-sibling"],
+  moves: [
+    ["#a", "#longlist-sibling"]
+  ],
+  postCheck: function(mutations) {
+    let remove = mutations[0];
+    is(remove.type, "childList", "First mutation should be a childList.")
+    ok(remove.removed.length > 0, "First mutation should be a removal.")
+    let add = mutations[1];
+    is(add.type, "childList", "Second mutation should be a childList removal.")
+    ok(add.added.length > 0, "Second mutation should be an addition.")
+    let a = add.added[0];
+    is(a.id, "a", "Added node should be #a");
+    is(a.parentNode(), add.target, "Should still be a child of longlist.");
+    is(add.target.id, "longlist-sibling", "long-sibling should be the target.");
+  }
+}));
+
+// Move an unseen node with a seen parent into our ownership tree - should generate a
+// childList pair with no adds or removes.
+addTest(mutationTest({
+  load: ["#longlist"],
+  moves: [
+    ["#longlist-sibling", "#longlist"]
+  ],
+  postCheck: function(mutations) {
+    is(mutations.length, 2, "Should generate two mutations");
+    is(mutations[0].type, "childList", "Should be childList mutations.");
+    is(mutations[0].added.length, 0, "Should have no adds.");
+    is(mutations[0].removed.length, 0, "Should have no removes.");
+    is(mutations[1].type, "childList", "Should be childList mutations.");
+    is(mutations[1].added.length, 0, "Should have no adds.");
+    is(mutations[1].removed.length, 0, "Should have no removes.");
+  }
+}));
+
+// Move an unseen node with an unseen parent into our ownership tree.  Should only
+// generate one childList mutation with no adds or removes.
+addTest(mutationTest({
+  load: ["#longlist div"],
+  moves: [
+    ["#longlist-sibling-firstchild", "#longlist"]
+  ],
+  postCheck: function(mutations) {
+    is(mutations.length, 1, "Should generate two mutations");
+    is(mutations[0].type, "childList", "Should be childList mutations.");
+    is(mutations[0].added.length, 0, "Should have no adds.");
+    is(mutations[0].removed.length, 0, "Should have no removes.");
+  }
+}));
+
+// Move a node between unseen nodes, should generate no mutations.
+addTest(mutationTest({
+  load: ["html"],
+  moves: [
+    ["#longlist-sibling", "#longlist"]
+  ],
+  postCheck: function(mutations) {
+    is(mutations.length, 0, "Should generate no mutations.");
+  }
+}));
+
+// Orphan a node.
+addTest(mutationTest({
+  load: ["#longlist div"],
+  moves: [
+    ["#longlist", null]
+  ],
+  postCheck: function(mutations) {
+    is(mutations.length, 1, "Should generate one mutation.");
+    let change = mutations[0];
+    is(change.type, "childList", "Should be a childList.");
+    is(change.removed.length, 1, "Should have removed a child.");
+    let ownership = clientOwnershipTree(gWalker);
+    is(ownership.orphaned.length, 1, "Should have one orphaned subtree.");
+    is(ownershipTreeSize(ownership.orphaned[0]), 27, "Should have orphaned longlist and 26 children.");
+  }
+}));
+
+// Orphan a node by moving it into the tree but out of our visible subtree.
+addTest(mutationTest({
+  load: ["#longlist div"],
+  moves: [
+    ["#longlist", "#longlist-sibling"]
+  ],
+  postCheck: function(mutations) {
+    is(mutations.length, 1, "Should generate one mutation.");
+    let change = mutations[0];
+    is(change.type, "childList", "Should be a childList.");
+    is(change.removed.length, 1, "Should have removed a child.");
+    let ownership = clientOwnershipTree(gWalker);
+    is(ownership.orphaned.length, 1, "Should have one orphaned subtree.");
+    is(ownershipTreeSize(ownership.orphaned[0]), 27, "Should have orphaned longlist and 26 children.");
+  }
+}));
+
+addTest(function cleanup() {
+  delete gInspectee;
+  delete gWalker;
+  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>