Start backup socket timer for preconnects too.

This is important because entering URLs into the omnibox will trigger preconnects.

BUG=81686
TEST=none


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

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

2 files changed, 57 insertions, 5 deletions

diff --git a/net/socket/client_socket_pool_base.cc b/net/socket/client_socket_pool_base.cc
index 2a5f216..0972534 100644
--- a/net/socket/client_socket_pool_base.cc
+++ b/net/socket/client_socket_pool_base.cc
@@ -334,8 +334,7 @@ int ClientSocketPoolBaseHelper::RequestSocketInternal(
     // creating a new one.  If the SYN is lost, this backup socket may complete
     // before the slow socket, improving end user latency.
     if (connect_backup_jobs_enabled_ &&
-        group->IsEmpty() && !group->HasBackupJob() &&
-        handle) {
+        group->IsEmpty() && !group->HasBackupJob()) {
       group->StartBackupSocketTimer(group_name, this);
     }
 
@@ -1065,10 +1064,8 @@ void ClientSocketPoolBaseHelper::Group::OnBackupSocketTimerFired(
     return;
   }
 
-  if (pending_requests_.empty()) {
-    LOG(DFATAL) << "No pending request for backup job.";
+  if (pending_requests_.empty())
     return;
-  }
 
   ConnectJob* backup_job = pool->connect_job_factory_->NewConnectJob(
       group_name, **pending_requests_.begin(), pool);
diff --git a/net/socket/client_socket_pool_base_unittest.cc b/net/socket/client_socket_pool_base_unittest.cc
index b0d9ba1..366af45 100644
--- a/net/socket/client_socket_pool_base_unittest.cc
+++ b/net/socket/client_socket_pool_base_unittest.cc
@@ -3129,6 +3129,61 @@ TEST_F(ClientSocketPoolBaseTest, PreconnectClosesIdleSocketRemovesGroup) {
   EXPECT_EQ(0, pool_->NumActiveSocketsInGroup("b"));
 }
 
+TEST_F(ClientSocketPoolBaseTest, PreconnectWithoutBackupTimer) {
+  CreatePool(kDefaultMaxSockets, kDefaultMaxSocketsPerGroup);
+  pool_->EnableConnectBackupJobs();
+
+  // Make the ConnectJob hang until it times out, shorten the timeout.
+  connect_job_factory_->set_job_type(TestConnectJob::kMockWaitingJob);
+  connect_job_factory_->set_timeout_duration(
+      base::TimeDelta::FromMilliseconds(500));
+  pool_->RequestSockets("a", &params_, 1, BoundNetLog());
+  EXPECT_EQ(1, pool_->NumConnectJobsInGroup("a"));
+  EXPECT_EQ(0, pool_->IdleSocketCountInGroup("a"));
+  MessageLoop::current()->RunAllPending();
+
+  // Make sure the backup timer doesn't fire, by making it a pending job instead
+  // of a waiting job, so it *would* complete if the timer fired.
+  connect_job_factory_->set_job_type(TestConnectJob::kMockPendingJob);
+  base::PlatformThread::Sleep(1000);
+  MessageLoop::current()->RunAllPending();
+  EXPECT_FALSE(pool_->HasGroup("a"));
+}
+
+TEST_F(ClientSocketPoolBaseTest, PreconnectWithBackupTimer) {
+  CreatePool(kDefaultMaxSockets, kDefaultMaxSocketsPerGroup);
+  pool_->EnableConnectBackupJobs();
+
+  // Make the ConnectJob hang forever.
+  connect_job_factory_->set_job_type(TestConnectJob::kMockWaitingJob);
+  pool_->RequestSockets("a", &params_, 1, BoundNetLog());
+  EXPECT_EQ(1, pool_->NumConnectJobsInGroup("a"));
+  EXPECT_EQ(0, pool_->IdleSocketCountInGroup("a"));
+  MessageLoop::current()->RunAllPending();
+
+  // Make the backup job be a pending job, so it completes normally.
+  connect_job_factory_->set_job_type(TestConnectJob::kMockPendingJob);
+  ClientSocketHandle handle;
+  TestCompletionCallback callback;
+  EXPECT_EQ(ERR_IO_PENDING, handle.Init("a",
+                                        params_,
+                                        kDefaultPriority,
+                                        &callback,
+                                        pool_.get(),
+                                        BoundNetLog()));
+  // Timer has started, but the other connect job shouldn't be created yet.
+  EXPECT_EQ(1, pool_->NumConnectJobsInGroup("a"));
+  EXPECT_EQ(0, pool_->IdleSocketCountInGroup("a"));
+  EXPECT_EQ(0, pool_->NumActiveSocketsInGroup("a"));
+  ASSERT_EQ(OK, callback.WaitForResult());
+
+  // The hung connect job should still be there, but everything else should be
+  // complete.
+  EXPECT_EQ(1, pool_->NumConnectJobsInGroup("a"));
+  EXPECT_EQ(0, pool_->IdleSocketCountInGroup("a"));
+  EXPECT_EQ(1, pool_->NumActiveSocketsInGroup("a"));
+}
+
 }  // namespace
 
 }  // namespace net