gfe-relnote: Use PushId instead of QuicStreamId argument type where appropriate. Protected by gfe2_reloadable_flag_quic_enable_version_draft_25_v3 and gfe2_reloadable_flag_quic_enable_version_draft_27. PushId is an alias for uint64_t, while QuicStreamId is an alias for uint32_t, so this might cause a slight behavior change with very large values, but the affected methods are only used in IETF QUIC, for which push is disabled except for some tests. PiperOrigin-RevId: 305871011 Change-Id: If533d822ff5ae1f2af09c23bc6c109f7f59b4833
diff --git a/quic/core/http/quic_spdy_client_session_base.cc b/quic/core/http/quic_spdy_client_session_base.cc index b8dae49..a0c70dd 100644 --- a/quic/core/http/quic_spdy_client_session_base.cc +++ b/quic/core/http/quic_spdy_client_session_base.cc
@@ -105,6 +105,8 @@ bool QuicSpdyClientSessionBase::HandlePromised(QuicStreamId /* associated_id */, QuicStreamId promised_id, const SpdyHeaderBlock& headers) { + // TODO(b/136295430): Do not treat |promised_id| as a stream ID when using + // IETF QUIC. // Due to pathalogical packet re-ordering, it is possible that // frames for the promised stream have already arrived, and the // promised stream could be active or closed.
diff --git a/quic/core/http/quic_spdy_client_session_test.cc b/quic/core/http/quic_spdy_client_session_test.cc index da05a15..f702671 100644 --- a/quic/core/http/quic_spdy_client_session_test.cc +++ b/quic/core/http/quic_spdy_client_session_test.cc
@@ -584,8 +584,7 @@ CompleteCryptoHandshake(); if (VersionHasIetfQuicFrames(connection_->transport_version())) { - session_->SetMaxPushId(GetNthServerInitiatedUnidirectionalStreamId( - connection_->transport_version(), 10)); + session_->SetMaxPushId(10); } MockQuicSpdyClientStream* stream = static_cast<MockQuicSpdyClientStream*>( @@ -605,25 +604,28 @@ session_.get(), std::make_unique<QuicSpdyClientStream>( stream_id, session_.get(), BIDIRECTIONAL)); - if (VersionHasIetfQuicFrames(connection_->transport_version())) { - session_->SetMaxPushId(GetNthServerInitiatedUnidirectionalStreamId( - connection_->transport_version(), 10)); - // TODO(b/136295430) Use PushId to represent Push IDs instead of - // QuicStreamId. - EXPECT_CALL( - *connection_, - CloseConnection(QUIC_INVALID_STREAM_ID, - "Received push stream id higher than MAX_PUSH_ID.", _)); - } - auto promise_id = GetNthServerInitiatedUnidirectionalStreamId( - connection_->transport_version(), 11); - auto headers = QuicHeaderList(); + QuicHeaderList headers; headers.OnHeaderBlockStart(); headers.OnHeader(":path", "/bar"); headers.OnHeader(":authority", "www.google.com"); headers.OnHeader(":method", "GET"); headers.OnHeader(":scheme", "https"); headers.OnHeaderBlockEnd(0, 0); + + if (VersionHasIetfQuicFrames(connection_->transport_version())) { + session_->SetMaxPushId(10); + // TODO(b/136295430) Use PushId to represent Push IDs instead of + // QuicStreamId. + EXPECT_CALL( + *connection_, + CloseConnection(QUIC_INVALID_STREAM_ID, + "Received push stream id higher than MAX_PUSH_ID.", _)); + const PushId promise_id = 11; + session_->OnPromiseHeaderList(stream_id, promise_id, 0, headers); + return; + } + const QuicStreamId promise_id = GetNthServerInitiatedUnidirectionalStreamId( + connection_->transport_version(), 11); session_->OnPromiseHeaderList(stream_id, promise_id, 0, headers); } @@ -647,8 +649,7 @@ CompleteCryptoHandshake(); if (VersionHasIetfQuicFrames(connection_->transport_version())) { - session_->SetMaxPushId(GetNthServerInitiatedUnidirectionalStreamId( - connection_->transport_version(), 10)); + session_->SetMaxPushId(10); } MockQuicSpdyClientStream* stream = static_cast<MockQuicSpdyClientStream*>(
diff --git a/quic/core/http/quic_spdy_session.cc b/quic/core/http/quic_spdy_session.cc index b5d5bcd..acef1b6 100644 --- a/quic/core/http/quic_spdy_session.cc +++ b/quic/core/http/quic_spdy_session.cc
@@ -1209,7 +1209,7 @@ } } -void QuicSpdySession::SetMaxPushId(QuicStreamId max_push_id) { +void QuicSpdySession::SetMaxPushId(PushId max_push_id) { DCHECK(VersionUsesHttp3(transport_version())); DCHECK_EQ(Perspective::IS_CLIENT, perspective()); if (max_push_id_.has_value()) { @@ -1232,7 +1232,7 @@ } } -bool QuicSpdySession::OnMaxPushIdFrame(QuicStreamId max_push_id) { +bool QuicSpdySession::OnMaxPushIdFrame(PushId max_push_id) { DCHECK(VersionUsesHttp3(transport_version())); DCHECK_EQ(Perspective::IS_SERVER, perspective()); @@ -1243,7 +1243,7 @@ QUIC_DVLOG(1) << "Setting max_push_id to: " << max_push_id << " from unset"; } - quiche::QuicheOptional<QuicStreamId> old_max_push_id = max_push_id_; + quiche::QuicheOptional<PushId> old_max_push_id = max_push_id_; max_push_id_ = max_push_id; if (!old_max_push_id.has_value() || @@ -1272,7 +1272,7 @@ ietf_server_push_enabled_ = true; } -bool QuicSpdySession::CanCreatePushStreamWithId(QuicStreamId push_id) { +bool QuicSpdySession::CanCreatePushStreamWithId(PushId push_id) { DCHECK(VersionUsesHttp3(transport_version())); return ietf_server_push_enabled_ && max_push_id_.has_value() &&
diff --git a/quic/core/http/quic_spdy_session.h b/quic/core/http/quic_spdy_session.h index 93fa09d..cf732fa 100644 --- a/quic/core/http/quic_spdy_session.h +++ b/quic/core/http/quic_spdy_session.h
@@ -308,14 +308,14 @@ // This method must only be called if protocol is IETF QUIC and perspective is // client. |max_push_id| must be greater than or equal to current // |max_push_id_|. - void SetMaxPushId(QuicStreamId max_push_id); + void SetMaxPushId(PushId max_push_id); // Sets |max_push_id_|. // This method must only be called if protocol is IETF QUIC and perspective is // server. It must only be called if a MAX_PUSH_ID frame is received. // Returns whether |max_push_id| is greater than or equal to current // |max_push_id_|. - bool OnMaxPushIdFrame(QuicStreamId max_push_id); + bool OnMaxPushIdFrame(PushId max_push_id); // Enables server push. // Must only be called when using IETF QUIC, for which server push is disabled @@ -332,8 +332,7 @@ // For a client this means that SetMaxPushId() has been called with // |max_push_id| greater than or equal to |push_id|. // Must only be called when using IETF QUIC. - // TODO(b/136295430): Use sequential PUSH IDs instead of stream IDs. - bool CanCreatePushStreamWithId(QuicStreamId push_id); + bool CanCreatePushStreamWithId(PushId push_id); int32_t destruction_indicator() const { return destruction_indicator_; } @@ -540,7 +539,7 @@ // 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. - quiche::QuicheOptional<QuicStreamId> max_push_id_; + quiche::QuicheOptional<PushId> max_push_id_; // An integer used for live check. The indicator is assigned a value in // constructor. As long as it is not the assigned value, that would indicate
diff --git a/quic/core/http/quic_spdy_session_test.cc b/quic/core/http/quic_spdy_session_test.cc index f7ceaec..307dabb 100644 --- a/quic/core/http/quic_spdy_session_test.cc +++ b/quic/core/http/quic_spdy_session_test.cc
@@ -3023,7 +3023,7 @@ StrictMock<MockHttp3DebugVisitor> debug_visitor; session_.set_debug_visitor(&debug_visitor); - const QuicStreamId max_push_id = 5; + const PushId max_push_id = 5; session_.SetMaxPushId(max_push_id); InSequence s;
diff --git a/quic/tools/quic_spdy_client_base.h b/quic/tools/quic_spdy_client_base.h index c4381f7..2517289 100644 --- a/quic/tools/quic_spdy_client_base.h +++ b/quic/tools/quic_spdy_client_base.h
@@ -140,7 +140,7 @@ // Set the max promise id for the client session. // TODO(b/151641466): Rename this method. - void SetMaxAllowedPushId(QuicStreamId max) { max_allowed_push_id_ = max; } + void SetMaxAllowedPushId(PushId max) { max_allowed_push_id_ = max; } // Disables the use of the QPACK dynamic table and of blocked streams. // Must be called before InitializeSession(). @@ -224,7 +224,7 @@ bool drop_response_body_ = false; // The max promise id to set on the client session when created. - QuicStreamId max_allowed_push_id_; + PushId max_allowed_push_id_; bool disable_qpack_dynamic_table_; };