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..b6f6aec10afd1c473ceffc552b9a05f5b7b563a6 100644
--- a/net/socket/ssl_client_socket_openssl.cc
+++ b/net/socket/ssl_client_socket_openssl.cc
@@ -87,14 +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
 
@@ -139,7 +131,7 @@ class SSLClientSocketOpenSSL::SSLContext {
   static std::string GetSessionCacheKey(const SSL* ssl) {
     SSLClientSocketOpenSSL* socket = GetInstance()->GetClientSocketFromSSL(ssl);
     DCHECK(socket);
-    return GetSocketSessionCacheKey(*socket);
+    return socket->GetSocketSessionCacheKey();
   }
 
   static SSLSessionCacheOpenSSL::Config kDefaultSessionCacheConfig;
@@ -340,6 +332,8 @@ SSLClientSocketOpenSSL::SSLClientSocketOpenSSL(
     : transport_send_busy_(false),
       transport_recv_busy_(false),
       transport_recv_eof_(false),
+      has_read_(false),
+      has_written_(false),
       weak_factory_(this),
       pending_read_error_(kNoPendingReadResult),
       transport_write_error_(OK),
@@ -349,6 +343,7 @@ SSLClientSocketOpenSSL::SSLClientSocketOpenSSL(
       client_auth_cert_needed_(false),
       cert_verifier_(context.cert_verifier),
       server_bound_cert_service_(context.server_bound_cert_service),
+      is_leader_(false),
       ssl_(NULL),
       transport_bio_(NULL),
       transport_(transport_socket.Pass()),
@@ -366,6 +361,41 @@ SSLClientSocketOpenSSL::~SSLClientSocketOpenSSL() {
   Disconnect();
 }
 
+// Compute a unique key string for the SSL session cache.
+// Return a string.
+std::string SSLClientSocketOpenSSL::GetSocketSessionCacheKey() const {
+  std::string result = host_and_port_.ToString();
+  result.append("/");
+  result.append(ssl_session_cache_shard_);
+  return result;
+}
+
+bool SSLClientSocketOpenSSL::InSessionCache() const {
+  SSLContext* context = SSLContext::GetInstance();
+  std::string cache_key = GetSocketSessionCacheKey();
+  return context->session_cache()->SSLSessionIsInCache(cache_key);
+}
+
+void SSLClientSocketOpenSSL::WatchSessionForCompletion(
+    const base::Closure& callback) const {
+  SSLContext* context = SSLContext::GetInstance();
+  context->session_cache()->NotifyOnSessionAdded(ssl_, callback);

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

IMPORTANT: in the destructor, we should remove the entry for ssl_ from
context->session_cache().

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

Done.

+}
+
+void SSLClientSocketOpenSSL::SetSocketFailureCallback(
+    const base::Closure& callback) {
+  error_callback_ = callback;
+}
+
+void SSLClientSocketOpenSSL::SetIsLeader() {
+  is_leader_ = true;
+}
+
+void SSLClientSocketOpenSSL::OnSocketFailure() {
+  if (is_leader_)
+    error_callback_.Run();

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

Should we reset error_callback_?

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

Done.

+}
+
 void SSLClientSocketOpenSSL::GetSSLCertRequestInfo(
     SSLCertRequestInfo* cert_request_info) {
   cert_request_info->host_and_port = host_and_port_;
@@ -658,7 +688,7 @@ int SSLClientSocketOpenSSL::Init() {
     return ERR_UNEXPECTED;
 
   trying_cached_session_ = context->session_cache()->SetSSLSessionWithKey(
-      ssl_, GetSocketSessionCacheKey(*this));
+      ssl_, GetSocketSessionCacheKey());
 
   BIO* ssl_bio = NULL;
   // 0 => use default buffer sizes.
@@ -1018,8 +1048,7 @@ int SSLClientSocketOpenSSL::DoHandshakeLoop(int last_io_result) {
   return rv;
 }
 
-int SSLClientSocketOpenSSL::DoReadLoop(int result) {
-  if (result < 0)
+int SSLClientSocketOpenSSL::DoReadLoop(int result) {if (result < 0)

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

Move "if (result < 0)" to the next line.

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

Done.

     return result;
 
   bool network_moved;
@@ -1064,6 +1093,9 @@ int SSLClientSocketOpenSSL::DoPayloadRead() {
   do {
     rv = SSL_read(ssl_, user_read_buf_->data() + total_bytes_read,
                   user_read_buf_len_ - total_bytes_read);
+    if (!has_read_ && rv != OK && SSLClientSocket::GetEnableJobWaiting())
+      OnSocketFailure();

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

Why do we need to call OnSocketFailure after the failure of the very first
SSL_read or SSL_write call? It seems that we should call OnSocketFailure after
the failure of SSL_do_handshake instead.

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

Done.

+    has_read_ = true;
     if (rv > 0)
       total_bytes_read += rv;
   } while (total_bytes_read < user_read_buf_len_ && rv > 0);
@@ -1116,7 +1148,9 @@ 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 (!has_written_ && rv != OK && SSLClientSocket::GetEnableJobWaiting())
+    OnSocketFailure();
+  has_written_ = true;
   if (rv >= 0) {
     net_log_.AddByteTransferEvent(NetLog::TYPE_SSL_SOCKET_BYTES_SENT, rv,
                                   user_write_buf_->data());