This CL adds unit tests for false start connections.

net/socket/ssl_client_socket_unittest.cc

Index: net/socket/ssl_client_socket_unittest.cc
diff --git a/net/socket/ssl_client_socket_unittest.cc b/net/socket/ssl_client_socket_unittest.cc
index 28083fcf8c0dcbbcd180315597ed4c0dc3f8948e..8bf86e5795436e4e3e24d2ed25167eecad4c9596 100644
--- a/net/socket/ssl_client_socket_unittest.cc
+++ b/net/socket/ssl_client_socket_unittest.cc
@@ -796,6 +796,9 @@ class SSLClientSocketCertRequestInfoTest : public SSLClientSocketTest {
 };
 
 class SSLClientSocketFalseStartTest : public SSLClientSocketTest {
+ public:
+  SSLClientSocketFalseStartTest() : monitor_handshake_callback_(false) {}
+
  protected:
   // Creates an SSLClientSocket with |client_config| attached to a
   // FakeBlockingStreamSocket, returning both in |*out_raw_transport| and
@@ -830,6 +833,12 @@ class SSLClientSocketFalseStartTest : public SSLClientSocketTest {
                               test_server()->host_port_pair(),
                               client_config);
 
+    if (monitor_handshake_callback_) {
+      sock->SetHandshakeCompletionCallback(
+          base::Bind(&SSLClientSocketTest::RecordCompletedHandshake,
+                     base::Unretained(this)));
+    }
+
     // Connect. Stop before the client processes the first server leg
     // (ServerHello, etc.)
     raw_transport->BlockReadResult();
@@ -904,6 +913,10 @@ class SSLClientSocketFalseStartTest : public SSLClientSocketTest {
       EXPECT_FALSE(callback.have_result());
     }
   }
+
+  // Indicates that the socket's handshake completion callback should
+  // be monitored.
+  bool monitor_handshake_callback_;
 };
 
 class SSLClientSocketChannelIDTest : public SSLClientSocketTest {
@@ -2770,6 +2783,64 @@ TEST_F(SSLClientSocketTest, HandshakeCallbackIsRun_WithNoneSessionCache) {
   EXPECT_TRUE(sock->IsConnected());
   EXPECT_TRUE(ran_handshake_completion_callback_);
 }
+
+TEST_F(SSLClientSocketTest, HandshakeCallbackIsRun_WithFalseStartFailure) {

[davidben, 2014/08/06 19:12:46]

Hrm. I'm not sure this test actually does anything different from
HandshakeCallbackIsRun_WithFailure. It's causing some[*] client write to fail,
but that's before false start kicks in[**].

The one case I can think of where False Start would affect your callback is if
the server's second leg failed to be read or was invalid somehow. I believe you
want to SetNextReadError right before
CreateAndConnectUntilServerFinishedReceived and then UnblockReadResult. Might
need to tweak CreateAndConnectUntilServerFinishedReceived to let you pass in a
StreamSocket parameter to wrap.

At the SSLClientSocket level, this would manifest itself as a Read() failure
rather than a Connect() failure. (Connect would actually succeed.)


[*] Transport writes and especially their errors in SSLClientSocket are
unfortunately really weird. So, the SetNextWriteError call causes the first
transport write to fail, which would be the ClientHello. So that all works out,
but we have a write buffer between TCPClientSocket and OpenSSL's state machine
for API reasons. And so OpenSSL will actually think the first write succeeded
and won't notice until the first write after we flush the write buffer. This
isn't actually a problem for HandshakeCallbackIsRun_WithFailure in so far as
you're guaranteed that'll happen before the client's second leg, so the
handshake will fail. Just a note that SetNextWriteError doesn't actually do the
obvious thing.

[**] False Start, if you're not familiar, makes the client, under some
circumstances, treat the handshake as completed after it has sent its second leg
(ClientKeyExchange, ChangeCipherSpec, Finished) and start sending application
data before waiting for the server's second leg (ChangeCipherSpec, Finished).

+  // False Start requires NPN and a forward-secret cipher suite.
+  SpawnedTestServer::SSLOptions server_options;
+  server_options.key_exchanges =
+      SpawnedTestServer::SSLOptions::KEY_EXCHANGE_DHE_RSA;
+  server_options.enable_npn = true;
+  SSLConfig client_config;
+  client_config.next_protos.push_back("http/1.1");
+
+  SpawnedTestServer test_server(SpawnedTestServer::TYPE_HTTPS,
+                                server_options,
+                                base::FilePath());
+  ASSERT_TRUE(test_server.Start());
+
+  AddressList addr;
+  ASSERT_TRUE(test_server.GetAddressList(&addr));
+
+  TestCompletionCallback callback;
+  scoped_ptr<StreamSocket> real_transport(
+      new TCPClientSocket(addr, NULL, NetLog::Source()));
+  scoped_ptr<SynchronousErrorStreamSocket> transport(
+      new SynchronousErrorStreamSocket(real_transport.Pass()));
+  int rv = callback.GetResult(transport->Connect(callback.callback()));
+  EXPECT_EQ(OK, rv);
+
+  SynchronousErrorStreamSocket* raw_transport = transport.get();
+  scoped_ptr<SSLClientSocket> sock(
+      CreateSSLClientSocket(transport.PassAs<StreamSocket>(),
+                            test_server.host_port_pair(),
+                            client_config));
+
+  sock->SetHandshakeCompletionCallback(base::Bind(
+      &SSLClientSocketTest::RecordCompletedHandshake, base::Unretained(this)));
+
+  raw_transport->SetNextWriteError(ERR_CONNECTION_RESET);
+
+  rv = callback.GetResult(sock->Connect(callback.callback()));
+  EXPECT_EQ(ERR_CONNECTION_RESET, rv);
+  EXPECT_FALSE(sock->IsConnected());
+
+  EXPECT_TRUE(ran_handshake_completion_callback_);
+}
+
+TEST_F(SSLClientSocketFalseStartTest,
+       HandshakeCallbackIsRun_WithFalseStartSuccess) {
+  // False Start requires NPN and a forward-secret cipher suite.
+  SpawnedTestServer::SSLOptions server_options;
+  server_options.key_exchanges =
+      SpawnedTestServer::SSLOptions::KEY_EXCHANGE_DHE_RSA;
+  server_options.enable_npn = true;
+  SSLConfig client_config;
+  client_config.next_protos.push_back("http/1.1");
+  monitor_handshake_callback_ = true;
+  ASSERT_NO_FATAL_FAILURE(TestFalseStart(server_options, client_config, true));
+  ASSERT_TRUE(ran_handshake_completion_callback_);
+}
+
 #endif  // defined(USE_OPENSSL)
 
 TEST_F(SSLClientSocketFalseStartTest, FalseStartEnabled) {