Revert 55071 - Reland 54771 (and 54795) To enable TCP Preconnection by default

Leaks reported with this CL.

http://build.chromium.org/buildbot/memory/builders/Linux%20Heapcheck/builds/6130/steps/heapcheck%20test:%20net/logs/stdio

Eg.

Leak of 24 bytes in 1 objects allocated from:
	 @ 84aece net::SSLClientSocketNSS::BufferRecv
	 @ 84b161 net::SSLClientSocketNSS::DoTransportIO
	 @ 84ca1f net::SSLClientSocketNSS::DoHandshakeLoop
	 @ 84ca6b net::SSLClientSocketNSS::OnHandshakeIOComplete
	 @ 84cadc net::SSLClientSocketNSS::OnRecvComplete
	 @ 84cbb0 net::SSLClientSocketNSS::BufferRecvComplete
	 @ 84ea4b void DispatchToMethod
	 @ 84ea7b CallbackImpl::RunWithParams
	 @ 4b3a10 CallbackRunner::Run
	 @ 853e7e net::TCPClientSocketLibevent::DoReadCallback
	 @ 85426f net::TCPClientSocketLibevent::DidCompleteRead
	 @ 856a5c net::TCPClientSocketLibevent::ReadWatcher::OnFileCanReadWithoutBlocking
	 @ 93d8fd base::MessagePumpLibevent::FileDescriptorWatcher::OnFileCanReadWithoutBlocking
	 @ 93d966 base::MessagePumpLibevent::OnLibeventNotification
	 @ 9da639 event_process_active
	 @ 9da923 event_base_loop
	 @ 93dfd0 base::MessagePumpLibevent::Run
	 @ 8f2873 MessageLoop::RunInternal
	 @ 8f2893 MessageLoop::RunHandler
	 @ 8f2938 MessageLoop::Run
	 @ 44b7f9 TestCompletionCallback::WaitForResult
	 @ 6a1ee6 SSLClientSocketTest_ConnectMismatched_Test::TestBody
	 @ 961831 testing::Test::Run
	 @ 965026 testing::internal::TestInfoImpl::Run
	 @ 96515c testing::TestCase::Run
	 @ 965bbe testing::internal::UnitTestImpl::RunAllTests
	 @ 965d35 testing::UnitTest::Run
	 @ 4a4bf7 TestSuite::Run
	 @ 4a3b6d main
	 @ 2adff5bb11c4 __libc_start_main

Suppression:
{
   <insert_a_suppression_name_here>
   Heapcheck:Leak
   fun:net::SSLClientSocketNSS::BufferRecv
   fun:net::SSLClientSocketNSS::DoTransportIO
   fun:net::SSLClientSocketNSS::DoHandshakeLoop
   fun:net::SSLClientSocketNSS::OnHandshakeIOComplete
   fun:net::SSLClientSocketNSS::OnRecvComplete
   fun:net::SSLClientSocketNSS::BufferRecvComplete
   fun:void DispatchToMethod
   fun:CallbackImpl::RunWithParams
   fun:CallbackRunner::Run
   fun:net::TCPClientSocketLibevent::DoReadCallback
   fun:net::TCPClientSocketLibevent::DidCompleteRead
   fun:net::TCPClientSocketLibevent::ReadWatcher::OnFileCanReadWithoutBlocking
   fun:base::MessagePumpLibevent::FileDescriptorWatcher::OnFileCanReadWithoutBlocking
   fun:base::MessagePumpLibevent::OnLibeventNotification
   fun:event_process_active
   fun:event_base_loop
   fun:base::MessagePumpLibevent::Run
   fun:MessageLoop::RunInternal
   fun:MessageLoop::RunHandler
   fun:MessageLoop::Run
   fun:TestCompletionCallback::WaitForResult
   fun:SSLClientSocketTest_ConnectMismatched_Test::TestBody
   fun:testing::Test::Run
   fun:testing::internal::TestInfoImpl::Run
   fun:testing::TestCase::Run
   fun:testing::internal::UnitTestImpl::RunAllTests
   fun:testing::UnitTest::Run
   fun:TestSuite::Run
   fun:main
   fun:__libc_start_main
}




I added defensive code in ClientSocketHandle::ReleaseSocket(),
which should preclude the crash that was reported on the
stability bot.

I added a second call to ReleaseSocket() from
~ClientSocketHandle to ensure that we updated the
related ClientSocket when we are torn down.

r=mbelshe
Review URL: http://codereview.chromium.org/3071022

TBR=jar@chromium.org

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

13 files changed, 38 insertions, 224 deletions

diff --git a/chrome/browser/browser_main.cc b/chrome/browser/browser_main.cc
index f35779a..04fd26b 100644
--- a/chrome/browser/browser_main.cc
+++ b/chrome/browser/browser_main.cc
@@ -1199,15 +1199,10 @@ int BrowserMain(const MainFunctionParams& parameters) {
   // pre-resolution, as well as TCP/IP connection pre-warming.
   // This also registers an observer to discard data when closing incognito
   // mode.
-  bool preconnect_enabled = true;  // Default status (easy to change!).
-  if (parsed_command_line.HasSwitch(switches::kDisablePreconnect))
-    preconnect_enabled = false;
-  else if (parsed_command_line.HasSwitch(switches::kEnablePreconnect))
-    preconnect_enabled = true;
   chrome_browser_net::PredictorInit dns_prefetch(
       user_prefs,
       local_state,
-      preconnect_enabled,
+      parsed_command_line.HasSwitch(switches::kEnablePreconnect),
       parsed_command_line.HasSwitch(switches::kPreconnectDespiteProxy));
 
 #if defined(OS_WIN)
diff --git a/chrome/browser/net/preconnect.cc b/chrome/browser/net/preconnect.cc
index 3872d2a..e8038ff 100644
--- a/chrome/browser/net/preconnect.cc
+++ b/chrome/browser/net/preconnect.cc
@@ -21,22 +21,18 @@ namespace chrome_browser_net {
 // static
 bool Preconnect::preconnect_despite_proxy_ = false;
 
-
-Preconnect::~Preconnect() {
-  if (!handle_.is_initialized())
-    return;
-  DCHECK(motivation_ == UrlInfo::LEARNED_REFERAL_MOTIVATED ||
-         motivation_ == UrlInfo::OMNIBOX_MOTIVATED);
-  if (motivation_ == UrlInfo::OMNIBOX_MOTIVATED)
-    handle_.socket()->SetOmniboxSpeculation();
-  else
-    handle_.socket()->SetSubresourceSpeculation();
-  handle_.Reset();
-}
+// We will deliberately leak this singular instance, which is used only for
+// callbacks.
+// static
+Preconnect* Preconnect::callback_instance_;
 
 // static
 void Preconnect::PreconnectOnUIThread(const GURL& url,
     UrlInfo::ResolutionMotivation motivation) {
+  // Try to do connection warming for this search provider.
+  URLRequestContextGetter* getter = Profile::GetDefaultRequestContext();
+  if (!getter)
+    return;
   // Prewarm connection to Search URL.
   ChromeThread::PostTask(
       ChromeThread::IO,
@@ -61,13 +57,6 @@ static void HistogramPreconnectStatus(ProxyStatus status) {
 // static
 void Preconnect::PreconnectOnIOThread(const GURL& url,
     UrlInfo::ResolutionMotivation motivation) {
-  scoped_refptr<Preconnect> preconnect = new Preconnect(motivation);
-  // TODO(jar): Should I use PostTask for LearnedSubresources to delay the
-  // preconnection a tad?
-  preconnect->Connect(url);
-}
-
-void Preconnect::Connect(const GURL& url) {
   URLRequestContextGetter* getter = Profile::GetDefaultRequestContext();
   if (!getter)
     return;
@@ -75,9 +64,6 @@ void Preconnect::Connect(const GURL& url) {
     LOG(DFATAL) << "This must be run only on the IO thread.";
     return;
   }
-
-  AddRef();  // Stay alive until socket is available.
-
   URLRequestContext* context = getter->GetURLRequestContext();
 
   if (preconnect_despite_proxy_) {
@@ -100,12 +86,16 @@ void Preconnect::Connect(const GURL& url) {
     }
   }
 
-  UMA_HISTOGRAM_ENUMERATION("Net.PreconnectMotivation", motivation_,
+  UMA_HISTOGRAM_ENUMERATION("Net.PreconnectMotivation", motivation,
                             UrlInfo::MAX_MOTIVATED);
 
   net::HttpTransactionFactory* factory = context->http_transaction_factory();
   net::HttpNetworkSession* session = factory->GetSession();
 
+  net::ClientSocketHandle handle;
+  if (!callback_instance_)
+    callback_instance_ = new Preconnect;
+
   scoped_refptr<net::TCPSocketParams> tcp_params =
       new net::TCPSocketParams(url.host(), url.EffectiveIntPort(), net::LOW,
                                GURL(), false);
@@ -113,19 +103,6 @@ void Preconnect::Connect(const GURL& url) {
   net::HostPortPair endpoint(url.host(), url.EffectiveIntPort());
   std::string group_name = endpoint.ToString();
 
-  // It almost doesn't matter whether we use net::LOWEST or net::HIGHEST
-  // priority here, as we won't make a request, and will surrender the created
-  // socket to the pool as soon as we can.  However, we would like to mark the
-  // speculative socket as such, and IF we use a net::LOWEST priority, and if
-  // a navigation asked for a socket (after us) then it would get our socket,
-  // and we'd get its later-arriving socket, which might make us record that
-  // the speculation didn't help :-/.  By using net::HIGHEST, we ensure that
-  // a socket is given to us if "we asked first" and this allows us to mark it
-  // as speculative, and better detect stats (if it gets used).
-  // TODO(jar): histogram to see how often we accidentally use a previously-
-  // unused socket, when a previously used socket was available.
-  net::RequestPriority priority = net::HIGHEST;
-
   if (url.SchemeIs("https")) {
     group_name = StringPrintf("ssl/%s", group_name.c_str());
 
@@ -143,20 +120,21 @@ void Preconnect::Connect(const GURL& url) {
     const scoped_refptr<net::SSLClientSocketPool>& pool =
         session->ssl_socket_pool();
 
-    handle_.Init(group_name, ssl_params, priority, this, pool,
-                 net::BoundNetLog());
+    handle.Init(group_name, ssl_params, net::LOWEST, callback_instance_, pool,
+                net::BoundNetLog());
+    handle.Reset();
     return;
   }
 
   const scoped_refptr<net::TCPClientSocketPool>& pool =
       session->tcp_socket_pool();
-  handle_.Init(group_name, tcp_params, priority, this, pool,
-               net::BoundNetLog());
+  handle.Init(group_name, tcp_params, net::LOWEST, callback_instance_, pool,
+              net::BoundNetLog());
+  handle.Reset();
 }
 
 void Preconnect::RunWithParams(const Tuple1<int>& params) {
-  if (params.a < 0 && handle_.socket())
-    handle_.socket()->Disconnect();
-  Release();
+  // This will rarely be called, as we reset the connection just after creating.
+  NOTREACHED();
 }
 }  // chrome_browser_net
diff --git a/chrome/browser/net/preconnect.h b/chrome/browser/net/preconnect.h
index f7d574d..d773636 100644
--- a/chrome/browser/net/preconnect.h
+++ b/chrome/browser/net/preconnect.h
@@ -19,8 +19,7 @@
 
 namespace chrome_browser_net {
 
-class Preconnect : public net::CompletionCallback,
-                   public base::RefCountedThreadSafe<Preconnect> {
+class Preconnect : public net::CompletionCallback {
  public:
   // Try to preconnect.  Typically motivated by OMNIBOX to reach search service.
   static void PreconnectOnUIThread(const GURL& url,
@@ -36,15 +35,11 @@ class Preconnect : public net::CompletionCallback,
   }
 
  private:
-  friend class base::RefCountedThreadSafe<Preconnect>;
+  Preconnect() {}
 
-  explicit Preconnect(UrlInfo::ResolutionMotivation motivation)
-      : motivation_(motivation) {
-  }
-  ~Preconnect();
-
-  // Request actual connection.
-  void Connect(const GURL& url);
+  // Supply an instance that could have been used in an IO callback, but will
+  // never actually be used (because we reset the connection so quickly).
+  static Preconnect* callback_instance_;
 
   // IO Callback which whould be performed when the connection is established.
   virtual void RunWithParams(const Tuple1<int>& params);
@@ -55,15 +50,6 @@ class Preconnect : public net::CompletionCallback,
   // much work anway).
   static bool preconnect_despite_proxy_;
 
-  // The handle holding the request. We need this so that we can mark the
-  // request as speculative when an actual socket is bound to it.
-  net::ClientSocketHandle handle_;
-
-  // Generally either LEARNED_REFERAL_MOTIVATED or OMNIBOX_MOTIVATED to indicate
-  // why we were trying to do a preconnection.
-  const UrlInfo::ResolutionMotivation motivation_;
-
-
   DISALLOW_COPY_AND_ASSIGN(Preconnect);
 };
 }  // chrome_browser_net
diff --git a/chrome/browser/net/predictor.cc b/chrome/browser/net/predictor.cc
index 5c1b518..eb4992c 100644
--- a/chrome/browser/net/predictor.cc
+++ b/chrome/browser/net/predictor.cc
@@ -149,20 +149,6 @@ void Predictor::PredictFrameSubresources(const GURL& url) {
   Referrers::iterator it = referrers_.find(url);
   if (referrers_.end() == it)
     return;
-  ChromeThread::PostTask(
-      ChromeThread::IO,
-      FROM_HERE,
-      NewRunnableMethod(this,
-                        &Predictor::PrepareFrameSubresources, url));
-}
-
-void Predictor::PrepareFrameSubresources(const GURL& url) {
-  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));
-  DCHECK(url.GetWithEmptyPath() == url);
-  Referrers::iterator it = referrers_.find(url);
-  if (referrers_.end() == it)
-    return;
-
   Referrer* referrer = &(it->second);
   referrer->IncrementUseCount();
   const UrlInfo::ResolutionMotivation motivation =
diff --git a/chrome/browser/net/predictor.h b/chrome/browser/net/predictor.h
index c613093..a5b71e7 100644
--- a/chrome/browser/net/predictor.h
+++ b/chrome/browser/net/predictor.h
@@ -83,11 +83,8 @@ class Predictor : public base::RefCountedThreadSafe<Predictor> {
   void Resolve(const GURL& url,
                UrlInfo::ResolutionMotivation motivation);
 
-  // Instigate pre-connection to any URLs, or pre-resolution of related host,
-  // that we predict will be needed after this navigation (typically
-  // more-embedded resources on a page).  This method will actually post a task
-  // to do the actual work, so as not to jump ahead of the frame navigation that
-  // instigated this activity.
+  // Instigate pre-connection to any URLs we predict will be needed after this
+  // navigation (typically more-embedded resources on a page).
   void PredictFrameSubresources(const GURL& url);
 
   // Record details of a navigation so that we can preresolve the host name
@@ -141,6 +138,8 @@ class Predictor : public base::RefCountedThreadSafe<Predictor> {
   FRIEND_TEST_ALL_PREFIXES(PredictorTest, PriorityQueueReorderTest);
   friend class WaitForResolutionHelper;  // For testing.
 
+  ~Predictor();
+
   class LookupRequest;
 
   // A simple priority queue for handling host names.
@@ -175,13 +174,6 @@ class Predictor : public base::RefCountedThreadSafe<Predictor> {
   // in a Referrer instance, which is a value in this map.
   typedef std::map<GURL, Referrer> Referrers;
 
-  ~Predictor();
-
-  // Perform actual resolution or preconnection to subresources now.  This is
-  // an internal worker method that is reached via a post task from
-  // PredictFrameSubresources().
-  void PrepareFrameSubresources(const GURL& url);
-
   // Only for testing. Returns true if hostname has been successfully resolved
   // (name found).
   bool WasFound(const GURL& url) const {
diff --git a/chrome/common/chrome_switches.cc b/chrome/common/chrome_switches.cc
index 97ef576..c12e9a1 100644
--- a/chrome/common/chrome_switches.cc
+++ b/chrome/common/chrome_switches.cc
@@ -164,9 +164,6 @@ const char kDisableInternalFlash[]          = "disable-internal-flash";
 // This flag can be overidden by the "enable-ipv6" flag.
 const char kDisableIPv6[]                   = "disable-ipv6";
 
-// Disable speculative TCP/IP preconnection.
-const char kDisablePreconnect[]              = "disable-preconnect";
-
 // Don't execute JavaScript (browser JS like the new tab page still runs).
 const char kDisableJavaScript[]             = "disable-javascript";
 
diff --git a/chrome/common/chrome_switches.h b/chrome/common/chrome_switches.h
index 50f75d4..b34fbb4 100644
--- a/chrome/common/chrome_switches.h
+++ b/chrome/common/chrome_switches.h
@@ -61,7 +61,6 @@ extern const char kDisableGeolocation[];
 extern const char kDisableHangMonitor[];
 extern const char kDisableInternalFlash[];
 extern const char kDisableIPv6[];
-extern const char kDisablePreconnect[];
 extern const char kDisableJavaScript[];
 extern const char kDisableJava[];
 extern const char kDisableLocalStorage[];
diff --git a/net/net.gyp b/net/net.gyp
index b7f6245..64eb95a 100644
--- a/net/net.gyp
+++ b/net/net.gyp
@@ -459,7 +459,6 @@
         'proxy/proxy_service.h',
         'proxy/sync_host_resolver_bridge.cc',
         'proxy/sync_host_resolver_bridge.h',
-        'socket/client_socket.cc',
         'socket/client_socket.h',
         'socket/client_socket_factory.cc',
         'socket/client_socket_factory.h',
diff --git a/net/socket/client_socket.cc b/net/socket/client_socket.cc
deleted file mode 100644
index dc63f35..0000000
--- a/net/socket/client_socket.cc
+++ /dev/null
@@ -1,69 +0,0 @@
-// Copyright (c) 2010 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.h"
-
-#include "base/histogram.h"
-
-namespace net {
-
-ClientSocket::ClientSocket()
-    : was_ever_connected_(false),
-      omnibox_speculation_(false),
-      subresource_speculation_(false),
-      was_used_to_transmit_data_(false) {}
-
-ClientSocket::~ClientSocket() {
-  EmitPreconnectionHistograms();
-}
-
-void ClientSocket::EmitPreconnectionHistograms() const {
-  DCHECK(!subresource_speculation_ || !omnibox_speculation_);
-  // 0 ==> non-speculative, never connected.
-  // 1 ==> non-speculative never used (but connected).
-  // 2 ==> non-spculative and used.
-  // 3 ==> omnibox_speculative never connected.
-  // 4 ==> omnibox_speculative never used (but connected).
-  // 5 ==> omnibox_speculative and used.
-  // 6 ==> subresource_speculative never connected.
-  // 7 ==> subresource_speculative never used (but connected).
-  // 8 ==> subresource_speculative and used.
-  int result;
-  if (was_used_to_transmit_data_)
-    result = 2;
-  else if (was_ever_connected_)
-    result = 1;
-  else
-    result = 0;  // Never used, and not really connected.
-
-  if (omnibox_speculation_)
-    result += 3;
-  else if (subresource_speculation_)
-    result += 6;
-  UMA_HISTOGRAM_ENUMERATION("Net.PreconnectUtilization", result, 9);
-}
-
-void ClientSocket::SetSubresourceSpeculation() {
-  if (was_used_to_transmit_data_)
-    return;
-  subresource_speculation_ = true;
-}
-
-void ClientSocket::SetOmniboxSpeculation() {
-  if (was_used_to_transmit_data_)
-    return;
-  omnibox_speculation_ = true;
-}
-
-void ClientSocket::UpdateConnectivityState(bool is_reused) {
-  // Record if this connection has every actually connected successfully.
-  // Note that IsConnected() won't be defined at destruction time, so we need
-  // to record this data now, while the derived class is present.
-  was_ever_connected_ |= IsConnected();
-  // A socket is_reused only after it has transmitted some data.
-  was_used_to_transmit_data_ |= is_reused;
-}
-
-}  // namespace net
-
diff --git a/net/socket/client_socket.h b/net/socket/client_socket.h
index 44ee085..29d297e 100644
--- a/net/socket/client_socket.h
+++ b/net/socket/client_socket.h
@@ -15,26 +15,6 @@ class BoundNetLog;
 
 class ClientSocket : public Socket {
  public:
-  ClientSocket();
-
-  // Destructor emits statistics for this socket's lifetime.
-  virtual ~ClientSocket();
-
-  // Set the annotation to indicate this socket was created for speculative
-  // reasons.  Note that if the socket was used before calling this method, then
-  // the call will be ignored (no annotation will be added).
-  void SetSubresourceSpeculation();
-  void SetOmniboxSpeculation();
-
-  // Establish values of was_ever_connected_ and was_used_to_transmit_data_.
-  // The argument indicates if the socket's state, as reported by a
-  // ClientSocketHandle::is_reused(), should show that the socket has already
-  // been used to transmit data.
-  // This is typically called when a transaction finishes, and
-  // ClientSocketHandle is being destroyed.  Calling at that point it allows us
-  // to aggregates the impact of that connect job into this instance.
-  void UpdateConnectivityState(bool is_reused);
-
   // Called to establish a connection.  Returns OK if the connection could be
   // established synchronously.  Otherwise, ERR_IO_PENDING is returned and the
   // given callback will run asynchronously when the connection is established
@@ -74,24 +54,6 @@ class ClientSocket : public Socket {
 
   // Gets the NetLog for this socket.
   virtual const BoundNetLog& NetLog() const = 0;
-
- private:
-  // Publish histogram to help evaluate preconnection utilization.
-  void EmitPreconnectionHistograms() const;
-
-  // Indicate if any ClientSocketHandle that used this socket was connected as
-  // would be indicated by the IsConnected() method.  This variable set by a
-  // ClientSocketHandle before releasing this ClientSocket.
-  bool was_ever_connected_;
-
-  // Indicate if this socket was first created for speculative use, and identify
-  // the motivation.
-  bool omnibox_speculation_;
-  bool subresource_speculation_;
-
-  // Indicate if this socket was ever used. This state is set by a
-  // ClientSocketHandle before releasing this ClientSocket.
-  bool was_used_to_transmit_data_;
 };
 
 }  // namespace net
diff --git a/net/socket/client_socket_handle.cc b/net/socket/client_socket_handle.cc
index ff0b2c5..d9ccbd5 100644
--- a/net/socket/client_socket_handle.cc
+++ b/net/socket/client_socket_handle.cc
@@ -22,9 +22,6 @@ ClientSocketHandle::ClientSocketHandle()
 
 ClientSocketHandle::~ClientSocketHandle() {
   Reset();
-  // Manually release socket, so that we also cause state to be recorded into
-  // associated socket_ as needed.
-  release_socket();
 }
 
 void ClientSocketHandle::Reset() {
diff --git a/net/socket/client_socket_handle.h b/net/socket/client_socket_handle.h
index bcb94cb..adccc89 100644
--- a/net/socket/client_socket_handle.h
+++ b/net/socket/client_socket_handle.h
@@ -132,16 +132,7 @@ class ClientSocketHandle {
   const std::string& group_name() const { return group_name_; }
   int id() const { return pool_id_; }
   ClientSocket* socket() { return socket_.get(); }
-  ClientSocket* release_socket() {
-    // Update the base class to record things like whether we've ever
-    // transmitted data, and whether the connection was able to be established.
-    // We use this data to construct histograms indicating whether a
-    // speculative connection was ever used, etc., when the ClientSocket is
-    // eventually discarded.
-    if (socket_.get())
-      socket_->UpdateConnectivityState(is_reused());
-    return socket_.release();
-  }
+  ClientSocket* release_socket() { return socket_.release(); }
   bool is_reused() const { return is_reused_; }
   base::TimeDelta idle_time() const { return idle_time_; }
   SocketReuseType reuse_type() const {
diff --git a/net/socket/client_socket_pool_base.h b/net/socket/client_socket_pool_base.h
index 96c78d5..30cff70 100644
--- a/net/socket/client_socket_pool_base.h
+++ b/net/socket/client_socket_pool_base.h
@@ -491,7 +491,7 @@ class ClientSocketPoolBase {
             const scoped_refptr<SocketParams>& params,
             const BoundNetLog& net_log)
         : internal::ClientSocketPoolBaseHelper::Request(
-              handle, callback, priority, net_log),
+            handle, callback, priority, net_log),
           params_(params) {}
 
     const scoped_refptr<SocketParams>& params() const { return params_; }
@@ -531,9 +531,9 @@ class ClientSocketPoolBase {
       ConnectJobFactory* connect_job_factory)
       : histograms_(histograms),
         helper_(new internal::ClientSocketPoolBaseHelper(
-            max_sockets, max_sockets_per_group,
-            unused_idle_socket_timeout, used_idle_socket_timeout,
-            new ConnectJobFactoryAdaptor(connect_job_factory))) {}
+          max_sockets, max_sockets_per_group,
+          unused_idle_socket_timeout, used_idle_socket_timeout,
+          new ConnectJobFactoryAdaptor(connect_job_factory))) {}
 
   virtual ~ClientSocketPoolBase() {}
 
@@ -611,7 +611,8 @@ class ClientSocketPoolBase {
     typedef typename ClientSocketPoolBase<SocketParams>::ConnectJobFactory
         ConnectJobFactory;
 
-    explicit ConnectJobFactoryAdaptor(ConnectJobFactory* connect_job_factory)
+    explicit ConnectJobFactoryAdaptor(
+        ConnectJobFactory* connect_job_factory)
         : connect_job_factory_(connect_job_factory) {}
     virtual ~ConnectJobFactoryAdaptor() {}