A/B experiment for re-establishing TCP connections.

We would like to test the impact of automatic retries when a TCP connection exceeds a certain threshold before it gets back an ACK. We are observing a fair number of sockets where the connection had been established, but the sockets
were not used thereafter.


r=mbelche


Review URL: http://codereview.chromium.org/3191019

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@57276 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/browser_main.cc

@@ -177,6 +177,7 @@ void BrowserMainParts::EarlyInitialization() {
   ProxyConnectionsFieldTrial();
   SpdyFieldTrial();
   PrefetchFieldTrial();
+  ConnectBackupJobsFieldTrial();
   InitializeSSL();
 
   if (parsed_command_line().HasSwitch(switches::kEnableDNSSECCerts))
@@ -392,6 +393,34 @@ void BrowserMainParts::PrefetchFieldTrial() {
   }
 }
 
+// If neither --enable-connect-backup-jobs or --disable-connect-backup-jobs is
+// specified, run an A/B test for automatically establishing backup TCP
+// connections when a certain timeout value is exceeded.
+void BrowserMainParts::ConnectBackupJobsFieldTrial() {
+  if (parsed_command_line().HasSwitch(switches::kEnableConnectBackupJobs)) {
+    net::internal::ClientSocketPoolBaseHelper::set_connect_backup_jobs_enabled(
+        true);
+  } else if (parsed_command_line().HasSwitch(
+        switches::kDisableConnectBackupJobs)) {
+    net::internal::ClientSocketPoolBaseHelper::set_connect_backup_jobs_enabled(
+        false);
+  } else {
+    const FieldTrial::Probability kConnectBackupJobsDivisor = 100;
+    // 50% probability.
+    const FieldTrial::Probability kConnectBackupJobsProbability = 50;
+    scoped_refptr<FieldTrial> trial =
+      new FieldTrial("ConnnectBackupJobs", kConnectBackupJobsDivisor);
+    const int connect_backup_jobs_enabled =
+        trial->AppendGroup("ConnectBackupJobsEnabled",
+            kConnectBackupJobsProbability);
+    trial->AppendGroup("ConnectBackupJobsDisabled",
+                       FieldTrial::kAllRemainingProbability);
+    const int trial_group = trial->group();
+    net::internal::ClientSocketPoolBaseHelper::set_connect_backup_jobs_enabled(
+        trial_group == connect_backup_jobs_enabled);
+  }
+}
+
 // BrowserMainParts: |MainMessageLoopStart()| and related ----------------------
 
 void BrowserMainParts::MainMessageLoopStart() {

chrome/browser/browser_main.h

@@ -116,6 +116,10 @@ class BrowserMainParts {
   // A/B test for prefetching with --(enable|disable)-prefetch not set.
   void PrefetchFieldTrial();
 
+  // A/B test for automatically establishing a backup TCP connection when a
+  // specified timeout value is reached.
+  void ConnectBackupJobsFieldTrial();
+
   // Used to initialize NSPR where appropriate.
   virtual void InitializeSSL() = 0;
 

chrome/common/chrome_switches.cc

@@ -145,6 +145,10 @@ const char kDisableBackingStoreLimit[]      = "disable-backing-store-limit";
 // Disable support for cached byte-ranges.
 const char kDisableByteRangeSupport[]       = "disable-byte-range-support";
 
+// Disables establishing a backup TCP connection if a specified timeout is
+// exceeded.
+const char kDisableConnectBackupJobs[]      = "disable-connect-backup-jobs";
+
 // Disable requests that webkit labels TargetIsPrefetch.  As of
 // writing only <link rel=prefetch...> but also eventually
 // Link: headers.
@@ -268,7 +272,7 @@ const char kDisableSyncPasswords[]          = "disable-sync-passwords";
 const char kDisableSyncPreferences[]        = "disable-sync-preferences";
 
 // Disable syncing of sessions.
-const char kDisableSyncSessions[]        = "disable-sync-sessions";
+const char kDisableSyncSessions[]           = "disable-sync-sessions";
 
 // Disable syncing of themes.
 const char kDisableSyncThemes[]             = "disable-sync-themes";
@@ -316,7 +320,7 @@ const char kDomAutomationController[]       = "dom-automation";
 const char kDumpHistogramsOnExit[]          = "dump-histograms-on-exit";
 
 // Enable displaying net log events on the command line.
-extern const char kLogNetLog[]             = "log-net-log";
+extern const char kLogNetLog[]              = "log-net-log";
 
 // Enable gpu-accelerated compositing.
 const char kEnableAcceleratedCompositing[]  = "enable-accelerated-compositing";
@@ -337,7 +341,7 @@ const char kEnableBenchmarking[]            = "enable-benchmarking";
 
 // This applies only when the process type is "service". Enables the
 // Chromoting Host Process within the service process.
-const char kEnableRemoting[]              = "enable-remoting";
+const char kEnableRemoting[]                = "enable-remoting";
 
 // Enable click-to-play for blocked plugins.
 const char kEnableClickToPlay[]             = "enable-click-to-play";
@@ -349,6 +353,10 @@ const char kEnableCloudPrintProxy[]         = "enable-cloud-print-proxy";
 // Enables the Cloud Print dialog hosting code.
 const char kEnableCloudPrint[]              = "enable-cloud-print";
 
+// Enables establishing a backup TCP connection if a specified timeout is
+// exceeded.
+const char kEnableConnectBackupJobs[]       = "enable-connect-backup-jobs";
+
 // Enable requests that webkit labels TargetIsPrefetch.  As of
 // writing only <link rel=prefetch...> but also eventually
 // Link: headers.
@@ -468,7 +476,7 @@ const char kEnableSyncPasswords[]           = "enable-sync-passwords";
 const char kEnableSyncPreferences[]         = "enable-sync-preferences";
 
 // Enable syncing browser sessions.
-const char kEnableSyncSessions[]         = "enable-sync-sessions";
+const char kEnableSyncSessions[]            = "enable-sync-sessions";
 
 // Enable syncing browser themes.
 const char kEnableSyncThemes[]              = "enable-sync-themes";

chrome/common/chrome_switches.h

@@ -54,6 +54,7 @@ extern const char kDisableAuthNegotiateCnameLookup[];
 extern const char kDisableBackgroundMode[];
 extern const char kDisableBackingStoreLimit[];
 extern const char kDisableByteRangeSupport[];
+extern const char kDisableConnectBackupJobs[];
 extern const char kDisableContentPrefetch[];
 extern const char kDisableCustomJumpList[];
 extern const char kDisableDatabases[];
@@ -112,6 +113,7 @@ extern const char kEnableRemoting[];
 extern const char kEnableClickToPlay[];
 extern const char kEnableCloudPrintProxy[];
 extern const char kEnableCloudPrint[];
+extern const char kEnableConnectBackupJobs[];
 extern const char kEnableContentPrefetch[];
 extern const char kEnableCookiePrompt[];
 extern const char kEnableDeviceMotion[];

chrome/renderer/render_view.cc

@@ -4911,6 +4911,43 @@ void RenderView::DumpLoadHistograms() const {
                   begin_to_finish_all_loads);
   }
 
+  // Histograms to determine if backup connection jobs have an impact on PLT.
+  static const bool connect_backup_jobs_fieldtrial(
+      FieldTrialList::Find("ConnnectBackupJobs") &&
+      !FieldTrialList::Find("ConnnectBackupJobs")->group_name().empty());
+  if (connect_backup_jobs_fieldtrial) {
+    UMA_HISTOGRAM_ENUMERATION(
+        FieldTrial::MakeName("PLT.Abandoned", "ConnnectBackupJobs"),
+        abandoned_page ? 1 : 0, 2);
+    UMA_HISTOGRAM_ENUMERATION(
+        FieldTrial::MakeName("PLT.LoadType", "ConnnectBackupJobs"),
+        load_type, NavigationState::kLoadTypeMax);
+    switch (load_type) {
+      case NavigationState::NORMAL_LOAD:
+        PLT_HISTOGRAM(FieldTrial::MakeName(
+            "PLT.BeginToFinish_NormalLoad", "ConnnectBackupJobs"),
+            begin_to_finish_all_loads);
+        break;
+      case NavigationState::LINK_LOAD_NORMAL:
+        PLT_HISTOGRAM(FieldTrial::MakeName(
+            "PLT.BeginToFinish_LinkLoadNormal", "ConnnectBackupJobs"),
+            begin_to_finish_all_loads);
+        break;
+      case NavigationState::LINK_LOAD_RELOAD:
+        PLT_HISTOGRAM(FieldTrial::MakeName(
+            "PLT.BeginToFinish_LinkLoadReload", "ConnnectBackupJobs"),
+            begin_to_finish_all_loads);
+        break;
+      case NavigationState::LINK_LOAD_CACHE_STALE_OK:
+        PLT_HISTOGRAM(FieldTrial::MakeName(
+            "PLT.BeginToFinish_LinkLoadStaleOk", "ConnnectBackupJobs"),
+            begin_to_finish_all_loads);
+        break;
+      default:
+        break;
+    }
+  }
+
   // Histograms to determine if the number of connections has an
   // impact on PLT.
   // TODO(jar): Consider removing the per-link-type versions.  We

net/socket/client_socket.cc

@@ -4,6 +4,7 @@
 
 #include "net/socket/client_socket.h"
 
+#include "base/field_trial.h"
 #include "base/histogram.h"
 
 namespace net {
@@ -41,7 +42,15 @@ void ClientSocket::EmitPreconnectionHistograms() const {
     result += 3;
   else if (subresource_speculation_)
     result += 6;
-  UMA_HISTOGRAM_ENUMERATION("Net.PreconnectUtilization", result, 9);
+
+  static const bool connect_backup_jobs_fieldtrial =
+      FieldTrialList::Find("ConnnectBackupJobs") &&
+      !FieldTrialList::Find("ConnnectBackupJobs")->group_name().empty();
+  if (connect_backup_jobs_fieldtrial) {
+    UMA_HISTOGRAM_ENUMERATION(
+        FieldTrial::MakeName("Net.PreconnectUtilization", "ConnnectBackupJobs"),
+        result, 9);
+  }
 }
 
 void ClientSocket::SetSubresourceSpeculation() {

net/socket/client_socket_pool_base.cc

@@ -27,6 +27,10 @@ namespace {
 // some conditions.  See http://crbug.com/4606.
 const int kCleanupInterval = 10;  // DO NOT INCREASE THIS TIMEOUT.
 
+// Indicate whether or not we should establish a new TCP connection after a
+// certain timeout has passed without receiving an ACK.
+bool g_connect_backup_jobs_enabled = true;
+
 }  // namespace
 
 namespace net {
@@ -137,7 +141,7 @@ ClientSocketPoolBaseHelper::ClientSocketPoolBaseHelper(
       unused_idle_socket_timeout_(unused_idle_socket_timeout),
       used_idle_socket_timeout_(used_idle_socket_timeout),
       connect_job_factory_(connect_job_factory),
-      backup_jobs_enabled_(false),
+      connect_backup_jobs_enabled_(false),
       ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)),
       pool_generation_number_(0),
       in_destructor_(false) {
@@ -247,10 +251,11 @@ int ClientSocketPoolBaseHelper::RequestSocketInternal(
     // If we don't have any sockets in this group, set a timer for potentially
     // creating a new one.  If the SYN is lost, this backup socket may complete
     // before the slow socket, improving end user latency.
-    if (group.IsEmpty() && !group.backup_job && backup_jobs_enabled_) {
-      group.backup_job = connect_job_factory_->NewConnectJob(group_name,
-                                                             *request,
-                                                             this);
+    if (group.IsEmpty() && !group.connect_backup_job &&
+        connect_backup_jobs_enabled_) {
+      group.connect_backup_job = connect_job_factory_->NewConnectJob(group_name,
+                                                                    *request,
+                                                                     this);
       StartBackupSocketTimer(group_name);
     }
 
@@ -327,7 +332,7 @@ void ClientSocketPoolBaseHelper::OnBackupSocketTimerFired(
   CHECK(group.backup_task);
   group.backup_task = NULL;
 
-  CHECK(group.backup_job);
+  CHECK(group.connect_backup_job);
 
   // If there are no more jobs pending, there is no work to do.
   // If we've done our cleanups correctly, this should not happen.
@@ -345,14 +350,15 @@ void ClientSocketPoolBaseHelper::OnBackupSocketTimerFired(
     return;
   }
 
-  group.backup_job->net_log().AddEvent(NetLog::TYPE_SOCKET_BACKUP_CREATED,
-                                       NULL);
+  group.connect_backup_job->net_log().AddEvent(
+      NetLog::TYPE_SOCKET_BACKUP_CREATED,
+      NULL);
   SIMPLE_STATS_COUNTER("socket.backup_created");
-  int rv = group.backup_job->Connect();
+  int rv = group.connect_backup_job->Connect();
   connecting_socket_count_++;
-  group.jobs.insert(group.backup_job);
-  ConnectJob* job = group.backup_job;
-  group.backup_job = NULL;
+  group.jobs.insert(group.connect_backup_job);
+  ConnectJob* job = group.connect_backup_job;
+  group.connect_backup_job = NULL;
   if (rv != ERR_IO_PENDING)
     OnConnectJobComplete(rv, job);
 }
@@ -495,6 +501,15 @@ void ClientSocketPoolBaseHelper::CleanupIdleSockets(bool force) {
   }
 }
 
+// static
+void ClientSocketPoolBaseHelper::set_connect_backup_jobs_enabled(bool enabled) {
+  g_connect_backup_jobs_enabled = enabled;
+}
+
+void ClientSocketPoolBaseHelper::EnableConnectBackupJobs() {
+  connect_backup_jobs_enabled_ = g_connect_backup_jobs_enabled;
+}
+
 void ClientSocketPoolBaseHelper::IncrementIdleCount() {
   if (++idle_socket_count_ == 1)
     timer_.Start(TimeDelta::FromSeconds(kCleanupInterval), this,

net/socket/client_socket_pool_base.h

@@ -239,7 +239,8 @@ class ClientSocketPoolBaseHelper
     return connect_job_factory_->ConnectionTimeout();
   }
 
-  void EnableBackupJobs() { backup_jobs_enabled_ = true; }
+  static void set_connect_backup_jobs_enabled(bool enabled);
+  void EnableConnectBackupJobs();
 
  private:
   friend class base::RefCounted<ClientSocketPoolBaseHelper>;
@@ -271,7 +272,7 @@ class ClientSocketPoolBaseHelper
   struct Group {
     Group()
         : active_socket_count(0),
-          backup_job(NULL),
+          connect_backup_job(NULL),
           backup_task(NULL) {
     }
 
@@ -299,9 +300,9 @@ class ClientSocketPoolBaseHelper
     }
 
     void CleanupBackupJob() {
-      if (backup_job) {
-        delete backup_job;
-        backup_job = NULL;
+      if (connect_backup_job) {
+        delete connect_backup_job;
+        connect_backup_job = NULL;
       }
       if (backup_task) {
         backup_task->Cancel();
@@ -314,7 +315,7 @@ class ClientSocketPoolBaseHelper
     RequestQueue pending_requests;
     int active_socket_count;  // number of active sockets used by clients
     // A backup job in case the connect for this group takes too long.
-    ConnectJob* backup_job;
+    ConnectJob* connect_backup_job;
     CancelableTask* backup_task;
   };
 
@@ -461,7 +462,7 @@ class ClientSocketPoolBaseHelper
   const scoped_ptr<ConnectJobFactory> connect_job_factory_;
 
   // TODO(vandebo) Remove when backup jobs move to TCPClientSocketPool
-  bool backup_jobs_enabled_;
+  bool connect_backup_jobs_enabled_;
 
   // A factory to pin the backup_job tasks.
   ScopedRunnableMethodFactory<ClientSocketPoolBaseHelper> method_factory_;
@@ -595,7 +596,7 @@ class ClientSocketPoolBase {
     return histograms_;
   }
 
-  void EnableBackupJobs() { helper_->EnableBackupJobs(); }
+  void EnableConnectBackupJobs() { helper_->EnableConnectBackupJobs(); }
 
   void Flush() { helper_->Flush(); }
 

net/socket/client_socket_pool_base_unittest.cc

@@ -419,7 +419,7 @@ class TestClientSocketPool : public ClientSocketPool {
 
   void CleanupTimedOutIdleSockets() { base_.CleanupIdleSockets(false); }
 
-  void EnableBackupJobs() { base_.EnableBackupJobs(); }
+  void EnableConnectBackupJobs() { base_.EnableConnectBackupJobs(); }
 
  private:
   ~TestClientSocketPool() {}
@@ -995,7 +995,7 @@ TEST_F(ClientSocketPoolBaseTest, WaitForStalledSocketAtSocketLimit) {
 // Regression test for http://crbug.com/40952.
 TEST_F(ClientSocketPoolBaseTest, CloseIdleSocketAtSocketLimitDeleteGroup) {
   CreatePool(kDefaultMaxSockets, kDefaultMaxSocketsPerGroup);
-  pool_->EnableBackupJobs();
+  pool_->EnableConnectBackupJobs();
   connect_job_factory_->set_job_type(TestConnectJob::kMockJob);
 
   for (int i = 0; i < kDefaultMaxSockets; ++i) {
@@ -1923,7 +1923,7 @@ TEST_F(ClientSocketPoolBaseTest, DoNotReuseSocketAfterFlush) {
 TEST_F(ClientSocketPoolBaseTest, BackupSocketCancelAtMaxSockets) {
   // Max 4 sockets globally, max 4 sockets per group.
   CreatePool(kDefaultMaxSockets, kDefaultMaxSockets);
-  pool_->EnableBackupJobs();
+  pool_->EnableConnectBackupJobs();
 
   // Create the first socket and set to ERR_IO_PENDING.  This creates a
   // backup job.

net/socket/tcp_client_socket_pool.cc

@@ -202,7 +202,7 @@ TCPClientSocketPool::TCPClientSocketPool(
             base::TimeDelta::FromSeconds(kUsedIdleSocketTimeout),
             new TCPConnectJobFactory(client_socket_factory,
                                      host_resolver, net_log)) {
-  base_.EnableBackupJobs();
+  base_.EnableConnectBackupJobs();
 }
 
 TCPClientSocketPool::~TCPClientSocketPool() {}