Fix ClientSocketPoolBaseTest.RequestSocketsMultipleTimesDoesNothing.

This CL switches from tracking "used" and "unused" preconnects to
tracking total number of unassigned jobs instead.  When a new request
is issued, if there's an unassigned job, we decrement the unassigned
job count.  We also no longer keep track of exactly which jobs are
unassigned, since late binding makes that largely irrelevant, and it
would lead to some oddly different behaviors for ConnectJobs that
began life as preconnects and those that didn't.

The change in behavior is subtle enough that all old tests still pass.

R=willchan@chromium.org
BUG=129365

Review URL: https://chromiumcodereview.appspot.com/10440031

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

3 files changed, 126 insertions, 70 deletions

diff --git a/net/socket/client_socket_pool_base.cc b/net/socket/client_socket_pool_base.cc
index 521436f..90f52f7 100644
--- a/net/socket/client_socket_pool_base.cc
+++ b/net/socket/client_socket_pool_base.cc
@@ -74,8 +74,7 @@ ConnectJob::ConnectJob(const std::string& group_name,
       timeout_duration_(timeout_duration),
       delegate_(delegate),
       net_log_(net_log),
-      idle_(true),
-      preconnect_state_(NOT_PRECONNECT) {
+      idle_(true) {
   DCHECK(!group_name.empty());
   DCHECK(delegate);
   net_log.BeginEvent(NetLog::TYPE_SOCKET_POOL_CONNECT_JOB, NULL);
@@ -85,13 +84,6 @@ ConnectJob::~ConnectJob() {
   net_log().EndEvent(NetLog::TYPE_SOCKET_POOL_CONNECT_JOB, NULL);
 }
 
-void ConnectJob::Initialize(bool is_preconnect) {
-  if (is_preconnect)
-    preconnect_state_ = UNUSED_PRECONNECT;
-  else
-    preconnect_state_ = NOT_PRECONNECT;
-}
-
 int ConnectJob::Connect() {
   if (timeout_duration_ != base::TimeDelta())
     timer_.Start(FROM_HERE, timeout_duration_, this, &ConnectJob::OnTimeout);
@@ -110,11 +102,6 @@ int ConnectJob::Connect() {
   return rv;
 }
 
-void ConnectJob::UseForNormalRequest() {
-  DCHECK_EQ(UNUSED_PRECONNECT, preconnect_state_);
-  preconnect_state_ = USED_PRECONNECT;
-}
-
 void ConnectJob::set_socket(StreamSocket* socket) {
   if (socket) {
     net_log().AddEvent(NetLog::TYPE_CONNECT_JOB_SET_SOCKET, make_scoped_refptr(
@@ -338,11 +325,14 @@ int ClientSocketPoolBaseHelper::RequestSocketInternal(
 
   if (!(request->flags() & NO_IDLE_SOCKETS)) {
     // Try to reuse a socket.
-    if (AssignIdleSocketToGroup(request, group))
+    if (AssignIdleSocketToRequest(request, group))
       return OK;
   }
 
-  if (!preconnecting && group->TryToUsePreconnectConnectJob())
+  // If there are more ConnectJobs than pending requests, don't need to do
+  // anything.  Can just wait for the extra job to connect, and then assign it
+  // to the request.
+  if (!preconnecting && group->TryToUseUnassignedConnectJob())
     return ERR_IO_PENDING;
 
   // Can we make another active socket now?
@@ -381,7 +371,6 @@ int ClientSocketPoolBaseHelper::RequestSocketInternal(
   scoped_ptr<ConnectJob> connect_job(
       connect_job_factory_->NewConnectJob(group_name, *request, this));
 
-  connect_job->Initialize(preconnecting);
   int rv = connect_job->Connect();
   if (rv == OK) {
     LogBoundConnectJobToRequest(connect_job->net_log().source(), request);
@@ -402,7 +391,7 @@ int ClientSocketPoolBaseHelper::RequestSocketInternal(
 
     connecting_socket_count_++;
 
-    group->AddJob(connect_job.release());
+    group->AddJob(connect_job.release(), preconnecting);
   } else {
     LogBoundConnectJobToRequest(connect_job->net_log().source(), request);
     StreamSocket* error_socket = NULL;
@@ -422,7 +411,7 @@ int ClientSocketPoolBaseHelper::RequestSocketInternal(
   return rv;
 }
 
-bool ClientSocketPoolBaseHelper::AssignIdleSocketToGroup(
+bool ClientSocketPoolBaseHelper::AssignIdleSocketToRequest(
     const Request* request, Group* group) {
   std::list<IdleSocket>* idle_sockets = group->mutable_idle_sockets();
   std::list<IdleSocket>::iterator idle_socket_it = idle_sockets->end();
@@ -1142,11 +1131,13 @@ void ClientSocketPoolBaseHelper::InvokeUserCallback(
 }
 
 ClientSocketPoolBaseHelper::Group::Group()
-    : active_socket_count_(0),
+    : unassigned_job_count_(0),
+      active_socket_count_(0),
       ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)) {}
 
 ClientSocketPoolBaseHelper::Group::~Group() {
   CleanupBackupJob();
+  DCHECK_EQ(0u, unassigned_job_count_);
 }
 
 void ClientSocketPoolBaseHelper::Group::StartBackupSocketTimer(
@@ -1163,16 +1154,31 @@ void ClientSocketPoolBaseHelper::Group::StartBackupSocketTimer(
       pool->ConnectRetryInterval());
 }
 
-bool ClientSocketPoolBaseHelper::Group::TryToUsePreconnectConnectJob() {
-  for (std::set<ConnectJob*>::iterator it = jobs_.begin();
-       it != jobs_.end(); ++it) {
-    ConnectJob* job = *it;
-    if (job->is_unused_preconnect()) {
-      job->UseForNormalRequest();
-      return true;
-    }
-  }
-  return false;
+bool ClientSocketPoolBaseHelper::Group::TryToUseUnassignedConnectJob() {
+  SanityCheck();
+
+  if (unassigned_job_count_ == 0)
+    return false;
+  --unassigned_job_count_;
+  return true;
+}
+
+void ClientSocketPoolBaseHelper::Group::AddJob(ConnectJob* job,
+                                               bool is_preconnect) {
+  SanityCheck();
+
+  if (is_preconnect)
+    ++unassigned_job_count_;
+  jobs_.insert(job);
+}
+
+void ClientSocketPoolBaseHelper::Group::RemoveJob(ConnectJob* job) {
+  SanityCheck();
+
+  jobs_.erase(job);
+  size_t job_count = jobs_.size();
+  if (job_count < unassigned_job_count_)
+    unassigned_job_count_ = job_count;
 }
 
 void ClientSocketPoolBaseHelper::Group::OnBackupSocketTimerFired(
@@ -1185,7 +1191,7 @@ void ClientSocketPoolBaseHelper::Group::OnBackupSocketTimerFired(
     return;
   }
 
-  // If our backup job is waiting on DNS, or if we can't create any sockets
+  // If our old job is waiting on DNS, or if we can't create any sockets
   // right now due to limits, just reset the timer.
   if (pool->ReachedMaxSocketsLimit() ||
       !HasAvailableSocketSlot(pool->max_sockets_per_group_) ||
@@ -1203,14 +1209,21 @@ void ClientSocketPoolBaseHelper::Group::OnBackupSocketTimerFired(
   SIMPLE_STATS_COUNTER("socket.backup_created");
   int rv = backup_job->Connect();
   pool->connecting_socket_count_++;
-  AddJob(backup_job);
+  AddJob(backup_job, false);
   if (rv != ERR_IO_PENDING)
     pool->OnConnectJobComplete(rv, backup_job);
 }
 
+void ClientSocketPoolBaseHelper::Group::SanityCheck() {
+  DCHECK_LE(unassigned_job_count_, jobs_.size());
+}
+
 void ClientSocketPoolBaseHelper::Group::RemoveAllJobs() {
+  SanityCheck();
+
   // Delete active jobs.
   STLDeleteElements(&jobs_);
+  unassigned_job_count_ = 0;
 
   // Cancel pending backup job.
   weak_factory_.InvalidateWeakPtrs();
diff --git a/net/socket/client_socket_pool_base.h b/net/socket/client_socket_pool_base.h
index 832c166..0775dc59 100644
--- a/net/socket/client_socket_pool_base.h
+++ b/net/socket/client_socket_pool_base.h
@@ -85,16 +85,6 @@ class NET_EXPORT_PRIVATE ConnectJob {
   // Accessors
   const std::string& group_name() const { return group_name_; }
   const BoundNetLog& net_log() { return net_log_; }
-  bool is_preconnect() const { return preconnect_state_ != NOT_PRECONNECT; }
-  bool is_unused_preconnect() const {
-    return preconnect_state_ == UNUSED_PRECONNECT;
-  }
-
-  // Initialized by the ClientSocketPoolBaseHelper.
-  // TODO(willchan): Move most of the constructor arguments over here.  We
-  // shouldn't give the ConnectJobFactory (subclasses) the ability to screw up
-  // the initialization.
-  void Initialize(bool is_preconnect);
 
   // Releases |socket_| to the client.  On connection error, this should return
   // NULL.
@@ -108,10 +98,6 @@ class NET_EXPORT_PRIVATE ConnectJob {
   // if it succeeded.
   int Connect();
 
-  // Precondition: is_unused_preconnect() must be true.  Marks the job as a
-  // used preconnect job.
-  void UseForNormalRequest();
-
   virtual LoadState GetLoadState() const = 0;
 
   // If Connect returns an error (or OnConnectJobComplete reports an error
@@ -128,12 +114,6 @@ class NET_EXPORT_PRIVATE ConnectJob {
   void ResetTimer(base::TimeDelta remainingTime);
 
  private:
-  enum PreconnectState {
-    NOT_PRECONNECT,
-    UNUSED_PRECONNECT,
-    USED_PRECONNECT,
-  };
-
   virtual int ConnectInternal() = 0;
 
   void LogConnectStart();
@@ -151,7 +131,6 @@ class NET_EXPORT_PRIVATE ConnectJob {
   BoundNetLog net_log_;
   // A ConnectJob is idle until Connect() has been called.
   bool idle_;
-  PreconnectState preconnect_state_;
 
   DISALLOW_COPY_AND_ASSIGN(ConnectJob);
 };
@@ -293,6 +272,10 @@ class NET_EXPORT_PRIVATE ClientSocketPoolBaseHelper
         ClientSocketPool::kMaxConnectRetryIntervalMs);
   }
 
+  int NumUnassignedConnectJobsInGroup(const std::string& group_name) const {
+    return group_map_.find(group_name)->second->unassigned_job_count();
+  }
+
   int NumConnectJobsInGroup(const std::string& group_name) const {
     return group_map_.find(group_name)->second->jobs().size();
   }
@@ -409,17 +392,19 @@ class NET_EXPORT_PRIVATE ClientSocketPoolBaseHelper
     void StartBackupSocketTimer(const std::string& group_name,
                                 ClientSocketPoolBaseHelper* pool);
 
-    // Searches |jobs_| to see if there's a preconnect ConnectJob, and if so,
-    // uses it.  Returns true on success.  Otherwise, returns false.
-    bool TryToUsePreconnectConnectJob();
+    // If there's a ConnectJob that's never been assigned to Request,
+    // decrements |unassigned_job_count_| and returns true.
+    // Otherwise, returns false.
+    bool TryToUseUnassignedConnectJob();
 
-    void AddJob(ConnectJob* job) { jobs_.insert(job); }
-    void RemoveJob(ConnectJob* job) { jobs_.erase(job); }
+    void AddJob(ConnectJob* job, bool is_preconnect);
+    void RemoveJob(ConnectJob* job);
     void RemoveAllJobs();
 
     void IncrementActiveSocketCount() { active_socket_count_++; }
     void DecrementActiveSocketCount() { active_socket_count_--; }
 
+    int unassigned_job_count() const { return unassigned_job_count_; }
     const std::set<ConnectJob*>& jobs() const { return jobs_; }
     const std::list<IdleSocket>& idle_sockets() const { return idle_sockets_; }
     const RequestQueue& pending_requests() const { return pending_requests_; }
@@ -433,6 +418,18 @@ class NET_EXPORT_PRIVATE ClientSocketPoolBaseHelper
         std::string group_name,
         ClientSocketPoolBaseHelper* pool);
 
+    // Checks that |unassigned_job_count_| does not execeed the number of
+    // ConnectJobs.
+    void SanityCheck();
+
+    // Total number of ConnectJobs that have never been assigned to a Request.
+    // Since jobs use late binding to requests, which ConnectJobs have or have
+    // not been assigned to a request are not tracked.  This is incremented on
+    // preconnect and decremented when a preconnect is assigned, or when there
+    // are fewer than |unassigned_job_count_| ConnectJobs.  Not incremented
+    // when a request is cancelled.
+    size_t unassigned_job_count_;
+
     std::list<IdleSocket> idle_sockets_;
     std::set<ConnectJob*> jobs_;
     RequestQueue pending_requests_;
@@ -525,7 +522,7 @@ class NET_EXPORT_PRIVATE ClientSocketPoolBaseHelper
 
   // Assigns an idle socket for the group to the request.
   // Returns |true| if an idle socket is available, false otherwise.
-  bool AssignIdleSocketToGroup(const Request* request, Group* group);
+  bool AssignIdleSocketToRequest(const Request* request, Group* group);
 
   static void LogBoundConnectJobToRequest(
       const NetLog::Source& connect_job_source, const Request* request);
@@ -734,6 +731,10 @@ class ClientSocketPoolBase {
     return helper_.OnConnectJobComplete(result, job);
   }
 
+  int NumUnassignedConnectJobsInGroup(const std::string& group_name) const {
+    return helper_.NumUnassignedConnectJobsInGroup(group_name);
+  }
+
   int NumConnectJobsInGroup(const std::string& group_name) const {
     return helper_.NumConnectJobsInGroup(group_name);
   }
diff --git a/net/socket/client_socket_pool_base_unittest.cc b/net/socket/client_socket_pool_base_unittest.cc
index eec553f..212438c 100644
--- a/net/socket/client_socket_pool_base_unittest.cc
+++ b/net/socket/client_socket_pool_base_unittest.cc
@@ -525,6 +525,10 @@ class TestClientSocketPool : public ClientSocketPool {
 
   const TestClientSocketPoolBase* base() const { return &base_; }
 
+  int NumUnassignedConnectJobsInGroup(const std::string& group_name) const {
+    return base_.NumUnassignedConnectJobsInGroup(group_name);
+  }
+
   int NumConnectJobsInGroup(const std::string& group_name) const {
     return base_.NumConnectJobsInGroup(group_name);
   }
@@ -1182,12 +1186,14 @@ TEST_F(ClientSocketPoolBaseTest, CancelPendingSocketAtSocketLimit) {
 
     // Since it is stalled, it should have no connect jobs.
     EXPECT_EQ(0, pool_->NumConnectJobsInGroup("foo"));
+    EXPECT_EQ(0, pool_->NumUnassignedConnectJobsInGroup("foo"));
 
     // Cancel the stalled request.
     handles[0].Reset();
 
     // Now we should have a connect job.
     EXPECT_EQ(1, pool_->NumConnectJobsInGroup("foo"));
+    EXPECT_EQ(0, pool_->NumUnassignedConnectJobsInGroup("foo"));
 
     // The stalled socket should connect.
     EXPECT_EQ(OK, callback.WaitForResult());
@@ -1196,6 +1202,7 @@ TEST_F(ClientSocketPoolBaseTest, CancelPendingSocketAtSocketLimit) {
               client_socket_factory_.allocation_count());
     EXPECT_EQ(0, pool_->IdleSocketCount());
     EXPECT_EQ(0, pool_->NumConnectJobsInGroup("foo"));
+    EXPECT_EQ(0, pool_->NumUnassignedConnectJobsInGroup("foo"));
 
     // Dropping out of scope will close all handles and return them to idle.
   }
@@ -2641,6 +2648,7 @@ TEST_F(ClientSocketPoolBaseTest, CancelBackupSocketAfterCancelingAllRequests) {
                                         BoundNetLog()));
   ASSERT_TRUE(pool_->HasGroup("bar"));
   EXPECT_EQ(1, pool_->NumConnectJobsInGroup("bar"));
+  EXPECT_EQ(0, pool_->NumUnassignedConnectJobsInGroup("bar"));
 
   // Cancel the socket request.  This should cancel the backup timer.  Wait for
   // the backup time to see if it indeed got canceled.
@@ -2987,6 +2995,7 @@ TEST_F(ClientSocketPoolBaseTest, RequestSockets) {
 
   ASSERT_TRUE(pool_->HasGroup("a"));
   EXPECT_EQ(2, pool_->NumConnectJobsInGroup("a"));
+  EXPECT_EQ(2, pool_->NumUnassignedConnectJobsInGroup("a"));
   EXPECT_EQ(0, pool_->IdleSocketCountInGroup("a"));
 
   ClientSocketHandle handle1;
@@ -3008,6 +3017,7 @@ TEST_F(ClientSocketPoolBaseTest, RequestSockets) {
                                          BoundNetLog()));
 
   EXPECT_EQ(2, pool_->NumConnectJobsInGroup("a"));
+  EXPECT_EQ(0, pool_->NumUnassignedConnectJobsInGroup("a"));
   EXPECT_EQ(0, pool_->IdleSocketCountInGroup("a"));
 
   EXPECT_EQ(OK, callback1.WaitForResult());
@@ -3016,6 +3026,7 @@ TEST_F(ClientSocketPoolBaseTest, RequestSockets) {
   handle2.Reset();
 
   EXPECT_EQ(0, pool_->NumConnectJobsInGroup("a"));
+  EXPECT_EQ(0, pool_->NumUnassignedConnectJobsInGroup("a"));
   EXPECT_EQ(2, pool_->IdleSocketCountInGroup("a"));
 }
 
@@ -3034,11 +3045,13 @@ TEST_F(ClientSocketPoolBaseTest, RequestSocketsWhenAlreadyHaveAConnectJob) {
 
   ASSERT_TRUE(pool_->HasGroup("a"));
   EXPECT_EQ(1, pool_->NumConnectJobsInGroup("a"));
+  EXPECT_EQ(0, pool_->NumUnassignedConnectJobsInGroup("a"));
   EXPECT_EQ(0, pool_->IdleSocketCountInGroup("a"));
 
   pool_->RequestSockets("a", &params_, 2, BoundNetLog());
 
   EXPECT_EQ(2, pool_->NumConnectJobsInGroup("a"));
+  EXPECT_EQ(1, pool_->NumUnassignedConnectJobsInGroup("a"));
   EXPECT_EQ(0, pool_->IdleSocketCountInGroup("a"));
 
   ClientSocketHandle handle2;
@@ -3051,6 +3064,7 @@ TEST_F(ClientSocketPoolBaseTest, RequestSocketsWhenAlreadyHaveAConnectJob) {
                                          BoundNetLog()));
 
   EXPECT_EQ(2, pool_->NumConnectJobsInGroup("a"));
+  EXPECT_EQ(0, pool_->NumUnassignedConnectJobsInGroup("a"));
   EXPECT_EQ(0, pool_->IdleSocketCountInGroup("a"));
 
   EXPECT_EQ(OK, callback1.WaitForResult());
@@ -3059,6 +3073,7 @@ TEST_F(ClientSocketPoolBaseTest, RequestSocketsWhenAlreadyHaveAConnectJob) {
   handle2.Reset();
 
   EXPECT_EQ(0, pool_->NumConnectJobsInGroup("a"));
+  EXPECT_EQ(0, pool_->NumUnassignedConnectJobsInGroup("a"));
   EXPECT_EQ(2, pool_->IdleSocketCountInGroup("a"));
 }
 
@@ -3096,11 +3111,13 @@ TEST_F(ClientSocketPoolBaseTest,
 
   ASSERT_TRUE(pool_->HasGroup("a"));
   EXPECT_EQ(3, pool_->NumConnectJobsInGroup("a"));
+  EXPECT_EQ(0, pool_->NumUnassignedConnectJobsInGroup("a"));
   EXPECT_EQ(0, pool_->IdleSocketCountInGroup("a"));
 
   pool_->RequestSockets("a", &params_, 2, BoundNetLog());
 
   EXPECT_EQ(3, pool_->NumConnectJobsInGroup("a"));
+  EXPECT_EQ(0, pool_->NumUnassignedConnectJobsInGroup("a"));
   EXPECT_EQ(0, pool_->IdleSocketCountInGroup("a"));
 
   EXPECT_EQ(OK, callback1.WaitForResult());
@@ -3111,6 +3128,7 @@ TEST_F(ClientSocketPoolBaseTest,
   handle3.Reset();
 
   EXPECT_EQ(0, pool_->NumConnectJobsInGroup("a"));
+  EXPECT_EQ(0, pool_->NumUnassignedConnectJobsInGroup("a"));
   EXPECT_EQ(3, pool_->IdleSocketCountInGroup("a"));
 }
 
@@ -3125,6 +3143,7 @@ TEST_F(ClientSocketPoolBaseTest, RequestSocketsAtMaxSocketLimit) {
 
   ASSERT_TRUE(pool_->HasGroup("a"));
   EXPECT_EQ(kDefaultMaxSockets, pool_->NumConnectJobsInGroup("a"));
+  EXPECT_EQ(kDefaultMaxSockets, pool_->NumUnassignedConnectJobsInGroup("a"));
 
   ASSERT_FALSE(pool_->HasGroup("b"));
 
@@ -3145,6 +3164,8 @@ TEST_F(ClientSocketPoolBaseTest, RequestSocketsHitMaxSocketLimit) {
 
   ASSERT_TRUE(pool_->HasGroup("a"));
   EXPECT_EQ(kDefaultMaxSockets - 1, pool_->NumConnectJobsInGroup("a"));
+  EXPECT_EQ(kDefaultMaxSockets - 1,
+            pool_->NumUnassignedConnectJobsInGroup("a"));
   EXPECT_FALSE(pool_->IsStalled());
 
   ASSERT_FALSE(pool_->HasGroup("b"));
@@ -3174,11 +3195,13 @@ TEST_F(ClientSocketPoolBaseTest, RequestSocketsCountIdleSockets) {
 
   ASSERT_TRUE(pool_->HasGroup("a"));
   EXPECT_EQ(0, pool_->NumConnectJobsInGroup("a"));
+  EXPECT_EQ(0, pool_->NumUnassignedConnectJobsInGroup("a"));
   EXPECT_EQ(1, pool_->IdleSocketCountInGroup("a"));
 
   pool_->RequestSockets("a", &params_, 2, BoundNetLog());
 
   EXPECT_EQ(1, pool_->NumConnectJobsInGroup("a"));
+  EXPECT_EQ(1, pool_->NumUnassignedConnectJobsInGroup("a"));
   EXPECT_EQ(1, pool_->IdleSocketCountInGroup("a"));
 }
 
@@ -3198,12 +3221,14 @@ TEST_F(ClientSocketPoolBaseTest, RequestSocketsCountActiveSockets) {
 
   ASSERT_TRUE(pool_->HasGroup("a"));
   EXPECT_EQ(0, pool_->NumConnectJobsInGroup("a"));
+  EXPECT_EQ(0, pool_->NumUnassignedConnectJobsInGroup("a"));
   EXPECT_EQ(0, pool_->IdleSocketCountInGroup("a"));
   EXPECT_EQ(1, pool_->NumActiveSocketsInGroup("a"));
 
   pool_->RequestSockets("a", &params_, 2, BoundNetLog());
 
   EXPECT_EQ(1, pool_->NumConnectJobsInGroup("a"));
+  EXPECT_EQ(1, pool_->NumUnassignedConnectJobsInGroup("a"));
   EXPECT_EQ(0, pool_->IdleSocketCountInGroup("a"));
   EXPECT_EQ(1, pool_->NumActiveSocketsInGroup("a"));
 }
@@ -3217,12 +3242,14 @@ TEST_F(ClientSocketPoolBaseTest, RequestSocketsSynchronous) {
 
   ASSERT_TRUE(pool_->HasGroup("a"));
   EXPECT_EQ(0, pool_->NumConnectJobsInGroup("a"));
+  EXPECT_EQ(0, pool_->NumUnassignedConnectJobsInGroup("a"));
   EXPECT_EQ(kDefaultMaxSocketsPerGroup, pool_->IdleSocketCountInGroup("a"));
 
   pool_->RequestSockets("b", &params_, kDefaultMaxSocketsPerGroup,
                         BoundNetLog());
 
   EXPECT_EQ(0, pool_->NumConnectJobsInGroup("b"));
+  EXPECT_EQ(0, pool_->NumUnassignedConnectJobsInGroup("b"));
   EXPECT_EQ(kDefaultMaxSocketsPerGroup, pool_->IdleSocketCountInGroup("b"));
 }
 
@@ -3243,17 +3270,7 @@ TEST_F(ClientSocketPoolBaseTest, RequestSocketsSynchronousError) {
   ASSERT_FALSE(pool_->HasGroup("a"));
 }
 
-// http://crbug.com/129364
-#if defined(OS_WIN)
-#define MAYBE_RequestSocketsMultipleTimesDoesNothing \
-    DISABLED_RequestSocketsMultipleTimesDoesNothing
-#else
-#define MAYBE_RequestSocketsMultipleTimesDoesNothing \
-    RequestSocketsMultipleTimesDoesNothing
-#endif
-
-TEST_F(ClientSocketPoolBaseTest,
-       MAYBE_RequestSocketsMultipleTimesDoesNothing) {
+TEST_F(ClientSocketPoolBaseTest, RequestSocketsMultipleTimesDoesNothing) {
   CreatePool(4, 4);
   connect_job_factory_->set_job_type(TestConnectJob::kMockPendingJob);
 
@@ -3261,10 +3278,12 @@ TEST_F(ClientSocketPoolBaseTest,
 
   ASSERT_TRUE(pool_->HasGroup("a"));
   EXPECT_EQ(2, pool_->NumConnectJobsInGroup("a"));
+  EXPECT_EQ(2, pool_->NumUnassignedConnectJobsInGroup("a"));
   EXPECT_EQ(0, pool_->IdleSocketCountInGroup("a"));
 
   pool_->RequestSockets("a", &params_, 2, BoundNetLog());
   EXPECT_EQ(2, pool_->NumConnectJobsInGroup("a"));
+  EXPECT_EQ(2, pool_->NumUnassignedConnectJobsInGroup("a"));
   EXPECT_EQ(0, pool_->IdleSocketCountInGroup("a"));
 
   ClientSocketHandle handle1;
@@ -3290,13 +3309,21 @@ TEST_F(ClientSocketPoolBaseTest,
     EXPECT_EQ(OK, callback2.WaitForResult());
   }
 
+  EXPECT_EQ(0, pool_->NumConnectJobsInGroup("a"));
+  EXPECT_EQ(0, pool_->NumUnassignedConnectJobsInGroup("a"));
+  EXPECT_EQ(2, pool_->NumActiveSocketsInGroup("a"));
+  EXPECT_EQ(0, pool_->IdleSocketCountInGroup("a"));
+
   handle1.Reset();
   handle2.Reset();
 
+  EXPECT_EQ(0, pool_->NumConnectJobsInGroup("a"));
+  EXPECT_EQ(0, pool_->NumUnassignedConnectJobsInGroup("a"));
   EXPECT_EQ(2, pool_->IdleSocketCountInGroup("a"));
 
   pool_->RequestSockets("a", &params_, 2, BoundNetLog());
   EXPECT_EQ(0, pool_->NumConnectJobsInGroup("a"));
+  EXPECT_EQ(0, pool_->NumUnassignedConnectJobsInGroup("a"));
   EXPECT_EQ(2, pool_->IdleSocketCountInGroup("a"));
 }
 
@@ -3308,18 +3335,22 @@ TEST_F(ClientSocketPoolBaseTest, RequestSocketsDifferentNumSockets) {
 
   ASSERT_TRUE(pool_->HasGroup("a"));
   EXPECT_EQ(1, pool_->NumConnectJobsInGroup("a"));
+  EXPECT_EQ(1, pool_->NumUnassignedConnectJobsInGroup("a"));
   EXPECT_EQ(0, pool_->IdleSocketCountInGroup("a"));
 
   pool_->RequestSockets("a", &params_, 2, BoundNetLog());
   EXPECT_EQ(2, pool_->NumConnectJobsInGroup("a"));
+  EXPECT_EQ(2, pool_->NumUnassignedConnectJobsInGroup("a"));
   EXPECT_EQ(0, pool_->IdleSocketCountInGroup("a"));
 
   pool_->RequestSockets("a", &params_, 3, BoundNetLog());
   EXPECT_EQ(3, pool_->NumConnectJobsInGroup("a"));
+  EXPECT_EQ(3, pool_->NumUnassignedConnectJobsInGroup("a"));
   EXPECT_EQ(0, pool_->IdleSocketCountInGroup("a"));
 
   pool_->RequestSockets("a", &params_, 1, BoundNetLog());
   EXPECT_EQ(3, pool_->NumConnectJobsInGroup("a"));
+  EXPECT_EQ(3, pool_->NumUnassignedConnectJobsInGroup("a"));
   EXPECT_EQ(0, pool_->IdleSocketCountInGroup("a"));
 }
 
@@ -3331,6 +3362,7 @@ TEST_F(ClientSocketPoolBaseTest, PreconnectJobsTakenByNormalRequests) {
 
   ASSERT_TRUE(pool_->HasGroup("a"));
   EXPECT_EQ(1, pool_->NumConnectJobsInGroup("a"));
+  EXPECT_EQ(1, pool_->NumUnassignedConnectJobsInGroup("a"));
   EXPECT_EQ(0, pool_->IdleSocketCountInGroup("a"));
 
   ClientSocketHandle handle1;
@@ -3343,6 +3375,7 @@ TEST_F(ClientSocketPoolBaseTest, PreconnectJobsTakenByNormalRequests) {
                                          BoundNetLog()));
 
   EXPECT_EQ(1, pool_->NumConnectJobsInGroup("a"));
+  EXPECT_EQ(0, pool_->NumUnassignedConnectJobsInGroup("a"));
   EXPECT_EQ(0, pool_->IdleSocketCountInGroup("a"));
 
   ASSERT_EQ(OK, callback1.WaitForResult());
@@ -3395,6 +3428,7 @@ TEST_F(ClientSocketPoolBaseTest, PreconnectClosesIdleSocketRemovesGroup) {
   ASSERT_EQ(OK, callback1.WaitForResult());
   ASSERT_EQ(OK, callback2.WaitForResult());
   EXPECT_EQ(0, pool_->IdleSocketCountInGroup("b"));
+  EXPECT_EQ(0, pool_->NumUnassignedConnectJobsInGroup("b"));
   EXPECT_EQ(2, pool_->NumActiveSocketsInGroup("b"));
 
   // Now we have 1 idle socket in "a" and 2 active sockets in "b".  This means
@@ -3404,9 +3438,11 @@ TEST_F(ClientSocketPoolBaseTest, PreconnectClosesIdleSocketRemovesGroup) {
 
   pool_->RequestSockets("a", &params_, 2, BoundNetLog());
   EXPECT_EQ(0, pool_->NumConnectJobsInGroup("a"));
+  EXPECT_EQ(0, pool_->NumUnassignedConnectJobsInGroup("a"));
   EXPECT_EQ(1, pool_->IdleSocketCountInGroup("a"));
   EXPECT_EQ(0, pool_->NumActiveSocketsInGroup("a"));
   EXPECT_EQ(0, pool_->NumConnectJobsInGroup("b"));
+  EXPECT_EQ(0, pool_->NumUnassignedConnectJobsInGroup("b"));
   EXPECT_EQ(0, pool_->IdleSocketCountInGroup("b"));
   EXPECT_EQ(2, pool_->NumActiveSocketsInGroup("b"));
 
@@ -3420,9 +3456,11 @@ TEST_F(ClientSocketPoolBaseTest, PreconnectClosesIdleSocketRemovesGroup) {
 
   pool_->RequestSockets("a", &params_, 2, BoundNetLog());
   EXPECT_EQ(1, pool_->NumConnectJobsInGroup("a"));
+  EXPECT_EQ(1, pool_->NumUnassignedConnectJobsInGroup("a"));
   EXPECT_EQ(1, pool_->IdleSocketCountInGroup("a"));
   EXPECT_EQ(0, pool_->NumActiveSocketsInGroup("a"));
   EXPECT_EQ(0, pool_->NumConnectJobsInGroup("b"));
+  EXPECT_EQ(0, pool_->NumUnassignedConnectJobsInGroup("b"));
   EXPECT_EQ(1, pool_->IdleSocketCountInGroup("b"));
   EXPECT_EQ(0, pool_->NumActiveSocketsInGroup("b"));
 }
@@ -3437,6 +3475,7 @@ TEST_F(ClientSocketPoolBaseTest, PreconnectWithoutBackupJob) {
       base::TimeDelta::FromMilliseconds(500));
   pool_->RequestSockets("a", &params_, 1, BoundNetLog());
   EXPECT_EQ(1, pool_->NumConnectJobsInGroup("a"));
+  EXPECT_EQ(1, pool_->NumUnassignedConnectJobsInGroup("a"));
   EXPECT_EQ(0, pool_->IdleSocketCountInGroup("a"));
 
   // Verify the backup timer doesn't create a backup job, by making
@@ -3457,6 +3496,7 @@ TEST_F(ClientSocketPoolBaseTest, PreconnectWithBackupJob) {
   connect_job_factory_->set_job_type(TestConnectJob::kMockWaitingJob);
   pool_->RequestSockets("a", &params_, 1, BoundNetLog());
   EXPECT_EQ(1, pool_->NumConnectJobsInGroup("a"));
+  EXPECT_EQ(1, pool_->NumUnassignedConnectJobsInGroup("a"));
   EXPECT_EQ(0, pool_->IdleSocketCountInGroup("a"));
   MessageLoop::current()->RunAllPending();
 
@@ -3472,6 +3512,7 @@ TEST_F(ClientSocketPoolBaseTest, PreconnectWithBackupJob) {
                                         BoundNetLog()));
   // Timer has started, but the backup connect job shouldn't be created yet.
   EXPECT_EQ(1, pool_->NumConnectJobsInGroup("a"));
+  EXPECT_EQ(0, pool_->NumUnassignedConnectJobsInGroup("a"));
   EXPECT_EQ(0, pool_->IdleSocketCountInGroup("a"));
   EXPECT_EQ(0, pool_->NumActiveSocketsInGroup("a"));
   ASSERT_EQ(OK, callback.WaitForResult());
@@ -3479,6 +3520,7 @@ TEST_F(ClientSocketPoolBaseTest, PreconnectWithBackupJob) {
   // The hung connect job should still be there, but everything else should be
   // complete.
   EXPECT_EQ(1, pool_->NumConnectJobsInGroup("a"));
+  EXPECT_EQ(0, pool_->NumUnassignedConnectJobsInGroup("a"));
   EXPECT_EQ(0, pool_->IdleSocketCountInGroup("a"));
   EXPECT_EQ(1, pool_->NumActiveSocketsInGroup("a"));
 }