Clean up socket backup job when there are no requests.

net/socket/client_socket_pool_base_unittest.cc

 // Copyright (c) 2010 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/socket/client_socket_pool_base.h"
 
 #include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/message_loop.h"
 #include "base/platform_thread.h"
@@ skipping 2014 matching lines @@
   // Cancel the pending request.
   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();
   EXPECT_EQ(kDefaultMaxSockets, client_socket_factory_.allocation_count());
 }
 
-TEST_F(ClientSocketPoolBaseTest, CancelBackupSocketWhenThereAreNoRequests) {
+TEST_F(ClientSocketPoolBaseTest, CancelBackupSocketAfterCancelingAllRequests) {
   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_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
 // up socket.
 TEST_F(ClientSocketPoolBaseTest, DelayedSocketBindingWaitingForConnect) {
   CreatePool(kDefaultMaxSockets, kDefaultMaxSocketsPerGroup);
   connect_job_factory_->set_job_type(TestConnectJob::kMockPendingJob);
 
   ClientSocketHandle handle1;
   TestCompletionCallback callback;
@@ skipping 165 matching lines @@
   EXPECT_EQ(1, pool_->NumConnectJobsInGroup("a"));
 
   EXPECT_EQ(ERR_CONNECTION_FAILED, callback1.WaitForResult());
   EXPECT_EQ(ERR_CONNECTION_FAILED, callback2.WaitForResult());
   EXPECT_FALSE(pool_->HasGroup("a"));
 }
 
 }  // namespace
 
 }  // namespace net