Move most of the code for TCPClientSocketPool into ClientSocketPoolBase for sharing.

Also rename idle_socket_count() to IdleSocketCount in ClientSocketPool and derived classes, since it's virtual.
BUG=http://crbug.com/13289

Review URL: http://codereview.chromium.org/125238

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@18622 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 150 insertions, 59 deletions

diff --git a/net/base/client_socket_pool.h b/net/base/client_socket_pool.h
index 86c67b8..4567108 100644
--- a/net/base/client_socket_pool.h
+++ b/net/base/client_socket_pool.h
@@ -73,7 +73,7 @@ class ClientSocketPool : public base::RefCounted<ClientSocketPool> {
   virtual HostResolver* GetHostResolver() const = 0;
 
   // The total number of idle sockets in the pool.
-  virtual int idle_socket_count() const = 0;
+  virtual int IdleSocketCount() const = 0;
 
   // The total number of idle sockets in a connection group.
   virtual int IdleSocketCountInGroup(const std::string& group_name) const = 0;
diff --git a/net/base/tcp_client_socket_pool.cc b/net/base/tcp_client_socket_pool.cc
index 6b6044f..969ad3d 100644
--- a/net/base/tcp_client_socket_pool.cc
+++ b/net/base/tcp_client_socket_pool.cc
@@ -38,7 +38,7 @@ ConnectingSocket::ConnectingSocket(
     const HostResolver::RequestInfo& resolve_info,
     const ClientSocketHandle* handle,
     ClientSocketFactory* client_socket_factory,
-    TCPClientSocketPool* pool)
+    ClientSocketPoolBase* pool)
     : group_name_(group_name),
       resolve_info_(resolve_info),
       handle_(handle),
@@ -69,7 +69,7 @@ int ConnectingSocket::OnIOCompleteInternal(
     int result, bool synchronous) {
   CHECK(result != ERR_IO_PENDING);
 
-  TCPClientSocketPool::Request* request = pool_->GetConnectingRequest(
+  ClientSocketPoolBase::Request* request = pool_->GetConnectingRequest(
       group_name_, handle_);
   CHECK(request);
 
@@ -123,7 +123,7 @@ int ConnectingSocket::OnIOCompleteInternal(
   return result;
 }
 
-TCPClientSocketPool::TCPClientSocketPool(
+ClientSocketPoolBase::ClientSocketPoolBase(
     int max_sockets_per_group,
     HostResolver* host_resolver,
     ClientSocketFactory* client_socket_factory)
@@ -133,7 +133,7 @@ TCPClientSocketPool::TCPClientSocketPool(
       host_resolver_(host_resolver) {
 }
 
-TCPClientSocketPool::~TCPClientSocketPool() {
+ClientSocketPoolBase::~ClientSocketPoolBase() {
   // Clean up any idle sockets.  Assert that we have no remaining active
   // sockets or pending requests.  They should have all been cleaned up prior
   // to the manager being destroyed.
@@ -147,7 +147,7 @@ TCPClientSocketPool::~TCPClientSocketPool() {
 // prioritized over requests of equal priority.
 //
 // static
-void TCPClientSocketPool::InsertRequestIntoQueue(
+void ClientSocketPoolBase::InsertRequestIntoQueue(
     const Request& r, RequestQueue* pending_requests) {
   RequestQueue::iterator it = pending_requests->begin();
   while (it != pending_requests->end() && r.priority <= it->priority)
@@ -155,7 +155,7 @@ void TCPClientSocketPool::InsertRequestIntoQueue(
   pending_requests->insert(it, r);
 }
 
-int TCPClientSocketPool::RequestSocket(
+int ClientSocketPoolBase::RequestSocket(
     const std::string& group_name,
     const HostResolver::RequestInfo& resolve_info,
     int priority,
@@ -206,8 +206,8 @@ int TCPClientSocketPool::RequestSocket(
   return rv;
 }
 
-void TCPClientSocketPool::CancelRequest(const std::string& group_name,
-                                        const ClientSocketHandle* handle) {
+void ClientSocketPoolBase::CancelRequest(const std::string& group_name,
+                                         const ClientSocketHandle* handle) {
   CHECK(ContainsKey(group_map_, group_name));
 
   Group& group = group_map_[group_name];
@@ -240,20 +240,20 @@ void TCPClientSocketPool::CancelRequest(const std::string& group_name,
   }
 }
 
-void TCPClientSocketPool::ReleaseSocket(const std::string& group_name,
-                                        ClientSocket* socket) {
+void ClientSocketPoolBase::ReleaseSocket(const std::string& group_name,
+                                         ClientSocket* socket) {
   // Run this asynchronously to allow the caller to finish before we let
   // another to begin doing work.  This also avoids nasty recursion issues.
   // NOTE: We cannot refer to the handle argument after this method returns.
   MessageLoop::current()->PostTask(FROM_HERE, NewRunnableMethod(
-      this, &TCPClientSocketPool::DoReleaseSocket, group_name, socket));
+      this, &ClientSocketPoolBase::DoReleaseSocket, group_name, socket));
 }
 
-void TCPClientSocketPool::CloseIdleSockets() {
+void ClientSocketPoolBase::CloseIdleSockets() {
   CleanupIdleSockets(true);
 }
 
-int TCPClientSocketPool::IdleSocketCountInGroup(
+int ClientSocketPoolBase::IdleSocketCountInGroup(
     const std::string& group_name) const {
   GroupMap::const_iterator i = group_map_.find(group_name);
   CHECK(i != group_map_.end());
@@ -261,7 +261,7 @@ int TCPClientSocketPool::IdleSocketCountInGroup(
   return i->second.idle_sockets.size();
 }
 
-LoadState TCPClientSocketPool::GetLoadState(
+LoadState ClientSocketPoolBase::GetLoadState(
     const std::string& group_name,
     const ClientSocketHandle* handle) const {
   if (!ContainsKey(group_map_, group_name)) {
@@ -297,13 +297,13 @@ LoadState TCPClientSocketPool::GetLoadState(
   return LOAD_STATE_IDLE;
 }
 
-bool TCPClientSocketPool::IdleSocket::ShouldCleanup(base::TimeTicks now) const {
+bool ClientSocketPoolBase::IdleSocket::ShouldCleanup(base::TimeTicks now) const {
   bool timed_out = (now - start_time) >=
       base::TimeDelta::FromSeconds(kIdleTimeout);
   return timed_out || !socket->IsConnectedAndIdle();
 }
 
-void TCPClientSocketPool::CleanupIdleSockets(bool force) {
+void ClientSocketPoolBase::CleanupIdleSockets(bool force) {
   if (idle_socket_count_ == 0)
     return;
 
@@ -337,19 +337,19 @@ void TCPClientSocketPool::CleanupIdleSockets(bool force) {
   }
 }
 
-void TCPClientSocketPool::IncrementIdleCount() {
+void ClientSocketPoolBase::IncrementIdleCount() {
   if (++idle_socket_count_ == 1)
     timer_.Start(TimeDelta::FromSeconds(kCleanupInterval), this,
-                 &TCPClientSocketPool::OnCleanupTimerFired);
+                 &ClientSocketPoolBase::OnCleanupTimerFired);
 }
 
-void TCPClientSocketPool::DecrementIdleCount() {
+void ClientSocketPoolBase::DecrementIdleCount() {
   if (--idle_socket_count_ == 0)
     timer_.Stop();
 }
 
-void TCPClientSocketPool::DoReleaseSocket(const std::string& group_name,
-                                          ClientSocket* socket) {
+void ClientSocketPoolBase::DoReleaseSocket(const std::string& group_name,
+                                           ClientSocket* socket) {
   GroupMap::iterator i = group_map_.find(group_name);
   CHECK(i != group_map_.end());
 
@@ -390,7 +390,7 @@ void TCPClientSocketPool::DoReleaseSocket(const std::string& group_name,
   }
 }
 
-TCPClientSocketPool::Request* TCPClientSocketPool::GetConnectingRequest(
+ClientSocketPoolBase::Request* ClientSocketPoolBase::GetConnectingRequest(
     const std::string& group_name, const ClientSocketHandle* handle) {
   GroupMap::iterator group_it = group_map_.find(group_name);
   if (group_it == group_map_.end())
@@ -406,7 +406,7 @@ TCPClientSocketPool::Request* TCPClientSocketPool::GetConnectingRequest(
   return &it->second;
 }
 
-CompletionCallback* TCPClientSocketPool::OnConnectingRequestComplete(
+CompletionCallback* ClientSocketPoolBase::OnConnectingRequestComplete(
     const std::string& group_name,
     const ClientSocketHandle* handle,
     bool deactivate,
@@ -443,7 +443,7 @@ CompletionCallback* TCPClientSocketPool::OnConnectingRequestComplete(
   return request.callback;
 }
 
-void TCPClientSocketPool::RemoveConnectingSocket(
+void ClientSocketPoolBase::RemoveConnectingSocket(
     const ClientSocketHandle* handle) {
   ConnectingSocketMap::iterator it = connecting_socket_map_.find(handle);
   CHECK(it != connecting_socket_map_.end());
@@ -451,4 +451,49 @@ void TCPClientSocketPool::RemoveConnectingSocket(
   connecting_socket_map_.erase(it);
 }
 
+TCPClientSocketPool::TCPClientSocketPool(
+    int max_sockets_per_group,
+    HostResolver* host_resolver,
+    ClientSocketFactory* client_socket_factory)
+    : base_(new ClientSocketPoolBase(
+        max_sockets_per_group, host_resolver, client_socket_factory)) {}
+
+TCPClientSocketPool::~TCPClientSocketPool() {}
+
+int TCPClientSocketPool::RequestSocket(
+    const std::string& group_name,
+    const HostResolver::RequestInfo& resolve_info,
+    int priority,
+    ClientSocketHandle* handle,
+    CompletionCallback* callback) {
+  return base_->RequestSocket(
+      group_name, resolve_info, priority, handle, callback);
+}
+
+void TCPClientSocketPool::CancelRequest(
+    const std::string& group_name,
+    const ClientSocketHandle* handle) {
+  base_->CancelRequest(group_name, handle);
+}
+
+void TCPClientSocketPool::ReleaseSocket(
+    const std::string& group_name,
+    ClientSocket* socket) {
+  base_->ReleaseSocket(group_name, socket);
+}
+
+void TCPClientSocketPool::CloseIdleSockets() {
+  base_->CloseIdleSockets();
+}
+
+int TCPClientSocketPool::IdleSocketCountInGroup(
+    const std::string& group_name) const {
+  return base_->IdleSocketCountInGroup(group_name);
+}
+
+LoadState TCPClientSocketPool::GetLoadState(
+    const std::string& group_name, const ClientSocketHandle* handle) const {
+  return base_->GetLoadState(group_name, handle);
+}
+
 }  // namespace net
diff --git a/net/base/tcp_client_socket_pool.h b/net/base/tcp_client_socket_pool.h
index 2d00d9d..1e18b2e 100644
--- a/net/base/tcp_client_socket_pool.h
+++ b/net/base/tcp_client_socket_pool.h
@@ -18,7 +18,7 @@
 namespace net {
 
 class ClientSocketFactory;
-class TCPClientSocketPool;
+class ClientSocketPoolBase;
 
 // ConnectingSocket handles the host resolution necessary for socket creation
 // and the socket Connect().
@@ -28,7 +28,7 @@ class ConnectingSocket {
                    const HostResolver::RequestInfo& resolve_info,
                    const ClientSocketHandle* handle,
                    ClientSocketFactory* client_socket_factory,
-                   TCPClientSocketPool* pool);
+                   ClientSocketPoolBase* pool);
   ~ConnectingSocket();
 
   // Begins the host resolution and the TCP connect.  Returns OK on success
@@ -58,7 +58,7 @@ class ConnectingSocket {
   ClientSocketFactory* const client_socket_factory_;
   CompletionCallbackImpl<ConnectingSocket> callback_;
   scoped_ptr<ClientSocket> socket_;
-  scoped_refptr<TCPClientSocketPool> pool_;
+  ClientSocketPoolBase* const pool_;
   SingleRequestHostResolver resolver_;
   AddressList addresses_;
 
@@ -68,10 +68,10 @@ class ConnectingSocket {
   DISALLOW_COPY_AND_ASSIGN(ConnectingSocket);
 };
 
-// A TCPClientSocketPool is used to restrict the number of TCP sockets open at
+// A ClientSocketPoolBase is used to restrict the number of sockets open at
 // a time.  It also maintains a list of idle persistent sockets.
 //
-class TCPClientSocketPool : public ClientSocketPool {
+class ClientSocketPoolBase : public base::RefCounted<ClientSocketPoolBase> {
  public:
   // A Request is allocated per call to RequestSocket that results in
   // ERR_IO_PENDING.
@@ -95,38 +95,38 @@ class TCPClientSocketPool : public ClientSocketPool {
     LoadState load_state;
   };
 
-  TCPClientSocketPool(int max_sockets_per_group,
-                      HostResolver* host_resolver,
-                      ClientSocketFactory* client_socket_factory);
+  ClientSocketPoolBase(int max_sockets_per_group,
+                       HostResolver* host_resolver,
+                       ClientSocketFactory* client_socket_factory);
 
-  // ClientSocketPool methods:
+  ~ClientSocketPoolBase();
 
-  virtual int RequestSocket(const std::string& group_name,
-                            const HostResolver::RequestInfo& resolve_info,
-                            int priority,
-                            ClientSocketHandle* handle,
-                            CompletionCallback* callback);
+  int RequestSocket(const std::string& group_name,
+                    const HostResolver::RequestInfo& resolve_info,
+                    int priority,
+                    ClientSocketHandle* handle,
+                    CompletionCallback* callback);
 
-  virtual void CancelRequest(const std::string& group_name,
-                             const ClientSocketHandle* handle);
+  void CancelRequest(const std::string& group_name,
+                     const ClientSocketHandle* handle);
 
-  virtual void ReleaseSocket(const std::string& group_name,
-                             ClientSocket* socket);
+  void ReleaseSocket(const std::string& group_name,
+                     ClientSocket* socket);
 
-  virtual void CloseIdleSockets();
+  void CloseIdleSockets();
 
-  virtual HostResolver* GetHostResolver() const {
+  HostResolver* GetHostResolver() const {
     return host_resolver_;
   }
 
-  virtual int idle_socket_count() const {
+  int idle_socket_count() const {
     return idle_socket_count_;
   }
 
-  virtual int IdleSocketCountInGroup(const std::string& group_name) const;
+  int IdleSocketCountInGroup(const std::string& group_name) const;
 
-  virtual LoadState GetLoadState(const std::string& group_name,
-                                 const ClientSocketHandle* handle) const;
+  LoadState GetLoadState(const std::string& group_name,
+                         const ClientSocketHandle* handle) const;
 
   // Used by ConnectingSocket until we remove the coupling between a specific
   // ConnectingSocket and a ClientSocketHandle:
@@ -184,8 +184,6 @@ class TCPClientSocketPool : public ClientSocketPool {
   typedef std::map<const ClientSocketHandle*, ConnectingSocket*>
       ConnectingSocketMap;
 
-  virtual ~TCPClientSocketPool();
-
   static void InsertRequestIntoQueue(const Request& r,
                                      RequestQueue* pending_requests);
 
@@ -218,7 +216,7 @@ class TCPClientSocketPool : public ClientSocketPool {
 
   // Timer used to periodically prune idle sockets that timed out or can't be
   // reused.
-  base::RepeatingTimer<TCPClientSocketPool> timer_;
+  base::RepeatingTimer<ClientSocketPoolBase> timer_;
 
   // The total number of idle sockets in the system.
   int idle_socket_count_;
@@ -228,7 +226,55 @@ class TCPClientSocketPool : public ClientSocketPool {
 
   // The host resolver that will be used to do DNS lookups for connecting
   // sockets.
-  HostResolver* host_resolver_;
+  HostResolver* const host_resolver_;
+
+  DISALLOW_COPY_AND_ASSIGN(ClientSocketPoolBase);
+};
+
+class TCPClientSocketPool : public ClientSocketPool {
+ public:
+  TCPClientSocketPool(int max_sockets_per_group,
+                      HostResolver* host_resolver,
+                      ClientSocketFactory* client_socket_factory);
+
+  // ClientSocketPool methods:
+
+  virtual int RequestSocket(const std::string& group_name,
+                            const HostResolver::RequestInfo& resolve_info,
+                            int priority,
+                            ClientSocketHandle* handle,
+                            CompletionCallback* callback);
+
+  virtual void CancelRequest(const std::string& group_name,
+                             const ClientSocketHandle* handle);
+
+  virtual void ReleaseSocket(const std::string& group_name,
+                             ClientSocket* socket);
+
+  virtual void CloseIdleSockets();
+
+  virtual HostResolver* GetHostResolver() const {
+    return base_->GetHostResolver();
+  }
+
+  virtual int IdleSocketCount() const {
+    return base_->idle_socket_count();
+  }
+
+  virtual int IdleSocketCountInGroup(const std::string& group_name) const;
+
+  virtual LoadState GetLoadState(const std::string& group_name,
+                                 const ClientSocketHandle* handle) const;
+
+ private:
+  virtual ~TCPClientSocketPool();
+
+  // One might ask why ClientSocketPoolBase is also refcounted if its
+  // containing ClientSocketPool is already refcounted.  The reason is because
+  // DoReleaseSocket() posts a task.  If ClientSocketPool gets deleted between
+  // the posting of the task and the execution, then we'll hit the DCHECK that
+  // |ClientSocketPoolBase::group_map_| is empty.
+  scoped_refptr<ClientSocketPoolBase> base_;
 
   DISALLOW_COPY_AND_ASSIGN(TCPClientSocketPool);
 };
diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc
index 7026639..bbb022b 100644
--- a/net/http/http_network_transaction_unittest.cc
+++ b/net/http/http_network_transaction_unittest.cc
@@ -1832,11 +1832,11 @@ TEST_F(HttpNetworkTransactionTest, DontRecycleTCPSocketForSSLTunnel) {
 
   // We now check to make sure the TCPClientSocket was not added back to
   // the pool.
-  EXPECT_EQ(0, session->connection_pool()->idle_socket_count());
+  EXPECT_EQ(0, session->connection_pool()->IdleSocketCount());
   trans.reset();
   MessageLoop::current()->RunAllPending();
   // Make sure that the socket didn't get recycled after calling the destructor.
-  EXPECT_EQ(0, session->connection_pool()->idle_socket_count());
+  EXPECT_EQ(0, session->connection_pool()->IdleSocketCount());
 }
 
 // Make sure that we recycle a socket after reading all of the response body.
@@ -1880,7 +1880,7 @@ TEST_F(HttpNetworkTransactionTest, RecycleSocket) {
   std::string status_line = response->headers->GetStatusLine();
   EXPECT_EQ("HTTP/1.1 200 OK", status_line);
 
-  EXPECT_EQ(0, session->connection_pool()->idle_socket_count());
+  EXPECT_EQ(0, session->connection_pool()->IdleSocketCount());
 
   std::string response_data;
   rv = ReadTransaction(trans.get(), &response_data);
@@ -1892,7 +1892,7 @@ TEST_F(HttpNetworkTransactionTest, RecycleSocket) {
   MessageLoop::current()->RunAllPending();
 
   // We now check to make sure the socket was added back to the pool.
-  EXPECT_EQ(1, session->connection_pool()->idle_socket_count());
+  EXPECT_EQ(1, session->connection_pool()->IdleSocketCount());
 }
 
 // Make sure that we recycle a socket after a zero-length response.
@@ -1938,7 +1938,7 @@ TEST_F(HttpNetworkTransactionTest, RecycleSocketAfterZeroContentLength) {
   std::string status_line = response->headers->GetStatusLine();
   EXPECT_EQ("HTTP/1.1 204 No Content", status_line);
 
-  EXPECT_EQ(0, session->connection_pool()->idle_socket_count());
+  EXPECT_EQ(0, session->connection_pool()->IdleSocketCount());
 
   std::string response_data;
   rv = ReadTransaction(trans.get(), &response_data);
@@ -1950,7 +1950,7 @@ TEST_F(HttpNetworkTransactionTest, RecycleSocketAfterZeroContentLength) {
   MessageLoop::current()->RunAllPending();
 
   // We now check to make sure the socket was added back to the pool.
-  EXPECT_EQ(1, session->connection_pool()->idle_socket_count());
+  EXPECT_EQ(1, session->connection_pool()->IdleSocketCount());
 }
 
 TEST_F(HttpNetworkTransactionTest, ResendRequestOnWriteBodyError) {