Plumbing for TCP FastOpen for SSL sockets.

net/socket/client_socket_pool_manager.cc

 // Copyright (c) 2012 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_manager.h"
 
 #include <string>
 
 #include "base/basictypes.h"
 #include "base/logging.h"
@@ skipping 66 matching lines @@
                          const SSLConfig& ssl_config_for_origin,
                          const SSLConfig& ssl_config_for_proxy,
                          bool force_tunnel,
                          PrivacyMode privacy_mode,
                          const BoundNetLog& net_log,
                          int num_preconnect_streams,
                          ClientSocketHandle* socket_handle,
                          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;
 
   bool using_ssl = request_url.SchemeIs("https") ||
       request_url.SchemeIs("wss") || force_spdy_over_ssl;
 
   HostPortPair origin_host_port =
       HostPortPair(request_url.HostNoBrackets(),
                    request_url.EffectiveIntPort());
@@ skipping 49 matching lines @@
           prefix = "sslv3/";
           break;
         default:
           CHECK(false);
           break;
       }
     }
     connection_group = prefix + connection_group;
   }
 
   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(
         new TransportSocketParams(*proxy_host_port,
                                   disable_resolver_cache,
                                   ignore_limits,
                                   resolution_callback));
 
     if (proxy_info.is_http() || proxy_info.is_https()) {
       std::string user_agent;
       request_extra_headers.GetHeader(HttpRequestHeaders::kUserAgent,
                                       &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,
                                          NULL,
                                          *proxy_host_port.get(),
                                          ssl_config_for_proxy,
                                          PRIVACY_MODE_DISABLED,
                                          load_flags,
                                          force_spdy_over_ssl,
                                          want_spdy_over_npn);
@@ skipping 25 matching lines @@
                                            origin_host_port);
     }
   }
 
   // Change group name if privacy mode is enabled.
   if (privacy_mode == PRIVACY_MODE_ENABLED)
     connection_group = "pm/" + connection_group;
 
   // 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,
                             ssl_config_for_origin,
                             privacy_mode,
                             load_flags,
                             force_spdy_over_ssl,
                             want_spdy_over_npn);
     SSLClientSocketPool* ssl_pool = NULL;
     if (proxy_info.is_direct()) {
@@ skipping 38 matching lines @@
                             num_preconnect_streams, net_log);
       return OK;
     }
 
     return socket_handle->Init(connection_group, socks_params,
                                request_priority, callback, pool,
                                net_log);
   }
 
   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) {
     RequestSocketsForPool(pool, connection_group, tcp_params,
                           num_preconnect_streams, net_log);
     return OK;
   }
 
   return socket_handle->Init(connection_group, tcp_params,
                              request_priority, callback,
@@ skipping 183 matching lines @@
     int num_preconnect_streams) {
   return InitSocketPoolHelper(
       request_url, request_extra_headers, request_load_flags, request_priority,
       session, proxy_info, force_spdy_over_ssl, want_spdy_over_npn,
       ssl_config_for_origin, ssl_config_for_proxy, false, privacy_mode, net_log,
       num_preconnect_streams, NULL, HttpNetworkSession::NORMAL_SOCKET_POOL,
       OnHostResolutionCallback(), CompletionCallback());
 }
 
 }  // namespace net