Bug 732842 - Add assertions for memory reports. r=jlebar.

gfx/thebes/gfxWindowsPlatform.cpp

@@ -143,7 +143,7 @@ NS_MEMORY_REPORTER_IMPLEMENT(
     KIND_OTHER,
     UNITS_BYTES,
     GetD2DSurfaceVramUsage,
-    "Video memory used by D2D surfaces")
+    "Video memory used by D2D surfaces.")
 
 #endif
 

storage/src/mozStorageService.cpp

@@ -169,15 +169,15 @@ public:
   StorageSQLiteMultiReporter(Service *aService) 
   : mService(aService)
   {
-    NS_NAMED_LITERAL_CSTRING(mStmtDesc,
+    mStmtDesc = NS_LITERAL_CSTRING(
       "Memory (approximate) used by all prepared statements used by "
       "connections to this database.");
 
-    NS_NAMED_LITERAL_CSTRING(mCacheDesc,
+    mCacheDesc = NS_LITERAL_CSTRING(
       "Memory (approximate) used by all pager caches used by connections "
       "to this database.");
 
-    NS_NAMED_LITERAL_CSTRING(mSchemaDesc,
+    mSchemaDesc = NS_LITERAL_CSTRING(
       "Memory (approximate) used to store the schema for all databases "
       "associated with connections to this database.");
   }

toolkit/components/aboutmemory/content/aboutMemory.css

@@ -97,6 +97,10 @@ h2 {
   -moz-user-select: none;  /* no need to include this when cutting+pasting */
 }
 
+.debug {
+  font-size: 80%;
+}
+
 .hidden {
   display: none;
 }

toolkit/components/aboutmemory/content/aboutMemory.js

@@ -82,17 +82,19 @@ function flipBackslashes(aUnsafeStr)
   return aUnsafeStr.replace(/\\/g, '/');
 }
 
+const gAssertionFailureMsgPrefix = "aboutMemory.js assertion failed: ";
+
 function assert(aCond, aMsg)
 {
   if (!aCond) {
     reportAssertionFailure(aMsg)
-    throw("aboutMemory.js assertion failed: " + aMsg);
+    throw(gAssertionFailureMsgPrefix + aMsg);
   }
 }
 
 function reportAssertionFailure(aMsg)
 {
-  var debug = Cc["@mozilla.org/xpcom/debug;1"].getService(Ci.nsIDebug2);
+  let debug = Cc["@mozilla.org/xpcom/debug;1"].getService(Ci.nsIDebug2);
   if (debug.isDebugBuild) {
     debug.assertion(aMsg, "false", "aboutMemory.js", 0);
   }
@@ -100,7 +102,7 @@ function reportAssertionFailure(aMsg)
 
 function debug(x)
 {
-  appendElementWithText(document.body, "div", "legend", JSON.stringify(x));
+  appendElementWithText(document.body, "div", "debug", JSON.stringify(x));
 }
 
 //---------------------------------------------------------------------------
@@ -199,18 +201,45 @@ function processMemoryReporters(aMgr, aIgnoreSingle, aIgnoreMulti,
   //
   // - After this point we never use the original memory report again.
 
+  function handleReport(aProcess, aUnsafePath, aKind, aUnits, aAmount,
+                        aDescription)
+  {
+    checkReport(aUnsafePath, aKind, aUnits, aAmount, aDescription);
+    aHandleReport(aProcess, aUnsafePath, aKind, aUnits, aAmount, aDescription);
+  }
+
+  function handleException(aReporterStr, aUnsafePathOrName, aE)
+  {
+    // There are two exception cases that must be distinguished here.
+    //
+    // - We want to halt proceedings on exceptions thrown within this file
+    //   (i.e. assertion failures in handleReport);  such exceptions contain
+    //   gAssertionFailureMsgPrefix in their string representation.
+    //
+    // - We want to continue on when faced with exceptions thrown outside this
+    //   file (i.e. in collectReports).
+
+    let str = aE.toString();
+    if (str.search(gAssertionFailureMsgPrefix) >= 0) {
+      throw(aE); 
+    } else {
+      debug("Bad memory " + aReporterStr + " '" + aUnsafePathOrName +
+            "': " + aE);
+    }
+  }
+
   let e = aMgr.enumerateReporters();
   while (e.hasMoreElements()) {
     let rOrig = e.getNext().QueryInterface(Ci.nsIMemoryReporter);
     let unsafePath = rOrig.path;
     try {
       if (!aIgnoreSingle(unsafePath)) {
-        aHandleReport(rOrig.process, unsafePath, rOrig.kind, rOrig.units,
-                      rOrig.amount, rOrig.description);
+        handleReport(rOrig.process, unsafePath, rOrig.kind, rOrig.units,
+                     rOrig.amount, rOrig.description);
       }
     }
     catch (e) {
-      debug("Bad memory reporter " + unsafePath + ": " + e);
+      handleException("reporter", unsafePath, e);
     }
   }
   let e = aMgr.enumerateMultiReporters();
@@ -219,15 +248,47 @@ function processMemoryReporters(aMgr, aIgnoreSingle, aIgnoreMulti,
     let name = mrOrig.name;
     try {
       if (!aIgnoreMulti(name)) {
-        mrOrig.collectReports(aHandleReport, null);
+        mrOrig.collectReports(handleReport, null);
       }
     }
     catch (e) {
-      debug("Bad memory multi-reporter " + name + ": " + e);
+      handleException("multi-reporter", name, e);
     }
   }
 }
 
+// This regexp matches sentences and sentence fragments, i.e. strings that
+// start with a capital letter and ends with a '.'.  (The final sentence may be
+// in parentheses, so a ')' might appear after the '.'.)
+const gSentenceRegExp = /^[A-Z].*\.\)?$/;
+
+function checkReport(aUnsafePath, aKind, aUnits, aAmount, aDescription)
+{
+  if (aUnsafePath.startsWith("explicit/")) {
+    assert(aKind === KIND_HEAP || aKind === KIND_NONHEAP, "bad explicit kind");
+    assert(aUnits === UNITS_BYTES, "bad explicit units");
+    assert(aDescription.match(gSentenceRegExp),
+           "non-sentence explicit description");
+
+  } else if (aUnsafePath.startsWith("smaps/")) {
+    assert(aKind === KIND_NONHEAP, "bad smaps kind");
+    assert(aUnits === UNITS_BYTES, "bad smaps units");
+    assert(aDescription !== "", "empty smaps description");
+
+  } else if (aUnsafePath.startsWith("compartments/")) {
+    assert(aKind === KIND_OTHER, "bad compartments kind");
+    assert(aUnits === UNITS_COUNT, "bad compartments units");
+    assert(aAmount === 1, "bad amount");
+    assert(aDescription === "", "bad description");
+
+  } else {
+    assert(aUnsafePath.indexOf("/") === -1, "'other' path contains '/'");
+    assert(aKind === KIND_OTHER, "bad other kind: " + aUnsafePath);
+    assert(aDescription.match(gSentenceRegExp),
+           "non-sentence other description");
+  }
+}
+
 //---------------------------------------------------------------------------
 
 function clearBody()
@@ -597,9 +658,6 @@ function buildTree(aReports, aTreeName)
     // Add any missing nodes in the tree implied by the unsafePath.
     let r = aReports[unsafePath];
     if (r.treeNameMatches(aTreeName)) {
-      assert(r._kind === KIND_HEAP || r._kind === KIND_NONHEAP,
-             "reports in the tree must have KIND_HEAP or KIND_NONHEAP");
-      assert(r._units === UNITS_BYTES, "r._units === UNITS_BYTES");
       let unsafeNames = r._unsafePath.split('/');
       let u = t;
       for (let i = 0; i < unsafeNames.length; i++) {
@@ -1427,8 +1485,6 @@ function appendOtherElements(aP, aReportsByProcess)
   for (let unsafePath in aReportsByProcess) {
     let r = aReportsByProcess[unsafePath];
     if (!r._done) {
-      assert(r._kind === KIND_OTHER,
-             "_kind !== KIND_OTHER for " + flipBackslashes(r._unsafePath));
       assert(r._nMerged === undefined, "dup'd OTHER report");
       let o = new OtherReport(r._unsafePath, r._units, r._amount,
                               r._description);
@@ -1556,14 +1612,7 @@ function getCompartmentsByProcess(aMgr)
                         aDescription)
   {
     let process = aProcess === "" ? "Main" : aProcess;
-
-    assert(aKind        === KIND_OTHER, "bad kind");
-    assert(aUnits       === UNITS_COUNT, "bad units");
-    assert(aAmount      === 1, "bad amount");
-    assert(aDescription === "", "bad description");
-
     let unsafeNames = aUnsafePath.split('/');
-
     let isSystemCompartment;
     if (unsafeNames[0] === "compartments" && unsafeNames[1] == "system" &&
         unsafeNames.length == 3)
@@ -1627,7 +1676,7 @@ function appendProcessCompartmentsElementsHelper(aP, aCompartments, aKindString)
   }
   compartmentTextArray.sort();
 
-  for (var i = 0; i < compartmentTextArray.length; i++) {
+  for (let i = 0; i < compartmentTextArray.length; i++) {
     appendElementWithText(uPre, "span", "", compartmentTextArray[i]);
   }
 

toolkit/components/aboutmemory/tests/test_aboutcompartments.xul

@@ -80,7 +80,7 @@
     { name: "fake",
       collectReports: function(aCbObj, aClosure) {
         function f(aP, aK, aU, aA) {
-          aCbObj.callback("", aP, aK, aU, aA, "(desc)", aClosure);
+          aCbObj.callback("", aP, aK, aU, aA, "Desc.", aClosure);
         }
         f("explicit/a/d",     HEAP,    BYTES,  13 * MB);
         f("explicit/b/c",     NONHEAP, BYTES,  10 * MB);
@@ -111,7 +111,7 @@
        collectReports: function(aCbObj, aClosure) {
         // The amounts are given in pages, so multiply here by 4kb.
         function f(aP, aA) {
-          aCbObj.callback("", aP, NONHEAP, BYTES, aA * 4 * KB, "(desc)", aClosure);
+          aCbObj.callback("", aP, NONHEAP, BYTES, aA * 4 * KB, "Desc.", aClosure);
         }
         f("smaps/vsize/a",     24);
         f("smaps/swap/a",       1);

toolkit/components/aboutmemory/tests/test_aboutmemory.xul

@@ -60,7 +60,7 @@
       path:        aPath,
       kind:        aKind,
       units:       aUnits,
-      description: "(description)",
+      description: "Desc.",
       amount:      aAmount 
     };
   }
@@ -83,14 +83,14 @@
     f("", "other1",             OTHER,   111 * MB),
     f2("", "other4",            OTHER,   COUNT_CUMULATIVE, 888),
     // These compartments ones shouldn't be displayed.
-    f("", "compartments/user/foo",   OTHER, COUNT, 1),
-    f("", "compartments/system/foo", OTHER, COUNT, 1)
+    f2("", "compartments/user/foo",   OTHER, COUNT, 1),
+    f2("", "compartments/system/foo", OTHER, COUNT, 1)
   ];
   let fakeMultiReporters = [
     { name: "fake1",
       collectReports: function(aCbObj, aClosure) {
         function f(aP, aK, aU, aA) {
-          aCbObj.callback("", aP, aK, aU, aA, "(desc)", aClosure);
+          aCbObj.callback("", aP, aK, aU, aA, "Desc.", aClosure);
         }
         f("explicit/c/d",     NONHEAP, BYTES,  13 * MB),
         f("explicit/c/d",     NONHEAP, BYTES,  10 * MB), // dup
@@ -106,7 +106,7 @@
     { name: "fake2",
       collectReports: function(aCbObj, aClosure) {
         function f(aP, aK, aU, aA) {
-          aCbObj.callback("", aP, aK, aU, aA, "(desc)", aClosure);
+          aCbObj.callback("", aP, aK, aU, aA, "Desc.", aClosure);
         }
         f("other3",           OTHER,   COUNT, 777);
         f("other2",           OTHER,   BYTES, 222 * MB);
@@ -119,7 +119,7 @@
       collectReports: function(aCbObj, aClosure) {
         // The amounts are given in pages, so multiply here by 4kb.
         function f(aP, aA) {
-          aCbObj.callback("", aP, NONHEAP, BYTES, aA * 4 * KB, "(desc)", aClosure);
+          aCbObj.callback("", aP, NONHEAP, BYTES, aA * 4 * KB, "Desc.", aClosure);
         }
         f("smaps/vsize/a",     24);
         f("smaps/swap/a",       1);
@@ -166,9 +166,7 @@
                                 HEAP,    200 * MB),
     f("2nd", "explicit/compartment(compartment-url)",
                                 HEAP,    200 * MB),
-    // The escaping of compartment names must prevent this script from running.
-    f("2nd", "danger<script>window.alert(1)</script>",
-                                OTHER,   666 * MB),
+    f("2nd", "other0",          OTHER,   666 * MB),
     f("2nd", "other1",          OTHER,   111 * MB),
     // Even though the "smaps" reporter is a multi-reporter, if its in a
     // child process it'll be passed to the main process as single reports.
@@ -205,10 +203,7 @@
     f2("4th", "other3",          OTHER,   COUNT, -333),
     f2("4th", "other4",          OTHER,   COUNT_CUMULATIVE, -444),
     f2("4th", "other5",          OTHER,   PERCENTAGE, -555),
-    // Escaping should again prevent this script from running when the name
-    // is printed in the warning.
-    f2("4th", "other6-danger<script>window.alert(1)</script>",
-                                 OTHER,   PERCENTAGE, 66666),
+    f2("4th", "other6",          OTHER,   PERCENTAGE, 66666),
 
     // If a negative value is within a collapsed sub-tree in non-verbose mode,
     // we should get the warning at the top and the relevant sub-trees should
@@ -283,9 +278,9 @@ Explicit Allocations\n\
 └────101.00 MB (10.10%) ── heap-unclassified\n\
 \n\
 Other Measurements\n\
-  666.00 MB ── danger<script>window.alert(1)</script>\n\
 1,000.00 MB ── heap-allocated\n\
   100.00 MB ── heap-unallocated\n\
+  666.00 MB ── other0\n\
   111.00 MB ── other1\n\
 \n\
 3rd Process\n\
@@ -321,7 +316,7 @@ WARNING: the following values are negative or unreasonably large.\n\
  other3\n\
  other4\n\
  other5\n\
- other6-danger<script>window.alert(1)</script>\n\
+ other6\n\
 This indicates a defect in one or more memory reporters.  The invalid values are highlighted.\n\
 \n\
 Explicit Allocations\n\
@@ -341,7 +336,7 @@ Other Measurements\n\
       -333 ── other3 [?!]\n\
       -444 ── other4 [?!]\n\
     -5.55% ── other5 [?!]\n\
-   666.66% ── other6-danger<script>window.alert(1)</script> [?!]\n\
+   666.66% ── other6 [?!]\n\
 \n\
 5th Process\n\
 \n\
@@ -442,9 +437,9 @@ Virtual Size Breakdown\n\
 196,608 B (100.0%) ++ vsize\n\
 \n\
 Other Measurements\n\
-  698,351,616 B ── danger<script>window.alert(1)</script>\n\
 1,048,576,000 B ── heap-allocated\n\
   104,857,600 B ── heap-unallocated\n\
+  698,351,616 B ── other0\n\
   116,391,936 B ── other1\n\
 \n\
 3rd Process\n\
@@ -480,7 +475,7 @@ WARNING: the following values are negative or unreasonably large.\n\
  other3\n\
  other4\n\
  other5\n\
- other6-danger<script>window.alert(1)</script>\n\
+ other6\n\
 This indicates a defect in one or more memory reporters.  The invalid values are highlighted.\n\
 \n\
 Explicit Allocations\n\
@@ -499,7 +494,7 @@ Other Measurements\n\
           -333 ── other3 [?!]\n\
           -444 ── other4 [?!]\n\
         -5.55% ── other5 [?!]\n\
-       666.66% ── other6-danger<script>window.alert(1)</script> [?!]\n\
+       666.66% ── other6 [?!]\n\
 \n\
 5th Process\n\
 \n\

toolkit/components/aboutmemory/tests/test_aboutmemory2.xul

@@ -52,7 +52,7 @@
       path:        aPath,
       kind:        aKind,
       units:       BYTES,
-      description: "(description)",
+      description: "Desc.",
       amount:      aAmount
     };
   }

xpcom/base/nsIMemoryReporter.idl

@@ -105,12 +105,17 @@ interface nsIMemoryReporter : nsISupports
    *   So in the example above, |a| may not count any allocations counted by
    *   |d|, and vice versa.
    *
+   *   Reporters in this category must have kind HEAP or NONHEAP, units BYTES,
+   *   and a description that is a sentence (i.e. starts with a capital letter
+   *   and ends with a period, or similar).
+   *
    * - Paths starting with "smaps/" represent regions of virtual memory that the
    *   process has mapped.  The rest of the path describes the type of
    *   measurement; for instance, the reporter "smaps/rss/[stack]" might report
    *   how much of the process's stack is currently in physical memory.
    *
-   *   Reporters in this category must have kind NONHEAP and units BYTES.
+   *   Reporters in this category must have kind NONHEAP, units BYTES, and
+   *   a non-empty description.
    *
    * - Paths starting with "compartments/" represent the names of JS
    *   compartments.  Reporters in this category must paths of the form
@@ -119,7 +124,8 @@ interface nsIMemoryReporter : nsISupports
    *
    * - All other paths represent cross-cutting values and may overlap with any
    *   other reporter.  Reporters in this category must have paths that do not
-   *   contain '/' separators, and kind OTHER.
+   *   contain '/' separators, kind OTHER, and a description that is a
+   *   sentence.
    */
   readonly attribute AUTF8String path;