Change QuicSpdySession::SendMaxPushId() to use max_allowed_push_id_ instead of requiring the caller to pass it in explicitly. Also clean up the implementation of QuicSpdySession::set_max_allowed_push_id() and rename it to SetMaxAllowedPushId() to match the style guide. gfe-relnote: Clean up QuicSpdySession::set_max_allowed_push_id. Protected by disabled QUIC v99 flag. PiperOrigin-RevId: 274851971 Change-Id: Ie4c6ad9aba91a4a26a9d13ebbff4a4399f9d10a9
diff --git a/quic/core/http/end_to_end_test.cc b/quic/core/http/end_to_end_test.cc index 9ce3cd6..e7d0329 100644 --- a/quic/core/http/end_to_end_test.cc +++ b/quic/core/http/end_to_end_test.cc
@@ -295,7 +295,7 @@ client->UseConnectionIdLength(override_server_connection_id_length_); client->UseClientConnectionIdLength(override_client_connection_id_length_); if (support_server_push_) { - client->client()->set_max_allowed_push_id(kMaxQuicStreamId); + client->client()->SetMaxAllowedPushId(kMaxQuicStreamId); } client->Connect(); return client; @@ -4233,7 +4233,7 @@ EXPECT_TRUE(client_->client()->WaitForCryptoHandshakeConfirmed()); static_cast<QuicSpdySession*>(client_->client()->session()) - ->set_max_allowed_push_id(kMaxQuicStreamId); + ->SetMaxAllowedPushId(kMaxQuicStreamId); client_->SendSynchronousRequest("/foo");
diff --git a/quic/core/http/quic_headers_stream_test.cc b/quic/core/http/quic_headers_stream_test.cc index a9779c8..0791506 100644 --- a/quic/core/http/quic_headers_stream_test.cc +++ b/quic/core/http/quic_headers_stream_test.cc
@@ -405,7 +405,7 @@ } TEST_P(QuicHeadersStreamTest, WritePushPromises) { - session_.set_max_allowed_push_id(kMaxQuicStreamId); + session_.SetMaxAllowedPushId(kMaxQuicStreamId); for (QuicStreamId stream_id = client_id_1_; stream_id < client_id_3_; stream_id += next_stream_id_) {
diff --git a/quic/core/http/quic_receive_control_stream.cc b/quic/core/http/quic_receive_control_stream.cc index b64aa4e..4d78106 100644 --- a/quic/core/http/quic_receive_control_stream.cc +++ b/quic/core/http/quic_receive_control_stream.cc
@@ -58,7 +58,7 @@ if (stream_->session()->perspective() == Perspective::IS_SERVER) { QuicSpdySession* spdy_session = static_cast<QuicSpdySession*>(stream_->session()); - spdy_session->set_max_allowed_push_id(frame.push_id); + spdy_session->SetMaxAllowedPushId(frame.push_id); return true; } CloseConnectionOnWrongFrame("Max Push Id");
diff --git a/quic/core/http/quic_spdy_client_session_base.cc b/quic/core/http/quic_spdy_client_session_base.cc index 1690df7..342a3e2 100644 --- a/quic/core/http/quic_spdy_client_session_base.cc +++ b/quic/core/http/quic_spdy_client_session_base.cc
@@ -43,8 +43,8 @@ CryptoHandshakeEvent event) { QuicSpdySession::OnCryptoHandshakeEvent(event); if (event == HANDSHAKE_CONFIRMED && max_allowed_push_id() > 0 && - VersionHasIetfQuicFrames(transport_version())) { - SendMaxPushId(max_allowed_push_id()); + VersionUsesHttp3(transport_version())) { + SendMaxPushId(); } }
diff --git a/quic/core/http/quic_spdy_client_session_test.cc b/quic/core/http/quic_spdy_client_session_test.cc index 7fe28f2..59634f8 100644 --- a/quic/core/http/quic_spdy_client_session_test.cc +++ b/quic/core/http/quic_spdy_client_session_test.cc
@@ -582,7 +582,7 @@ // Initialize crypto before the client session will create a stream. CompleteCryptoHandshake(); - session_->set_max_allowed_push_id(GetNthServerInitiatedUnidirectionalStreamId( + session_->SetMaxAllowedPushId(GetNthServerInitiatedUnidirectionalStreamId( connection_->transport_version(), 10)); MockQuicSpdyClientStream* stream = static_cast<MockQuicSpdyClientStream*>( @@ -602,7 +602,7 @@ session_.get(), std::make_unique<QuicSpdyClientStream>( stream_id, session_.get(), BIDIRECTIONAL)); - session_->set_max_allowed_push_id(GetNthServerInitiatedUnidirectionalStreamId( + session_->SetMaxAllowedPushId(GetNthServerInitiatedUnidirectionalStreamId( connection_->transport_version(), 10)); if (VersionHasIetfQuicFrames(connection_->transport_version())) { // TODO(b/136295430) Use PushId to represent Push IDs instead of @@ -644,7 +644,7 @@ // Initialize crypto before the client session will create a stream. CompleteCryptoHandshake(); - session_->set_max_allowed_push_id(GetNthServerInitiatedUnidirectionalStreamId( + session_->SetMaxAllowedPushId(GetNthServerInitiatedUnidirectionalStreamId( connection_->transport_version(), 10)); MockQuicSpdyClientStream* stream = static_cast<MockQuicSpdyClientStream*>( @@ -912,7 +912,7 @@ session_.get(), std::make_unique<QuicSpdyClientStream>( stream_id, session_.get(), BIDIRECTIONAL)); - session_->set_max_allowed_push_id(kMaxQuicStreamId); + session_->SetMaxAllowedPushId(kMaxQuicStreamId); EXPECT_CALL(*connection_, OnStreamReset(_, QUIC_REFUSED_STREAM));
diff --git a/quic/core/http/quic_spdy_session.cc b/quic/core/http/quic_spdy_session.cc index c8e550d..401ade6 100644 --- a/quic/core/http/quic_spdy_session.cc +++ b/quic/core/http/quic_spdy_session.cc
@@ -1017,25 +1017,33 @@ } } -void QuicSpdySession::set_max_allowed_push_id( - QuicStreamId max_allowed_push_id) { - if (VersionHasIetfQuicFrames(transport_version()) && - perspective() == Perspective::IS_SERVER && - max_allowed_push_id > max_allowed_push_id_) { - OnCanCreateNewOutgoingStream(true); +void QuicSpdySession::SetMaxAllowedPushId(QuicStreamId max_allowed_push_id) { + if (!VersionUsesHttp3(transport_version())) { + return; } + QuicStreamId old_max_allowed_push_id = max_allowed_push_id_; max_allowed_push_id_ = max_allowed_push_id; + QUIC_DVLOG(1) << "Setting max_allowed_push_id to: " << max_allowed_push_id_ + << " from: " << old_max_allowed_push_id; - if (VersionHasIetfQuicFrames(transport_version()) && - perspective() == Perspective::IS_CLIENT && IsHandshakeConfirmed()) { - SendMaxPushId(max_allowed_push_id); + if (perspective() == Perspective::IS_SERVER) { + if (max_allowed_push_id_ > old_max_allowed_push_id) { + OnCanCreateNewOutgoingStream(true); + } + return; + } + + DCHECK(perspective() == Perspective::IS_CLIENT); + if (IsHandshakeConfirmed()) { + SendMaxPushId(); + send_control_stream_->SendMaxPushIdFrame(max_allowed_push_id_); } } -void QuicSpdySession::SendMaxPushId(QuicStreamId max_allowed_push_id) { +void QuicSpdySession::SendMaxPushId() { DCHECK(VersionUsesHttp3(transport_version())); - send_control_stream_->SendMaxPushIdFrame(max_allowed_push_id); + send_control_stream_->SendMaxPushIdFrame(max_allowed_push_id_); } void QuicSpdySession::CloseConnectionOnDuplicateHttp3UnidirectionalStreams(
diff --git a/quic/core/http/quic_spdy_session.h b/quic/core/http/quic_spdy_session.h index 29046c3..0eb3751 100644 --- a/quic/core/http/quic_spdy_session.h +++ b/quic/core/http/quic_spdy_session.h
@@ -225,7 +225,7 @@ // those streams are not initialized yet. void OnCanCreateNewOutgoingStream(bool unidirectional) override; - void set_max_allowed_push_id(QuicStreamId max_allowed_push_id); + void SetMaxAllowedPushId(QuicStreamId max_allowed_push_id); QuicStreamId max_allowed_push_id() { return max_allowed_push_id_; } @@ -324,7 +324,7 @@ void set_max_uncompressed_header_bytes( size_t set_max_uncompressed_header_bytes); - void SendMaxPushId(QuicStreamId max_allowed_push_id); + void SendMaxPushId(); private: friend class test::QuicSpdySessionPeer;
diff --git a/quic/tools/quic_simple_server_session_test.cc b/quic/tools/quic_simple_server_session_test.cc index 08697d4..a4bab0f 100644 --- a/quic/tools/quic_simple_server_session_test.cc +++ b/quic/tools/quic_simple_server_session_test.cc
@@ -734,7 +734,7 @@ // opened and send push response. TEST_P(QuicSimpleServerSessionServerPushTest, TestPromisePushResources) { MaybeConsumeHeadersStreamData(); - session_->set_max_allowed_push_id(kMaxQuicStreamId); + session_->SetMaxAllowedPushId(kMaxQuicStreamId); size_t num_resources = kMaxStreamsForTest + 5; PromisePushResources(num_resources); EXPECT_EQ(kMaxStreamsForTest, session_->GetNumOpenOutgoingStreams()); @@ -745,7 +745,7 @@ TEST_P(QuicSimpleServerSessionServerPushTest, HandlePromisedPushRequestsAfterStreamDraining) { MaybeConsumeHeadersStreamData(); - session_->set_max_allowed_push_id(kMaxQuicStreamId); + session_->SetMaxAllowedPushId(kMaxQuicStreamId); size_t num_resources = kMaxStreamsForTest + 1; QuicByteCount data_frame_header_length = PromisePushResources(num_resources); QuicStreamId next_out_going_stream_id; @@ -814,7 +814,7 @@ TEST_P(QuicSimpleServerSessionServerPushTest, ResetPromisedStreamToCancelServerPush) { MaybeConsumeHeadersStreamData(); - session_->set_max_allowed_push_id(kMaxQuicStreamId); + session_->SetMaxAllowedPushId(kMaxQuicStreamId); // Having two extra resources to be send later. One of them will be reset, so // when opened stream become close, only one will become open. @@ -903,7 +903,7 @@ TEST_P(QuicSimpleServerSessionServerPushTest, CloseStreamToHandleMorePromisedStream) { MaybeConsumeHeadersStreamData(); - session_->set_max_allowed_push_id(kMaxQuicStreamId); + session_->SetMaxAllowedPushId(kMaxQuicStreamId); size_t num_resources = kMaxStreamsForTest + 1; if (VersionHasIetfQuicFrames(transport_version())) { // V99 will send out a stream-id-blocked frame when the we desired to exceed
diff --git a/quic/tools/quic_spdy_client_base.cc b/quic/tools/quic_spdy_client_base.cc index 6c16695..ab1542d 100644 --- a/quic/tools/quic_spdy_client_base.cc +++ b/quic/tools/quic_spdy_client_base.cc
@@ -63,7 +63,7 @@ client_session()->Initialize(); client_session()->CryptoConnect(); if (max_allowed_push_id_ > 0) { - client_session()->set_max_allowed_push_id(max_allowed_push_id_); + client_session()->SetMaxAllowedPushId(max_allowed_push_id_); } }
diff --git a/quic/tools/quic_spdy_client_base.h b/quic/tools/quic_spdy_client_base.h index b04ba66..3107d60 100644 --- a/quic/tools/quic_spdy_client_base.h +++ b/quic/tools/quic_spdy_client_base.h
@@ -137,7 +137,7 @@ bool drop_response_body() const { return drop_response_body_; } // Set the max promise id for the client session. - void set_max_allowed_push_id(QuicStreamId max) { max_allowed_push_id_ = max; } + void SetMaxAllowedPushId(QuicStreamId max) { max_allowed_push_id_ = max; } protected: int GetNumSentClientHellosFromSession() override;