Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1404)

Unified Diff: net/socket/ssl_client_socket_pool.cc

Issue 981723008: Unwind the SSL connection holdback experiment and remove related code (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 5 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: net/socket/ssl_client_socket_pool.cc
diff --git a/net/socket/ssl_client_socket_pool.cc b/net/socket/ssl_client_socket_pool.cc
index 6643ef4e3af0ac5e00c9c0b27f31b37e278abd1d..c5de88ab536b67d3544d0b987c157bd26f0e4998 100644
--- a/net/socket/ssl_client_socket_pool.cc
+++ b/net/socket/ssl_client_socket_pool.cc
@@ -9,7 +9,6 @@
#include "base/metrics/field_trial.h"
#include "base/metrics/histogram.h"
#include "base/metrics/sparse_histogram.h"
-#include "base/stl_util.h"
#include "base/values.h"
#include "net/base/host_port_pair.h"
#include "net/base/net_errors.h"
@@ -95,77 +94,6 @@ SSLSocketParams::GetHttpProxyConnectionParams() const {
return http_proxy_params_;
}
-SSLConnectJobMessenger::SocketAndCallback::SocketAndCallback(
- SSLClientSocket* ssl_socket,
- const base::Closure& job_resumption_callback)
- : socket(ssl_socket), callback(job_resumption_callback) {
-}
-
-SSLConnectJobMessenger::SocketAndCallback::~SocketAndCallback() {
-}
-
-SSLConnectJobMessenger::SSLConnectJobMessenger(
- const base::Closure& messenger_finished_callback)
- : messenger_finished_callback_(messenger_finished_callback),
- weak_factory_(this) {
-}
-
-SSLConnectJobMessenger::~SSLConnectJobMessenger() {
-}
-
-void SSLConnectJobMessenger::RemovePendingSocket(SSLClientSocket* ssl_socket) {
- // Sockets do not need to be removed from connecting_sockets_ because
- // OnSSLHandshakeCompleted will do this.
- for (SSLPendingSocketsAndCallbacks::iterator it =
- pending_sockets_and_callbacks_.begin();
- it != pending_sockets_and_callbacks_.end();
- ++it) {
- if (it->socket == ssl_socket) {
- pending_sockets_and_callbacks_.erase(it);
- return;
- }
- }
-}
-
-bool SSLConnectJobMessenger::CanProceed(SSLClientSocket* ssl_socket) {
- // If there are no connecting sockets, allow the connection to proceed.
- return connecting_sockets_.empty();
-}
-
-void SSLConnectJobMessenger::MonitorConnectionResult(
- SSLClientSocket* ssl_socket) {
- connecting_sockets_.push_back(ssl_socket);
- ssl_socket->SetHandshakeCompletionCallback(
- base::Bind(&SSLConnectJobMessenger::OnSSLHandshakeCompleted,
- weak_factory_.GetWeakPtr()));
-}
-
-void SSLConnectJobMessenger::AddPendingSocket(SSLClientSocket* ssl_socket,
- const base::Closure& callback) {
- DCHECK(!connecting_sockets_.empty());
- pending_sockets_and_callbacks_.push_back(
- SocketAndCallback(ssl_socket, callback));
-}
-
-void SSLConnectJobMessenger::OnSSLHandshakeCompleted() {
- connecting_sockets_.clear();
- SSLPendingSocketsAndCallbacks temp_list;
- temp_list.swap(pending_sockets_and_callbacks_);
- base::Closure messenger_finished_callback = messenger_finished_callback_;
- messenger_finished_callback.Run();
- RunAllCallbacks(temp_list);
-}
-
-void SSLConnectJobMessenger::RunAllCallbacks(
- const SSLPendingSocketsAndCallbacks& pending_sockets_and_callbacks) {
- for (std::vector<SocketAndCallback>::const_iterator it =
- pending_sockets_and_callbacks.begin();
- it != pending_sockets_and_callbacks.end();
- ++it) {
- it->callback.Run();
- }
-}
-
// Timeout for the SSL handshake portion of the connect.
static const int kSSLHandshakeTimeoutInSeconds = 30;
@@ -178,7 +106,6 @@ SSLConnectJob::SSLConnectJob(const std::string& group_name,
HttpProxyClientSocketPool* http_proxy_pool,
ClientSocketFactory* client_socket_factory,
const SSLClientSocketContext& context,
- const GetMessengerCallback& get_messenger_callback,
Delegate* delegate,
NetLog* net_log)
: ConnectJob(group_name,
@@ -199,17 +126,10 @@ SSLConnectJob::SSLConnectJob(const std::string& group_name,
(params->privacy_mode() == PRIVACY_MODE_ENABLED
? "pm/" + context.ssl_session_cache_shard
: context.ssl_session_cache_shard)),
- io_callback_(
- base::Bind(&SSLConnectJob::OnIOComplete, base::Unretained(this))),
- messenger_(NULL),
- get_messenger_callback_(get_messenger_callback),
- weak_factory_(this) {
-}
+ callback_(base::Bind(&SSLConnectJob::OnIOComplete,
+ base::Unretained(this))) {}
-SSLConnectJob::~SSLConnectJob() {
- if (ssl_socket_.get() && messenger_)
- messenger_->RemovePendingSocket(ssl_socket_.get());
-}
+SSLConnectJob::~SSLConnectJob() {}
LoadState SSLConnectJob::GetLoadState() const {
switch (next_state_) {
@@ -223,8 +143,6 @@ LoadState SSLConnectJob::GetLoadState() const {
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;
@@ -281,12 +199,6 @@ int SSLConnectJob::DoLoop(int result) {
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();
@@ -314,14 +226,14 @@ int SSLConnectJob::DoTransportConnect() {
return transport_socket_handle_->Init(group_name(),
direct_params,
priority(),
- io_callback_,
+ callback_,
transport_pool_,
net_log());
}
int SSLConnectJob::DoTransportConnectComplete(int result) {
if (result == OK)
- next_state_ = STATE_CREATE_SSL_SOCKET;
+ next_state_ = STATE_SSL_CONNECT;
return result;
}
@@ -335,14 +247,14 @@ int SSLConnectJob::DoSOCKSConnect() {
return transport_socket_handle_->Init(group_name(),
socks_proxy_params,
priority(),
- io_callback_,
+ callback_,
socks_pool_,
net_log());
}
int SSLConnectJob::DoSOCKSConnectComplete(int result) {
if (result == OK)
- next_state_ = STATE_CREATE_SSL_SOCKET;
+ next_state_ = STATE_SSL_CONNECT;
return result;
}
@@ -357,7 +269,7 @@ int SSLConnectJob::DoTunnelConnect() {
return transport_socket_handle_->Init(group_name(),
http_proxy_params,
priority(),
- io_callback_,
+ callback_,
http_proxy_pool_,
net_log());
}
@@ -376,16 +288,18 @@ int SSLConnectJob::DoTunnelConnectComplete(int result) {
}
if (result < 0)
return result;
- next_state_ = STATE_CREATE_SSL_SOCKET;
+
+ next_state_ = STATE_SSL_CONNECT;
return result;
}
-int SSLConnectJob::DoCreateSSLSocket() {
+int SSLConnectJob::DoSSLConnect() {
// TODO(pkasting): Remove ScopedTracker below once crbug.com/462815 is fixed.
tracked_objects::ScopedTracker tracking_profile(
FROM_HERE_WITH_EXPLICIT_FUNCTION(
- "462815 SSLConnectJob::DoCreateSSLSocket"));
- next_state_ = STATE_CHECK_FOR_RESUME;
+ "462815 SSLConnectJob::DoSSLConnect"));
+
+ next_state_ = STATE_SSL_CONNECT_COMPLETE;
// Reset the timeout to just the time allowed for the SSL handshake.
ResetTimer(base::TimeDelta::FromSeconds(kSSLHandshakeTimeoutInSeconds));
@@ -404,48 +318,14 @@ int SSLConnectJob::DoCreateSSLSocket() {
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_);
-
- if (!ssl_socket_->InSessionCache())
- messenger_ = get_messenger_callback_.Run(ssl_socket_->GetSessionCacheKey());
-
- return OK;
-}
-
-int SSLConnectJob::DoCheckForResume() {
- next_state_ = STATE_SSL_CONNECT;
-
- if (!messenger_)
- return OK;
-
- if (messenger_->CanProceed(ssl_socket_.get())) {
- messenger_->MonitorConnectionResult(ssl_socket_.get());
- // The SSLConnectJob no longer needs access to the messenger after this
- // point.
- messenger_ = NULL;
- return OK;
- }
-
- messenger_->AddPendingSocket(ssl_socket_.get(),
- base::Bind(&SSLConnectJob::ResumeSSLConnection,
- weak_factory_.GetWeakPtr()));
-
- return ERR_IO_PENDING;
-}
-
-int SSLConnectJob::DoSSLConnect() {
- // TODO(pkasting): Remove ScopedTracker below once crbug.com/462813 is fixed.
- tracked_objects::ScopedTracker tracking_profile(
- FROM_HERE_WITH_EXPLICIT_FUNCTION("462813 SSLConnectJob::DoSSLConnect"));
- next_state_ = STATE_SSL_CONNECT_COMPLETE;
-
- connect_timing_.ssl_start = base::TimeTicks::Now();
-
- return ssl_socket_->Connect(io_callback_);
+ return ssl_socket_->Connect(callback_);
}
int SSLConnectJob::DoSSLConnectComplete(int result) {
@@ -534,9 +414,9 @@ int SSLConnectJob::DoSSLConnectComplete(int result) {
}
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,
@@ -578,12 +458,6 @@ int SSLConnectJob::DoSSLConnectComplete(int result) {
return result;
}
-void SSLConnectJob::ResumeSSLConnection() {
- DCHECK_EQ(next_state_, STATE_SSL_CONNECT);
- messenger_ = NULL;
- OnIOComplete(OK);
-}
-
SSLConnectJob::State SSLConnectJob::GetInitialState(
SSLSocketParams::ConnectionType connection_type) {
switch (connection_type) {
@@ -609,14 +483,12 @@ SSLClientSocketPool::SSLConnectJobFactory::SSLConnectJobFactory(
HttpProxyClientSocketPool* http_proxy_pool,
ClientSocketFactory* client_socket_factory,
const SSLClientSocketContext& context,
- const SSLConnectJob::GetMessengerCallback& get_messenger_callback,
NetLog* net_log)
: transport_pool_(transport_pool),
socks_pool_(socks_pool),
http_proxy_pool_(http_proxy_pool),
client_socket_factory_(client_socket_factory),
context_(context),
- get_messenger_callback_(get_messenger_callback),
net_log_(net_log) {
base::TimeDelta max_transport_timeout = base::TimeDelta();
base::TimeDelta pool_timeout;
@@ -654,15 +526,11 @@ SSLClientSocketPool::SSLClientSocketPool(
SOCKSClientSocketPool* socks_pool,
HttpProxyClientSocketPool* http_proxy_pool,
SSLConfigService* ssl_config_service,
- bool enable_ssl_connect_job_waiting,
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(
@@ -676,12 +544,8 @@ SSLClientSocketPool::SSLClientSocketPool(
cert_transparency_verifier,
cert_policy_enforcer,
ssl_session_cache_shard),
- base::Bind(
- &SSLClientSocketPool::GetOrCreateSSLConnectJobMessenger,
- base::Unretained(this)),
net_log)),
- ssl_config_service_(ssl_config_service),
- enable_ssl_connect_job_waiting_(enable_ssl_connect_job_waiting) {
+ ssl_config_service_(ssl_config_service) {
if (ssl_config_service_.get())
ssl_config_service_->AddObserver(this);
if (transport_pool_)
@@ -693,13 +557,12 @@ SSLClientSocketPool::SSLClientSocketPool(
}
SSLClientSocketPool::~SSLClientSocketPool() {
- STLDeleteContainerPairSecondPointers(messenger_map_.begin(),
- messenger_map_.end());
if (ssl_config_service_.get())
ssl_config_service_->RemoveObserver(this);
}
-scoped_ptr<ConnectJob> SSLClientSocketPool::SSLConnectJobFactory::NewConnectJob(
+scoped_ptr<ConnectJob>
+SSLClientSocketPool::SSLConnectJobFactory::NewConnectJob(
const std::string& group_name,
const PoolBase::Request& request,
ConnectJob::Delegate* delegate) const {
@@ -712,13 +575,12 @@ scoped_ptr<ConnectJob> SSLClientSocketPool::SSLConnectJobFactory::NewConnectJob(
http_proxy_pool_,
client_socket_factory_,
context_,
- get_messenger_callback_,
delegate,
net_log_));
}
-base::TimeDelta SSLClientSocketPool::SSLConnectJobFactory::ConnectionTimeout()
- const {
+base::TimeDelta
+SSLClientSocketPool::SSLConnectJobFactory::ConnectionTimeout() const {
return timeout_;
}
@@ -833,32 +695,6 @@ bool SSLClientSocketPool::CloseOneIdleConnection() {
return base_.CloseOneIdleConnectionInHigherLayeredPool();
}
-SSLConnectJobMessenger* SSLClientSocketPool::GetOrCreateSSLConnectJobMessenger(
- const std::string& cache_key) {
- if (!enable_ssl_connect_job_waiting_)
- return NULL;
- MessengerMap::const_iterator it = messenger_map_.find(cache_key);
- if (it == messenger_map_.end()) {
- std::pair<MessengerMap::iterator, bool> iter =
- messenger_map_.insert(MessengerMap::value_type(
- cache_key,
- new SSLConnectJobMessenger(
- base::Bind(&SSLClientSocketPool::DeleteSSLConnectJobMessenger,
- base::Unretained(this),
- cache_key))));
- it = iter.first;
- }
- return it->second;
-}
-
-void SSLClientSocketPool::DeleteSSLConnectJobMessenger(
- const std::string& cache_key) {
- MessengerMap::iterator it = messenger_map_.find(cache_key);
- CHECK(it != messenger_map_.end());
- delete it->second;
- messenger_map_.erase(it);
-}
-
void SSLClientSocketPool::OnSSLConfigChanged() {
FlushWithError(ERR_NETWORK_CHANGED);
}

Powered by Google App Engine
This is Rietveld 408576698