Instrument SdchManager using MemoryDumpProvider

net/base/sdch_manager.cc

Index: net/base/sdch_manager.cc
diff --git a/net/base/sdch_manager.cc b/net/base/sdch_manager.cc
index e57d73cfb24ab437fff321a170dbc42a5c7e067f..5ad05dc4b6d8993ef7ca6e5979c5e6361ded33ba 100644
--- a/net/base/sdch_manager.cc
+++ b/net/base/sdch_manager.cc
@@ -13,7 +13,10 @@
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
+#include "base/strings/stringprintf.h"
 #include "base/time/default_clock.h"
+#include "base/trace_event/memory_allocator_dump.h"
+#include "base/trace_event/process_memory_dump.h"
 #include "base/values.h"
 #include "crypto/sha2.h"
 #include "net/base/parse_number.h"
@@ -324,6 +327,37 @@ void SdchManager::RemoveObserver(SdchObserver* observer) {
   observers_.RemoveObserver(observer);
 }
 
+void SdchManager::DumpMemoryStats(
+    base::trace_event::ProcessMemoryDump* pmd,
+    const std::string& parent_dump_absolute_name) const {
+  std::string name = base::StringPrintf("net/sdch_manager_%p", this);
+  base::trace_event::MemoryAllocatorDump* dump = pmd->GetAllocatorDump(name);
+  if (dump == nullptr) {
+    // If there are no dictionaries stored, return early without creating a new
+    // MemoryAllocatorDump.
+    size_t total_count = dictionaries_.size();
+    if (total_count == 0)
+      return;

[ssid, 2016/12/15 23:00:32]

I feel if you could move this to the top of the function, it is easier to
understand what is going on. It took a moment to understand when the dump is
created.

[xunjieli, 2016/12/16 15:54:02]

Done.

+    dump = pmd->CreateAllocatorDump(name);
+    size_t total_size = 0;
+    for (const auto& dictionary : dictionaries_) {
+      total_size += dictionary.second->data.text().size();
+    }
+    dump->AddScalar(base::trace_event::MemoryAllocatorDump::kNameSize,
+                    base::trace_event::MemoryAllocatorDump::kUnitsBytes,
+                    total_size);
+    dump->AddScalar(base::trace_event::MemoryAllocatorDump::kNameObjectCount,
+                    base::trace_event::MemoryAllocatorDump::kUnitsObjects,
+                    total_count);
+  }
+  // Create an empty row under parent's dump so size can be attributed correctly
+  // if |this| is shared between URLRequestContexts.
+  base::trace_event::MemoryAllocatorDump* empty_row_dump =

[ssid, 2016/12/15 23:00:32]

I really wish there was a way to add size to this dump rather than the
"net/sdch_manager" dump. Then we would clearly see how much usage is caused by
media or safe_browsing.
There's nothing else you could do here though.

[xunjieli, 2016/12/16 15:54:02]

Acknowledged.

+      pmd->CreateAllocatorDump(base::StringPrintf(
+          "%s/sdch_manager", parent_dump_absolute_name.c_str()));

[ssid, 2016/12/15 23:00:32]

You could just do (parent_dump_absolute_name + "/sdch_manager")

[xunjieli, 2016/12/16 15:54:02]

Done.

+  pmd->AddOwnershipEdge(empty_row_dump->guid(), dump->guid());
+}
+
 SdchProblemCode SdchManager::AddSdchDictionary(
     const std::string& dictionary_text,
     const GURL& dictionary_url,