Bug 986300 - Fix a tricky corner case involving memory report diffs. r=kats.

--HG--
extra : rebase_source : 001ae6ee0e81c40a7437ccf65add3e735fada6fa

toolkit/components/aboutmemory/content/aboutMemory.js

@@ -720,10 +720,11 @@ DReport.prototype = {
   }
 };
 
-// Constants that indicate if a DReport was present in both data sets, or
-// added/removed in the second data set.
+// Constants that indicate if a DReport was present only in one of the data
+// sets, or had to be added for balance.
 DReport.PRESENT_IN_FIRST_ONLY  = 1;
 DReport.PRESENT_IN_SECOND_ONLY = 2;
+DReport.ADDED_FOR_BALANCE = 3;
 
 /**
  * Make a report map, which has combined path+process strings for keys, and
@@ -1112,13 +1113,31 @@ function fillInTree(aRoot)
 
     } else {
       // Non-leaf node with multiple children.  Derive its _amount and
-      // _description entirely from its children.
+      // _description entirely from its children...
       let kidsBytes = 0;
       for (let i = 0; i < aT._kids.length; i++) {
         kidsBytes += fillInNonLeafNodes(aT._kids[i]);
       }
-      assert(aT._amount === undefined, "_amount already set for non-leaf node");
-      aT._amount = kidsBytes;
+
+      // ... except in one special case. When diffing two memory report sets,
+      // if one set has a node with children and the other has the same node
+      // but without children -- e.g. the first has "a/b/c" and "a/b/d", but
+      // the second only has "a/b" -- we need to add a fake node "a/b/(fake)"
+      // to the second to make the trees comparable. It's ugly, but it works.
+      if (aT._amount !== undefined &&
+          (aT._presence === DReport.PRESENT_IN_FIRST_ONLY ||
+           aT._presence === DReport.PRESENT_IN_SECOND_ONLY)) {
+        aT._amount += kidsBytes;
+        let fake = new TreeNode('(fake child)', aT._units);
+        fake._presence = DReport.ADDED_FOR_BALANCE;
+        fake._amount = aT._amount - kidsBytes;
+        aT._kids.push(fake);
+        delete aT._presence;
+      } else {
+        assert(aT._amount === undefined,
+               "_amount already set for non-leaf node")
+        aT._amount = kidsBytes;
+      }
       aT._description = "The sum of all entries below this one.";
     }
     return aT._amount;
@@ -1580,23 +1599,28 @@ function appendMrNameSpan(aP, aDescription, aUnsafeName, aIsInvalid, aNMerged,
   }
 
   if (aPresence) {
-    let c, nth;
+    let c, title;
     switch (aPresence) {
      case DReport.PRESENT_IN_FIRST_ONLY:
       c = '-';
-      nth = "first";
+      title = "This value was only present in the first set of memory reports.";
       break;
      case DReport.PRESENT_IN_SECOND_ONLY:
       c = '+';
-      nth = "second";
+      title = "This value was only present in the second set of memory reports.";
+      break;
+     case DReport.ADDED_FOR_BALANCE:
+      c = '!';
+      title = "One of the sets of memory reports lacked children for this " +
+              "node's parent. This is a fake child node added to make the " +
+              "two memory sets comparable.";
       break;
      default: assert(false, "bad presence");
       break;
     }
     let noteSpan = appendElementWithText(aP, "span", "mrNote",
                                          " [" + c + "]\n");
-    noteSpan.title =
-      "This value was only present in the " + nth + " set of memory reports.";
+    noteSpan.title = title;
   }
 }
 

toolkit/components/aboutmemory/tests/memory-reports-diff1.json

@@ -17,7 +17,15 @@
 
     {"process": "P3", "path": "p3", "kind": 2, "units": 0, "amount": 55, "description": "Desc."},
 
-    {"process": "P5", "path": "p5", "kind": 2, "units": 0, "amount": 0, "description": "Desc."}
+    {"process": "P5", "path": "p5", "kind": 2, "units": 0, "amount": 0, "description": "Desc."},
+
+    {"process": "P7", "path": "p7", "kind": 2, "units": 0, "amount": 5, "description": "Desc."},
+
+    {"process": "P8", "path": "p8/a/b/c/d", "kind": 2, "units": 0, "amount": 3, "description": "Desc."},
+    {"process": "P8", "path": "p8/a/b/c/e", "kind": 2, "units": 0, "amount": 4, "description": "Desc."},
+    {"process": "P8", "path": "p8/a/b/f", "kind": 2, "units": 0, "amount": 5, "description": "Desc."},
+    {"process": "P8", "path": "p8/a/g/h", "kind": 2, "units": 0, "amount": 6, "description": "Desc."},
+    {"process": "P8", "path": "p8/a/g/i", "kind": 2, "units": 0, "amount": 7, "description": "Desc."}
   ]
 }
 

toolkit/components/aboutmemory/tests/memory-reports-diff2.json

@@ -18,7 +18,13 @@
 
     {"process": "P4", "path": "p4", "kind": 2, "units": 0, "amount": 66, "description": "Desc."},
 
-    {"process": "P6", "path": "p6", "kind": 2, "units": 0, "amount": 0, "description": "Desc."}
+    {"process": "P6", "path": "p6", "kind": 2, "units": 0, "amount": 0, "description": "Desc."},
+
+    {"process": "P7", "path": "p7/b", "kind": 2, "units": 0, "amount": 3, "description": "Desc."},
+    {"process": "P7", "path": "p7/c", "kind": 2, "units": 0, "amount": 4, "description": "Desc."},
+
+    {"process": "P8", "path": "p8/a/b", "kind": 2, "units": 0, "amount": 1, "description": "Desc."},
+    {"process": "P8", "path": "p8/a/g", "kind": 2, "units": 0, "amount": 2, "description": "Desc."}
   ]
 }
 

toolkit/components/aboutmemory/tests/test_aboutmemory3.xul

@@ -302,6 +302,33 @@ Other Measurements\n\
 66 B ── p4 [+]\n\
 \n\
 End of P4\n\
+P7\n\
+Other Measurements\n\
+\n\
+7 B (100.0%) -- p7\n\
+├──4 B (57.14%) ── c [+]\n\
+└──3 B (42.86%) ── b [+]\n\
+\n\
+-5 B ── p7 [-]\n\
+\n\
+End of P7\n\
+P8\n\
+Other Measurements\n\
+\n\
+-22 B (100.0%) -- p8\n\
+└──-22 B (100.0%) -- a\n\
+   ├──-11 B (50.00%) -- b\n\
+   │  ├───-7 B (31.82%) -- c\n\
+   │  │   ├──-4 B (18.18%) ── e [-]\n\
+   │  │   └──-3 B (13.64%) ── d [-]\n\
+   │  ├───-5 B (22.73%) ── f [-]\n\
+   │  └────1 B (-4.55%) ── (fake child) [!]\n\
+   └──-11 B (50.00%) -- g\n\
+      ├───-7 B (31.82%) ── i [-]\n\
+      ├───-6 B (27.27%) ── h [-]\n\
+      └────2 B (-9.09%) ── (fake child) [!]\n\
+\n\
+End of P8\n\
 ";
 
   let frames = [