This CL corrects a bug in which the OnHandshakeComplete callback for an ssl session was never called

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 a6f5caf188d96dac7c1a339e732990cb05ebd6f3..d6d74b19fb5af713c1b1d092bef2d39d96145b54 100644
--- a/net/socket/ssl_client_socket_openssl.cc
+++ b/net/socket/ssl_client_socket_openssl.cc
@@ -363,6 +363,7 @@ SSLClientSocketOpenSSL::SSLClientSocketOpenSSL(
       next_handshake_state_(STATE_NONE),
       npn_status_(kNextProtoUnsupported),
       channel_id_xtn_negotiated_(false),
+      session_completion_count_(0),
       net_log_(transport_->socket()->NetLog()) {
 }
 
@@ -435,14 +436,6 @@ int SSLClientSocketOpenSSL::Connect(const CompletionCallback& callback) {
     return rv;
   }
 
-  if (!handshake_completion_callback_.is_null()) {
-    SSLContext* context = SSLContext::GetInstance();
-    context->session_cache()->SetSessionAddedCallback(
-        ssl_,
-        base::Bind(&SSLClientSocketOpenSSL::OnHandshakeCompletion,
-                   base::Unretained(this)));
-  }
-
   // Set SSL to client mode. Handshake happens in the loop below.
   SSL_set_connect_state(ssl_);
 
@@ -465,8 +458,6 @@ void SSLClientSocketOpenSSL::Disconnect() {
   // completed, this is a no-op.
   OnHandshakeCompletion();
   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_);
@@ -687,6 +678,16 @@ int SSLClientSocketOpenSSL::SetSendBufferSize(int32 size) {
   return transport_->socket()->SetSendBufferSize(size);
 }
 
+// static
+void SSLClientSocketOpenSSL::OnSessionFinishedCallback(const SSL* ssl,
+                                                       int result,
+                                                       int unused) {
+  SSLClientSocketOpenSSL* ssl_socket =
+      SSLContext::GetInstance()->GetClientSocketFromSSL(ssl);
+  if (result == SSL_CB_HANDSHAKE_DONE)
+    ssl_socket->CheckIfSessionFinished();
+}
+
 int SSLClientSocketOpenSSL::Init() {
   DCHECK(!ssl_);
   DCHECK(!transport_bio_);
@@ -701,6 +702,9 @@ int SSLClientSocketOpenSSL::Init() {
   if (!SSL_set_tlsext_host_name(ssl_, host_and_port_.host().c_str()))
     return ERR_UNEXPECTED;
 
+  // Set an OpenSSL callback to monitor this SSL*'s connection.
+  SSL_set_info_callback(ssl_, &OnSessionFinishedCallback);
+
   trying_cached_session_ = context->session_cache()->SetSSLSessionWithKey(
       ssl_, GetSessionCacheKey());
 
@@ -1032,6 +1036,7 @@ int SSLClientSocketOpenSSL::DoVerifyCertComplete(int result) {
     // TODO(joth): Work out if we need to remember the intermediate CA certs
     // when the server sends them to us, and do so here.
     SSLContext::GetInstance()->session_cache()->MarkSSLSessionAsGood(ssl_);
+    CheckIfSessionFinished();
   } else {
     DVLOG(1) << "DoVerifyCertComplete error " << ErrorToString(result)
              << " (" << result << ")";
@@ -1577,6 +1582,20 @@ long SSLClientSocketOpenSSL::MaybeReplayTransportError(
   return retvalue;
 }
 
+// Determines if the session for |ssl| is in the cache, and calls the
+// appropriate callback if that is the case.
+//
+// CheckIfSessionFinished is called twice per connection: once after
+// MarkSSLSessionAsGood, when the certificate has been verified, and
+// once via an OpenSSL callback when the handshake has completed. On the
+// second call, the connection's handshake completion callback is run.
+void SSLClientSocketOpenSSL::CheckIfSessionFinished() {
+  // Increment the session's completion count.
+  if (++session_completion_count_ == 2) {

[Ryan Sleevi, 2014/08/06 01:31:30]

Is this still necessary / the right approach, now that you're within the socket?

It seems like you have access to all the information you need internally, and
can make this more explicit. For example, rather than this count (which has
implicit state), you could set a bool when the cb was called.

if (cb_called_ &&
SSLContext::GetInstance()->session_cache()->SessionIsGood(ssl_))
  ...

[mshelley, 2014/08/06 03:09:31]

Done.

+    OnHandshakeCompletion();
+  }
+}
+
 // static
 long SSLClientSocketOpenSSL::BIOCallback(
     BIO *bio,