Bug 1226879 - Fix census tree sorting issue with inverted allocation stack breakdown; r=jimb

devtools/shared/heapsnapshot/CensusUtils.js

@@ -359,7 +359,7 @@ function diff(breakdown, startCensus, endCensus) {
   walk(breakdown, startCensus, visitor);
   return visitor.results();
 };
-exports.diff = diff
+exports.diff = diff;
 
 /**
  * Creates a hash map mapping node IDs to its parent node.

devtools/shared/heapsnapshot/census-tree-node.js

@@ -520,18 +520,6 @@ function invert(tree) {
     path.pop();
   }(tree));
 
-  // Next, do a depth-first search of the inverted tree and ensure that siblings
-  // are sorted by their self bytes/count.
-
-  (function ensureSorted(node) {
-    if (node.children) {
-      node.children.sort(compareBySelf);
-      for (let i = 0, length = node.children.length; i < length; i++) {
-        ensureSorted(node.children[i]);
-      }
-    }
-  }(inverted.node));
-
   // Ensure that the root node always has the totals.
   inverted.node.totalBytes = tree.totalBytes;
   inverted.node.totalCount = tree.totalCount;
@@ -677,5 +665,17 @@ exports.censusReportToCensusTreeNode = function (breakdown, report,
     result.totalCount = report[basisTotalCount];
   }
 
+  // Inverting and filtering could have messed up the sort order, so do a
+  // depth-first search of the tree and ensure that siblings are sorted.
+  const comparator = options.invert ? compareBySelf : compareByTotal;
+  (function ensureSorted(node) {
+    if (node.children) {
+      node.children.sort(comparator);
+      for (let i = 0, length = node.children.length; i < length; i++) {
+        ensureSorted(node.children[i]);
+      }
+    }
+  }(result));
+
   return result;
 };

devtools/shared/heapsnapshot/tests/unit/test_census_filtering_05.js

@@ -0,0 +1,71 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Test that filtered and inverted allocation stack census trees are sorted
+// properly.
+
+function run_test() {
+  const countBreakdown = { by: "count", count: true, bytes: true };
+
+  const BREAKDOWN = {
+    by: "allocationStack",
+    then: countBreakdown,
+    noStack: countBreakdown,
+  };
+
+  const stacks = [];
+
+  function foo(depth = 1) {
+    stacks.push(saveStack(depth));
+    bar(depth + 1);
+    baz(depth + 1);
+    stacks.push(saveStack(depth));
+  }
+
+  function bar(depth = 1) {
+    stacks.push(saveStack(depth));
+    stacks.push(saveStack(depth));
+  }
+
+  function baz(depth = 1) {
+    stacks.push(saveStack(depth));
+    bang(depth + 1);
+    stacks.push(saveStack(depth));
+  }
+
+  function bang(depth = 1) {
+    stacks.push(saveStack(depth));
+    stacks.push(saveStack(depth));
+    stacks.push(saveStack(depth));
+  }
+
+  foo();
+  bar();
+  baz();
+  bang();
+
+  const REPORT = new Map(stacks.map((s, i) => {
+    return [s, {
+      count: i + 1,
+      bytes: (i + 1) * 10
+    }];
+  }));
+
+  const tree = censusReportToCensusTreeNode(BREAKDOWN, REPORT, {
+    filter: "baz",
+    invert: true
+  });
+
+  dumpn("tree = " + JSON.stringify(tree, savedFrameReplacer, 4));
+
+  (function assertSortedBySelf(node) {
+    if (node.children) {
+      let lastSelfBytes = Infinity;
+      for (let child of node.children) {
+        ok(child.bytes <= lastSelfBytes, `${child.bytes} <= ${lastSelfBytes}`);
+        lastSelfBytes = child.bytes;
+        assertSortedBySelf(child);
+      }
+    }
+  }(tree));
+}

devtools/shared/heapsnapshot/tests/unit/xpcshell.ini

@@ -21,6 +21,7 @@ support-files =
 [test_census_filtering_02.js]
 [test_census_filtering_03.js]
 [test_census_filtering_04.js]
+[test_census_filtering_05.js]
 [test_census-tree-node-01.js]
 [test_census-tree-node-02.js]
 [test_census-tree-node-03.js]