Prefer used idle sockets over unused idle sockets.

* Pick used idle sockets LIFO.
* In absence of used idle sockets, pick unused idle sockets FIFO.

BUG=57491
TEST=ClientSocketPoolBaseTest.PreferUsedSocketToUnusedSocket

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

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

3 files changed, 120 insertions, 24 deletions

diff --git a/net/socket/client_socket_pool_base.cc b/net/socket/client_socket_pool_base.cc
index 9b0aa6d..994e1d2 100644
--- a/net/socket/client_socket_pool_base.cc
+++ b/net/socket/client_socket_pool_base.cc
@@ -277,23 +277,52 @@ int ClientSocketPoolBaseHelper::RequestSocketInternal(
 
 bool ClientSocketPoolBaseHelper::AssignIdleSocketToGroup(
     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->mutable_idle_sockets()->pop_back();
-    DecrementIdleCount();
-    if (idle_socket.socket->IsConnectedAndIdle()) {
+  std::list<IdleSocket>* idle_sockets = group->mutable_idle_sockets();
+  std::list<IdleSocket>::iterator idle_socket_it = idle_sockets->end();
+
+  // Iterate through the idle sockets forwards (oldest to newest)
+  //   * Delete any disconnected ones.
+  //   * If we find a used idle socket, assign to |idle_socket|.  At the end,
+  //   the |idle_socket_it| will be set to the newest used idle socket.
+  for (std::list<IdleSocket>::iterator it = idle_sockets->begin();
+       it != idle_sockets->end();) {
+    if (!it->socket->IsConnectedAndIdle()) {
+      DecrementIdleCount();
+      delete it->socket;
+      it = idle_sockets->erase(it);
+      continue;
+    }
+
+    if (it->socket->WasEverUsed()) {
       // We found one we can reuse!
-      base::TimeDelta idle_time =
-          base::TimeTicks::Now() - idle_socket.start_time;
-      HandOutSocket(
-          idle_socket.socket, idle_socket.socket->WasEverUsed(),
-          request->handle(), idle_time, group, request->net_log());
-      return true;
+      idle_socket_it = it;
     }
-    delete idle_socket.socket;
+
+    ++it;
   }
+
+  // If we haven't found an idle socket, that means there are no used idle
+  // sockets.  Pick the oldest (first) idle socket (FIFO).
+
+  if (idle_socket_it == idle_sockets->end() && !idle_sockets->empty())
+    idle_socket_it = idle_sockets->begin();
+
+  if (idle_socket_it != idle_sockets->end()) {
+    DecrementIdleCount();
+    base::TimeDelta idle_time =
+        base::TimeTicks::Now() - idle_socket_it->start_time;
+    IdleSocket idle_socket = *idle_socket_it;
+    idle_sockets->erase(idle_socket_it);
+    HandOutSocket(
+        idle_socket.socket,
+        idle_socket.socket->WasEverUsed(),
+        request->handle(),
+        idle_time,
+        group,
+        request->net_log());
+    return true;
+  }
+
   return false;
 }
 
@@ -429,7 +458,7 @@ DictionaryValue* ClientSocketPoolBaseHelper::GetInfoAsValue(
     group_dict->SetInteger("active_socket_count", group->active_socket_count());
 
     ListValue* idle_socket_list = new ListValue();
-    std::deque<IdleSocket>::const_iterator idle_socket;
+    std::list<IdleSocket>::const_iterator idle_socket;
     for (idle_socket = group->idle_sockets().begin();
          idle_socket != group->idle_sockets().end();
          idle_socket++) {
@@ -479,7 +508,7 @@ void ClientSocketPoolBaseHelper::CleanupIdleSockets(bool force) {
   while (i != group_map_.end()) {
     Group* group = i->second;
 
-    std::deque<IdleSocket>::iterator j = group->mutable_idle_sockets()->begin();
+    std::list<IdleSocket>::iterator j = group->mutable_idle_sockets()->begin();
     while (j != group->idle_sockets().end()) {
       base::TimeDelta timeout =
           j->socket->WasEverUsed() ?
@@ -841,12 +870,11 @@ void ClientSocketPoolBaseHelper::CloseOneIdleSocket() {
 
   for (GroupMap::iterator i = group_map_.begin(); i != group_map_.end(); ++i) {
     Group* group = i->second;
+    std::list<IdleSocket>* idle_sockets = group->mutable_idle_sockets();
 
-    if (!group->idle_sockets().empty()) {
-      std::deque<IdleSocket>::iterator j =
-          group->mutable_idle_sockets()->begin();
-      delete j->socket;
-      group->mutable_idle_sockets()->erase(j);
+    if (!idle_sockets->empty()) {
+      delete idle_sockets->front().socket;
+      idle_sockets->pop_front();
       DecrementIdleCount();
       if (group->IsEmpty())
         RemoveGroup(i);
diff --git a/net/socket/client_socket_pool_base.h b/net/socket/client_socket_pool_base.h
index 77eed2f..a39eeaa 100644
--- a/net/socket/client_socket_pool_base.h
+++ b/net/socket/client_socket_pool_base.h
@@ -24,6 +24,7 @@
 #pragma once
 
 #include <deque>
+#include <list>
 #include <map>
 #include <set>
 #include <string>
@@ -323,14 +324,14 @@ class ClientSocketPoolBaseHelper
     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 std::list<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_; }
+    std::list<IdleSocket>* mutable_idle_sockets() { return &idle_sockets_; }
 
    private:
-    std::deque<IdleSocket> idle_sockets_;
+    std::list<IdleSocket> idle_sockets_;
     std::set<const ConnectJob*> jobs_;
     RequestQueue pending_requests_;
     int active_socket_count_;  // number of active sockets used by clients
diff --git a/net/socket/client_socket_pool_base_unittest.cc b/net/socket/client_socket_pool_base_unittest.cc
index 1624a46..87f2ae4 100644
--- a/net/socket/client_socket_pool_base_unittest.cc
+++ b/net/socket/client_socket_pool_base_unittest.cc
@@ -2563,6 +2563,73 @@ TEST_F(ClientSocketPoolBaseTest, SynchronouslyProcessOnePendingRequest) {
   EXPECT_FALSE(pool_->HasGroup("a"));
 }
 
+TEST_F(ClientSocketPoolBaseTest, PreferUsedSocketToUnusedSocket) {
+  CreatePool(kDefaultMaxSockets, kDefaultMaxSockets);
+
+  connect_job_factory_->set_job_type(TestConnectJob::kMockPendingJob);
+
+  ClientSocketHandle handle1;
+  TestCompletionCallback callback1;
+  EXPECT_EQ(ERR_IO_PENDING, handle1.Init("a",
+                                         params_,
+                                         kDefaultPriority,
+                                         &callback1,
+                                         pool_.get(),
+                                         BoundNetLog()));
+
+  ClientSocketHandle handle2;
+  TestCompletionCallback callback2;
+  EXPECT_EQ(ERR_IO_PENDING, handle2.Init("a",
+                                         params_,
+                                         kDefaultPriority,
+                                         &callback2,
+                                         pool_.get(),
+                                         BoundNetLog()));
+  ClientSocketHandle handle3;
+  TestCompletionCallback callback3;
+  EXPECT_EQ(ERR_IO_PENDING, handle3.Init("a",
+                                         params_,
+                                         kDefaultPriority,
+                                         &callback3,
+                                         pool_.get(),
+                                         BoundNetLog()));
+
+  EXPECT_EQ(OK, callback1.WaitForResult());
+  EXPECT_EQ(OK, callback2.WaitForResult());
+  EXPECT_EQ(OK, callback3.WaitForResult());
+
+  // Use the socket.
+  EXPECT_EQ(1, handle1.socket()->Write(NULL, 1, NULL));
+  EXPECT_EQ(1, handle3.socket()->Write(NULL, 1, NULL));
+
+  handle1.Reset();
+  handle2.Reset();
+  handle3.Reset();
+
+  EXPECT_EQ(OK, handle1.Init("a",
+                             params_,
+                             kDefaultPriority,
+                             &callback1,
+                             pool_.get(),
+                             BoundNetLog()));
+  EXPECT_EQ(OK, handle2.Init("a",
+                             params_,
+                             kDefaultPriority,
+                             &callback2,
+                             pool_.get(),
+                             BoundNetLog()));
+  EXPECT_EQ(OK, handle3.Init("a",
+                             params_,
+                             kDefaultPriority,
+                             &callback3,
+                             pool_.get(),
+                             BoundNetLog()));
+
+  EXPECT_TRUE(handle1.socket()->WasEverUsed());
+  EXPECT_TRUE(handle2.socket()->WasEverUsed());
+  EXPECT_FALSE(handle3.socket()->WasEverUsed());
+}
+
 }  // namespace
 
 }  // namespace net