Tidy heap snapshotting implementation.

Adjust the division of labor between BaseHeap::takeSnapshot() and
the heap page implementations of takeSnapshot() --

 - have BaseHeap::takeSnapshot() handle creation of page dump objects
   (and their naming scheme.)
 - tally the heap free count+size in a separate object; only added to
   by "normal" heap pages.

R=
BUG=

Review URL: https://codereview.chromium.org/1700723002

Cr-Commit-Position: refs/heads/master@{#375455}

2 files changed, 24 insertions, 26 deletions

diff --git a/third_party/WebKit/Source/platform/heap/HeapPage.cpp b/third_party/WebKit/Source/platform/heap/HeapPage.cpp
index 5fb397c..338b682 100644
--- a/third_party/WebKit/Source/platform/heap/HeapPage.cpp
+++ b/third_party/WebKit/Source/platform/heap/HeapPage.cpp
@@ -137,25 +137,22 @@ void BaseHeap::takeSnapshot(const String& dumpBaseName, ThreadState::GCSnapshotI
 {
     // |dumpBaseName| at this point is "blink_gc/thread_X/heaps/HeapName"
     WebMemoryAllocatorDump* allocatorDump = BlinkGCMemoryDumpProvider::instance()->createMemoryAllocatorDumpForCurrentGC(dumpBaseName);
-    size_t pageIndex = 0;
-    size_t heapTotalFreeSize = 0;
-    size_t heapTotalFreeCount = 0;
+    size_t pageCount = 0;
+    BasePage::HeapSnapshotInfo heapInfo;
     for (BasePage* page = m_firstUnsweptPage; page; page = page->next()) {
-        size_t heapPageFreeSize = 0;
-        size_t heapPageFreeCount = 0;
-        page->takeSnapshot(dumpBaseName, pageIndex, info, &heapPageFreeSize, &heapPageFreeCount);
-        heapTotalFreeSize += heapPageFreeSize;
-        heapTotalFreeCount += heapPageFreeCount;
-        pageIndex++;
+        String dumpName = dumpBaseName + String::format("/pages/page_%lu", static_cast<unsigned long>(pageCount++));
+        WebMemoryAllocatorDump* pageDump = BlinkGCMemoryDumpProvider::instance()->createMemoryAllocatorDumpForCurrentGC(dumpName);
+
+        page->takeSnapshot(pageDump, info, heapInfo);
     }
-    allocatorDump->addScalar("blink_page_count", "objects", pageIndex);
+    allocatorDump->addScalar("blink_page_count", "objects", pageCount);
 
     // When taking a full dump (w/ freelist), both the /buckets and /pages
     // report their free size but they are not meant to be added together.
     // Therefore, here we override the free_size of the parent heap to be
     // equal to the free_size of the sum of its heap pages.
-    allocatorDump->addScalar("free_size", "bytes", heapTotalFreeSize);
-    allocatorDump->addScalar("free_count", "objects", heapTotalFreeCount);
+    allocatorDump->addScalar("free_size", "bytes", heapInfo.freeSize);
+    allocatorDump->addScalar("free_count", "objects", heapInfo.freeCount);
 }
 
 #if ENABLE(ASSERT)
@@ -1392,11 +1389,8 @@ void NormalPage::markOrphaned()
     BasePage::markOrphaned();
 }
 
-void NormalPage::takeSnapshot(String dumpName, size_t pageIndex, ThreadState::GCSnapshotInfo& info, size_t* outFreeSize, size_t* outFreeCount)
+void NormalPage::takeSnapshot(WebMemoryAllocatorDump* pageDump, ThreadState::GCSnapshotInfo& info, HeapSnapshotInfo& heapInfo)
 {
-    dumpName.append(String::format("/pages/page_%lu", static_cast<unsigned long>(pageIndex)));
-    WebMemoryAllocatorDump* pageDump = BlinkGCMemoryDumpProvider::instance()->createMemoryAllocatorDumpForCurrentGC(dumpName);
-
     HeapObjectHeader* header = nullptr;
     size_t liveCount = 0;
     size_t deadCount = 0;
@@ -1432,8 +1426,8 @@ void NormalPage::takeSnapshot(String dumpName, size_t pageIndex, ThreadState::GC
     pageDump->addScalar("live_size", "bytes", liveSize);
     pageDump->addScalar("dead_size", "bytes", deadSize);
     pageDump->addScalar("free_size", "bytes", freeSize);
-    *outFreeSize = freeSize;
-    *outFreeCount = freeCount;
+    heapInfo.freeSize += freeSize;
+    heapInfo.freeCount += freeCount;
 }
 
 #if ENABLE(ASSERT)
@@ -1528,11 +1522,8 @@ void LargeObjectPage::markOrphaned()
     BasePage::markOrphaned();
 }
 
-void LargeObjectPage::takeSnapshot(String dumpName, size_t pageIndex, ThreadState::GCSnapshotInfo& info, size_t* outFreeSize, size_t* outFreeCount)
+void LargeObjectPage::takeSnapshot(WebMemoryAllocatorDump* pageDump, ThreadState::GCSnapshotInfo& info, HeapSnapshotInfo&)
 {
-    dumpName.append(String::format("/pages/page_%lu", static_cast<unsigned long>(pageIndex)));
-    WebMemoryAllocatorDump* pageDump = BlinkGCMemoryDumpProvider::instance()->createMemoryAllocatorDumpForCurrentGC(dumpName);
-
     size_t liveSize = 0;
     size_t deadSize = 0;
     size_t liveCount = 0;
diff --git a/third_party/WebKit/Source/platform/heap/HeapPage.h b/third_party/WebKit/Source/platform/heap/HeapPage.h
index bea2e80..683c3da 100644
--- a/third_party/WebKit/Source/platform/heap/HeapPage.h
+++ b/third_party/WebKit/Source/platform/heap/HeapPage.h
@@ -129,7 +129,7 @@ class NormalPageHeap;
 class OrphanedPagePool;
 class PageMemory;
 class PageMemoryRegion;
-class WebProcessMemoryDump;
+class WebMemoryAllocatorDump;
 
 // HeapObjectHeader is 4 byte (32 bit) that has the following layout:
 //
@@ -392,7 +392,14 @@ public:
     virtual void checkAndMarkPointer(Visitor*, Address) = 0;
     virtual void markOrphaned();
 
-    virtual void takeSnapshot(String dumpBaseName, size_t pageIndex, ThreadState::GCSnapshotInfo&, size_t* outFreeSize, size_t* outFreeCount) = 0;
+    class HeapSnapshotInfo {
+        STACK_ALLOCATED();
+    public:
+        size_t freeCount = 0;
+        size_t freeSize = 0;
+    };
+
+    virtual void takeSnapshot(WebMemoryAllocatorDump*, ThreadState::GCSnapshotInfo&, HeapSnapshotInfo&) = 0;
 #if ENABLE(ASSERT)
     virtual bool contains(Address) = 0;
 #endif
@@ -468,7 +475,7 @@ public:
     void checkAndMarkPointer(Visitor*, Address) override;
     void markOrphaned() override;
 
-    void takeSnapshot(String dumpBaseName, size_t pageIndex, ThreadState::GCSnapshotInfo&, size_t* outFreeSize, size_t* outFreeCount) override;
+    void takeSnapshot(WebMemoryAllocatorDump*, ThreadState::GCSnapshotInfo&, HeapSnapshotInfo&) override;
 #if ENABLE(ASSERT)
     // Returns true for the whole blinkPageSize page that the page is on, even
     // for the header, and the unmapped guard page at the start. That ensures
@@ -525,7 +532,7 @@ public:
     void checkAndMarkPointer(Visitor*, Address) override;
     void markOrphaned() override;
 
-    void takeSnapshot(String dumpBaseName, size_t pageIndex, ThreadState::GCSnapshotInfo&, size_t* outFreeSize, size_t* outFreeCount) override;
+    void takeSnapshot(WebMemoryAllocatorDump*, ThreadState::GCSnapshotInfo&, HeapSnapshotInfo&) override;
 #if ENABLE(ASSERT)
     // Returns true for any address that is on one of the pages that this
     // large object uses. That ensures that we can use a negative result to