Clean up socket backup job when there are no requests.

BUG=56532
TEST=ClientSocketPoolBaseTest.CancelBackupSocketAfterFinishingAllRequests

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

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

3 files changed, 39 insertions, 16 deletions

diff --git a/net/socket/client_socket_pool_base.cc b/net/socket/client_socket_pool_base.cc
index cdfb3f3..c40830d 100644
--- a/net/socket/client_socket_pool_base.cc
+++ b/net/socket/client_socket_pool_base.cc
@@ -180,9 +180,12 @@ void ClientSocketPoolBaseHelper::InsertRequestIntoQueue(
 // static
 const ClientSocketPoolBaseHelper::Request*
 ClientSocketPoolBaseHelper::RemoveRequestFromQueue(
-    RequestQueue::iterator it, RequestQueue* pending_requests) {
+    RequestQueue::iterator it, Group* group) {
   const Request* req = *it;
-  pending_requests->erase(it);
+  group->mutable_pending_requests()->erase(it);
+  // If there are no more requests, we kill the backup timer.
+  if (group->pending_requests().empty())
+    group->CleanupBackupJob();
   return req;
 }
 
@@ -324,8 +327,7 @@ void ClientSocketPoolBaseHelper::CancelRequest(
   RequestQueue::iterator it = group->mutable_pending_requests()->begin();
   for (; it != group->pending_requests().end(); ++it) {
     if ((*it)->handle() == handle) {
-      const Request* req =
-          RemoveRequestFromQueue(it, group->mutable_pending_requests());
+      const Request* req = RemoveRequestFromQueue(it, group);
       req->net_log().AddEvent(NetLog::TYPE_CANCELLED, NULL);
       req->net_log().EndEvent(NetLog::TYPE_SOCKET_POOL, NULL);
       delete req;
@@ -335,10 +337,6 @@ void ClientSocketPoolBaseHelper::CancelRequest(
         RemoveConnectJob(*group->jobs().begin(), group);
         CheckForStalledSocketGroups();
       }
-
-      // If there are no more requests, we kill the backup timer.
-      if (group->pending_requests().empty())
-        group->CleanupBackupJob();
       break;
     }
   }
@@ -636,8 +634,7 @@ void ClientSocketPoolBaseHelper::OnConnectJobComplete(
     RemoveConnectJob(job, group);
     if (!group->pending_requests().empty()) {
       scoped_ptr<const Request> r(RemoveRequestFromQueue(
-          group->mutable_pending_requests()->begin(),
-          group->mutable_pending_requests()));
+          group->mutable_pending_requests()->begin(), group));
       LogBoundConnectJobToRequest(job_log.source(), r.get());
       HandOutSocket(
           socket.release(), false /* unused socket */, r->handle(),
@@ -655,8 +652,7 @@ void ClientSocketPoolBaseHelper::OnConnectJobComplete(
     bool handed_out_socket = false;
     if (!group->pending_requests().empty()) {
       scoped_ptr<const Request> r(RemoveRequestFromQueue(
-          group->mutable_pending_requests()->begin(),
-          group->mutable_pending_requests()));
+          group->mutable_pending_requests()->begin(), group));
       LogBoundConnectJobToRequest(job_log.source(), r.get());
       job->GetAdditionalErrorState(r->handle());
       RemoveConnectJob(job, group);
@@ -722,8 +718,7 @@ void ClientSocketPoolBaseHelper::ProcessPendingRequest(
                                  *group->pending_requests().begin());
   if (rv != ERR_IO_PENDING) {
     scoped_ptr<const Request> request(RemoveRequestFromQueue(
-          group->mutable_pending_requests()->begin(),
-          group->mutable_pending_requests()));
+          group->mutable_pending_requests()->begin(), group));
     if (group->IsEmpty())
       RemoveGroup(group_name);
 
diff --git a/net/socket/client_socket_pool_base.h b/net/socket/client_socket_pool_base.h
index bea0b22..2e33abc 100644
--- a/net/socket/client_socket_pool_base.h
+++ b/net/socket/client_socket_pool_base.h
@@ -356,7 +356,7 @@ class ClientSocketPoolBaseHelper
   static void InsertRequestIntoQueue(const Request* r,
                                      RequestQueue* pending_requests);
   static const Request* RemoveRequestFromQueue(RequestQueue::iterator it,
-                                               RequestQueue* pending_requests);
+                                               Group* group);
 
   Group* GetOrCreateGroup(const std::string& group_name);
   void RemoveGroup(const std::string& group_name);
diff --git a/net/socket/client_socket_pool_base_unittest.cc b/net/socket/client_socket_pool_base_unittest.cc
index f8e3009..51aa25d 100644
--- a/net/socket/client_socket_pool_base_unittest.cc
+++ b/net/socket/client_socket_pool_base_unittest.cc
@@ -2032,7 +2032,7 @@ TEST_F(ClientSocketPoolBaseTest, BackupSocketCancelAtMaxSockets) {
   EXPECT_EQ(kDefaultMaxSockets, client_socket_factory_.allocation_count());
 }
 
-TEST_F(ClientSocketPoolBaseTest, CancelBackupSocketWhenThereAreNoRequests) {
+TEST_F(ClientSocketPoolBaseTest, CancelBackupSocketAfterCancelingAllRequests) {
   CreatePool(kDefaultMaxSockets, kDefaultMaxSockets);
   pool_->EnableConnectBackupJobs();
 
@@ -2056,6 +2056,34 @@ TEST_F(ClientSocketPoolBaseTest, CancelBackupSocketWhenThereAreNoRequests) {
   EXPECT_EQ(1, pool_->NumConnectJobsInGroup("bar"));
 }
 
+TEST_F(ClientSocketPoolBaseTest, CancelBackupSocketAfterFinishingAllRequests) {
+  CreatePool(kDefaultMaxSockets, kDefaultMaxSockets);
+  pool_->EnableConnectBackupJobs();
+
+  // Create the first socket and set to ERR_IO_PENDING.  This starts the backup
+  // timer.
+  connect_job_factory_->set_job_type(TestConnectJob::kMockWaitingJob);
+  ClientSocketHandle handle;
+  TestCompletionCallback callback;
+  EXPECT_EQ(ERR_IO_PENDING, handle.Init("bar", params_, kDefaultPriority,
+                                        &callback, pool_, BoundNetLog()));
+  connect_job_factory_->set_job_type(TestConnectJob::kMockPendingJob);
+  ClientSocketHandle handle2;
+  TestCompletionCallback callback2;
+  EXPECT_EQ(ERR_IO_PENDING, handle2.Init("bar", params_, kDefaultPriority,
+                                         &callback2, pool_, BoundNetLog()));
+  ASSERT_TRUE(pool_->HasGroup("bar"));
+  EXPECT_EQ(2, pool_->NumConnectJobsInGroup("bar"));
+
+  // Cancel request 1 and then complete request 2.  With the requests finished,
+  // the backup timer should be cancelled.
+  handle.Reset();
+  EXPECT_EQ(OK, callback2.WaitForResult());
+  // Wait for the backup timer to fire (add some slop to ensure it fires)
+  PlatformThread::Sleep(ClientSocketPool::kMaxConnectRetryIntervalMs / 2 * 3);
+  MessageLoop::current()->RunAllPending();
+}
+
 // Test delayed socket binding for the case where we have two connects,
 // and while one is waiting on a connect, the other frees up.
 // The socket waiting on a connect should switch immediately to the freed