Bug 1194555 - Part 3: Remove |getReportsForThisProcess| from the nsIMemoryReporterManager interface. r=njn
authorEric Rahm <erahm@mozilla.com>
Wed, 14 Oct 2015 16:52:55 -0700
changeset 301183 c08f47894887fc3d6777842d041f4cc176f234a6
parent 301182 61a127fdd89b7cff2bbeca51b38b1c6b1063c9c8
child 301184 c3a95dcbc56b273686e9d613cbf907afb600b96d
push id5392
push userraliiev@mozilla.com
push dateMon, 14 Dec 2015 20:08:23 +0000
treeherdermozilla-beta@16ce8562a975 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnjn
bugs1194555
milestone44.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
toolkit/components/aboutmemory/tests/test_memoryReporters.xul
toolkit/components/aboutmemory/tests/test_sqliteMultiReporter.xul
xpcom/base/nsIMemoryReporter.idl
xpcom/base/nsMemoryReporterManager.cpp
--- a/image/test/mochitest/test_bug601470.html
+++ b/image/test/mochitest/test_bug601470.html
@@ -25,20 +25,21 @@ window.onload = function() {
   var mgr = SpecialPowers.Cc["@mozilla.org/memory-reporter-manager;1"]
     .getService(SpecialPowers.Ci.nsIMemoryReporterManager);
 
   var amount = 0;
   var handleReport = function(aProcess, aPath, aKind, aUnits, aAmount, aDesc) {
     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>
 </pre>
 </body>
 </html>
--- a/toolkit/components/aboutmemory/tests/test_memoryReporters.xul
+++ b/toolkit/components/aboutmemory/tests/test_memoryReporters.xul
@@ -39,16 +39,18 @@
   const COUNT_CUMULATIVE = Ci.nsIMemoryReporter.UNITS_COUNT_CUMULATIVE;
   const PERCENTAGE = Ci.nsIMemoryReporter.UNITS_PERCENTAGE;
 
   // Use backslashes instead of forward slashes due to memory reporting's hacky
   // handling of URLs.
   const XUL_NS =
     "http:\\\\www.mozilla.org\\keymaster\\gatekeeper\\there.is.only.xul";
 
+  SimpleTest.waitForExplicitFinish();
+
   let vsizeAmounts = [];
   let residentAmounts = [];
   let heapAllocatedAmounts = [];
   let storageSqliteAmounts = [];
 
   let jsGcHeapUsedGcThingsTotal = 0;
   let jsGcHeapUsedGcThings = {};
 
@@ -184,21 +186,43 @@
   let otherSize = {};
   let totalSize = {};
   let jsMilliseconds = {};
   let nonJSMilliseconds = {};
   mgr.sizeOfTab(window, jsObjectsSize, jsStringsSize, jsOtherSize,
                 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
+  ];
+
+  function runNext() {
+    setTimeout(asyncSteps.shift(), 0);
+  }
 
-  mgr.getReportsForThisProcess(handleReportAnonymized, null,
-                               /* anonymize = */ true);
+  function getReportsNormal()
+  {
+    mgr.getReports(handleReportNormal, null,
+                   runNext, null,
+                   /* anonymize = */ false);
+  }
+
+  function getReportsAnonymized()
+  {
+    mgr.getReports(handleReportAnonymized, null,
+                   runNext, null,
+                   /* anonymize = */ true);
+  }
 
   function checkSizeReasonable(aName, aAmount)
   {
     // Check the size is reasonable -- i.e. not ridiculously large or small.
     ok(100 * 1000 <= aAmount && aAmount <= 10 * 1000 * 1000 * 1000,
        aName + "'s size is reasonable");
   }
 
@@ -206,55 +230,60 @@
   {
     ok(aAmounts.length == 1, aName + " has " + aAmounts.length + " report");
     let n = aAmounts[0];
     if (!aCanBeUnreasonable) {
       checkSizeReasonable(aName, n);
     }
   }
 
-  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);
+  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);
+    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.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);
+    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();
+  }
 
   // Reporter registration tests
 
   // collectReports() calls to the test reporter.
   let called = 0;
 
   // The test memory reporter, testing the various report units.
   // Also acts as a report collector, verifying the reported values match the
@@ -337,37 +366,38 @@
 
   // General memory reporter + registerStrongReporter tests.
   function test_register_strong() {
     let reporterAndCallback = new MemoryReporterAndCallback();
     // Registration works.
     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();
     try {
       // Should fail! nsMemoryReporterManager will only hold a raw pointer to
       // "weak" reporters.  When registering a weak reporter, XPConnect will
       // create a WrappedJS for JS components.  This WrappedJS would be
       // successfully registered with the manager, only to be destroyed
@@ -377,15 +407,18 @@
       // See bug 950391 comment #0.
       mgr.registerWeakReporter(reporterAndCallback);
       ok(false, "Shouldn't be allowed to register a JS component (WrappedJS)");
     }
     catch (ex) {
       ok(ex.message.indexOf("NS_ERROR_") >= 0,
          "WrappedJS reporter got rejected: " + ex);
     }
+
+    runNext();
   }
 
-  test_register_weak();
+  // Kick-off the async tests.
+  runNext();
 
   ]]>
   </script>
 </window>
--- a/toolkit/components/aboutmemory/tests/test_sqliteMultiReporter.xul
+++ b/toolkit/components/aboutmemory/tests/test_sqliteMultiReporter.xul
@@ -19,33 +19,36 @@
   // Nb: this test is all JS and chould be done with an xpcshell test,
   // but all the other memory reporter tests are mochitests, so it's easier
   // if this one is too.
 
   const Cc = Components.classes;
   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).
              get("ProfD", Ci.nsIFile);
   file.append("test_sqliteMultiReporter-fake-DB-tmp.sqlite");
 
   // Open and close the DB.
   let storage = Cc["@mozilla.org/storage/service;1"].
                 getService(Ci.mozIStorageService);
   let db = storage.openDatabase(file);
   db.close();
 
   // Invoke all the reporters.  The SQLite multi-reporter is among
   // 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>
 </window>
--- a/xpcom/base/nsIMemoryReporter.idl
+++ b/xpcom/base/nsIMemoryReporter.idl
@@ -200,17 +200,17 @@ interface nsIMemoryReporter : nsISupport
 };
 
 [scriptable, function, uuid(548b3909-c04d-4ca6-8466-b8bee3837457)]
 interface nsIFinishReportingCallback : nsISupports
 {
   void callback(in nsISupports data);
 };
 
-[scriptable, builtinclass, uuid(fcad440e-64dd-4fb8-9a5e-b19d1b91423b)]
+[scriptable, builtinclass, uuid(0ea5aa0a-bbe4-48c9-9019-eba2c5614734)]
 interface nsIMemoryReporterManager : nsISupports
 {
   /*
    * Initialize.
    */
   void init();
 
   /*
@@ -285,24 +285,16 @@ interface nsIMemoryReporterManager : nsI
                        in nsISupports handleReportData,
                        in nsIFinishReportingCallback finishReporting,
                        in nsISupports finishReportingData,
                        in boolean anonymize,
                        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.
    */
   [noscript] void
     getReportsForThisProcessExtended(in nsIMemoryReporterCallback handleReport,
                                      in nsISupports handleReportData,
                                      in boolean anonymize,
                                      in FILE DMDFile);
@@ -310,19 +302,18 @@ interface nsIMemoryReporterManager : nsI
   /*
    * The memory reporter manager, for the most part, treats reporters
    * registered with it as a black box.  However, there are some
    * "distinguished" amounts (as could be reported by a memory reporter) that
    * the manager provides as attributes, because they are sufficiently
    * 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.
    *
    * If you add a new distinguished amount, please update
    * toolkit/components/aboutmemory/tests/test_memoryReporters.xul.
    *
    * |vsize| (UNITS_BYTES)  The virtual size, i.e. the amount of address space
--- a/xpcom/base/nsMemoryReporterManager.cpp
+++ b/xpcom/base/nsMemoryReporterManager.cpp
@@ -1501,25 +1501,16 @@ nsMemoryReporterManager::StartGettingRep
 
   // The parent's report is done; make note of that, and start
   // launching child process reports (if any).
   EndProcessReport(s->mGeneration, true);
   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,
   bool aAnonymize, FILE* aDMDFile)
 {
   // Memory reporters are not necessarily threadsafe, so this function must
   // be called from the main thread.
   if (!NS_IsMainThread()) {
     MOZ_CRASH();