Bug 1274443 - Replace mPresArenaAllocCount with a hashtable to make it easy to spot the cause of double-frees. r=heycam

layout/base/nsIPresShell.h

@@ -227,19 +227,15 @@ public:
    */
   void* AllocateFrame(nsQueryFrame::FrameIID aID, size_t aSize)
   {
-#ifdef DEBUG
-    mPresArenaAllocCount++;
-#endif
     void* result = mFrameArena.AllocateByFrameID(aID, aSize);
+    RecordAlloc(result);
     memset(result, 0, aSize);
     return result;
   }
 
   void FreeFrame(nsQueryFrame::FrameIID aID, void* aPtr)
   {
-#ifdef DEBUG
-    mPresArenaAllocCount--;
-#endif
+    RecordFree(aPtr);
     if (!mIsDestroying)
       mFrameArena.FreeByFrameID(aID, aPtr);
   }
@@ -252,19 +248,15 @@ public:
    */
   void* AllocateByObjectID(mozilla::ArenaObjectID aID, size_t aSize)
   {
-#ifdef DEBUG
-    mPresArenaAllocCount++;
-#endif
     void* result = mFrameArena.AllocateByObjectID(aID, aSize);
+    RecordAlloc(result);
     memset(result, 0, aSize);
     return result;
   }
 
   void FreeByObjectID(mozilla::ArenaObjectID aID, void* aPtr)
   {
-#ifdef DEBUG
-    mPresArenaAllocCount--;
-#endif
+    RecordFree(aPtr);
     if (!mIsDestroying)
       mFrameArena.FreeByObjectID(aID, aPtr);
   }
@@ -280,17 +272,14 @@ public:
    */
   void* AllocateMisc(size_t aSize)
   {
-#ifdef DEBUG
-    mPresArenaAllocCount++;
-#endif
-    return mFrameArena.AllocateBySize(aSize);
+    void* result = mFrameArena.AllocateBySize(aSize);
+    RecordAlloc(result);
+    return result;
   }
 
   void FreeMisc(size_t aSize, void* aPtr)
   {
-#ifdef DEBUG
-    mPresArenaAllocCount--;
-#endif
+    RecordFree(aPtr);
     if (!mIsDestroying)
       mFrameArena.FreeBySize(aSize, aPtr);
   }
@@ -1641,6 +1630,20 @@ protected:
    */
   void RecomputeFontSizeInflationEnabled();
 
+  void RecordAlloc(void* aPtr) {
+#ifdef DEBUG
+    MOZ_ASSERT(!mAllocatedPointers.Contains(aPtr));
+    mAllocatedPointers.PutEntry(aPtr);
+#endif
+  }
+
+  void RecordFree(void* aPtr) {
+#ifdef DEBUG
+    MOZ_ASSERT(mAllocatedPointers.Contains(aPtr));
+    mAllocatedPointers.RemoveEntry(aPtr);
+#endif
+  }
+
 public:
   bool AddRefreshObserver(nsARefreshObserver* aObserver,
                           mozFlushType aFlushType) {
@@ -1748,10 +1751,13 @@ protected:
 
 #ifdef DEBUG
   nsIFrame*                 mDrawEventTargetFrame;
-  // Ensure that every allocation from the PresArena is eventually freed.
-  uint32_t                  mPresArenaAllocCount;
+
+  // We track allocated pointers in a debug-only hashtable to assert against
+  // missing/double frees.
+  nsTHashtable<nsPtrHashKey<void>> mAllocatedPointers;
 #endif
 
+
   // Count of the number of times this presshell has been painted to a window.
   uint64_t                  mPaintCount;
 

layout/base/nsPresShell.cpp

@@ -789,9 +789,6 @@ PresShell::PresShell()
 #endif
   mPresShellId = sNextPresShellId++;
   mFrozen = false;
-#ifdef DEBUG
-  mPresArenaAllocCount = 0;
-#endif
   mRenderFlags = 0;
 
   mScrollPositionClampingScrollPortSizeSet = false;
@@ -840,10 +837,7 @@ PresShell::~PresShell()
     mPresContext->RefreshDriver()->Thaw();
   }
 
-#ifdef DEBUG
-  MOZ_ASSERT(mPresArenaAllocCount == 0,
-             "Some pres arena objects were not freed");
-#endif
+  MOZ_ASSERT(mAllocatedPointers.IsEmpty(), "Some pres arena objects were not freed");
 
   mStyleSet->Delete();
   delete mFrameConstructor;
@@ -1245,7 +1239,7 @@ PresShell::Destroy()
   // pointing to objects in the arena now.  This is done:
   //
   //   (a) before mFrameArena's destructor runs so that our
-  //       mPresArenaAllocCount gets down to 0 and doesn't trip the assertion
+  //       mAllocatedPointers becomes empty and doesn't trip the assertion
   //       in ~PresShell,
   //   (b) before the mPresContext->SetShell(nullptr) below, so
   //       that when we clear the ArenaRefPtrs they'll still be able to