Close idle sockets next time we are about to send data.

net/socket/client_socket_pool_base.cc

Index: net/socket/client_socket_pool_base.cc
diff --git a/net/socket/client_socket_pool_base.cc b/net/socket/client_socket_pool_base.cc
index 231254a5b7d5cb126920812afbeea606bdf7b2be..065c7b63b22367cc438f78e3e53ee511450feaa8 100644
--- a/net/socket/client_socket_pool_base.cc
+++ b/net/socket/client_socket_pool_base.cc
@@ -182,6 +182,7 @@ ClientSocketPoolBaseHelper::ClientSocketPoolBaseHelper(
       handed_out_socket_count_(0),
       max_sockets_(max_sockets),
       max_sockets_per_group_(max_sockets_per_group),
+      use_cleanup_timer_(true),
       unused_idle_socket_timeout_(unused_idle_socket_timeout),
       used_idle_socket_timeout_(used_idle_socket_timeout),
       connect_job_factory_(connect_job_factory),
@@ -191,6 +192,9 @@ ClientSocketPoolBaseHelper::ClientSocketPoolBaseHelper(
   DCHECK_LE(0, max_sockets_per_group);
   DCHECK_LE(max_sockets_per_group, max_sockets);
 
+#if defined(OS_ANDROID)
+  SetCleanupTimerEnabled(false);

[willchan no longer on Chromium, 2011/11/11 23:47:50]

I think you should do this differently. The problem with this is there's no way
to change this for OS_ANDROID then. There's no public API
(ClientSocketPoolBaseHelper is an internal class, not exposed to net consumers)
to tweak this. Either we should be passing this in as a parameter to the
constructor, or we should be initializing the member variable from a global
(this is a pattern we've done elsewhere) in the constructor.

I think the "right" thing to do is pass it in as a constructor argument. That
might require a bunch of changes to existing ClientSocketPool classes. I'm find
deferring that change due to time constraints, but would like it to be done
eventually. The expedient change would be to just use a global variable (see
g_connect_backup_jobs_enabled).

+#endif
   NetworkChangeNotifier::AddIPAddressObserver(this);
 }
 
@@ -237,6 +241,10 @@ int ClientSocketPoolBaseHelper::RequestSocket(
   CHECK(request->callback());
   CHECK(request->handle());
 
+  // Cleanup any timed-out idle sockets if no timer is used.
+  if (!use_cleanup_timer_)
+    CleanupIdleSockets(false);

[willchan no longer on Chromium, 2011/11/11 23:47:50]

Move this after the BeginEvent().

+
   request->net_log().BeginEvent(NetLog::TYPE_SOCKET_POOL, NULL);
   Group* group = GetOrCreateGroup(group_name);
 
@@ -258,6 +266,10 @@ void ClientSocketPoolBaseHelper::RequestSockets(
   DCHECK(!request.callback());
   DCHECK(!request.handle());
 
+  // Cleanup any timed out idle sockets if no timer is used.
+  if (!use_cleanup_timer_)
+    CleanupIdleSockets(false);
+
   if (num_sockets > max_sockets_per_group_) {
     num_sockets = max_sockets_per_group_;
   }
@@ -696,9 +708,8 @@ void ClientSocketPoolBaseHelper::EnableConnectBackupJobs() {
 }
 
 void ClientSocketPoolBaseHelper::IncrementIdleCount() {
-  if (++idle_socket_count_ == 1)
-    timer_.Start(FROM_HERE, TimeDelta::FromSeconds(kCleanupInterval), this,
-                 &ClientSocketPoolBaseHelper::OnCleanupTimerFired);
+  if (++idle_socket_count_ == 1 && use_cleanup_timer_)
+    StartIdleSocketTimer();
 }
 
 void ClientSocketPoolBaseHelper::DecrementIdleCount() {
@@ -706,6 +717,21 @@ void ClientSocketPoolBaseHelper::DecrementIdleCount() {
     timer_.Stop();
 }
 
+void ClientSocketPoolBaseHelper::SetCleanupTimerEnabled(bool enabled) {

[willchan no longer on Chromium, 2011/11/11 23:47:50]

I think it's not necessary to have this code where you can dynamically turn
on/off cleanup timers. I think it's OK for us to just assume it never changes
after construction, and furthermore, I'd rather you do that because it makes
this already complicated code easier to reason about.

+  use_cleanup_timer_ = enabled;
+  if (enabled) {
+    if (idle_socket_count_ > 0)
+      StartIdleSocketTimer();
+  } else {
+    timer_.Stop();
+  }
+}
+
+void ClientSocketPoolBaseHelper::StartIdleSocketTimer() {
+  timer_.Start(FROM_HERE, TimeDelta::FromSeconds(kCleanupInterval), this,
+               &ClientSocketPoolBaseHelper::OnCleanupTimerFired);
+}
+
 void ClientSocketPoolBaseHelper::ReleaseSocket(const std::string& group_name,
                                                StreamSocket* socket,
                                                int id) {
@@ -896,8 +922,7 @@ void ClientSocketPoolBaseHelper::ProcessPendingRequest(
       RemoveGroup(group_name);
 
     request->net_log().EndEventWithNetErrorCode(NetLog::TYPE_SOCKET_POOL, rv);
-    InvokeUserCallbackLater(
-        request->handle(), request->callback(), rv);
+    InvokeUserCallbackLater(request->handle(), request->callback(), rv);
   }
 }