Refactor ConnectingSocket out of TCPClientSocketPool.

BUG=http://crbug.com/13289
TEST=covered by existing tcp_client_socket_pool_unittest.cc tests.

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

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

2 files changed, 182 insertions, 111 deletions

diff --git a/net/base/tcp_client_socket_pool.cc b/net/base/tcp_client_socket_pool.cc
index fa8117c..6b6044f 100644
--- a/net/base/tcp_client_socket_pool.cc
+++ b/net/base/tcp_client_socket_pool.cc
@@ -33,64 +33,60 @@ const int kIdleTimeout = 300;  // 5 minutes.
 
 namespace net {
 
-TCPClientSocketPool::ConnectingSocket::ConnectingSocket(
+ConnectingSocket::ConnectingSocket(
     const std::string& group_name,
+    const HostResolver::RequestInfo& resolve_info,
     const ClientSocketHandle* handle,
     ClientSocketFactory* client_socket_factory,
     TCPClientSocketPool* pool)
     : group_name_(group_name),
+      resolve_info_(resolve_info),
       handle_(handle),
       client_socket_factory_(client_socket_factory),
       ALLOW_THIS_IN_INITIALIZER_LIST(
           callback_(this,
-                    &TCPClientSocketPool::ConnectingSocket::OnIOComplete)),
+                    &ConnectingSocket::OnIOComplete)),
       pool_(pool),
       resolver_(pool->GetHostResolver()) {}
 
-TCPClientSocketPool::ConnectingSocket::~ConnectingSocket() {
+ConnectingSocket::~ConnectingSocket() {
   // We don't worry about cancelling the host resolution and TCP connect, since
   // ~SingleRequestHostResolver and ~ClientSocket will take care of it.
 }
 
-int TCPClientSocketPool::ConnectingSocket::Connect(
-    const HostResolver::RequestInfo& resolve_info) {
-  int rv = resolver_.Resolve(resolve_info, &addresses_, &callback_);
+int ConnectingSocket::Connect() {
+  int rv = resolver_.Resolve(resolve_info_, &addresses_, &callback_);
   if (rv != ERR_IO_PENDING)
     rv = OnIOCompleteInternal(rv, true /* synchronous */);
   return rv;
 }
 
-void TCPClientSocketPool::ConnectingSocket::OnIOComplete(int result) {
+void ConnectingSocket::OnIOComplete(int result) {
   OnIOCompleteInternal(result, false /* asynchronous */);
 }
 
-int TCPClientSocketPool::ConnectingSocket::OnIOCompleteInternal(
+int ConnectingSocket::OnIOCompleteInternal(
     int result, bool synchronous) {
   CHECK(result != ERR_IO_PENDING);
 
-  GroupMap::iterator group_it = pool_->group_map_.find(group_name_);
-  CHECK(group_it != pool_->group_map_.end());
-
-  Group& group = group_it->second;
-
-  RequestMap* request_map = &group.connecting_requests;
-  RequestMap::iterator it = request_map->find(handle_);
-  CHECK(it != request_map->end());
+  TCPClientSocketPool::Request* request = pool_->GetConnectingRequest(
+      group_name_, handle_);
+  CHECK(request);
 
-  if (result == OK && it->second.load_state == LOAD_STATE_RESOLVING_HOST) {
-    it->second.load_state = LOAD_STATE_CONNECTING;
+  if (result == OK && request->load_state == LOAD_STATE_RESOLVING_HOST) {
+    request->load_state = LOAD_STATE_CONNECTING;
     socket_.reset(client_socket_factory_->CreateTCPClientSocket(addresses_));
-    connect_start_time_ = base::Time::Now();
+    connect_start_time_ = base::TimeTicks::Now();
     result = socket_->Connect(&callback_);
     if (result == ERR_IO_PENDING)
       return result;
   }
 
   if (result == OK) {
-    CHECK(it->second.load_state == LOAD_STATE_CONNECTING);
-    CHECK(connect_start_time_ != base::Time());
+    CHECK(request->load_state == LOAD_STATE_CONNECTING);
+    CHECK(connect_start_time_ != base::TimeTicks());
     base::TimeDelta connect_duration =
-        base::Time::Now() - connect_start_time_;
+        base::TimeTicks::Now() - connect_start_time_;
 
     UMA_HISTOGRAM_CLIPPED_TIMES("Net.TCP_Connection_Latency",
         connect_duration,
@@ -102,27 +98,28 @@ int TCPClientSocketPool::ConnectingSocket::OnIOCompleteInternal(
   // Now, we either succeeded at Connect()'ing, or we failed at host resolution
   // or Connect()'ing.  Either way, we'll run the callback to alert the client.
 
-  Request request = it->second;
-  request_map->erase(it);
+  CompletionCallback* callback = NULL;
 
   if (result == OK) {
-    request.handle->set_socket(socket_.release());
-    request.handle->set_is_reused(false);
+    callback = pool_->OnConnectingRequestComplete(
+        group_name_,
+        handle_,
+        false /* don't deactivate socket */,
+        socket_.release());
   } else {
-    group.active_socket_count--;
-
-    // Delete group if no longer needed.
-    if (group.active_socket_count == 0 && group.idle_sockets.empty()) {
-      CHECK(group.pending_requests.empty());
-      CHECK(group.connecting_requests.empty());
-      pool_->group_map_.erase(group_it);
-    }
+    callback = pool_->OnConnectingRequestComplete(
+        group_name_,
+        handle_,
+        true /* deactivate socket */,
+        NULL /* no connected socket to give */);
   }
 
-  pool_->RemoveConnectingSocket(handle_);  // will delete |this|.
+  // |this| is deleted after this point.
+
+  CHECK(callback);
 
   if (!synchronous)
-    request.callback->Run(result);
+    callback->Run(result);
   return result;
 }
 
@@ -202,9 +199,10 @@ int TCPClientSocketPool::RequestSocket(
   CHECK(!ContainsKey(connecting_socket_map_, handle));
 
   ConnectingSocket* connecting_socket =
-      new ConnectingSocket(group_name, handle, client_socket_factory_, this);
+      new ConnectingSocket(group_name, resolve_info, handle,
+                           client_socket_factory_, this);
   connecting_socket_map_[handle] = connecting_socket;
-  int rv = connecting_socket->Connect(resolve_info);
+  int rv = connecting_socket->Connect();
   return rv;
 }
 
@@ -392,6 +390,59 @@ void TCPClientSocketPool::DoReleaseSocket(const std::string& group_name,
   }
 }
 
+TCPClientSocketPool::Request* TCPClientSocketPool::GetConnectingRequest(
+    const std::string& group_name, const ClientSocketHandle* handle) {
+  GroupMap::iterator group_it = group_map_.find(group_name);
+  if (group_it == group_map_.end())
+    return NULL;
+
+  Group& group = group_it->second;
+
+  RequestMap* request_map = &group.connecting_requests;
+  RequestMap::iterator it = request_map->find(handle);
+  if (it == request_map->end())
+    return NULL;
+
+  return &it->second;
+}
+
+CompletionCallback* TCPClientSocketPool::OnConnectingRequestComplete(
+    const std::string& group_name,
+    const ClientSocketHandle* handle,
+    bool deactivate,
+    ClientSocket* socket) {
+  CHECK((deactivate && !socket) || (!deactivate && socket));
+  GroupMap::iterator group_it = group_map_.find(group_name);
+  CHECK(group_it != group_map_.end());
+  Group& group = group_it->second;
+
+  RequestMap* request_map = &group.connecting_requests;
+
+  RequestMap::iterator it = request_map->find(handle);
+  CHECK(it != request_map->end());
+  Request request = it->second;
+  request_map->erase(it);
+  DCHECK_EQ(request.handle, handle);
+
+  if (deactivate) {
+    group.active_socket_count--;
+
+    // Delete group if no longer needed.
+    if (group.active_socket_count == 0 && group.idle_sockets.empty()) {
+      DCHECK(group.pending_requests.empty());
+      DCHECK(group.connecting_requests.empty());
+      group_map_.erase(group_name);
+    }
+  } else {
+    request.handle->set_socket(socket);
+    request.handle->set_is_reused(false);
+  }
+
+  RemoveConnectingSocket(request.handle);
+
+  return request.callback;
+}
+
 void TCPClientSocketPool::RemoveConnectingSocket(
     const ClientSocketHandle* handle) {
   ConnectingSocketMap::iterator it = connecting_socket_map_.find(handle);
diff --git a/net/base/tcp_client_socket_pool.h b/net/base/tcp_client_socket_pool.h
index 33d2a5b..2d00d9d 100644
--- a/net/base/tcp_client_socket_pool.h
+++ b/net/base/tcp_client_socket_pool.h
@@ -18,12 +18,83 @@
 namespace net {
 
 class ClientSocketFactory;
+class TCPClientSocketPool;
+
+// ConnectingSocket handles the host resolution necessary for socket creation
+// and the socket Connect().
+class ConnectingSocket {
+ public:
+  ConnectingSocket(const std::string& group_name,
+                   const HostResolver::RequestInfo& resolve_info,
+                   const ClientSocketHandle* handle,
+                   ClientSocketFactory* client_socket_factory,
+                   TCPClientSocketPool* pool);
+  ~ConnectingSocket();
+
+  // Begins the host resolution and the TCP connect.  Returns OK on success
+  // and ERR_IO_PENDING if it cannot immediately service the request.
+  // Otherwise, it returns a net error code.
+  int Connect();
+
+  // Called by the TCPClientSocketPool to cancel this ConnectingSocket.  Only
+  // necessary if a ClientSocketHandle is reused.
+  void Cancel();
+
+ private:
+  // Handles asynchronous completion of IO.  |result| represents the result of
+  // the IO operation.
+  void OnIOComplete(int result);
+
+  // Handles both asynchronous and synchronous completion of IO.  |result|
+  // represents the result of the IO operation.  |synchronous| indicates
+  // whether or not the previous IO operation completed synchronously or
+  // asynchronously.  OnIOCompleteInternal returns the result of the next IO
+  // operation that executes, or just the value of |result|.
+  int OnIOCompleteInternal(int result, bool synchronous);
+
+  const std::string group_name_;
+  const HostResolver::RequestInfo resolve_info_;
+  const ClientSocketHandle* const handle_;
+  ClientSocketFactory* const client_socket_factory_;
+  CompletionCallbackImpl<ConnectingSocket> callback_;
+  scoped_ptr<ClientSocket> socket_;
+  scoped_refptr<TCPClientSocketPool> pool_;
+  SingleRequestHostResolver resolver_;
+  AddressList addresses_;
+
+  // The time the Connect() method was called (if it got called).
+  base::TimeTicks connect_start_time_;
+
+  DISALLOW_COPY_AND_ASSIGN(ConnectingSocket);
+};
 
 // A TCPClientSocketPool is used to restrict the number of TCP sockets open at
 // a time.  It also maintains a list of idle persistent sockets.
 //
 class TCPClientSocketPool : public ClientSocketPool {
  public:
+  // A Request is allocated per call to RequestSocket that results in
+  // ERR_IO_PENDING.
+  struct Request {
+    // HostResolver::RequestInfo has no default constructor, so fudge something.
+    Request() : resolve_info(std::string(), 0) {}
+
+    Request(ClientSocketHandle* handle,
+            CompletionCallback* callback,
+            int priority,
+            const HostResolver::RequestInfo& resolve_info,
+            LoadState load_state)
+        : handle(handle), callback(callback), priority(priority),
+          resolve_info(resolve_info), load_state(load_state) {
+    }
+
+    ClientSocketHandle* handle;
+    CompletionCallback* callback;
+    int priority;
+    HostResolver::RequestInfo resolve_info;
+    LoadState load_state;
+  };
+
   TCPClientSocketPool(int max_sockets_per_group,
                       HostResolver* host_resolver,
                       ClientSocketFactory* client_socket_factory);
@@ -57,29 +128,29 @@ class TCPClientSocketPool : public ClientSocketPool {
   virtual LoadState GetLoadState(const std::string& group_name,
                                  const ClientSocketHandle* handle) const;
 
- private:
-  // A Request is allocated per call to RequestSocket that results in
-  // ERR_IO_PENDING.
-  struct Request {
-    // HostResolver::RequestInfo has no default constructor, so fudge something.
-    Request() : resolve_info(std::string(), 0) {}
-
-    Request(ClientSocketHandle* handle,
-            CompletionCallback* callback,
-            int priority,
-            const HostResolver::RequestInfo& resolve_info,
-            LoadState load_state)
-        : handle(handle), callback(callback), priority(priority),
-          resolve_info(resolve_info), load_state(load_state) {
-    }
-
-    ClientSocketHandle* handle;
-    CompletionCallback* callback;
-    int priority;
-    HostResolver::RequestInfo resolve_info;
-    LoadState load_state;
-  };
+  // Used by ConnectingSocket until we remove the coupling between a specific
+  // ConnectingSocket and a ClientSocketHandle:
+ 
+  // Returns NULL if not found.  Otherwise it returns the Request*
+  // corresponding to the ConnectingSocket (keyed by |group_name| and |handle|.
+  // Note that this pointer may be invalidated after any call that might mutate
+  // the RequestMap or GroupMap, so the user should not hold onto the pointer
+  // for long.
+  Request* GetConnectingRequest(const std::string& group_name,
+                                const ClientSocketHandle* handle);
+ 
+  // Handles the completed Request corresponding to the ConnectingSocket (keyed
+  // by |group_name| and |handle|.  |deactivate| indicates whether or not to
+  // deactivate the socket, making the socket slot available for a new socket
+  // connection.  If |deactivate| is false, then set |socket| into |handle|.
+  // Returns the callback to run.
+  CompletionCallback* OnConnectingRequestComplete(
+      const std::string& group_name,
+      const ClientSocketHandle* handle,
+      bool deactivate,
+      ClientSocket* socket);
 
+ private:
   // Entry for a persistent socket which became idle at time |start_time|.
   struct IdleSocket {
     ClientSocket* socket;
@@ -110,57 +181,6 @@ class TCPClientSocketPool : public ClientSocketPool {
 
   typedef std::map<std::string, Group> GroupMap;
 
-  // ConnectingSocket handles the host resolution necessary for socket creation
-  // and the Connect().
-  class ConnectingSocket {
-   public:
-    enum State {
-      STATE_RESOLVE_HOST,
-      STATE_CONNECT
-    };
-
-    ConnectingSocket(const std::string& group_name,
-                     const ClientSocketHandle* handle,
-                     ClientSocketFactory* client_socket_factory,
-                     TCPClientSocketPool* pool);
-    ~ConnectingSocket();
-
-    // Begins the host resolution and the TCP connect.  Returns OK on success
-    // and ERR_IO_PENDING if it cannot immediately service the request.
-    // Otherwise, it returns a net error code.
-    int Connect(const HostResolver::RequestInfo& resolve_info);
-
-    // Called by the TCPClientSocketPool to cancel this ConnectingSocket.  Only
-    // necessary if a ClientSocketHandle is reused.
-    void Cancel();
-
-   private:
-    // Handles asynchronous completion of IO.  |result| represents the result of
-    // the IO operation.
-    void OnIOComplete(int result);
-
-    // Handles both asynchronous and synchronous completion of IO.  |result|
-    // represents the result of the IO operation.  |synchronous| indicates
-    // whether or not the previous IO operation completed synchronously or
-    // asynchronously.  OnIOCompleteInternal returns the result of the next IO
-    // operation that executes, or just the value of |result|.
-    int OnIOCompleteInternal(int result, bool synchronous);
-
-    const std::string group_name_;
-    const ClientSocketHandle* const handle_;
-    ClientSocketFactory* const client_socket_factory_;
-    CompletionCallbackImpl<ConnectingSocket> callback_;
-    scoped_ptr<ClientSocket> socket_;
-    scoped_refptr<TCPClientSocketPool> pool_;
-    SingleRequestHostResolver resolver_;
-    AddressList addresses_;
-
-    // The time the Connect() method was called (if it got called).
-    base::Time connect_start_time_;
-
-    DISALLOW_COPY_AND_ASSIGN(ConnectingSocket);
-  };
-
   typedef std::map<const ClientSocketHandle*, ConnectingSocket*>
       ConnectingSocketMap;