Bug 674158 - OOify aboutMemory.js. r=dolske.
authorNicholas Nethercote <nnethercote@mozilla.com>
Thu, 11 Aug 2011 00:04:44 -0700
changeset 74244 48c58d9cc3f3569cc04cf4fe111e597d04435480
parent 74243 dae43481a41baf907376a3a62a6a35b1f3a304fb
child 74245 ddf95830a967af2ba480da2cdc7926746fc05579
push id2
push userbsmedberg@mozilla.com
push dateFri, 19 Aug 2011 14:38:13 +0000
reviewersdolske
bugs674158
milestone8.0a1
Bug 674158 - OOify aboutMemory.js. r=dolske.
toolkit/components/aboutmemory/content/aboutMemory.js
toolkit/components/aboutmemory/tests/chrome/test_aboutmemory.xul
--- a/toolkit/components/aboutmemory/content/aboutMemory.js
+++ b/toolkit/components/aboutmemory/content/aboutMemory.js
@@ -130,86 +130,92 @@ function sendHeapMinNotifications()
     else
       runSoon(update);
   }
 
   var j = 0;
   sendHeapMinNotificationsInner();
 }
 
+function Reporter(aPath, aKind, aUnits, aAmount, aDescription)
+{
+  this._path        = aPath;
+  this._kind        = aKind;
+  this._units       = aUnits;
+  this._amount      = aAmount;
+  this._description = aDescription;
+  // this._nMerged is only defined if > 1
+  // this._done is defined when getBytes is called
+}
+
+Reporter.prototype = {
+  // Sum the values (accounting for possible kUnknown amounts), and mark |this|
+  // as a dup.  We mark dups because it's useful to know when a reporter is
+  // duplicated;  it might be worth investigating and splitting up to have
+  // non-duplicated names.
+  merge: function(r) {
+    if (this._amount !== kUnknown && r._amount !== kUnknown) {
+      this._amount += r._amount;
+    } else if (this._amount === kUnknown && r._amount !== kUnknown) {
+      this._amount = r._amount;
+    }
+    this._nMerged = this._nMerged ? this._nMerged + 1 : 2;
+  },
+
+  treeNameMatches: function(aTreeName) {
+    return this._path.slice(0, aTreeName.length) === aTreeName;
+  }
+};
+
 function getReportersByProcess()
 {
   var mgr = Cc["@mozilla.org/memory-reporter-manager;1"].
       getService(Ci.nsIMemoryReporterManager);
 
   // Process each memory reporter:
   // - Make a copy of it into a sub-table indexed by its process.  Each copy
-  //   looks like this:
-  //
-  //     interface Reporter {
-  //       _path:        string;
-  //       _kind:        number;
-  //       _units:       number;
-  //       _amount:      number;
-  //       _description: string;
-  //       _nMerged:     number;  (only defined if >= 2)
-  //     }
-  //
-  //   After this point we never use the original memory reporter again.
+  //   is a Reporter object.  After this point we never use the original memory
+  //   reporter again.
   //
   // - Note that copying rOrig.amount (which calls a C++ function under the
   //   IDL covers) to r._amount for every reporter now means that the
   //   results as consistent as possible -- measurements are made all at
   //   once before most of the memory required to generate this page is
   //   allocated.
   var reportersByProcess = {};
 
   function addReporter(aProcess, aPath, aKind, aUnits, aAmount, aDescription)
   {
     var process = aProcess === "" ? "Main" : aProcess;
-    var r = {
-      _path:        aPath,
-      _kind:        aKind,
-      _units:       aUnits,
-      _amount:      aAmount,
-      _description: aDescription
-    };
+    var r = new Reporter(aPath, aKind, aUnits, aAmount, aDescription);
     if (!reportersByProcess[process]) {
       reportersByProcess[process] = {};
     }
     var reporters = reportersByProcess[process];
     var reporter = reporters[r._path];
     if (reporter) {
-      // Already an entry;  must be a duplicated reporter.  This can
-      // happen legitimately.  Sum the values (accounting for possible kUnknown
-      // amounts), and mark the reporter as a dup.  We mark dups because it's
-      // useful to know when a reporter is duplicated;  it might be worth
-      // investigating and splitting up to have non-duplicated names.
-      if (reporter._amount !== kUnknown && r._amount !== kUnknown) {
-        reporter._amount += r._amount;
-      } else if (reporter._amount === kUnknown && r._amount !== kUnknown) {
-        reporter._amount = r._amount;
-      }
-      reporter._nMerged = reporter._nMerged ? reporter._nMerged + 1 : 2;
+      // Already an entry;  must be a duplicated reporter.  This can happen
+      // legitimately.  Merge them.
+      reporter.merge(r);
     } else {
       reporters[r._path] = r;
     }
   }
 
   // Process vanilla reporters first, then multi-reporters.
   var e = mgr.enumerateReporters();
   while (e.hasMoreElements()) {
     var rOrig = e.getNext().QueryInterface(Ci.nsIMemoryReporter);
     addReporter(rOrig.process, rOrig.path, rOrig.kind, rOrig.units,
                 rOrig.amount, rOrig.description);
   }
   var e = mgr.enumerateMultiReporters();
   while (e.hasMoreElements()) {
-    var r = e.getNext().QueryInterface(Ci.nsIMemoryMultiReporter);
-    r.collectReports(addReporter, null);
+    var mrOrig = e.getNext().QueryInterface(Ci.nsIMemoryMultiReporter);
+    mrOrig.collectReports(addReporter, null);
   }
 
   return reportersByProcess;
 }
 
 /**
  * Top-level function that does the work of generating the page.
  */
@@ -261,161 +267,148 @@ function update()
           "reporter to see a detailed description of what it measures.</span>"
           "</div>";
 
   var div = document.createElement("div");
   div.innerHTML = text;
   content.appendChild(div);
 }
 
-// Compare two 'explicit' memory reporter nodes.
-function cmpExplicitReporters(a, b)
+// There are two kinds of TreeNode.  Those that correspond to Reporters
+// have more properties.  The remainder are just scaffolding nodes for the
+// tree, whose values are derived from their children.
+function TreeNode(aName)
 {
-  assert(a._units === undefined && b._units === undefined,
-         "'explicit' tree nodes must not have _units defined");
-  return b._amount - a._amount;
+  // Nb: _units is not needed, it's always UNITS_BYTES.
+  this._name = aName;
+  this._kids = [];
+  // All TreeNodes have these properties added later:
+  // - _amount (which is never |kUnknown|)
+  // - _description
+  //
+  // TreeNodes corresponding to Reporters have these properties added later:
+  // - _kind
+  // - _nMerged (if > 1)
+  // - _hasProblem (only defined if true)
+}
+
+TreeNode.prototype = {
+  findKid: function(aName) {
+    for (var i = 0; i < this._kids.length; i++) {
+      if (this._kids[i]._name === aName) {
+        return this._kids[i];
+      }
+    }
+    return undefined;
+  },
+
+  toString: function() {
+    return formatBytes(this._amount);
+  }
 };
 
-// Compare two memory reporter nodes from the 'other measurements' list.
-function cmpOtherReporters(a, b)
-{
-  return a._path < b._path ? -1 :
-         a._path > b._path ?  1 :
-         0;
+TreeNode.compare = function(a, b) {
+  return b._amount - a._amount;
 };
 
-function findKid(aName, aKids)
-{
-  for (var i = 0; i < aKids.length; i++) {
-    if (aKids[i]._name === aName) {
-      return aKids[i];
-    }
-  }
-  return undefined;
-}
-
 /**
  * From a list of memory reporters, builds a tree that mirrors the tree
  * structure that will be shown as output.
  *
  * @param aReporters
- *        The list of memory reporters.
- * @return The built tree.  The tree nodes have this structure:
- *         interface Node {
- *           _name:        string;
- *           _kind:        number;
- *           _amount:      number;    (non-negative or 'kUnknown')
- *           _description: string;
- *           _kids:        [Node];
- *           _hasReporter: boolean;   (only defined if 'true')
- *           _hasProblem:  boolean;   (only defined if 'true')
- *           _nMerged:     number;    (only defined if >= 2)
- *         }
- * _units isn't needed because it's always UNITS_BYTES for 'explicit'
- * reporters.
+ *        The table of Reporters, indexed by path.
+ * @return The built tree.
  */
 function buildTree(aReporters)
 {
   const treeName = "explicit";
 
-  // We want to process all reporters that begin with 'treeName'.  First we
-  // build the tree but only fill in '_name', '_kind', '_kids', maybe
-  // '_hasReporter' and maybe '_nMerged'.  This is done top-down from the
-  // reporters.
-  var t = {
-    _name: "falseRoot",
-    _kind: KIND_OTHER,
-    _kids: []
-  };
+  // We want to process all reporters that begin with |treeName|.  First we
+  // build the tree but only fill the properties that we can with a top-down
+  // traversal.
+  var t = new TreeNode("falseRoot");
   for (var path in aReporters) {
+    // Add any missing nodes in the tree implied by the path.
     var r = aReporters[path];
-    if (r._path.slice(0, treeName.length) === treeName) {
+    if (r.treeNameMatches(treeName)) {
       assert(r._kind === KIND_HEAP || r._kind === KIND_NONHEAP,
              "reporters in the tree must have KIND_HEAP or KIND_NONHEAP");
       assert(r._units === UNITS_BYTES);
       var names = r._path.split('/');
       var u = t;
       for (var i = 0; i < names.length; i++) {
         var name = names[i];
-        var uMatch = findKid(name, u._kids);
+        var uMatch = u.findKid(name);
         if (uMatch) {
           u = uMatch;
         } else {
-          var v = {
-            _name: name,
-            _kind: KIND_OTHER,
-            _kids: []
-          };
+          var v = new TreeNode(name);
           u._kids.push(v);
           u = v;
         }
       }
+      // Fill in extra details from the Reporter.
       u._kind = r._kind;
-      u._hasReporter = true;
       if (r._nMerged) {
         u._nMerged = r._nMerged;
       }
     }
   }
   // Using falseRoot makes the above code simpler.  Now discard it, leaving
   // treeName at the root.
   t = t._kids[0];
 
-  // Next, fill in '_description' and '_amount', and maybe '_hasProblem'
-  // for each node.  This is done bottom-up because for most non-leaf nodes
-  // '_amount' and '_description' are determined from the child nodes.
+  // Next, fill in the remaining properties bottom-up.
+  // Note that this function never returns kUnknown.
   function fillInTree(aT, aPrepath)
   {
     var path = aPrepath ? aPrepath + '/' + aT._name : aT._name;
     if (aT._kids.length === 0) {
       // Leaf node.  Must have a reporter.
+      assert(aT._kind !== undefined);
       aT._description = getDescription(aReporters, path);
       var amount = getBytes(aReporters, path);
       if (amount !== kUnknown) {
         aT._amount = amount;
       } else {
         aT._amount = 0;
         aT._hasProblem = true;
       }
     } else {
       // Non-leaf node.  Get the size of the children.
       var childrenBytes = 0;
       for (var i = 0; i < aT._kids.length; i++) {
         // Allow for kUnknown, treat it like 0.
-        var b = fillInTree(aT._kids[i], path);
-        childrenBytes += (b === kUnknown ? 0 : b);
+        childrenBytes += fillInTree(aT._kids[i], path);
       }
-      if (aT._hasReporter === true) {
+      if (aT._kind !== undefined) {
         aT._description = getDescription(aReporters, path);
         var amount = getBytes(aReporters, path);
         if (amount !== kUnknown) {
           // Non-leaf node with its own reporter.  Use the reporter and add
           // an "other" child node.
           aT._amount = amount;
-          var other = {
-            _name: "other",
-            _kind: KIND_OTHER,
-            _description: "All unclassified " + aT._name + " memory.",
-            _amount: aT._amount - childrenBytes,
-            _kids: []
-          };
+          var other = new TreeNode("other");
+          other._description = "All unclassified " + aT._name + " memory.",
+          other._amount = aT._amount - childrenBytes,
           aT._kids.push(other);
         } else {
           // Non-leaf node with a reporter that returns kUnknown.
           // Use the sum of the children and mark it as problematic.
           aT._amount = childrenBytes;
           aT._hasProblem = true;
         }
       } else {
         // Non-leaf node without its own reporter.  Derive its size and
         // description entirely from its children.
         aT._amount = childrenBytes;
         aT._description = "The sum of all entries below '" + aT._name + "'.";
       }
     }
+    assert(aT._amount !== kUnknown);
     return aT._amount;
   }
   fillInTree(t, "");
 
   // Determine how many bytes are reported by heap reporters.  Be careful
   // with non-leaf reporters;  if we count a non-leaf reporter we don't want
   // to count any of its child reporters.
   var s = "";
@@ -437,139 +430,117 @@ function buildTree(aReporters)
   // in the "Other Measurements" list.
   var heapUsedBytes = getBytes(aReporters, "heap-allocated", true);
   var unknownHeapUsedBytes = 0;
   var hasProblem = true;
   if (heapUsedBytes !== kUnknown) {
     unknownHeapUsedBytes = heapUsedBytes - getKnownHeapUsedBytes(t);
     hasProblem = false;
   }
-  var heapUnclassified = {
-    _name: "heap-unclassified",
-    _kind: KIND_HEAP,
-    _description:
+  var heapUnclassified = new TreeNode("heap-unclassified");
+  // This kindToString() ensures the "(Heap)" prefix is set without having to
+  // set the _kind property, which would mean that there is a corresponding
+  // Reporter for this TreeNode (which isn't true).
+  heapUnclassified._description =
+      kindToString(KIND_HEAP) +
       "Memory not classified by a more specific reporter. This includes " +
       "waste due to internal fragmentation in the heap allocator (caused " +
-      "when the allocator rounds up request sizes).",
-    _amount: unknownHeapUsedBytes,
-    _hasProblem: hasProblem,
-    _kids: []
+      "when the allocator rounds up request sizes).";
+  heapUnclassified._amount = unknownHeapUsedBytes;
+  if (hasProblem) {
+    heapUnclassified._hasProblem = true;
   }
+
   t._kids.push(heapUnclassified);
   t._amount += unknownHeapUsedBytes;
 
   return t;
 }
 
 /**
- * Sort all kid nodes from largest to smallest and aggregate
- * insignificant nodes.
+ * Sort all kid nodes from largest to smallest and aggregate insignificant
+ * nodes.
  *
  * @param aTotalBytes
- *        The size of the tree's root node
+ *        The size of the tree's root node.
  * @param aT
- *        The tree
+ *        The tree.
  */
 function filterTree(aTotalBytes, aT)
 {
   const omitThresholdPerc = 0.5; /* percent */
 
   function shouldOmit(aBytes)
   {
     return !gVerbose &&
            aTotalBytes !== kUnknown &&
            (100 * aBytes / aTotalBytes) < omitThresholdPerc;
   }
 
-  aT._kids.sort(cmpExplicitReporters);
+  aT._kids.sort(TreeNode.compare);
 
   for (var i = 0; i < aT._kids.length; i++) {
     if (shouldOmit(aT._kids[i]._amount)) {
       // This sub-tree is below the significance threshold
       // Remove it and all remaining (smaller) sub-trees, and
       // replace them with a single aggregate node.
       var i0 = i;
       var aggBytes = 0;
       for ( ; i < aT._kids.length; i++) {
         aggBytes += aT._kids[i]._amount;
       }
       aT._kids.splice(i0);
       var n = i - i0;
-      var rSub = {
-        _name: "(" + n + " omitted)",
-        _kind: KIND_OTHER,
-        _amount: aggBytes,
-        _description: n + " sub-trees that were below the " + 
-                      omitThresholdPerc + "% significance threshold.  " +
-                      "Click 'More verbose' at the bottom of this page " +
-                      "to see them.",
-        _kids: []
-      };
-      // Add the "omitted" sub-tree at the end and then resort, because the
-      // sum of the omitted sub-trees may be larger than some of the
-      // shown sub-trees.
+      var rSub = new TreeNode("(" + n + " omitted)");
+      rSub._amount = aggBytes;
+      rSub._description =
+        n + " sub-trees that were below the " + omitThresholdPerc +
+        "% significance threshold.  Click 'More verbose' at the bottom of " +
+        "this page to see them.";
+
+      // Add the "omitted" sub-tree at the end and then re-sort, because the
+      // sum of the omitted sub-trees may be larger than some of the shown
+      // sub-trees.
       aT._kids[i0] = rSub;
-      aT._kids.sort(cmpExplicitReporters);
+      aT._kids.sort(TreeNode.compare);
       break;
     }
     filterTree(aTotalBytes, aT._kids[i]);
   }
 }
 
 /**
  * Generates the text for a single process.
  *
  * @param aProcess
- *        The name of the process
+ *        The name of the process.
  * @param aReporters
- *        Table of reporters for this process, indexed by _path
- * @return The generated text
+ *        Table of Reporters for this process, indexed by _path.
+ * @return The generated text.
  */
 function genProcessText(aProcess, aReporters)
 {
   var tree = buildTree(aReporters);
   filterTree(tree._amount, tree);
 
   // Nb: the newlines give nice spacing if we cut+paste into a text buffer.
   var text = "";
   text += "<h1>" + aProcess + " Process</h1>\n\n";
   text += genTreeText(tree);
   text += genOtherText(aReporters);
   text += "<hr></hr>";
   return text;
 }
 
 /**
- * Returns the reporter's amount formatted as a human-readable string (with
- * units, if applicable).
- *
- * @param aReporter
- *        The reporter whose usage we're formatting
- * @return The reporter's amount formatted as a human-readable string
- */
-function formatReporterAmount(aReporter)
-{
-  switch(aReporter._units) {
-    case UNITS_BYTES:
-      return formatBytes(aReporter._amount);
-    case UNITS_COUNT:
-    case UNITS_COUNT_CUMULATIVE:
-      return formatInt(aReporter._amount);
-    case UNITS_PERCENTAGE:
-      return formatPercentage(aReporter._amount);
-    default: return "(???)"
-  }
-}
-
-/**
  * Formats an int as a human-readable string.
  *
  * @param aN
- *        The integer to format
- * @return A human-readable string representing the int
+ *        The integer to format.
+ * @return A human-readable string representing the int.
  */
 function formatInt(aN)
 {
   var neg = false;
   if (aN < 0) {
     neg = true;
     aN = -aN;
   }
@@ -592,18 +563,18 @@ function formatInt(aN)
   }
   return neg ? "-" + s : s;
 }
 
 /**
  * Converts a byte count to an appropriate string representation.
  *
  * @param aBytes
- *        The byte count
- * @return The string representation
+ *        The byte count.
+ * @return The string representation.
  */
 function formatBytes(aBytes)
 {
   var unit = gVerbose ? "B" : "MB";
 
   var s;
   if (gVerbose) {
     s = formatInt(aBytes) + " " + unit;
@@ -614,100 +585,96 @@ function formatBytes(aBytes)
   }
   return s;
 }
 
 /**
  * Converts a percentage to an appropriate string representation.
  *
  * @param aPerc100x
- *        The percentage, multiplied by 100 (see nsIMemoryReporter)
+ *        The percentage, multiplied by 100 (see nsIMemoryReporter).
  * @return The string representation
  */
 function formatPercentage(aPerc100x)
 {
   return (aPerc100x / 100).toFixed(2) + "%";
 }
 
 /**
- * Right-justifies a string in a field of a given width, padding as necessary
+ * Right-justifies a string in a field of a given width, padding as necessary.
  *
  * @param aS
- *        The string
+ *        The string.
  * @param aN
- *        The field width
+ *        The field width.
  * @param aC
- *        The char used to pad
- * @return The string representation
+ *        The char used to pad.
+ * @return The string representation.
  */
 function pad(aS, aN, aC)
 {
   var padding = "";
   var n2 = aN - aS.length;
   for (var i = 0; i < n2; i++) {
     padding += aC;
   }
   return padding + aS;
 }
 
 /**
- * Gets the byte count for a particular memory reporter and sets its _done
+ * Gets the byte count for a particular Reporter and sets its _done
  * property.
  *
  * @param aReporters
- *        Table of reporters for this process, indexed by _path
+ *        Table of Reporters for this process, indexed by _path.
  * @param aPath
- *        The path of the memory reporter
+ *        The path of the R.
  * @param aDoNotMark
- *        If set, the _done property is not set.
- * @return The byte count
+ *        If true, the _done property is not set.
+ * @return The byte count.
  */
 function getBytes(aReporters, aPath, aDoNotMark)
 {
   var r = aReporters[aPath];
-  if (r) {
-    var bytes = r._amount;
-    if (!aDoNotMark) {
-      r._done = true;
-    }
-    return bytes;
+  assert(r, "getBytes: no such Reporter: " + aPath);
+  if (!aDoNotMark) {
+    r._done = true;
   }
-  // Nb: this should never occur; all paths have been extracted from
-  // the original list of reporters and so the lookup should succeed.  Return
-  // an obviously wrong number that will likely be noticed.
-  return -2 * 1024 * 1024;
+  return r._amount;
 }
 
 /**
- * Gets the description for a particular memory reporter.
+ * Gets the description for a particular Reporter.
  *
  * @param aReporters
- *        Table of reporters for this process, indexed by _path
+ *        Table of Reporters for this process, indexed by _path.
  * @param aPath
- *        The path of the memory reporter
- * @return The description
+ *        The path of the Reporter.
+ * @return The description.
  */
 function getDescription(aReporters, aPath)
 {
   var r = aReporters[aPath];
-  return r ? r._description : "???";
+  assert(r, "getDescription: no such Reporter: " + aPath);
+  return r._description;
 }
 
 function genMrValueText(aValue)
 {
   return "<span class='mrValue'>" + aValue + "</span>";
 }
 
 function kindToString(aKind)
 {
   switch (aKind) {
    case KIND_NONHEAP: return "(Non-heap) ";
    case KIND_HEAP:    return "(Heap) ";
-   case KIND_OTHER:   return "";
-   default:           return "(???) ";
+   case KIND_OTHER:
+   case undefined:    return "";
+   default:           assert(false, "bad kind in kindToString");
   }
 }
 
 function escapeQuotes(aStr)
 {
   return aStr.replace(/\&/g, '&amp;').replace(/'/g, '&#39;');
 }
 
@@ -765,39 +732,39 @@ function genMrNameText(aKind, aDesc, aNa
   }
   return text + '\n';
 }
 
 /**
  * Generates the text for the tree, including its heading.
  *
  * @param aT
- *        The tree
- * @return The generated text
+ *        The tree.
+ * @return The generated text.
  */
 function genTreeText(aT)
 {
   var treeBytes = aT._amount;
-  var treeBytesLength = formatBytes(treeBytes).length;
+  var rootStringLength = aT.toString().length;
 
   /**
    * Generates the text for a particular tree, without a heading.
    *
    * @param aT
-   *        The tree
+   *        The tree.
    * @param aIndentGuide
    *        Records what indentation is required for this tree.  It has one
    *        entry per level of indentation.  For each entry, ._isLastKid
    *        records whether the node in question is the last child, and
    *        ._depth records how many chars of indentation are required.
-   * @param aParentBytesLength
-   *        The length of the formatted byte count of the top node in the tree
-   * @return The generated text
+   * @param aParentStringLength
+   *        The length of the formatted byte count of the top node in the tree.
+   * @return The generated text.
    */
-  function genTreeText2(aT, aIndentGuide, aParentBytesLength)
+  function genTreeText2(aT, aIndentGuide, aParentStringLength)
   {
     function repeatStr(aC, aN)
     {
       var s = "";
       for (var i = 0; i < aN; i++) {
         s += aC;
       }
       return s;
@@ -819,19 +786,18 @@ function genTreeText(aT)
         indent += repeatStr(" ", aIndentGuide[i]._depth - 1);
       }
       indent += aIndentGuide[i]._isLastKid ? kUpAndRight : kVerticalAndRight;
       indent += repeatStr(kHorizontal, aIndentGuide[i]._depth - 1);
     }
 
     // Indent more if this entry is narrower than its parent, and update
     // aIndentGuide accordingly.
-    var tMemoryUsedStr = formatBytes(aT._amount);
-    var tBytesLength = tMemoryUsedStr.length;
-    var extraIndentLength = Math.max(aParentBytesLength - tBytesLength, 0);
+    var tString = aT.toString();
+    var extraIndentLength = Math.max(aParentStringLength - tString.length, 0);
     if (extraIndentLength > 0) {
       for (var i = 0; i < extraIndentLength; i++) {
         indent += kHorizontal;
       }
       aIndentGuide[aIndentGuide.length - 1]._depth += extraIndentLength;
     }
     indent += "</span>";
 
@@ -840,30 +806,30 @@ function genTreeText(aT)
     if (aT._amount === treeBytes) {
       perc = "100.0";
     } else {
       perc = (100 * aT._amount / treeBytes).toFixed(2);
       perc = pad(perc, 5, '0');
     }
     perc = "<span class='mrPerc'>(" + perc + "%)</span> ";
 
-    var text = indent + genMrValueText(tMemoryUsedStr) + " " + perc +
+    var text = indent + genMrValueText(tString) + " " + perc +
                genMrNameText(aT._kind, aT._description, aT._name,
                              aT._hasProblem, aT._nMerged);
 
     for (var i = 0; i < aT._kids.length; i++) {
       // 3 is the standard depth, the callee adjusts it if necessary.
       aIndentGuide.push({ _isLastKid: (i === aT._kids.length - 1), _depth: 3 });
-      text += genTreeText2(aT._kids[i], aIndentGuide, tBytesLength);
+      text += genTreeText2(aT._kids[i], aIndentGuide, tString.length);
       aIndentGuide.pop();
     }
     return text;
   }
 
-  var text = genTreeText2(aT, [], treeBytesLength);
+  var text = genTreeText2(aT, [], rootStringLength);
   // Nb: the newlines give nice spacing if we cut+paste into a text buffer.
   const desc =
     "This tree covers explicit memory allocations by the application, " +
     "both at the operating system level (via calls to functions such as " +
     "VirtualAlloc, vm_allocate, and mmap), and at the heap allocation level " +
     "(via functions such as malloc, calloc, realloc, memalign, operator " +
     "new, and operator new[]).  It excludes memory that is mapped implicitly " +
     "such as code and data segments, and thread stacks.  It also excludes " +
@@ -872,65 +838,89 @@ function genTreeText(aT)
     "explicit allocation, but it does cover most (including the entire " +
     "heap), and therefore it is the single best number to focus on when " +
     "trying to reduce memory usage.";
                
   return "<h2 class='hasDesc' title='" + escapeQuotes(desc) +
          "'>Explicit Allocations</h2>\n" + "<pre>" + text + "</pre>\n";
 }
 
+function OtherReporter(aPath, aUnits, aAmount, aDescription, 
+                       aNMerged)
+{
+  // Nb: _kind is not needed, it's always KIND_OTHER.
+  this._path        = aPath;
+  this._units       = aUnits;
+  if (aAmount === kUnknown) {
+    this._amount     = 0;
+    this._hasProblem = true;
+  } else {
+    this._amount = aAmount;
+  }
+  this._description = aDescription;
+  this.asString = this.toString();
+}
+
+OtherReporter.prototype = {
+  toString: function() {
+    switch(this._units) {
+      case UNITS_BYTES:            return formatBytes(this._amount);
+      case UNITS_COUNT:
+      case UNITS_COUNT_CUMULATIVE: return formatInt(this._amount);
+      case UNITS_PERCENTAGE:       return formatPercentage(this._amount);
+      default:
+        assert(false, "bad units in OtherReporter.toString");
+    }
+  }
+};
+
+OtherReporter.compare = function(a, b) {
+  return a._path < b._path ? -1 :
+         a._path > b._path ?  1 :
+         0;
+};
+
 /**
  * Generates the text for the "Other Measurements" section.
  *
- * @param aReporters
- *        Table of reporters for this process, indexed by _path
- * @return The generated text
+ * @param aReportersByProcess
+ *        Table of Reporters for this process, indexed by _path.
+ * @return The generated text.
  */
-function genOtherText(aReporters)
+function genOtherText(aReportersByProcess)
 {
   // Generate an array of Reporter-like elements, stripping out all the
-  // reporters that have already been handled.  Also find the width of the
+  // Reporters that have already been handled.  Also find the width of the
   // widest element, so we can format things nicely.
-  var maxAmountLength = 0;
-  var rArray = [];
-  for (var path in aReporters) {
-    var r = aReporters[path];
+  var maxStringLength = 0;
+  var otherReporters = [];
+  for (var path in aReportersByProcess) {
+    var r = aReportersByProcess[path];
     if (!r._done) {
+      assert(r._kind === KIND_OTHER, "_kind !== KIND_OTHER for " + r._path);
+      assert(r.nMerged === undefined);  // we don't allow dup'd OTHER reporters 
       var hasProblem = false;
       if (r._amount === kUnknown) {
         hasProblem = true;
       }
-      var elem = {
-        _path:        r._path,
-        _kind:        r._kind,
-        _units:       r._units,
-        _amount:      hasProblem ? 0 : r._amount,
-        _description: r._description,
-        _hasProblem:  hasProblem,
-        _nMerged:     r._nMerged
-      };
-      rArray.push(elem);
-      var thisAmountLength = formatReporterAmount(elem).length;
-      if (thisAmountLength > maxAmountLength) {
-        maxAmountLength = thisAmountLength;
+      var o = new OtherReporter(r._path, r._units, r._amount, r._description);
+      otherReporters.push(o);
+      if (o.asString.length > maxStringLength) {
+        maxStringLength = o.asString.length;
       }
     }
   }
-  rArray.sort(cmpOtherReporters);
+  otherReporters.sort(OtherReporter.compare);
 
   // Generate text for the not-yet-printed values.
   var text = "";
-  for (var i = 0; i < rArray.length; i++) {
-    var elem = rArray[i];
-    assert(elem._kind === KIND_OTHER,
-           "elem._kind is not KIND_OTHER for " + elem._path);
-    text += genMrValueText(
-              pad(formatReporterAmount(elem), maxAmountLength, ' ')) + " ";
-    text += genMrNameText(elem._kind, elem._description, elem._path,
-                          elem._hasProblem, elem._nMerged);
+  for (var i = 0; i < otherReporters.length; i++) {
+    var o = otherReporters[i];
+    text += genMrValueText(pad(o.asString, maxStringLength, ' ')) + " ";
+    text += genMrNameText(KIND_OTHER, o._description, o._path, o._hasProblem);
   }
 
   // Nb: the newlines give nice spacing if we cut+paste into a text buffer.
   const desc = "This list contains other memory measurements that cross-cut " +
                "the requested memory measurements above."
   return "<h2 class='hasDesc' title='" + desc + "'>Other Measurements</h2>\n" +
          "<pre>" + text + "</pre>\n";
 }
--- a/toolkit/components/aboutmemory/tests/chrome/test_aboutmemory.xul
+++ b/toolkit/components/aboutmemory/tests/chrome/test_aboutmemory.xul
@@ -80,18 +80,17 @@
     f("", "explicit/b/c/a",     HEAP,     70 * MB),
     f("", "explicit/b/c/b",     HEAP,      2 * MB), // omitted
     f("", "explicit/c",         NONHEAP, 100 * MB),
     f("", "explicit/c/d",       NONHEAP,  13 * MB), // subsumed by parent
     f("", "explicit/g",         HEAP,      1 * MB), // internal, dup: merge
     f("", "explicit/g/a",       HEAP,      6 * MB),
     f("", "explicit/g/b",       HEAP,      5 * MB),
     f("", "other1",             OTHER,   111 * MB),
-    f2("", "other4",            OTHER,   COUNT_CUMULATIVE, 888),
-    f2("", "unknown-unit",      OTHER,   /*bogus unit*/999, 999)
+    f2("", "other4",            OTHER,   COUNT_CUMULATIVE, 888)
   ];
   var fakeMultiReporters = [
      { collectReports: function(cbObj, closure) {
           function f(p, k, u, a) { cbObj.callback("", p, k, u, a, "(desc)", closure); }
           f("explicit/c/d",     NONHEAP, BYTES,  10 * MB), // dup, subsumed by parent
           f("explicit/cc",      NONHEAP, BYTES,  13 * MB);
           f("explicit/cc",      NONHEAP, BYTES,  10 * MB); // dup
           f("explicit/d",       NONHEAP, BYTES, 499 * KB); // omitted
@@ -198,17 +197,16 @@ Other Measurements\n\
 500.00 MB -- heap-allocated\n\
 100.00 MB -- heap-unallocated\n\
 111.00 MB -- other1\n\
 222.00 MB -- other2\n\
       777 -- other3\n\
       888 -- other4\n\
    45.67% -- perc1\n\
   100.00% -- perc2\n\
-    (???) -- unknown-unit\n\
 \n\
 2nd Process\n\
 \n\
 Explicit Allocations\n\
 1,000.00 MB (100.0%) -- explicit\n\
 ├────499.00 MB (49.90%) -- a\n\
 │    └──499.00 MB (49.90%) -- b\n\
 │       └──499.00 MB (49.90%) -- c [3]\n\
@@ -271,17 +269,16 @@ Other Measurements\n\
 524,288,000 B -- heap-allocated\n\
 104,857,600 B -- heap-unallocated\n\
 116,391,936 B -- other1\n\
 232,783,872 B -- other2\n\
           777 -- other3\n\
           888 -- other4\n\
        45.67% -- perc1\n\
       100.00% -- perc2\n\
-        (???) -- unknown-unit\n\
 \n\
 2nd Process\n\
 \n\
 Explicit Allocations\n\
 1,048,576,000 B (100.0%) -- explicit\n\
 ├────523,239,424 B (49.90%) -- a\n\
 │    └──523,239,424 B (49.90%) -- b\n\
 │       └──523,239,424 B (49.90%) -- c [3]\n\