Plumbing for TCP FastOpen for SSL sockets.

net/socket/transport_client_socket_pool_unittest.cc

Index: net/socket/transport_client_socket_pool_unittest.cc
diff --git a/net/socket/transport_client_socket_pool_unittest.cc b/net/socket/transport_client_socket_pool_unittest.cc
index 1cc015a1fa1b4842f5fcc1d1b6568353e24e057f..64130f63abd49815121368ff328bdeb9edcf327d 100644
--- a/net/socket/transport_client_socket_pool_unittest.cc
+++ b/net/socket/transport_client_socket_pool_unittest.cc
@@ -28,6 +28,14 @@ namespace net {
 
 using internal::ClientSocketPoolBaseHelper;
 
+class TransportSocketParamsPeer {
+ public:
+  static void SetCombineConnectAndWritePolicy(TransportSocketParams* params,
+      TransportSocketParams::CombineConnectAndWritePolicy new_policy) {
+    params->combine_connect_and_write_ = new_policy;
+  }
+};
+
 namespace {
 
 const int kMaxSockets = 32;
@@ -40,9 +48,12 @@ class TransportClientSocketPoolTest : public testing::Test {
       : connect_backup_jobs_enabled_(
             ClientSocketPoolBaseHelper::set_connect_backup_jobs_enabled(true)),
         params_(
-            new TransportSocketParams(HostPortPair("www.google.com", 80),
-                                      false, false,
-                                      OnHostResolutionCallback())),
+            new TransportSocketParams(
+                HostPortPair("www.google.com", 80),
+                false,
+                false,
+                OnHostResolutionCallback(),
+                TransportSocketParams::COMBINE_CONNECT_AND_WRITE_DEFAULT)),
         histograms_(new ClientSocketPoolHistograms("TCPUnitTest")),
         host_resolver_(new MockHostResolver),
         client_socket_factory_(&net_log_),
@@ -62,7 +73,8 @@ class TransportClientSocketPoolTest : public testing::Test {
   int StartRequest(const std::string& group_name, RequestPriority priority) {
     scoped_refptr<TransportSocketParams> params(new TransportSocketParams(
         HostPortPair("www.google.com", 80), false, false,
-        OnHostResolutionCallback()));
+        OnHostResolutionCallback(),
+        TransportSocketParams::COMBINE_CONNECT_AND_WRITE_DEFAULT));
     return test_base_.StartRequestUsingPool(
         &pool_, group_name, priority, params);
   }
@@ -201,8 +213,8 @@ TEST_F(TransportClientSocketPoolTest, InitHostResolutionFailure) {
   ClientSocketHandle handle;
   HostPortPair host_port_pair("unresolvable.host.name", 80);
   scoped_refptr<TransportSocketParams> dest(new TransportSocketParams(
-      host_port_pair, false, false,
-      OnHostResolutionCallback()));
+      host_port_pair, false, false, OnHostResolutionCallback(),
+      TransportSocketParams::COMBINE_CONNECT_AND_WRITE_DEFAULT));
   EXPECT_EQ(ERR_IO_PENDING,
             handle.Init("a", dest, kDefaultPriority, callback.callback(),
                         &pool_, BoundNetLog()));
@@ -477,7 +489,8 @@ class RequestSocketCallback : public TestCompletionCallbackBase {
       within_callback_ = true;
       scoped_refptr<TransportSocketParams> dest(new TransportSocketParams(
           HostPortPair("www.google.com", 80), false, false,
-          OnHostResolutionCallback()));
+          OnHostResolutionCallback(),
+          TransportSocketParams::COMBINE_CONNECT_AND_WRITE_DEFAULT));
       int rv = handle_->Init("a", dest, LOWEST, callback(), pool_,
                              BoundNetLog());
       EXPECT_EQ(OK, rv);
@@ -497,7 +510,8 @@ TEST_F(TransportClientSocketPoolTest, RequestTwice) {
   RequestSocketCallback callback(&handle, &pool_);
   scoped_refptr<TransportSocketParams> dest(new TransportSocketParams(
       HostPortPair("www.google.com", 80), false, false,
-      OnHostResolutionCallback()));
+      OnHostResolutionCallback(),
+      TransportSocketParams::COMBINE_CONNECT_AND_WRITE_DEFAULT));
   int rv = handle.Init("a", dest, LOWEST, callback.callback(), &pool_,
                        BoundNetLog());
   ASSERT_EQ(ERR_IO_PENDING, rv);
@@ -964,6 +978,95 @@ TEST_F(TransportClientSocketPoolTest, IPv4HasNoFallback) {
   EXPECT_EQ(1, client_socket_factory_.allocation_count());
 }
 
+// Test that if TCP FastOpen is enabled, it is set on the socket
+// when we have only an IPv4 address.
+TEST_F(TransportClientSocketPoolTest, TCPFastOpenOnIPv4WithNoFallback) {
+  // Create a pool without backup jobs.
+  ClientSocketPoolBaseHelper::set_connect_backup_jobs_enabled(false);
+  TransportClientSocketPool pool(kMaxSockets,
+                                 kMaxSocketsPerGroup,
+                                 histograms_.get(),
+                                 host_resolver_.get(),
+                                 &client_socket_factory_,
+                                 NULL);
+  client_socket_factory_.set_default_client_socket_type(
+      MockTransportClientSocketFactory::MOCK_DELAYED_CLIENT_SOCKET);
+  // Resolve an AddressList with only IPv4 addresses.
+  host_resolver_->rules()->AddIPLiteralRule("*", "1.1.1.1", std::string());
+
+  TestCompletionCallback callback;
+  ClientSocketHandle handle;
+  // Enable TCP FastOpen in TransportSocketParams.
+  TransportSocketParamsPeer::SetCombineConnectAndWritePolicy(

[mmenke, 2014/09/11 18:14:35]

Rather than creating a peer for this purpose, suggest either just creating a new
set of params inline, copying the other values from params_.

[mmenke, 2014/09/11 18:20:37]

Oops...left out my "or".  Or making a function to create params, that takes in a
combine connect and write setting.

[Jana, 2014/09/12 01:04:39]

Done.

+      params_.get(),
+      TransportSocketParams::COMBINE_CONNECT_AND_WRITE_DESIRED);
+  handle.Init("a", params_, LOW, callback.callback(), &pool, BoundNetLog());
+  EXPECT_EQ(OK, callback.WaitForResult());
+  EXPECT_TRUE(handle.socket()->UsingTCPFastOpen());
+}
+
+// Test that if TCP FastOpen is enabled, it is set on the socket
+// when we have only IPv6 addresses.
+TEST_F(TransportClientSocketPoolTest, TCPFastOpenOnIPv6WithNoFallback) {
+  // Create a pool without backup jobs.
+  ClientSocketPoolBaseHelper::set_connect_backup_jobs_enabled(false);
+  TransportClientSocketPool pool(kMaxSockets,
+                                 kMaxSocketsPerGroup,
+                                 histograms_.get(),
+                                 host_resolver_.get(),
+                                 &client_socket_factory_,
+                                 NULL);
+  client_socket_factory_.set_default_client_socket_type(
+      MockTransportClientSocketFactory::MOCK_DELAYED_CLIENT_SOCKET);
+  // Resolve an AddressList with only IPv6 addresses.
+  host_resolver_->rules()
+      ->AddIPLiteralRule("*", "2:abcd::3:4:ff,3:abcd::3:4:ff", std::string());
+
+  TestCompletionCallback callback;
+  ClientSocketHandle handle;
+  // Enable TCP FastOpen in TransportSocketParams.
+  TransportSocketParamsPeer::SetCombineConnectAndWritePolicy(
+      params_.get(),
+      TransportSocketParams::COMBINE_CONNECT_AND_WRITE_DESIRED);
+  handle.Init("a", params_, LOW, callback.callback(), &pool, BoundNetLog());
+  EXPECT_EQ(OK, callback.WaitForResult());
+  EXPECT_TRUE(handle.socket()->UsingTCPFastOpen());
+}
+
+// Test that if TCP FastOpen is enabled, it does not do anything when there
+// is a IPv6 address with fallback to an IPv4 address.
+TEST_F(TransportClientSocketPoolTest, NoTCPFastOpenOnIPv6WithIPv4Fallback) {

[mmenke, 2014/09/11 18:14:35]

Suggest a test just like this, except the IPv6 socket succeeds, to make sure
that one isn't using fast open, either.

Not sure if you'll need to set up a stalled IPv4 socket or not - I'd guess not,
but you should double chek that.

[Jana, 2014/09/12 01:04:39]

Added test, PTAL.

+  // Create a pool without backup jobs.
+  ClientSocketPoolBaseHelper::set_connect_backup_jobs_enabled(false);
+  TransportClientSocketPool pool(kMaxSockets,
+                                 kMaxSocketsPerGroup,
+                                 histograms_.get(),
+                                 host_resolver_.get(),
+                                 &client_socket_factory_,
+                                 NULL);
+
+  MockTransportClientSocketFactory::ClientSocketType case_types[] = {
+    // This is the IPv6 socket.
+    MockTransportClientSocketFactory::MOCK_STALLED_CLIENT_SOCKET,
+    // This is the IPv4 socket.
+    MockTransportClientSocketFactory::MOCK_PENDING_CLIENT_SOCKET
+  };
+  client_socket_factory_.set_client_socket_types(case_types, 2);
+  // Resolve an AddressList with a IPv6 address first and then a IPv4 address.
+  host_resolver_->rules()
+      ->AddIPLiteralRule("*", "2:abcd::3:4:ff,2.2.2.2", std::string());
+
+  TestCompletionCallback callback;
+  ClientSocketHandle handle;
+  // Enable TCP FastOpen in TransportSocketParams.
+  TransportSocketParamsPeer::SetCombineConnectAndWritePolicy(
+      params_.get(),
+      TransportSocketParams::COMBINE_CONNECT_AND_WRITE_DESIRED);
+  handle.Init("a", params_, LOW, callback.callback(), &pool, BoundNetLog());
+  EXPECT_EQ(OK, callback.WaitForResult());
+  EXPECT_FALSE(handle.socket()->UsingTCPFastOpen());

[mmenke, 2014/09/11 18:14:35]

As a sanity check, suggest you check that the connected socket is using an IPv4
IP.  We have tests for that above, but I'm paranoid.

[Jana, 2014/09/12 01:04:39]

In general, I like to not duplicate test coverage. In this case, since we need
to ensure that the cases of IPv6 succeeding vs. failing are presuppositions for
the rest of the test, adding this check has a slightly different value than for
the tests above, so adding it.

+}
+
 }  // namespace
 
 }  // namespace net