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_;
};
