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..7e95d60ba32535899d116152b7dc104424743fa2 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),
+      ran_session_finished_callback_(false),
       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,18 @@ int SSLClientSocketOpenSSL::SetSendBufferSize(int32 size) {
   return transport_->socket()->SetSendBufferSize(size);
 }
 
+// static
+void SSLClientSocketOpenSSL::OnSessionFinishedCallback(const SSL* ssl,
+                                                       int result,
+                                                       int unused) {

[wtc, 2014/08/07 02:10:12]

The Style Guide recommends commenting out the name of the unused variable, like
this:

  int /*unused*/

See
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Function_Decla...

+  SSLClientSocketOpenSSL* ssl_socket =
+      SSLContext::GetInstance()->GetClientSocketFromSSL(ssl);
+  if (result == SSL_CB_HANDSHAKE_DONE) {
+    ssl_socket->ran_session_finished_callback_ = true;
+    ssl_socket->CheckIfSessionFinished();
+  }
+}
+
 int SSLClientSocketOpenSSL::Init() {
   DCHECK(!ssl_);
   DCHECK(!transport_bio_);
@@ -701,6 +704,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 +1038,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 +1584,21 @@ long SSLClientSocketOpenSSL::MaybeReplayTransportError(
   return retvalue;
 }
 
+// Determines if the session for |ssl| is in the cache, and calls the

[wtc, 2014/08/07 02:10:12]

|ssl| => |ssl_|

+// appropriate callback if that is the case.

[wtc, 2014/08/07 02:10:12]

Nit: be more specific than "the appropriate callback".

+//
+// 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, when the certificate has been verified and the handshake
+// has completed, the connection's handshake completion callback is run.
+void SSLClientSocketOpenSSL::CheckIfSessionFinished() {

[wtc, 2014/08/07 02:10:12]

This method should be renamed, but I don't have a good suggestion right now.

Maybe "CheckIfHandshakeCompleted"?

+  if (ran_session_finished_callback_ &&
+      SSLContext::GetInstance()->session_cache()->SessionIsGood(ssl_)) {

[wtc, 2014/08/07 02:10:12]

IMPORTANT: It seems that we are doing this the hard way.

SessionIsGood() returns true iff we have called MarkSSLSessionAsGood().

Therefore, all we need is a boolean member to remember if we have called
MarkSSLSessionAsGood(), or you can use your "completion count" solution here.

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