Reland r57100. Only create the backup ConnectJob when it is needed.

Includes a fix.  I forgot that the iterator is invalidated after std::map::erase().

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

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

2 files changed, 216 insertions, 188 deletions

diff --git a/net/socket/client_socket_pool_base.cc b/net/socket/client_socket_pool_base.cc
index 3c2a9c7..70358d1 100644
--- a/net/socket/client_socket_pool_base.cc
+++ b/net/socket/client_socket_pool_base.cc
@@ -142,7 +142,6 @@ ClientSocketPoolBaseHelper::ClientSocketPoolBaseHelper(
       used_idle_socket_timeout_(used_idle_socket_timeout),
       connect_job_factory_(connect_job_factory),
       connect_backup_jobs_enabled_(false),
-      ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)),
       pool_generation_number_(0),
       in_destructor_(false) {
   DCHECK_LE(0, max_sockets_per_group);
@@ -192,7 +191,7 @@ int ClientSocketPoolBaseHelper::RequestSocket(
     const std::string& group_name,
     const Request* request) {
   request->net_log().BeginEvent(NetLog::TYPE_SOCKET_POOL, NULL);
-  Group& group = group_map_[group_name];
+  Group* group = GetOrCreateGroup(group_name);
 
   int rv = RequestSocketInternal(group_name, request);
   if (rv != ERR_IO_PENDING) {
@@ -200,7 +199,7 @@ int ClientSocketPoolBaseHelper::RequestSocket(
     CHECK(!request->handle()->is_initialized());
     delete request;
   } else {
-    InsertRequestIntoQueue(request, &group.pending_requests);
+    InsertRequestIntoQueue(request, group->mutable_pending_requests());
   }
   return rv;
 }
@@ -213,14 +212,14 @@ int ClientSocketPoolBaseHelper::RequestSocketInternal(
   CHECK(callback);
   ClientSocketHandle* const handle = request->handle();
   CHECK(handle);
-  Group& group = group_map_[group_name];
+  Group* group = GetOrCreateGroup(group_name);
 
   // Try to reuse a socket.
-  if (AssignIdleSocketToGroup(&group, request))
+  if (AssignIdleSocketToGroup(request, group))
     return OK;
 
   // Can we make another active socket now?
-  if (!group.HasAvailableSocketSlot(max_sockets_per_group_)) {
+  if (!group->HasAvailableSocketSlot(max_sockets_per_group_)) {
     request->net_log().AddEvent(
         NetLog::TYPE_SOCKET_POOL_STALLED_MAX_SOCKETS_PER_GROUP, NULL);
     return ERR_IO_PENDING;
@@ -246,32 +245,27 @@ int ClientSocketPoolBaseHelper::RequestSocketInternal(
   if (rv == OK) {
     LogBoundConnectJobToRequest(connect_job->net_log().source(), request);
     HandOutSocket(connect_job->ReleaseSocket(), false /* not reused */,
-                  handle, base::TimeDelta(), &group, request->net_log());
+                  handle, base::TimeDelta(), group, request->net_log());
   } else if (rv == ERR_IO_PENDING) {
     // If we don't have any sockets in this group, set a timer for potentially
     // creating a new one.  If the SYN is lost, this backup socket may complete
     // before the slow socket, improving end user latency.
-    if (group.IsEmpty() && !group.connect_backup_job &&
-        connect_backup_jobs_enabled_) {
-      group.connect_backup_job = connect_job_factory_->NewConnectJob(group_name,
-                                                                    *request,
-                                                                     this);
-      StartBackupSocketTimer(group_name);
-    }
+    if (group->IsEmpty() && !group->HasBackupJob() &&
+        connect_backup_jobs_enabled_)
+      group->StartBackupSocketTimer(group_name, this);
 
     connecting_socket_count_++;
 
-    ConnectJob* job = connect_job.release();
-    group.jobs.insert(job);
+    group->AddJob(connect_job.release());
   } else {
     LogBoundConnectJobToRequest(connect_job->net_log().source(), request);
     connect_job->GetAdditionalErrorState(handle);
     ClientSocket* error_socket = connect_job->ReleaseSocket();
     if (error_socket) {
       HandOutSocket(error_socket, false /* not reused */, handle,
-                    base::TimeDelta(), &group, request->net_log());
-    } else if (group.IsEmpty()) {
-      group_map_.erase(group_name);
+                    base::TimeDelta(), group, request->net_log());
+    } else if (group->IsEmpty()) {
+      RemoveGroup(group_name);
     }
   }
 
@@ -279,12 +273,12 @@ int ClientSocketPoolBaseHelper::RequestSocketInternal(
 }
 
 bool ClientSocketPoolBaseHelper::AssignIdleSocketToGroup(
-    Group* group, const Request* request) {
+    const Request* request, Group* group) {
   // Iterate through the list of idle sockets until we find one or exhaust
   // the list.
-  while (!group->idle_sockets.empty()) {
-    IdleSocket idle_socket = group->idle_sockets.back();
-    group->idle_sockets.pop_back();
+  while (!group->idle_sockets().empty()) {
+    IdleSocket idle_socket = group->idle_sockets().back();
+    group->mutable_idle_sockets()->pop_back();
     DecrementIdleCount();
     if (idle_socket.socket->IsConnectedAndIdle()) {
       // We found one we can reuse!
@@ -308,61 +302,6 @@ void ClientSocketPoolBaseHelper::LogBoundConnectJobToRequest(
       new NetLogSourceParameter("source_dependency", connect_job_source));
 }
 
-void ClientSocketPoolBaseHelper::StartBackupSocketTimer(
-    const std::string& group_name) {
-  CHECK(ContainsKey(group_map_, group_name));
-  Group& group = group_map_[group_name];
-
-  // Only allow one timer pending to create a backup socket.
-  if (group.backup_task)
-    return;
-
-  group.backup_task = method_factory_.NewRunnableMethod(
-      &ClientSocketPoolBaseHelper::OnBackupSocketTimerFired, group_name);
-  MessageLoop::current()->PostDelayedTask(FROM_HERE, group.backup_task,
-                                          ConnectRetryIntervalMs());
-}
-
-void ClientSocketPoolBaseHelper::OnBackupSocketTimerFired(
-    const std::string& group_name) {
-  CHECK(ContainsKey(group_map_, group_name));
-
-  Group& group = group_map_[group_name];
-
-  CHECK(group.backup_task);
-  group.backup_task = NULL;
-
-  CHECK(group.connect_backup_job);
-
-  // If there are no more jobs pending, there is no work to do.
-  // If we've done our cleanups correctly, this should not happen.
-  if (group.jobs.empty()) {
-    NOTREACHED();
-    return;
-  }
-
-  // If our backup job is waiting on DNS, or if we can't create any sockets
-  // right now due to limits, just reset the timer.
-  if (ReachedMaxSocketsLimit() ||
-      !group.HasAvailableSocketSlot(max_sockets_per_group_) ||
-      (*group.jobs.begin())->GetLoadState() == LOAD_STATE_RESOLVING_HOST) {
-    StartBackupSocketTimer(group_name);
-    return;
-  }
-
-  group.connect_backup_job->net_log().AddEvent(
-      NetLog::TYPE_SOCKET_BACKUP_CREATED,
-      NULL);
-  SIMPLE_STATS_COUNTER("socket.backup_created");
-  int rv = group.connect_backup_job->Connect();
-  connecting_socket_count_++;
-  group.jobs.insert(group.connect_backup_job);
-  ConnectJob* job = group.connect_backup_job;
-  group.connect_backup_job = NULL;
-  if (rv != ERR_IO_PENDING)
-    OnConnectJobComplete(rv, job);
-}
-
 void ClientSocketPoolBaseHelper::CancelRequest(
     const std::string& group_name, ClientSocketHandle* handle) {
   PendingCallbackMap::iterator callback_it = pending_callback_map_.find(handle);
@@ -380,20 +319,21 @@ void ClientSocketPoolBaseHelper::CancelRequest(
 
   CHECK(ContainsKey(group_map_, group_name));
 
-  Group& group = group_map_[group_name];
+  Group* group = GetOrCreateGroup(group_name);
 
   // Search pending_requests for matching handle.
-  RequestQueue::iterator it = group.pending_requests.begin();
-  for (; it != group.pending_requests.end(); ++it) {
+  RequestQueue::iterator it = group->mutable_pending_requests()->begin();
+  for (; it != group->pending_requests().end(); ++it) {
     if ((*it)->handle() == handle) {
-      const Request* req = RemoveRequestFromQueue(it, &group.pending_requests);
+      const Request* req =
+          RemoveRequestFromQueue(it, group->mutable_pending_requests());
       req->net_log().AddEvent(NetLog::TYPE_CANCELLED, NULL);
       req->net_log().EndEvent(NetLog::TYPE_SOCKET_POOL, NULL);
       delete req;
 
       // We let the job run, unless we're at the socket limit.
-      if (group.jobs.size() && ReachedMaxSocketsLimit()) {
-        RemoveConnectJob(*group.jobs.begin(), &group);
+      if (group->jobs().size() && ReachedMaxSocketsLimit()) {
+        RemoveConnectJob(*group->jobs().begin(), group);
         CheckForStalledSocketGroups();
       }
       break;
@@ -414,7 +354,7 @@ int ClientSocketPoolBaseHelper::IdleSocketCountInGroup(
   GroupMap::const_iterator i = group_map_.find(group_name);
   CHECK(i != group_map_.end());
 
-  return i->second.idle_sockets.size();
+  return i->second->idle_sockets().size();
 }
 
 LoadState ClientSocketPoolBaseHelper::GetLoadState(
@@ -430,16 +370,16 @@ LoadState ClientSocketPoolBaseHelper::GetLoadState(
   }
 
   // Can't use operator[] since it is non-const.
-  const Group& group = group_map_.find(group_name)->second;
+  const Group& group = *group_map_.find(group_name)->second;
 
   // Search pending_requests for matching handle.
-  RequestQueue::const_iterator it = group.pending_requests.begin();
-  for (size_t i = 0; it != group.pending_requests.end(); ++it, ++i) {
+  RequestQueue::const_iterator it = group.pending_requests().begin();
+  for (size_t i = 0; it != group.pending_requests().end(); ++it, ++i) {
     if ((*it)->handle() == handle) {
-      if (i < group.jobs.size()) {
+      if (i < group.jobs().size()) {
         LoadState max_state = LOAD_STATE_IDLE;
-        for (ConnectJobSet::const_iterator job_it = group.jobs.begin();
-             job_it != group.jobs.end(); ++job_it) {
+        for (ConnectJobSet::const_iterator job_it = group.jobs().begin();
+             job_it != group.jobs().end(); ++job_it) {
           max_state = std::max(max_state, (*job_it)->GetLoadState());
         }
         return max_state;
@@ -481,15 +421,15 @@ void ClientSocketPoolBaseHelper::CleanupIdleSockets(bool force) {
 
   GroupMap::iterator i = group_map_.begin();
   while (i != group_map_.end()) {
-    Group& group = i->second;
+    Group* group = i->second;
 
-    std::deque<IdleSocket>::iterator j = group.idle_sockets.begin();
-    while (j != group.idle_sockets.end()) {
+    std::deque<IdleSocket>::iterator j = group->mutable_idle_sockets()->begin();
+    while (j != group->idle_sockets().end()) {
       base::TimeDelta timeout =
           j->used ? used_idle_socket_timeout_ : unused_idle_socket_timeout_;
       if (force || j->ShouldCleanup(now, timeout)) {
         delete j->socket;
-        j = group.idle_sockets.erase(j);
+        j = group->mutable_idle_sockets()->erase(j);
         DecrementIdleCount();
       } else {
         ++j;
@@ -497,14 +437,36 @@ void ClientSocketPoolBaseHelper::CleanupIdleSockets(bool force) {
     }
 
     // Delete group if no longer needed.
-    if (group.IsEmpty()) {
-      group_map_.erase(i++);
+    if (group->IsEmpty()) {
+      RemoveGroup(i++);
     } else {
       ++i;
     }
   }
 }
 
+ClientSocketPoolBaseHelper::Group* ClientSocketPoolBaseHelper::GetOrCreateGroup(
+    const std::string& group_name) {
+  GroupMap::iterator it = group_map_.find(group_name);
+  if (it != group_map_.end())
+    return it->second;
+  Group* group = new Group;
+  group_map_[group_name] = group;
+  return group;
+}
+
+void ClientSocketPoolBaseHelper::RemoveGroup(const std::string& group_name) {
+  GroupMap::iterator it = group_map_.find(group_name);
+  CHECK(it != group_map_.end());
+
+  RemoveGroup(it);
+}
+
+void ClientSocketPoolBaseHelper::RemoveGroup(GroupMap::iterator it) {
+  delete it->second;
+  group_map_.erase(it);
+}
+
 // static
 void ClientSocketPoolBaseHelper::set_connect_backup_jobs_enabled(bool enabled) {
   g_connect_backup_jobs_enabled = enabled;
@@ -531,20 +493,20 @@ void ClientSocketPoolBaseHelper::ReleaseSocket(const std::string& group_name,
   GroupMap::iterator i = group_map_.find(group_name);
   CHECK(i != group_map_.end());
 
-  Group& group = i->second;
+  Group* group = i->second;
 
   CHECK_GT(handed_out_socket_count_, 0);
   handed_out_socket_count_--;
 
-  CHECK_GT(group.active_socket_count, 0);
-  group.active_socket_count--;
+  CHECK_GT(group->active_socket_count(), 0);
+  group->DecrementActiveSocketCount();
 
   const bool can_reuse = socket->IsConnectedAndIdle() &&
       id == pool_generation_number_;
   if (can_reuse) {
     // Add it to the idle list.
-    AddIdleSocket(socket, true /* used socket */, &group);
-    OnAvailableSocketSlot(group_name, &group);
+    AddIdleSocket(socket, true /* used socket */, group);
+    OnAvailableSocketSlot(group_name, group);
   } else {
     delete socket;
   }
@@ -587,16 +549,16 @@ bool ClientSocketPoolBaseHelper::FindTopStalledGroup(Group** group,
   bool has_stalled_group = false;
   for (GroupMap::iterator i = group_map_.begin();
        i != group_map_.end(); ++i) {
-    Group& group = i->second;
-    const RequestQueue& queue = group.pending_requests;
+    Group* curr_group = i->second;
+    const RequestQueue& queue = curr_group->pending_requests();
     if (queue.empty())
       continue;
-    if (group.IsStalled(max_sockets_per_group_)) {
+    if (curr_group->IsStalled(max_sockets_per_group_)) {
       has_stalled_group = true;
       bool has_higher_priority = !top_group ||
-          group.TopPendingPriority() < top_group->TopPendingPriority();
+          curr_group->TopPendingPriority() < top_group->TopPendingPriority();
       if (has_higher_priority) {
-        top_group = &group;
+        top_group = curr_group;
         top_group_name = &i->first;
       }
     }
@@ -615,7 +577,7 @@ void ClientSocketPoolBaseHelper::OnConnectJobComplete(
   const std::string group_name = job->group_name();
   GroupMap::iterator group_it = group_map_.find(group_name);
   CHECK(group_it != group_map_.end());
-  Group& group = group_it->second;
+  Group* group = group_it->second;
 
   scoped_ptr<ClientSocket> socket(job->ReleaseSocket());
 
@@ -623,44 +585,46 @@ void ClientSocketPoolBaseHelper::OnConnectJobComplete(
 
   if (result == OK) {
     DCHECK(socket.get());
-    RemoveConnectJob(job, &group);
-    if (!group.pending_requests.empty()) {
+    RemoveConnectJob(job, group);
+    if (!group->pending_requests().empty()) {
       scoped_ptr<const Request> r(RemoveRequestFromQueue(
-          group.pending_requests.begin(), &group.pending_requests));
+          group->mutable_pending_requests()->begin(),
+          group->mutable_pending_requests()));
       LogBoundConnectJobToRequest(job_log.source(), r.get());
       HandOutSocket(
           socket.release(), false /* unused socket */, r->handle(),
-          base::TimeDelta(), &group, r->net_log());
+          base::TimeDelta(), group, r->net_log());
       r->net_log().EndEvent(NetLog::TYPE_SOCKET_POOL, NULL);
       InvokeUserCallbackLater(r->handle(), r->callback(), result);
     } else {
-      AddIdleSocket(socket.release(), false /* unused socket */, &group);
-      OnAvailableSocketSlot(group_name, &group);
+      AddIdleSocket(socket.release(), false /* unused socket */, group);
+      OnAvailableSocketSlot(group_name, group);
       CheckForStalledSocketGroups();
     }
   } else {
     // If we got a socket, it must contain error information so pass that
     // up so that the caller can retrieve it.
     bool handed_out_socket = false;
-    if (!group.pending_requests.empty()) {
+    if (!group->pending_requests().empty()) {
       scoped_ptr<const Request> r(RemoveRequestFromQueue(
-          group.pending_requests.begin(), &group.pending_requests));
+          group->mutable_pending_requests()->begin(),
+          group->mutable_pending_requests()));
       LogBoundConnectJobToRequest(job_log.source(), r.get());
       job->GetAdditionalErrorState(r->handle());
-      RemoveConnectJob(job, &group);
+      RemoveConnectJob(job, group);
       if (socket.get()) {
         handed_out_socket = true;
         HandOutSocket(socket.release(), false /* unused socket */, r->handle(),
-                      base::TimeDelta(), &group, r->net_log());
+                      base::TimeDelta(), group, r->net_log());
       }
       r->net_log().EndEvent(NetLog::TYPE_SOCKET_POOL,
                             new NetLogIntegerParameter("net_error", result));
       InvokeUserCallbackLater(r->handle(), r->callback(), result);
     } else {
-      RemoveConnectJob(job, &group);
+      RemoveConnectJob(job, group);
     }
     if (!handed_out_socket) {
-      OnAvailableSocketSlot(group_name, &group);
+      OnAvailableSocketSlot(group_name, group);
       CheckForStalledSocketGroups();
     }
   }
@@ -681,12 +645,12 @@ void ClientSocketPoolBaseHelper::RemoveConnectJob(const ConnectJob* job,
   connecting_socket_count_--;
 
   DCHECK(group);
-  DCHECK(ContainsKey(group->jobs, job));
-  group->jobs.erase(job);
+  DCHECK(ContainsKey(group->jobs(), job));
+  group->RemoveJob(job);
 
   // If we've got no more jobs for this group, then we no longer need a
   // backup job either.
-  if (group->jobs.empty())
+  if (group->jobs().empty())
     group->CleanupBackupJob();
 
   DCHECK(job);
@@ -697,20 +661,21 @@ void ClientSocketPoolBaseHelper::OnAvailableSocketSlot(
     const std::string& group_name, Group* group) {
   DCHECK(ContainsKey(group_map_, group_name));
   if (group->IsEmpty())
-    group_map_.erase(group_name);
-  else if (!group->pending_requests.empty())
+    RemoveGroup(group_name);
+  else if (!group->pending_requests().empty())
     ProcessPendingRequest(group_name, group);
 }
 
 void ClientSocketPoolBaseHelper::ProcessPendingRequest(
     const std::string& group_name, Group* group) {
   int rv = RequestSocketInternal(group_name,
-                                 *group->pending_requests.begin());
+                                 *group->pending_requests().begin());
   if (rv != ERR_IO_PENDING) {
     scoped_ptr<const Request> request(RemoveRequestFromQueue(
-          group->pending_requests.begin(), &group->pending_requests));
+          group->mutable_pending_requests()->begin(),
+          group->mutable_pending_requests()));
     if (group->IsEmpty())
-      group_map_.erase(group_name);
+      RemoveGroup(group_name);
 
     scoped_refptr<NetLog::EventParameters> params;
     if (rv != OK)
@@ -746,7 +711,7 @@ void ClientSocketPoolBaseHelper::HandOutSocket(
                        "source_dependency", socket->NetLog().source()));
 
   handed_out_socket_count_++;
-  group->active_socket_count++;
+  group->IncrementActiveSocketCount();
 }
 
 void ClientSocketPoolBaseHelper::AddIdleSocket(
@@ -757,24 +722,19 @@ void ClientSocketPoolBaseHelper::AddIdleSocket(
   idle_socket.start_time = base::TimeTicks::Now();
   idle_socket.used = used;
 
-  group->idle_sockets.push_back(idle_socket);
+  group->mutable_idle_sockets()->push_back(idle_socket);
   IncrementIdleCount();
 }
 
 void ClientSocketPoolBaseHelper::CancelAllConnectJobs() {
   for (GroupMap::iterator i = group_map_.begin(); i != group_map_.end();) {
-    Group& group = i->second;
-    connecting_socket_count_ -= group.jobs.size();
-    STLDeleteElements(&group.jobs);
-
-    if (group.backup_task) {
-      group.backup_task->Cancel();
-      group.backup_task = NULL;
-    }
+    Group* group = i->second;
+    connecting_socket_count_ -= group->jobs().size();
+    group->RemoveAllJobs();
 
     // Delete group if no longer needed.
-    if (group.IsEmpty()) {
-      group_map_.erase(i++);
+    if (group->IsEmpty()) {
+      RemoveGroup(i++);
     } else {
       ++i;
     }
@@ -796,15 +756,16 @@ void ClientSocketPoolBaseHelper::CloseOneIdleSocket() {
   CHECK_GT(idle_socket_count(), 0);
 
   for (GroupMap::iterator i = group_map_.begin(); i != group_map_.end(); ++i) {
-    Group& group = i->second;
+    Group* group = i->second;
 
-    if (!group.idle_sockets.empty()) {
-      std::deque<IdleSocket>::iterator j = group.idle_sockets.begin();
+    if (!group->idle_sockets().empty()) {
+      std::deque<IdleSocket>::iterator j =
+          group->mutable_idle_sockets()->begin();
       delete j->socket;
-      group.idle_sockets.erase(j);
+      group->mutable_idle_sockets()->erase(j);
       DecrementIdleCount();
-      if (group.IsEmpty())
-        group_map_.erase(i);
+      if (group->IsEmpty())
+        RemoveGroup(i);
 
       return;
     }
@@ -840,6 +801,66 @@ void ClientSocketPoolBaseHelper::InvokeUserCallback(
   callback->Run(result);
 }
 
+ClientSocketPoolBaseHelper::Group::Group()
+    : active_socket_count_(0),
+      ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)) {}
+
+ClientSocketPoolBaseHelper::Group::~Group() {
+  CleanupBackupJob();
+}
+
+void ClientSocketPoolBaseHelper::Group::StartBackupSocketTimer(
+    const std::string& group_name,
+    ClientSocketPoolBaseHelper* pool) {
+  // Only allow one timer pending to create a backup socket.
+  if (!method_factory_.empty())
+    return;
+
+  MessageLoop::current()->PostDelayedTask(
+      FROM_HERE,
+      method_factory_.NewRunnableMethod(
+          &Group::OnBackupSocketTimerFired, group_name, pool),
+      pool->ConnectRetryIntervalMs());
+}
+
+void ClientSocketPoolBaseHelper::Group::OnBackupSocketTimerFired(
+    std::string group_name,
+    ClientSocketPoolBaseHelper* pool) {
+  // If there are no more jobs pending, there is no work to do.
+  // If we've done our cleanups correctly, this should not happen.
+  if (jobs_.empty()) {
+    NOTREACHED();
+    return;
+  }
+
+  // If our backup 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_) ||
+      (*jobs_.begin())->GetLoadState() == LOAD_STATE_RESOLVING_HOST) {
+    StartBackupSocketTimer(group_name, pool);
+    return;
+  }
+
+  ConnectJob* backup_job = pool->connect_job_factory_->NewConnectJob(
+      group_name, **pending_requests_.begin(), pool);
+  backup_job->net_log().AddEvent(NetLog::TYPE_SOCKET_BACKUP_CREATED, NULL);
+  SIMPLE_STATS_COUNTER("socket.backup_created");
+  int rv = backup_job->Connect();
+  pool->connecting_socket_count_++;
+  AddJob(backup_job);
+  if (rv != ERR_IO_PENDING)
+    pool->OnConnectJobComplete(rv, backup_job);
+}
+
+void ClientSocketPoolBaseHelper::Group::RemoveAllJobs() {
+  // Delete active jobs.
+  STLDeleteElements(&jobs_);
+
+  // Cancel pending backup job.
+  method_factory_.RevokeAll();
+}
+
 }  // namespace internal
 
 }  // namespace net
diff --git a/net/socket/client_socket_pool_base.h b/net/socket/client_socket_pool_base.h
index b1ea6e3..0fbdf27 100644
--- a/net/socket/client_socket_pool_base.h
+++ b/net/socket/client_socket_pool_base.h
@@ -228,7 +228,7 @@ class ClientSocketPoolBaseHelper
   virtual void OnIPAddressChanged();
 
   int NumConnectJobsInGroup(const std::string& group_name) const {
-    return group_map_.find(group_name)->second.jobs.size();
+    return group_map_.find(group_name)->second->jobs().size();
   }
 
   bool HasGroup(const std::string& group_name) const;
@@ -271,57 +271,69 @@ class ClientSocketPoolBaseHelper
   // A Group is allocated per group_name when there are idle sockets or pending
   // requests.  Otherwise, the Group object is removed from the map.
   // |active_socket_count| tracks the number of sockets held by clients.
-  struct Group {
-    Group()
-        : active_socket_count(0),
-          connect_backup_job(NULL),
-          backup_task(NULL) {
-    }
-
-    ~Group() {
-      CleanupBackupJob();
-    }
+  class Group {
+   public:
+    Group();
+    ~Group();
 
     bool IsEmpty() const {
-      return active_socket_count == 0 && idle_sockets.empty() && jobs.empty() &&
-          pending_requests.empty();
+      return active_socket_count_ == 0 && idle_sockets_.empty() &&
+          jobs_.empty() && pending_requests_.empty();
     }
 
     bool HasAvailableSocketSlot(int max_sockets_per_group) const {
-      return active_socket_count + static_cast<int>(jobs.size()) <
+      return active_socket_count_ + static_cast<int>(jobs_.size()) <
           max_sockets_per_group;
     }
 
     bool IsStalled(int max_sockets_per_group) const {
       return HasAvailableSocketSlot(max_sockets_per_group) &&
-          pending_requests.size() > jobs.size();
+          pending_requests_.size() > jobs_.size();
     }
 
     RequestPriority TopPendingPriority() const {
-      return pending_requests.front()->priority();
+      return pending_requests_.front()->priority();
     }
 
+    bool HasBackupJob() const { return !method_factory_.empty(); }
+
     void CleanupBackupJob() {
-      if (connect_backup_job) {
-        delete connect_backup_job;
-        connect_backup_job = NULL;
-      }
-      if (backup_task) {
-        backup_task->Cancel();
-        backup_task = NULL;
-      }
+      method_factory_.RevokeAll();
     }
 
-    std::deque<IdleSocket> idle_sockets;
-    std::set<const ConnectJob*> jobs;
-    RequestQueue pending_requests;
-    int active_socket_count;  // number of active sockets used by clients
-    // A backup job in case the connect for this group takes too long.
-    ConnectJob* connect_backup_job;
-    CancelableTask* backup_task;
+    // Set a timer to create a backup socket if it takes too long to create one.
+    void StartBackupSocketTimer(const std::string& group_name,
+                                ClientSocketPoolBaseHelper* pool);
+
+    // Called when the backup socket timer fires.
+    void OnBackupSocketTimerFired(
+        std::string group_name,
+        ClientSocketPoolBaseHelper* pool);
+
+    void AddJob(const ConnectJob* job) { jobs_.insert(job); }
+    void RemoveJob(const ConnectJob* job) { jobs_.erase(job); }
+    void RemoveAllJobs();
+
+    void IncrementActiveSocketCount() { active_socket_count_++; }
+    void DecrementActiveSocketCount() { active_socket_count_--; }
+
+    const std::set<const ConnectJob*>& jobs() const { return jobs_; }
+    const std::deque<IdleSocket>& idle_sockets() const { return idle_sockets_; }
+    const RequestQueue& pending_requests() const { return pending_requests_; }
+    int active_socket_count() const { return active_socket_count_; }
+    RequestQueue* mutable_pending_requests() { return &pending_requests_; }
+    std::deque<IdleSocket>* mutable_idle_sockets() { return &idle_sockets_; }
+
+   private:
+    std::deque<IdleSocket> idle_sockets_;
+    std::set<const ConnectJob*> jobs_;
+    RequestQueue pending_requests_;
+    int active_socket_count_;  // number of active sockets used by clients
+    // A factory to pin the backup_job tasks.
+    ScopedRunnableMethodFactory<Group> method_factory_;
   };
 
-  typedef std::map<std::string, Group> GroupMap;
+  typedef std::map<std::string, Group*> GroupMap;
 
   typedef std::set<const ConnectJob*> ConnectJobSet;
 
@@ -344,6 +356,10 @@ class ClientSocketPoolBaseHelper
   static const Request* RemoveRequestFromQueue(RequestQueue::iterator it,
                                                RequestQueue* pending_requests);
 
+  Group* GetOrCreateGroup(const std::string& group_name);
+  void RemoveGroup(const std::string& group_name);
+  void RemoveGroup(GroupMap::iterator it);
+
   // Called when the number of idle sockets changes.
   void IncrementIdleCount();
   void DecrementIdleCount();
@@ -397,17 +413,11 @@ class ClientSocketPoolBaseHelper
 
   // Assigns an idle socket for the group to the request.
   // Returns |true| if an idle socket is available, false otherwise.
-  bool AssignIdleSocketToGroup(Group* group, const Request* request);
+  bool AssignIdleSocketToGroup(const Request* request, Group* group);
 
   static void LogBoundConnectJobToRequest(
       const NetLog::Source& connect_job_source, const Request* request);
 
-  // Set a timer to create a backup socket if it takes too long to create one.
-  void StartBackupSocketTimer(const std::string& group_name);
-
-  // Called when the backup socket timer fires.
-  void OnBackupSocketTimerFired(const std::string& group_name);
-
   // Closes one idle socket.  Picks the first one encountered.
   // TODO(willchan): Consider a better algorithm for doing this.  Perhaps we
   // should keep an ordered list of idle sockets, and close them in order.
@@ -466,9 +476,6 @@ class ClientSocketPoolBaseHelper
   // TODO(vandebo) Remove when backup jobs move to TCPClientSocketPool
   bool connect_backup_jobs_enabled_;
 
-  // A factory to pin the backup_job tasks.
-  ScopedRunnableMethodFactory<ClientSocketPoolBaseHelper> method_factory_;
-
   // A unique id for the pool.  It gets incremented every time we Flush() the
   // pool.  This is so that when sockets get released back to the pool, we can
   // make sure that they are discarded rather than reused.