From bba3823dbc18cb50d6c43a9777f21f052b81dca7 Mon Sep 17 00:00:00 2001 From: "ziadh@chromium.org" Date: Fri, 6 Aug 2010 22:55:46 +0000 Subject: Bad interaction between 2 A/B experiments There is a 1/20 chance that the max number of sockets per group be greater than the max number of sockets per proxy server. This would result in hitting a DCHECK in client_socket_pool_base.cc:145. This CL attempts to circumvent that event. r=jar Review URL: http://codereview.chromium.org/3044051 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@55303 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/browser/browser_main.cc | 17 ++++++++++++++--- net/http/http_network_session.cc | 9 ++++++++- net/http/http_network_session.h | 1 + 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/chrome/browser/browser_main.cc b/chrome/browser/browser_main.cc index 7c35799..79f3f0d 100644 --- a/chrome/browser/browser_main.cc +++ b/chrome/browser/browser_main.cc @@ -165,6 +165,8 @@ BrowserMainParts::~BrowserMainParts() { void BrowserMainParts::EarlyInitialization() { PreEarlyInitialization(); + // Note: make sure to call ConnectionFieldTrial() before + // ProxyConnectionsFieldTrial(). ConnectionFieldTrial(); SocketTimeoutFieldTrial(); ProxyConnectionsFieldTrial(); @@ -273,9 +275,18 @@ void BrowserMainParts::ProxyConnectionsFieldTrial() { scoped_refptr proxy_connection_trial = new FieldTrial("ProxyConnectionImpact", kProxyConnectionsDivisor); - const int proxy_connections_8 = - proxy_connection_trial->AppendGroup("_proxy_connections_8", - kProxyConnectionProbability); + // The number of max sockets per group cannot be greater than the max number + // of sockets per proxy server. Consequently, we eliminate the + // |proxy_connections_8| trial group if we do happen to pass this lower bound + // (which may vary due to the existence of field trials). This leaves us with + // [16, 32, 64] to choose from. Otherwise, we have a set of four values: + // [8, 16, 32, 64]. + int proxy_connections_8 = -1; + if (net::HttpNetworkSession::max_sockets_per_group() <= 8) { + proxy_connections_8 = + proxy_connection_trial->AppendGroup("_proxy_connections_8", + kProxyConnectionProbability); + } const int proxy_connections_16 = proxy_connection_trial->AppendGroup("_proxy_connections_16", kProxyConnectionProbability); diff --git a/net/http/http_network_session.cc b/net/http/http_network_session.cc index 204c46a..e49d66b 100644 --- a/net/http/http_network_session.cc +++ b/net/http/http_network_session.cc @@ -138,6 +138,11 @@ HttpNetworkSession::GetSocketPoolForSSLWithProxy( } // static +int HttpNetworkSession::max_sockets_per_group() { + return g_max_sockets_per_group; +} + +// static void HttpNetworkSession::set_max_sockets_per_group(int socket_count) { DCHECK_LT(0, socket_count); // The following is a sanity check... but we should NEVER be near this value. @@ -149,7 +154,9 @@ void HttpNetworkSession::set_max_sockets_per_group(int socket_count) { void HttpNetworkSession::set_max_sockets_per_proxy_server(int socket_count) { DCHECK_LT(0, socket_count); DCHECK_GT(100, socket_count); // Sanity check. - + // Assert this case early on. The max number of sockets per group cannot + // exceed the max number of sockets per proxy server. + DCHECK_LE(g_max_sockets_per_group, socket_count); g_max_sockets_per_proxy_server = socket_count; } diff --git a/net/http/http_network_session.h b/net/http/http_network_session.h index fdda68d..650a831 100644 --- a/net/http/http_network_session.h +++ b/net/http/http_network_session.h @@ -101,6 +101,7 @@ class HttpNetworkSession : public base::RefCounted, return network_delegate_; } + static int max_sockets_per_group(); static void set_max_sockets_per_group(int socket_count); static void set_max_sockets_per_proxy_server(int socket_count); -- cgit v1.1