From c4e291151f6cebe0f1c981a3cb16cac4db25a93d Mon Sep 17 00:00:00 2001 From: vadimt Date: Wed, 17 Dec 2014 17:44:21 -0800 Subject: More instrumentations for jank in SSLClientSocketOpenSSL::DoHandshake BUG=424386 Review URL: https://codereview.chromium.org/814613004 Cr-Commit-Position: refs/heads/master@{#308915} --- net/socket/ssl_client_socket_openssl.cc | 25 ++++++++++++++---- net/socket/ssl_session_cache_openssl.cc | 46 +++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 5 deletions(-) diff --git a/net/socket/ssl_client_socket_openssl.cc b/net/socket/ssl_client_socket_openssl.cc index d74a755..de74b04 100644 --- a/net/socket/ssl_client_socket_openssl.cc +++ b/net/socket/ssl_client_socket_openssl.cc @@ -927,12 +927,12 @@ int SSLClientSocketOpenSSL::DoHandshake() { int net_error = OK; int rv = SSL_do_handshake(ssl_); - // TODO(vadimt): Remove ScopedTracker below once crbug.com/424386 is fixed. - tracked_objects::ScopedTracker tracking_profile2( - FROM_HERE_WITH_EXPLICIT_FUNCTION( - "424386 SSLClientSocketOpenSSL::DoHandshake2")); - if (client_auth_cert_needed_) { + // TODO(vadimt): Remove ScopedTracker below once crbug.com/424386 is fixed. + tracked_objects::ScopedTracker tracking_profile2( + FROM_HERE_WITH_EXPLICIT_FUNCTION( + "424386 SSLClientSocketOpenSSL::DoHandshake2")); + net_error = ERR_SSL_CLIENT_AUTH_CERT_NEEDED; // If the handshake already succeeded (because the server requests but // doesn't require a client cert), we need to invalidate the SSL session @@ -948,6 +948,11 @@ int SSLClientSocketOpenSSL::DoHandshake() { } } } else if (rv == 1) { + // TODO(vadimt): Remove ScopedTracker below once crbug.com/424386 is fixed. + tracked_objects::ScopedTracker tracking_profile3( + FROM_HERE_WITH_EXPLICIT_FUNCTION( + "424386 SSLClientSocketOpenSSL::DoHandshake3")); + if (trying_cached_session_ && logging::DEBUG_MODE) { DVLOG(2) << "Result of session reuse for " << host_and_port_.ToString() << " is: " << (SSL_session_reused(ssl_) ? "Success" : "Fail"); @@ -995,6 +1000,11 @@ int SSLClientSocketOpenSSL::DoHandshake() { UpdateServerCert(); GotoState(STATE_VERIFY_CERT); } else { + // TODO(vadimt): Remove ScopedTracker below once crbug.com/424386 is fixed. + tracked_objects::ScopedTracker tracking_profile4( + FROM_HERE_WITH_EXPLICIT_FUNCTION( + "424386 SSLClientSocketOpenSSL::DoHandshake4")); + int ssl_error = SSL_get_error(ssl_, rv); if (ssl_error == SSL_ERROR_WANT_CHANNEL_ID_LOOKUP) { @@ -1200,6 +1210,11 @@ void SSLClientSocketOpenSSL::DoConnectCallback(int rv) { } void SSLClientSocketOpenSSL::UpdateServerCert() { + // TODO(vadimt): Remove ScopedTracker below once crbug.com/424386 is fixed. + tracked_objects::ScopedTracker tracking_profile( + FROM_HERE_WITH_EXPLICIT_FUNCTION( + "424386 SSLClientSocketOpenSSL::UpdateServerCert")); + server_cert_chain_->Reset(SSL_get_peer_cert_chain(ssl_)); server_cert_ = server_cert_chain_->AsOSChain(); diff --git a/net/socket/ssl_session_cache_openssl.cc b/net/socket/ssl_session_cache_openssl.cc index 92ae44b..c77790f 100644 --- a/net/socket/ssl_session_cache_openssl.cc +++ b/net/socket/ssl_session_cache_openssl.cc @@ -13,6 +13,7 @@ #include "base/containers/hash_tables.h" #include "base/lazy_instance.h" #include "base/logging.h" +#include "base/profiler/scoped_tracker.h" #include "base/synchronization/lock.h" namespace net { @@ -156,6 +157,11 @@ class SSLSessionCacheOpenSSLImpl { SSLSessionCacheOpenSSLImpl(SSL_CTX* ctx, const SSLSessionCacheOpenSSL::Config& config) : ctx_(ctx), config_(config), expiration_check_(0) { + // TODO(vadimt): Remove ScopedTracker below once crbug.com/424386 is fixed. + tracked_objects::ScopedTracker tracking_profile( + FROM_HERE_WITH_EXPLICIT_FUNCTION( + "424386 SSLSessionCacheOpenSSLImpl::SSLSessionCacheOpenSSLImpl")); + DCHECK(ctx); // NO_INTERNAL_STORE disables OpenSSL's builtin cache, and @@ -196,6 +202,11 @@ class SSLSessionCacheOpenSSLImpl { // // Return true if a cached session ID was found, false otherwise. bool SetSSLSession(SSL* ssl) { + // TODO(vadimt): Remove ScopedTracker below once crbug.com/424386 is fixed. + tracked_objects::ScopedTracker tracking_profile( + FROM_HERE_WITH_EXPLICIT_FUNCTION( + "424386 SSLSessionCacheOpenSSLImpl::SetSSLSession")); + std::string cache_key = config_.key_func(ssl); if (cache_key.empty()) return false; @@ -206,6 +217,11 @@ class SSLSessionCacheOpenSSLImpl { // Variant of SetSSLSession to be used when the client already has computed // the cache key. Avoid a call to the configuration's |key_func| function. bool SetSSLSessionWithKey(SSL* ssl, const std::string& cache_key) { + // TODO(vadimt): Remove ScopedTracker below once crbug.com/424386 is fixed. + tracked_objects::ScopedTracker tracking_profile( + FROM_HERE_WITH_EXPLICIT_FUNCTION( + "424386 SSLSessionCacheOpenSSLImpl::SetSSLSessionWithKey")); + base::AutoLock locked(lock_); DCHECK_EQ(config_.key_func(ssl), cache_key); @@ -239,6 +255,11 @@ class SSLSessionCacheOpenSSLImpl { // Return true iff a cached session was associated with the given |cache_key|. bool SSLSessionIsInCache(const std::string& cache_key) const { + // TODO(vadimt): Remove ScopedTracker below once crbug.com/424386 is fixed. + tracked_objects::ScopedTracker tracking_profile( + FROM_HERE_WITH_EXPLICIT_FUNCTION( + "424386 SSLSessionCacheOpenSSLImpl::SSLSessionIsInCache")); + base::AutoLock locked(lock_); KeyIndex::const_iterator it = key_index_.find(cache_key); if (it == key_index_.end()) @@ -254,6 +275,11 @@ class SSLSessionCacheOpenSSLImpl { } void MarkSSLSessionAsGood(SSL* ssl) { + // TODO(vadimt): Remove ScopedTracker below once crbug.com/424386 is fixed. + tracked_objects::ScopedTracker tracking_profile( + FROM_HERE_WITH_EXPLICIT_FUNCTION( + "424386 SSLSessionCacheOpenSSLImpl::MarkSSLSessionAsGood")); + SSL_SESSION* session = SSL_get_session(ssl); CHECK(session); @@ -264,6 +290,11 @@ class SSLSessionCacheOpenSSLImpl { // Flush all entries from the cache. void Flush() { + // TODO(vadimt): Remove ScopedTracker below once crbug.com/424386 is fixed. + tracked_objects::ScopedTracker tracking_profile( + FROM_HERE_WITH_EXPLICIT_FUNCTION( + "424386 SSLSessionCacheOpenSSLImpl::Flush")); + base::AutoLock lock(lock_); id_index_.clear(); key_index_.clear(); @@ -358,6 +389,11 @@ class SSLSessionCacheOpenSSLImpl { // to indicate that it took ownership of the session, i.e. that the caller // should not decrement its reference count after completion. static int NewSessionCallbackStatic(SSL* ssl, SSL_SESSION* session) { + // TODO(vadimt): Remove ScopedTracker below once crbug.com/424386 is fixed. + tracked_objects::ScopedTracker tracking_profile( + FROM_HERE_WITH_EXPLICIT_FUNCTION( + "424386 SSLSessionCacheOpenSSLImpl::NewSessionCallbackStatic")); + SSLSessionCacheOpenSSLImpl* cache = GetCache(ssl->ctx); cache->OnSessionAdded(ssl, session); return 1; @@ -366,6 +402,11 @@ class SSLSessionCacheOpenSSLImpl { // Called by OpenSSL to indicate that a session must be removed from the // cache. This happens when SSL_CTX is destroyed. static void RemoveSessionCallbackStatic(SSL_CTX* ctx, SSL_SESSION* session) { + // TODO(vadimt): Remove ScopedTracker below once crbug.com/424386 is fixed. + tracked_objects::ScopedTracker tracking_profile( + FROM_HERE_WITH_EXPLICIT_FUNCTION( + "424386 SSLSessionCacheOpenSSLImpl::RemoveSessionCallbackStatic")); + GetCache(ctx)->OnSessionRemoved(session); } @@ -387,6 +428,11 @@ class SSLSessionCacheOpenSSLImpl { static int GenerateSessionIdStatic(const SSL* ssl, unsigned char* id, unsigned* id_len) { + // TODO(vadimt): Remove ScopedTracker below once crbug.com/424386 is fixed. + tracked_objects::ScopedTracker tracking_profile( + FROM_HERE_WITH_EXPLICIT_FUNCTION( + "424386 SSLSessionCacheOpenSSLImpl::GenerateSessionIdStatic")); + if (!GetCache(ssl->ctx)->OnGenerateSessionId(id, *id_len)) return 0; -- cgit v1.1