Bug 848560 (part 2) - Prevent memory reporters from being registered while about:memory's tests are running. r=jlebar.
authorNicholas Nethercote <nnethercote@mozilla.com>
Mon, 01 Apr 2013 21:39:44 -0700
changeset 128128 560ce5e96e6b319397dd66bf34ca2500cc2f1818
parent 128127 dcf9107c178585181f93e987bd308fd1569ed321
child 128129 0e47bb858304b373330af0be986dd44717faaf1c
push id24522
push userryanvm@gmail.com
push dateTue, 09 Apr 2013 23:24:02 +0000
treeherdermozilla-central@9db46ddfb517 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjlebar
bugs848560
milestone23.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 848560 (part 2) - Prevent memory reporters from being registered while about:memory's tests are running. r=jlebar.
toolkit/components/aboutmemory/tests/test_aboutcompartments.xul
toolkit/components/aboutmemory/tests/test_aboutmemory.xul
toolkit/components/aboutmemory/tests/test_aboutmemory2.xul
toolkit/components/aboutmemory/tests/test_aboutmemory3.xul
xpcom/base/nsIMemoryReporter.idl
xpcom/base/nsMemoryReporterManager.cpp
xpcom/base/nsMemoryReporterManager.h
--- a/toolkit/components/aboutmemory/tests/test_aboutcompartments.xul
+++ b/toolkit/components/aboutmemory/tests/test_aboutcompartments.xul
@@ -21,16 +21,17 @@
 
   const Cc = Components.classes;
   const Ci = Components.interfaces;
   let mgr = Cc["@mozilla.org/memory-reporter-manager;1"].
             getService(Ci.nsIMemoryReporterManager);
 
   // Remove all the real reporters and multi-reporters;  save them to
   // restore at the end.
+  mgr.blockRegistration();
   var e = mgr.enumerateReporters();
   var realReporters = [];
   while (e.hasMoreElements()) {
     var r = e.getNext().QueryInterface(Ci.nsIMemoryReporter);
     mgr.unregisterReporter(r);
     realReporters.push(r);
   }
   e = mgr.enumerateMultiReporters();
@@ -126,20 +127,20 @@
         f("smaps/vsize/a",     24);
         f("smaps/swap/a",       1);
       },
       explicitNonHeap: 0
     }
   ];
 
   for (var i = 0; i < fakeReporters.length; i++) {
-    mgr.registerReporter(fakeReporters[i]);
+    mgr.registerReporterEvenIfBlocked(fakeReporters[i]);
   }
   for (var i = 0; i < fakeMultiReporters.length; i++) {
-    mgr.registerMultiReporter(fakeMultiReporters[i]);
+    mgr.registerMultiReporterEvenIfBlocked(fakeMultiReporters[i]);
   }
   ]]>
   </script>
 
   <iframe id="acFrame"  height="400" src="about:compartments"></iframe>
   <!-- vary the capitalization to make sure that works -->
   <iframe id="acvFrame" height="400" src="abouT:compartMENTS?veRBOse"></iframe>
 
@@ -188,21 +189,23 @@ Ghost Windows\n\
     // them.
     for (var i = 0; i < fakeReporters.length; i++) {
       mgr.unregisterReporter(fakeReporters[i]);
     }
     for (var i = 0; i < fakeMultiReporters.length; i++) {
       mgr.unregisterMultiReporter(fakeMultiReporters[i]);
     }
     for (var i = 0; i < realReporters.length; i++) {
-      mgr.registerReporter(realReporters[i]);
+      mgr.registerReporterEvenIfBlocked(realReporters[i]);
     }
     for (var i = 0; i < realMultiReporters.length; i++) {
-      mgr.registerMultiReporter(realMultiReporters[i]);
+      mgr.registerMultiReporterEvenIfBlocked(realMultiReporters[i]);
     }
+    mgr.unblockRegistration();
+
     SimpleTest.finish();
   }
 
   // Cut+paste the entire page and check that the cut text matches what we
   // expect.  This tests the output in general and also that the cutting and
   // pasting works as expected.
   function test(aFrameId, aExpected, aNext) {
     ok(document.title === "about:compartments", "document.title is correct");
--- a/toolkit/components/aboutmemory/tests/test_aboutmemory.xul
+++ b/toolkit/components/aboutmemory/tests/test_aboutmemory.xul
@@ -24,16 +24,17 @@
   const Cc = Components.classes;
   const Ci = Components.interfaces;
   const Cr = Components.results;
   let mgr = Cc["@mozilla.org/memory-reporter-manager;1"].
             getService(Ci.nsIMemoryReporterManager);
 
   // Remove all the real reporters and multi-reporters;  save them to
   // restore at the end.
+  mgr.blockRegistration();
   let e = mgr.enumerateReporters();
   let realReporters = [];
   while (e.hasMoreElements()) {
     let r = e.getNext().QueryInterface(Ci.nsIMemoryReporter);
     mgr.unregisterReporter(r);
     realReporters.push(r);
   }
   e = mgr.enumerateMultiReporters();
@@ -157,20 +158,20 @@
         }
         f("compartments/user/bar");
         f("compartments/system/bar");
       },
       explicitNonHeap: 0
     }
   ];
   for (let i = 0; i < fakeReporters.length; i++) {
-    mgr.registerReporter(fakeReporters[i]);
+    mgr.registerReporterEvenIfBlocked(fakeReporters[i]);
   }
   for (let i = 0; i < fakeMultiReporters.length; i++) {
-    mgr.registerMultiReporter(fakeMultiReporters[i]);
+    mgr.registerMultiReporterEvenIfBlocked(fakeMultiReporters[i]);
   }
 
   // mgr.explicit sums "heap-allocated" and all the appropriate NONHEAP ones:
   // - "explicit/c", "explicit/cc" x 2, "explicit/d", "explicit/e"
   // - but *not* "explicit/c/d" x 2
   // Check explicit now before we add the fake reporters for the fake 2nd
   // and subsequent processes.
   //
@@ -238,17 +239,17 @@
     f("5th", "explicit/a/neg1",  NONHEAP, -20 * KB),
     f("5th", "explicit/a/neg2",  NONHEAP, -10 * KB),
     f("5th", "explicit/b/c/d/e", NONHEAP,  20 * KB),
     f("5th", "explicit/b/c/d/f", NONHEAP, -60 * KB),
     f("5th", "explicit/b/c/g/h", NONHEAP,  10 * KB),
     f("5th", "explicit/b/c/i/j", NONHEAP,   5 * KB)
   ];
   for (let i = 0; i < fakeReporters2.length; i++) {
-    mgr.registerReporter(fakeReporters2[i]);
+    mgr.registerReporterEvenIfBlocked(fakeReporters2[i]);
   }
   fakeReporters = fakeReporters.concat(fakeReporters2);
   ]]>
   </script>
 
   <iframe id="amFrame"  height="400" src="about:memory"></iframe>
   <!-- vary the capitalization to make sure that works -->
   <iframe id="amvFrame" height="400" src="About:Memory?verbose"></iframe>
@@ -590,21 +591,23 @@ 104,857,600 B ── heap-allocated\n\
     // them.
     for (let i = 0; i < fakeReporters.length; i++) {
       mgr.unregisterReporter(fakeReporters[i]);
     }
     for (let i = 0; i < fakeMultiReporters.length; i++) {
       mgr.unregisterMultiReporter(fakeMultiReporters[i]);
     }
     for (let i = 0; i < realReporters.length; i++) {
-      mgr.registerReporter(realReporters[i]);
+      mgr.registerReporterEvenIfBlocked(realReporters[i]);
     }
     for (let i = 0; i < realMultiReporters.length; i++) {
-      mgr.registerMultiReporter(realMultiReporters[i]);
+      mgr.registerMultiReporterEvenIfBlocked(realMultiReporters[i]);
     }
+    mgr.unblockRegistration();
+
     SimpleTest.finish();
   }
 
   // Cut+paste the entire page and check that the cut text matches what we
   // expect.  This tests the output in general and also that the cutting and
   // pasting works as expected.
   function test(aFrameId, aExpected, aNext) {
     SimpleTest.executeSoon(function() {
--- a/toolkit/components/aboutmemory/tests/test_aboutmemory2.xul
+++ b/toolkit/components/aboutmemory/tests/test_aboutmemory2.xul
@@ -19,16 +19,17 @@
 
   const Cc = Components.classes;
   const Ci = Components.interfaces;
   let mgr = Cc["@mozilla.org/memory-reporter-manager;1"].
             getService(Ci.nsIMemoryReporterManager);
 
   // Remove all the real reporters and multi-reporters;  save them to
   // restore at the end.
+  mgr.blockRegistration();
   let e = mgr.enumerateReporters();
   let realReporters = [];
   while (e.hasMoreElements()) {
     let r = e.getNext().QueryInterface(Ci.nsIMemoryReporter);
     mgr.unregisterReporter(r);
     realReporters.push(r);
   }
   e = mgr.enumerateMultiReporters();
@@ -73,17 +74,17 @@
     hi2Report,
     jkReport,
     jk2Report,
     f("explicit/a/l/m",     HEAP,    0.1 * MB),
     f("explicit/a/l/n",     HEAP,    0.1 * MB),
   ];
 
   for (let i = 0; i < fakeReporters.length; i++) {
-    mgr.registerReporter(fakeReporters[i]);
+    mgr.registerReporterEvenIfBlocked(fakeReporters[i]);
   }
 
   ]]>
   </script>
 
   <iframe id="amFrame"  height="500" src="about:memory"></iframe>
 
   <script type="application/javascript">
@@ -92,21 +93,23 @@
   {
     // Unregister fake reporters and multi-reporters, re-register the real
     // reporters and multi-reporters, just in case subsequent tests rely on
     // them.
     for (let i = 0; i < fakeReporters.length; i++) {
       mgr.unregisterReporter(fakeReporters[i]);
     }
     for (let i = 0; i < realReporters.length; i++) {
-      mgr.registerReporter(realReporters[i]);
+      mgr.registerReporterEvenIfBlocked(realReporters[i]);
     }
     for (let i = 0; i < realMultiReporters.length; i++) {
-      mgr.registerMultiReporter(realMultiReporters[i]);
+      mgr.registerMultiReporterEvenIfBlocked(realMultiReporters[i]);
     }
+    mgr.unblockRegistration();
+
     SimpleTest.finish();
   }
 
   // Click on the identified element, then cut+paste the entire page and
   // check that the cut text matches what we expect.
   function test(aId, aExpected, aNext) {
     let win = document.getElementById("amFrame").contentWindow;
     if (aId) {
--- a/toolkit/components/aboutmemory/tests/test_aboutmemory3.xul
+++ b/toolkit/components/aboutmemory/tests/test_aboutmemory3.xul
@@ -19,16 +19,17 @@
 
   const Cc = Components.classes;
   const Ci = Components.interfaces;
   let mgr = Cc["@mozilla.org/memory-reporter-manager;1"].
             getService(Ci.nsIMemoryReporterManager);
 
   // Remove all the real reporters and multi-reporters;  save them to
   // restore at the end.
+  mgr.blockRegistration();
   let e = mgr.enumerateReporters();
   let realReporters = [];
   while (e.hasMoreElements()) {
     let r = e.getNext().QueryInterface(Ci.nsIMemoryReporter);
     mgr.unregisterReporter(r);
     realReporters.push(r);
   }
   e = mgr.enumerateMultiReporters();
@@ -59,17 +60,17 @@
 
   let fakeReporters = [
     f("heap-allocated",     OTHER,   250 * MB),
     f("explicit/a/b",       HEAP,     50 * MB),
     f("other",              OTHER,   0.1 * MB),
   ];
 
   for (let i = 0; i < fakeReporters.length; i++) {
-    mgr.registerReporter(fakeReporters[i]);
+    mgr.registerReporterEvenIfBlocked(fakeReporters[i]);
   }
 
   ]]>
   </script>
 
   <iframe id="amGoodFrame" height="400" src="about:memory"></iframe>
   <iframe id="amBadFrame"  height="400" src="about:memory"></iframe>
 
@@ -79,21 +80,23 @@
   {
     // Unregister fake reporters and multi-reporters, re-register the real
     // reporters and multi-reporters, just in case subsequent tests rely on
     // them.
     for (let i = 0; i < fakeReporters.length; i++) {
       mgr.unregisterReporter(fakeReporters[i]);
     }
     for (let i = 0; i < realReporters.length; i++) {
-      mgr.registerReporter(realReporters[i]);
+      mgr.registerReporterEvenIfBlocked(realReporters[i]);
     }
     for (let i = 0; i < realMultiReporters.length; i++) {
-      mgr.registerMultiReporter(realMultiReporters[i]);
+      mgr.registerMultiReporterEvenIfBlocked(realMultiReporters[i]);
     }
+    mgr.unblockRegistration();
+
     SimpleTest.finish();
   }
 
   // Load the given file into the frame, then copy+paste the entire frame and
   // check that the cut text matches what we expect.
   function test(aFrameId, aFilename, aExpected, aNext) {
     let frame = document.getElementById(aFrameId);
     frame.focus();
--- a/xpcom/base/nsIMemoryReporter.idl
+++ b/xpcom/base/nsIMemoryReporter.idl
@@ -223,17 +223,17 @@ interface nsIMemoryMultiReporter : nsISu
    * nsIMemoryReporterManager::explicit efficiently, which is important --
    * multi-reporters can special-case this operation so it's much faster
    * than getting all the reports, filtering out the unneeded ones, and
    * summing the remainder.
    */
   readonly attribute int64_t explicitNonHeap;
 };
 
-[scriptable, builtinclass, uuid(0baaa958-3112-4952-b557-2a0c57eabb8f)]
+[scriptable, builtinclass, uuid(70b0e608-8dbf-4dc7-b88f-f1c745c1b48c)]
 interface nsIMemoryReporterManager : nsISupports
 {
   /*
    * Return an enumerator of nsIMemoryReporters that are currently registered.
    */
   nsISimpleEnumerator enumerateReporters ();
 
   /*
@@ -262,16 +262,24 @@ interface nsIMemoryReporterManager : nsI
    */
   void unregisterReporter (in nsIMemoryReporter reporter);
 
   /*
    * Unregister the given memory multi-reporter.
    */
   void unregisterMultiReporter (in nsIMemoryMultiReporter reporter);
 
+  /**
+   * These functions should only be used for testing purposes.
+   */
+  void blockRegistration();
+  void unblockRegistration();
+  void registerReporterEvenIfBlocked(in nsIMemoryReporter aReporter);
+  void registerMultiReporterEvenIfBlocked(in nsIMemoryMultiReporter aReporter);
+
   /*
    * Initialize.
    */
   void init ();
 
   /*
    * Get the resident size (aka. RSS, physical memory used).  This reporter
    * is special-cased because it's interesting, is available on all
--- a/xpcom/base/nsMemoryReporterManager.cpp
+++ b/xpcom/base/nsMemoryReporterManager.cpp
@@ -817,17 +817,18 @@ HashtableEnumerator::GetNext(nsISupports
 
     *aNext = nullptr;
     return NS_ERROR_FAILURE;
 }
 
 } // anonymous namespace
 
 nsMemoryReporterManager::nsMemoryReporterManager()
-  : mMutex("nsMemoryReporterManager::mMutex")
+  : mMutex("nsMemoryReporterManager::mMutex"),
+    mIsRegistrationBlocked(false)
 {
     mReporters.Init();
     mMultiReporters.Init();
 }
 
 nsMemoryReporterManager::~nsMemoryReporterManager()
 {
 }
@@ -872,23 +873,24 @@ DebugAssertRefcountIsNonZero(nsISupports
 #ifdef DEBUG
     // This will probably crash if the object's refcount is 0.
     uint32_t refcnt = NS_ADDREF(aObj);
     MOZ_ASSERT(refcnt >= 2);
     NS_RELEASE(aObj);
 #endif
 }
 
-NS_IMETHODIMP
-nsMemoryReporterManager::RegisterReporter(nsIMemoryReporter *reporter)
+nsresult
+nsMemoryReporterManager::RegisterReporterHelper(
+    nsIMemoryReporter *reporter, bool aForce)
 {
     // This method is thread-safe.
     mozilla::MutexAutoLock autoLock(mMutex);
 
-    if (mReporters.Contains(reporter)) {
+    if ((mIsRegistrationBlocked && !aForce) || mReporters.Contains(reporter)) {
         return NS_ERROR_FAILURE;
     }
 
     // This method needs to be safe even if |reporter| has a refcnt of 0, so we
     // take a kung fu death grip before calling PutEntry.  Otherwise, if
     // PutEntry addref'ed and released reporter before finally addref'ing it for
     // good, it would free reporter!
     //
@@ -903,36 +905,64 @@ nsMemoryReporterManager::RegisterReporte
     }
 
     DebugAssertRefcountIsNonZero(reporter);
 
     return NS_OK;
 }
 
 NS_IMETHODIMP
-nsMemoryReporterManager::RegisterMultiReporter(nsIMemoryMultiReporter *reporter)
+nsMemoryReporterManager::RegisterReporter(nsIMemoryReporter *reporter)
+{
+    return RegisterReporterHelper(reporter, /* force = */ false);
+}
+
+NS_IMETHODIMP
+nsMemoryReporterManager::RegisterReporterEvenIfBlocked(
+    nsIMemoryReporter *reporter)
+{
+    return RegisterReporterHelper(reporter, /* force = */ true);
+}
+
+nsresult
+nsMemoryReporterManager::RegisterMultiReporterHelper(
+    nsIMemoryMultiReporter *reporter, bool aForce)
 {
     // This method is thread-safe.
     mozilla::MutexAutoLock autoLock(mMutex);
 
-    if (mMultiReporters.Contains(reporter)) {
+    if ((mIsRegistrationBlocked && !aForce) ||
+         mMultiReporters.Contains(reporter)) {
         return NS_ERROR_FAILURE;
     }
 
     {
         nsCOMPtr<nsIMemoryMultiReporter> kungFuDeathGrip = reporter;
         mMultiReporters.PutEntry(reporter);
     }
 
     DebugAssertRefcountIsNonZero(reporter);
 
     return NS_OK;
 }
 
 NS_IMETHODIMP
+nsMemoryReporterManager::RegisterMultiReporter(nsIMemoryMultiReporter *reporter)
+{
+    return RegisterMultiReporterHelper(reporter, /* force = */ false);
+}
+
+NS_IMETHODIMP
+nsMemoryReporterManager::RegisterMultiReporterEvenIfBlocked(
+    nsIMemoryMultiReporter *reporter)
+{
+    return RegisterMultiReporterHelper(reporter, /* force = */ true);
+}
+
+NS_IMETHODIMP
 nsMemoryReporterManager::UnregisterReporter(nsIMemoryReporter *reporter)
 {
     // This method is thread-safe.
     mozilla::MutexAutoLock autoLock(mMutex);
 
     if (!mReporters.Contains(reporter)) {
         return NS_ERROR_FAILURE;
     }
@@ -951,16 +981,40 @@ nsMemoryReporterManager::UnregisterMulti
         return NS_ERROR_FAILURE;
     }
 
     mMultiReporters.RemoveEntry(reporter);
     return NS_OK;
 }
 
 NS_IMETHODIMP
+nsMemoryReporterManager::BlockRegistration()
+{
+    // This method is thread-safe.
+    mozilla::MutexAutoLock autoLock(mMutex);
+    if (mIsRegistrationBlocked) {
+        return NS_ERROR_FAILURE;
+    }
+    mIsRegistrationBlocked = true;
+    return NS_OK;
+}
+
+NS_IMETHODIMP
+nsMemoryReporterManager::UnblockRegistration()
+{
+    // This method is thread-safe.
+    mozilla::MutexAutoLock autoLock(mMutex);
+    if (!mIsRegistrationBlocked) {
+        return NS_ERROR_FAILURE;
+    }
+    mIsRegistrationBlocked = false;
+    return NS_OK;
+}
+
+NS_IMETHODIMP
 nsMemoryReporterManager::GetResident(int64_t *aResident)
 {
 #ifdef HAVE_VSIZE_AND_RESIDENT_REPORTERS
     return ::GetResident(aResident);
 #else
     *aResident = 0;
     return NS_ERROR_NOT_AVAILABLE;
 #endif
--- a/xpcom/base/nsMemoryReporterManager.h
+++ b/xpcom/base/nsMemoryReporterManager.h
@@ -18,16 +18,21 @@ class nsMemoryReporterManager : public n
 public:
   NS_DECL_ISUPPORTS
   NS_DECL_NSIMEMORYREPORTERMANAGER
 
   nsMemoryReporterManager();
   virtual ~nsMemoryReporterManager();
 
 private:
+  nsresult RegisterReporterHelper(nsIMemoryReporter *reporter, bool aForce);
+  nsresult RegisterMultiReporterHelper(nsIMemoryMultiReporter *reporter,
+                                       bool aForce);
+
   nsTHashtable<nsISupportsHashKey> mReporters;
   nsTHashtable<nsISupportsHashKey> mMultiReporters;
   Mutex mMutex;
+  bool mIsRegistrationBlocked;
 };
 
 #define NS_MEMORY_REPORTER_MANAGER_CID \
 { 0xfb97e4f5, 0x32dd, 0x497a, \
 { 0xba, 0xa2, 0x7d, 0x1e, 0x55, 0x7, 0x99, 0x10 } }