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

net/socket/client_socket_pool_base.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/socket/client_socket_pool_base.h"
 
 #include <math.h>
 #include "base/compiler_specific.h"
 #include "base/format_macros.h"
 #include "base/logging.h"
@@ skipping 164 matching lines @@
     int max_sockets,
     int max_sockets_per_group,
     base::TimeDelta unused_idle_socket_timeout,
     base::TimeDelta used_idle_socket_timeout,
     ConnectJobFactory* connect_job_factory)
     : idle_socket_count_(0),
       connecting_socket_count_(0),
       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),
       connect_backup_jobs_enabled_(false),
       pool_generation_number_(0),
       method_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
   DCHECK_LE(0, max_sockets_per_group);
   DCHECK_LE(max_sockets_per_group, max_sockets);
 
+#if defined(OS_ANDROID)
+  use_cleanup_timer_ = false;

[mmenke, 2011/11/11 03:33:01]

Just for consistency, and so it's only modified in one place, I'd suggest using
set_cleanup_timer_enabled(false) here instead.

[selim, 2011/11/11 19:30:58]

done

+#endif
   NetworkChangeNotifier::AddIPAddressObserver(this);
 }
 
 ClientSocketPoolBaseHelper::~ClientSocketPoolBaseHelper() {
   // Clean up any idle sockets and pending connect jobs.  Assert that we have no
   // remaining active sockets or pending requests.  They should have all been
   // cleaned up prior to |this| being destroyed.
   Flush();
   DCHECK(group_map_.empty());
   DCHECK(pending_callback_map_.empty());
@@ skipping 37 matching lines @@
   Group* group = GetOrCreateGroup(group_name);
 
   int rv = RequestSocketInternal(group_name, request);
   if (rv != ERR_IO_PENDING) {
     request->net_log().EndEventWithNetErrorCode(NetLog::TYPE_SOCKET_POOL, rv);
     CHECK(!request->handle()->is_initialized());
     delete request;
   } else {
     InsertRequestIntoQueue(request, group->mutable_pending_requests());
   }
+  // Cleanup any timed-out idle sockets if no timer is used.
+  if (!use_cleanup_timer_) CleanupIdleSockets(false);

[mmenke, 2011/11/11 03:33:01]

nit:  Google style is to always split ifs onto two lines.

See http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Conditionals

[selim, 2011/11/11 19:30:58]

done. Even though I find it less readable, code conventions is code conventions
and there is no way around!

   return rv;
 }
 
 void ClientSocketPoolBaseHelper::RequestSockets(
     const std::string& group_name,
     const Request& request,
     int num_sockets) {
   DCHECK(!request.callback());
   DCHECK(!request.handle());
 
@@ skipping 27 matching lines @@
       // error.
       NOTREACHED();
       deleted_group = true;
       break;
     }
   }
 
   if (!deleted_group && group->IsEmpty())
     RemoveGroup(group_name);
 
+  // Cleanup any timed out idle sockets if no timer is used.
+  if (!use_cleanup_timer_) CleanupIdleSockets(false);

[mmenke, 2011/11/11 03:33:01]

nit:  Google style is to always split ifs onto two lines.

[selim, 2011/11/11 19:30:58]

done

+
   if (rv == ERR_IO_PENDING)
     rv = OK;
   request.net_log().EndEventWithNetErrorCode(
       NetLog::TYPE_SOCKET_POOL_CONNECTING_N_SOCKETS, rv);
 }
 
 int ClientSocketPoolBaseHelper::RequestSocketInternal(
     const std::string& group_name,
     const Request* request) {
   DCHECK_GE(request->priority(), 0);
@@ skipping 381 matching lines @@
   bool old_value = g_connect_backup_jobs_enabled;
   g_connect_backup_jobs_enabled = enabled;
   return old_value;
 }
 
 void ClientSocketPoolBaseHelper::EnableConnectBackupJobs() {
   connect_backup_jobs_enabled_ = g_connect_backup_jobs_enabled;
 }
 
 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() {
   if (--idle_socket_count_ == 0)
     timer_.Stop();
 }
 
+bool ClientSocketPoolBaseHelper::cleanup_timer_enabled() {
+  return use_cleanup_timer_;

[mmenke, 2011/11/11 03:33:01]

nit:  non-virtual one-liners like this are generally inlined in the header
files.

[selim, 2011/11/11 19:30:58]

done

+}
+
+void ClientSocketPoolBaseHelper::set_cleanup_timer_enabled( bool enabled ) {

[mmenke, 2011/11/11 03:33:01]

nit:  "(bool enabled)"

[mmenke, 2011/11/11 03:33:01]

nit:  Try to put these in the same order relative to other functions as they are
in the header files.  This often isn't possible to do perfectly, as a result of
refactorings, but can at least put it before or after one of the same functions.

[selim, 2011/11/11 19:30:58]

done

+  use_cleanup_timer_ = enabled;
+  if (enabled) {
+    if (idle_socket_count_ > 0)
+      startIdleSocketTimer();
+  } else
+    timer_.Stop();

[mmenke, 2011/11/11 03:33:01]

nit:  If you use braces on the first block of a conditional statement, always
use them on the second block, too.

[selim, 2011/11/11 19:30:58]

done

+}
+
+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) {
   GroupMap::iterator i = group_map_.find(group_name);
   CHECK(i != group_map_.end());
 
   Group* group = i->second;
 
   CHECK_GT(handed_out_socket_count_, 0);
   handed_out_socket_count_--;
@@ skipping 170 matching lines @@
     const std::string& group_name, Group* group) {
   int rv = RequestSocketInternal(group_name,
                                  *group->pending_requests().begin());
   if (rv != ERR_IO_PENDING) {
     scoped_ptr<const Request> request(RemoveRequestFromQueue(
           group->mutable_pending_requests()->begin(), group));
     if (group->IsEmpty())
       RemoveGroup(group_name);
 
     request->net_log().EndEventWithNetErrorCode(NetLog::TYPE_SOCKET_POOL, rv);
-    InvokeUserCallbackLater(
-        request->handle(), request->callback(), rv);
+    InvokeUserCallbackLater(request->handle(), request->callback(), rv);
   }
 }
 
 void ClientSocketPoolBaseHelper::HandOutSocket(
     StreamSocket* socket,
     bool reused,
     ClientSocketHandle* handle,
     base::TimeDelta idle_time,
     Group* group,
     const BoundNetLog& net_log) {
@@ skipping 212 matching lines @@
   // Delete active jobs.
   STLDeleteElements(&jobs_);
 
   // Cancel pending backup job.
   method_factory_.RevokeAll();
 }
 
 }  // namespace internal
 
 }  // namespace net