Make net::URLRequestContext a MemoryDumpProvider

net/url_request/url_request_context.h

Index: net/url_request/url_request_context.h
diff --git a/net/url_request/url_request_context.h b/net/url_request/url_request_context.h
index 0158d9aca17282fb5153f318f65d596a3c5fb6ef..24b03bb8a58dc86d11cb99c835e288367bb56e7f 100644
--- a/net/url_request/url_request_context.h
+++ b/net/url_request/url_request_context.h
@@ -16,6 +16,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
 #include "base/threading/non_thread_safe.h"
+#include "base/trace_event/memory_dump_provider.h"
 #include "net/base/net_export.h"
 #include "net/base/request_priority.h"
 #include "net/http/http_network_session.h"
@@ -24,6 +25,12 @@
 #include "net/ssl/ssl_config_service.h"
 #include "net/url_request/url_request.h"
 
+namespace base {
+namespace trace_event {
+class ProcessMemoryDump;
+}
+}
+
 namespace net {
 class CertVerifier;
 class ChannelIDService;
@@ -51,10 +58,11 @@ class URLRequestThrottlerManager;
 // URLRequestContext rather than creating a new one, as guaranteeing that the
 // URLRequestContext is destroyed before its members can be difficult.
 class NET_EXPORT URLRequestContext
-    : NON_EXPORTED_BASE(public base::NonThreadSafe) {
+    : NON_EXPORTED_BASE(public base::NonThreadSafe),
+      public base::trace_event::MemoryDumpProvider {
  public:
   URLRequestContext();
-  virtual ~URLRequestContext();
+  ~URLRequestContext() override;
 
   // Copies the state from |other| into this context.
   void CopyFrom(const URLRequestContext* other);
@@ -230,6 +238,15 @@ class NET_EXPORT URLRequestContext
 
   bool enable_brotli() const { return enable_brotli_; }
 
+  // Sets a name for this URLRequestContext. Currently the name is used in
+  // MemoryDumpProvier to annotate memory usage. The name does not need to be
+  // unique.
+  void set_name(const std::string& name) { name_ = name; }
+
+  // MemoryDumpProvider implementation:
+  bool OnMemoryDump(const base::trace_event::MemoryDumpArgs& args,
+                    base::trace_event::ProcessMemoryDump* pmd) override;

[mmenke, 2016/12/02 15:47:28]

Does this work?  The URLRequestContext depends on all its subcomponents.  If
those subcomponents use the URLRequestContext as a memory dump provider, that
seems to create a circular dependency, no?

Worth noting that URLRequestJobs and URLRequests already do have circular
dependencies on the URLRequestContext, but adding more such dependencies to
lower layers of the network stack (Which may even be shared between
URLRequestContexts, some of which could be destroyed well before the main
URLRequestContext, and the shared HttpNetworkSession...) seems like it would
make the situation worse.

[xunjieli, 2016/12/02 16:13:14]

Great question. I don't think there's a circular dependency. MemoryDumpProvider
is polling based. Here we are just trying to reach associated components. The
subcomponents don't need to know about the parent/caller. If a subcomponent goes
away, we can't reach it from URLRequestContext, so I think we won't complicate
the lifetime issues.

I have a draft CL at https://codereview.chromium.org/2525743002/ which outlines
the overall approach.

+
  private:
   // ---------------------------------------------------------------------------
   // Important: When adding any new members below, consider whether they need to
@@ -269,6 +286,11 @@ class NET_EXPORT URLRequestContext
   // Enables Brotli Content-Encoding support.
   bool enable_brotli_;
 
+  // An optional name which can be set to describe this URLRequestContext.
+  // Used in MemoryDumpProvier to annotate memory usage. The name does not need
+  // to be unique.
+  std::string name_;
+
   DISALLOW_COPY_AND_ASSIGN(URLRequestContext);
 };