Bug 1194555 - Part 3: Remove |getReportsForThisProcess| from the nsIMemoryReporterManager interface. r=njn

|getReportsForThisProcess| differs from |getReports| in that it is limited to current process and is synchronous. When asynchronous memory reporters are added the function will no longer be able tobe synchronous. There isn't much utility in only measuring the current process, so we can remove the function and switch existing users to |getReports|.

image/test/mochitest/test_bug601470.html

@@ -30,12 +30,13 @@ window.onload = function() {
     amount += aAmount;
   }
 
-  mgr.getReportsForThisProcess(handleReport, null, /* anonymize = */ false);
+  var finished = function() {
+        ok(amount > 0, "we should be using a nonzero amount of memory");
+        ok(true, "yay, didn't crash!");
+        SimpleTest.finish();
+  }
 
-  ok(amount > 0, "we should be using a nonzero amount of memory");
-  ok(true, "yay, didn't crash!");
-
-  SimpleTest.finish();
+  mgr.getReports(handleReport, null, finished, null, /* anonymize = */ false);
 }
 
 </script>

toolkit/components/aboutmemory/tests/test_memoryReporters.xul

@@ -44,6 +44,8 @@
   const XUL_NS =
     "http:\\\\www.mozilla.org\\keymaster\\gatekeeper\\there.is.only.xul";
 
+  SimpleTest.waitForExplicitFinish();
+
   let vsizeAmounts = [];
   let residentAmounts = [];
   let heapAllocatedAmounts = [];
@@ -189,11 +191,33 @@
                 domSize, styleSize, otherSize, totalSize,
                 jsMilliseconds, nonJSMilliseconds);
 
-  mgr.getReportsForThisProcess(handleReportNormal, null,
-                               /* anonymize = */ false);
+  let asyncSteps = [
+    getReportsNormal,
+    getReportsAnonymized,
+    checkResults,
+    test_register_strong,
+    test_register_strong, // Make sure re-registering works
+    test_register_weak,
+    SimpleTest.finish
+  ];
 
-  mgr.getReportsForThisProcess(handleReportAnonymized, null,
-                               /* anonymize = */ true);
+  function runNext() {
+    setTimeout(asyncSteps.shift(), 0);
+  }
+
+  function getReportsNormal()
+  {
+    mgr.getReports(handleReportNormal, null,
+                   runNext, null,
+                   /* anonymize = */ false);
+  }
+
+  function getReportsAnonymized()
+  {
+    mgr.getReports(handleReportAnonymized, null,
+                   runNext, null,
+                   /* anonymize = */ true);
+  }
 
   function checkSizeReasonable(aName, aAmount)
   {
@@ -211,45 +235,50 @@
     }
   }
 
-  try {
-    // Nb: mgr.heapAllocated will throw NS_ERROR_NOT_AVAILABLE if this is a
-    // --disable-jemalloc build.  Allow for skipping this test on that
-    // exception, but *only* that exception.
-    let dummy = mgr.heapAllocated;
-    checkSpecialReport("heap-allocated", heapAllocatedAmounts);
-  } catch (ex) {
-    is(ex.result, Cr.NS_ERROR_NOT_AVAILABLE, "mgr.heapAllocated exception");
+  function checkResults()
+  {
+    try {
+      // Nb: mgr.heapAllocated will throw NS_ERROR_NOT_AVAILABLE if this is a
+      // --disable-jemalloc build.  Allow for skipping this test on that
+      // exception, but *only* that exception.
+      let dummy = mgr.heapAllocated;
+      checkSpecialReport("heap-allocated", heapAllocatedAmounts);
+    } catch (ex) {
+      is(ex.result, Cr.NS_ERROR_NOT_AVAILABLE, "mgr.heapAllocated exception");
+    }
+    // vsize may be unreasonable if ASAN is enabled
+    checkSpecialReport("vsize",          vsizeAmounts, /*canBeUnreasonable*/true);
+    checkSpecialReport("resident",       residentAmounts);
+
+    for (var reporter in jsGcHeapUsedGcThings) {
+      ok(jsGcHeapUsedGcThings[reporter] == 1);
+    }
+    checkSizeReasonable("js-main-runtime-gc-heap-committed/used/gc-things",
+                        jsGcHeapUsedGcThingsTotal);
+
+    ok(present.jsNonWindowCompartments,     "js-non-window compartments are present");
+    ok(present.windowObjectsJsCompartments, "window-objects/.../js compartments are present");
+    ok(present.places,                      "places is present");
+    ok(present.images,                      "images is present");
+    ok(present.xptiWorkingSet,              "xpti-working-set is present");
+    ok(present.atomTablesMain,              "atom-tables/main is present");
+    ok(present.sandboxLocation,             "sandbox locations are present");
+    ok(present.bigString,                   "large string is present");
+    ok(present.smallString1,                "small string 1 is present");
+    ok(present.smallString2,                "small string 2 is present");
+
+    ok(!present.anonymizedWhenUnnecessary,
+       "anonymized paths are not present when unnecessary. Failed case: " +
+       present.anonymizedWhenUnnecessary);
+    ok(!present.httpWhenAnonymized,
+       "http URLs are anonymized when necessary. Failed case: " +
+       present.httpWhenAnonymized);
+    ok(!present.unanonymizedFilePathWhenAnonymized,
+       "file URLs are anonymized when necessary. Failed case: " +
+       present.unanonymizedFilePathWhenAnonymized);
+
+    runNext();
   }
-  // vsize may be unreasonable if ASAN is enabled
-  checkSpecialReport("vsize",          vsizeAmounts, /*canBeUnreasonable*/true);
-  checkSpecialReport("resident",       residentAmounts);
-
-  for (var reporter in jsGcHeapUsedGcThings) {
-    ok(jsGcHeapUsedGcThings[reporter] == 1);
-  }
-  checkSizeReasonable("js-main-runtime-gc-heap-committed/used/gc-things",
-                      jsGcHeapUsedGcThingsTotal);
-
-  ok(present.jsNonWindowCompartments,     "js-non-window compartments are present");
-  ok(present.windowObjectsJsCompartments, "window-objects/.../js compartments are present");
-  ok(present.places,                      "places is present");
-  ok(present.images,                      "images is present");
-  ok(present.xptiWorkingSet,              "xpti-working-set is present");
-  ok(present.atomTablesMain,              "atom-tables/main is present");
-  ok(present.sandboxLocation,             "sandbox locations are present");
-  ok(present.bigString,                   "large string is present");
-  ok(present.smallString1,                "small string 1 is present");
-  ok(present.smallString2,                "small string 2 is present");
-
-  ok(!present.anonymizedWhenUnnecessary,
-     "anonymized paths are not present when unnecessary. Failed case: " +
-     present.anonymizedWhenUnnecessary);
-  ok(!present.httpWhenAnonymized,
-     "http URLs are anonymized when necessary. Failed case: " +
-     present.httpWhenAnonymized);
-  ok(!present.unanonymizedFilePathWhenAnonymized,
-     "file URLs are anonymized when necessary. Failed case: " +
-     present.unanonymizedFilePathWhenAnonymized);
 
   // Reporter registration tests
 
@@ -342,27 +371,28 @@
     mgr.registerStrongReporter(reporterAndCallback);
 
     // Check the generated reports.
-    mgr.getReportsForThisProcess(reporterAndCallback, null,
-                                 /* anonymize = */ false);
-    reporterAndCallback.finish();
+    mgr.getReports(reporterAndCallback, null,
+      () => {
+        reporterAndCallback.finish();
+        window.setTimeout(test_unregister_strong, 0, reporterAndCallback);
+      }, null,
+      /* anonymize = */ false);
+  }
 
-    // Unregistration works.
-    mgr.unregisterStrongReporter(reporterAndCallback);
+  function test_unregister_strong(aReporterAndCallback)
+  {
+    mgr.unregisterStrongReporter(aReporterAndCallback);
 
     // The reporter was unregistered, hence there shouldn't be any reports from
     // the test reporter.
-    mgr.getReportsForThisProcess(reporterAndCallback, null,
-                                 /* anonymize = */ false);
-    reporterAndCallback.finish(0);
+    mgr.getReports(aReporterAndCallback, null,
+      () => {
+        aReporterAndCallback.finish(0);
+        runNext();
+      }, null,
+      /* anonymize = */ false);
   }
 
-  test_register_strong();
-
-  // Check strong reporters a second time, to make sure a reporter can be
-  // re-registered.
-  test_register_strong();
-
-
   // Check that you cannot register JS components as weak reporters.
   function test_register_weak() {
     let reporterAndCallback = new MemoryReporterAndCallback();
@@ -382,9 +412,12 @@
       ok(ex.message.indexOf("NS_ERROR_") >= 0,
          "WrappedJS reporter got rejected: " + ex);
     }
+
+    runNext();
   }
 
-  test_register_weak();
+  // Kick-off the async tests.
+  runNext();
 
   ]]>
   </script>

toolkit/components/aboutmemory/tests/test_sqliteMultiReporter.xul

@@ -24,6 +24,8 @@
   const Ci = Components.interfaces;
   const Cu = Components.utils;
 
+  SimpleTest.waitForExplicitFinish();
+
   // Make a fake DB file.
   let file = Cc["@mozilla.org/file/directory_service;1"].
              getService(Ci.nsIProperties).
@@ -40,11 +42,12 @@
   // them.  It shouldn't crash.
   let mgr = Cc["@mozilla.org/memory-reporter-manager;1"].
             getService(Ci.nsIMemoryReporterManager);
-  mgr.getReportsForThisProcess(function(){}, null, /* anonymize = */ false);
-
-  // If we haven't crashed, we've passed, but the test harness requires that
-  // we explicitly check something.
-  ok(true, "didn't crash");
+  mgr.getReports(function(){}, null,
+    () => {
+      ok(true, "didn't crash");
+      SimpleTest.finish();
+    }, null,
+    /* anonymize = */ false);
 
   ]]>
   </script>

xpcom/base/nsIMemoryReporter.idl

@@ -205,7 +205,7 @@ interface nsIFinishReportingCallback : nsISupports
   void callback(in nsISupports data);
 };
 
-[scriptable, builtinclass, uuid(461c477a-54f9-4fe0-b00e-1857f96690f9)]
+[scriptable, builtinclass, uuid(6cc4aa56-e18c-4b61-9290-6297172d6978)]
 interface nsIMemoryReporterManager : nsISupports
 {
   /*
@@ -289,14 +289,6 @@ interface nsIMemoryReporterManager : nsISupports
                        in boolean minimizeMemoryUsage,
                        in AString DMDDumpIdent);
 
-  /*
-   * Get memory reports in the current process only.  |handleReport| is called
-   * for each report.
-   */
-  void getReportsForThisProcess(in nsIMemoryReporterCallback handleReport,
-                                in nsISupports handleReportData,
-                                in boolean anonymize);
-
   /*
    * As above, but if DMD is enabled and |DMDFile| is non-null then
    * write a DMD report to that file and close it.
@@ -315,9 +307,8 @@ interface nsIMemoryReporterManager : nsISupports
    * interesting that we want external code (e.g. telemetry) to be able to rely
    * on them.
    *
-   * Note that these are not reporters and so getReports() and
-   * getReportsForThisProcess() do not look at them.  However, distinguished
-   * amounts can be embedded in a reporter.
+   * Note that these are not reporters and so getReports() does not look at
+   * them.  However, distinguished amounts can be embedded in a reporter.
    *
    * Access to these attributes can fail.  In particular, some of them are not
    * available on all platforms.

xpcom/base/nsMemoryReporterManager.cpp

@@ -1505,15 +1505,6 @@ nsMemoryReporterManager::StartGettingReports()
   return NS_OK;
 }
 
-NS_IMETHODIMP
-nsMemoryReporterManager::GetReportsForThisProcess(
-  nsIHandleReportCallback* aHandleReport,
-  nsISupports* aHandleReportData, bool aAnonymize)
-{
-  return GetReportsForThisProcessExtended(aHandleReport, aHandleReportData,
-                                          aAnonymize, nullptr);
-}
-
 NS_IMETHODIMP
 nsMemoryReporterManager::GetReportsForThisProcessExtended(
   nsIHandleReportCallback* aHandleReport, nsISupports* aHandleReportData,