Implements new, more robust design for communicating between SSLConnectJobs.

net/socket/socket_test_util.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/socket_test_util.h"
 
 #include <algorithm>
 #include <vector>
 
 #include "base/basictypes.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
+#include "base/callback_helpers.h"
 #include "base/compiler_specific.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
 #include "base/time/time.h"
 #include "net/base/address_family.h"
 #include "net/base/address_list.h"
 #include "net/base/auth.h"
 #include "net/base/load_timing_info.h"
 #include "net/http/http_network_session.h"
 #include "net/http/http_request_headers.h"
@@ skipping 247 matching lines @@
 }
 
 SSLSocketDataProvider::SSLSocketDataProvider(IoMode mode, int result)
     : connect(mode, result),
       next_proto_status(SSLClientSocket::kNextProtoUnsupported),
       was_npn_negotiated(false),
       protocol_negotiated(kProtoUnknown),
       client_cert_sent(false),
       cert_request_info(NULL),
       channel_id_sent(false),
-      connection_status(0) {
+      connection_status(0),
+      should_block_on_connect(false),
+      is_in_session_cache(false) {
   SSLConnectionStatusSetVersion(SSL_CONNECTION_VERSION_TLS1_2,
                                 &connection_status);
   // Set to TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305
   SSLConnectionStatusSetCipherSuite(0xcc14, &connection_status);
 }
 
 SSLSocketDataProvider::~SSLSocketDataProvider() {
 }
 
 void SSLSocketDataProvider::SetNextProto(NextProto proto) {
@@ skipping 401 matching lines @@
       new MockTCPClientSocket(addresses, net_log, data_provider));
   data_provider->set_socket(socket.get());
   return socket.PassAs<StreamSocket>();
 }
 
 scoped_ptr<SSLClientSocket> MockClientSocketFactory::CreateSSLClientSocket(
     scoped_ptr<ClientSocketHandle> transport_socket,
     const HostPortPair& host_and_port,
     const SSLConfig& ssl_config,
     const SSLClientSocketContext& context) {
-  return scoped_ptr<SSLClientSocket>(
+  scoped_ptr<MockSSLClientSocket> socket(
       new MockSSLClientSocket(transport_socket.Pass(),
-                              host_and_port, ssl_config,
+                              host_and_port,
+                              ssl_config,
                               mock_ssl_data_.GetNext()));
+  ssl_client_sockets_.push_back(socket.get());
+  return socket.PassAs<SSLClientSocket>();
 }
 
 void MockClientSocketFactory::ClearSSLSessionCache() {
 }
 
 const char MockClientSocket::kTlsUnique[] = "MOCK_TLSUNIQ";
 
 MockClientSocket::MockClientSocket(const BoundNetLog& net_log)
     : connected_(false),
       net_log_(net_log),
@@ skipping 35 matching lines @@
   bool rv = ParseIPLiteralToNumber("192.0.2.33", &ip);
   CHECK(rv);
   *address = IPEndPoint(ip, 123);
   return OK;
 }
 
 const BoundNetLog& MockClientSocket::NetLog() const {
   return net_log_;
 }
 
+bool MockClientSocket::InSessionCache() const {
+  NOTIMPLEMENTED();
+  return false;
+}
+
+void MockClientSocket::SetHandshakeCompletionCallback(const base::Closure& cb) {
+  NOTIMPLEMENTED();
+}
+
 void MockClientSocket::GetSSLCertRequestInfo(
   SSLCertRequestInfo* cert_request_info) {
 }
 
 int MockClientSocket::ExportKeyingMaterial(const base::StringPiece& label,
                                            bool has_context,
                                            const base::StringPiece& context,
                                            unsigned char* out,
                                            unsigned int outlen) {
   memset(out, 'A', outlen);
@@ skipping 520 matching lines @@
 
 bool DeterministicMockTCPClientSocket::GetSSLInfo(SSLInfo* ssl_info) {
   return false;
 }
 
 void DeterministicMockTCPClientSocket::OnReadComplete(const MockRead& data) {}
 
 void DeterministicMockTCPClientSocket::OnConnectComplete(
     const MockConnect& data) {}
 
-// static
-void MockSSLClientSocket::ConnectCallback(
-    MockSSLClientSocket* ssl_client_socket,
-    const CompletionCallback& callback,
-    int rv) {
-  if (rv == OK)
-    ssl_client_socket->connected_ = true;
-  callback.Run(rv);
-}
-
 MockSSLClientSocket::MockSSLClientSocket(
     scoped_ptr<ClientSocketHandle> transport_socket,
     const HostPortPair& host_port_pair,
     const SSLConfig& ssl_config,
     SSLSocketDataProvider* data)
     : MockClientSocket(
-         // Have to use the right BoundNetLog for LoadTimingInfo regression
-         // tests.
-         transport_socket->socket()->NetLog()),
+          // Have to use the right BoundNetLog for LoadTimingInfo regression
+          // tests.
+          transport_socket->socket()->NetLog()),
       transport_(transport_socket.Pass()),
       data_(data),
       is_npn_state_set_(false),
       new_npn_value_(false),
       is_protocol_negotiated_set_(false),
-      protocol_negotiated_(kProtoUnknown) {
+      protocol_negotiated_(kProtoUnknown),
+      next_connect_state_(STATE_NONE),
+      weak_factory_(this) {
   DCHECK(data_);
   peer_addr_ = data->connect.peer_addr;
 }
 
 MockSSLClientSocket::~MockSSLClientSocket() {
   Disconnect();
 }
 
 int MockSSLClientSocket::Read(IOBuffer* buf, int buf_len,
                               const CompletionCallback& callback) {
   return transport_->socket()->Read(buf, buf_len, callback);
 }
 
 int MockSSLClientSocket::Write(IOBuffer* buf, int buf_len,
                                const CompletionCallback& callback) {
   return transport_->socket()->Write(buf, buf_len, callback);
 }
 
 int MockSSLClientSocket::Connect(const CompletionCallback& callback) {
-  int rv = transport_->socket()->Connect(
-      base::Bind(&ConnectCallback, base::Unretained(this), callback));
-  if (rv == OK) {
-    if (data_->connect.result == OK)
-      connected_ = true;
-    if (data_->connect.mode == ASYNC) {
-      RunCallbackAsync(callback, data_->connect.result);
-      return ERR_IO_PENDING;
-    }
-    return data_->connect.result;
-  }
+  next_connect_state_ = STATE_TRANSPORT_CONNECT;
+  int rv = DoConnectLoop(OK);
+  if (rv == ERR_IO_PENDING)
+    connect_callback_ = callback;
   return rv;
 }
 
 void MockSSLClientSocket::Disconnect() {
+  weak_factory_.InvalidateWeakPtrs();
   MockClientSocket::Disconnect();
   if (transport_->socket() != NULL)
     transport_->socket()->Disconnect();
 }
 
 bool MockSSLClientSocket::IsConnected() const {
-  return transport_->socket()->IsConnected();
+  return transport_->socket()->IsConnected() && connected_;

[Ryan Sleevi, 2014/07/29 23:19:21]

This seems to violate the SSLClientSocket behaviour - where IsConnected()
doesn't return true until the handshake has completed.

See comments below.

 }
 
 bool MockSSLClientSocket::WasEverUsed() const {
   return transport_->socket()->WasEverUsed();
 }
 
 bool MockSSLClientSocket::UsingTCPFastOpen() const {
   return transport_->socket()->UsingTCPFastOpen();
 }
 
 int MockSSLClientSocket::GetPeerAddress(IPEndPoint* address) const {
   return transport_->socket()->GetPeerAddress(address);
 }
 
 bool MockSSLClientSocket::GetSSLInfo(SSLInfo* ssl_info) {
   ssl_info->Reset();
   ssl_info->cert = data_->cert;
   ssl_info->client_cert_sent = data_->client_cert_sent;
   ssl_info->channel_id_sent = data_->channel_id_sent;
   ssl_info->connection_status = data_->connection_status;
   return true;
 }
 
+bool MockSSLClientSocket::InSessionCache() const {
+  return data_->is_in_session_cache;
+}
+
+void MockSSLClientSocket::SetHandshakeCompletionCallback(
+    const base::Closure& cb) {
+  handshake_completion_callback_ = cb;
+}
+
 void MockSSLClientSocket::GetSSLCertRequestInfo(
     SSLCertRequestInfo* cert_request_info) {
   DCHECK(cert_request_info);
   if (data_->cert_request_info) {
     cert_request_info->host_and_port =
         data_->cert_request_info->host_and_port;
     cert_request_info->client_certs = data_->cert_request_info->client_certs;
   } else {
     cert_request_info->Reset();
   }
@@ skipping 42 matching lines @@
 }
 
 void MockSSLClientSocket::OnReadComplete(const MockRead& data) {
   NOTIMPLEMENTED();
 }
 
 void MockSSLClientSocket::OnConnectComplete(const MockConnect& data) {
   NOTIMPLEMENTED();
 }
 
+void MockSSLClientSocket::RestartPausedConnect() {
+  DCHECK(data_->should_block_on_connect);
+  DCHECK_EQ(next_connect_state_, STATE_SSL_CONNECT_COMPLETE);
+  OnIOComplete(data_->connect.result);
+}
+
+void MockSSLClientSocket::OnIOComplete(int result) {
+  int rv = DoConnectLoop(result);
+  if (rv != ERR_IO_PENDING) {
+    DCHECK(!connect_callback_.is_null());

[Ryan Sleevi, 2014/07/29 23:19:22]

You can remove this DCHECK. It's implied by line 1472.

[mshelley, 2014/07/30 18:33:54]

Done.

[wtc, 2014/07/30 21:56:57]

Why is this DCHECK implied by line 1472?

+    base::ResetAndReturn(&connect_callback_).Run(rv);
+  }
+}
+
+int MockSSLClientSocket::DoConnectLoop(int result) {
+  DCHECK_NE(next_connect_state_, STATE_NONE);
+
+  int rv = result;
+  do {
+    ConnectState state = next_connect_state_;
+    next_connect_state_ = STATE_NONE;
+    switch (state) {
+      case STATE_TRANSPORT_CONNECT:
+        DCHECK_EQ(OK, rv);

[Ryan Sleevi, 2014/07/29 23:19:22]

DCHECKs in test code are (generally) not desirable. I think you're fine to
remove these two DCHECKS (also on 1491)

[mshelley, 2014/07/30 18:33:54]

Done.

+        rv = DoTransportConnect();
+        break;
+      case STATE_TRANSPORT_CONNECT_COMPLETE:
+        rv = DoTransportConnectComplete(rv);
+      case STATE_SSL_CONNECT:
+        DCHECK_EQ(OK, rv);
+        rv = DoSSLConnect();
+        break;
+      case STATE_SSL_CONNECT_COMPLETE:
+        rv = DoSSLConnectComplete(rv);
+        break;
+      default:
+        NOTREACHED() << "bad state";
+        rv = ERR_UNEXPECTED;
+        break;
+    }
+  } while (rv != ERR_IO_PENDING && next_connect_state_ != STATE_NONE);
+
+  return rv;
+}
+
+int MockSSLClientSocket::DoTransportConnect() {
+  next_connect_state_ = STATE_TRANSPORT_CONNECT_COMPLETE;
+  return transport_->socket()->Connect(
+      base::Bind(&MockSSLClientSocket::OnIOComplete, base::Unretained(this)));
+}
+
+int MockSSLClientSocket::DoTransportConnectComplete(int result) {
+  if (result == OK)
+    next_connect_state_ = STATE_SSL_CONNECT;
+  return result;
+}
+
+int MockSSLClientSocket::DoSSLConnect() {
+  next_connect_state_ = STATE_SSL_CONNECT_COMPLETE;
+
+  // TODO(mshelley) Move this to DoSSLConnectComplete and add a seperate member
+  // var to check that the connection has started during testing.
+  if (data_->connect.result == OK)
+    connected_ = true;

[Ryan Sleevi, 2014/07/29 23:19:22]

Doesn't this belong after lines 1527-1528?

Your "should_block_on_connect" is meant to simulate "not yet connected", so it
seems odd.

[mshelley, 2014/07/30 18:33:54]

Right now its set up this way so that I can use connected_ to determine that the
socket started to connect in my tests. I intend to add a different member var to
use for that purpose, and then block before setting connected (hence the todo
comment).

+
+  if (data_->should_block_on_connect)
+    return ERR_IO_PENDING;
+
+  if (data_->connect.mode == ASYNC) {
+    base::MessageLoop::current()->PostTask(
+        FROM_HERE,
+        base::Bind(&MockSSLClientSocket::OnIOComplete,
+                   weak_factory_.GetWeakPtr(),
+                   data_->connect.result));
+    return ERR_IO_PENDING;
+  }
+
+  return data_->connect.result;
+}
+
+int MockSSLClientSocket::DoSSLConnectComplete(int result) {
+  if (!handshake_completion_callback_.is_null())
+    base::ResetAndReturn(&handshake_completion_callback_).Run();
+  return result;
+}
+
 MockUDPClientSocket::MockUDPClientSocket(SocketDataProvider* data,
                                          net::NetLog* net_log)
     : connected_(false),
       data_(data),
       read_offset_(0),
       read_data_(SYNCHRONOUS, ERR_UNEXPECTED),
       need_read_data_(true),
       source_port_(123),
       pending_buf_(NULL),
       pending_buf_len_(0),
@@ skipping 444 matching lines @@
 
 const char kSOCKS5OkRequest[] =
     { 0x05, 0x01, 0x00, 0x03, 0x04, 'h', 'o', 's', 't', 0x00, 0x50 };
 const int kSOCKS5OkRequestLength = arraysize(kSOCKS5OkRequest);
 
 const char kSOCKS5OkResponse[] =
     { 0x05, 0x00, 0x00, 0x01, 127, 0, 0, 1, 0x00, 0x50 };
 const int kSOCKS5OkResponseLength = arraysize(kSOCKS5OkResponse);
 
 }  // namespace net