Implements new, more robust design for communicating between SSLConnectJobs.

net/socket/socket_test_util.h

Index: net/socket/socket_test_util.h
diff --git a/net/socket/socket_test_util.h b/net/socket/socket_test_util.h
index 2918aad2dc55ec4f3b8ff5a63c0e8b584aa1af5c..c49ffc7ae7d7a6719f4931380fe7290bcad5b329 100644
--- a/net/socket/socket_test_util.h
+++ b/net/socket/socket_test_util.h
@@ -335,6 +335,12 @@ struct SSLSocketDataProvider {
   bool channel_id_sent;
   ServerBoundCertService* server_bound_cert_service;
   int connection_status;
+  // Indicates that the socket should block in the Connect method.
+  bool should_block_on_connect;
+  // Whether or not the Socket should behave like there is a pre-existing
+  // session to resume. Whether or not such a session is reported as
+  // resumed is controlled by |connection_status|.
+  bool is_in_session_cache;
 };
 
 // A DataProvider where the client must write a request before the reads (e.g.
@@ -638,6 +644,12 @@ class MockClientSocketFactory : public ClientSocketFactory {
     return mock_data_;
   }
 
+  // Note: this method is unsafe; the elements of the returned vector
+  // are not necessarily valid.
+  const std::vector<MockSSLClientSocket*>& ssl_client_sockets() const {
+    return ssl_client_sockets_;
+  }
+
   // ClientSocketFactory
   virtual scoped_ptr<DatagramClientSocket> CreateDatagramClientSocket(
       DatagramSocket::BindType bind_type,
@@ -658,6 +670,7 @@ class MockClientSocketFactory : public ClientSocketFactory {
  private:
   SocketDataProviderArray<SocketDataProvider> mock_data_;
   SocketDataProviderArray<SSLSocketDataProvider> mock_ssl_data_;
+  std::vector<MockSSLClientSocket*> ssl_client_sockets_;
 };
 
 class MockClientSocket : public SSLClientSocket {
@@ -691,6 +704,8 @@ class MockClientSocket : public SSLClientSocket {
   virtual void SetOmniboxSpeculation() OVERRIDE {}
 
   // SSLClientSocket implementation.
+  virtual bool InSessionCache() const OVERRIDE;
+  virtual void SetHandshakeCompletionCallback(const base::Closure& cb) OVERRIDE;
   virtual void GetSSLCertRequestInfo(SSLCertRequestInfo* cert_request_info)
       OVERRIDE;
   virtual int ExportKeyingMaterial(const base::StringPiece& label,
@@ -720,6 +735,7 @@ class MockClientSocket : public SSLClientSocket {
 
   BoundNetLog net_log_;
 
+ private:
   base::WeakPtrFactory<MockClientSocket> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(MockClientSocket);
@@ -950,6 +966,8 @@ class MockSSLClientSocket : public MockClientSocket, public AsyncSocket {
   virtual bool GetSSLInfo(SSLInfo* ssl_info) OVERRIDE;
 
   // SSLClientSocket implementation.
+  virtual bool InSessionCache() const OVERRIDE;
+  virtual void SetHandshakeCompletionCallback(const base::Closure& cb) OVERRIDE;
   virtual void GetSSLCertRequestInfo(SSLCertRequestInfo* cert_request_info)
       OVERRIDE;
   virtual NextProtoStatus GetNextProto(std::string* proto,
@@ -966,10 +984,31 @@ class MockSSLClientSocket : public MockClientSocket, public AsyncSocket {
   virtual void set_channel_id_sent(bool channel_id_sent) OVERRIDE;
   virtual ServerBoundCertService* GetServerBoundCertService() const OVERRIDE;
 
+  // Resumes the connection of a socket that was paused for testing.
+  // |connect_callback_| should be set before invoking this method.
+  void RestartPausedConnect();
+
  private:
-  static void ConnectCallback(MockSSLClientSocket* ssl_client_socket,
-                              const CompletionCallback& callback,
-                              int rv);
+  enum ConnectState {
+    STATE_NONE,
+    STATE_TCP_CONNECT,
+    STATE_TCP_CONNECT_COMPLETE,

[wtc, 2014/07/29 20:59:34]

Nit: I suggest renaming these two states
    STATE_TRANSPORT_CONNECT,
    STATE_TRANSPORT_CONNECT_COMPLETE,
and renaming the corresponding functions
    int DoTransportConnect();
    int DoTransportComplete(int result);
because the |transport_| socket doesn't need to be TCP.

[mshelley, 2014/07/29 21:40:13]

Done.

+    STATE_SSL_CONNECT,
+    STATE_SSL_CONNECT_COMPLETE,
+  };
+
+  void OnIOComplete(int result);
+
+  // Runs the state transistion loop.
+  int DoConnectLoop(int result);
+
+  int DoTCPConnect();
+  int DoTCPConnectComplete(int result);
+  int DoSSLConnect();
+  int DoSSLConnectComplete(int result);

[wtc, 2014/07/29 20:59:34]

Please make sure these six private methods are defined in the .cc file in this
order. They should all be defined after the GetServerBoundCertService() and
RestartPausedConnect() methods.

Note that the MockSSLClientSocket methods in the .cc file don't match the
declaration order in the .h file exactly. So just try your best to match the
order.

[mshelley, 2014/07/29 21:40:12]

Done.

+
+  // Callback to be used to resume the connection of a paused socket.

[wtc, 2014/07/29 20:59:34]

This comment is wrong. Please delete or fix this comment.

[mshelley, 2014/07/29 21:40:13]

Done.

+  CompletionCallback connect_callback_;

[wtc, 2014/07/29 20:59:34]

Nit: move this member down to be close to the related next_connect_state_
member.

[mshelley, 2014/07/29 21:40:13]

Done.

 
   scoped_ptr<ClientSocketHandle> transport_;
   SSLSocketDataProvider* data_;
@@ -978,6 +1017,13 @@ class MockSSLClientSocket : public MockClientSocket, public AsyncSocket {
   bool is_protocol_negotiated_set_;
   NextProto protocol_negotiated_;
 
+  // Indicates what state of Connect the socket should enter.
+  ConnectState next_connect_state_;
+
+  base::Closure handshake_completion_callback_;

[wtc, 2014/07/29 20:59:34]

Nit: if you think this member name is too long, we can shorten it to
handshake_callback_.

+
+  base::WeakPtrFactory<MockSSLClientSocket> weak_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(MockSSLClientSocket);
 };