Revert r85014 because it doesn't work fully and we're doing something else.

=====

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

TBR=willchan@chromium.org
Review URL: http://codereview.chromium.org/6973022

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

3 files changed, 8 insertions, 61 deletions

diff --git a/net/socket/client_socket_pool_base.cc b/net/socket/client_socket_pool_base.cc
index 55ce2ec..0972534 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->CleanupBackupJobTimer();
+    group->CleanupBackupJob();
   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->BackupJobTimerIsRunning()) {
+        group->IsEmpty() && !group->HasBackupJob()) {
       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->BackupJobTimerIsRunning());
+    group_dict->SetBoolean("has_backup_job", group->HasBackupJob());
 
     all_groups_dict->SetWithoutPathExpansion(it->first, group_dict);
   }
@@ -687,11 +687,6 @@ 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();
@@ -833,10 +828,8 @@ 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->CleanupBackupJobTimer();
+    group->CleanupBackupJob();
 
   DCHECK(job);
   delete job;
@@ -1022,19 +1015,8 @@ ClientSocketPoolBaseHelper::Group::Group()
     : active_socket_count_(0),
       ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)) {}
 
-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;
+ClientSocketPoolBaseHelper::Group::~Group() {
+  CleanupBackupJob();
 }
 
 void ClientSocketPoolBaseHelper::Group::StartBackupSocketTimer(
diff --git a/net/socket/client_socket_pool_base.h b/net/socket/client_socket_pool_base.h
index 90beed89..25bc7c7 100644
--- a/net/socket/client_socket_pool_base.h
+++ b/net/socket/client_socket_pool_base.h
@@ -360,11 +360,9 @@ class ClientSocketPoolBaseHelper
       return pending_requests_.front()->priority();
     }
 
-    bool HasBackupJob() const;
+    bool HasBackupJob() const { return !method_factory_.empty(); }
 
-    bool BackupJobTimerIsRunning() const { return !method_factory_.empty(); }
-
-    void CleanupBackupJobTimer() {
+    void CleanupBackupJob() {
       method_factory_.RevokeAll();
     }
 
diff --git a/net/socket/client_socket_pool_base_unittest.cc b/net/socket/client_socket_pool_base_unittest.cc
index ce2546c..366af45 100644
--- a/net/socket/client_socket_pool_base_unittest.cc
+++ b/net/socket/client_socket_pool_base_unittest.cc
@@ -3184,39 +3184,6 @@ 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