Make sure the socket is deleted when ConnectJob times out.

It's benign since ClientSocketPoolBase has a scoped_ptr that will delete it anyway, but it hits a DCHECK since ConnectJob::ReleaseSocket() should return NULL when the ConnectJob encountered an error.
Update TestConnectJobDelegate to call ConnectJob::ReleaseSocket() and EXPECT on its value.
Replicate the ConnectJob tests for the late binding case (doesn't add any extra coverage since we're not executing ClientSocketPoolBase code, just ConnectJob, but this way I don't accidentally delete necessary tests when I delete all the old tests after switching over completely to late binding).
BUG=none
TEST=net_unittests

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

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

2 files changed, 48 insertions, 1 deletions

diff --git a/net/socket/client_socket_pool_base.cc b/net/socket/client_socket_pool_base.cc
index 00b9753..ec32383 100644
--- a/net/socket/client_socket_pool_base.cc
+++ b/net/socket/client_socket_pool_base.cc
@@ -55,6 +55,8 @@ int ConnectJob::Connect() {
 }
 
 void ConnectJob::OnTimeout() {
+  // Make sure the socket is NULL before calling into |delegate|.
+  set_socket(NULL);
   // The delegate will delete |this|.
   Delegate *delegate = delegate_;
   delegate_ = NULL;
diff --git a/net/socket/client_socket_pool_base_unittest.cc b/net/socket/client_socket_pool_base_unittest.cc
index d922b2b..dd417ae 100644
--- a/net/socket/client_socket_pool_base_unittest.cc
+++ b/net/socket/client_socket_pool_base_unittest.cc
@@ -126,6 +126,7 @@ class TestConnectJob : public ConnectJob {
   virtual int ConnectInternal() {
     AddressList ignored;
     client_socket_factory_->CreateTCPClientSocket(ignored);
+    set_socket(new MockClientSocket());
     switch (job_type_) {
       case kMockJob:
         return DoConnect(true /* successful */, false /* sync */);
@@ -161,6 +162,7 @@ class TestConnectJob : public ConnectJob {
         return ERR_IO_PENDING;
       default:
         NOTREACHED();
+        set_socket(NULL);
         return ERR_FAILED;
     }
   }
@@ -169,8 +171,9 @@ class TestConnectJob : public ConnectJob {
     int result = ERR_CONNECTION_FAILED;
     if (succeed) {
       result = OK;
-      set_socket(new MockClientSocket());
       socket()->Connect(NULL);
+    } else {
+      set_socket(NULL);
     }
 
     if (was_async)
@@ -309,6 +312,12 @@ class TestConnectJobDelegate : public ConnectJob::Delegate {
 
   virtual void OnConnectJobComplete(int result, ConnectJob* job) {
     result_ = result;
+    scoped_ptr<ClientSocket> socket(job->ReleaseSocket());
+    if (result == OK) {
+      EXPECT_TRUE(socket.get() != NULL);
+    } else {
+      EXPECT_EQ(NULL, socket.get());
+    }
     delete job;
     have_result_ = true;
     if (waiting_for_result_)
@@ -1094,6 +1103,42 @@ class ClientSocketPoolBaseTest_LateBinding : public ClientSocketPoolBaseTest {
   }
 };
 
+// Even though a timeout is specified, it doesn't time out on a synchronous
+// completion.
+TEST_F(ClientSocketPoolBaseTest_LateBinding,
+       ConnectJob_NoTimeoutOnSynchronousCompletion) {
+  TestConnectJobDelegate delegate;
+  ClientSocketPoolBase::Request request;
+  ClientSocketHandle ignored(pool_.get());
+  request.handle = &ignored;
+  scoped_ptr<TestConnectJob> job(
+      new TestConnectJob(TestConnectJob::kMockJob,
+                         "a", 
+                         request,
+                         base::TimeDelta::FromMicroseconds(1),
+                         &delegate,
+                         &client_socket_factory_));
+  EXPECT_EQ(OK, job->Connect());
+}
+
+TEST_F(ClientSocketPoolBaseTest_LateBinding, ConnectJob_TimedOut) {
+  TestConnectJobDelegate delegate;
+  ClientSocketPoolBase::Request request;
+  ClientSocketHandle ignored(pool_.get());
+  request.handle = &ignored;
+  // Deleted by TestConnectJobDelegate.
+  TestConnectJob* job =
+      new TestConnectJob(TestConnectJob::kMockPendingJob,
+                         "a", 
+                         request,
+                         base::TimeDelta::FromMicroseconds(1),
+                         &delegate,
+                         &client_socket_factory_);
+  ASSERT_EQ(ERR_IO_PENDING, job->Connect());
+  PlatformThread::Sleep(1);
+  EXPECT_EQ(ERR_TIMED_OUT, delegate.WaitForResult());
+}
+
 TEST_F(ClientSocketPoolBaseTest_LateBinding, BasicSynchronous) {
   CreatePool(kDefaultMaxSockets, kDefaultMaxSocketsPerGroup);