Implements new, more robust design for communicating between SSLConnectJobs.

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 3aa445a72f8795eac9bd305e20c203ab41595d64..36aa3a22d1be98dc1a1eda5d15bc89632986c0e0 100644
--- a/net/socket/ssl_client_socket_unittest.cc
+++ b/net/socket/ssl_client_socket_unittest.cc
@@ -25,6 +25,7 @@
 #include "net/socket/tcp_client_socket.h"
 #include "net/ssl/channel_id_service.h"
 #include "net/ssl/default_channel_id_store.h"
+#include "net/ssl/openssl_client_key_store.h"

[wtc, 2014/07/31 23:05:25]

IMPORTANT: is this part of the CL?

[mshelley, 2014/08/02 23:59:15]

Done.

 #include "net/ssl/ssl_cert_request_info.h"
 #include "net/ssl/ssl_config_service.h"
 #include "net/test/cert_test_util.h"
@@ -664,6 +665,8 @@ class SSLClientSocketTest : public PlatformTest {
     context_.transport_security_state = transport_security_state_.get();
   }
 
+  void RecordCompletedHandshake() { ran_handshake_completion_callback_ = true; }
+
  protected:
   // The address of the spawned test server, after calling StartTestServer().
   const AddressList& addr() const { return addr_; }
@@ -730,6 +733,12 @@ class SSLClientSocketTest : public PlatformTest {
       return false;
     }
 
+#if defined(USE_OPENSSL)
+    sock_->SetHandshakeCompletionCallback(
+        base::Bind(&SSLClientSocketTest::RecordCompletedHandshake,
+                   base::Unretained(this)));
+#endif

[wtc, 2014/07/31 23:05:25]

IMPORTANT: should we delete this? Ah, I see the
CompletionCallbackIsRun_WithSuccess test requires this.

+
     *result = callback_.GetResult(sock_->Connect(callback_.callback()));
     return true;
   }
@@ -740,6 +749,7 @@ class SSLClientSocketTest : public PlatformTest {
   SSLClientSocketContext context_;
   scoped_ptr<SSLClientSocket> sock_;
   CapturingNetLog log_;
+  bool ran_handshake_completion_callback_;
 
  private:
   scoped_ptr<StreamSocket> transport_;
@@ -2814,4 +2824,159 @@ TEST_F(SSLClientSocketChannelIDTest, FailingChannelIDAsync) {
   EXPECT_FALSE(sock_->IsConnected());
 }
 
+#if defined(USE_OPENSSL)
+
+TEST_F(SSLClientSocketTest, CompletionCallbackIsRun_WithFailure) {
+  SpawnedTestServer test_server(SpawnedTestServer::TYPE_HTTPS,
+                                SpawnedTestServer::kLocalhost,
+                                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);
+
+  // Disable TLS False Start to avoid handshake non-determinism.
+  SSLConfig ssl_config;
+  ssl_config.false_start_enabled = false;
+
+  SynchronousErrorStreamSocket* raw_transport = transport.get();
+  scoped_ptr<SSLClientSocket> sock(
+      CreateSSLClientSocket(transport.PassAs<StreamSocket>(),
+                            test_server.host_port_pair(),
+                            ssl_config));
+
+  sock->SetHandshakeCompletionCallback(base::Bind(
+      &SSLClientSocketTest::RecordCompletedHandshake, base::Unretained(this)));
+
+  rv = callback.GetResult(sock->Connect(callback.callback()));
+  EXPECT_EQ(OK, rv);
+  EXPECT_TRUE(sock->IsConnected());
+
+  const char request_text[] = "GET / HTTP/1.0\r\n\r\n";
+  static const int kRequestTextSize =
+      static_cast<int>(arraysize(request_text) - 1);
+  scoped_refptr<IOBuffer> request_buffer(new IOBuffer(kRequestTextSize));
+  memcpy(request_buffer->data(), request_text, kRequestTextSize);
+
+  rv = callback.GetResult(
+      sock->Write(request_buffer.get(), kRequestTextSize, callback.callback()));
+  EXPECT_EQ(kRequestTextSize, rv);
+
+  // Simulate an unclean/forcible shutdown.
+  raw_transport->SetNextReadError(ERR_CONNECTION_RESET);
+
+  scoped_refptr<IOBuffer> buf(new IOBuffer(4096));
+
+  // Note: This test will hang if this bug has regressed. Simply checking that
+  // rv != ERR_IO_PENDING is insufficient, as ERR_IO_PENDING is a legitimate
+  // result when using a dedicated task runner for NSS.
+  rv = callback.GetResult(sock->Read(buf.get(), 4096, callback.callback()));
+
+  EXPECT_TRUE(ran_handshake_completion_callback_);
+}
+
+TEST_F(SSLClientSocketTest, CompletionCallbackIsRun_WithFalseStartFailure) {
+  SpawnedTestServer test_server(SpawnedTestServer::TYPE_HTTPS,
+                                SpawnedTestServer::kLocalhost,
+                                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()));
+  // Note: |error_socket|'s ownership is handed to |transport|, but a pointer
+  // is retained in order to configure additional errors.
+  scoped_ptr<SynchronousErrorStreamSocket> error_socket(
+      new SynchronousErrorStreamSocket(real_transport.Pass()));
+  SynchronousErrorStreamSocket* raw_error_socket = error_socket.get();
+  scoped_ptr<FakeBlockingStreamSocket> transport(
+      new FakeBlockingStreamSocket(error_socket.PassAs<StreamSocket>()));
+  FakeBlockingStreamSocket* raw_transport = transport.get();
+  int rv = callback.GetResult(transport->Connect(callback.callback()));
+  EXPECT_EQ(OK, rv);
+
+  SSLConfig ssl_config;
+  ssl_config.false_start_enabled = true;
+
+  scoped_ptr<SSLClientSocket> sock(
+      CreateSSLClientSocket(transport.PassAs<StreamSocket>(),
+                            test_server.host_port_pair(),
+                            ssl_config));
+
+  sock->SetHandshakeCompletionCallback(base::Bind(
+      &SSLClientSocketTest::RecordCompletedHandshake, base::Unretained(this)));
+
+  rv = callback.GetResult(sock->Connect(callback.callback()));
+  EXPECT_EQ(OK, rv);
+  EXPECT_TRUE(sock->IsConnected());
+
+  const char request_text[] = "GET / HTTP/1.0\r\n\r\n";
+  static const int kRequestTextSize =
+      static_cast<int>(arraysize(request_text) - 1);
+  scoped_refptr<IOBuffer> request_buffer(new IOBuffer(kRequestTextSize));
+  memcpy(request_buffer->data(), request_text, kRequestTextSize);
+
+  // Simulate an unclean/forcible shutdown on the underlying socket.
+  // However, simulate this error asynchronously.
+  raw_error_socket->SetNextWriteError(ERR_CONNECTION_RESET);
+  raw_transport->BlockWrite();
+
+  // This write should complete synchronously, because the TLS ciphertext
+  // can be created and placed into the outgoing buffers independent of the
+  // underlying transport.
+  rv = callback.GetResult(
+      sock->Write(request_buffer.get(), kRequestTextSize, callback.callback()));
+  EXPECT_EQ(kRequestTextSize, rv);
+
+  scoped_refptr<IOBuffer> buf(new IOBuffer(4096));
+
+  rv = sock->Read(buf.get(), 4096, callback.callback());
+  EXPECT_EQ(ERR_IO_PENDING, rv);
+
+  // Now unblock the outgoing request, having it fail with the connection
+  // being reset.
+  raw_transport->UnblockWrite();
+
+  // Note: This will cause an inifite loop if this bug has regressed. Simply
+  // checking that rv != ERR_IO_PENDING is insufficient, as ERR_IO_PENDING
+  // is a legitimate result when using a dedicated task runner for NSS.
+  rv = callback.GetResult(rv);
+
+  EXPECT_TRUE(ran_handshake_completion_callback_);
+}
+
+// Connect to a server requesting client authentication. Send it a
+// matching certificate. It should allow the connection.

[wtc, 2014/07/31 23:05:25]

Please update this comment because this test is testing something else.

Does the test server really request client authentication?

[mshelley, 2014/08/02 23:59:15]

Done.

+TEST_F(SSLClientSocketTest, CompletionCallbackIsRun_WithSuccess) {
+  SpawnedTestServer::SSLOptions ssl_options;
+
+  ASSERT_TRUE(ConnectToTestServer(ssl_options));
+
+  base::FilePath certs_dir = GetTestCertsDirectory();
+  SSLConfig ssl_config = kDefaultSSLConfig;
+  ssl_config.send_client_cert = true;
+  ssl_config.client_cert = ImportCertFromFile(certs_dir, "client_1.pem");
+
+  int rv;
+  ASSERT_TRUE(CreateAndConnectSSLClientSocket(ssl_config, &rv));
+  EXPECT_EQ(OK, rv);
+
+  EXPECT_TRUE(sock_->IsConnected());
+
+  EXPECT_TRUE(ran_handshake_completion_callback_);
+}
+
+#endif  // defined(USE_OPENSSL)
+
 }  // namespace net