QUIC - minor cleanup of the following field trials and used defaults for

them.

   "enable_truncated_connection_ids": "true",
   "load_server_info_time_to_srtt": "0.25"

Deleted "load_server_info_timeout" field trial, because
"load_server_info_time_to_srtt" field trial accomplishes the same in a
better manner.

R=rch@chromium.org

Review URL: https://codereview.chromium.org/975613002

Cr-Commit-Position: refs/heads/master@{#318957}

chrome/browser/io_thread.cc

@@ -1024,12 +1024,8 @@ void IOThread::InitializeNetworkSessionParamsFromGlobals(
       &params->quic_always_require_handshake_confirmation);
   globals.quic_disable_connection_pooling.CopyToIfSet(
       &params->quic_disable_connection_pooling);
-  globals.quic_load_server_info_timeout_ms.CopyToIfSet(
-      &params->quic_load_server_info_timeout_ms);
   globals.quic_load_server_info_timeout_srtt_multiplier.CopyToIfSet(
       &params->quic_load_server_info_timeout_srtt_multiplier);
-  globals.quic_enable_truncated_connection_ids.CopyToIfSet(
-      &params->quic_enable_truncated_connection_ids);
   globals.quic_enable_connection_racing.CopyToIfSet(
       &params->quic_enable_connection_racing);
   globals.quic_enable_non_blocking_io.CopyToIfSet(
@@ -1156,16 +1152,6 @@ void IOThread::ConfigureQuicGlobals(
         ShouldQuicAlwaysRequireHandshakeConfirmation(quic_trial_params));
     globals->quic_disable_connection_pooling.set(
         ShouldQuicDisableConnectionPooling(quic_trial_params));
-    int load_server_info_timeout_ms =
-        GetQuicLoadServerInfoTimeout(quic_trial_params);
-    if (load_server_info_timeout_ms != 0) {
-      globals->quic_load_server_info_timeout_ms.set(
-          load_server_info_timeout_ms);
-    }
-    globals->quic_enable_truncated_connection_ids.set(
-        ShouldQuicEnableTruncatedConnectionIds(quic_trial_params));
-    globals->quic_enable_connection_racing.set(
-        ShouldQuicEnableConnectionRacing(quic_trial_params));
     int receive_buffer_size = GetQuicSocketReceiveBufferSize(quic_trial_params);
     if (receive_buffer_size != 0) {
       globals->quic_socket_receive_buffer_size.set(receive_buffer_size);
@@ -1176,8 +1162,6 @@ void IOThread::ConfigureQuicGlobals(
       globals->quic_load_server_info_timeout_srtt_multiplier.set(
           load_server_info_timeout_srtt_multiplier);
     }
-    globals->quic_enable_truncated_connection_ids.set(
-        ShouldQuicEnableTruncatedConnectionIds(quic_trial_params));
     globals->quic_enable_connection_racing.set(
         ShouldQuicEnableConnectionRacing(quic_trial_params));
     globals->quic_enable_non_blocking_io.set(
@@ -1348,18 +1332,6 @@ bool IOThread::ShouldQuicDisableConnectionPooling(
       "true");
 }
 
-// static
-int IOThread::GetQuicLoadServerInfoTimeout(
-    const VariationParameters& quic_trial_params) {
-  int value;
-  if (base::StringToInt(GetVariationParam(quic_trial_params,
-                                          "load_server_info_timeout"),
-                        &value)) {
-    return value;
-  }
-  return 0;
-}
-
 // static
 float IOThread::GetQuicLoadServerInfoTimeoutSrttMultiplier(
     const VariationParameters& quic_trial_params) {
@@ -1372,14 +1344,6 @@ float IOThread::GetQuicLoadServerInfoTimeoutSrttMultiplier(
   return 0.0f;
 }
 
-// static
-bool IOThread::ShouldQuicEnableTruncatedConnectionIds(
-    const VariationParameters& quic_trial_params) {
-  return LowerCaseEqualsASCII(
-      GetVariationParam(quic_trial_params, "enable_truncated_connection_ids"),
-      "true");
-}
-
 // static
 bool IOThread::ShouldQuicEnableConnectionRacing(
     const VariationParameters& quic_trial_params) {

chrome/browser/io_thread.h

@@ -186,9 +186,7 @@ class IOThread : public content::BrowserThreadDelegate {
     Optional<bool> enable_quic_port_selection;
     Optional<bool> quic_always_require_handshake_confirmation;
     Optional<bool> quic_disable_connection_pooling;
-    Optional<int> quic_load_server_info_timeout_ms;
     Optional<float> quic_load_server_info_timeout_srtt_multiplier;
-    Optional<bool> quic_enable_truncated_connection_ids;
     Optional<bool> quic_enable_connection_racing;
     Optional<bool> quic_enable_non_blocking_io;
     Optional<bool> quic_disable_disk_cache;
@@ -366,22 +364,12 @@ class IOThread : public content::BrowserThreadDelegate {
   static bool ShouldQuicDisableConnectionPooling(
       const VariationParameters& quic_trial_params);
 
-  // Returns the timeout value for loading of QUIC sever information from disk
-  // cache based on field trial. Returns 0 if there is an error parsing the
-  // field trial params, or if the default value should be used.
-  static int GetQuicLoadServerInfoTimeout(
-      const VariationParameters& quic_trial_params);
-
   // Returns the ratio of time to load QUIC sever information from disk cache to
   // 'smoothed RTT' based on field trial. Returns 0 if there is an error parsing
   // the field trial params, or if the default value should be used.
   static float GetQuicLoadServerInfoTimeoutSrttMultiplier(
       const VariationParameters& quic_trial_params);
 
-  // Returns true if QUIC's TruncatedConnectionIds should be enabled.
-  static bool ShouldQuicEnableTruncatedConnectionIds(
-      const VariationParameters& quic_trial_params);
-
   // Returns true if QUIC's connection racing should be enabled.
   static bool ShouldQuicEnableConnectionRacing(
       const VariationParameters& quic_trial_params);

chrome/browser/io_thread_unittest.cc

@@ -135,9 +135,7 @@ TEST_F(IOThreadTest, EnableQuicFromFieldTrialGroup) {
   EXPECT_EQ(net::QuicTagVector(), params.quic_connection_options);
   EXPECT_FALSE(params.quic_always_require_handshake_confirmation);
   EXPECT_FALSE(params.quic_disable_connection_pooling);
-  EXPECT_EQ(0, params.quic_load_server_info_timeout_ms);
-  EXPECT_EQ(0.0f, params.quic_load_server_info_timeout_srtt_multiplier);
-  EXPECT_FALSE(params.quic_enable_truncated_connection_ids);
+  EXPECT_EQ(0.25f, params.quic_load_server_info_timeout_srtt_multiplier);
   EXPECT_FALSE(params.quic_enable_connection_racing);
   EXPECT_FALSE(params.quic_enable_non_blocking_io);
   EXPECT_FALSE(params.quic_disable_disk_cache);
@@ -294,15 +292,6 @@ TEST_F(IOThreadTest,
   EXPECT_TRUE(params.quic_disable_connection_pooling);
 }
 
-TEST_F(IOThreadTest, QuicLoadServerInfoTimeoutFromFieldTrialParams) {
-  field_trial_group_ = "Enabled";
-  field_trial_params_["load_server_info_timeout"] = "50";
-  ConfigureQuicGlobals();
-  net::HttpNetworkSession::Params params;
-  InitializeNetworkSessionParams(&params);
-  EXPECT_EQ(50, params.quic_load_server_info_timeout_ms);
-}
-
 TEST_F(IOThreadTest, QuicLoadServerInfoTimeToSmoothedRttFromFieldTrialParams) {
   field_trial_group_ = "Enabled";
   field_trial_params_["load_server_info_time_to_srtt"] = "0.5";
@@ -312,15 +301,6 @@ TEST_F(IOThreadTest, QuicLoadServerInfoTimeToSmoothedRttFromFieldTrialParams) {
   EXPECT_EQ(0.5f, params.quic_load_server_info_timeout_srtt_multiplier);
 }
 
-TEST_F(IOThreadTest, QuicEnableTruncatedConnectionIds) {
-  field_trial_group_ = "Enabled";
-  field_trial_params_["enable_truncated_connection_ids"] = "true";
-  ConfigureQuicGlobals();
-  net::HttpNetworkSession::Params params;
-  InitializeNetworkSessionParams(&params);
-  EXPECT_TRUE(params.quic_enable_truncated_connection_ids);
-}
-
 TEST_F(IOThreadTest, QuicEnableConnectionRacing) {
   field_trial_group_ = "Enabled";
   field_trial_params_["enable_connection_racing"] = "true";

net/http/http_network_session.cc

@@ -90,9 +90,7 @@ HttpNetworkSession::Params::Params()
       enable_quic_port_selection(true),
       quic_always_require_handshake_confirmation(false),
       quic_disable_connection_pooling(false),
-      quic_load_server_info_timeout_ms(0),
-      quic_load_server_info_timeout_srtt_multiplier(0.0f),
-      quic_enable_truncated_connection_ids(false),
+      quic_load_server_info_timeout_srtt_multiplier(0.25f),
       quic_enable_connection_racing(false),
       quic_enable_non_blocking_io(false),
       quic_disable_disk_cache(false),
@@ -139,9 +137,7 @@ HttpNetworkSession::HttpNetworkSession(const Params& params)
           params.enable_quic_port_selection,
           params.quic_always_require_handshake_confirmation,
           params.quic_disable_connection_pooling,
-          params.quic_load_server_info_timeout_ms,
           params.quic_load_server_info_timeout_srtt_multiplier,
-          params.quic_enable_truncated_connection_ids,
           params.quic_enable_connection_racing,
           params.quic_enable_non_blocking_io,
           params.quic_disable_disk_cache,

net/http/http_network_session.h

@@ -117,9 +117,7 @@ class NET_EXPORT HttpNetworkSession
     bool enable_quic_port_selection;
     bool quic_always_require_handshake_confirmation;
     bool quic_disable_connection_pooling;
-    int quic_load_server_info_timeout_ms;
     float quic_load_server_info_timeout_srtt_multiplier;
-    bool quic_enable_truncated_connection_ids;
     bool quic_enable_connection_racing;
     bool quic_enable_non_blocking_io;
     bool quic_disable_disk_cache;

net/quic/quic_stream_factory.cc

@@ -395,20 +395,18 @@ int QuicStreamFactory::Job::DoLoadServerInfo() {
 
   // To mitigate the effects of disk cache taking too long to load QUIC server
   // information, set up a timer to cancel WaitForDataReady's callback.
-  int64 load_server_info_timeout_ms = factory_->load_server_info_timeout_ms_;
   if (factory_->load_server_info_timeout_srtt_multiplier_ > 0) {
-    DCHECK_EQ(0, load_server_info_timeout_ms);
-    load_server_info_timeout_ms =
+    int64 load_server_info_timeout_ms =
         (factory_->load_server_info_timeout_srtt_multiplier_ *
          factory_->GetServerNetworkStatsSmoothedRttInMicroseconds(server_id_)) /
         1000;
-  }
-  if (load_server_info_timeout_ms > 0) {
-    factory_->task_runner_->PostDelayedTask(
-        FROM_HERE,
-        base::Bind(&QuicStreamFactory::Job::CancelWaitForDataReadyCallback,
-                   GetWeakPtr()),
-        base::TimeDelta::FromMilliseconds(load_server_info_timeout_ms));
+    if (load_server_info_timeout_ms > 0) {
+      factory_->task_runner_->PostDelayedTask(
+          FROM_HERE,
+          base::Bind(&QuicStreamFactory::Job::CancelWaitForDataReadyCallback,
+                     GetWeakPtr()),
+          base::TimeDelta::FromMilliseconds(load_server_info_timeout_ms));
+    }
   }
 
   int rv = server_info_->WaitForDataReady(
@@ -594,9 +592,7 @@ QuicStreamFactory::QuicStreamFactory(
     bool enable_port_selection,
     bool always_require_handshake_confirmation,
     bool disable_connection_pooling,
-    int load_server_info_timeout,
     float load_server_info_timeout_srtt_multiplier,
-    bool enable_truncated_connection_ids,
     bool enable_connection_racing,
     bool enable_non_blocking_io,
     bool disable_disk_cache,
@@ -618,10 +614,8 @@ QuicStreamFactory::QuicStreamFactory(
       always_require_handshake_confirmation_(
           always_require_handshake_confirmation),
       disable_connection_pooling_(disable_connection_pooling),
-      load_server_info_timeout_ms_(load_server_info_timeout),
       load_server_info_timeout_srtt_multiplier_(
           load_server_info_timeout_srtt_multiplier),
-      enable_truncated_connection_ids_(enable_truncated_connection_ids),
       enable_connection_racing_(enable_connection_racing),
       enable_non_blocking_io_(enable_non_blocking_io),
       disable_disk_cache_(disable_disk_cache),
@@ -1153,8 +1147,7 @@ int QuicStreamFactory::CreateSession(const QuicServerId& server_id,
       FROM_HERE_WITH_EXPLICIT_FUNCTION(
           "422516 QuicStreamFactory::CreateSession57"));
 
-  if (enable_truncated_connection_ids_)
-    config.SetBytesForConnectionIdToSend(0);
+  config.SetBytesForConnectionIdToSend(0);
 
   if (quic_server_info_factory_ && !server_info) {
     // TODO(vadimt): Remove ScopedTracker below once crbug.com/422516 is fixed.

net/quic/quic_stream_factory.h

@@ -105,9 +105,7 @@ class NET_EXPORT_PRIVATE QuicStreamFactory
       bool enable_port_selection,
       bool always_require_handshake_confirmation,
       bool disable_connection_pooling,
-      int load_server_info_timeout,
       float load_server_info_timeout_srtt_multiplier,
-      bool enable_truncated_connection_ids,
       bool enable_connection_racing,
       bool enable_non_blocking_io,
       bool disable_disk_cache,
@@ -313,20 +311,12 @@ class NET_EXPORT_PRIVATE QuicStreamFactory
   // Set if we do not want connection pooling.
   bool disable_connection_pooling_;
 
-  // Specifies the timeout in milliseconds to wait for loading of QUIC server
-  // information. If we don't want to timeout, set
-  // |load_server_info_timeout_ms_| to 0.
-  int load_server_info_timeout_ms_;
-
   // Specifies the ratio between time to load QUIC server information from disk
   // cache to 'smoothed RTT'. This ratio is used to calculate the timeout in
   // milliseconds to wait for loading of QUIC server information. If we don't
   // want to timeout, set |load_server_info_timeout_srtt_multiplier_| to 0.
   float load_server_info_timeout_srtt_multiplier_;
 
-  // Set this for setting config's BytesForConnectionIdToSend (TCID param) to 0.
-  bool enable_truncated_connection_ids_;
-
   // Set if we want to race connections - one connection that sends
   // INCHOATE_HELLO and another connection that sends CHLO after loading server
   // config from the disk cache.

net/quic/quic_stream_factory_test.cc

@@ -125,11 +125,6 @@ class QuicStreamFactoryPeer {
     factory->task_runner_ = task_runner;
   }
 
-  static void SetLoadServerInfoTimeout(QuicStreamFactory* factory,
-                                       size_t load_server_info_timeout) {
-    factory->load_server_info_timeout_ms_ = load_server_info_timeout;
-  }
-
   static void SetEnableConnectionRacing(QuicStreamFactory* factory,
                                         bool enable_connection_racing) {
     factory->enable_connection_racing_ = enable_connection_racing;
@@ -207,9 +202,7 @@ class QuicStreamFactoryTest : public ::testing::TestWithParam<TestParams> {
                  /*enable_port_selection=*/true,
                  /*always_require_handshake_confirmation=*/false,
                  /*disable_connection_pooling=*/false,
-                 /*load_server_info_timeout=*/0u,
                  /*load_server_info_timeout_srtt_multiplier=*/0.0f,
-                 /*enable_truncated_connection_ids=*/true,
                  /*enable_connection_racing=*/false,
                  /*enable_non_blocking_io=*/true,
                  /*disable_disk_cache=*/false,
@@ -1613,62 +1606,11 @@ TEST_P(QuicStreamFactoryTest, CryptoConfigWhenProofIsInvalid) {
   }
 }
 
-TEST_P(QuicStreamFactoryTest, CancelWaitForDataReady) {
-  // Don't race quic connections when testing cancel reading of server config
-  // from disk cache.
-  if (GetParam().enable_connection_racing)
-    return;
-  factory_.set_quic_server_info_factory(&quic_server_info_factory_);
-  QuicStreamFactoryPeer::SetTaskRunner(&factory_, runner_.get());
-  const size_t kLoadServerInfoTimeoutMs = 50;
-  QuicStreamFactoryPeer::SetLoadServerInfoTimeout(
-      &factory_, kLoadServerInfoTimeoutMs);
-
-  MockRead reads[] = {
-    MockRead(ASYNC, OK, 0)  // EOF
-  };
-  DeterministicSocketData socket_data(reads, arraysize(reads), nullptr, 0);
-  socket_factory_.AddSocketDataProvider(&socket_data);
-  socket_data.StopAfter(1);
-
-  crypto_client_stream_factory_.set_handshake_mode(
-      MockCryptoClientStream::ZERO_RTT);
-  host_resolver_.set_synchronous_mode(true);
-  host_resolver_.rules()->AddIPLiteralRule(host_port_pair_.host(),
-                                           "192.168.0.1", "");
-
-  QuicStreamRequest request(&factory_);
-  EXPECT_EQ(ERR_IO_PENDING,
-            request.Request(host_port_pair_,
-                            is_https_,
-                            privacy_mode_,
-                            "GET",
-                            net_log_,
-                            callback_.callback()));
-
-  // Verify that the CancelWaitForDataReady task has been posted.
-  ASSERT_EQ(1u, runner_->GetPostedTasks().size());
-  EXPECT_EQ(base::TimeDelta::FromMilliseconds(kLoadServerInfoTimeoutMs),
-            runner_->GetPostedTasks()[0].delay);
-
-  runner_->RunNextTask();
-  ASSERT_EQ(0u, runner_->GetPostedTasks().size());
-
-  scoped_ptr<QuicHttpStream> stream = request.ReleaseStream();
-  EXPECT_TRUE(stream.get());
-  EXPECT_TRUE(socket_data.at_read_eof());
-  EXPECT_TRUE(socket_data.at_write_eof());
-}
-
 TEST_P(QuicStreamFactoryTest, RacingConnections) {
   if (!GetParam().enable_connection_racing)
     return;
   factory_.set_quic_server_info_factory(&quic_server_info_factory_);
   QuicStreamFactoryPeer::SetTaskRunner(&factory_, runner_.get());
-  const size_t kLoadServerInfoTimeoutMs = 50;
-  QuicStreamFactoryPeer::SetLoadServerInfoTimeout(&factory_,
-                                                  kLoadServerInfoTimeoutMs);
-
   MockRead reads[] = {
       MockRead(ASYNC, OK, 0)  // EOF
   };
@@ -1710,9 +1652,6 @@ TEST_P(QuicStreamFactoryTest, RacingConnections) {
 TEST_P(QuicStreamFactoryTest, EnableNotLoadFromDiskCache) {
   factory_.set_quic_server_info_factory(&quic_server_info_factory_);
   QuicStreamFactoryPeer::SetTaskRunner(&factory_, runner_.get());
-  const size_t kLoadServerInfoTimeoutMs = 50;
-  QuicStreamFactoryPeer::SetLoadServerInfoTimeout(&factory_,
-                                                  kLoadServerInfoTimeoutMs);
   QuicStreamFactoryPeer::SetDisableDiskCache(&factory_, true);
 
   MockRead reads[] = {