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

I pulled out the code to update the socket connectivity stats.

I added defensive code which should preclude the crash that 
was reported on the stability bot.

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

As noted in the original checkin:

Enable speculative preconnection by default

Added histogram to track preconnection utilization (vs waste).

BUG=42694
r=mbelshe
Review URL: http://codereview.chromium.org/3050040

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

6 files changed, 127 insertions, 7 deletions

diff --git a/net/net.gyp b/net/net.gyp
index 64eb95a..b7f6245 100644
--- a/net/net.gyp
+++ b/net/net.gyp
@@ -459,6 +459,7 @@
         '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
new file mode 100644
index 0000000..c0c62cd
--- /dev/null
+++ b/net/socket/client_socket.cc
@@ -0,0 +1,69 @@
+// 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-speculative 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 29d297e..44ee085 100644
--- a/net/socket/client_socket.h
+++ b/net/socket/client_socket.h
@@ -15,6 +15,26 @@ 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
@@ -54,6 +74,24 @@ 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 d9ccbd5..ee7c233 100644
--- a/net/socket/client_socket_handle.cc
+++ b/net/socket/client_socket_handle.cc
@@ -22,6 +22,7 @@ ClientSocketHandle::ClientSocketHandle()
 
 ClientSocketHandle::~ClientSocketHandle() {
   Reset();
+  UpdateConnectivityStateForSocket();
 }
 
 void ClientSocketHandle::Reset() {
diff --git a/net/socket/client_socket_handle.h b/net/socket/client_socket_handle.h
index adccc89..a3de0d6 100644
--- a/net/socket/client_socket_handle.h
+++ b/net/socket/client_socket_handle.h
@@ -132,7 +132,10 @@ 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() { return socket_.release(); }
+  ClientSocket* release_socket() {
+    UpdateConnectivityStateForSocket();
+    return socket_.release();
+  }
   bool is_reused() const { return is_reused_; }
   base::TimeDelta idle_time() const { return idle_time_; }
   SocketReuseType reuse_type() const {
@@ -174,6 +177,15 @@ class ClientSocketHandle {
   // Resets the supplemental error state.
   void ResetErrorState();
 
+  // 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.
+  void UpdateConnectivityStateForSocket() const {
+    if (socket_.get())
+      socket_->UpdateConnectivityState(is_reused());
+  }
+
   bool is_initialized_;
   scoped_refptr<ClientSocketPool> pool_;
   scoped_ptr<ClientSocket> socket_;
diff --git a/net/socket/client_socket_pool_base.h b/net/socket/client_socket_pool_base.h
index 30cff70..96c78d5 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,8 +611,7 @@ 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() {}