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..e9edb586341f27fb521852d1c066a4b13bb5ed3e 100644
--- a/net/socket/socket_test_util.h
+++ b/net/socket/socket_test_util.h
@@ -335,6 +335,10 @@ struct SSLSocketDataProvider {
   bool channel_id_sent;
   ServerBoundCertService* server_bound_cert_service;
   int connection_status;
+
+  // Denotes that this job may have pending jobs dependant upon its connection.
+  bool is_leader_;
+  bool is_in_session_cache_;

[Ryan Sleevi, 2014/07/12 00:12:15]

Please update the SSLSocketDataProvider ctor to set values for these members,
and to suitable defaults. It's not clear whether it should be false or true in
order to match existing tests, which also sort of highlights the challenge here
(this bookkeeping likely belongs in the Socket itself)

Your previous comment indicated you were removing |is_in_session_cache_|, but
that no longer seems correct - i.e. whether or not a session is in the cache IS
a behaviour you want to control/mock before.

However, is_leader_ seems to be testing a layer that the socket is not
responsible for, but the pool/messenger are.

[mshelley, 2014/07/17 00:28:44]

Sorry -- my previous response was incorrect. There was a time when I made
is_in_session_cache a member of the socket -- I thought I had forgotten to
remove it but that wasn't the case. 

I moved it into the SSLSocketDataProvider because that made it easy for me to
set and retrieve it's value. To avoid issues with other tests I could easily
make it a member variable again and add setter/getter methods. In theory, no
other tests should be calling InSessionCache and accessing this variable though.

Yes, is_leader_ is used to test the messenger; namely the order in which the
messenger allows sockets to connect. 

 };
 
 // A DataProvider where the client must write a request before the reads (e.g.
@@ -634,6 +638,12 @@ class MockClientSocketFactory : public ClientSocketFactory {
   void AddSSLSocketDataProvider(SSLSocketDataProvider* socket);
   void ResetNextMockIndexes();
 
+  // Tells the client socket factory that the leading job has connected.
+  static void SetLeaderConnected();
+
+  // Returns |leader_connected_|.
+  static bool IsLeaderConnected();
+
   SocketDataProviderArray<SocketDataProvider>& mock_data() {
     return mock_data_;
   }
@@ -654,10 +664,13 @@ class MockClientSocketFactory : public ClientSocketFactory {
       const SSLConfig& ssl_config,
       const SSLClientSocketContext& context) OVERRIDE;
   virtual void ClearSSLSessionCache() OVERRIDE;
+  virtual std::vector<MockSSLClientSocket*> GetSSLClientSockets();
 
  private:
   SocketDataProviderArray<SocketDataProvider> mock_data_;
   SocketDataProviderArray<SSLSocketDataProvider> mock_ssl_data_;
+  static bool leader_connected_;
+  std::vector<MockSSLClientSocket*> ssl_client_sockets_;
 };
 
 class MockClientSocket : public SSLClientSocket {
@@ -691,6 +704,11 @@ class MockClientSocket : public SSLClientSocket {
   virtual void SetOmniboxSpeculation() OVERRIDE {}
 
   // SSLClientSocket implementation.
+  virtual bool InSessionCache() const OVERRIDE;
+  virtual void SetHandshakeSuccessCallback(const base::Closure& cb) OVERRIDE;
+  virtual void SetHandshakeFailureCallback(const base::Closure& cb) OVERRIDE;
+  virtual void OnSocketFailure() OVERRIDE;
+  virtual void SetIsLeader() OVERRIDE;

[wtc, 2014/07/15 19:27:58]

Delete these two methods.

[mshelley, 2014/07/17 00:28:44]

Done.

   virtual void GetSSLCertRequestInfo(SSLCertRequestInfo* cert_request_info)
       OVERRIDE;
   virtual int ExportKeyingMaterial(const base::StringPiece& label,
@@ -950,6 +968,10 @@ class MockSSLClientSocket : public MockClientSocket, public AsyncSocket {
   virtual bool GetSSLInfo(SSLInfo* ssl_info) OVERRIDE;
 
   // SSLClientSocket implementation.
+  virtual bool InSessionCache() const OVERRIDE;
+  virtual void SetHandshakeSuccessCallback(const base::Closure& cb) OVERRIDE;
+  virtual void SetHandshakeFailureCallback(const base::Closure& cb) OVERRIDE;
+  virtual void OnHandshakeFailure() OVERRIDE;

[wtc, 2014/07/15 19:27:58]

Delete this method.

[mshelley, 2014/07/17 00:28:44]

Done.

   virtual void GetSSLCertRequestInfo(SSLCertRequestInfo* cert_request_info)
       OVERRIDE;
   virtual NextProtoStatus GetNextProto(std::string* proto,
@@ -966,6 +988,15 @@ class MockSSLClientSocket : public MockClientSocket, public AsyncSocket {
   virtual void set_channel_id_sent(bool channel_id_sent) OVERRIDE;
   virtual ServerBoundCertService* GetServerBoundCertService() const OVERRIDE;
 
+  // Returns a bool indicating whether or not the socket connected at
+  // the correct time relative to other sockets with the same host/port pair.
+  //
+  // If |is_leader_| is true, the socket should connect before all other
+  // sockets.
+  // If |is_leader_| is false, the socket should not connect until the leading
+  // socket has connected.
+  bool IsGoodOrdering() const;
+
  private:
   static void ConnectCallback(MockSSLClientSocket* ssl_client_socket,
                               const CompletionCallback& callback,
@@ -977,6 +1008,9 @@ class MockSSLClientSocket : public MockClientSocket, public AsyncSocket {
   bool new_npn_value_;
   bool is_protocol_negotiated_set_;
   NextProto protocol_negotiated_;
+  bool good_ordering_;
+  base::Closure success_callback_;
+  base::Closure error_callback_;
 
   DISALLOW_COPY_AND_ASSIGN(MockSSLClientSocket);
 };