[discardable] Don't double-report virtual size and size.

Virtual size and size are reported by each segment in detailed memory
dumps, and also by the heap implementation. This is not an issue in
reported metrics, but can lead to double-counting in about:tracing, as
the per-segment sized are summed with the overall ones in the UI.

Bug: 1106364
Change-Id: I7099909b50d7902c3b7f0117934d3ca6dba75cfb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2372327Reviewed-by: Peng Huang <penghuang@chromium.org>
Commit-Queue: Benoit L <lizeb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#801343}

components/discardable_memory/common/discardable_shared_memory_heap.cc

@@ -259,28 +259,32 @@ bool DiscardableSharedMemoryHeap::OnMemoryDump(
   auto* total_dump = pmd->CreateAllocatorDump(base::StringPrintf(
       "discardable/child_0x%" PRIXPTR, reinterpret_cast<uintptr_t>(this)));
   const size_t freelist_size = GetSizeOfFreeLists();
-  const size_t total_size = GetSize();
   total_dump->AddScalar("freelist_size",
                         base::trace_event::MemoryAllocatorDump::kUnitsBytes,
                         freelist_size);
-  total_dump->AddScalar("virtual_size",
-                        base::trace_event::MemoryAllocatorDump::kUnitsBytes,
-                        total_size);
   if (args.level_of_detail ==
       base::trace_event::MemoryDumpLevelOfDetail::BACKGROUND) {
+    // These metrics (size and virtual size) are also reported by each
+    // individual segment. If we report both, then the counts are artificially
+    // inflated in detailed dumps, depending on aggregation (for instance, in
+    // about:tracing's UI).
+    const size_t total_size = GetSize();
     total_dump->AddScalar(base::trace_event::MemoryAllocatorDump::kNameSize,
                           base::trace_event::MemoryAllocatorDump::kUnitsBytes,
                           total_size - freelist_size);
-    return true;
+    total_dump->AddScalar("virtual_size",
+                          base::trace_event::MemoryAllocatorDump::kUnitsBytes,
+                          total_size);
+  } else {
+    // This iterates over all the memory allocated by the heap, and calls
+    // |OnMemoryDump| for each. It does not contain any information about the
+    // DiscardableSharedMemoryHeap itself.
+    std::for_each(memory_segments_.begin(), memory_segments_.end(),
+                  [pmd](const std::unique_ptr<ScopedMemorySegment>& segment) {
+                    segment->OnMemoryDump(pmd);
+                  });
   }
 
-  // This iterates over all the memory allocated by the heap, and calls
-  // |OnMemoryDump| for each. It does not contain any information about the
-  // DiscardableSharedMemoryHeap itself.
-  std::for_each(memory_segments_.begin(), memory_segments_.end(),
-                [pmd](const std::unique_ptr<ScopedMemorySegment>& segment) {
-                  segment->OnMemoryDump(pmd);
-                });
   return true;
 }
 

components/discardable_memory/common/discardable_shared_memory_heap_unittest.cc

@@ -9,6 +9,7 @@
 #include <utility>
 
 #include "base/bind.h"
+#include "base/bind_helpers.h"
 #include "base/memory/discardable_shared_memory.h"
 #include "base/process/process_metrics.h"
 #include "base/strings/stringprintf.h"
@@ -407,5 +408,38 @@ TEST(DiscardableSharedMemoryHeapTest, OnMemoryDumpTest) {
   }
 }
 
+TEST(DiscardableSharedMemoryHeapTest, DetailedDumpsDontContainRedundantData) {
+  using testing::ByRef;
+  using testing::Contains;
+  using testing::Eq;
+  using testing::Not;
+  DiscardableSharedMemoryHeap heap;
+
+  base::trace_event::MemoryDumpArgs args = {
+      base::trace_event::MemoryDumpLevelOfDetail::DETAILED};
+  size_t block_size = base::GetPageSize();
+
+  auto memory = std::make_unique<base::DiscardableSharedMemory>();
+  ASSERT_TRUE(memory->CreateAndMap(block_size));
+  auto span = heap.Grow(std::move(memory), block_size, 1, base::DoNothing());
+
+  base::trace_event::ProcessMemoryDump pmd(args);
+  heap.OnMemoryDump(args, &pmd);
+  auto* dump = pmd.GetAllocatorDump(base::StringPrintf(
+      "discardable/child_0x%" PRIXPTR, reinterpret_cast<uintptr_t>(&heap)));
+  ASSERT_NE(nullptr, dump);
+
+  base::trace_event::MemoryAllocatorDump::Entry freelist("freelist_size",
+                                                         "bytes", 0);
+  EXPECT_THAT(dump->entries(), Contains(Eq(ByRef(freelist))));
+
+  // Detailed dumps do not contain virtual size.
+  base::trace_event::MemoryAllocatorDump::Entry virtual_size(
+      "virtual_size", "bytes", block_size);
+  EXPECT_THAT(dump->entries(), Not(Contains(Eq(ByRef(virtual_size)))));
+
+  heap.MergeIntoFreeLists(std::move(span));
+}
+
 }  // namespace
 }  // namespace discardable_memory