ClientSocketPoolBaseHelper can try to read an invalid iterator. Fix that.

If we hit the backup socket timer and there is no pending request, then the
code still tries to create a backup socket using pending_requests.begin().
We change the code to cancel the backup socket timer when the pending_requests
hit zero.  We also check before reading pending_requests.begin().

BUG=53860
TEST=ClientsocketPoolBaseTest.CancelBackupSocketWhenThereAreNoRequests

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

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

2 files changed, 35 insertions, 2 deletions

diff --git a/net/socket/client_socket_pool_base.cc b/net/socket/client_socket_pool_base.cc
index 49fa09e..379c041 100644
--- a/net/socket/client_socket_pool_base.cc
+++ b/net/socket/client_socket_pool_base.cc
@@ -336,6 +336,10 @@ 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;
     }
   }
@@ -890,6 +894,11 @@ void ClientSocketPoolBaseHelper::Group::OnBackupSocketTimerFired(
     return;
   }
 
+  if (pending_requests_.empty()) {
+    LOG(DFATAL) << "No pending request for backup job.";
+    return;
+  }
+
   ConnectJob* backup_job = pool->connect_job_factory_->NewConnectJob(
       group_name, **pending_requests_.begin(), pool);
   backup_job->net_log().AddEvent(NetLog::TYPE_SOCKET_BACKUP_CREATED, NULL);
diff --git a/net/socket/client_socket_pool_base_unittest.cc b/net/socket/client_socket_pool_base_unittest.cc
index ff67e3b..71074a9 100644
--- a/net/socket/client_socket_pool_base_unittest.cc
+++ b/net/socket/client_socket_pool_base_unittest.cc
@@ -1940,8 +1940,8 @@ TEST_F(ClientSocketPoolBaseTest, BackupSocketCancelAtMaxSockets) {
   CreatePool(kDefaultMaxSockets, kDefaultMaxSockets);
   pool_->EnableConnectBackupJobs();
 
-  // Create the first socket and set to ERR_IO_PENDING.  This creates a
-  // backup job.
+  // 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;
@@ -1969,6 +1969,30 @@ TEST_F(ClientSocketPoolBaseTest, BackupSocketCancelAtMaxSockets) {
   EXPECT_EQ(kDefaultMaxSockets, client_socket_factory_.allocation_count());
 }
 
+TEST_F(ClientSocketPoolBaseTest, CancelBackupSocketWhenThereAreNoRequests) {
+  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()));
+  ASSERT_TRUE(pool_->HasGroup("bar"));
+  EXPECT_EQ(1, pool_->NumConnectJobsInGroup("bar"));
+
+  // Cancel the socket request.  This should cancel the backup timer.  Wait for
+  // the backup time to see if it indeed got canceled.
+  handle.Reset();
+  // Wait for the backup timer to fire (add some slop to ensure it fires)
+  PlatformThread::Sleep(ClientSocketPool::kMaxConnectRetryIntervalMs / 2 * 3);
+  MessageLoop::current()->RunAllPending();
+  ASSERT_TRUE(pool_->HasGroup("bar"));
+  EXPECT_EQ(1, pool_->NumConnectJobsInGroup("bar"));
+}
+
 // 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