SSL Connect Job Waiting

net/socket/ssl_client_socket_pool.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/socket/ssl_client_socket_pool.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
@@ skipping 89 matching lines @@
 SSLConnectJob::SSLConnectJob(const std::string& group_name,
                              RequestPriority priority,
                              const scoped_refptr<SSLSocketParams>& params,
                              const base::TimeDelta& timeout_duration,
                              TransportClientSocketPool* transport_pool,
                              SOCKSClientSocketPool* socks_pool,
                              HttpProxyClientSocketPool* http_proxy_pool,
                              ClientSocketFactory* client_socket_factory,
                              HostResolver* host_resolver,
                              const SSLClientSocketContext& context,
+                             PendingJobList* pending_jobs_list,
                              Delegate* delegate,
                              NetLog* net_log)
     : ConnectJob(group_name,
                  timeout_duration,
                  priority,
                  delegate,
                  BoundNetLog::Make(net_log, NetLog::SOURCE_CONNECT_JOB)),
       params_(params),
       transport_pool_(transport_pool),
       socks_pool_(socks_pool),
       http_proxy_pool_(http_proxy_pool),
       client_socket_factory_(client_socket_factory),
       host_resolver_(host_resolver),
       context_(context.cert_verifier,
                context.server_bound_cert_service,
                context.transport_security_state,
                context.cert_transparency_verifier,
                (params->privacy_mode() == PRIVACY_MODE_ENABLED
                     ? "pm/" + context.ssl_session_cache_shard
                     : context.ssl_session_cache_shard)),
-      callback_(base::Bind(&SSLConnectJob::OnIOComplete,
-                           base::Unretained(this))) {}
+      io_callback_(
+          base::Bind(&SSLConnectJob::OnIOComplete, base::Unretained(this))),
+      pending_jobs_(pending_jobs_list) {
+}
 
 SSLConnectJob::~SSLConnectJob() {}
 
+// static
+bool SSLConnectJob::enable_job_waiting_ = false;
+
 LoadState SSLConnectJob::GetLoadState() const {
   switch (next_state_) {
     case STATE_TUNNEL_CONNECT_COMPLETE:
       if (transport_socket_handle_->socket())
         return LOAD_STATE_ESTABLISHING_PROXY_TUNNEL;
       // else, fall through.
     case STATE_TRANSPORT_CONNECT:
     case STATE_TRANSPORT_CONNECT_COMPLETE:
     case STATE_SOCKS_CONNECT:
     case STATE_SOCKS_CONNECT_COMPLETE:
     case STATE_TUNNEL_CONNECT:
       return transport_socket_handle_->GetLoadState();
+    case STATE_CREATE_SSL_SOCKET:
+    case STATE_CHECK_FOR_RESUME:
     case STATE_SSL_CONNECT:
     case STATE_SSL_CONNECT_COMPLETE:
       return LOAD_STATE_SSL_HANDSHAKE;
     default:
       NOTREACHED();
       return LOAD_STATE_IDLE;
   }
 }
 
 void SSLConnectJob::GetAdditionalErrorState(ClientSocketHandle* handle) {
   // Headers in |error_response_info_| indicate a proxy tunnel setup
   // problem. See DoTunnelConnectComplete.
   if (error_response_info_.headers.get()) {
     handle->set_pending_http_proxy_connection(
         transport_socket_handle_.release());
   }
   handle->set_ssl_error_response_info(error_response_info_);
   if (!connect_timing_.ssl_start.is_null())
     handle->set_is_ssl_error(true);
 }
 
+void SSLConnectJob::EnableJobWaiting(bool enable) {
+  enable_job_waiting_ = enable;
+}
+
 void SSLConnectJob::OnIOComplete(int result) {
   int rv = DoLoop(result);
   if (rv != ERR_IO_PENDING)
     NotifyDelegateOfCompletion(rv);  // Deletes |this|.
 }
 
 int SSLConnectJob::DoLoop(int result) {
   DCHECK_NE(next_state_, STATE_NONE);
 
   int rv = result;
@@ skipping 15 matching lines @@
       case STATE_SOCKS_CONNECT_COMPLETE:
         rv = DoSOCKSConnectComplete(rv);
         break;
       case STATE_TUNNEL_CONNECT:
         DCHECK_EQ(OK, rv);
         rv = DoTunnelConnect();
         break;
       case STATE_TUNNEL_CONNECT_COMPLETE:
         rv = DoTunnelConnectComplete(rv);
         break;
+      case STATE_CREATE_SSL_SOCKET:
+        rv = DoCreateSSLSocket();
+        break;
+      case STATE_CHECK_FOR_RESUME:
+        rv = DoCheckForResume();
+        break;
       case STATE_SSL_CONNECT:
         DCHECK_EQ(OK, rv);
         rv = DoSSLConnect();
         break;
       case STATE_SSL_CONNECT_COMPLETE:
         rv = DoSSLConnectComplete(rv);
         break;
+      case STATE_PROCESS_PENDING_JOBS:
+        rv = DoProcessPendingJobs(rv);
+        break;
       default:
         NOTREACHED() << "bad state";
         rv = ERR_FAILED;
         break;
     }
   } while (rv != ERR_IO_PENDING && next_state_ != STATE_NONE);
 
   return rv;
 }
 
 int SSLConnectJob::DoTransportConnect() {
   DCHECK(transport_pool_);
 
   next_state_ = STATE_TRANSPORT_CONNECT_COMPLETE;
   transport_socket_handle_.reset(new ClientSocketHandle());
   scoped_refptr<TransportSocketParams> direct_params =
       params_->GetDirectConnectionParams();
   return transport_socket_handle_->Init(group_name(),
                                         direct_params,
                                         priority(),
-                                        callback_,
+                                        io_callback_,
                                         transport_pool_,
                                         net_log());
 }
 
 int SSLConnectJob::DoTransportConnectComplete(int result) {
-  if (result == OK)
-    next_state_ = STATE_SSL_CONNECT;
+  if (result != OK)
+    return result;
+
+  next_state_ = STATE_CREATE_SSL_SOCKET;
 
   return result;
 }
 
 int SSLConnectJob::DoSOCKSConnect() {
   DCHECK(socks_pool_);
   next_state_ = STATE_SOCKS_CONNECT_COMPLETE;
   transport_socket_handle_.reset(new ClientSocketHandle());
   scoped_refptr<SOCKSSocketParams> socks_proxy_params =
       params_->GetSocksProxyConnectionParams();
   return transport_socket_handle_->Init(group_name(),
                                         socks_proxy_params,
                                         priority(),
-                                        callback_,
+                                        io_callback_,
                                         socks_pool_,
                                         net_log());
 }
 
 int SSLConnectJob::DoSOCKSConnectComplete(int result) {
-  if (result == OK)
-    next_state_ = STATE_SSL_CONNECT;
+  if (result != OK)
+    return result;
+
+  next_state_ = STATE_CREATE_SSL_SOCKET;
 
   return result;
 }
 
 int SSLConnectJob::DoTunnelConnect() {
   DCHECK(http_proxy_pool_);
   next_state_ = STATE_TUNNEL_CONNECT_COMPLETE;
 
   transport_socket_handle_.reset(new ClientSocketHandle());
   scoped_refptr<HttpProxySocketParams> http_proxy_params =
       params_->GetHttpProxyConnectionParams();
   return transport_socket_handle_->Init(group_name(),
                                         http_proxy_params,
                                         priority(),
-                                        callback_,
+                                        io_callback_,
                                         http_proxy_pool_,
                                         net_log());
 }
 
 int SSLConnectJob::DoTunnelConnectComplete(int result) {
   // Extract the information needed to prompt for appropriate proxy
   // authentication so that when ClientSocketPoolBaseHelper calls
   // |GetAdditionalErrorState|, we can easily set the state.
   if (result == ERR_SSL_CLIENT_AUTH_CERT_NEEDED) {
     error_response_info_ = transport_socket_handle_->ssl_error_response_info();
   } else if (result == ERR_PROXY_AUTH_REQUESTED ||
              result == ERR_HTTPS_PROXY_TUNNEL_RESPONSE) {
     StreamSocket* socket = transport_socket_handle_->socket();
     HttpProxyClientSocket* tunnel_socket =
         static_cast<HttpProxyClientSocket*>(socket);
     error_response_info_ = *tunnel_socket->GetConnectResponseInfo();
   }
   if (result < 0)
     return result;
 
-  next_state_ = STATE_SSL_CONNECT;
+  next_state_ = STATE_CREATE_SSL_SOCKET;
   return result;
 }
 
-int SSLConnectJob::DoSSLConnect() {
-  next_state_ = STATE_SSL_CONNECT_COMPLETE;
+int SSLConnectJob::DoCreateSSLSocket() {
+  if (enable_job_waiting_)
+    next_state_ = STATE_CHECK_FOR_RESUME;
+  else
+    next_state_ = STATE_SSL_CONNECT;
   // Reset the timeout to just the time allowed for the SSL handshake.
   ResetTimer(base::TimeDelta::FromSeconds(kSSLHandshakeTimeoutInSeconds));
 
   // If the handle has a fresh socket, get its connect start and DNS times.
   // This should always be the case.
   const LoadTimingInfo::ConnectTiming& socket_connect_timing =
       transport_socket_handle_->connect_timing();
   if (!transport_socket_handle_->is_reused() &&
       !socket_connect_timing.connect_start.is_null()) {
     // Overwriting |connect_start| serves two purposes - it adjusts timing so
-    // |connect_start| doesn't include dns times, and it adjusts the time so
+    // |connect_start| doesn't include DNS times, and it adjusts the time so
     // as not to include time spent waiting for an idle socket.
     connect_timing_.connect_start = socket_connect_timing.connect_start;
     connect_timing_.dns_start = socket_connect_timing.dns_start;
     connect_timing_.dns_end = socket_connect_timing.dns_end;
   }
 
-  connect_timing_.ssl_start = base::TimeTicks::Now();
-
   ssl_socket_ = client_socket_factory_->CreateSSLClientSocket(
       transport_socket_handle_.Pass(),
       params_->host_and_port(),
       params_->ssl_config(),
       context_);
-  return ssl_socket_->Connect(callback_);
+  return OK;
+}
+
+int SSLConnectJob::DoCheckForResume() {
+  // If the group's session is in the cache, continue with the session
+  // resumption SSL handshake.

[wtc, 2014/06/26 17:48:17]

Move this comment to line 358.

+  next_state_ = STATE_SSL_CONNECT;
+
+  if (ssl_socket_->InSessionCache())
+    return OK;
+
+  // If there are pending jobs, wait.
+  if (!pending_jobs_->empty()) {
+    pending_jobs_->push_back(this);
+    return ERR_IO_PENDING;
+  }
+
+  // If there are no pending jobs, continue the full SSL handshake
+  // because a resumption handshake is not possible.
+  //
+  // Add |this| to pending jobs to ensure that future jobs
+  // will know that there is a job in progress.
+  pending_jobs_->push_back(this);
+  return OK;

[wtc, 2014/06/26 17:48:17]

Nit: it is possible to rewrite lines 361-373 to eliminate the duplicate
pending_jobs_->push_back(this) calls:

  int rv;
  if (pending_jobs_->empty()) {
    // If there are no pending jobs, continue the full SSL handshake
    // because a resumption handshake is not possible.
    rv = OK;
  } else {
    // If there are pending jobs, wait.
    rv = ERR_IO_PENDING;
  }

  pending_jobs_->push_back(this);
  return rv;

The comment I asked you to add at lines 370-371 would be very confusing in the
restructured code, so I didn't include it.

If the restructured code is not clear to you, you can ignore this suggestion.

+}
+
+int SSLConnectJob::DoSSLConnect() {
+  next_state_ = STATE_SSL_CONNECT_COMPLETE;
+
+  connect_timing_.ssl_start = base::TimeTicks::Now();
+
+  return ssl_socket_->Connect(io_callback_);
 }
 
 int SSLConnectJob::DoSSLConnectComplete(int result) {
   connect_timing_.ssl_end = base::TimeTicks::Now();
 
+  if (enable_job_waiting_)
+    next_state_ = STATE_PROCESS_PENDING_JOBS;
+
   SSLClientSocket::NextProtoStatus status =
       SSLClientSocket::kNextProtoUnsupported;
   std::string proto;
   std::string server_protos;
   // GetNextProto will fail and and trigger a NOTREACHED if we pass in a socket
   // that hasn't had SSL_ImportFD called on it. If we get a certificate error
   // here, then we know that we called SSL_ImportFD.
   if (result == OK || IsCertificateError(result))
     status = ssl_socket_->GetNextProto(&proto, &server_protos);
 
   // If we want spdy over npn, make sure it succeeded.
   if (status == SSLClientSocket::kNextProtoNegotiated) {
     ssl_socket_->set_was_npn_negotiated(true);
     NextProto protocol_negotiated =
         SSLClientSocket::NextProtoFromString(proto);
     ssl_socket_->set_protocol_negotiated(protocol_negotiated);
     // If we negotiated a SPDY version, it must have been present in
     // SSLConfig::next_protos.
     // TODO(mbelshe): Verify this.
     if (protocol_negotiated >= kProtoSPDYMinimumVersion &&
         protocol_negotiated <= kProtoSPDYMaximumVersion) {
       ssl_socket_->set_was_spdy_negotiated(true);
     }
   }
-  if (params_->want_spdy_over_npn() && !ssl_socket_->was_spdy_negotiated())
+  if (params_->want_spdy_over_npn() && !ssl_socket_->was_spdy_negotiated()) {
     return ERR_NPN_NEGOTIATION_FAILED;
+  }
 
   // Spdy might be turned on by default, or it might be over npn.
   bool using_spdy = params_->force_spdy_over_ssl() ||
       params_->want_spdy_over_npn();
 
   if (result == OK ||
       ssl_socket_->IgnoreCertError(result, params_->load_flags())) {
     DCHECK(!connect_timing_.ssl_start.is_null());
     base::TimeDelta connect_duration =
         connect_timing_.ssl_end - connect_timing_.ssl_start;
@@ skipping 38 matching lines @@
                                  100);
     } else if (ssl_info.handshake_type == SSLInfo::HANDSHAKE_FULL) {
       UMA_HISTOGRAM_CUSTOM_TIMES("Net.SSL_Connection_Latency_Full_Handshake",
                                  connect_duration,
                                  base::TimeDelta::FromMilliseconds(1),
                                  base::TimeDelta::FromMinutes(1),
                                  100);
     }
 
     const std::string& host = params_->host_and_port().host();
-    bool is_google = host == "google.com" ||
-                     (host.size() > 11 &&
-                      host.rfind(".google.com") == host.size() - 11);
+    bool is_google =
+        host == "google.com" ||
+        (host.size() > 11 && host.rfind(".google.com") == host.size() - 11);
     if (is_google) {
       UMA_HISTOGRAM_CUSTOM_TIMES("Net.SSL_Connection_Latency_Google2",
                                  connect_duration,
                                  base::TimeDelta::FromMilliseconds(1),
                                  base::TimeDelta::FromMinutes(1),
                                  100);
       if (ssl_info.handshake_type == SSLInfo::HANDSHAKE_RESUME) {
         UMA_HISTOGRAM_CUSTOM_TIMES("Net.SSL_Connection_Latency_Google_"
                                        "Resume_Handshake",
                                    connect_duration,
@@ skipping 15 matching lines @@
     SetSocket(ssl_socket_.PassAs<StreamSocket>());
   } else if (result == ERR_SSL_CLIENT_AUTH_CERT_NEEDED) {
     error_response_info_.cert_request_info = new SSLCertRequestInfo;
     ssl_socket_->GetSSLCertRequestInfo(
         error_response_info_.cert_request_info.get());
   }
 
   return result;
 }
 
+int SSLConnectJob::DoProcessPendingJobs(int result) {
+  ProcessPendingJobs(result);
+  return result;
+}
+
+void SSLConnectJob::ProcessPendingJobs(int result) {

[wtc, 2014/06/26 17:48:17]

Nit: I would get rid of the ProcessPendingJobs function. Just inline the code in
DoProcessPendingJobs.

+  // If there is a pending list
+  if (!pending_jobs_->empty()) {
+    // The first element of the vector will always be the leading job.
+    PendingJobList::iterator it = pending_jobs_->begin();
+
+    if (result == OK) {
+      // If the connection was successful, tell all pending jobs to proceed.
+      PendingJobList pending_jobs(it + 1, pending_jobs_->end());
+      pending_jobs_->clear();
+      for (PendingJobList::iterator job = pending_jobs.begin();

[wtc, 2014/06/26 17:48:17]

Nit: this can be a const_iterator.

+           job != pending_jobs.end();
+           ++job) {
+        (*job)->ResumeSSLConnection();
+      }
+
+    } else {
+      // If the connection failed, tell one (if any) of the remaining jobs
+      // to proceed as the new leader.
+      it = pending_jobs_->erase(it);
+      if (it != pending_jobs_->end())
+        (*it)->ResumeSSLConnection();
+    }
+  }
+}
+
+void SSLConnectJob::ResumeSSLConnection() {
+  DCHECK_EQ(next_state_, STATE_SSL_CONNECT);
+  next_state_ = STATE_SSL_CONNECT;

[wtc, 2014/06/26 17:48:17]

Delete this line.

+  OnIOComplete(OK);
+}
+
+bool SSLConnectJob::GetEnableJobWaiting() {

[wtc, 2014/06/26 17:48:17]

Move this to line 178, to follow the definition of EnableJobWaiting. This
matches their declaration order in the .h file.

+  return enable_job_waiting_;
+}
+
 SSLConnectJob::State SSLConnectJob::GetInitialState(
     SSLSocketParams::ConnectionType connection_type) {
   switch (connection_type) {
     case SSLSocketParams::DIRECT:
       return STATE_TRANSPORT_CONNECT;
     case SSLSocketParams::HTTP_PROXY:
       return STATE_TUNNEL_CONNECT;
     case SSLSocketParams::SOCKS_PROXY:
       return STATE_SOCKS_CONNECT;
   }
@@ skipping 13 matching lines @@
     ClientSocketFactory* client_socket_factory,
     HostResolver* host_resolver,
     const SSLClientSocketContext& context,
     NetLog* net_log)
     : transport_pool_(transport_pool),
       socks_pool_(socks_pool),
       http_proxy_pool_(http_proxy_pool),
       client_socket_factory_(client_socket_factory),
       host_resolver_(host_resolver),
       context_(context),
-      net_log_(net_log) {
+      net_log_(net_log),
+      pending_jobs_map_(new PendingJobMap()) {
   base::TimeDelta max_transport_timeout = base::TimeDelta();
   base::TimeDelta pool_timeout;
   if (transport_pool_)
     max_transport_timeout = transport_pool_->ConnectionTimeout();
   if (socks_pool_) {
     pool_timeout = socks_pool_->ConnectionTimeout();
     if (pool_timeout > max_transport_timeout)
       max_transport_timeout = pool_timeout;
   }
   if (http_proxy_pool_) {
@@ skipping 17 matching lines @@
     const std::string& ssl_session_cache_shard,
     ClientSocketFactory* client_socket_factory,
     TransportClientSocketPool* transport_pool,
     SOCKSClientSocketPool* socks_pool,
     HttpProxyClientSocketPool* http_proxy_pool,
     SSLConfigService* ssl_config_service,
     NetLog* net_log)
     : transport_pool_(transport_pool),
       socks_pool_(socks_pool),
       http_proxy_pool_(http_proxy_pool),
-      base_(this, max_sockets, max_sockets_per_group, histograms,
+      base_(this,
+            max_sockets,
+            max_sockets_per_group,
+            histograms,
             ClientSocketPool::unused_idle_socket_timeout(),
             ClientSocketPool::used_idle_socket_timeout(),
-            new SSLConnectJobFactory(transport_pool,
-                                     socks_pool,
-                                     http_proxy_pool,
-                                     client_socket_factory,
-                                     host_resolver,
-                                     SSLClientSocketContext(
-                                         cert_verifier,
-                                         server_bound_cert_service,
-                                         transport_security_state,
-                                         cert_transparency_verifier,
-                                         ssl_session_cache_shard),
-                                     net_log)),
+            new SSLConnectJobFactory(
+                transport_pool,
+                socks_pool,
+                http_proxy_pool,
+                client_socket_factory,
+                host_resolver,
+                SSLClientSocketContext(cert_verifier,
+                                       server_bound_cert_service,
+                                       transport_security_state,
+                                       cert_transparency_verifier,
+                                       ssl_session_cache_shard),
+                net_log)),
       ssl_config_service_(ssl_config_service) {
   if (ssl_config_service_.get())
     ssl_config_service_->AddObserver(this);
   if (transport_pool_)
     base_.AddLowerLayeredPool(transport_pool_);
   if (socks_pool_)
     base_.AddLowerLayeredPool(socks_pool_);
   if (http_proxy_pool_)
     base_.AddLowerLayeredPool(http_proxy_pool_);
 }
 
 SSLClientSocketPool::~SSLClientSocketPool() {
   if (ssl_config_service_.get())
     ssl_config_service_->RemoveObserver(this);
 }
 
 scoped_ptr<ConnectJob>
 SSLClientSocketPool::SSLConnectJobFactory::NewConnectJob(
     const std::string& group_name,
     const PoolBase::Request& request,
     ConnectJob::Delegate* delegate) const {
-  return scoped_ptr<ConnectJob>(
-      new SSLConnectJob(group_name, request.priority(), request.params(),
-                        ConnectionTimeout(), transport_pool_, socks_pool_,
-                        http_proxy_pool_, client_socket_factory_,
-                        host_resolver_, context_, delegate, net_log_));
+  SSLConnectJob::PendingJobList* pending_job_list;
+  if (SSLConnectJob::GetEnableJobWaiting()) {
+    PendingJobMap::const_iterator it = pending_jobs_map_->find(group_name);
+    if (it == pending_jobs_map_->end()) {
+      std::pair<PendingJobMap::iterator, bool> iter =

[wtc, 2014/06/26 17:48:17]

Nit: it is a little confusing to have variables named "it" and "iter" in
proximity. I suggest renaming this variable, such as "result" or "inserted".

+          pending_jobs_map_->insert(PendingJobMap::value_type(

[wtc, 2014/06/26 17:48:17]

Nit: it seems that this line is indented with three spaces. Please run git cl
format.

+              group_name, new SSLConnectJob::PendingJobList()));
+
+      it = iter.first;
+    }
+
+    pending_job_list = it->second;
+  } else {
+    pending_job_list = NULL;
+  }
+
+  return scoped_ptr<ConnectJob>(new SSLConnectJob(group_name,
+                                                  request.priority(),
+                                                  request.params(),
+                                                  ConnectionTimeout(),
+                                                  transport_pool_,
+                                                  socks_pool_,
+                                                  http_proxy_pool_,
+                                                  client_socket_factory_,
+                                                  host_resolver_,
+                                                  context_,
+                                                  pending_job_list,
+                                                  delegate,
+                                                  net_log_));
 }
 
 base::TimeDelta
 SSLClientSocketPool::SSLConnectJobFactory::ConnectionTimeout() const {
   return timeout_;
 }
 
 int SSLClientSocketPool::RequestSocket(const std::string& group_name,
                                        const void* socket_params,
                                        RequestPriority priority,
@@ skipping 103 matching lines @@
   if (base_.CloseOneIdleSocket())
     return true;
   return base_.CloseOneIdleConnectionInHigherLayeredPool();
 }
 
 void SSLClientSocketPool::OnSSLConfigChanged() {
   FlushWithError(ERR_NETWORK_CHANGED);
 }
 
 }  // namespace net