Implements new, more robust design for communicating between SSLConnectJobs.

net/socket/ssl_client_socket_openssl.cc

Index: net/socket/ssl_client_socket_openssl.cc
diff --git a/net/socket/ssl_client_socket_openssl.cc b/net/socket/ssl_client_socket_openssl.cc
index 4ff8d438e965b1fdf0b5b7f8b4ecdb75e1d14d66..7badfa1764e7a894473a4b2fac0850dbe06416e2 100644
--- a/net/socket/ssl_client_socket_openssl.cc
+++ b/net/socket/ssl_client_socket_openssl.cc
@@ -87,15 +87,6 @@ int GetNetSSLVersion(SSL* ssl) {
   }
 }
 
-// Compute a unique key string for the SSL session cache. |socket| is an
-// input socket object. Return a string.
-std::string GetSocketSessionCacheKey(const SSLClientSocketOpenSSL& socket) {
-  std::string result = socket.host_and_port().ToString();
-  result.append("/");
-  result.append(socket.ssl_session_cache_shard());
-  return result;
-}
-
 }  // namespace
 
 class SSLClientSocketOpenSSL::SSLContext {
@@ -139,7 +130,7 @@ class SSLClientSocketOpenSSL::SSLContext {
   static std::string GetSessionCacheKey(const SSL* ssl) {
     SSLClientSocketOpenSSL* socket = GetInstance()->GetClientSocketFromSSL(ssl);
     DCHECK(socket);
-    return GetSocketSessionCacheKey(*socket);
+    return socket->GetSessionCacheKey();
   }
 
   static SSLSessionCacheOpenSSL::Config kDefaultSessionCacheConfig;
@@ -360,12 +351,34 @@ SSLClientSocketOpenSSL::SSLClientSocketOpenSSL(
       npn_status_(kNextProtoUnsupported),
       channel_id_request_return_value_(ERR_UNEXPECTED),
       channel_id_xtn_negotiated_(false),
-      net_log_(transport_->socket()->NetLog()) {}
+      net_log_(transport_->socket()->NetLog()) {
+}
 
 SSLClientSocketOpenSSL::~SSLClientSocketOpenSSL() {
   Disconnect();
 }
 
+bool SSLClientSocketOpenSSL::InSessionCache() const {
+  SSLContext* context = SSLContext::GetInstance();
+  std::string cache_key = GetSessionCacheKey();
+  return context->session_cache()->SSLSessionIsInCache(cache_key);
+}
+
+void SSLClientSocketOpenSSL::SetHandshakeSuccessCallback(
+    const base::Closure& callback) {
+  success_callback_ = callback;
+  SSLContext* context = SSLContext::GetInstance();
+  context->session_cache()->SetSessionAddedCallback(
+      ssl_,
+      base::Bind(&SSLClientSocketOpenSSL::OnHandshakeSuccess,
+                 base::Unretained(this)));
+}
+
+void SSLClientSocketOpenSSL::SetHandshakeFailureCallback(
+    const base::Closure& callback) {
+  error_callback_ = callback;
+}
+
 void SSLClientSocketOpenSSL::GetSSLCertRequestInfo(
     SSLCertRequestInfo* cert_request_info) {
   cert_request_info->host_and_port = host_and_port_;
@@ -436,7 +449,10 @@ int SSLClientSocketOpenSSL::Connect(const CompletionCallback& callback) {
 }
 
 void SSLClientSocketOpenSSL::Disconnect() {
+  OnHandshakeFailure();
   if (ssl_) {
+    SSLContext* context = SSLContext::GetInstance();
+    context->session_cache()->RemoveSessionAddedCallback(ssl_);
     // Calling SSL_shutdown prevents the session from being marked as
     // unresumable.
     SSL_shutdown(ssl_);
@@ -658,7 +674,7 @@ int SSLClientSocketOpenSSL::Init() {
     return ERR_UNEXPECTED;
 
   trying_cached_session_ = context->session_cache()->SetSSLSessionWithKey(
-      ssl_, GetSocketSessionCacheKey(*this));
+      ssl_, GetSessionCacheKey());
 
   BIO* ssl_bio = NULL;
   // 0 => use default buffer sizes.
@@ -775,6 +791,21 @@ void SSLClientSocketOpenSSL::DoWriteCallback(int rv) {
   base::ResetAndReturn(&user_write_callback_).Run(rv);
 }
 
+std::string SSLClientSocketOpenSSL::GetSessionCacheKey() const {
+  return GetSessionCacheKey(host_and_port_, ssl_session_cache_shard_);
+}
+
+void SSLClientSocketOpenSSL::OnHandshakeSuccess() {
+  error_callback_.Reset();
+  base::ResetAndReturn(&success_callback_).Run();
+}
+
+void SSLClientSocketOpenSSL::OnHandshakeFailure() {
+  if (!error_callback_.is_null())
+    success_callback_.Reset();
+  base::ResetAndReturn(&error_callback_).Run();
+}
+
 bool SSLClientSocketOpenSSL::DoTransportIO() {
   bool network_moved = false;
   int rv;
@@ -1014,7 +1045,11 @@ int SSLClientSocketOpenSSL::DoHandshakeLoop(int last_io_result) {
       // the transport IO may allow DoHandshake to make progress.
       rv = OK;  // This causes us to stay in the loop.
     }
+
   } while (rv != ERR_IO_PENDING && next_handshake_state_ != STATE_NONE);
+
+  if (rv < OK && rv != ERR_IO_PENDING)
+    OnHandShakeFailure();

[Ryan Sleevi, 2014/07/18 22:01:17]

Rather than place this here, it should belong with the places that call
DoHandshakeLoop()

We don't call callbacks in the loop, because they're not the "terminal" place
for the code (entry point / exit point)

Also, typo-> HandShake -> Handshake

[mshelley, 2014/07/21 23:00:08]

Done.

   return rv;
 }
 
@@ -1064,8 +1099,13 @@ int SSLClientSocketOpenSSL::DoPayloadRead() {
   do {
     rv = SSL_read(ssl_, user_read_buf_->data() + total_bytes_read,
                   user_read_buf_len_ - total_bytes_read);
-    if (rv > 0)
+    if (rv > 0) {
       total_bytes_read += rv;
+    } else {
+      // Failure of a read attempt may indicate a failed false start
+      // connection.
+      OnHandshakeFailure();

[Ryan Sleevi, 2014/07/18 22:01:17]

We don't want to synchronously make these calls here, because these callbacks
may result in the underlying socket being deleted.

We also want to make sure that all of the dependent variables have been updated,
in case someone attempts to look at the SSLClientSocket*

[mshelley, 2014/07/21 23:00:08]

Done.

+    }
   } while (total_bytes_read < user_read_buf_len_ && rv > 0);
 
   if (total_bytes_read == user_read_buf_len_) {
@@ -1116,13 +1156,15 @@ int SSLClientSocketOpenSSL::DoPayloadRead() {
 int SSLClientSocketOpenSSL::DoPayloadWrite() {
   crypto::OpenSSLErrStackTracer err_tracer(FROM_HERE);
   int rv = SSL_write(ssl_, user_write_buf_->data(), user_write_buf_len_);
-
   if (rv >= 0) {
     net_log_.AddByteTransferEvent(NetLog::TYPE_SSL_SOCKET_BYTES_SENT, rv,
                                   user_write_buf_->data());
     return rv;
   }
 
+  // Failure of a write attempt may indicate a failed false start
+  // connection.
+  OnHandshakeFailure();

[Ryan Sleevi, 2014/07/18 22:01:17]

Ditto

[mshelley, 2014/07/21 23:00:08]

Done.

   int err = SSL_get_error(ssl_, rv);
   return MapOpenSSLError(err, err_tracer);
 }