Refactor ConnectJob and TCPConnectJob.

ConnectJob will only call OnConnectJobComplete() on asynchronous completion.
TCPConnectJob now uses DoLoop() internally.
BUG=http://crbug.com/13289
TEST=none

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

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

5 files changed, 291 insertions, 163 deletions

diff --git a/net/socket/client_socket_pool_base.cc b/net/socket/client_socket_pool_base.cc
index 123c3cf..56eb1ab 100644
--- a/net/socket/client_socket_pool_base.cc
+++ b/net/socket/client_socket_pool_base.cc
@@ -30,6 +30,20 @@ const int kIdleTimeout = 300;  // 5 minutes.
 
 namespace net {
 
+ConnectJob::ConnectJob(const std::string& group_name,
+                       const ClientSocketHandle* key_handle,
+                       Delegate* delegate)
+    : group_name_(group_name),
+      key_handle_(key_handle),
+      delegate_(delegate),
+      load_state_(LOAD_STATE_IDLE) {
+  DCHECK(!group_name.empty());
+  DCHECK(key_handle);
+  DCHECK(delegate);
+}
+
+ConnectJob::~ConnectJob() {}
+
 ClientSocketPoolBase::ClientSocketPoolBase(
     int max_sockets_per_group,
     ConnectJobFactory* connect_job_factory)
@@ -70,29 +84,21 @@ int ClientSocketPoolBase::RequestSocket(
   DCHECK(callback);
   Group& group = group_map_[group_name];
 
-  CheckSocketCounts(group);
-
   // Can we make another active socket now?
-  if (group.active_socket_count == max_sockets_per_group_) {
+  if (!group.HasAvailableSocketSlot(max_sockets_per_group_)) {
     CHECK(callback);
     Request r(handle, callback, priority, resolve_info);
     InsertRequestIntoQueue(r, &group.pending_requests);
     return ERR_IO_PENDING;
   }
 
-  // OK, we are going to activate one.
-  group.active_socket_count++;
-
   while (!group.idle_sockets.empty()) {
     IdleSocket idle_socket = group.idle_sockets.back();
     group.idle_sockets.pop_back();
     DecrementIdleCount();
     if (idle_socket.socket->IsConnectedAndIdle()) {
       // We found one we can reuse!
-      handle->set_socket(idle_socket.socket);
-      handle->set_is_reused(true);
-      group.sockets_handed_out_count++;
-      CheckSocketCounts(group);
+      HandOutSocket(idle_socket.socket, true /* reuse */, handle, &group);
       return OK;
     }
     delete idle_socket.socket;
@@ -102,14 +108,23 @@ int ClientSocketPoolBase::RequestSocket(
 
   CHECK(callback);
   Request r(handle, callback, priority, resolve_info);
-  group.connecting_requests[handle] = r;
-
-  CHECK(!ContainsKey(connect_job_map_, handle));
+  scoped_ptr<ConnectJob> connect_job(
+      connect_job_factory_->NewConnectJob(group_name, r, this));
+
+  int rv = connect_job->Connect();
+  if (rv == OK) {
+    HandOutSocket(connect_job->ReleaseSocket(), false /* not reused */,
+                  handle, &group);
+  } else if (rv == ERR_IO_PENDING) {
+    group.connecting_requests[handle] = r;
+    CHECK(!ContainsKey(connect_job_map_, handle));
+    connect_job_map_[handle] = connect_job.release();
+  } else {
+    if (group.IsEmpty())
+      group_map_.erase(group_name);
+  }
 
-  ConnectJob* connect_job =
-      connect_job_factory_->NewConnectJob(group_name, r, this);
-  connect_job_map_[handle] = connect_job;
-  return connect_job->Connect();
+  return rv;
 }
 
 void ClientSocketPoolBase::CancelRequest(const std::string& group_name,
@@ -118,8 +133,6 @@ void ClientSocketPoolBase::CancelRequest(const std::string& group_name,
 
   Group& group = group_map_[group_name];
 
-  CheckSocketCounts(group);
-
   // Search pending_requests for matching handle.
   RequestQueue::iterator it = group.pending_requests.begin();
   for (; it != group.pending_requests.end(); ++it) {
@@ -136,7 +149,7 @@ void ClientSocketPoolBase::CancelRequest(const std::string& group_name,
   if (map_it != group.connecting_requests.end()) {
     RemoveConnectJob(handle);
     group.connecting_requests.erase(map_it);
-    RemoveActiveSocket(group_name, &group);
+    OnAvailableSocketSlot(group_name, &group);
   }
 }
 
@@ -229,9 +242,8 @@ void ClientSocketPoolBase::CleanupIdleSockets(bool force) {
     }
 
     // Delete group if no longer needed.
-    if (group.active_socket_count == 0 && group.idle_sockets.empty()) {
+    if (group.IsEmpty()) {
       CHECK(group.pending_requests.empty());
-      CHECK(group.connecting_requests.empty());
       group_map_.erase(i++);
     } else {
       ++i;
@@ -258,9 +270,7 @@ void ClientSocketPoolBase::DoReleaseSocket(const std::string& group_name,
   Group& group = i->second;
 
   CHECK(group.active_socket_count > 0);
-  CheckSocketCounts(group);
-
-  group.sockets_handed_out_count--;
+  group.active_socket_count--;
 
   const bool can_reuse = socket->IsConnectedAndIdle();
   if (can_reuse) {
@@ -274,53 +284,37 @@ void ClientSocketPoolBase::DoReleaseSocket(const std::string& group_name,
     delete socket;
   }
 
-  RemoveActiveSocket(group_name, &group);
+  OnAvailableSocketSlot(group_name, &group);
 }
 
-void ClientSocketPoolBase::OnConnectJobComplete(
-    const std::string& group_name,
-    const ClientSocketHandle* key_handle,
-    ClientSocket* socket,
-    int result,
-    bool was_async) {
+void ClientSocketPoolBase::OnConnectJobComplete(int result, ConnectJob* job) {
+  DCHECK_NE(ERR_IO_PENDING, result);
+  const std::string group_name = job->group_name();
   GroupMap::iterator group_it = group_map_.find(group_name);
   CHECK(group_it != group_map_.end());
   Group& group = group_it->second;
 
-  CheckSocketCounts(group);
-
   RequestMap* request_map = &group.connecting_requests;
 
-  RequestMap::iterator it = request_map->find(key_handle);
+  RequestMap::iterator it = request_map->find(job->key_handle());
   CHECK(it != request_map->end());
-  Request request = it->second;
+  ClientSocketHandle* const handle = it->second.handle;
+  CompletionCallback* const callback = it->second.callback;
   request_map->erase(it);
-  DCHECK_EQ(request.handle, key_handle);
+  DCHECK_EQ(handle, job->key_handle());
 
-  if (!socket) {
-    RemoveActiveSocket(group_name, &group);
-  } else {
-    request.handle->set_socket(socket);
-    request.handle->set_is_reused(false);
-    group.sockets_handed_out_count++;
+  ClientSocket* const socket = job->ReleaseSocket();
+  RemoveConnectJob(job->key_handle());
 
-    CheckSocketCounts(group);
+  if (result != OK) {
+    callback->Run(result);  // |group| is not necessarily valid after this.
+    // |group| may be invalid after the callback, we need to search
+    // |group_map_| again.
+    MaybeOnAvailableSocketSlot(group_name);
+  } else {
+    HandOutSocket(socket, false /* not reused */, handle, &group);
+    callback->Run(result);
   }
-
-  RemoveConnectJob(request.handle);
-
-  if (was_async)
-    request.callback->Run(result);
-}
-
-// static
-void ClientSocketPoolBase::CheckSocketCounts(const Group& group) {
-  CHECK(group.active_socket_count ==
-        group.sockets_handed_out_count +
-        static_cast<int>(group.connecting_requests.size()))
-      << "[active_socket_count: " << group.active_socket_count
-      << " ] [sockets_handed_out_count: " << group.sockets_handed_out_count
-      << " ] [connecting_requests size: " << group.connecting_requests.size();
 }
 
 void ClientSocketPoolBase::RemoveConnectJob(
@@ -331,20 +325,25 @@ void ClientSocketPoolBase::RemoveConnectJob(
   connect_job_map_.erase(it);
 }
 
-void ClientSocketPoolBase::RemoveActiveSocket(const std::string& group_name,
-                                              Group* group) {
-  group->active_socket_count--;
+void ClientSocketPoolBase::MaybeOnAvailableSocketSlot(
+    const std::string& group_name) {
+  GroupMap::iterator it = group_map_.find(group_name);
+  if (it != group_map_.end()) {
+    Group& group = it->second;
+    if (group.HasAvailableSocketSlot(max_sockets_per_group_))
+      OnAvailableSocketSlot(group_name, &group);
+  }
+}
 
+void ClientSocketPoolBase::OnAvailableSocketSlot(const std::string& group_name,
+                                                 Group* group) {
   if (!group->pending_requests.empty()) {
     ProcessPendingRequest(group_name, group);
     // |group| may no longer be valid after this point.  Be careful not to
     // access it again.
-  } else if (group->active_socket_count == 0 && group->idle_sockets.empty()) {
+  } else if (group->IsEmpty()) {
     // Delete |group| if no longer needed.  |group| will no longer be valid.
-    DCHECK(group->connecting_requests.empty());
     group_map_.erase(group_name);
-  } else {
-    CheckSocketCounts(*group);
   }
 }
 
@@ -356,10 +355,25 @@ void ClientSocketPoolBase::ProcessPendingRequest(const std::string& group_name,
   int rv = RequestSocket(
       group_name, r.resolve_info, r.priority, r.handle, r.callback);
 
-  // |group| may be invalid after RequestSocket.
-
-  if (rv != ERR_IO_PENDING)
+  if (rv != ERR_IO_PENDING) {
     r.callback->Run(rv);
+    if (rv != OK) {
+      // |group| may be invalid after the callback, we need to search
+      // |group_map_| again.
+      MaybeOnAvailableSocketSlot(group_name);
+    }
+  }
+}
+
+void ClientSocketPoolBase::HandOutSocket(
+    ClientSocket* socket,
+    bool reused,
+    ClientSocketHandle* handle,
+    Group* group) {
+  DCHECK(socket);
+  handle->set_socket(socket);
+  handle->set_is_reused(reused);
+  group->active_socket_count++;
 }
 
 }  // namespace net
diff --git a/net/socket/client_socket_pool_base.h b/net/socket/client_socket_pool_base.h
index 6d05b41..8c4b3e7 100644
--- a/net/socket/client_socket_pool_base.h
+++ b/net/socket/client_socket_pool_base.h
@@ -17,11 +17,11 @@
 #include "net/base/completion_callback.h"
 #include "net/base/host_resolver.h"
 #include "net/base/load_states.h"
+#include "net/socket/client_socket.h"
 #include "net/socket/client_socket_pool.h"
 
 namespace net {
 
-class ClientSocket;
 class ClientSocketHandle;
 class ClientSocketPoolBase;
 
@@ -35,40 +35,47 @@ class ConnectJob {
     Delegate() {}
     virtual ~Delegate() {}
 
-    // Alerts the delegate that the connection completed (though not necessarily
-    // successfully).  |group_name| indicates the connection group this
-    // ConnectJob corresponds to.  |key_handle| uniquely identifies the
-    // ClientSocketHandle that this job is coupled to.  |socket| is non-NULL if
-    // the connection completed successfully, and ownership is transferred to
-    // the delegate.  |was_async| indicates whether or not the connect job
-    // completed asynchronously.
-    virtual void OnConnectJobComplete(
-        const std::string& group_name,
-        const ClientSocketHandle* key_handle,
-        ClientSocket* socket,
-        int result,
-        bool was_async) = 0;
+    // Alerts the delegate that the connection completed.
+    virtual void OnConnectJobComplete(int result, ConnectJob* job) = 0;
 
    private:
     DISALLOW_COPY_AND_ASSIGN(Delegate);
   };
 
-  ConnectJob() {}
-  virtual ~ConnectJob() {}
+  ConnectJob(const std::string& group_name,
+             const ClientSocketHandle* key_handle,
+             Delegate* delegate);
+  virtual ~ConnectJob();
 
-  // Returns the LoadState of this ConnectJob.
+  // Accessors
+  const std::string& group_name() const { return group_name_; }
   LoadState load_state() const { return load_state_; }
+  const ClientSocketHandle* key_handle() const { return key_handle_; }
+
+  // Releases |socket_| to the client.
+  ClientSocket* ReleaseSocket() { return socket_.release(); }
 
   // Begins connecting the socket.  Returns OK on success, ERR_IO_PENDING if it
   // cannot complete synchronously without blocking, or another net error code
-  // on error.
+  // on error.  In asynchronous completion, the ConnectJob will notify
+  // |delegate_| via OnConnectJobComplete.  In both asynchronous and synchronous
+  // completion, ReleaseSocket() can be called to acquire the connected socket
+  // if it succeeded.
   virtual int Connect() = 0;
 
  protected:
   void set_load_state(LoadState load_state) { load_state_ = load_state; }
+  void set_socket(ClientSocket* socket) { socket_.reset(socket); }
+  ClientSocket* socket() { return socket_.get(); }
+  Delegate* delegate() { return delegate_; }
 
  private:
+  const std::string group_name_;
+  // Temporarily needed until we switch to late binding.
+  const ClientSocketHandle* const key_handle_;
+  Delegate* const delegate_;
   LoadState load_state_;
+  scoped_ptr<ClientSocket> socket_;
 
   DISALLOW_COPY_AND_ASSIGN(ConnectJob);
 };
@@ -142,14 +149,7 @@ class ClientSocketPoolBase
   LoadState GetLoadState(const std::string& group_name,
                          const ClientSocketHandle* handle) const;
 
-  // If |was_async| is true, then ClientSocketPoolBase will pick a callback to
-  // run from a request associated with |group_name|.
-  virtual void OnConnectJobComplete(
-      const std::string& group_name,
-      const ClientSocketHandle* key_handle,
-      ClientSocket* socket,
-      int result,
-      bool was_async);
+  virtual void OnConnectJobComplete(int result, ConnectJob* job);
 
  private:
   // Entry for a persistent socket which became idle at time |start_time|.
@@ -173,12 +173,23 @@ class ClientSocketPoolBase
   // A Group is allocated per group_name when there are idle sockets or pending
   // requests.  Otherwise, the Group object is removed from the map.
   struct Group {
-    Group() : active_socket_count(0), sockets_handed_out_count(0) {}
+    Group() : active_socket_count(0) {}
+
+    bool IsEmpty() const {
+      return active_socket_count == 0 && idle_sockets.empty() &&
+          connecting_requests.empty();
+    }
+
+    bool HasAvailableSocketSlot(int max_sockets_per_group) const {
+      return active_socket_count +
+          static_cast<int>(connecting_requests.size()) <
+          max_sockets_per_group;
+    }
+
     std::deque<IdleSocket> idle_sockets;
     RequestQueue pending_requests;
     RequestMap connecting_requests;
-    int active_socket_count;  // number of active sockets
-    int sockets_handed_out_count;  // number of sockets given to clients
+    int active_socket_count;  // number of active sockets used by clients
   };
 
   typedef std::map<std::string, Group> GroupMap;
@@ -209,14 +220,22 @@ class ClientSocketPoolBase
   // |connect_job_map_|.
   void RemoveConnectJob(const ClientSocketHandle* handle);
 
-  static void CheckSocketCounts(const Group& group);
+  // Same as OnAvailableSocketSlot except it looks up the Group first to see if
+  // it's there.
+  void MaybeOnAvailableSocketSlot(const std::string& group_name);
 
-  // Remove an active socket.
-  void RemoveActiveSocket(const std::string& group_name, Group* group);
+  // Might delete the Group from |group_map_|.
+  void OnAvailableSocketSlot(const std::string& group_name, Group* group);
 
   // Process a request from a group's pending_requests queue.
   void ProcessPendingRequest(const std::string& group_name, Group* group);
 
+  // Assigns |socket| to |handle| and updates |group|'s counters appropriately.
+  void HandOutSocket(ClientSocket* socket,
+                     bool reused,
+                     ClientSocketHandle* handle,
+                     Group* group);
+
   GroupMap group_map_;
 
   ConnectJobMap connect_job_map_;
diff --git a/net/socket/client_socket_pool_base_unittest.cc b/net/socket/client_socket_pool_base_unittest.cc
index ef9f15f8..7bb5ae7 100644
--- a/net/socket/client_socket_pool_base_unittest.cc
+++ b/net/socket/client_socket_pool_base_unittest.cc
@@ -134,11 +134,9 @@ class TestConnectJob : public ConnectJob {
                  const ClientSocketPoolBase::Request& request,
                  ConnectJob::Delegate* delegate,
                  ClientSocketFactory* client_socket_factory)
-      : job_type_(job_type),
-        group_name_(group_name),
-        handle_(request.handle),
+      : ConnectJob(group_name, request.handle, delegate),
+        job_type_(job_type),
         client_socket_factory_(client_socket_factory),
-        delegate_(delegate),
         method_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {}
 
   // ConnectJob methods:
@@ -176,22 +174,19 @@ class TestConnectJob : public ConnectJob {
  private:
   int DoConnect(bool succeed, bool was_async) {
     int result = ERR_CONNECTION_FAILED;
-    ClientSocket* socket = NULL;
     if (succeed) {
       result = OK;
-      socket = new MockClientSocket();
-      socket->Connect(NULL);
+      set_socket(new MockClientSocket());
+      socket()->Connect(NULL);
     }
-    delegate_->OnConnectJobComplete(
-        group_name_, handle_, socket, result, was_async);
+
+    if (was_async)
+      delegate()->OnConnectJobComplete(result, this);
     return result;
   }
 
   const JobType job_type_;
-  const std::string group_name_;
-  const ClientSocketHandle* handle_;
   ClientSocketFactory* const client_socket_factory_;
-  Delegate* const delegate_;
   ScopedRunnableMethodFactory<TestConnectJob> method_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(TestConnectJob);
@@ -530,20 +525,35 @@ TEST_F(ClientSocketPoolBaseTest, CancelRequest) {
 
 class RequestSocketCallback : public CallbackRunner< Tuple1<int> > {
  public:
-  RequestSocketCallback(ClientSocketHandle* handle)
+  RequestSocketCallback(ClientSocketHandle* handle,
+                        TestConnectJobFactory* test_connect_job_factory,
+                        TestConnectJob::JobType next_job_type)
       : handle_(handle),
-        within_callback_(false) {}
+        within_callback_(false),
+        test_connect_job_factory_(test_connect_job_factory),
+        next_job_type_(next_job_type) {}
 
   virtual void RunWithParams(const Tuple1<int>& params) {
     callback_.RunWithParams(params);
     ASSERT_EQ(OK, params.a);
 
     if (!within_callback_) {
+      test_connect_job_factory_->set_job_type(next_job_type_);
       handle_->Reset();
       within_callback_ = true;
       int rv = handle_->Init(
           "a", HostResolver::RequestInfo("www.google.com", 80), 0, this);
-      EXPECT_EQ(ERR_IO_PENDING, rv);
+      switch (next_job_type_) {
+        case TestConnectJob::kMockJob:
+          EXPECT_EQ(OK, rv);
+          break;
+        case TestConnectJob::kMockPendingJob:
+          EXPECT_EQ(ERR_IO_PENDING, rv);
+          break;
+        default:
+          FAIL() << "Unexpected job type: " << next_job_type_;
+          break;
+      }
     }
   }
 
@@ -554,19 +564,34 @@ class RequestSocketCallback : public CallbackRunner< Tuple1<int> > {
  private:
   ClientSocketHandle* const handle_;
   bool within_callback_;
+  TestConnectJobFactory* const test_connect_job_factory_;
+  TestConnectJob::JobType next_job_type_;
   TestCompletionCallback callback_;
 };
 
-TEST_F(ClientSocketPoolBaseTest, RequestTwice) {
+TEST_F(ClientSocketPoolBaseTest, RequestPendingJobTwice) {
   connect_job_factory_->set_job_type(TestConnectJob::kMockPendingJob); 
   ClientSocketHandle handle(pool_.get());
-  RequestSocketCallback callback(&handle);
+  RequestSocketCallback callback(
+      &handle, connect_job_factory_, TestConnectJob::kMockPendingJob);
   int rv = handle.Init(
       "a", ignored_request_info_, 0, &callback);
   ASSERT_EQ(ERR_IO_PENDING, rv);
 
   EXPECT_EQ(OK, callback.WaitForResult());
+  handle.Reset();
+}
 
+TEST_F(ClientSocketPoolBaseTest, RequestPendingJobThenSynchronous) {
+  connect_job_factory_->set_job_type(TestConnectJob::kMockPendingJob); 
+  ClientSocketHandle handle(pool_.get());
+  RequestSocketCallback callback(
+      &handle, connect_job_factory_, TestConnectJob::kMockJob);
+  int rv = handle.Init(
+      "a", ignored_request_info_, 0, &callback);
+  ASSERT_EQ(ERR_IO_PENDING, rv);
+
+  EXPECT_EQ(OK, callback.WaitForResult());
   handle.Reset();
 }
 
@@ -615,6 +640,45 @@ TEST_F(ClientSocketPoolBaseTest, FailingActiveRequestWithPendingRequests) {
     EXPECT_EQ(ERR_CONNECTION_FAILED, reqs[i]->WaitForResult());
 }
 
+// A pending asynchronous job completes, which will free up a socket slot.  The
+// next job finishes synchronously.  The callback for the asynchronous job
+// should be first though.
+TEST_F(ClientSocketPoolBaseTest, PendingJobCompletionOrder) {
+  // First two jobs are async.
+  connect_job_factory_->set_job_type(TestConnectJob::kMockPendingFailingJob); 
+
+  // Start job 1 (async error).
+  TestSocketRequest req1(pool_.get(), &request_order_);
+  int rv = req1.handle.Init("a", ignored_request_info_, 5, &req1);
+  EXPECT_EQ(ERR_IO_PENDING, rv);
+
+  // Start job 2 (async error).
+  TestSocketRequest req2(pool_.get(), &request_order_);
+  rv = req2.handle.Init("a", ignored_request_info_, 5, &req2);
+  EXPECT_EQ(ERR_IO_PENDING, rv);
+
+  // The pending job is sync.
+  connect_job_factory_->set_job_type(TestConnectJob::kMockJob); 
+
+  // Request 3 does not have a ConnectJob yet.  It's just pending.
+  TestSocketRequest req3(pool_.get(), &request_order_);
+  rv = req3.handle.Init("a", ignored_request_info_, 5, &req3);
+  EXPECT_EQ(ERR_IO_PENDING, rv);
+
+  EXPECT_EQ(ERR_CONNECTION_FAILED, req1.WaitForResult());
+  EXPECT_EQ(ERR_CONNECTION_FAILED, req2.WaitForResult());
+  EXPECT_EQ(OK, req3.WaitForResult());
+
+  ASSERT_EQ(3U, request_order_.size());
+
+  // After job 1 finishes unsuccessfully, it will try to process the pending
+  // requests queue, so it starts up job 3 for request 3.  This job
+  // synchronously succeeds, so the request order is 1, 3, 2.
+  EXPECT_EQ(&req1, request_order_[0]);
+  EXPECT_EQ(&req2, request_order_[2]);
+  EXPECT_EQ(&req3, request_order_[1]);
+}
+
 }  // namespace
 
 }  // namespace net
diff --git a/net/socket/tcp_client_socket_pool.cc b/net/socket/tcp_client_socket_pool.cc
index c9af808..a5b5595 100644
--- a/net/socket/tcp_client_socket_pool.cc
+++ b/net/socket/tcp_client_socket_pool.cc
@@ -25,14 +25,12 @@ TCPConnectJob::TCPConnectJob(
     ClientSocketFactory* client_socket_factory,
     HostResolver* host_resolver,
     Delegate* delegate)
-    : group_name_(group_name),
+    : ConnectJob(group_name, handle, delegate),
       resolve_info_(resolve_info),
-      handle_(handle),
       client_socket_factory_(client_socket_factory),
       ALLOW_THIS_IN_INITIALIZER_LIST(
           callback_(this,
                     &TCPConnectJob::OnIOComplete)),
-      delegate_(delegate),
       resolver_(host_resolver) {}
 
 TCPConnectJob::~TCPConnectJob() {
@@ -41,33 +39,73 @@ TCPConnectJob::~TCPConnectJob() {
 }
 
 int TCPConnectJob::Connect() {
-  set_load_state(LOAD_STATE_RESOLVING_HOST);
-  int rv = resolver_.Resolve(resolve_info_, &addresses_, &callback_);
+  next_state_ = kStateResolveHost;
+  return DoLoop(OK);
+}
+
+void TCPConnectJob::OnIOComplete(int result) {
+  int rv = DoLoop(result);
   if (rv != ERR_IO_PENDING)
-    rv = OnIOCompleteInternal(rv, true /* synchronous */);
+    delegate()->OnConnectJobComplete(rv, this);  // Deletes |this|
+}
+
+int TCPConnectJob::DoLoop(int result) {
+  DCHECK_NE(next_state_, kStateNone);
+
+  int rv = result;
+  do {
+    State state = next_state_;
+    next_state_ = kStateNone;
+    switch (state) {
+      case kStateResolveHost:
+        DCHECK_EQ(OK, rv);
+        rv = DoResolveHost();
+        break;
+      case kStateResolveHostComplete:
+        rv = DoResolveHostComplete(rv);
+        break;
+      case kStateTCPConnect:
+        DCHECK_EQ(OK, rv);
+        rv = DoTCPConnect();
+        break;
+      case kStateTCPConnectComplete:
+        rv = DoTCPConnectComplete(rv);
+        break;
+      default:
+        NOTREACHED();
+        rv = ERR_FAILED;
+        break;
+    }
+  } while (rv != ERR_IO_PENDING && next_state_ != kStateNone);
+
   return rv;
 }
 
-void TCPConnectJob::OnIOComplete(int result) {
-  OnIOCompleteInternal(result, false /* asynchronous */);
+int TCPConnectJob::DoResolveHost() {
+  set_load_state(LOAD_STATE_RESOLVING_HOST);
+  next_state_ = kStateResolveHostComplete;
+  return resolver_.Resolve(resolve_info_, &addresses_, &callback_);
 }
 
-int TCPConnectJob::OnIOCompleteInternal(
-    int result, bool synchronous) {
-  CHECK(result != ERR_IO_PENDING);
-
-  if (result == OK && load_state() == LOAD_STATE_RESOLVING_HOST) {
-    set_load_state(LOAD_STATE_CONNECTING);
-    socket_.reset(client_socket_factory_->CreateTCPClientSocket(addresses_));
-    connect_start_time_ = base::TimeTicks::Now();
-    result = socket_->Connect(&callback_);
-    if (result == ERR_IO_PENDING)
-      return result;
-  }
+int TCPConnectJob::DoResolveHostComplete(int result) {
+  DCHECK_EQ(LOAD_STATE_RESOLVING_HOST, load_state());
+  if (result == OK)
+    next_state_ = kStateTCPConnect;
+  return result;
+}
 
+int TCPConnectJob::DoTCPConnect() {
+  next_state_ = kStateTCPConnectComplete;
+  set_load_state(LOAD_STATE_CONNECTING);
+  set_socket(client_socket_factory_->CreateTCPClientSocket(addresses_));
+  connect_start_time_ = base::TimeTicks::Now();
+  return socket()->Connect(&callback_);
+}
+
+int TCPConnectJob::DoTCPConnectComplete(int result) {
+  DCHECK_EQ(load_state(), LOAD_STATE_CONNECTING);
   if (result == OK) {
-    DCHECK_EQ(load_state(), LOAD_STATE_CONNECTING);
-    CHECK(connect_start_time_ != base::TimeTicks());
+    DCHECK(connect_start_time_ != base::TimeTicks());
     base::TimeDelta connect_duration =
         base::TimeTicks::Now() - connect_start_time_;
 
@@ -78,17 +116,6 @@ int TCPConnectJob::OnIOCompleteInternal(
         100);
   }
 
-  // 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.
-
-  delegate_->OnConnectJobComplete(
-      group_name_,
-      handle_,
-      result == OK ? socket_.release() : NULL,
-      result,
-      !synchronous);
-
-  // |this| is deleted after this point.
   return result;
 }
 
diff --git a/net/socket/tcp_client_socket_pool.h b/net/socket/tcp_client_socket_pool.h
index 4bbb9ba..33c38bb 100644
--- a/net/socket/tcp_client_socket_pool.h
+++ b/net/socket/tcp_client_socket_pool.h
@@ -37,26 +37,30 @@ class TCPConnectJob : public ConnectJob {
   virtual int Connect();
 
  private:
-  // Handles asynchronous completion of IO.  |result| represents the result of
-  // the IO operation.
+  enum State {
+    kStateResolveHost,
+    kStateResolveHostComplete,
+    kStateTCPConnect,
+    kStateTCPConnectComplete,
+    kStateNone,
+  };
+
   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);
+  // Runs the state transition loop.
+  int DoLoop(int result);
+
+  int DoResolveHost();
+  int DoResolveHostComplete(int result);
+  int DoTCPConnect();
+  int DoTCPConnectComplete(int result);
 
-  const std::string group_name_;
   const HostResolver::RequestInfo resolve_info_;
-  const ClientSocketHandle* const handle_;
   ClientSocketFactory* const client_socket_factory_;
   CompletionCallbackImpl<TCPConnectJob> callback_;
-  scoped_ptr<ClientSocket> socket_;
-  Delegate* const delegate_;
   SingleRequestHostResolver resolver_;
   AddressList addresses_;
+  State next_state_;
 
   // The time the Connect() method was called (if it got called).
   base::TimeTicks connect_start_time_;