Make URLRequestContext a MemoryDumpProvider (Abandoned)

net/base/sdch_manager.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/base/sdch_manager.h"
 
 #include <limits.h>
 
 #include <utility>
 
 #include "base/base64url.h"
 #include "base/logging.h"
 #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"
 #include "net/base/sdch_net_log_params.h"
 #include "net/base/sdch_observer.h"
 #include "net/url_request/url_request_http_job.h"
 
 namespace {
 
 void StripTrailingDot(GURL* gurl) {
@@ skipping 290 matching lines @@
 }
 
 void SdchManager::AddObserver(SdchObserver* observer) {
   observers_.AddObserver(observer);
 }
 
 void SdchManager::RemoveObserver(SdchObserver* observer) {
   observers_.RemoveObserver(observer);
 }
 
+void SdchManager::DumpMemoryStats(
+    base::trace_event::ProcessMemoryDump* pmd) const {
+  std::string name = base::StringPrintf("net/sdch_manager_%p", this);
+  base::trace_event::MemoryAllocatorDump* dictionary_dump =
+      pmd->GetAllocatorDump(name);
+  if (dictionary_dump)

[eroman, 2016/11/29 22:22:50]

What are the circumstances when this would be reached?

Is the thinking that we may have a shared SdchManager between
URLRequestContexts? And hence when we enumerate through the URLRequestContexts
we skip duplicate logging here?

Please add a comment on intent. Should it add an ownership edge like in the
other location with shared state?

[xunjieli, 2016/11/30 16:02:03]

Done.

URLRequestContexts currently don't share a SdchManager, though they in theory
can. I have added a comment here and an ownership edge.

+    return;
+  size_t total_size = 0;
+  size_t total_count = 0;
+  for (const auto& dictionary : dictionaries_) {
+    total_size += dictionary.second->data.text().size();
+    ++total_count;

[eroman, 2016/11/29 22:22:50]

maybe just dictionaries_.size() outside of loop?

[xunjieli, 2016/11/30 16:02:03]

Done.

+  }
+  if (total_count == 0)
+    return;
+  dictionary_dump = pmd->CreateAllocatorDump(name);
+  dictionary_dump->AddScalar(
+      base::trace_event::MemoryAllocatorDump::kNameSize,
+      base::trace_event::MemoryAllocatorDump::kUnitsBytes, total_size);
+
+  // Use the existing column name so as not to add a new column.

[eroman, 2016/11/29 22:22:50]

I am not sure what this comment means regarding "existing" column? Is this about
the UI presentation or "count" being a common name used somewhere else?

[xunjieli, 2016/11/30 16:02:03]

Done. Sorry, there is a constant
(base::trace_event::MemoryAllocatorDump::kObjectsCount) that I can used here. I
have removed the confusing comment.

+  dictionary_dump->AddScalar(
+      "count", base::trace_event::MemoryAllocatorDump::kUnitsObjects,
+      total_count);
+}
+
 SdchProblemCode SdchManager::AddSdchDictionary(
     const std::string& dictionary_text,
     const GURL& dictionary_url,
     std::string* server_hash_p) {
   DCHECK(thread_checker_.CalledOnValidThread());
   std::string client_hash;
   std::string server_hash;
   GenerateHash(dictionary_text, &client_hash, &server_hash);
   if (dictionaries_.find(server_hash) != dictionaries_.end())
     return SDCH_DICTIONARY_ALREADY_LOADED;  // Already loaded.
@@ skipping 157 matching lines @@
       entry_dict->SetInteger("tries", it->second.count);
     entry_dict->SetInteger("reason", it->second.reason);
     entry_list->Append(std::move(entry_dict));
   }
   value->Set("blacklisted", std::move(entry_list));
 
   return std::move(value);
 }
 
 }  // namespace net