Implements new, more robust design for communicating between SSLConnectJobs.

net/socket/socket_test_util.h

 // 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.
 
 #ifndef NET_SOCKET_SOCKET_TEST_UTIL_H_
 #define NET_SOCKET_SOCKET_TEST_UTIL_H_
 
 #include <cstring>
 #include <deque>
 #include <string>
@@ skipping 317 matching lines @@
   std::string next_proto;
   std::string server_protos;
   bool was_npn_negotiated;
   NextProto protocol_negotiated;
   bool client_cert_sent;
   SSLCertRequestInfo* cert_request_info;
   scoped_refptr<X509Certificate> cert;
   bool channel_id_sent;
   ServerBoundCertService* server_bound_cert_service;
   int connection_status;
+  // Indicates that the socket should block in the Connect method.
+  bool block_in_connect_;
+  // Indicates that the socket previously blocked in the Connect method.
+  bool blocked_in_connect_;

[wtc, 2014/07/18 01:17:14]

Nit: these two members look similar. I suggest naming the first one
"should_block_in_connect_".

[mshelley, 2014/07/18 21:08:31]

Done.

+  bool is_in_session_cache_;

[wtc, 2014/07/18 15:39:17]

Since SSLSocketDataProvider is a struct, the three new data members should
follow the naming convention of struct members, without a trailing underscore.
See the Style Guide:

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Variable_Names

[mshelley, 2014/07/18 21:08:31]

Done.

 };
 
 // A DataProvider where the client must write a request before the reads (e.g.
 // the response) will complete.
 class DelayedSocketData : public StaticSocketDataProvider {
  public:
   // |write_delay| the number of MockWrites to complete before allowing
   //               a MockRead to complete.
   // |reads| the list of MockRead completions.
   // |writes| the list of MockWrite completions.
@@ skipping 299 matching lines @@
   virtual scoped_ptr<StreamSocket> CreateTransportClientSocket(
       const AddressList& addresses,
       NetLog* net_log,
       const NetLog::Source& source) OVERRIDE;
   virtual scoped_ptr<SSLClientSocket> CreateSSLClientSocket(
       scoped_ptr<ClientSocketHandle> transport_socket,
       const HostPortPair& host_and_port,
       const SSLConfig& ssl_config,
       const SSLClientSocketContext& context) OVERRIDE;
   virtual void ClearSSLSessionCache() OVERRIDE;
+  virtual std::vector<MockSSLClientSocket*> GetSSLClientSockets();

[wtc, 2014/07/18 15:39:17]

1. Please declare this method before the "ClientSocketFactory" comment on line
646 because this method is not part of the ClientSocketFactory interface.

2. This getter method should be named ssl_client_sockets(). See the Style Guide:

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Function_Names

3. This getter method should be const and defined inline:

  virtual std::vector<MockSSLClientSocket*> ssl_client_sockets() const {
    return ssl_client_sockets_;
  }

It may be OK for it to return a const reference.

[mshelley, 2014/07/18 21:08:31]

Done.

 
  private:
   SocketDataProviderArray<SocketDataProvider> mock_data_;
   SocketDataProviderArray<SSLSocketDataProvider> mock_ssl_data_;
+  std::vector<MockSSLClientSocket*> ssl_client_sockets_;
 };
 
 class MockClientSocket : public SSLClientSocket {
  public:
   // Value returned by GetTLSUniqueChannelBinding().
   static const char kTlsUnique[];
 
   // The BoundNetLog is needed to test LoadTimingInfo, which uses NetLog IDs as
   // unique socket IDs.
   explicit MockClientSocket(const BoundNetLog& net_log);
 
   // Socket implementation.
   virtual int Read(IOBuffer* buf,
                    int buf_len,
                    const CompletionCallback& callback) = 0;
   virtual int Write(IOBuffer* buf,
                     int buf_len,
                     const CompletionCallback& callback) = 0;
   virtual int SetReceiveBufferSize(int32 size) OVERRIDE;
   virtual int SetSendBufferSize(int32 size) OVERRIDE;
 
   // StreamSocket implementation.
   virtual int Connect(const CompletionCallback& callback) = 0;
   virtual void Disconnect() OVERRIDE;
   virtual bool IsConnected() const OVERRIDE;
+  virtual bool IsConnectedSSL() const OVERRIDE;

[wtc, 2014/07/18 01:17:14]

IMPORTANT: IsConnectedSSL is not a method of the StreamSocket interface. So this
method should not be listed in this section (see the comment on line 689).

Also, this method may not need to be a virtual method.

[wtc, 2014/07/18 15:39:17]

MockSSLClientSocket has the same problem on line 964.

[mshelley, 2014/07/18 21:08:31]

Done.

   virtual bool IsConnectedAndIdle() const OVERRIDE;
   virtual int GetPeerAddress(IPEndPoint* address) const OVERRIDE;
   virtual int GetLocalAddress(IPEndPoint* address) const OVERRIDE;
   virtual const BoundNetLog& NetLog() const OVERRIDE;
   virtual void SetSubresourceSpeculation() OVERRIDE {}
   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 GetSSLCertRequestInfo(SSLCertRequestInfo* cert_request_info)
       OVERRIDE;
   virtual int ExportKeyingMaterial(const base::StringPiece& label,
                                    bool has_context,
                                    const base::StringPiece& context,
                                    unsigned char* out,
                                    unsigned int outlen) OVERRIDE;
   virtual int GetTLSUniqueChannelBinding(std::string* out) OVERRIDE;
   virtual NextProtoStatus GetNextProto(std::string* proto,
                                        std::string* server_protos) OVERRIDE;
   virtual ServerBoundCertService* GetServerBoundCertService() const OVERRIDE;
 
+  // Resumes the connection of a socket that was paused for testing.

[wtc, 2014/07/18 01:17:14]

This should say "Returns a callback that resumes the connection ..."

[mshelley, 2014/07/18 21:08:31]

Done.

+  // |resumption_callback_| should be set before invoking this method.
+  CompletionCallback GetResumptionCallback();

[wtc, 2014/07/18 15:39:17]

This is a getter method, so it should be named and declared as follows:

  CompletionCallback resumption_callback() const {
    return resumption_callback_;
  }

[mshelley, 2014/07/18 21:08:31]

Done.

+
  protected:
   virtual ~MockClientSocket();
   void RunCallbackAsync(const CompletionCallback& callback, int result);
   void RunCallback(const CompletionCallback& callback, int result);
 
+  // Callback to be used to resume the connection of a paused socket.
+  CompletionCallback resumption_callback_;

[wtc, 2014/07/18 15:39:16]

1. Declare this data member after line 731 because data members should be
declared after methods. See the Style Guide:

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Declaration_Order

2. Since pausing the connection is a notion of SSLConnectJob rather than
SSLClientSocket, it is not ideal to add this member and its getter method to
MockClientSocket.

Update: I found this member is the completion callback for Connect. It is not
the same as the resumption callback of SSLConnectJob. So I think this member
should be renamed "connect_callback_".

[mshelley, 2014/07/18 21:08:31]

Done.

+
   // SSLClientSocket implementation.
   virtual scoped_refptr<X509Certificate> GetUnverifiedServerCertificateChain()
       const OVERRIDE;
 
   // True if Connect completed successfully and Disconnect hasn't been called.
   bool connected_;
 
   // Address of the "remote" peer we're connected to.
   IPEndPoint peer_addr_;
 
@@ skipping 215 matching lines @@
                    int buf_len,
                    const CompletionCallback& callback) OVERRIDE;
   virtual int Write(IOBuffer* buf,
                     int buf_len,
                     const CompletionCallback& callback) OVERRIDE;
 
   // StreamSocket implementation.
   virtual int Connect(const CompletionCallback& callback) OVERRIDE;
   virtual void Disconnect() OVERRIDE;
   virtual bool IsConnected() const OVERRIDE;
+  virtual bool IsConnectedSSL() const OVERRIDE;
   virtual bool WasEverUsed() const OVERRIDE;
   virtual bool UsingTCPFastOpen() const OVERRIDE;
   virtual int GetPeerAddress(IPEndPoint* address) const OVERRIDE;
   virtual bool WasNpnNegotiated() const OVERRIDE;
   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 GetSSLCertRequestInfo(SSLCertRequestInfo* cert_request_info)
       OVERRIDE;
   virtual NextProtoStatus GetNextProto(std::string* proto,
                                        std::string* server_protos) OVERRIDE;
   virtual bool set_was_npn_negotiated(bool negotiated) OVERRIDE;
   virtual void set_protocol_negotiated(NextProto protocol_negotiated) OVERRIDE;
   virtual NextProto GetNegotiatedProtocol() const OVERRIDE;
 
   // This MockSocket does not implement the manual async IO feature.
   virtual void OnReadComplete(const MockRead& data) OVERRIDE;
   virtual void OnConnectComplete(const MockConnect& data) OVERRIDE;
 
   virtual bool WasChannelIDSent() const OVERRIDE;
   virtual void set_channel_id_sent(bool channel_id_sent) OVERRIDE;
   virtual ServerBoundCertService* GetServerBoundCertService() const OVERRIDE;
 
  private:
   static void ConnectCallback(MockSSLClientSocket* ssl_client_socket,
                               const CompletionCallback& callback,
                               int rv);
 
   scoped_ptr<ClientSocketHandle> transport_;
   SSLSocketDataProvider* data_;
   bool is_npn_state_set_;
   bool new_npn_value_;
   bool is_protocol_negotiated_set_;
   NextProto protocol_negotiated_;
 
+  base::Closure success_callback_;
+  base::Closure error_callback_;
+
   DISALLOW_COPY_AND_ASSIGN(MockSSLClientSocket);
 };
 
 class MockUDPClientSocket : public DatagramClientSocket, public AsyncSocket {
  public:
   MockUDPClientSocket(SocketDataProvider* data, net::NetLog* net_log);
   virtual ~MockUDPClientSocket();
 
   // Socket implementation.
   virtual int Read(IOBuffer* buf,
@@ skipping 294 matching lines @@
 
 extern const char kSOCKS5OkRequest[];
 extern const int kSOCKS5OkRequestLength;
 
 extern const char kSOCKS5OkResponse[];
 extern const int kSOCKS5OkResponseLength;
 
 }  // namespace net
 
 #endif  // NET_SOCKET_SOCKET_TEST_UTIL_H_