Fix sending MAX_PUSH_ID frame during 0-RTT. Assume SetMaxPushId() is called with some initial value. During 0-RTT, SendInitialData() is called, it sends a MAX_PUSH_ID and sets |http3_max_push_id_sent_|. If SetMaxPushId() is called with a different value after this but before 1-RTT keys are available, it does not send the MAX_PUSH_ID frame with the updated value because OneRttKeysAvailable() is false. Then SendInitialData() is called when 1-RTT keys are available, but it does not send the updated MAX_PUSH_ID frame either, because |http3_max_push_id_sent_| is true. This CL fixes that by gating on IsEncryptionEstablished() instead of OneRttKeysAvailable() in SetMaxPushId(). However, it introduces a new bug: Assume there is no initial SetMaxPushId() call. During 0-RTT, SendInitialData() is called, but it does not send a MAX_PUSH_ID frame because |max_push_id_| is not set. Then SetMaxPushId() is called before 1-RTT keys are available, and it now sends the MAX_PUSH_ID frame because IsEncryptionEstablished() is true. Then SendInitialData() is called when 1-RTT keys are available. It should not send another MAX_PUSH_ID frame with the same value. In order to achieve this, |http3_max_push_id_sent_| must be set to true when the MAX_PUSH_ID frame is sent by SetMaxPushId(). This is done by setting |http3_max_push_id_sent_| in SendMaxPushId() instead of SendInitialData(). Client-only change. PiperOrigin-RevId: 315734365 Change-Id: Ib4dc2f96624b1c8c9a27916aa4a6ab98fbb71494

diff --git a/quic/core/http/quic_spdy_client_session_test.cc b/quic/core/http/quic_spdy_client_session_test.cc
index ca9b1d5..fc6343e 100644
--- a/quic/core/http/quic_spdy_client_session_test.cc
+++ b/quic/core/http/quic_spdy_client_session_test.cc

@@ -43,6 +43,7 @@
 using ::testing::AtLeast;
 using ::testing::AtMost;
 using ::testing::Invoke;
+using ::testing::StrictMock;
 using ::testing::Truly;
 
 namespace quic {
@@ -1234,6 +1235,119 @@
       connection_, crypto_stream_, AlpnForVersion(connection_->version()));
 }
 
+TEST_P(QuicSpdyClientSessionTest, SetMaxPushIdBeforeEncryptionEstablished) {
+  // 0-RTT is TLS-only, MAX_PUSH_ID frame is HTTP/3-only.
+  if (!session_->version().UsesTls() || !session_->version().UsesHttp3()) {
+    return;
+  }
+
+  CompleteFirstConnection();
+
+  CreateConnection();
+  StrictMock<MockHttp3DebugVisitor> debug_visitor;
+  session_->set_debug_visitor(&debug_visitor);
+
+  // No MAX_PUSH_ID frame is sent before encryption is established.
+  session_->SetMaxPushId(5);
+
+  EXPECT_FALSE(session_->IsEncryptionEstablished());
+  EXPECT_FALSE(session_->OneRttKeysAvailable());
+  EXPECT_EQ(ENCRYPTION_INITIAL, session_->connection()->encryption_level());
+
+  // MAX_PUSH_ID frame is sent upon encryption establishment with the value set
+  // by the earlier SetMaxPushId() call.
+  EXPECT_CALL(debug_visitor, OnSettingsFrameSent(_));
+  EXPECT_CALL(debug_visitor, OnMaxPushIdFrameSent(_))
+      .WillOnce(Invoke(
+          [](const MaxPushIdFrame& frame) { EXPECT_EQ(5u, frame.push_id); }));
+  session_->CryptoConnect();
+  testing::Mock::VerifyAndClearExpectations(&debug_visitor);
+
+  EXPECT_TRUE(session_->IsEncryptionEstablished());
+  EXPECT_FALSE(session_->OneRttKeysAvailable());
+  EXPECT_EQ(ENCRYPTION_ZERO_RTT, session_->connection()->encryption_level());
+
+  // Another SetMaxPushId() call with the same value does not trigger sending
+  // another MAX_PUSH_ID frame.
+  session_->SetMaxPushId(5);
+
+  // Calling SetMaxPushId() with a different value results in sending another
+  // MAX_PUSH_ID frame.
+  EXPECT_CALL(debug_visitor, OnMaxPushIdFrameSent(_))
+      .WillOnce(Invoke(
+          [](const MaxPushIdFrame& frame) { EXPECT_EQ(10u, frame.push_id); }));
+  session_->SetMaxPushId(10);
+  testing::Mock::VerifyAndClearExpectations(&debug_visitor);
+
+  QuicConfig config = DefaultQuicConfig();
+  crypto_test_utils::HandshakeWithFakeServer(
+      &config, server_crypto_config_.get(), &helper_, &alarm_factory_,
+      connection_, crypto_stream_, AlpnForVersion(connection_->version()));
+
+  EXPECT_TRUE(session_->IsEncryptionEstablished());
+  EXPECT_TRUE(session_->OneRttKeysAvailable());
+  EXPECT_EQ(ENCRYPTION_FORWARD_SECURE,
+            session_->connection()->encryption_level());
+  EXPECT_TRUE(session_->GetCryptoStream()->IsResumption());
+}
+
+TEST_P(QuicSpdyClientSessionTest, SetMaxPushIdAfterEncryptionEstablished) {
+  // 0-RTT is TLS-only, MAX_PUSH_ID frame is HTTP/3-only.
+  if (!session_->version().UsesTls() || !session_->version().UsesHttp3()) {
+    return;
+  }
+
+  CompleteFirstConnection();
+
+  CreateConnection();
+  StrictMock<MockHttp3DebugVisitor> debug_visitor;
+  session_->set_debug_visitor(&debug_visitor);
+
+  EXPECT_FALSE(session_->IsEncryptionEstablished());
+  EXPECT_FALSE(session_->OneRttKeysAvailable());
+  EXPECT_EQ(ENCRYPTION_INITIAL, session_->connection()->encryption_level());
+
+  // No MAX_PUSH_ID frame is sent if SetMaxPushId() has not been called.
+  EXPECT_CALL(debug_visitor, OnSettingsFrameSent(_));
+  session_->CryptoConnect();
+  testing::Mock::VerifyAndClearExpectations(&debug_visitor);
+
+  EXPECT_TRUE(session_->IsEncryptionEstablished());
+  EXPECT_FALSE(session_->OneRttKeysAvailable());
+  EXPECT_EQ(ENCRYPTION_ZERO_RTT, session_->connection()->encryption_level());
+
+  // Calling SetMaxPushId() for the first time after encryption is established
+  // results in sending a MAX_PUSH_ID frame.
+  EXPECT_CALL(debug_visitor, OnMaxPushIdFrameSent(_))
+      .WillOnce(Invoke(
+          [](const MaxPushIdFrame& frame) { EXPECT_EQ(5u, frame.push_id); }));
+  session_->SetMaxPushId(5);
+  testing::Mock::VerifyAndClearExpectations(&debug_visitor);
+
+  // Another SetMaxPushId() call with the same value does not trigger sending
+  // another MAX_PUSH_ID frame.
+  session_->SetMaxPushId(5);
+
+  // Calling SetMaxPushId() with a different value results in sending another
+  // MAX_PUSH_ID frame.
+  EXPECT_CALL(debug_visitor, OnMaxPushIdFrameSent(_))
+      .WillOnce(Invoke(
+          [](const MaxPushIdFrame& frame) { EXPECT_EQ(10u, frame.push_id); }));
+  session_->SetMaxPushId(10);
+  testing::Mock::VerifyAndClearExpectations(&debug_visitor);
+
+  QuicConfig config = DefaultQuicConfig();
+  crypto_test_utils::HandshakeWithFakeServer(
+      &config, server_crypto_config_.get(), &helper_, &alarm_factory_,
+      connection_, crypto_stream_, AlpnForVersion(connection_->version()));
+
+  EXPECT_TRUE(session_->IsEncryptionEstablished());
+  EXPECT_TRUE(session_->OneRttKeysAvailable());
+  EXPECT_EQ(ENCRYPTION_FORWARD_SECURE,
+            session_->connection()->encryption_level());
+  EXPECT_TRUE(session_->GetCryptoStream()->IsResumption());
+}
+
 }  // namespace
 }  // namespace test
 }  // namespace quic

diff --git a/quic/core/http/quic_spdy_session.cc b/quic/core/http/quic_spdy_session.cc
index cb6a7f6..386fb8a 100644
--- a/quic/core/http/quic_spdy_session.cc
+++ b/quic/core/http/quic_spdy_session.cc

@@ -739,7 +739,6 @@
   if (perspective() == Perspective::IS_CLIENT && max_push_id_.has_value() &&
       !http3_max_push_id_sent_) {
     SendMaxPushId();
-    http3_max_push_id_sent_ = true;
   }
 }
 
@@ -1241,7 +1240,7 @@
   }
   max_push_id_ = max_push_id;
 
-  if (OneRttKeysAvailable()) {
+  if (IsEncryptionEstablished()) {
     SendMaxPushId();
   }
 }
@@ -1284,6 +1283,7 @@
   DCHECK_EQ(Perspective::IS_CLIENT, perspective());
 
   send_control_stream_->SendMaxPushIdFrame(max_push_id_.value());
+  http3_max_push_id_sent_ = true;
 }
 
 void QuicSpdySession::EnableServerPush() {

diff --git a/quic/core/http/quic_spdy_session.h b/quic/core/http/quic_spdy_session.h
index 069cceb..013bfc3 100644
--- a/quic/core/http/quic_spdy_session.h
+++ b/quic/core/http/quic_spdy_session.h

@@ -466,8 +466,11 @@
   void CloseConnectionOnDuplicateHttp3UnidirectionalStreams(
       quiche::QuicheStringPiece type);
 
-  // Sends any data which should be sent at the start of a connection,
-  // including the initial SETTINGS frame, etc.
+  // Sends any data which should be sent at the start of a connection, including
+  // the initial SETTINGS frame, and (when IETF QUIC is used) also a MAX_PUSH_ID
+  // frame if SetMaxPushId() had been called before encryption was established.
+  // When using 0-RTT, this method is called twice: once when encryption is
+  // established, and again when 1-RTT keys are available.
   void SendInitialData();
 
   // Send a MAX_PUSH_ID frame.  Used in IETF QUIC only.
@@ -534,12 +537,17 @@
   // Server push is enabled for a client by calling SetMaxPushId().
   bool ietf_server_push_enabled_;
 
-  // Used in IETF QUIC only.  Unset until a MAX_PUSH_ID frame is received/sent.
-  // For a server, the push ID in the most recently received MAX_PUSH_ID frame.
-  // For a client before 1-RTT keys are available, the push ID to be sent in the
-  // initial MAX_PUSH_ID frame.
-  // For a client after 1-RTT keys are available, the push ID in the most
-  // recently sent MAX_PUSH_ID frame.
+  // Used in IETF QUIC only.
+  // For a server:
+  //   the push ID in the most recently received MAX_PUSH_ID frame,
+  //   or unset if no MAX_PUSH_ID frame has been received.
+  // For a client:
+  //   unset until SetMaxPushId() is called;
+  //   before encryption is established, the push ID to be sent in the initial
+  //   MAX_PUSH_ID frame;
+  //   after encryption is established, the push ID in the most recently sent
+  //   MAX_PUSH_ID frame.
+  // Once set, never goes back to unset.
   quiche::QuicheOptional<PushId> max_push_id_;
 
   // An integer used for live check. The indicator is assigned a value in
@@ -555,9 +563,9 @@
   // If the endpoint has sent HTTP/3 GOAWAY frame.
   bool http3_goaway_sent_;
 
-  // If SendMaxPushId() has been called from SendInitialData().  Note that a
-  // MAX_PUSH_ID frame is only sent if SetMaxPushId() had been called
-  // beforehand.
+  // Only used by a client, only with IETF QUIC.  True if a MAX_PUSH_ID frame
+  // has been sent, in which case |max_push_id_| has the value sent in the most
+  // recent MAX_PUSH_ID frame.  Once true, never goes back to false.
   bool http3_max_push_id_sent_;
 
   // Priority values received in PRIORITY_UPDATE frames for streams that are not