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 11 matching lines @@
 #include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
 #include "net/base/net_log.h"
 #include "net/base/test_completion_callback.h"
 #include "net/http/http_auth_controller.h"
 #include "net/http/http_proxy_client_socket_pool.h"
 #include "net/socket/client_socket_factory.h"
 #include "net/socket/client_socket_handle.h"
 #include "net/socket/socks_client_socket_pool.h"
 #include "net/socket/ssl_client_socket.h"
+#include "net/socket/ssl_client_socket_openssl.h"

[wtc, 2014/07/08 01:25:41]

IMPORTANT: this header should not need to include
"net/socket/ssl_client_socket_openssl.h".

[mshelley, 2014/07/09 19:51:00]

Done.

 #include "net/socket/ssl_client_socket_pool.h"
 #include "net/socket/transport_client_socket_pool.h"
 #include "net/ssl/ssl_config_service.h"
 #include "net/udp/datagram_client_socket.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace net {
 
 enum {
   // A private network error code used by the socket test utility classes.
@@ skipping 286 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;
+  bool is_in_session_cache_;
+  bool is_leader_;

[wtc, 2014/07/08 01:25:41]

is_leader_ should be documented because its meaning is not obvious.

I can guess what is_in_session_cache_ means. Since none of the other members are
documented, it is fine to not document is_in_session_cache_.

[mshelley, 2014/07/09 19:51:00]

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 279 matching lines @@
 // socket types.
 class MockClientSocketFactory : public ClientSocketFactory {
  public:
   MockClientSocketFactory();
   virtual ~MockClientSocketFactory();
 
   void AddSocketDataProvider(SocketDataProvider* socket);
   void AddSSLSocketDataProvider(SSLSocketDataProvider* socket);
   void ResetNextMockIndexes();
 
+  // Tell the client socket factory that the leading job has connected.

[wtc, 2014/07/08 01:25:41]

Nit: Tell => Tells

[mshelley, 2014/07/09 19:50:59]

Done.

+  static void LeaderConnected();

[wtc, 2014/07/08 01:25:41]

Name this method "SetLeaderConnected".

[mshelley, 2014/07/09 19:51:00]

Done.

+
+  // Returns |leader_connected_|.
+  static bool IsLeaderConnected();

[wtc, 2014/07/08 01:25:41]

IMPORTANT: it seems that the leader connect job is a notion of the client socket
pool rather than the client socket factory. So I don't know why we need to add
these two methods to MockClientSocketFactory.

[mshelley, 2014/07/09 19:51:00]

I agree that it would make more sense to have these methods in the client socket
pool. However, in this case I'm not using a mock client socket pool, so I
couldn't add these methods there. The client socket factory seemed like the next
most logical place to keep track of this information. 

+
   SocketDataProviderArray<SocketDataProvider>& mock_data() {
     return mock_data_;
   }
 
   // ClientSocketFactory
   virtual scoped_ptr<DatagramClientSocket> CreateDatagramClientSocket(
       DatagramSocket::BindType bind_type,
       const RandIntCallback& rand_int_cb,
       NetLog* net_log,
       const NetLog::Source& source) OVERRIDE;
   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();
 
  private:
   SocketDataProviderArray<SocketDataProvider> mock_data_;
   SocketDataProviderArray<SSLSocketDataProvider> mock_ssl_data_;
+  static NET_EXPORT bool leader_connected_;

[wtc, 2014/07/08 01:25:41]

Use NET_EXPORT_PRIVATE instead. But, is this really necessary?

[mshelley, 2014/07/09 19:50:59]

Done.

+  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);
@@ skipping 13 matching lines @@
   virtual void Disconnect() OVERRIDE;
   virtual bool IsConnected() const OVERRIDE;
   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 WatchSessionForCompletion(
+      const base::Closure& cb) const OVERRIDE;
+  virtual void SetSocketFailureCallback(const base::Closure& cb) OVERRIDE;
+  virtual void OnSocketFailure() 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;
@@ skipping 239 matching lines @@
   virtual int Connect(const CompletionCallback& callback) OVERRIDE;
   virtual void Disconnect() OVERRIDE;
   virtual bool IsConnected() 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 WatchSessionForCompletion(
+      const base::Closure& cb) const OVERRIDE;
+  virtual void SetSocketFailureCallback(const base::Closure& cb) OVERRIDE;
+  virtual void OnSocketFailure() 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;
 
+  bool IsGoodOrdering() const;

[wtc, 2014/07/08 01:25:41]

Document this method.

[mshelley, 2014/07/09 19:51:00]

Done.

+
  private:
   static void ConnectCallback(MockSSLClientSocket* ssl_client_socket,
                               const CompletionCallback& callback,
                               int rv);
 
+  mutable base::Closure process_pending_jobs_callback_;

[wtc, 2014/07/08 01:25:41]

Why does this member need to be marked mutable? Can this be avoided?

[mshelley, 2014/07/09 19:51:00]

This member was marked as mutable because I need to set its value inside of
WatchSessionForCompletion, which is a const method. Normally, the callback
passed into this method would be passed into the cache, and stored there to be
called upon the connection's completion. However, because this test doesn't
actually utilize the cache, I needed to set the member here. 

Instead of making this mutable I'll just change the base class so that the
method is not const.  

+
   scoped_ptr<ClientSocketHandle> transport_;
   SSLSocketDataProvider* data_;
   bool is_npn_state_set_;
   bool new_npn_value_;
   bool is_protocol_negotiated_set_;
   NextProto protocol_negotiated_;
+  bool good_ordering_;
 
   DISALLOW_COPY_AND_ASSIGN(MockSSLClientSocket);
 };
 
 class MockUDPClientSocket : public DatagramClientSocket, public AsyncSocket {
  public:
   MockUDPClientSocket(SocketDataProvider* data, net::NetLog* net_log);
   virtual ~MockUDPClientSocket();
 
   // Socket implementation.
@@ skipping 295 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_