Plumbing for TCP FastOpen for SSL sockets.

net/socket/client_socket_pool_manager.cc

Index: net/socket/client_socket_pool_manager.cc
diff --git a/net/socket/client_socket_pool_manager.cc b/net/socket/client_socket_pool_manager.cc
index a7bf1fe5b753bc7796b1019c7260649f06a6bc06..7d8292507f719823b9ac3d51a62462afa9a3e129 100644
--- a/net/socket/client_socket_pool_manager.cc
+++ b/net/socket/client_socket_pool_manager.cc
@@ -84,7 +84,6 @@ int InitSocketPoolHelper(const GURL& request_url,
                          HttpNetworkSession::SocketPoolType socket_pool_type,
                          const OnHostResolutionCallback& resolution_callback,
                          const CompletionCallback& callback) {
-  scoped_refptr<TransportSocketParams> tcp_params;
   scoped_refptr<HttpProxySocketParams> http_proxy_params;
   scoped_refptr<SOCKSSocketParams> socks_params;
   scoped_ptr<HostPortPair> proxy_host_port;
@@ -155,12 +154,7 @@ int InitSocketPoolHelper(const GURL& request_url,
   }
 
   bool ignore_limits = (request_load_flags & LOAD_IGNORE_LIMITS) != 0;

[mmenke, 2014/08/14 17:56:03]

The struct was effectively const before, suggest keeping that way, and adding a
parameter.  You can keep the old construction order, but instead pass in
using_ssl as the new argument when you construct it here...

I'm also fine with splitting the constructor calls in the SSL and non-SSL cases,
as you're doing here (With the additional constructor parameter), just not sure
how much it gets us.

[Jana, 2014/08/15 20:00:06]

As we discussed offline, I've added a new constructor that accepts this param.
I've also added a TODO to clean this up by replacing the single constructor
instead of adding a new one. Not doing it in this CL to avoid touching many more
files here.

-  if (proxy_info.is_direct()) {
-    tcp_params = new TransportSocketParams(origin_host_port,
-                                           disable_resolver_cache,
-                                           ignore_limits,
-                                           resolution_callback);
-  } else {
+  if (!proxy_info.is_direct()) {
     ProxyServer proxy_server = proxy_info.proxy_server();
     proxy_host_port.reset(new HostPortPair(proxy_server.host_port_pair()));
     scoped_refptr<TransportSocketParams> proxy_tcp_params(
@@ -175,6 +169,9 @@ int InitSocketPoolHelper(const GURL& request_url,
                                       &user_agent);
       scoped_refptr<SSLSocketParams> ssl_params;
       if (proxy_info.is_https()) {
+        // Enable TCP FastOpen for SSL sockets.
+        // TODO (jri): Finch trial this.
+        proxy_tcp_params->enable_use_tcp_fastopen();
         // Set ssl_params, and unset proxy_tcp_params
         ssl_params = new SSLSocketParams(proxy_tcp_params,
                                          NULL,
@@ -220,8 +217,16 @@ int InitSocketPoolHelper(const GURL& request_url,
 
   // Deal with SSL - which layers on top of any given proxy.
   if (using_ssl) {
+    scoped_refptr<TransportSocketParams> ssl_tcp_params =
+      new TransportSocketParams(origin_host_port,
+                                disable_resolver_cache,
+                                ignore_limits,
+                                resolution_callback);
+    // Enable TCP FastOpen for SSL socket.
+    // TODO (jri): Finch trial this.
+    ssl_tcp_params->enable_use_tcp_fastopen();
     scoped_refptr<SSLSocketParams> ssl_params =
-        new SSLSocketParams(tcp_params,
+        new SSLSocketParams(ssl_tcp_params,
                             socks_params,
                             http_proxy_params,
                             origin_host_port,
@@ -280,7 +285,11 @@ int InitSocketPoolHelper(const GURL& request_url,
   }
 
   DCHECK(proxy_info.is_direct());
-
+  scoped_refptr<TransportSocketParams> tcp_params =
+    new TransportSocketParams(origin_host_port,
+                              disable_resolver_cache,
+                              ignore_limits,
+                              resolution_callback);

[mmenke, 2014/08/14 17:56:03]

Shouldn't we be enabling fastopen if g_tcp_fastopen_enabled is true?

[mmenke, 2014/08/14 17:56:03]

nit:  +2 indent

[Jana, 2014/08/15 20:00:06]

Yes -- but the global is being used on *all* sockets and therefore enabled
further below

[Jana, 2014/08/16 00:01:11]

Sorry, I seem to have left this sentence hanging. I meant to say that I enable
it in tcp_socket_libevent.cc, because this global is used to turn TFO on for all
TCP sockets.

   TransportClientSocketPool* pool =
       session->GetTransportSocketPool(socket_pool_type);
   if (num_preconnect_streams) {