Implements new, more robust design for communicating between SSLConnectJobs.

net/socket/ssl_client_socket.h

Index: net/socket/ssl_client_socket.h
diff --git a/net/socket/ssl_client_socket.h b/net/socket/ssl_client_socket.h
index a43e58cc26b852fb86d92a1a5ac7c4e7c357b286..e8e35385c8a552faf848dc9282259a9fd217aa10 100644
--- a/net/socket/ssl_client_socket.h
+++ b/net/socket/ssl_client_socket.h
@@ -81,6 +81,32 @@ class NET_EXPORT SSLClientSocket : public SSLSocket {
   virtual bool WasNpnNegotiated() const OVERRIDE;
   virtual NextProto GetNegotiatedProtocol() const OVERRIDE;
 
+  // Format a unique key for the SSL session cache. This method
+  // is necessary so that all classes create cache keys in a consistent
+  // manner.
+  static std::string FormatSessionCacheKey(std::string host_and_port,
+                                           std::string ssl_session_cache_shard);

[Ryan Sleevi, 2014/07/10 19:51:05]

From a design perspective, I think this will cause us problems down the road.

That there is a SessionCacheKey is merely an implementation detail that we're
not passing along an SSLSessionCache object itself through. Exposing this as a
static method bakes into the design the assumption the session cache is global,
when it's not/shouldn't be.

It might help if you list what places/purposes this is used for, and we can see
how to work this through. It may be an acceptable solution, but I do want to
make sure we've investigated alternatives -and know why they'd be less than
desirable compared to this.

[mshelley, 2014/07/10 22:07:15]

I implemented this in response to Wan-Teh's comment here:
https://codereview.chromium.org/353713005/diff/160001/net/socket/ssl_client_s...


Basically I needed to create a session cache key outside of the SSlCLientSocket
so that I could index the messenger_map_ by cache key. I figured that creating
this static function would standardize cache key creation between the connect
job factory and the client socket. 

However, I just realized that after my changes in
https://codereview.chromium.org/384873002/ I will no longer need to do this,
because I don't add messengers to messenger_map_ until after the socket is
created. 

Would it be acceptable to just leave this function here for now, but remove it
in #384873002 where I no longer have any use for it?

+
+  // Returns true if there is a cache entry in the ssl session cache
+  // for the given cache key.
+  //
+  // The cache key consists of a host and port concatenated with a session
+  // cache shard.
+  virtual bool InSessionCache() const = 0;
+
+  // Tells the session cache to monitor this socket's session and inform
+  // its SSLConnectJobMessenger upon the session's connection's completion.
+  virtual void WatchSessionForCompletion(const base::Closure& callback) = 0;
+
+  // Sets the callback to be used if the socket's connection should fail.
+  virtual void SetSocketFailureCallback(const base::Closure& callback) = 0;
+
+  // Marks this socket as the leading connection.
+  virtual void SetIsLeader() = 0;
+
+  // In the case of a failed connection, run the appropriate callback.
+  virtual void OnSocketFailure() = 0;
+
   // Gets the SSL CertificateRequest info of the socket after Connect failed
   // with ERR_SSL_CLIENT_AUTH_CERT_NEEDED.
   virtual void GetSSLCertRequestInfo(
@@ -133,6 +159,11 @@ class NET_EXPORT SSLClientSocket : public SSLSocket {
   // Public for ssl_client_socket_openssl_unittest.cc.
   virtual bool WasChannelIDSent() const;
 
+  // Enable SSLConnectJob waiting if |enable| is true.
+  static NET_EXPORT void EnableConnectJobWaiting(bool enable);
+
+  static NET_EXPORT bool GetEnableConnectJobWaiting();

[Ryan Sleevi, 2014/07/10 19:51:05]

I realize this is a comment late-to-the-game, but does this make more sense to
set on the Pool (as a static), rather than the Socket? After all, the pool is
responsible for coordinating and the concept of socket waiting.

Recall that we use SSLClientSockets outside of pooled scenarios - notably,
extensions and Pepper - and we won't have the pool coordinating between sockets
here.

[mshelley, 2014/07/10 22:07:15]

Done.

+
  protected:
   virtual void set_channel_id_sent(bool channel_id_sent);
 
@@ -185,6 +216,8 @@ class NET_EXPORT SSLClientSocket : public SSLSocket {
   bool signed_cert_timestamps_received_;
   // True if a stapled OCSP response was received.
   bool stapled_ocsp_response_received_;
+
+  static bool enable_connect_job_waiting_;
 };
 
 }  // namespace net