Remove QuicNegotiableValue

This CL reimplements QUIC idle timeout negotiation without QuicNegotiableValue, and removes QuicNegotiableValue. QuicNegotiableValue is considered overly complex and hard to understand and use correctly, and has caused bugs in the past. Idle timeout was the only user of it, so reusing QuicFixedUint32 and bespoke handling was the simplest option. The wire format for idle timeout is unchanged by this CL: it's still encoded as a 32bit integer in seconds for QUIC_CRYPTO and a 62bit integer in milliseconds for QUIC+TLS. Therefore this CL does not change any behavior.

gfe-relnote: refactor idle timeout, no behavior change, not flag protected

Startblock:
  after 2020-05-05 09:00 in US/Pacific
PiperOrigin-RevId: 309988666
Change-Id: I67ae7caad8ec73c05e27c8e49d940a2393d4b2df
diff --git a/quic/core/quic_config.cc b/quic/core/quic_config.cc
index 644c8f0..2508764 100644
--- a/quic/core/quic_config.cc
+++ b/quic/core/quic_config.cc
@@ -59,74 +59,6 @@
     : tag_(tag), presence_(presence) {}
 QuicConfigValue::~QuicConfigValue() {}
 
-QuicNegotiableValue::QuicNegotiableValue(QuicTag tag,
-                                         QuicConfigPresence presence)
-    : QuicConfigValue(tag, presence), negotiated_(false) {}
-QuicNegotiableValue::~QuicNegotiableValue() {}
-
-QuicNegotiableUint32::QuicNegotiableUint32(QuicTag tag,
-                                           QuicConfigPresence presence)
-    : QuicNegotiableValue(tag, presence),
-      max_value_(0),
-      default_value_(0),
-      negotiated_value_(0) {}
-QuicNegotiableUint32::~QuicNegotiableUint32() {}
-
-void QuicNegotiableUint32::set(uint32_t max, uint32_t default_value) {
-  DCHECK_LE(default_value, max);
-  max_value_ = max;
-  default_value_ = default_value;
-}
-
-uint32_t QuicNegotiableUint32::GetUint32() const {
-  if (negotiated()) {
-    return negotiated_value_;
-  }
-  return default_value_;
-}
-
-// Returns the maximum value negotiable.
-uint32_t QuicNegotiableUint32::GetMax() const {
-  return max_value_;
-}
-
-void QuicNegotiableUint32::ToHandshakeMessage(
-    CryptoHandshakeMessage* out) const {
-  if (negotiated()) {
-    out->SetValue(tag_, negotiated_value_);
-  } else {
-    out->SetValue(tag_, max_value_);
-  }
-}
-
-QuicErrorCode QuicNegotiableUint32::ProcessPeerHello(
-    const CryptoHandshakeMessage& peer_hello,
-    HelloType hello_type,
-    std::string* error_details) {
-  DCHECK(!negotiated());
-  DCHECK(error_details != nullptr);
-  uint32_t value;
-  QuicErrorCode error = ReadUint32(peer_hello, tag_, presence_, default_value_,
-                                   &value, error_details);
-  if (error != QUIC_NO_ERROR) {
-    return error;
-  }
-  return ReceiveValue(value, hello_type, error_details);
-}
-
-QuicErrorCode QuicNegotiableUint32::ReceiveValue(uint32_t value,
-                                                 HelloType hello_type,
-                                                 std::string* error_details) {
-  if (hello_type == SERVER && value > max_value_) {
-    *error_details = "Invalid value received for " + QuicTagToString(tag_);
-    return QUIC_INVALID_NEGOTIATED_VALUE;
-  }
-
-  set_negotiated(true);
-  negotiated_value_ = std::min(value, max_value_);
-  return QUIC_NO_ERROR;
-}
-
 QuicFixedUint32::QuicFixedUint32(QuicTag tag, QuicConfigPresence presence)
     : QuicConfigValue(tag, presence),
       has_send_value_(false),
@@ -495,12 +427,13 @@
 }
 
 QuicConfig::QuicConfig()
-    : max_time_before_crypto_handshake_(QuicTime::Delta::Zero()),
+    : negotiated_(false),
+      max_time_before_crypto_handshake_(QuicTime::Delta::Zero()),
       max_idle_time_before_crypto_handshake_(QuicTime::Delta::Zero()),
       max_undecryptable_packets_(0),
       connection_options_(kCOPT, PRESENCE_OPTIONAL),
       client_connection_options_(kCLOP, PRESENCE_OPTIONAL),
-      idle_network_timeout_seconds_(kICSL, PRESENCE_REQUIRED),
+      idle_timeout_to_send_(QuicTime::Delta::Infinite()),
       max_bidirectional_streams_(kMIBS, PRESENCE_REQUIRED),
       max_unidirectional_streams_(kMIUS, PRESENCE_OPTIONAL),
       bytes_for_connection_id_(kTCID, PRESENCE_OPTIONAL),
@@ -604,14 +537,21 @@
 }
 
 void QuicConfig::SetIdleNetworkTimeout(QuicTime::Delta idle_network_timeout) {
-  idle_network_timeout_seconds_.set(
-      static_cast<uint32_t>(idle_network_timeout.ToSeconds()),
-      static_cast<uint32_t>(idle_network_timeout.ToSeconds()));
+  if (idle_network_timeout.ToMicroseconds() <= 0) {
+    QUIC_BUG << "Invalid idle network timeout " << idle_network_timeout;
+    return;
+  }
+  idle_timeout_to_send_ = idle_network_timeout;
 }
 
 QuicTime::Delta QuicConfig::IdleNetworkTimeout() const {
-  return QuicTime::Delta::FromSeconds(
-      idle_network_timeout_seconds_.GetUint32());
+  // TODO(b/152032210) add a QUIC_BUG to ensure that is not called before we've
+  // received the peer's values. This is true in production code but not in all
+  // of our tests that use a fake QuicConfig.
+  if (!received_idle_timeout_.has_value()) {
+    return idle_timeout_to_send_;
+  }
+  return received_idle_timeout_.value();
 }
 
 void QuicConfig::SetMaxBidirectionalStreamsToSend(uint32_t max_streams) {
@@ -925,9 +865,7 @@
 }
 
 bool QuicConfig::negotiated() const {
-  // TODO(ianswett): Add the negotiated parameters once and iterate over all
-  // of them in negotiated, ToHandshakeMessage, and ProcessPeerHello.
-  return idle_network_timeout_seconds_.negotiated();
+  return negotiated_;
 }
 
 void QuicConfig::SetCreateSessionTagIndicators(QuicTagVector tags) {
@@ -959,7 +897,20 @@
 void QuicConfig::ToHandshakeMessage(
     CryptoHandshakeMessage* out,
     QuicTransportVersion transport_version) const {
-  idle_network_timeout_seconds_.ToHandshakeMessage(out);
+  // Idle timeout has custom rules that are different from other values.
+  // We configure ourselves with the minumum value between the one sent and
+  // the one received. Additionally, when QUIC_CRYPTO is used, the server
+  // MUST send an idle timeout no greater than the idle timeout it received
+  // from the client. We therefore send the received value if it is lower.
+  QuicFixedUint32 idle_timeout_seconds(kICSL, PRESENCE_REQUIRED);
+  uint32_t idle_timeout_to_send_seconds = idle_timeout_to_send_.ToSeconds();
+  if (received_idle_timeout_.has_value() &&
+      received_idle_timeout_->ToSeconds() < idle_timeout_to_send_seconds) {
+    idle_timeout_to_send_seconds = received_idle_timeout_->ToSeconds();
+  }
+  idle_timeout_seconds.SetSendValue(idle_timeout_to_send_seconds);
+  idle_timeout_seconds.ToHandshakeMessage(out);
+
   // Do not need a version check here, max...bi... will encode
   // as "MIDS" -- the max initial dynamic streams tag -- if
   // doing some version other than IETF QUIC.
@@ -994,8 +945,29 @@
 
   QuicErrorCode error = QUIC_NO_ERROR;
   if (error == QUIC_NO_ERROR) {
-    error = idle_network_timeout_seconds_.ProcessPeerHello(
-        peer_hello, hello_type, error_details);
+    // Idle timeout has custom rules that are different from other values.
+    // We configure ourselves with the minumum value between the one sent and
+    // the one received. Additionally, when QUIC_CRYPTO is used, the server
+    // MUST send an idle timeout no greater than the idle timeout it received
+    // from the client.
+    QuicFixedUint32 idle_timeout_seconds(kICSL, PRESENCE_REQUIRED);
+    error = idle_timeout_seconds.ProcessPeerHello(peer_hello, hello_type,
+                                                  error_details);
+    if (error == QUIC_NO_ERROR) {
+      if (idle_timeout_seconds.GetReceivedValue() >
+          idle_timeout_to_send_.ToSeconds()) {
+        // The received value is higher than ours, ignore it if from the client
+        // and raise an error if from the server.
+        if (hello_type == SERVER) {
+          error = QUIC_INVALID_NEGOTIATED_VALUE;
+          *error_details =
+              "Invalid value received for " + QuicTagToString(kICSL);
+        }
+      } else {
+        received_idle_timeout_ = QuicTime::Delta::FromSeconds(
+            idle_timeout_seconds.GetReceivedValue());
+      }
+    }
   }
   if (error == QUIC_NO_ERROR) {
     error = max_bidirectional_streams_.ProcessPeerHello(peer_hello, hello_type,
@@ -1058,12 +1030,15 @@
     error = ack_delay_exponent_.ProcessPeerHello(peer_hello, hello_type,
                                                  error_details);
   }
+  if (error == QUIC_NO_ERROR) {
+    negotiated_ = true;
+  }
   return error;
 }
 
 bool QuicConfig::FillTransportParameters(TransportParameters* params) const {
   params->idle_timeout_milliseconds.set_value(
-      idle_network_timeout_seconds_.GetMax() * kNumMillisPerSecond);
+      idle_timeout_to_send_.ToMilliseconds());
 
   if (stateless_reset_token_.HasSendValue()) {
     QuicUint128 stateless_reset_token = stateless_reset_token_.GetSendValue();
@@ -1133,21 +1108,14 @@
     const TransportParameters& params,
     HelloType hello_type,
     std::string* error_details) {
-  // Intentionally round down to probe too often rather than not often enough.
-  uint64_t idle_timeout_seconds =
-      params.idle_timeout_milliseconds.value() / kNumMillisPerSecond;
-  // An idle timeout of zero indicates it is disabled (in other words, it is
-  // set to infinity). When the idle timeout is very high, we set it to our
-  // preferred maximum and still probe that often.
-  if (idle_timeout_seconds > idle_network_timeout_seconds_.GetMax() ||
-      idle_timeout_seconds == 0) {
-    idle_timeout_seconds = idle_network_timeout_seconds_.GetMax();
-  }
-  QuicErrorCode error = idle_network_timeout_seconds_.ReceiveValue(
-      idle_timeout_seconds, hello_type, error_details);
-  if (error != QUIC_NO_ERROR) {
-    DCHECK(!error_details->empty());
-    return error;
+  if (params.idle_timeout_milliseconds.value() > 0 &&
+      params.idle_timeout_milliseconds.value() <
+          static_cast<uint64_t>(idle_timeout_to_send_.ToMilliseconds())) {
+    // An idle timeout of zero indicates it is disabled.
+    // We also ignore values higher than ours which will cause us to use the
+    // smallest value between ours and our peer's.
+    received_idle_timeout_ = QuicTime::Delta::FromMilliseconds(
+        params.idle_timeout_milliseconds.value());
   }
 
   if (!params.stateless_reset_token.empty()) {
@@ -1222,7 +1190,7 @@
 
   const CryptoHandshakeMessage* peer_params = params.google_quic_params.get();
   if (peer_params != nullptr) {
-    error = initial_round_trip_time_us_.ProcessPeerHello(
+    QuicErrorCode error = initial_round_trip_time_us_.ProcessPeerHello(
         *peer_params, hello_type, error_details);
     if (error != QUIC_NO_ERROR) {
       DCHECK(!error_details->empty());
@@ -1238,6 +1206,7 @@
 
   received_custom_transport_parameters_ = params.custom_parameters;
 
+  negotiated_ = true;
   *error_details = "";
   return QUIC_NO_ERROR;
 }
diff --git a/quic/core/quic_config.h b/quic/core/quic_config.h
index a46af4d..a968369 100644
--- a/quic/core/quic_config.h
+++ b/quic/core/quic_config.h
@@ -14,6 +14,7 @@
 #include "net/third_party/quiche/src/quic/core/quic_time.h"
 #include "net/third_party/quiche/src/quic/platform/api/quic_export.h"
 #include "net/third_party/quiche/src/quic/platform/api/quic_uint128.h"
+#include "net/third_party/quiche/src/common/platform/api/quiche_optional.h"
 
 namespace quic {
 
@@ -62,67 +63,6 @@
   const QuicConfigPresence presence_;
 };
 
-class QUIC_EXPORT_PRIVATE QuicNegotiableValue : public QuicConfigValue {
- public:
-  QuicNegotiableValue(QuicTag tag, QuicConfigPresence presence);
-  ~QuicNegotiableValue() override;
-
-  bool negotiated() const { return negotiated_; }
-
- protected:
-  void set_negotiated(bool negotiated) { negotiated_ = negotiated; }
-
- private:
-  bool negotiated_;
-};
-
-class QUIC_EXPORT_PRIVATE QuicNegotiableUint32 : public QuicNegotiableValue {
-  // TODO(fayang): some negotiated values use uint32 as bool (e.g., silent
-  // close). Consider adding a QuicNegotiableBool type.
- public:
-  // Default and max values default to 0.
-  QuicNegotiableUint32(QuicTag name, QuicConfigPresence presence);
-  ~QuicNegotiableUint32() override;
-
-  // Sets the maximum possible value that can be achieved after negotiation and
-  // also the default values to be assumed if PRESENCE_OPTIONAL and the *HLO msg
-  // doesn't contain a value corresponding to |name_|. |max| is serialised via
-  // ToHandshakeMessage call if |negotiated_| is false.
-  void set(uint32_t max, uint32_t default_value);
-
-  // Returns the value negotiated if |negotiated_| is true, otherwise returns
-  // default_value_ (used to set default values before negotiation finishes).
-  uint32_t GetUint32() const;
-
-  // Returns the maximum value negotiable.
-  uint32_t GetMax() const;
-
-  // Serialises |name_| and value to |out|. If |negotiated_| is true then
-  // |negotiated_value_| is serialised, otherwise |max_value_| is serialised.
-  void ToHandshakeMessage(CryptoHandshakeMessage* out) const override;
-
-  // Processes the corresponding value from |peer_hello| and if present calls
-  // ReceiveValue with it. If the corresponding value is missing and
-  // PRESENCE_OPTIONAL then |negotiated_value_| is set to |default_value_|.
-  QuicErrorCode ProcessPeerHello(const CryptoHandshakeMessage& peer_hello,
-                                 HelloType hello_type,
-                                 std::string* error_details) override;
-
-  // Takes a value |value| parsed from a handshake message (whether a TLS
-  // ClientHello/ServerHello or a CryptoHandshakeMessage) whose sender was
-  // |hello_type|, and sets |negotiated_value_| to the minimum of |value| and
-  // |max_value_|. On success this function returns QUIC_NO_ERROR; if there is
-  // an error, details are put in |*error_details|.
-  QuicErrorCode ReceiveValue(uint32_t value,
-                             HelloType hello_type,
-                             std::string* error_details);
-
- private:
-  uint32_t max_value_;
-  uint32_t default_value_;
-  uint32_t negotiated_value_;
-};
-
 // Stores uint32_t from CHLO or SHLO messages that are not negotiated.
 class QUIC_EXPORT_PRIVATE QuicFixedUint32 : public QuicConfigValue {
  public:
@@ -540,6 +480,9 @@
   // SetDefaults sets the members to sensible, default values.
   void SetDefaults();
 
+  // Whether we've received the peer's config.
+  bool negotiated_;
+
   // Configurations options that are not negotiated.
   // Maximum time the session can be alive before crypto handshake is finished.
   QuicTime::Delta max_time_before_crypto_handshake_;
@@ -553,9 +496,12 @@
   QuicFixedTagVector connection_options_;
   // Connection options which only affect the client side.
   QuicFixedTagVector client_connection_options_;
-  // Idle network timeout in seconds.
+  // Idle network timeout.
   // Uses the max_idle_timeout transport parameter in IETF QUIC.
-  QuicNegotiableUint32 idle_network_timeout_seconds_;
+  // Note that received_idle_timeout_ is only populated if we receive the
+  // peer's value, which isn't guaranteed in IETF QUIC as sending is optional.
+  QuicTime::Delta idle_timeout_to_send_;
+  quiche::QuicheOptional<QuicTime::Delta> received_idle_timeout_;
   // Maximum number of dynamic streams that a Google QUIC connection
   // can support or the maximum number of bidirectional streams that
   // an IETF QUIC connection can support.
diff --git a/quic/core/quic_config_test.cc b/quic/core/quic_config_test.cc
index cb8d3be..8671d7e 100644
--- a/quic/core/quic_config_test.cc
+++ b/quic/core/quic_config_test.cc
@@ -24,15 +24,19 @@
 const uint32_t kMaxPacketSizeForTest = 1234;
 const uint32_t kMaxDatagramFrameSizeForTest = 1333;
 
-class QuicConfigTest : public QuicTestWithParam<QuicTransportVersion> {
+class QuicConfigTest : public QuicTestWithParam<ParsedQuicVersion> {
+ public:
+  QuicConfigTest() : version_(GetParam()) {}
+
  protected:
+  ParsedQuicVersion version_;
   QuicConfig config_;
 };
 
 // Run all tests with all versions of QUIC.
 INSTANTIATE_TEST_SUITE_P(QuicConfigTests,
                          QuicConfigTest,
-                         ::testing::ValuesIn(AllSupportedTransportVersions()),
+                         ::testing::ValuesIn(AllSupportedVersions()),
                          ::testing::PrintToStringParamName());
 
 TEST_P(QuicConfigTest, SetDefaults) {
@@ -85,13 +89,17 @@
 }
 
 TEST_P(QuicConfigTest, ToHandshakeMessage) {
+  if (version_.UsesTls()) {
+    // CryptoHandshakeMessage is only used for QUIC_CRYPTO.
+    return;
+  }
   config_.SetInitialStreamFlowControlWindowToSend(
       kInitialStreamFlowControlWindowForTest);
   config_.SetInitialSessionFlowControlWindowToSend(
       kInitialSessionFlowControlWindowForTest);
   config_.SetIdleNetworkTimeout(QuicTime::Delta::FromSeconds(5));
   CryptoHandshakeMessage msg;
-  config_.ToHandshakeMessage(&msg, GetParam());
+  config_.ToHandshakeMessage(&msg, version_.transport_version);
 
   uint32_t value;
   QuicErrorCode error = msg.GetUint32(kICSL, &value);
@@ -108,6 +116,10 @@
 }
 
 TEST_P(QuicConfigTest, ProcessClientHello) {
+  if (version_.UsesTls()) {
+    // CryptoHandshakeMessage is only used for QUIC_CRYPTO.
+    return;
+  }
   const uint32_t kTestMaxAckDelayMs =
       static_cast<uint32_t>(kDefaultDelayedAckTimeMs + 1);
   QuicConfig client_config;
@@ -125,7 +137,7 @@
   client_config.SetConnectionOptionsToSend(copt);
   client_config.SetMaxAckDelayToSendMs(kTestMaxAckDelayMs);
   CryptoHandshakeMessage msg;
-  client_config.ToHandshakeMessage(&msg, GetParam());
+  client_config.ToHandshakeMessage(&msg, version_.transport_version);
 
   std::string error_details;
   QuicTagVector initial_received_options;
@@ -169,6 +181,10 @@
 }
 
 TEST_P(QuicConfigTest, ProcessServerHello) {
+  if (version_.UsesTls()) {
+    // CryptoHandshakeMessage is only used for QUIC_CRYPTO.
+    return;
+  }
   QuicIpAddress host;
   host.FromString("127.0.3.1");
   const QuicSocketAddress kTestServerAddress = QuicSocketAddress(host, 1234);
@@ -189,7 +205,7 @@
   server_config.SetStatelessResetTokenToSend(kTestResetToken);
   server_config.SetMaxAckDelayToSendMs(kTestMaxAckDelayMs);
   CryptoHandshakeMessage msg;
-  server_config.ToHandshakeMessage(&msg, GetParam());
+  server_config.ToHandshakeMessage(&msg, version_.transport_version);
   std::string error_details;
   const QuicErrorCode error =
       config_.ProcessPeerHello(msg, SERVER, &error_details);
@@ -223,6 +239,10 @@
 }
 
 TEST_P(QuicConfigTest, MissingOptionalValuesInCHLO) {
+  if (version_.UsesTls()) {
+    // CryptoHandshakeMessage is only used for QUIC_CRYPTO.
+    return;
+  }
   CryptoHandshakeMessage msg;
   msg.SetValue(kICSL, 1);
 
@@ -239,6 +259,10 @@
 }
 
 TEST_P(QuicConfigTest, MissingOptionalValuesInSHLO) {
+  if (version_.UsesTls()) {
+    // CryptoHandshakeMessage is only used for QUIC_CRYPTO.
+    return;
+  }
   CryptoHandshakeMessage msg;
 
   // Set all REQUIRED tags.
@@ -254,6 +278,10 @@
 }
 
 TEST_P(QuicConfigTest, MissingValueInCHLO) {
+  if (version_.UsesTls()) {
+    // CryptoHandshakeMessage is only used for QUIC_CRYPTO.
+    return;
+  }
   // Server receives CHLO with missing kICSL.
   CryptoHandshakeMessage msg;
   std::string error_details;
@@ -263,6 +291,10 @@
 }
 
 TEST_P(QuicConfigTest, MissingValueInSHLO) {
+  if (version_.UsesTls()) {
+    // CryptoHandshakeMessage is only used for QUIC_CRYPTO.
+    return;
+  }
   // Client receives SHLO with missing kICSL.
   CryptoHandshakeMessage msg;
   std::string error_details;
@@ -272,12 +304,16 @@
 }
 
 TEST_P(QuicConfigTest, OutOfBoundSHLO) {
+  if (version_.UsesTls()) {
+    // CryptoHandshakeMessage is only used for QUIC_CRYPTO.
+    return;
+  }
   QuicConfig server_config;
   server_config.SetIdleNetworkTimeout(
       QuicTime::Delta::FromSeconds(2 * kMaximumIdleTimeoutSecs));
 
   CryptoHandshakeMessage msg;
-  server_config.ToHandshakeMessage(&msg, GetParam());
+  server_config.ToHandshakeMessage(&msg, version_.transport_version);
   std::string error_details;
   const QuicErrorCode error =
       config_.ProcessPeerHello(msg, SERVER, &error_details);
@@ -298,6 +334,10 @@
 }
 
 TEST_P(QuicConfigTest, HasClientSentConnectionOption) {
+  if (version_.UsesTls()) {
+    // CryptoHandshakeMessage is only used for QUIC_CRYPTO.
+    return;
+  }
   QuicConfig client_config;
   QuicTagVector copt;
   copt.push_back(kTBBR);
@@ -306,7 +346,7 @@
       kTBBR, Perspective::IS_CLIENT));
 
   CryptoHandshakeMessage msg;
-  client_config.ToHandshakeMessage(&msg, GetParam());
+  client_config.ToHandshakeMessage(&msg, version_.transport_version);
 
   std::string error_details;
   const QuicErrorCode error =
@@ -321,13 +361,17 @@
 }
 
 TEST_P(QuicConfigTest, DontSendClientConnectionOptions) {
+  if (version_.UsesTls()) {
+    // CryptoHandshakeMessage is only used for QUIC_CRYPTO.
+    return;
+  }
   QuicConfig client_config;
   QuicTagVector copt;
   copt.push_back(kTBBR);
   client_config.SetClientConnectionOptions(copt);
 
   CryptoHandshakeMessage msg;
-  client_config.ToHandshakeMessage(&msg, GetParam());
+  client_config.ToHandshakeMessage(&msg, version_.transport_version);
 
   std::string error_details;
   const QuicErrorCode error =
@@ -339,6 +383,10 @@
 }
 
 TEST_P(QuicConfigTest, HasClientRequestedIndependentOption) {
+  if (version_.UsesTls()) {
+    // CryptoHandshakeMessage is only used for QUIC_CRYPTO.
+    return;
+  }
   QuicConfig client_config;
   QuicTagVector client_opt;
   client_opt.push_back(kRENO);
@@ -354,7 +402,7 @@
       kTBBR, Perspective::IS_CLIENT));
 
   CryptoHandshakeMessage msg;
-  client_config.ToHandshakeMessage(&msg, GetParam());
+  client_config.ToHandshakeMessage(&msg, version_.transport_version);
 
   std::string error_details;
   const QuicErrorCode error =
@@ -371,6 +419,10 @@
 }
 
 TEST_P(QuicConfigTest, IncomingLargeIdleTimeoutTransportParameter) {
+  if (!version_.UsesTls()) {
+    // TransportParameters are only used for QUIC+TLS.
+    return;
+  }
   // Configure our idle timeout to 60s, then receive 120s from peer.
   // Since the received value is above ours, we should then use ours.
   config_.SetIdleNetworkTimeout(quic::QuicTime::Delta::FromSeconds(60));
@@ -387,6 +439,10 @@
 }
 
 TEST_P(QuicConfigTest, FillTransportParams) {
+  if (!version_.UsesTls()) {
+    // TransportParameters are only used for QUIC+TLS.
+    return;
+  }
   config_.SetInitialMaxStreamDataBytesIncomingBidirectionalToSend(
       2 * kMinimumFlowControlSendWindow);
   config_.SetInitialMaxStreamDataBytesOutgoingBidirectionalToSend(
@@ -415,6 +471,10 @@
 }
 
 TEST_P(QuicConfigTest, ProcessTransportParametersServer) {
+  if (!version_.UsesTls()) {
+    // TransportParameters are only used for QUIC+TLS.
+    return;
+  }
   TransportParameters params;
 
   params.initial_max_stream_data_bidi_local.set_value(
@@ -509,6 +569,10 @@
 }
 
 TEST_P(QuicConfigTest, DisableMigrationTransportParameter) {
+  if (!version_.UsesTls()) {
+    // TransportParameters are only used for QUIC+TLS.
+    return;
+  }
   TransportParameters params;
   params.disable_migration = true;
   std::string error_details;
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index 8b6f752..613358a 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -4251,8 +4251,7 @@
   QuicTagVector options;
   options.push_back(kTLPR);
   config.SetConnectionOptionsToSend(options);
-  QuicConfigPeer::ReceiveIdleNetworkTimeout(
-      &config, SERVER, QuicTime::Delta::FromSeconds(kMaximumIdleTimeoutSecs));
+  QuicConfigPeer::SetNegotiated(&config, true);
   connection_.SetFromConfig(config);
   connection_.SetMaxTailLossProbes(1);
 
@@ -6099,8 +6098,7 @@
   QuicTagVector connection_options;
   connection_options.push_back(k5RTO);
   config.SetConnectionOptionsToSend(connection_options);
-  QuicConfigPeer::ReceiveIdleNetworkTimeout(
-      &config, SERVER, QuicTime::Delta::FromSeconds(kMaximumIdleTimeoutSecs));
+  QuicConfigPeer::SetNegotiated(&config, true);
   connection_.SetFromConfig(config);
 
   // Send stream data.
@@ -9871,8 +9869,7 @@
   connection_options.push_back(k1PTO);
   connection_options.push_back(k6PTO);
   config.SetConnectionOptionsToSend(connection_options);
-  QuicConfigPeer::ReceiveIdleNetworkTimeout(
-      &config, SERVER, QuicTime::Delta::FromSeconds(kMaximumIdleTimeoutSecs));
+  QuicConfigPeer::SetNegotiated(&config, true);
   EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _));
   connection_.SetFromConfig(config);
   EXPECT_FALSE(connection_.GetRetransmissionAlarm()->IsSet());
@@ -9919,8 +9916,7 @@
   connection_options.push_back(k2PTO);
   connection_options.push_back(k7PTO);
   config.SetConnectionOptionsToSend(connection_options);
-  QuicConfigPeer::ReceiveIdleNetworkTimeout(
-      &config, SERVER, QuicTime::Delta::FromSeconds(kMaximumIdleTimeoutSecs));
+  QuicConfigPeer::SetNegotiated(&config, true);
   EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _));
   connection_.SetFromConfig(config);
   EXPECT_FALSE(connection_.GetRetransmissionAlarm()->IsSet());
@@ -9965,8 +9961,7 @@
   QuicTagVector connection_options;
   connection_options.push_back(k2PTO);
   connection_options.push_back(k8PTO);
-  QuicConfigPeer::ReceiveIdleNetworkTimeout(
-      &config, SERVER, QuicTime::Delta::FromSeconds(kMaximumIdleTimeoutSecs));
+  QuicConfigPeer::SetNegotiated(&config, true);
   config.SetConnectionOptionsToSend(connection_options);
   EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _));
   connection_.SetFromConfig(config);