commit f67b5c59444a44f5f6856c44e6461a4b26deeb23
author dschinazi <dschinazi@google.com> Mon May 18 15:18:14 2020 -0700
committer Copybara-Service <copybara-worker@google.com> Mon May 18 15:18:51 2020 -0700
tree ced9d724c223fb715901b797facfdf8b187ced0b
parent a26a4615dc37ee038c862627561e9df565bf70fd

Send correct max_ack_delay in QUIC+TLS

This CL fixes a bug where QuicConfig was sending the default value for max_ack_delay instead of the configured vale, when using QUIC+TLS. The only code that sets a non-default value here is either tests or quartc. Quartc is client-only, therefore we do not need flag-protection for this change.

Client-only change

PiperOrigin-RevId: 312164256
Change-Id: Ida7203ca219707d8ac44664c3a3326c215114f1e

quic/core/http/end_to_end_test.cc

diff --git a/quic/core/http/end_to_end_test.cc b/quic/core/http/end_to_end_test.cc
index eb89efa..75e6f44 100644
--- a/quic/core/http/end_to_end_test.cc
+++ b/quic/core/http/end_to_end_test.cc
@@ -599,14 +599,9 @@
 // Simple transaction, but set a non-default ack delay at the client
 // and ensure it gets to the server.
 TEST_P(EndToEndTest, SimpleRequestResponseWithAckDelayChange) {
-  if (version_.UsesTls()) {
-    // TODO(b/155316241): Enable this test for TLS.
-    Initialize();
-    return;
-  }
   // Force the ACK delay to be something other than the default.
-  // Note that it is sent only if doing IETF QUIC.
-  client_config_.SetMaxAckDelayToSendMs(kDefaultDelayedAckTimeMs + 100u);
+  constexpr uint32_t kClientMaxAckDelay = kDefaultDelayedAckTimeMs + 100u;
+  client_config_.SetMaxAckDelayToSendMs(kClientMaxAckDelay);
   ASSERT_TRUE(Initialize());
 
   EXPECT_EQ(kFooResponseBody, client_->SendSynchronousRequest("/foo"));
@@ -614,10 +609,9 @@
   EXPECT_FALSE(client_->client()->EarlyDataAccepted());
   EXPECT_FALSE(client_->client()->ReceivedInchoateReject());
   if (GetQuicReloadableFlag(quic_negotiate_ack_delay_time)) {
-    EXPECT_EQ(kDefaultDelayedAckTimeMs + 100u,
-              GetSentPacketManagerFromFirstServerSession()
-                  ->peer_max_ack_delay()
-                  .ToMilliseconds());
+    EXPECT_EQ(kClientMaxAckDelay, GetSentPacketManagerFromFirstServerSession()
+                                      ->peer_max_ack_delay()
+                                      .ToMilliseconds());
   } else {
     EXPECT_EQ(kDefaultDelayedAckTimeMs,
               GetSentPacketManagerFromFirstServerSession()

quic/core/quic_config.cc

diff --git a/quic/core/quic_config.cc b/quic/core/quic_config.cc
index 7c781f4..8cdc121 100644
--- a/quic/core/quic_config.cc
+++ b/quic/core/quic_config.cc
@@ -592,7 +592,7 @@
   return max_ack_delay_ms_.SetSendValue(max_ack_delay_ms);
 }
 
-uint32_t QuicConfig::GetMaxAckDelayToToSendMs() const {
+uint32_t QuicConfig::GetMaxAckDelayToSendMs() const {
   return max_ack_delay_ms_.GetSendValue();
 }
 
@@ -1110,7 +1110,7 @@
       GetMaxUnidirectionalStreamsToSend());
   if (GetQuicReloadableFlag(quic_negotiate_ack_delay_time)) {
     QUIC_RELOADABLE_FLAG_COUNT_N(quic_negotiate_ack_delay_time, 3, 4);
-    params->max_ack_delay.set_value(kDefaultDelayedAckTimeMs);
+    params->max_ack_delay.set_value(GetMaxAckDelayToSendMs());
   }
   params->ack_delay_exponent.set_value(GetAckDelayExponentToSend());
   params->disable_migration =

quic/core/quic_config.h

diff --git a/quic/core/quic_config.h b/quic/core/quic_config.h
index 4d2bacc..6809c28 100644
--- a/quic/core/quic_config.h
+++ b/quic/core/quic_config.h
@@ -411,7 +411,7 @@
   // The received delay is the value received from
   // the peer (QuicSentPacketManager::peer_max_ack_delay_).
   void SetMaxAckDelayToSendMs(uint32_t max_ack_delay_ms);
-  uint32_t GetMaxAckDelayToToSendMs() const;
+  uint32_t GetMaxAckDelayToSendMs() const;
   bool HasReceivedMaxAckDelayMs() const;
   uint32_t ReceivedMaxAckDelayMs() const;