Bug 1499906 (attempt 2) - Use toLocaleString(). r=erahm
authorNicholas Nethercote <nnethercote@mozilla.com>
Fri, 19 Oct 2018 15:11:36 +1100
changeset 490393 edee090da6bdce1e744b951145b0afa4ac4bd9d4
parent 490392 838f16e6dcf2239d5b94dc5d5086f22558467dba
child 490394 d8c9c172c24599b14ce744fd5b9f01f5916e5ae9
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewerserahm
bugs1499906
milestone64.0a1
Bug 1499906 (attempt 2) - Use toLocaleString(). r=erahm
toolkit/components/aboutmemory/content/aboutMemory.js
toolkit/components/aboutmemory/tests/test_aboutmemory.xul
toolkit/components/aboutmemory/tests/test_aboutmemory3.xul
--- a/toolkit/components/aboutmemory/content/aboutMemory.js
+++ b/toolkit/components/aboutmemory/content/aboutMemory.js
@@ -1122,17 +1122,17 @@ TreeNode.prototype = {
     this._maxAbsDescendant = max;
     return max;
   },
 
   toString() {
     switch (this._units) {
       case UNITS_BYTES: return formatBytes(this._amount);
       case UNITS_COUNT:
-      case UNITS_COUNT_CUMULATIVE: return formatInt(this._amount);
+      case UNITS_COUNT_CUMULATIVE: return formatNum(this._amount);
       case UNITS_PERCENTAGE: return formatPercentage(this._amount);
       default:
         throw "Invalid memory report(s): bad units in TreeNode.toString";
     }
   },
 };
 
 // Sort TreeNodes first by size, then by name.  The latter is important for the
@@ -1500,107 +1500,103 @@ function appendProcessAboutMemoryElement
     appendWarningElements(warningsDiv, hasKnownHeapAllocated,
                           aHasMozMallocUsableSize);
   }
 
   appendElementWithText(aP, "h3", "", "End of " + aProcess);
   appendLink("end", "start", "↑");
 }
 
-/**
- * Determines if a number has a negative sign when converted to a string.
- * Works even for -0.
- *
- * @param aN
- *        The number.
- * @return A boolean.
- */
-function hasNegativeSign(aN) {
-  if (aN === 0) { // this succeeds for 0 and -0
-    return 1 / aN === -Infinity; // this succeeds for -0
-  }
-  return aN < 0;
-}
+// Used for UNITS_BYTES values that are printed as MiB.
+const kMBStyle = {
+  minimumFractionDigits: 2,
+  maximumFractionDigits: 2,
+};
+
+// Used for UNITS_PERCENTAGE values.
+const kPercStyle = {
+  style: "percent",
+  minimumFractionDigits: 2,
+  maximumFractionDigits: 2,
+};
+
+// Used for fractions within the tree.
+const kFracStyle = {
+  style: "percent",
+  minimumIntegerDigits: 2,
+  minimumFractionDigits: 2,
+  maximumFractionDigits: 2,
+};
+
+// Used for special-casing 100% fractions within the tree.
+const kFrac1Style = {
+  style: "percent",
+  minimumIntegerDigits: 3,
+  minimumFractionDigits: 1,
+  maximumFractionDigits: 1,
+};
 
 /**
  * Formats an int as a human-readable string.
  *
  * @param aN
  *        The integer to format.
- * @param aExtra
- *        An extra string to tack onto the end.
+ * @param aOptions
+ *        Optional options object.
  * @return A human-readable string representing the int.
- *
- * Note: building an array of chars and converting that to a string with
- * Array.join at the end is more memory efficient than using string
- * concatenation.  See bug 722972 for details.
  */
-function formatInt(aN, aExtra) {
-  let neg = false;
-  if (hasNegativeSign(aN)) {
-    neg = true;
-    aN = -aN;
-  }
-  let s = [];
-  while (true) {
-    let k = aN % 1000;
-    aN = Math.floor(aN / 1000);
-    if (aN > 0) {
-      if (k < 10) {
-        s.unshift(",00", k);
-      } else if (k < 100) {
-        s.unshift(",0", k);
-      } else {
-        s.unshift(",", k);
-      }
-    } else {
-      s.unshift(k);
-      break;
-    }
-  }
-  if (neg) {
-    s.unshift("-");
-  }
-  if (aExtra) {
-    s.push(aExtra);
-  }
-  return s.join("");
+function formatNum(aN, aOptions) {
+  return aN.toLocaleString("en-US", aOptions);
 }
 
 /**
  * Converts a byte count to an appropriate string representation.
  *
  * @param aBytes
  *        The byte count.
  * @return The string representation.
  */
 function formatBytes(aBytes) {
-  let unit = gVerbose.checked ? " B" : " MB";
-
-  let s;
-  if (gVerbose.checked) {
-    s = formatInt(aBytes, unit);
-  } else {
-    let mbytes = (aBytes / (1024 * 1024)).toFixed(2);
-    let a = String(mbytes).split(".");
-    // If the argument to formatInt() is -0, it will print the negative sign.
-    s = formatInt(Number(a[0])) + "." + a[1] + unit;
-  }
-  return s;
+  return gVerbose.checked
+       ? `${formatNum(aBytes)} B`
+       : `${formatNum(aBytes / (1024 * 1024), kMBStyle)} MB`;
 }
 
 /**
- * Converts a percentage to an appropriate string representation.
+ * Converts a UNITS_PERCENTAGE value to an appropriate string representation.
  *
  * @param aPerc100x
  *        The percentage, multiplied by 100 (see nsIMemoryReporter).
  * @return The string representation
  */
 function formatPercentage(aPerc100x) {
-  return (aPerc100x / 100).toFixed(2) + "%";
+  // A percentage like 12.34% will have an aPerc100x value of 1234, and we need
+  // to divide that by 10,000 to get the 0.1234 that toLocaleString() wants.
+  return formatNum(aPerc100x / 10000, kPercStyle);
+}
+
+/*
+ * Converts a tree fraction to an appropriate string representation.
+ *
+ * @param aNum
+ *        The numerator.
+ * @param aDenom
+ *        The denominator.
+ * @return The string representation
+ */
+function formatTreeFrac(aNum, aDenom) {
+  // Two special behaviours here:
+  // - We treat 0 / 0 as 100%.
+  // - We want 4 digits, as much as possible, because it gives good vertical
+  //   alignment. For positive numbers, 00.00%--99.99% works straighforwardly,
+  //   but 100.0% needs special handling.
+  let num = aDenom === 0 ? 1 : (aNum / aDenom);
+  return (0.99995 <= num && num <= 1)
+         ? formatNum(1, kFrac1Style)
+         : formatNum(num, kFracStyle);
 }
 
 /**
  * Right-justifies a string in a field of a given width, padding as necessary.
  *
  * @param aS
  *        The string.
  * @param aN
@@ -1844,25 +1840,19 @@ function appendTreeElements(aP, aRoot, a
       d = aP;
     }
 
     // The value.
     appendElementWithText(d, "span", "mrValue" + (tIsInvalid ? " invalid" : ""),
                           valueText);
 
     // The percentage (omitted for single entries).
-    let percText;
     if (!aT._isDegenerate) {
-      // Treat 0 / 0 as 100%.
-      let num = aRoot._amount === 0 ? 100 : (100 * aT._amount / aRoot._amount);
-      let numText = num.toFixed(2);
-      percText = numText === "100.00"
-               ? " (100.0%)"
-               : (0 <= num && num < 10 ? " (0" : " (") + numText + "%)";
-      appendElementWithText(d, "span", "mrPerc", percText);
+      let percText = formatTreeFrac(aT._amount, aRoot._amount);
+      appendElementWithText(d, "span", "mrPerc", ` (${percText})`);
     }
 
     // The separator.
     appendElementWithText(d, "span", "mrSep", sep);
 
     // The entry's name.
     appendMrNameSpan(d, aT._description, aT._unsafeName,
                      tIsInvalid, aT._nMerged, aT._presence);
--- a/toolkit/components/aboutmemory/tests/test_aboutmemory.xul
+++ b/toolkit/components/aboutmemory/tests/test_aboutmemory.xul
@@ -266,17 +266,17 @@ WARNING: the following values are negati
 \n\
 This indicates a defect in one or more memory reporters. The invalid values are highlighted.\n\
 Explicit Allocations\n\
 \n\
 98.00 MB (100.0%) -- explicit\n\
 ├──150.00 MB (153.06%) ── js/compartment(http://too-big.com/)/stuff [?!]\n\
 ├───5.00 MB (05.10%) ── ok\n\
 └──-57.00 MB (-58.16%) -- (2 tiny) [?!]\n\
-   ├───-2.00 MB (-2.04%) ── neg1 [?!]\n\
+   ├───-2.00 MB (-02.04%) ── neg1 [?!]\n\
    └──-55.00 MB (-56.12%) ── heap-unclassified [?!]\n\
 \n\
 Other Measurements\n\
 \n\
  100.00 MB ── heap-allocated\n\
   -0.00 MB ── other1 [?!]\n\
 -222.00 MB ── other2 [?!]\n\
       -333 ── other3 [?!]\n\
@@ -333,24 +333,24 @@ This indicates a defect in one or more m
 Explicit Allocations\n\
 \n\
 99.95 MB (100.0%) -- explicit\n\
 ├──99.00 MB (99.05%) ── big\n\
 └───0.95 MB (00.95%) -- (3 tiny)\n\
     ├──0.96 MB (00.96%) ── heap-unclassified\n\
     ├──0.01 MB (00.01%) -- a\n\
     │  ├──0.04 MB (00.04%) ── pos\n\
-    │  ├──-0.01 MB (-0.01%) ── neg2 [?!]\n\
-    │  └──-0.02 MB (-0.02%) ── neg1 [?!]\n\
-    └──-0.02 MB (-0.02%) -- b/c [?!]\n\
+    │  ├──-0.01 MB (-00.01%) ── neg2 [?!]\n\
+    │  └──-0.02 MB (-00.02%) ── neg1 [?!]\n\
+    └──-0.02 MB (-00.02%) -- b/c [?!]\n\
        ├───0.01 MB (00.01%) ── g/h\n\
        ├───0.00 MB (00.00%) ── i/j\n\
-       └──-0.04 MB (-0.04%) -- d [?!]\n\
+       └──-0.04 MB (-00.04%) -- d [?!]\n\
           ├───0.02 MB (00.02%) ── e\n\
-          └──-0.06 MB (-0.06%) ── f [?!]\n\
+          └──-0.06 MB (-00.06%) ── f [?!]\n\
 \n\
 Other Measurements\n\
 \n\
 100.00 MB ── heap-allocated\n\
 \n\
 End of 5th\n\
 ";
 
@@ -436,17 +436,17 @@ WARNING: the following values are negati
     other5 \n\
 \n\
 This indicates a defect in one or more memory reporters. The invalid values are highlighted.\n\
 Explicit Allocations\n\
 \n\
 102,760,448 B (100.0%) -- explicit\n\
 ├──157,286,400 B (153.06%) ── js/compartment(http://too-big.com/)/stuff [?!]\n\
 ├────5,242,880 B (05.10%) ── ok\n\
-├───-2,097,152 B (-2.04%) ── neg1 [?!]\n\
+├───-2,097,152 B (-02.04%) ── neg1 [?!]\n\
 └──-57,671,680 B (-56.12%) ── heap-unclassified [?!]\n\
 \n\
 Other Measurements\n\
 \n\
  104,857,600 B ── heap-allocated\n\
         -111 B ── other1 [?!]\n\
 -232,783,872 B ── other2 [?!]\n\
           -333 ── other3 [?!]\n\
@@ -502,24 +502,24 @@ WARNING: the following values are negati
 This indicates a defect in one or more memory reporters. The invalid values are highlighted.\n\
 Explicit Allocations\n\
 \n\
 104,801,280 B (100.0%) -- explicit\n\
 ├──103,809,024 B (99.05%) ── big\n\
 ├────1,007,616 B (00.96%) ── heap-unclassified\n\
 ├───────10,240 B (00.01%) -- a\n\
 │       ├──40,960 B (00.04%) ── pos\n\
-│       ├──-10,240 B (-0.01%) ── neg2 [?!]\n\
-│       └──-20,480 B (-0.02%) ── neg1 [?!]\n\
-└──────-25,600 B (-0.02%) -- b/c [?!]\n\
+│       ├──-10,240 B (-00.01%) ── neg2 [?!]\n\
+│       └──-20,480 B (-00.02%) ── neg1 [?!]\n\
+└──────-25,600 B (-00.02%) -- b/c [?!]\n\
        ├───10,240 B (00.01%) ── g/h\n\
        ├────5,120 B (00.00%) ── i/j\n\
-       └──-40,960 B (-0.04%) -- d [?!]\n\
+       └──-40,960 B (-00.04%) -- d [?!]\n\
           ├───20,480 B (00.02%) ── e\n\
-          └──-61,440 B (-0.06%) ── f [?!]\n\
+          └──-61,440 B (-00.06%) ── f [?!]\n\
 \n\
 Other Measurements\n\
 \n\
 104,857,600 B ── heap-allocated\n\
 \n\
 End of 5th\n\
 ";
 
--- a/toolkit/components/aboutmemory/tests/test_aboutmemory3.xul
+++ b/toolkit/components/aboutmemory/tests/test_aboutmemory3.xul
@@ -358,37 +358,37 @@ Other Measurements\n\
 \n\
 -0.00 MB (100.0%) -- p8\n\
 └──-0.00 MB (100.0%) -- a\n\
    ├──-0.00 MB (50.00%) -- b\n\
    │  ├──-0.00 MB (31.82%) -- c\n\
    │  │  ├──-0.00 MB (18.18%) ── e [-]\n\
    │  │  └──-0.00 MB (13.64%) ── d [-]\n\
    │  ├──-0.00 MB (22.73%) ── f [-]\n\
-   │  └───0.00 MB (-4.55%) ── (fake child) [!]\n\
+   │  └───0.00 MB (-04.55%) ── (fake child) [!]\n\
    └──-0.00 MB (50.00%) -- g\n\
       ├──-0.00 MB (31.82%) ── i [-]\n\
       ├──-0.00 MB (27.27%) ── h [-]\n\
-      └───0.00 MB (-9.09%) ── (fake child) [!]\n\
+      └───0.00 MB (-09.09%) ── (fake child) [!]\n\
 \n\
 End of P8\n\
 ";
 
   // This is the output for a verbose diff.
   let expectedDiffVerbose =
 "\
 P\n\
 \n\
 WARNING: the 'heap-allocated' memory reporter does not work for this platform and/or configuration. This means that 'heap-unclassified' is not shown and the 'explicit' tree shows less memory than it should.\n\
 Explicit Allocations\n\
 \n\
 -10,005 B (100.0%) -- explicit\n\
 ├──-10,000 B (99.95%) ── storage/prefixset/goog-phish-shavar\n\
 ├───────-6 B (00.06%) ── spell-check [2]\n\
-└────────1 B (-0.01%) ── xpcom/category-manager\n\
+└────────1 B (-00.01%) ── xpcom/category-manager\n\
 \n\
 Other Measurements\n\
 \n\
 1,002,000 B (100.0%) -- a\n\
 ├──1,000,000 B (99.80%) ── b\n\
 ├──────1,000 B (00.10%) -- c\n\
 │      ├──-999,000 B (-99.70%) ── e\n\
 │      ├──998,000 B (99.60%) ── d\n\
@@ -456,21 +456,21 @@ 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\
+   │  └────1 B (-04.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\
+      └────2 B (-09.09%) ── (fake child) [!]\n\
 \n\
 End of P8\n\
 ";
 
   // This is the output for the crash reports diff.
   let expectedDiff2 =
 "\
 Main Process (pid NNN)\n\