Fix to bug 39063.

The problem was the backup connect job was failed; and we didn't
properly track the backup_job in the group jobs list.

BUG=39063
TEST=TCPClientSocketPoolTest.BackupConnectFail

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

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

2 files changed, 98 insertions, 7 deletions

diff --git a/net/socket/client_socket_pool_base.cc b/net/socket/client_socket_pool_base.cc
index c3172e5..d6a8caf 100644
--- a/net/socket/client_socket_pool_base.cc
+++ b/net/socket/client_socket_pool_base.cc
@@ -307,13 +307,12 @@ void ClientSocketPoolBaseHelper::OnBackupSocketTimerFired(
   group.backup_job->net_log().AddEvent(NetLog::TYPE_SOCKET_BACKUP_CREATED);
   SIMPLE_STATS_COUNTER("socket.backup_created");
   int rv = group.backup_job->Connect();
-  if (rv == ERR_IO_PENDING) {
-    connecting_socket_count_++;
-    group.jobs.insert(group.backup_job);
-    group.backup_job = NULL;
-  } else {
-    OnConnectJobComplete(rv, group.backup_job);
-  }
+  connecting_socket_count_++;
+  group.jobs.insert(group.backup_job);
+  ConnectJob* job = group.backup_job;
+  group.backup_job = NULL;
+  if (rv != ERR_IO_PENDING)
+    OnConnectJobComplete(rv, job);
 }
 
 void ClientSocketPoolBaseHelper::CancelRequest(
diff --git a/net/socket/tcp_client_socket_pool_unittest.cc b/net/socket/tcp_client_socket_pool_unittest.cc
index 4ef6b98..e7c8a9f 100644
--- a/net/socket/tcp_client_socket_pool_unittest.cc
+++ b/net/socket/tcp_client_socket_pool_unittest.cc
@@ -228,6 +228,7 @@ class MockClientSocketFactory : public ClientSocketFactory {
   // Set a list of ClientSocketTypes to be used.
   void set_client_socket_types(ClientSocketType* type_list) {
     client_socket_types_ = type_list;
+    client_socket_index_ = 0;
   }
 
  private:
@@ -750,6 +751,97 @@ TEST_F(TCPClientSocketPoolTest, BackupSocketCancel) {
   }
 }
 
+// Test the case where a socket took long enough to start the creation
+// of the backup socket and never completes, and then the backup
+// connection fails.
+TEST_F(TCPClientSocketPoolTest, BackupSocketFailAfterStall) {
+  MockClientSocketFactory::ClientSocketType case_types[] = {
+    // The first socket will not connect.
+    MockClientSocketFactory::MOCK_STALLED_CLIENT_SOCKET,
+    // The second socket will fail immediately.
+    MockClientSocketFactory::MOCK_FAILING_CLIENT_SOCKET
+  };
+
+  client_socket_factory_.set_client_socket_types(case_types);
+
+  EXPECT_EQ(0, pool_->IdleSocketCount());
+
+  TestCompletionCallback callback;
+  ClientSocketHandle handle;
+  TCPSocketParams dest("www.google.com", 80, LOW, GURL(), false);
+  int rv = handle.Init("b", dest, LOW, &callback, pool_, NULL);
+  EXPECT_EQ(ERR_IO_PENDING, rv);
+  EXPECT_FALSE(handle.is_initialized());
+  EXPECT_FALSE(handle.socket());
+
+  // Create the first socket, set the timer.
+  MessageLoop::current()->RunAllPending();
+
+  // Wait for the backup socket timer to fire.
+  PlatformThread::Sleep(ClientSocketPool::kMaxConnectRetryIntervalMs);
+
+  // Let the second connect be synchronous. Otherwise, the emulated
+  // host resolution takes an extra trip through the message loop.
+  host_resolver_->set_synchronous_mode(true);
+
+  // Let the appropriate socket connect.
+  MessageLoop::current()->RunAllPending();
+
+  EXPECT_EQ(ERR_CONNECTION_FAILED, callback.WaitForResult());
+  EXPECT_FALSE(handle.is_initialized());
+  EXPECT_FALSE(handle.socket());
+  EXPECT_EQ(0, pool_->IdleSocketCount());
+  handle.Reset();
+
+  // Reset for the next case.
+  host_resolver_->set_synchronous_mode(false);
+}
+
+// Test the case where a socket took long enough to start the creation
+// of the backup socket and eventually completes, but the backup socket
+// fails.
+TEST_F(TCPClientSocketPoolTest, BackupSocketFailAfterDelay) {
+  MockClientSocketFactory::ClientSocketType case_types[] = {
+    // The first socket will connect, although delayed.
+    MockClientSocketFactory::MOCK_DELAYED_CLIENT_SOCKET,
+    // The second socket will not connect.
+    MockClientSocketFactory::MOCK_FAILING_CLIENT_SOCKET
+  };
+
+  client_socket_factory_.set_client_socket_types(case_types);
+
+  EXPECT_EQ(0, pool_->IdleSocketCount());
+
+  TestCompletionCallback callback;
+  ClientSocketHandle handle;
+  TCPSocketParams dest("www.google.com", 80, LOW, GURL(), false);
+  int rv = handle.Init("b", dest, LOW, &callback, pool_, NULL);
+  EXPECT_EQ(ERR_IO_PENDING, rv);
+  EXPECT_FALSE(handle.is_initialized());
+  EXPECT_FALSE(handle.socket());
+
+  // Create the first socket, set the timer.
+  MessageLoop::current()->RunAllPending();
+
+  // Wait for the backup socket timer to fire.
+  PlatformThread::Sleep(ClientSocketPool::kMaxConnectRetryIntervalMs);
+
+  // Let the second connect be synchronous. Otherwise, the emulated
+  // host resolution takes an extra trip through the message loop.
+  host_resolver_->set_synchronous_mode(true);
+
+  // Let the appropriate socket connect.
+  MessageLoop::current()->RunAllPending();
+
+  EXPECT_EQ(ERR_CONNECTION_FAILED, callback.WaitForResult());
+  EXPECT_FALSE(handle.is_initialized());
+  EXPECT_FALSE(handle.socket());
+  handle.Reset();
+
+  // Reset for the next case.
+  host_resolver_->set_synchronous_mode(false);
+}
+
 }  // namespace
 
 }  // namespace net