gfe-relnote: For server side QUIC connection, mark version_negotiated_ earlier. Protected by --gfe2_reloadable_flag_quic_version_negotiated_by_default_at_server.

PiperOrigin-RevId: 282365816
Change-Id: I9083e05d758f564befaab126e427f7ab434d2f6f

quic/core/http/quic_server_session_base_test.cc

diff --git a/quic/core/http/quic_server_session_base_test.cc b/quic/core/http/quic_server_session_base_test.cc
index cd45c50..9e8c749 100644
--- a/quic/core/http/quic_server_session_base_test.cc
+++ b/quic/core/http/quic_server_session_base_test.cc
@@ -152,8 +152,10 @@
         QuicCryptoServerConfig::ConfigOptions());
     SetQuicReloadableFlag(quic_supports_tls_handshake, true);
     session_->Initialize();
-    QuicSessionPeer::GetMutableCryptoStream(session_.get())
-        ->OnSuccessfulVersionNegotiation(supported_versions.front());
+    if (!GetQuicReloadableFlag(quic_version_negotiated_by_default_at_server)) {
+      QuicSessionPeer::GetMutableCryptoStream(session_.get())
+          ->OnSuccessfulVersionNegotiation(supported_versions.front());
+    }
     QuicConfigPeer::SetReceivedInitialSessionFlowControlWindow(
         session_->config(), kMinimumFlowControlSendWindow);
     session_->OnConfigNegotiated();

quic/core/quic_connection.cc

diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index c457ca4..a4efa01 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -336,7 +336,9 @@
       treat_queued_packets_as_sent_(
           GetQuicReloadableFlag(quic_treat_queued_packets_as_sent) ||
           version().CanSendCoalescedPackets()),
-      mtu_discovery_v2_(GetQuicReloadableFlag(quic_mtu_discovery_v2)) {
+      mtu_discovery_v2_(GetQuicReloadableFlag(quic_mtu_discovery_v2)),
+      quic_version_negotiated_by_default_at_server_(
+          GetQuicReloadableFlag(quic_version_negotiated_by_default_at_server)) {
   QUIC_DLOG(INFO) << ENDPOINT << "Created connection with server connection ID "
                   << server_connection_id
                   << " and version: " << ParsedQuicVersionToString(version());
@@ -370,6 +372,13 @@
   DCHECK(perspective_ == Perspective::IS_CLIENT ||
          supported_versions.size() == 1);
   InstallInitialCrypters(server_connection_id_);
+
+  if (quic_version_negotiated_by_default_at_server() &&
+      perspective_ == Perspective::IS_SERVER) {
+    QUIC_RELOADABLE_FLAG_COUNT(quic_version_negotiated_by_default_at_server);
+    version_negotiated_ = true;
+    framer_.InferPacketHeaderTypeFromVersion();
+  }
 }
 
 void QuicConnection::InstallInitialCrypters(QuicConnectionId connection_id) {
@@ -759,32 +768,41 @@
     return false;
   }
 
-  if (!version_negotiated_ && perspective_ == Perspective::IS_SERVER) {
-    if (!header.version_flag) {
-      // Packets should have the version flag till version negotiation is
-      // done.
-      std::string error_details =
-          QuicStrCat(ENDPOINT, "Packet ", header.packet_number.ToUint64(),
-                     " without version flag before version negotiated.");
-      QUIC_DLOG(WARNING) << error_details;
-      CloseConnection(QUIC_INVALID_VERSION, error_details,
-                      ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
-      return false;
-    } else {
-      DCHECK_EQ(header.version, version());
-      version_negotiated_ = true;
-      framer_.InferPacketHeaderTypeFromVersion();
-      visitor_->OnSuccessfulVersionNegotiation(version());
-      if (debug_visitor_ != nullptr) {
-        debug_visitor_->OnSuccessfulVersionNegotiation(version());
+  if (!quic_version_negotiated_by_default_at_server()) {
+    if (!version_negotiated_ && perspective_ == Perspective::IS_SERVER) {
+      if (!header.version_flag) {
+        // Packets should have the version flag till version negotiation is
+        // done.
+        std::string error_details =
+            QuicStrCat(ENDPOINT, "Packet ", header.packet_number.ToUint64(),
+                       " without version flag before version negotiated.");
+        QUIC_DLOG(WARNING) << error_details;
+        CloseConnection(QUIC_INVALID_VERSION, error_details,
+                        ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
+        return false;
+      } else {
+        DCHECK_EQ(header.version, version());
+        version_negotiated_ = true;
+        framer_.InferPacketHeaderTypeFromVersion();
+        visitor_->OnSuccessfulVersionNegotiation(version());
+        if (debug_visitor_ != nullptr) {
+          debug_visitor_->OnSuccessfulVersionNegotiation(version());
+        }
       }
+      DCHECK(version_negotiated_);
     }
-    DCHECK(version_negotiated_);
   }
 
   return true;
 }
 
+void QuicConnection::OnSuccessfulVersionNegotiation() {
+  visitor_->OnSuccessfulVersionNegotiation(version());
+  if (debug_visitor_ != nullptr) {
+    debug_visitor_->OnSuccessfulVersionNegotiation(version());
+  }
+}
+
 void QuicConnection::OnDecryptedPacket(EncryptionLevel level) {
   last_decrypted_packet_level_ = level;
   last_packet_decrypted_ = true;
@@ -1959,10 +1977,7 @@
         packet_creator_.StopSendingVersion();
       }
       version_negotiated_ = true;
-      visitor_->OnSuccessfulVersionNegotiation(version());
-      if (debug_visitor_ != nullptr) {
-        debug_visitor_->OnSuccessfulVersionNegotiation(version());
-      }
+      OnSuccessfulVersionNegotiation();
     }
   }
 

quic/core/quic_connection.h

diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h
index de26b3c..55173cc 100644
--- a/quic/core/quic_connection.h
+++ b/quic/core/quic_connection.h
@@ -895,6 +895,13 @@
     return treat_queued_packets_as_sent_;
   }
 
+  // Called when version is considered negotiated.
+  void OnSuccessfulVersionNegotiation();
+
+  bool quic_version_negotiated_by_default_at_server() const {
+    return quic_version_negotiated_by_default_at_server_;
+  }
+
  protected:
   // Calls cancel() on all the alarms owned by this connection.
   void CancelAllAlarms();
@@ -1373,6 +1380,7 @@
   QuicSentPacketManager sent_packet_manager_;
 
   // Indicates whether connection version has been negotiated.
+  // Always true for server connections.
   bool version_negotiated_;
 
   // Tracks if the connection was created by the server or the client.
@@ -1524,6 +1532,9 @@
   const bool mtu_discovery_v2_;
   // Only used if quic_mtu_discovery_v2 is true.
   QuicConnectionMtuDiscoverer mtu_discoverer_;
+
+  // Latched value of quic_version_negotiated_by_default_at_server.
+  const bool quic_version_negotiated_by_default_at_server_;
 };
 
 }  // namespace quic

quic/core/quic_connection_test.cc

diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index 1fbd121..111f94f 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -1047,6 +1047,7 @@
     EXPECT_CALL(visitor_, OnCongestionWindowChange(_)).Times(AnyNumber());
     EXPECT_CALL(visitor_, OnPacketReceived(_, _, _)).Times(AnyNumber());
     EXPECT_CALL(visitor_, OnForwardProgressConfirmed()).Times(AnyNumber());
+    EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)).Times(AnyNumber());
 
     EXPECT_CALL(*loss_algorithm_, GetLossTimeout())
         .WillRepeatedly(Return(QuicTime::Zero()));
@@ -1551,6 +1552,10 @@
     connection_.set_perspective(perspective);
     if (perspective == Perspective::IS_SERVER) {
       connection_.set_can_truncate_connection_ids(true);
+      QuicConnectionPeer::SetNegotiatedVersion(&connection_);
+      if (GetQuicReloadableFlag(quic_version_negotiated_by_default_at_server)) {
+        connection_.OnSuccessfulVersionNegotiation();
+      }
     }
     QuicFramerPeer::SetPerspective(&peer_framer_,
                                    QuicUtils::InvertPerspective(perspective));
@@ -1613,7 +1618,6 @@
 
   void MtuDiscoveryTestInit() {
     set_perspective(Perspective::IS_SERVER);
-    QuicConnectionPeer::SetNegotiatedVersion(&connection_);
     QuicPacketCreatorPeer::SetSendVersionInPacket(creator_, false);
     connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
     peer_creator_.set_encryption_level(ENCRYPTION_FORWARD_SECURE);
@@ -1729,8 +1733,6 @@
 }
 
 TEST_P(QuicConnectionTest, SelfAddressChangeAtServer) {
-  EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
-
   set_perspective(Perspective::IS_SERVER);
   QuicPacketCreatorPeer::SetSendVersionInPacket(creator_, false);
 
@@ -1760,8 +1762,6 @@
 }
 
 TEST_P(QuicConnectionTest, AllowSelfAddressChangeToMappedIpv4AddressAtServer) {
-  EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
-
   set_perspective(Perspective::IS_SERVER);
   QuicPacketCreatorPeer::SetSendVersionInPacket(creator_, false);
 
@@ -1795,7 +1795,6 @@
 }
 
 TEST_P(QuicConnectionTest, ClientAddressChangeAndPacketReordered) {
-  EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
   set_perspective(Perspective::IS_SERVER);
   QuicPacketCreatorPeer::SetSendVersionInPacket(creator_, false);
 
@@ -1834,7 +1833,6 @@
 }
 
 TEST_P(QuicConnectionTest, PeerAddressChangeAtServer) {
-  EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
   set_perspective(Perspective::IS_SERVER);
   QuicPacketCreatorPeer::SetSendVersionInPacket(creator_, false);
   EXPECT_EQ(Perspective::IS_SERVER, connection_.perspective());
@@ -1872,7 +1870,6 @@
 }
 
 TEST_P(QuicConnectionTest, EffectivePeerAddressChangeAtServer) {
-  EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
   set_perspective(Perspective::IS_SERVER);
   QuicPacketCreatorPeer::SetSendVersionInPacket(creator_, false);
   EXPECT_EQ(Perspective::IS_SERVER, connection_.perspective());
@@ -1958,7 +1955,6 @@
 }
 
 TEST_P(QuicConnectionTest, ReceivePaddedPingAtServer) {
-  EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
   set_perspective(Perspective::IS_SERVER);
   QuicPacketCreatorPeer::SetSendVersionInPacket(creator_, false);
   EXPECT_EQ(Perspective::IS_SERVER, connection_.perspective());
@@ -2083,7 +2079,6 @@
 }
 
 TEST_P(QuicConnectionTest, ReceiveConnectivityProbingAtServer) {
-  EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
   set_perspective(Perspective::IS_SERVER);
   QuicPacketCreatorPeer::SetSendVersionInPacket(creator_, false);
   EXPECT_EQ(Perspective::IS_SERVER, connection_.perspective());
@@ -2142,7 +2137,6 @@
 }
 
 TEST_P(QuicConnectionTest, ReceiveReorderedConnectivityProbingAtServer) {
-  EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
   set_perspective(Perspective::IS_SERVER);
   QuicPacketCreatorPeer::SetSendVersionInPacket(creator_, false);
   EXPECT_EQ(Perspective::IS_SERVER, connection_.perspective());
@@ -2199,7 +2193,6 @@
 }
 
 TEST_P(QuicConnectionTest, MigrateAfterProbingAtServer) {
-  EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
   set_perspective(Perspective::IS_SERVER);
   QuicPacketCreatorPeer::SetSendVersionInPacket(creator_, false);
   EXPECT_EQ(Perspective::IS_SERVER, connection_.perspective());
@@ -2400,8 +2393,6 @@
 }
 
 TEST_P(QuicConnectionTest, IncreaseServerMaxPacketSize) {
-  EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
-
   set_perspective(Perspective::IS_SERVER);
   connection_.SetMaxPacketLength(1000);
 
@@ -2446,8 +2437,6 @@
 }
 
 TEST_P(QuicConnectionTest, IncreaseServerMaxPacketSizeWhileWriterLimited) {
-  EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
-
   const QuicByteCount lower_max_packet_size = 1240;
   writer_->set_max_packet_size(lower_max_packet_size);
   set_perspective(Perspective::IS_SERVER);
@@ -7870,7 +7859,6 @@
 
   // Ack data.
   clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(5));
-  EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
   EXPECT_CALL(*send_algorithm_, OnCongestionEvent(true, _, _, _, _));
   QuicAckFrame frame =
       InitAckFrame({{QuicPacketNumber(1u), QuicPacketNumber(2u)}});
@@ -7905,8 +7893,6 @@
 }
 
 TEST_P(QuicConnectionTest, ServerReceivesChloOnNonCryptoStream) {
-  EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
-
   set_perspective(Perspective::IS_SERVER);
   QuicPacketCreatorPeer::SetSendVersionInPacket(creator_, false);
 
@@ -8855,7 +8841,6 @@
         0u, QuicStringPiece()));
     EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(1);
   }
-  EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
   ProcessFramePacketWithAddresses(frame, kSelfAddress, kPeerAddress);
 
   // Let connection process a Google QUIC packet.
@@ -9128,7 +9113,6 @@
   if (!framer_.version().SupportsClientConnectionIds()) {
     return;
   }
-  EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
   set_perspective(Perspective::IS_SERVER);
   QuicPacketHeader header = ConstructPacketHeader(1, ENCRYPTION_INITIAL);
   header.source_connection_id = TestConnectionId(0x33);
@@ -9569,7 +9553,6 @@
   if (!connection_.version().SupportsAntiAmplificationLimit()) {
     return;
   }
-  EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
   EXPECT_CALL(visitor_, OnCryptoFrame(_)).Times(AnyNumber());
 
   set_perspective(Perspective::IS_SERVER);

quic/core/quic_crypto_server_stream_test.cc

diff --git a/quic/core/quic_crypto_server_stream_test.cc b/quic/core/quic_crypto_server_stream_test.cc
index 360c268..2f7055d 100644
--- a/quic/core/quic_crypto_server_stream_test.cc
+++ b/quic/core/quic_crypto_server_stream_test.cc
@@ -100,8 +100,10 @@
     crypto_test_utils::SetupCryptoServerConfigForTest(
         server_connection_->clock(), server_connection_->random_generator(),
         &server_crypto_config_);
-    server_session_->GetMutableCryptoStream()->OnSuccessfulVersionNegotiation(
-        supported_versions_.front());
+    if (!GetQuicReloadableFlag(quic_version_negotiated_by_default_at_server)) {
+      server_session_->GetMutableCryptoStream()->OnSuccessfulVersionNegotiation(
+          supported_versions_.front());
+    }
   }
 
   QuicCryptoServerStream* server_stream() {

quic/core/quic_session.cc

diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc
index cba27f0..2870b37 100644
--- a/quic/core/quic_session.cc
+++ b/quic/core/quic_session.cc
@@ -114,6 +114,13 @@
   connection_->SetDataProducer(this);
   connection_->SetFromConfig(config_);
 
+  // On the server side, version negotiation has been done by the dispatcher,
+  // and the server session is created with the right version.
+  if (connection_->quic_version_negotiated_by_default_at_server() &&
+      perspective() == Perspective::IS_SERVER) {
+    connection_->OnSuccessfulVersionNegotiation();
+  }
+
   if (QuicVersionUsesCryptoFrames(transport_version())) {
     return;
   }

quic/quic_transport/quic_transport_server_session_test.cc

diff --git a/quic/quic_transport/quic_transport_server_session_test.cc b/quic/quic_transport/quic_transport_server_session_test.cc
index 42c3eef..5e67748 100644
--- a/quic/quic_transport/quic_transport_server_session_test.cc
+++ b/quic/quic_transport/quic_transport_server_session_test.cc
@@ -72,7 +72,9 @@
     session_->Initialize();
     crypto_stream_ = static_cast<QuicCryptoServerStream*>(
         session_->GetMutableCryptoStream());
-    crypto_stream_->OnSuccessfulVersionNegotiation(GetVersions()[0]);
+    if (!GetQuicReloadableFlag(quic_version_negotiated_by_default_at_server)) {
+      crypto_stream_->OnSuccessfulVersionNegotiation(GetVersions()[0]);
+    }
   }
 
   void Connect() {

quic/test_tools/crypto_test_utils.cc

diff --git a/quic/test_tools/crypto_test_utils.cc b/quic/test_tools/crypto_test_utils.cc
index 83efd21..6716dec 100644
--- a/quic/test_tools/crypto_test_utils.cc
+++ b/quic/test_tools/crypto_test_utils.cc
@@ -235,8 +235,10 @@
       server_conn, *server_quic_config, client_conn->supported_versions(),
       crypto_config, &compressed_certs_cache);
   server_session.Initialize();
-  server_session.OnSuccessfulVersionNegotiation(
-      client_conn->supported_versions().front());
+  if (!GetQuicReloadableFlag(quic_version_negotiated_by_default_at_server)) {
+    server_session.OnSuccessfulVersionNegotiation(
+        client_conn->supported_versions().front());
+  }
   EXPECT_CALL(*server_session.helper(),
               CanAcceptClientHello(testing::_, testing::_, testing::_,
                                    testing::_, testing::_))

quic/test_tools/quic_connection_peer.cc

diff --git a/quic/test_tools/quic_connection_peer.cc b/quic/test_tools/quic_connection_peer.cc
index 197514e..d48ff05 100644
--- a/quic/test_tools/quic_connection_peer.cc
+++ b/quic/test_tools/quic_connection_peer.cc
@@ -296,6 +296,11 @@
 // static
 void QuicConnectionPeer::SetNegotiatedVersion(QuicConnection* connection) {
   connection->version_negotiated_ = true;
+  if (connection->perspective() == Perspective::IS_SERVER &&
+      !QuicFramerPeer::infer_packet_header_type_from_version(
+          &connection->framer_)) {
+    connection->framer_.InferPacketHeaderTypeFromVersion();
+  }
 }
 
 // static

quic/test_tools/quic_framer_peer.h

diff --git a/quic/test_tools/quic_framer_peer.h b/quic/test_tools/quic_framer_peer.h
index 661def5..d462555 100644
--- a/quic/test_tools/quic_framer_peer.h
+++ b/quic/test_tools/quic_framer_peer.h
@@ -187,6 +187,10 @@
       uint64_t current_received_frame_type) {
     framer->current_received_frame_type_ = current_received_frame_type;
   }
+
+  static bool infer_packet_header_type_from_version(QuicFramer* framer) {
+    return framer->infer_packet_header_type_from_version_;
+  }
 };
 
 }  // namespace test

quic/tools/quic_simple_server_session_test.cc

diff --git a/quic/tools/quic_simple_server_session_test.cc b/quic/tools/quic_simple_server_session_test.cc
index 4a12c09..a548c6c 100644
--- a/quic/tools/quic_simple_server_session_test.cc
+++ b/quic/tools/quic_simple_server_session_test.cc
@@ -230,8 +230,10 @@
         QuicRandom::GetInstance(), &clock,
         QuicCryptoServerConfig::ConfigOptions());
     session_->Initialize();
-    QuicSessionPeer::GetMutableCryptoStream(session_.get())
-        ->OnSuccessfulVersionNegotiation(supported_versions.front());
+    if (!GetQuicReloadableFlag(quic_version_negotiated_by_default_at_server)) {
+      QuicSessionPeer::GetMutableCryptoStream(session_.get())
+          ->OnSuccessfulVersionNegotiation(supported_versions.front());
+    }
 
     if (VersionHasIetfQuicFrames(transport_version())) {
       EXPECT_CALL(*connection_, SendControlFrame(_))
@@ -590,8 +592,10 @@
         config_, connection_, &owner_, &stream_helper_, &crypto_config_,
         &compressed_certs_cache_, &memory_cache_backend_);
     session_->Initialize();
-    QuicSessionPeer::GetMutableCryptoStream(session_.get())
-        ->OnSuccessfulVersionNegotiation(supported_versions.front());
+    if (!GetQuicReloadableFlag(quic_version_negotiated_by_default_at_server)) {
+      QuicSessionPeer::GetMutableCryptoStream(session_.get())
+          ->OnSuccessfulVersionNegotiation(supported_versions.front());
+    }
     // Needed to make new session flow control window and server push work.
 
     if (VersionHasIetfQuicFrames(transport_version())) {