Slightly tweak socket late binding behavior to make it more deterministic which is useful for tests.

When a socket has been released to ClientSocketPoolBase, it is not immediately released, but a DoReleaseSocket() task gets posted to the MessageLoop. In many of our tests, what can happen is right after ClientSocketHandle::Reset() gets called, which posts the aforementioned release task, another ClientSocketHandle requests a socket, which starts up a ConnectJob that will completely asynchronously, but AFTER the release task happens.  Note that if the released socket is no longer connected, then when DoReleaseSocket() completes, it will try to process a pending request, thereby initiating an extra ConnectJob.  So, even though ClientSocketHandle::Init() may only have been called twice, we can see 3 ConnectJobs, which is weird.  It's mostly harmless since this race condition probably doesn't happen much in real use, and the only side effect is an extra ConnectJob which might even be faster, but this behavior introduces difficult to control behavior in tests.
The solution taken here is to keep track of the number of pending released sockets for a ClientSocketPool group.  If there are any pending released sockets, then don't start up a new ConnectJob yet, wait for the sockets to get released.  This can introduce some delays for starting up ConnectJobs, but the delay should be very small (one MessageLoop iteration through the IO thread should not take long) and it only happens in the edge case where a new ClientSocketPool request comes in for the same group where a DoReleaseSocket() task has been posted.  In these cases, we may also prevent unnecessary ConnectJobs from getting started up, especially if the socket is released and able to be reused.
Existing unit tests (ClientSocketPoolBaseTest_LateBinding_RequestPendingJob*) cover this change in behavior.

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

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

1 files changed, 15 insertions, 7 deletions

diff --git a/net/socket/client_socket_pool_base_unittest.cc b/net/socket/client_socket_pool_base_unittest.cc
index db61140..6f3f5ef 100644
--- a/net/socket/client_socket_pool_base_unittest.cc
+++ b/net/socket/client_socket_pool_base_unittest.cc
@@ -981,7 +981,16 @@ class RequestSocketCallback : public CallbackRunner< Tuple1<int> > {
 
     if (!within_callback_) {
       test_connect_job_factory_->set_job_type(next_job_type_);
+
+      // Don't allow reuse of the socket.  Disconnect it and then release it and
+      // run through the MessageLoop once to get it completely released.
+      handle_->socket()->Disconnect();
       handle_->Reset();
+      {
+        MessageLoop::ScopedNestableTaskAllower nestable(
+            MessageLoop::current());
+        MessageLoop::current()->RunAllPending();
+      }
       within_callback_ = true;
       TestCompletionCallback next_job_callback;
       int rv = InitHandle(
@@ -1000,9 +1009,12 @@ class RequestSocketCallback : public CallbackRunner< Tuple1<int> > {
           // We need to give it a little bit of time to run, so that all the
           // operations that happen on timers (e.g. cleanup of idle
           // connections) can execute.
-          MessageLoop::current()->SetNestableTasksAllowed(true);
-          PlatformThread::Sleep(10);
-          EXPECT_EQ(OK, next_job_callback.WaitForResult());
+          {
+            MessageLoop::ScopedNestableTaskAllower nestable(
+                MessageLoop::current());
+            PlatformThread::Sleep(10);
+            EXPECT_EQ(OK, next_job_callback.WaitForResult());
+          }
           break;
         default:
           FAIL() << "Unexpected job type: " << next_job_type_;
@@ -1037,7 +1049,6 @@ TEST_F(ClientSocketPoolBaseTest, RequestPendingJobTwice) {
   ASSERT_EQ(ERR_IO_PENDING, rv);
 
   EXPECT_EQ(OK, callback.WaitForResult());
-  handle.Reset();
 }
 
 TEST_F(ClientSocketPoolBaseTest, RequestPendingJobThenSynchronous) {
@@ -1052,7 +1063,6 @@ TEST_F(ClientSocketPoolBaseTest, RequestPendingJobThenSynchronous) {
   ASSERT_EQ(ERR_IO_PENDING, rv);
 
   EXPECT_EQ(OK, callback.WaitForResult());
-  handle.Reset();
 }
 
 // Make sure that pending requests get serviced after active requests get
@@ -1598,7 +1608,6 @@ TEST_F(ClientSocketPoolBaseTest_LateBinding, RequestPendingJobTwice) {
   ASSERT_EQ(ERR_IO_PENDING, rv);
 
   EXPECT_EQ(OK, callback.WaitForResult());
-  handle.Reset();
 }
 
 TEST_F(ClientSocketPoolBaseTest_LateBinding, RequestPendingJobThenSynchronous) {
@@ -1614,7 +1623,6 @@ TEST_F(ClientSocketPoolBaseTest_LateBinding, RequestPendingJobThenSynchronous) {
   ASSERT_EQ(ERR_IO_PENDING, rv);
 
   EXPECT_EQ(OK, callback.WaitForResult());
-  handle.Reset();
 }
 
 // Make sure that pending requests get serviced after active requests get