Add load states for requests stalled waiting for a socket slot

in socket pools / socket pool groups.  This is intended to
make it easier to identify issues where something is holding
on to sockets and not releasing them, as is the case in two of the referenced bugs.  Also helpful to identify proxy issues
where the global socket pool limit is reached.

Bug=241844,233181,248412

Review URL: https://chromiumcodereview.appspot.com/19278016

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

3 files changed, 210 insertions, 51 deletions

diff --git a/net/base/load_states_list.h b/net/base/load_states_list.h
index 003df44..23d14c9 100644
--- a/net/base/load_states_list.h
+++ b/net/base/load_states_list.h
@@ -15,6 +15,17 @@
 // called Read yet).
 LOAD_STATE(IDLE)
 
+// When a socket pool group is below the maximum number of sockets allowed per
+// group, but a new socket cannot be created due to the per-pool socket limit,
+// this state is returned by all requests for the group waiting on an idle
+// connection, except those that may be serviced by a pending new connection.
+LOAD_STATE(WAITING_FOR_STALLED_SOCKET_POOL)
+
+// When a socket pool group has reached the maximum number of sockets allowed
+// per group, this state is returned for all requests that don't have a socket,
+// except those that correspond to a pending new connection.
+LOAD_STATE(WAITING_FOR_AVAILABLE_SOCKET)
+
 // This state indicates that the URLRequest delegate has chosen to block this
 // request before it was sent over the network. When in this state, the
 // delegate should set a load state parameter on the URLRequest describing
diff --git a/net/socket/client_socket_pool_base.cc b/net/socket/client_socket_pool_base.cc
index ee80551..3332e04 100644
--- a/net/socket/client_socket_pool_base.cc
+++ b/net/socket/client_socket_pool_base.cc
@@ -560,27 +560,29 @@ LoadState ClientSocketPoolBaseHelper::GetLoadState(
   // Can't use operator[] since it is non-const.
   const Group& group = *group_map_.find(group_name)->second;
 
-  // Search pending_requests for matching handle.
+  // Search the first group.jobs().size() |pending_requests| for |handle|.
+  // If it's farther back in the deque than that, it doesn't have a
+  // corresponding ConnectJob.
+  size_t connect_jobs = group.jobs().size();
   RequestQueue::const_iterator it = group.pending_requests().begin();
-  for (size_t i = 0; it != group.pending_requests().end(); ++it, ++i) {
-    if ((*it)->handle() == handle) {
-      if (i < group.jobs().size()) {
-        LoadState max_state = LOAD_STATE_IDLE;
-        for (ConnectJobSet::const_iterator job_it = group.jobs().begin();
-             job_it != group.jobs().end(); ++job_it) {
-          max_state = std::max(max_state, (*job_it)->GetLoadState());
-        }
-        return max_state;
-      } else {
-        // TODO(wtc): Add a state for being on the wait list.
-        // See http://crbug.com/5077.
-        return LOAD_STATE_IDLE;
-      }
+  for (size_t i = 0; it != group.pending_requests().end() && i < connect_jobs;
+       ++it, ++i) {
+    if ((*it)->handle() != handle)
+      continue;
+
+    // Just return the state  of the farthest along ConnectJob for the first
+    // group.jobs().size() pending requests.
+    LoadState max_state = LOAD_STATE_IDLE;
+    for (ConnectJobSet::const_iterator job_it = group.jobs().begin();
+         job_it != group.jobs().end(); ++job_it) {
+      max_state = std::max(max_state, (*job_it)->GetLoadState());
     }
+    return max_state;
   }
 
-  NOTREACHED();
-  return LOAD_STATE_IDLE;
+  if (group.IsStalledOnPoolMaxSockets(max_sockets_per_group_))
+    return LOAD_STATE_WAITING_FOR_STALLED_SOCKET_POOL;
+  return LOAD_STATE_WAITING_FOR_AVAILABLE_SOCKET;
 }
 
 base::DictionaryValue* ClientSocketPoolBaseHelper::GetInfoAsValue(
diff --git a/net/socket/client_socket_pool_base_unittest.cc b/net/socket/client_socket_pool_base_unittest.cc
index 839b888..5eeda97 100644
--- a/net/socket/client_socket_pool_base_unittest.cc
+++ b/net/socket/client_socket_pool_base_unittest.cc
@@ -220,8 +220,13 @@ class MockClientSocketFactory : public ClientSocketFactory {
   }
 
   void WaitForSignal(TestConnectJob* job) { waiting_jobs_.push_back(job); }
+
   void SignalJobs();
 
+  void SignalJob(size_t job);
+
+  void SetJobLoadState(size_t job, LoadState load_state);
+
   int allocation_count() const { return allocation_count_; }
 
  private:
@@ -237,7 +242,6 @@ class TestConnectJob : public ConnectJob {
     kMockPendingJob,
     kMockPendingFailingJob,
     kMockWaitingJob,
-    kMockAdvancingLoadStateJob,
     kMockRecoverableJob,
     kMockPendingRecoverableJob,
     kMockAdditionalErrorStateJob,
@@ -267,6 +271,10 @@ class TestConnectJob : public ConnectJob {
     DoConnect(waiting_success_, true /* async */, false /* recoverable */);
   }
 
+  void set_load_state(LoadState load_state) { load_state_ = load_state; }
+
+  // From ConnectJob:
+
   virtual LoadState GetLoadState() const OVERRIDE { return load_state_; }
 
   virtual void GetAdditionalErrorState(ClientSocketHandle* handle) OVERRIDE {
@@ -280,7 +288,7 @@ class TestConnectJob : public ConnectJob {
   }
 
  private:
-  // ConnectJob implementation.
+  // From ConnectJob:
 
   virtual int ConnectInternal() OVERRIDE {
     AddressList ignored;
@@ -329,14 +337,10 @@ class TestConnectJob : public ConnectJob {
             base::TimeDelta::FromMilliseconds(2));
         return ERR_IO_PENDING;
       case kMockWaitingJob:
+        set_load_state(LOAD_STATE_CONNECTING);
         client_socket_factory_->WaitForSignal(this);
         waiting_success_ = true;
         return ERR_IO_PENDING;
-      case kMockAdvancingLoadStateJob:
-        base::MessageLoop::current()->PostTask(
-            FROM_HERE, base::Bind(&TestConnectJob::AdvanceLoadState,
-                                  weak_factory_.GetWeakPtr(), load_state_));
-        return ERR_IO_PENDING;
       case kMockRecoverableJob:
         return DoConnect(false /* error */, false /* sync */,
                          true /* recoverable */);
@@ -374,8 +378,6 @@ class TestConnectJob : public ConnectJob {
     }
   }
 
-  void set_load_state(LoadState load_state) { load_state_ = load_state; }
-
   int DoConnect(bool succeed, bool was_async, bool recoverable) {
     int result = OK;
     if (succeed) {
@@ -392,22 +394,6 @@ class TestConnectJob : public ConnectJob {
     return result;
   }
 
-  // This function helps simulate the progress of load states on a ConnectJob.
-  // Each time it is called it advances the load state and posts a task to be
-  // called again.  It stops at the last connecting load state (the one
-  // before LOAD_STATE_SENDING_REQUEST).
-  void AdvanceLoadState(LoadState state) {
-    int tmp = state;
-    tmp++;
-    if (tmp < LOAD_STATE_SENDING_REQUEST) {
-      state = static_cast<LoadState>(tmp);
-      set_load_state(state);
-      base::MessageLoop::current()->PostTask(
-          FROM_HERE, base::Bind(&TestConnectJob::AdvanceLoadState,
-                                weak_factory_.GetWeakPtr(), state));
-    }
-  }
-
   bool waiting_success_;
   const JobType job_type_;
   MockClientSocketFactory* const client_socket_factory_;
@@ -624,6 +610,18 @@ void MockClientSocketFactory::SignalJobs() {
   waiting_jobs_.clear();
 }
 
+void MockClientSocketFactory::SignalJob(size_t job) {
+  ASSERT_LT(job, waiting_jobs_.size());
+  waiting_jobs_[job]->Signal();
+  waiting_jobs_.erase(waiting_jobs_.begin() + job);
+}
+
+void MockClientSocketFactory::SetJobLoadState(size_t job,
+                                              LoadState load_state) {
+  ASSERT_LT(job, waiting_jobs_.size());
+  waiting_jobs_[job]->set_load_state(load_state);
+}
+
 class TestConnectJobDelegate : public ConnectJob::Delegate {
  public:
   TestConnectJobDelegate()
@@ -1924,10 +1922,10 @@ TEST_F(ClientSocketPoolBaseTest, PendingJobCompletionOrder) {
   EXPECT_EQ(&req3, request_order[2]);
 }
 
-TEST_F(ClientSocketPoolBaseTest, LoadState) {
+// Test GetLoadState in the case there's only one socket request.
+TEST_F(ClientSocketPoolBaseTest, LoadStateOneRequest) {
   CreatePool(kDefaultMaxSockets, kDefaultMaxSocketsPerGroup);
-  connect_job_factory_->set_job_type(
-      TestConnectJob::kMockAdvancingLoadStateJob);
+  connect_job_factory_->set_job_type(TestConnectJob::kMockWaitingJob);
 
   ClientSocketHandle handle;
   TestCompletionCallback callback;
@@ -1938,17 +1936,165 @@ TEST_F(ClientSocketPoolBaseTest, LoadState) {
                        pool_.get(),
                        BoundNetLog());
   EXPECT_EQ(ERR_IO_PENDING, rv);
-  EXPECT_EQ(LOAD_STATE_IDLE, handle.GetLoadState());
+  EXPECT_EQ(LOAD_STATE_CONNECTING, handle.GetLoadState());
 
-  base::MessageLoop::current()->RunUntilIdle();
+  client_socket_factory_.SetJobLoadState(0, LOAD_STATE_SSL_HANDSHAKE);
+  EXPECT_EQ(LOAD_STATE_SSL_HANDSHAKE, handle.GetLoadState());
+
+  // No point in completing the connection, since ClientSocketHandles only
+  // expect the LoadState to be checked while connecting.
+}
+
+// Test GetLoadState in the case there are two socket requests.
+TEST_F(ClientSocketPoolBaseTest, LoadStateTwoRequests) {
+  CreatePool(2, 2);
+  connect_job_factory_->set_job_type(TestConnectJob::kMockWaitingJob);
+
+  ClientSocketHandle handle;
+  TestCompletionCallback callback;
+  int rv = handle.Init("a",
+                       params_,
+                       kDefaultPriority,
+                       callback.callback(),
+                       pool_.get(),
+                       BoundNetLog());
+  EXPECT_EQ(ERR_IO_PENDING, rv);
 
   ClientSocketHandle handle2;
   TestCompletionCallback callback2;
-  rv = handle2.Init("a", params_, kDefaultPriority, callback2.callback(),
-                    pool_.get(), BoundNetLog());
+  rv = handle2.Init("a",
+                    params_,
+                    kDefaultPriority,
+                    callback2.callback(),
+                    pool_.get(),
+                    BoundNetLog());
   EXPECT_EQ(ERR_IO_PENDING, rv);
-  EXPECT_NE(LOAD_STATE_IDLE, handle.GetLoadState());
-  EXPECT_NE(LOAD_STATE_IDLE, handle2.GetLoadState());
+
+  // If the first Job is in an earlier state than the second, the state of
+  // the second job should be used for both handles.
+  client_socket_factory_.SetJobLoadState(0, LOAD_STATE_RESOLVING_HOST);
+  EXPECT_EQ(LOAD_STATE_CONNECTING, handle.GetLoadState());
+  EXPECT_EQ(LOAD_STATE_CONNECTING, handle2.GetLoadState());
+
+  // If the second Job is in an earlier state than the second, the state of
+  // the first job should be used for both handles.
+  client_socket_factory_.SetJobLoadState(0, LOAD_STATE_SSL_HANDSHAKE);
+  // One request is farther
+  EXPECT_EQ(LOAD_STATE_SSL_HANDSHAKE, handle.GetLoadState());
+  EXPECT_EQ(LOAD_STATE_SSL_HANDSHAKE, handle2.GetLoadState());
+
+  // Farthest along job connects and the first request gets the socket.  The
+  // second handle switches to the state of the remaining ConnectJob.
+  client_socket_factory_.SignalJob(0);
+  EXPECT_EQ(OK, callback.WaitForResult());
+  EXPECT_EQ(LOAD_STATE_CONNECTING, handle2.GetLoadState());
+}
+
+// Test GetLoadState in the case the per-group limit is reached.
+TEST_F(ClientSocketPoolBaseTest, LoadStateGroupLimit) {
+  CreatePool(2, 1);
+  connect_job_factory_->set_job_type(TestConnectJob::kMockWaitingJob);
+
+  ClientSocketHandle handle;
+  TestCompletionCallback callback;
+  int rv = handle.Init("a",
+                       params_,
+                       MEDIUM,
+                       callback.callback(),
+                       pool_.get(),
+                       BoundNetLog());
+  EXPECT_EQ(ERR_IO_PENDING, rv);
+  EXPECT_EQ(LOAD_STATE_CONNECTING, handle.GetLoadState());
+
+  // Request another socket from the same pool, buth with a higher priority.
+  // The first request should now be stalled at the socket group limit.
+  ClientSocketHandle handle2;
+  TestCompletionCallback callback2;
+  rv = handle2.Init("a",
+                    params_,
+                    HIGHEST,
+                    callback2.callback(),
+                    pool_.get(),
+                    BoundNetLog());
+  EXPECT_EQ(ERR_IO_PENDING, rv);
+  EXPECT_EQ(LOAD_STATE_WAITING_FOR_AVAILABLE_SOCKET, handle.GetLoadState());
+  EXPECT_EQ(LOAD_STATE_CONNECTING, handle2.GetLoadState());
+
+  // The first handle should remain stalled as the other socket goes through
+  // the connect process.
+
+  client_socket_factory_.SetJobLoadState(0, LOAD_STATE_SSL_HANDSHAKE);
+  EXPECT_EQ(LOAD_STATE_WAITING_FOR_AVAILABLE_SOCKET, handle.GetLoadState());
+  EXPECT_EQ(LOAD_STATE_SSL_HANDSHAKE, handle2.GetLoadState());
+
+  client_socket_factory_.SignalJob(0);
+  EXPECT_EQ(OK, callback2.WaitForResult());
+  EXPECT_EQ(LOAD_STATE_WAITING_FOR_AVAILABLE_SOCKET, handle.GetLoadState());
+
+  // Closing the second socket should cause the stalled handle to finally get a
+  // ConnectJob.
+  handle2.socket()->Disconnect();
+  handle2.Reset();
+  EXPECT_EQ(LOAD_STATE_CONNECTING, handle.GetLoadState());
+}
+
+// Test GetLoadState in the case the per-pool limit is reached.
+TEST_F(ClientSocketPoolBaseTest, LoadStatePoolLimit) {
+  CreatePool(2, 2);
+  connect_job_factory_->set_job_type(TestConnectJob::kMockWaitingJob);
+
+  ClientSocketHandle handle;
+  TestCompletionCallback callback;
+  int rv = handle.Init("a",
+                       params_,
+                       kDefaultPriority,
+                       callback.callback(),
+                       pool_.get(),
+                       BoundNetLog());
+  EXPECT_EQ(ERR_IO_PENDING, rv);
+
+  // Request for socket from another pool.
+  ClientSocketHandle handle2;
+  TestCompletionCallback callback2;
+  rv = handle2.Init("b",
+                    params_,
+                    kDefaultPriority,
+                    callback2.callback(),
+                    pool_.get(),
+                    BoundNetLog());
+  EXPECT_EQ(ERR_IO_PENDING, rv);
+
+  // Request another socket from the first pool.  Request should stall at the
+  // socket pool limit.
+  ClientSocketHandle handle3;
+  TestCompletionCallback callback3;
+  rv = handle3.Init("a",
+                    params_,
+                    kDefaultPriority,
+                    callback2.callback(),
+                    pool_.get(),
+                    BoundNetLog());
+  EXPECT_EQ(ERR_IO_PENDING, rv);
+
+  // The third handle should remain stalled as the other sockets in its group
+  // goes through the connect process.
+
+  EXPECT_EQ(LOAD_STATE_CONNECTING, handle.GetLoadState());
+  EXPECT_EQ(LOAD_STATE_WAITING_FOR_STALLED_SOCKET_POOL, handle3.GetLoadState());
+
+  client_socket_factory_.SetJobLoadState(0, LOAD_STATE_SSL_HANDSHAKE);
+  EXPECT_EQ(LOAD_STATE_SSL_HANDSHAKE, handle.GetLoadState());
+  EXPECT_EQ(LOAD_STATE_WAITING_FOR_STALLED_SOCKET_POOL, handle3.GetLoadState());
+
+  client_socket_factory_.SignalJob(0);
+  EXPECT_EQ(OK, callback.WaitForResult());
+  EXPECT_EQ(LOAD_STATE_WAITING_FOR_STALLED_SOCKET_POOL, handle3.GetLoadState());
+
+  // Closing a socket should allow the stalled handle to finally get a new
+  // ConnectJob.
+  handle.socket()->Disconnect();
+  handle.Reset();
+  EXPECT_EQ(LOAD_STATE_CONNECTING, handle3.GetLoadState());
 }
 
 TEST_F(ClientSocketPoolBaseTest, Recoverable) {