Clean up socket backup job when there are no requests.

net/socket/client_socket_pool_base_unittest.cc

Index: net/socket/client_socket_pool_base_unittest.cc
diff --git a/net/socket/client_socket_pool_base_unittest.cc b/net/socket/client_socket_pool_base_unittest.cc
index f8e3009a82023f26b27b8943fdd3851c5da90781..51aa25de0f40c5305b201f57fdbded6269543e59 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);

[Mike Belshe, 2010/09/27 19:00:49]

nit: line 2070 is unnecessary due to line 2065.

[willchan no longer on Chromium, 2010/09/27 19:05:04]

It's necessary.  Waiting != Pending.  We need a job to be pending so that
handle2.WaitForResult() will complete.  If they're both Waiting, then it'll
never finish.

[Mike Belshe, 2010/09/28 15:14:00]

Doh - I misread.  Somehow I read them as the same state.

+  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);

[Paweł Hajdan Jr., 2010/09/28 08:08:34]

Can we use some smarter method to wait than Sleep?

+  MessageLoop::current()->RunAllPending();

[Mike Belshe, 2010/09/27 19:00:49]

nit: We're not actually verifying that the backup socket didn't fire here?

[willchan no longer on Chromium, 2010/09/27 19:05:04]

Well, previously we would hit a LOG(DFATAL) saying that the socket timer fired
even though we had no requests.  So, by doing RunAllPending(), we can verify
that the runnable method was revoked properly.

+}
+
 // 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