Make WebSocketTransportClientSocketPool not a TransportClientSocketPool

This CL makes it so that WebSocketTransportClientSocketPool no longer
inherits from TransportClientSocketPool. Since it has almost no shared
logic, this inheritence was unnecessary and a little confusing.

Bug: 944097
Change-Id: I5c70257e3b48bf05bf5e7e9c5732cedb24166904
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1532919
Commit-Queue: Matt Menke <mmenke@chromium.org>
Reviewed-by: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#644634}

net/socket/client_socket_pool.cc

@@ -9,6 +9,8 @@
 #include "base/bind.h"
 #include "base/logging.h"
 #include "net/http/http_proxy_connect_job.h"
+#include "net/log/net_log_event_type.h"
+#include "net/log/net_log_with_source.h"
 #include "net/socket/socks_connect_job.h"
 #include "net/socket/ssl_connect_job.h"
 #include "net/socket/stream_socket.h"
@@ -167,4 +169,24 @@ void ClientSocketPool::set_used_idle_socket_timeout(base::TimeDelta timeout) {
 
 ClientSocketPool::ClientSocketPool() = default;
 
+void ClientSocketPool::NetLogTcpClientSocketPoolRequestedSocket(
+    const NetLogWithSource& net_log,
+    const GroupId& group_id) {
+  if (net_log.IsCapturing()) {
+    // TODO(eroman): Split out the host and port parameters.
+    net_log.AddEvent(NetLogEventType::TCP_CLIENT_SOCKET_POOL_REQUESTED_SOCKET,
+                     base::BindRepeating(&NetLogGroupIdCallback,
+                                         base::Unretained(&group_id)));
+  }
+}
+
+std::unique_ptr<base::Value> ClientSocketPool::NetLogGroupIdCallback(
+    const ClientSocketPool::GroupId* group_id,
+    NetLogCaptureMode /* capture_mode */) {
+  std::unique_ptr<base::DictionaryValue> event_params(
+      new base::DictionaryValue());
+  event_params->SetString("group_id", group_id->ToString());
+  return event_params;
+}
+
 }  // namespace net

net/socket/client_socket_pool.h

@@ -17,10 +17,12 @@
 #include "net/base/request_priority.h"
 #include "net/dns/host_resolver.h"
 #include "net/http/http_request_info.h"
+#include "net/log/net_log_capture_mode.h"
 #include "net/socket/connect_job.h"
 
 namespace base {
 class DictionaryValue;
+class Value;
 namespace trace_event {
 class ProcessMemoryDump;
 }
@@ -334,6 +336,14 @@ class NET_EXPORT ClientSocketPool : public LowerLayeredPool {
  protected:
   ClientSocketPool();
 
+  void NetLogTcpClientSocketPoolRequestedSocket(const NetLogWithSource& net_log,
+                                                const GroupId& group_id);
+
+  // Utility method to log a GroupId with a NetLog event.
+  static std::unique_ptr<base::Value> NetLogGroupIdCallback(
+      const ClientSocketPool::GroupId* group_id,
+      NetLogCaptureMode capture_mode);
+
  private:
   DISALLOW_COPY_AND_ASSIGN(ClientSocketPool);
 };

net/socket/client_socket_pool_manager_impl.cc

@@ -80,8 +80,7 @@ ClientSocketPool* ClientSocketPoolManagerImpl::GetSocketPool(
       proxy_server.is_direct()) {
     new_pool = std::make_unique<WebSocketTransportClientSocketPool>(
         sockets_per_proxy_server, sockets_per_group,
-        unused_idle_socket_timeout(pool_type_),
-        &websocket_common_connect_job_params_, ssl_config_service_);
+        &websocket_common_connect_job_params_);
   } else {
     new_pool = std::make_unique<TransportClientSocketPool>(
         sockets_per_proxy_server, sockets_per_group,

net/socket/transport_client_socket_pool.cc

@@ -46,15 +46,6 @@ std::unique_ptr<base::Value> NetLogCreateConnectJobCallback(
   return std::move(dict);
 }
 
-std::unique_ptr<base::Value> NetLogGroupIdCallback(
-    const ClientSocketPool::GroupId* group_id,
-    NetLogCaptureMode /* capture_mode */) {
-  std::unique_ptr<base::DictionaryValue> event_params(
-      new base::DictionaryValue());
-  event_params->SetString("group_id", group_id->ToString());
-  return event_params;
-}
-
 // ConnectJobFactory implementation that creates the standard ConnectJob
 // classes, using SocketParams.
 class TransportConnectJobFactory
@@ -62,7 +53,12 @@ class TransportConnectJobFactory
  public:
   explicit TransportConnectJobFactory(
       const CommonConnectJobParams* common_connect_job_params)
-      : common_connect_job_params_(common_connect_job_params) {}
+      : common_connect_job_params_(common_connect_job_params) {
+    // This class should not be used with WebSockets. Note that
+    // |common_connect_job_params| may be nullptr in tests.
+    DCHECK(!common_connect_job_params ||
+           !common_connect_job_params->websocket_endpoint_lock_manager);
+  }
 
   ~TransportConnectJobFactory() override = default;
 
@@ -987,17 +983,6 @@ void TransportClientSocketPool::OnIPAddressChanged() {
   FlushWithError(ERR_NETWORK_CHANGED);
 }
 
-void TransportClientSocketPool::NetLogTcpClientSocketPoolRequestedSocket(
-    const NetLogWithSource& net_log,
-    const GroupId& group_id) {
-  if (net_log.IsCapturing()) {
-    // TODO(eroman): Split out the host and port parameters.
-    net_log.AddEvent(NetLogEventType::TCP_CLIENT_SOCKET_POOL_REQUESTED_SOCKET,
-                     base::BindRepeating(&NetLogGroupIdCallback,
-                                         base::Unretained(&group_id)));
-  }
-}
-
 void TransportClientSocketPool::FlushWithError(int error) {
   pool_generation_number_++;
   CancelAllConnectJobs();

net/socket/transport_client_socket_pool.h

@@ -266,11 +266,6 @@ class NET_EXPORT_PRIVATE TransportClientSocketPool
   // NetworkChangeNotifier::IPAddressObserver methods:
   void OnIPAddressChanged() override;
 
- protected:
-  // Methods shared with WebSocketTransportClientSocketPool
-  void NetLogTcpClientSocketPoolRequestedSocket(const NetLogWithSource& net_log,
-                                                const GroupId& group_id);
-
  private:
   // Entry for a persistent socket which became idle at time |start_time|.
   struct IdleSocket {

net/socket/websocket_transport_client_socket_pool.cc

@@ -29,15 +29,8 @@ namespace net {
 WebSocketTransportClientSocketPool::WebSocketTransportClientSocketPool(
     int max_sockets,
     int max_sockets_per_group,
-    base::TimeDelta unused_idle_socket_timeout,
-    const CommonConnectJobParams* common_connect_job_params,
-    SSLConfigService* ssl_config_service)
-    : TransportClientSocketPool(max_sockets,
-                                max_sockets_per_group,
-                                unused_idle_socket_timeout,
-                                common_connect_job_params,
-                                ssl_config_service),
-      common_connect_job_params_(common_connect_job_params),
+    const CommonConnectJobParams* common_connect_job_params)
+    : common_connect_job_params_(common_connect_job_params),
       max_sockets_(max_sockets),
       handed_out_socket_count_(0),
       flushing_(false),
@@ -248,10 +241,28 @@ WebSocketTransportClientSocketPool::GetInfoAsValue(
   return dict;
 }
 
+void WebSocketTransportClientSocketPool::DumpMemoryStats(
+    base::trace_event::ProcessMemoryDump* pmd,
+    const std::string& parent_dump_absolute_name) const {
+  // Not supported.
+}
+
 bool WebSocketTransportClientSocketPool::IsStalled() const {
   return !stalled_request_queue_.empty();
 }
 
+void WebSocketTransportClientSocketPool::AddHigherLayeredPool(
+    HigherLayeredPool* higher_pool) {
+  // This class doesn't use connection limits like the pools for HTTP do, so no
+  // need to track higher layered pools.
+}
+
+void WebSocketTransportClientSocketPool::RemoveHigherLayeredPool(
+    HigherLayeredPool* higher_pool) {
+  // This class doesn't use connection limits like the pools for HTTP do, so no
+  // need to track higher layered pools.
+}
+
 bool WebSocketTransportClientSocketPool::TryHandOutSocket(
     int result,
     ConnectJobDelegate* connect_job_delegate) {

net/socket/websocket_transport_client_socket_pool.h

@@ -22,27 +22,26 @@
 #include "net/socket/client_socket_pool.h"
 #include "net/socket/connect_job.h"
 #include "net/socket/ssl_client_socket.h"
-#include "net/socket/transport_client_socket_pool.h"
 
 namespace base {
 class DictionaryValue;
+namespace trace_event {
+class ProcessMemoryDump;
 }
+}  // namespace base
 
 namespace net {
 
 struct CommonConnectJobParams;
-class SSLConfigService;
 class WebSocketTransportConnectJob;
 
 class NET_EXPORT_PRIVATE WebSocketTransportClientSocketPool
-    : public TransportClientSocketPool {
+    : public ClientSocketPool {
  public:
   WebSocketTransportClientSocketPool(
       int max_sockets,
       int max_sockets_per_group,
-      base::TimeDelta unused_idle_socket_timeout,
-      const CommonConnectJobParams* common_connect_job_params,
-      SSLConfigService* ssl_config_service);
+      const CommonConnectJobParams* common_connect_job_params);
 
   ~WebSocketTransportClientSocketPool() override;
 
@@ -87,9 +86,14 @@ class NET_EXPORT_PRIVATE WebSocketTransportClientSocketPool
   std::unique_ptr<base::DictionaryValue> GetInfoAsValue(
       const std::string& name,
       const std::string& type) const override;
+  void DumpMemoryStats(
+      base::trace_event::ProcessMemoryDump* pmd,
+      const std::string& parent_dump_absolute_name) const override;
 
   // HigherLayeredPool implementation.
   bool IsStalled() const override;
+  void AddHigherLayeredPool(HigherLayeredPool* higher_pool) override;
+  void RemoveHigherLayeredPool(HigherLayeredPool* higher_pool) override;
 
  private:
   class ConnectJobDelegate : public ConnectJob::Delegate {

net/socket/websocket_transport_client_socket_pool_unittest.cc

@@ -46,8 +46,6 @@ namespace {
 
 const int kMaxSockets = 32;
 const int kMaxSocketsPerGroup = 6;
-constexpr base::TimeDelta kUnusedIdleSocketTimeout =
-    base::TimeDelta::FromSeconds(10);
 const RequestPriority kDefaultPriority = LOW;
 
 // RunLoop doesn't support this natively but it is easy to emulate.
@@ -88,11 +86,7 @@ class WebSocketTransportClientSocketPoolTest
             nullptr /* network_quality_estimator */,
             nullptr /* netlog */,
             &websocket_endpoint_lock_manager_),
-        pool_(kMaxSockets,
-              kMaxSocketsPerGroup,
-              kUnusedIdleSocketTimeout,
-              &common_connect_job_params_,
-              nullptr /* ssl_config_service */) {
+        pool_(kMaxSockets, kMaxSocketsPerGroup, &common_connect_job_params_) {
     websocket_endpoint_lock_manager_.SetUnlockDelayForTesting(
         base::TimeDelta());
   }

net/spdy/spdy_session_pool.cc

@@ -29,6 +29,7 @@
 #include "net/log/net_log_event_type.h"
 #include "net/log/net_log_source.h"
 #include "net/log/net_log_with_source.h"
+#include "net/socket/client_socket_handle.h"
 #include "net/spdy/bidirectional_stream_spdy_impl.h"
 #include "net/spdy/spdy_http_stream.h"
 #include "net/spdy/spdy_session.h"