[OBSOLETE][memory-infra] Whitelist net dump providers in field trials

net/url_request/url_request_context.cc

Index: net/url_request/url_request_context.cc
diff --git a/net/url_request/url_request_context.cc b/net/url_request/url_request_context.cc
index cefa7b9db2593c3e0abc8ed1d39485fef36e267b..f7207e9a16b45efd25b321c0d6cb7aed120587c2 100644
--- a/net/url_request/url_request_context.cc
+++ b/net/url_request/url_request_context.cc
@@ -127,20 +127,30 @@ bool URLRequestContext::OnMemoryDump(
     base::trace_event::ProcessMemoryDump* pmd) {
   if (name_.empty())
     name_ = "unknown";
-  base::trace_event::MemoryAllocatorDump* dump = pmd->CreateAllocatorDump(
-      base::StringPrintf("net/url_request_context/%s_%p", name_.c_str(), this));
-  dump->AddScalar(base::trace_event::MemoryAllocatorDump::kNameObjectCount,
-                  base::trace_event::MemoryAllocatorDump::kUnitsObjects,
-                  url_requests_->size());
+
+  SSLClientSocketImpl::DumpSSLClientSessionMemoryStats(pmd);
+
+  // The details about the url request name is not added in BACKGROUND mode

[xunjieli, 2017/02/02 22:33:27]

nit: s/is/are
Think you meant to say "url request context" instead of "url request"

[ssid, 2017/02/03 02:26:00]

Done.

+  // dumps.
+  std::string dump_name;
+  if (args.level_of_detail !=
+      base::trace_event::MemoryDumpLevelOfDetail::BACKGROUND) {
+    dump_name = base::StringPrintf("net/url_request_context/%s_%p",

[xunjieli, 2017/02/02 22:33:27]

I was about to instrument net::URLRequest and attribute the memory allocation to
net::URLRequestContext. If you get rid of the "/url_request_context" dump in
background mode, how do I attribute the memory of net::URLRequest?

CL is at https://codereview.chromium.org/2540233002/. URLRequestContext
(indirectly) "owns" a set of URLRequests.

[ssid, 2017/02/02 23:24:57]

I see. I removed the url_requet_context dumps because they were just taking more
space in trace without any memory attributed to them. If you are going to add
child dumps then we should include these. Currently the only reason for these
dumps to exist is to add the name of the context in trace.

Okay wdyt should be done here? Should we enable the existing net provider in
slow reports and then continue to add more details or should we just wait till
you finish instrumenting all parts then turn on for slow reports? I thought the
instrumentation is mostly done and you mailed about asking to enable on slow
reports, so sent this CL.

The other issue is every time you add more details you'd have to update the
whitelist and also take care that the performance of the provider is not
regressing much since it will be directly enabled on user devices and the perf
tests would not catch this regression since telemetry does not support net very
well.

[xunjieli, 2017/02/02 23:41:06]

I see. I seriously wonder whether there is an end to the instrumentation. As I
try to instrument more of the codebase, I find even more to instrument. Just see
how many TODOs (!) I left behind :).
I think we should go ahead and enable this in SlowReports. It might be another
two quarters when net/ is properly instrumented. We might find interesting
behavior in the meanwhile.
I am fine with hiding "url_request_context" in background mode until they have
memory attributed to them.

+                                   name_.c_str(), this);
+    base::trace_event::MemoryAllocatorDump* dump =
+        pmd->CreateAllocatorDump(dump_name);
+    dump->AddScalar(base::trace_event::MemoryAllocatorDump::kNameObjectCount,
+                    base::trace_event::MemoryAllocatorDump::kUnitsObjects,
+                    url_requests_->size());
+  }
   HttpTransactionFactory* transaction_factory = http_transaction_factory();
   if (transaction_factory) {
     HttpNetworkSession* network_session = transaction_factory->GetSession();
     if (network_session)
-      network_session->DumpMemoryStats(pmd, dump->absolute_name());
+      network_session->DumpMemoryStats(pmd, dump_name);
   }
-  SSLClientSocketImpl::DumpSSLClientSessionMemoryStats(pmd);
   if (sdch_manager_)
-    sdch_manager_->DumpMemoryStats(pmd, dump->absolute_name());
+    sdch_manager_->DumpMemoryStats(pmd, dump_name);
   return true;
 }