Make HttpNetworkSession a client of memory coordinator

net/http/http_network_session.h

Index: net/http/http_network_session.h
diff --git a/net/http/http_network_session.h b/net/http/http_network_session.h
index 67c884ee41681be0ba033f2684a01ec99c236ead..ed1bd805ecba9a29fbd55ac030886f08e2b6c20e 100644
--- a/net/http/http_network_session.h
+++ b/net/http/http_network_session.h
@@ -14,6 +14,7 @@
 #include <vector>
 
 #include "base/bind.h"
+#include "base/memory/memory_coordinator_client.h"
 #include "base/memory/memory_pressure_monitor.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
@@ -61,7 +62,8 @@ class TransportSecurityState;
 
 // This class holds session objects used by HttpNetworkTransaction objects.
 class NET_EXPORT HttpNetworkSession
-    : NON_EXPORTED_BASE(public base::NonThreadSafe) {
+    : NON_EXPORTED_BASE(public base::NonThreadSafe),
+      public base::MemoryCoordinatorClient {

[Ryan Sleevi, 2016/09/26 15:15:59]

Just from a design perspective, base::MemoryPressureListener seems like a safer
design pattern/paradigm.

That is, it automatically includes the de-registration aspect (when it goes out
of scope) and it avoids the need for public inheritance (by virtue of invoking
the callback).

The interface pattern is useful if we anticipate there being multiple methods a
memory coordinator client can have invoked, but if we just expect one (a state
transition), the callback approach seems to offer much better design decoupling.

We could easily imagine this being done by something like

class HttpNetworkSession {
 private:
  class Helper;

  void OnMemoryStateChange(base::MemoryState state);

  std::unique_ptr<Helper> helper_;
};

in .cc
class HttpNetworkSession::Helper {
 public:
  explicit Helper(const base::Callback<base::MemoryState>& callback) :
cb_(callback) {
   ClientRegistry->AddObserver(this);
  }
  ~Helper() {
   ClientRegistry->RemoveObserver(this);
  }

  void OnMemoryStateChange(base::MemoryState state) {
   cb_.Run(state);
  }
};



I mention this because I think having a bunch of classes inherit from
base::MemoryCoordinatorClient as part of their public interfaces probably isn't
what we want/desire long term, because that exposes the implementation detail
publicly, versus the MemoryPressureListener 'implementation detail' approach.

  public:
   struct NET_EXPORT Params {
     Params();
@@ -199,7 +201,7 @@ class NET_EXPORT HttpNetworkSession
   };
 
   explicit HttpNetworkSession(const Params& params);
-  ~HttpNetworkSession();
+  ~HttpNetworkSession() override;
 
   HttpAuthCache* http_auth_cache() { return &http_auth_cache_; }
   SSLClientAuthCache* ssl_client_auth_cache() {
@@ -279,6 +281,9 @@ class NET_EXPORT HttpNetworkSession
   void OnMemoryPressure(
       base::MemoryPressureListener::MemoryPressureLevel memory_pressure_level);
 
+  // base::MemoryCoordinatorClient implementation:
+  void OnMemoryStateChange(base::MemoryState state) override;
+
   NetLog* const net_log_;
   HttpServerProperties* const http_server_properties_;
   CertVerifier* const cert_verifier_;