Plumbing for TCP FastOpen for SSL sockets.

net/socket/transport_client_socket_pool.cc

Index: net/socket/transport_client_socket_pool.cc
diff --git a/net/socket/transport_client_socket_pool.cc b/net/socket/transport_client_socket_pool.cc
index 7527eca52edf547d3e75a102bf8b79374ba846a8..2203de5066ed5be2a24d03ab162206a52b726cad 100644
--- a/net/socket/transport_client_socket_pool.cc
+++ b/net/socket/transport_client_socket_pool.cc
@@ -25,6 +25,8 @@
 #include "net/socket/tcp_client_socket.h"
 
 using base::TimeDelta;
+extern bool g_tcp_fastopen_user_enabled;
+extern bool g_tcp_fastopen_supported;
 
 namespace net {
 
@@ -60,12 +62,22 @@ TransportSocketParams::TransportSocketParams(
     const HostPortPair& host_port_pair,
     bool disable_resolver_cache,
     bool ignore_limits,
-    const OnHostResolutionCallback& host_resolution_callback)
+    const OnHostResolutionCallback& host_resolution_callback,
+    AutoConnectWithWritePolicy auto_connect_with_write_if_supported)
     : destination_(host_port_pair),
       ignore_limits_(ignore_limits),
-      host_resolution_callback_(host_resolution_callback) {
+      host_resolution_callback_(host_resolution_callback),
+      auto_connect_with_write_(auto_connect_with_write_if_supported) {
   if (disable_resolver_cache)
     destination_.set_allow_cached_response(false);
+  // Auto-connect currently translates to only TCP Fast Open.
+  // Enable TCP FastOpen if user wants it and system supports it.
+  if (!g_tcp_fastopen_supported)
+    auto_connect_with_write_ = AUTO_CONNECT_DISABLED;
+  else

[mmenke, 2014/09/09 14:29:28]

Use braces for both cases of an if/else statement when any one of the blocks
takes up more than one line.

[Jana, 2014/09/10 16:03:11]

Done.

+    if (g_tcp_fastopen_user_enabled &&
+        auto_connect_with_write_ == AUTO_CONNECT_USE_DEFAULT)
+      auto_connect_with_write_ = AUTO_CONNECT_ENABLED;

[mmenke, 2014/09/09 14:29:28]

Use braces for an if when the antecedent takes up more than one line.

[Jana, 2014/09/10 16:03:11]

Done.

 }
 
 TransportSocketParams::~TransportSocketParams() {}
@@ -261,6 +273,15 @@ int TransportConnectJob::DoTransportConnect() {
   transport_socket_ =
       helper_.client_socket_factory()->CreateTransportClientSocket(
           helper_.addresses(), net_log().net_log(), net_log().source());
+
+  // Enable TCP FastOpen if indicated by transport socket params.
+  // Note: We currently do not turn on TCP FastOpen for destinations where
+  // we might try a TCP connect over IPv6 with fallback to IPv4.
+  if (helper_.params()->auto_connect_with_write() == AUTO_CONNECT_ENABLED &&
+      (helper_.addresses().front().GetFamily() != ADDRESS_FAMILY_IPV6 ||
+       AddressListOnlyContainsIPv6(helper_.addresses())))

[mmenke, 2014/09/09 14:29:28]

optional:  Think this is a little clearer if the last line is indented 4 spaces
instead of 1.

[mmenke, 2014/09/09 16:03:33]

Should we have any tests for this?  Not sure how hard they'd be to manage...Just
use mock sockets, and check that fast open was/wasn't set?

[mmenke, 2014/09/09 16:03:33]

optional:  Or may be clearer as a function in an anonymous namespace: 
AddressListContainsIPv6AndIPv4 or something.  Up to you.  It's not a complicated
check, but think removing it reduces complexity when reading the code.

[Jana, 2014/09/10 16:03:11]

Acknowledged.

[Jana, 2014/09/10 16:03:11]

Do you have a pointer for a test that currently tests happy eyeballs? One
difficulty is that currently setting fast open on a socket requires the system
to support it (this is the /proc file read for checking if the system supports
fast open.) 

[mmenke, 2014/09/10 17:32:22]

Yea, we'd need to force the tests to think they had (Or did not have) happy
eyeballs support.

TransportClientSocketPoolTest.IPv6FallbackSocketIPv4FinishesFirst and the nearby
tests check happy eyeballs behavior.

[Jana, 2014/09/11 16:02:36]

Tests added in transport_client_socket_pool_unittest.cc, PTAL.

+    transport_socket_->EnableTCPFastOpen();

[mmenke, 2014/09/09 14:29:28]

use braces.

[Jana, 2014/09/10 16:03:11]

This code has changed, but what's the formatting logic for using braces here?
There is no body that is > 1 line. Is this because the previous expression (the
boolean expression) is > 1 line?

[mmenke, 2014/09/10 17:32:22]

It's because the antecedent of the if statement takes up more than one line -
basically, if anything takes up more than one line, use braces.  They're also
often, though not always, used on else/ifs, even if everything is a single line.

[Jana, 2014/09/11 16:02:36]

Acknowledged.

+
   int rv = transport_socket_->Connect(helper_.on_io_complete());
   if (rv == ERR_IO_PENDING &&
       helper_.addresses().front().GetFamily() == ADDRESS_FAMILY_IPV6 &&