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

net/url_request/url_request_context.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/url_request/url_request_context.h"
 
 #include "base/compiler_specific.h"
 #include "base/debug/alias.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/string_util.h"
@@ skipping 109 matching lines @@
     CHECK(false) << "Leaked " << num_requests << " URLRequest(s). First URL: "
                  << request->url().spec().c_str() << ".";
   }
 }
 
 bool URLRequestContext::OnMemoryDump(
     const base::trace_event::MemoryDumpArgs& args,
     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;
 }
 
 }  // namespace net