Restart socket pool backup job timer on ReleaseSocket.

This is a temporary hack while we pursue longer term solutions that will require bigger changes.

BUG=81686
TEST=none

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

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

3 files changed, 61 insertions, 8 deletions

diff --git a/net/socket/client_socket_pool_base.cc b/net/socket/client_socket_pool_base.cc
index 0972534..55ce2ec 100644
--- a/net/socket/client_socket_pool_base.cc
+++ b/net/socket/client_socket_pool_base.cc
@@ -202,7 +202,7 @@ ClientSocketPoolBaseHelper::RemoveRequestFromQueue(
   group->mutable_pending_requests()->erase(it);
   // If there are no more requests, we kill the backup timer.
   if (group->pending_requests().empty())
-    group->CleanupBackupJob();
+    group->CleanupBackupJobTimer();
   return req;
 }
 
@@ -334,7 +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()) {
+        group->IsEmpty() && !group->BackupJobTimerIsRunning()) {
       group->StartBackupSocketTimer(group_name, this);
     }
 
@@ -562,7 +562,7 @@ DictionaryValue* ClientSocketPoolBaseHelper::GetInfoAsValue(
 
     group_dict->SetBoolean("is_stalled",
                            group->IsStalled(max_sockets_per_group_));
-    group_dict->SetBoolean("has_backup_job", group->HasBackupJob());
+    group_dict->SetBoolean("has_backup_job", group->BackupJobTimerIsRunning());
 
     all_groups_dict->SetWithoutPathExpansion(it->first, group_dict);
   }
@@ -687,6 +687,11 @@ void ClientSocketPoolBaseHelper::ReleaseSocket(const std::string& group_name,
     OnAvailableSocketSlot(group_name, group);
   } else {
     delete socket;
+    if (connect_backup_jobs_enabled_ &&
+        !group->BackupJobTimerIsRunning() && !group->HasBackupJob() &&
+        !group->pending_requests().empty()) {
+      group->StartBackupSocketTimer(group_name, this);
+    }
   }
 
   CheckForStalledSocketGroups();
@@ -828,8 +833,10 @@ void ClientSocketPoolBaseHelper::RemoveConnectJob(ConnectJob* job,
 
   // If we've got no more jobs for this group, then we no longer need a
   // backup job either.
+  // TODO(willchan): Probably should only cancel the backup job timer if
+  // RemoveConnectJob() is called on success.
   if (group->jobs().empty())
-    group->CleanupBackupJob();
+    group->CleanupBackupJobTimer();
 
   DCHECK(job);
   delete job;
@@ -1015,8 +1022,19 @@ ClientSocketPoolBaseHelper::Group::Group()
     : active_socket_count_(0),
       ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)) {}
 
-ClientSocketPoolBaseHelper::Group::~Group() {
-  CleanupBackupJob();
+ClientSocketPoolBaseHelper::Group::~Group() {}
+
+bool ClientSocketPoolBaseHelper::Group::HasBackupJob() const {
+  for (std::set<ConnectJob*>::const_iterator it = jobs_.begin();
+       it != jobs_.end(); ++it) {
+    const ConnectJob* job = *it;
+    // NOTE(willchan): We should probably create a better signal at some point.
+    // I consider this code a temporary hack anyway, so I'll fix this later.
+    if (job->prefer_ipv4())
+      return true;
+  }
+
+  return false;
 }
 
 void ClientSocketPoolBaseHelper::Group::StartBackupSocketTimer(
diff --git a/net/socket/client_socket_pool_base.h b/net/socket/client_socket_pool_base.h
index 25bc7c7..90beed89 100644
--- a/net/socket/client_socket_pool_base.h
+++ b/net/socket/client_socket_pool_base.h
@@ -360,9 +360,11 @@ class ClientSocketPoolBaseHelper
       return pending_requests_.front()->priority();
     }
 
-    bool HasBackupJob() const { return !method_factory_.empty(); }
+    bool HasBackupJob() const;
 
-    void CleanupBackupJob() {
+    bool BackupJobTimerIsRunning() const { return !method_factory_.empty(); }
+
+    void CleanupBackupJobTimer() {
       method_factory_.RevokeAll();
     }
 
diff --git a/net/socket/client_socket_pool_base_unittest.cc b/net/socket/client_socket_pool_base_unittest.cc
index 366af45..ce2546c 100644
--- a/net/socket/client_socket_pool_base_unittest.cc
+++ b/net/socket/client_socket_pool_base_unittest.cc
@@ -3184,6 +3184,39 @@ TEST_F(ClientSocketPoolBaseTest, PreconnectWithBackupTimer) {
   EXPECT_EQ(1, pool_->NumActiveSocketsInGroup("a"));
 }
 
+TEST_F(ClientSocketPoolBaseTest, ReleaseSocketRestartsBackupJobTimer) {
+  CreatePool(kDefaultMaxSockets, kDefaultMaxSockets);
+  pool_->EnableConnectBackupJobs();
+
+  // Make the ConnectJob hang forever.
+  connect_job_factory_->set_job_type(TestConnectJob::kMockWaitingJob);
+  ClientSocketHandle handle1;
+  TestCompletionCallback callback1;
+  EXPECT_EQ(ERR_IO_PENDING, handle1.Init("a",
+                                         params_,
+                                         kDefaultPriority,
+                                         &callback1,
+                                         pool_.get(),
+                                         BoundNetLog()));
+  ClientSocketHandle handle2;
+  TestCompletionCallback callback2;
+  EXPECT_EQ(ERR_IO_PENDING, handle2.Init("a",
+                                         params_,
+                                         kDefaultPriority,
+                                         &callback2,
+                                         pool_.get(),
+                                         BoundNetLog()));
+  EXPECT_EQ(2, pool_->NumConnectJobsInGroup("a"));
+  EXPECT_EQ(0, pool_->IdleSocketCountInGroup("a"));
+
+  // Make the backup job be a pending job, so it completes normally.
+  connect_job_factory_->set_job_type(TestConnectJob::kMockPendingJob);
+  ASSERT_EQ(OK, callback1.WaitForResult());
+  handle1.socket()->Disconnect();
+  handle1.Reset();
+  ASSERT_EQ(OK, callback2.WaitForResult());
+}
+
 }  // namespace
 
 }  // namespace net