Remove headers stream from IETF QUIC.
gfe-relnote: v99 only, not protected.
PiperOrigin-RevId: 260762103
Change-Id: I99d74c65b66e18a30b864b54ec2ec6d4a7a3ed61
diff --git a/quic/core/http/end_to_end_test.cc b/quic/core/http/end_to_end_test.cc
index 10455e8..be6e388 100644
--- a/quic/core/http/end_to_end_test.cc
+++ b/quic/core/http/end_to_end_test.cc
@@ -1948,7 +1948,6 @@
// TODO(nharper): Needs to get turned back to EndToEndTestWithTls
// when we figure out why the test doesn't work on chrome.
TEST_P(EndToEndTest, MaxStreamsUberTest) {
- SetQuicFlag(FLAGS_quic_headers_stream_id_in_v99, 0);
// Connect with lower fake packet loss than we'd like to test. Until
// b/10126687 is fixed, losing handshake packets is pretty brutal.
SetPacketLossPercentage(1);
@@ -2269,10 +2268,7 @@
QuicSpdySession* const client_session = client_->client()->client_session();
auto* server_session = static_cast<QuicSpdySession*>(GetServerSession());
- if (VersionHasStreamType(client_->client()
- ->client_session()
- ->connection()
- ->transport_version())) {
+ if (VersionHasStreamType(server_session->connection()->transport_version())) {
// Settings frame will be sent through control streams, which contribute
// to the session's flow controller. And due to the timing issue described
// below, the settings frame might not be received.
@@ -3051,7 +3047,6 @@
const size_t kNumMaxStreams = 10;
EndToEndTestServerPush() : EndToEndTest() {
- SetQuicFlag(FLAGS_quic_headers_stream_id_in_v99, 0);
client_config_.SetMaxIncomingBidirectionalStreamsToSend(kNumMaxStreams);
server_config_.SetMaxIncomingBidirectionalStreamsToSend(kNumMaxStreams);
client_config_.SetMaxIncomingUnidirectionalStreamsToSend(kNumMaxStreams);
@@ -3453,6 +3448,12 @@
// PUSH_PROMISE, its headers stream's sequencer buffer should be released.
ASSERT_TRUE(Initialize());
client_->SendSynchronousRequest("/foo");
+ if (VersionUsesQpack(client_->client()
+ ->client_session()
+ ->connection()
+ ->transport_version())) {
+ return;
+ }
QuicHeadersStream* headers_stream = QuicSpdySessionPeer::GetHeadersStream(
client_->client()->client_session());
QuicStreamSequencer* sequencer = QuicStreamPeer::sequencer(headers_stream);
diff --git a/quic/core/http/quic_server_session_base_test.cc b/quic/core/http/quic_server_session_base_test.cc
index 698193b..ecca4e7 100644
--- a/quic/core/http/quic_server_session_base_test.cc
+++ b/quic/core/http/quic_server_session_base_test.cc
@@ -474,6 +474,11 @@
};
TEST_P(QuicServerSessionBaseTest, BandwidthEstimates) {
+ if (GetParam().handshake_protocol == PROTOCOL_TLS1_3) {
+ // TODO(nharper, b/112643533): Figure out why this test fails when TLS is
+ // enabled and fix it.
+ return;
+ }
// Test that bandwidth estimate updates are sent to the client, only when
// bandwidth resumption is enabled, the bandwidth estimate has changed
// sufficiently, enough time has passed,
@@ -493,18 +498,22 @@
const std::string serving_region = "not a real region";
session_->set_serving_region(serving_region);
- session_->UnregisterStreamPriority(
- QuicUtils::GetHeadersStreamId(connection_->transport_version()),
- /*is_static=*/true);
+ if (!VersionUsesQpack(transport_version())) {
+ session_->UnregisterStreamPriority(
+ QuicUtils::GetHeadersStreamId(connection_->transport_version()),
+ /*is_static=*/true);
+ }
QuicServerSessionBasePeer::SetCryptoStream(session_.get(), nullptr);
MockQuicCryptoServerStream* crypto_stream =
new MockQuicCryptoServerStream(&crypto_config_, &compressed_certs_cache_,
session_.get(), &stream_helper_);
QuicServerSessionBasePeer::SetCryptoStream(session_.get(), crypto_stream);
- session_->RegisterStreamPriority(
- QuicUtils::GetHeadersStreamId(connection_->transport_version()),
- /*is_static=*/true,
- spdy::SpdyStreamPrecedence(QuicStream::kDefaultPriority));
+ if (!VersionUsesQpack(transport_version())) {
+ session_->RegisterStreamPriority(
+ QuicUtils::GetHeadersStreamId(connection_->transport_version()),
+ /*is_static=*/true,
+ spdy::SpdyStreamPrecedence(QuicStream::kDefaultPriority));
+ }
// Set some initial bandwidth values.
QuicSentPacketManager* sent_packet_manager =
diff --git a/quic/core/http/quic_spdy_client_session_base.cc b/quic/core/http/quic_spdy_client_session_base.cc
index f27411a..fecda92 100644
--- a/quic/core/http/quic_spdy_client_session_base.cc
+++ b/quic/core/http/quic_spdy_client_session_base.cc
@@ -193,7 +193,9 @@
// Since promised_by_id_ contains the unique_ptr, this will destroy
// promised.
promised_by_id_.erase(promised->id());
- headers_stream()->MaybeReleaseSequencerBuffer();
+ if (!VersionUsesQpack(connection()->transport_version())) {
+ headers_stream()->MaybeReleaseSequencerBuffer();
+ }
}
void QuicSpdyClientSessionBase::OnPushStreamTimedOut(
@@ -211,7 +213,9 @@
void QuicSpdyClientSessionBase::CloseStreamInner(QuicStreamId stream_id,
bool locally_reset) {
QuicSpdySession::CloseStreamInner(stream_id, locally_reset);
- headers_stream()->MaybeReleaseSequencerBuffer();
+ if (!VersionUsesQpack(connection()->transport_version())) {
+ headers_stream()->MaybeReleaseSequencerBuffer();
+ }
}
bool QuicSpdyClientSessionBase::ShouldReleaseHeadersStreamSequencerBuffer() {
diff --git a/quic/core/http/quic_spdy_client_session_test.cc b/quic/core/http/quic_spdy_client_session_test.cc
index 070acdf..0f29b06 100644
--- a/quic/core/http/quic_spdy_client_session_test.cc
+++ b/quic/core/http/quic_spdy_client_session_test.cc
@@ -243,9 +243,6 @@
EXPECT_CALL(*connection_, OnStreamReset(_, _)).Times(AnyNumber());
uint32_t kServerMaxIncomingStreams = 1;
- if (VersionLacksHeadersStream(GetParam().transport_version)) {
- kServerMaxIncomingStreams = 0;
- }
CompleteCryptoHandshake(kServerMaxIncomingStreams);
QuicSpdyClientStream* stream = session_->CreateOutgoingBidirectionalStream();
@@ -261,22 +258,9 @@
EXPECT_FALSE(stream);
if (VersionHasIetfQuicFrames(GetParam().transport_version)) {
- if (VersionLacksHeadersStream(GetParam().transport_version)) {
- EXPECT_EQ(1u,
- QuicSessionPeer::v99_bidirectional_stream_id_manager(&*session_)
- ->outgoing_stream_count());
-
- } else {
- // Ensure that we have/have had 3 open streams, crypto, header, and the
- // 1 test stream. Primary purpose of this is to fail when crypto
- // no longer uses a normal stream. Some above constants will then need
- // to be changed.
- EXPECT_EQ(QuicSessionPeer::v99_bidirectional_stream_id_manager(&*session_)
- ->outgoing_static_stream_count() +
- 1,
- QuicSessionPeer::v99_bidirectional_stream_id_manager(&*session_)
- ->outgoing_stream_count());
- }
+ EXPECT_EQ(1u,
+ QuicSessionPeer::v99_bidirectional_stream_id_manager(&*session_)
+ ->outgoing_stream_count());
}
}
@@ -291,9 +275,6 @@
EXPECT_CALL(*connection_, OnStreamReset(_, _)).Times(AnyNumber());
uint32_t kServerMaxIncomingStreams = 1;
- if (VersionLacksHeadersStream(GetParam().transport_version)) {
- kServerMaxIncomingStreams = 0;
- }
CompleteCryptoHandshake(kServerMaxIncomingStreams);
QuicSpdyClientStream* stream = session_->CreateOutgoingBidirectionalStream();
@@ -326,10 +307,7 @@
if (VersionHasIetfQuicFrames(GetParam().transport_version)) {
// Ensure that we have/have had three open streams: two test streams and the
// header stream.
- QuicStreamCount expected_stream_count = 3;
- if (VersionLacksHeadersStream(GetParam().transport_version)) {
- expected_stream_count = 2;
- }
+ QuicStreamCount expected_stream_count = 2;
EXPECT_EQ(expected_stream_count,
QuicSessionPeer::v99_bidirectional_stream_id_manager(&*session_)
->outgoing_stream_count());
@@ -348,9 +326,6 @@
// the client should result in all outstanding stream state being tidied up
// (including flow control, and number of available outgoing streams).
uint32_t kServerMaxIncomingStreams = 1;
- if (VersionLacksHeadersStream(GetParam().transport_version)) {
- kServerMaxIncomingStreams = 0;
- }
CompleteCryptoHandshake(kServerMaxIncomingStreams);
QuicSpdyClientStream* stream = session_->CreateOutgoingBidirectionalStream();
@@ -413,10 +388,7 @@
if (VersionHasIetfQuicFrames(GetParam().transport_version)) {
// Ensure that we have/have had three open streams: two test streams and the
// header stream.
- QuicStreamCount expected_stream_count = 3;
- if (VersionLacksHeadersStream(GetParam().transport_version)) {
- expected_stream_count = 2;
- }
+ QuicStreamCount expected_stream_count = 2;
EXPECT_EQ(expected_stream_count,
QuicSessionPeer::v99_bidirectional_stream_id_manager(&*session_)
->outgoing_stream_count());
@@ -474,10 +446,20 @@
trailers.OnHeader(kFinalOffsetHeaderKey, "0");
trailers.OnHeaderBlockEnd(0, 0);
+ // Initialize H/3 control stream.
+ QuicStreamId id;
+ if (VersionUsesQpack(connection_->transport_version())) {
+ id = GetNthServerInitiatedUnidirectionalStreamId(
+ connection_->transport_version(), 3);
+ char type[] = {0x00};
+
+ QuicStreamFrame data1(id, false, 0, QuicStringPiece(type, 1));
+ session_->OnStreamFrame(data1);
+ } else {
+ id = QuicUtils::GetHeadersStreamId(connection_->transport_version());
+ }
EXPECT_CALL(*connection_, CloseConnection(_, _, _)).Times(1);
- session_->OnPromiseHeaderList(
- QuicUtils::GetHeadersStreamId(connection_->transport_version()),
- promised_stream_id_, 0, trailers);
+ session_->OnPromiseHeaderList(id, promised_stream_id_, 0, trailers);
}
TEST_P(QuicSpdyClientSessionTest, GoAwayReceived) {
diff --git a/quic/core/http/quic_spdy_session.cc b/quic/core/http/quic_spdy_session.cc
index 3c3f8e0..0454ad2 100644
--- a/quic/core/http/quic_spdy_session.cc
+++ b/quic/core/http/quic_spdy_session.cc
@@ -322,7 +322,7 @@
void QuicSpdySession::Initialize() {
QuicSession::Initialize();
- if (!connection()->version().DoesNotHaveHeadersStream()) {
+ if (!VersionUsesQpack(connection()->transport_version())) {
if (perspective() == Perspective::IS_SERVER) {
set_largest_peer_created_stream_id(
QuicUtils::GetHeadersStreamId(connection()->transport_version()));
@@ -331,9 +331,14 @@
DCHECK_EQ(headers_stream_id, QuicUtils::GetHeadersStreamId(
connection()->transport_version()));
}
- }
+ auto headers_stream = QuicMakeUnique<QuicHeadersStream>((this));
+ DCHECK_EQ(QuicUtils::GetHeadersStreamId(connection()->transport_version()),
+ headers_stream->id());
- if (VersionUsesQpack(connection()->transport_version())) {
+ headers_stream_ = headers_stream.get();
+ RegisterStaticStream(std::move(headers_stream),
+ /*stream_already_counted = */ false);
+ } else {
qpack_encoder_ =
QuicMakeUnique<QpackEncoder>(this, &encoder_stream_sender_delegate_);
qpack_decoder_ =
@@ -344,14 +349,6 @@
kDefaultQpackMaxDynamicTableCapacity);
}
- auto headers_stream = QuicMakeUnique<QuicHeadersStream>((this));
- DCHECK_EQ(QuicUtils::GetHeadersStreamId(connection()->transport_version()),
- headers_stream->id());
-
- headers_stream_ = headers_stream.get();
- RegisterStaticStream(std::move(headers_stream),
- /*stream_already_counted = */ false);
-
if (VersionHasStreamType(connection()->transport_version())) {
MaybeInitializeHttp3UnidirectionalStreams();
}
@@ -462,6 +459,7 @@
QuicStreamId parent_stream_id,
int weight,
bool exclusive) {
+ DCHECK(!VersionUsesQpack(connection()->transport_version()));
if (connection()->transport_version() <= QUIC_VERSION_39) {
return 0;
}
diff --git a/quic/core/http/quic_spdy_session.h b/quic/core/http/quic_spdy_session.h
index 1fa0be7..2e467ae 100644
--- a/quic/core/http/quic_spdy_session.h
+++ b/quic/core/http/quic_spdy_session.h
@@ -245,6 +245,10 @@
bool IsConnected() { return connection()->connected(); }
+ const QuicReceiveControlStream* receive_control_stream() const {
+ return receive_control_stream_;
+ }
+
private:
friend class test::QuicSpdySessionPeer;
diff --git a/quic/core/http/quic_spdy_session_test.cc b/quic/core/http/quic_spdy_session_test.cc
index 9f4671f..a2b4846 100644
--- a/quic/core/http/quic_spdy_session_test.cc
+++ b/quic/core/http/quic_spdy_session_test.cc
@@ -478,12 +478,9 @@
// stream ID, the next ID should fail. Since the actual limit
// is not the number of open streams, we allocate the max and the max+2.
// Get the max allowed stream ID, this should succeed.
- QuicStreamCount headers_stream_offset =
- VersionLacksHeadersStream(QUIC_VERSION_99) ? 1 : 0;
QuicStreamId stream_id = StreamCountToId(
QuicSessionPeer::v99_streamid_manager(&session_)
- ->actual_max_allowed_incoming_bidirectional_streams() -
- headers_stream_offset,
+ ->actual_max_allowed_incoming_bidirectional_streams(),
Perspective::IS_CLIENT, // Client initates stream, allocs stream id.
/*bidirectional=*/true);
EXPECT_NE(nullptr, session_.GetOrCreateStream(stream_id));
@@ -498,7 +495,7 @@
stream_id = StreamCountToId(
QuicSessionPeer::v99_streamid_manager(&session_)
->actual_max_allowed_incoming_bidirectional_streams() +
- 1 - headers_stream_offset,
+ 1,
Perspective::IS_CLIENT,
/*bidirectional=*/true);
EXPECT_EQ(nullptr, session_.GetOrCreateStream(stream_id));
@@ -928,14 +925,16 @@
TestCryptoStream* crypto_stream = session_.GetMutableCryptoStream();
EXPECT_CALL(*crypto_stream, OnCanWrite());
}
- TestHeadersStream* headers_stream;
- QuicSpdySessionPeer::SetHeadersStream(&session_, nullptr);
- headers_stream = new TestHeadersStream(&session_);
- QuicSpdySessionPeer::SetHeadersStream(&session_, headers_stream);
- session_.MarkConnectionLevelWriteBlocked(
- QuicUtils::GetHeadersStreamId(connection_->transport_version()));
- EXPECT_CALL(*headers_stream, OnCanWrite());
+ if (!VersionUsesQpack(connection_->transport_version())) {
+ TestHeadersStream* headers_stream;
+ QuicSpdySessionPeer::SetHeadersStream(&session_, nullptr);
+ headers_stream = new TestHeadersStream(&session_);
+ QuicSpdySessionPeer::SetHeadersStream(&session_, headers_stream);
+ session_.MarkConnectionLevelWriteBlocked(
+ QuicUtils::GetHeadersStreamId(connection_->transport_version()));
+ EXPECT_CALL(*headers_stream, OnCanWrite());
+ }
// After the crypto and header streams perform a write, the connection will be
// blocked by the flow control, hence it should become application-limited.
@@ -1067,10 +1066,20 @@
}
TEST_P(QuicSpdySessionTestServer, OnStreamFrameFinStaticStreamId) {
+ QuicStreamId id;
+ // Initialize HTTP/3 control stream.
+ if (VersionUsesQpack(transport_version())) {
+ id = GetNthClientInitiatedUnidirectionalStreamId(transport_version(), 3);
+ char type[] = {kControlStream};
+
+ QuicStreamFrame data1(id, false, 0, QuicStringPiece(type, 1));
+ session_.OnStreamFrame(data1);
+ } else {
+ id = QuicUtils::GetHeadersStreamId(connection_->transport_version());
+ }
+
// Send two bytes of payload.
- QuicStreamFrame data1(
- QuicUtils::GetHeadersStreamId(connection_->transport_version()), true, 0,
- QuicStringPiece("HT"));
+ QuicStreamFrame data1(id, true, 0, QuicStringPiece("HT"));
EXPECT_CALL(*connection_,
CloseConnection(
QUIC_INVALID_STREAM_ID, "Attempt to close a static stream",
@@ -1079,11 +1088,21 @@
}
TEST_P(QuicSpdySessionTestServer, OnRstStreamStaticStreamId) {
+ QuicStreamId id;
+ // Initialize HTTP/3 control stream.
+ if (VersionUsesQpack(transport_version())) {
+ id = GetNthClientInitiatedUnidirectionalStreamId(transport_version(), 3);
+ char type[] = {kControlStream};
+
+ QuicStreamFrame data1(id, false, 0, QuicStringPiece(type, 1));
+ session_.OnStreamFrame(data1);
+ } else {
+ id = QuicUtils::GetHeadersStreamId(connection_->transport_version());
+ }
+
// Send two bytes of payload.
- QuicRstStreamFrame rst1(
- kInvalidControlFrameId,
- QuicUtils::GetHeadersStreamId(connection_->transport_version()),
- QUIC_ERROR_PROCESSING_STREAM, 0);
+ QuicRstStreamFrame rst1(kInvalidControlFrameId, id,
+ QUIC_ERROR_PROCESSING_STREAM, 0);
EXPECT_CALL(*connection_,
CloseConnection(
QUIC_INVALID_STREAM_ID, "Attempt to reset a static stream",
@@ -1496,11 +1515,10 @@
}
TEST_P(QuicSpdySessionTestServer, WindowUpdateUnblocksHeadersStream) {
- if (GetParam().handshake_protocol == PROTOCOL_TLS1_3) {
- // TODO(nharper, b/112643533): Figure out why this test fails when TLS is
- // enabled and fix it.
+ if (VersionUsesQpack(GetParam().transport_version)) {
return;
}
+
// Test that a flow control blocked headers stream gets unblocked on recipt of
// a WINDOW_UPDATE frame.
@@ -1585,7 +1603,7 @@
*connection_,
CloseConnection(QUIC_INVALID_STREAM_ID,
testing::MatchesRegex(
- "Stream id \\d+ would exceed stream count limit 6"),
+ "Stream id \\d+ would exceed stream count limit 5"),
_));
}
// Create one more data streams to exceed limit of open stream.
diff --git a/quic/core/http/quic_spdy_stream_test.cc b/quic/core/http/quic_spdy_stream_test.cc
index 1defc5a..ab90036 100644
--- a/quic/core/http/quic_spdy_stream_test.cc
+++ b/quic/core/http/quic_spdy_stream_test.cc
@@ -1378,7 +1378,7 @@
TEST_P(QuicSpdyStreamTest, HeaderStreamNotiferCorrespondingSpdyStream) {
// There is no headers stream if QPACK is used.
- if (!VersionUsesQpack(GetParam().transport_version)) {
+ if (VersionUsesQpack(GetParam().transport_version)) {
return;
}
diff --git a/quic/core/quic_session_test.cc b/quic/core/quic_session_test.cc
index 2e15df1..dce8ec6 100644
--- a/quic/core/quic_session_test.cc
+++ b/quic/core/quic_session_test.cc
@@ -1289,7 +1289,7 @@
}
TEST_P(QuicSessionTestServer, OnStreamFrameFinStaticStreamId) {
- if (connection_->version().DoesNotHaveHeadersStream()) {
+ if (VersionUsesQpack(connection_->transport_version())) {
return;
}
QuicStreamId headers_stream_id =
@@ -1308,7 +1308,7 @@
}
TEST_P(QuicSessionTestServer, OnRstStreamStaticStreamId) {
- if (connection_->version().DoesNotHaveHeadersStream()) {
+ if (VersionUsesQpack(connection_->transport_version())) {
return;
}
QuicStreamId headers_stream_id =
diff --git a/quic/core/quic_utils.cc b/quic/core/quic_utils.cc
index 40c50af..160ac65 100644
--- a/quic/core/quic_utils.cc
+++ b/quic/core/quic_utils.cc
@@ -11,6 +11,7 @@
#include "net/third_party/quiche/src/quic/core/quic_connection_id.h"
#include "net/third_party/quiche/src/quic/core/quic_constants.h"
#include "net/third_party/quiche/src/quic/core/quic_types.h"
+#include "net/third_party/quiche/src/quic/core/quic_versions.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_aligned.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_arraysize.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_bug_tracker.h"
@@ -400,11 +401,7 @@
// static
QuicStreamId QuicUtils::GetHeadersStreamId(QuicTransportVersion version) {
- if (version == QUIC_VERSION_99) {
- // TODO(b/130659182) Turn this into a QUIC_BUG once we've fully removed
- // the headers stream in those versions.
- return GetQuicFlag(FLAGS_quic_headers_stream_id_in_v99);
- }
+ // TODO(b/138653948): Add DCHECK(!VersionUsesQpack(version));
return GetFirstBidirectionalStreamId(version, Perspective::IS_CLIENT);
}
diff --git a/quic/core/quic_versions.cc b/quic/core/quic_versions.cc
index 950dd1d..adfec06 100644
--- a/quic/core/quic_versions.cc
+++ b/quic/core/quic_versions.cc
@@ -80,17 +80,6 @@
return transport_version >= QUIC_VERSION_99;
}
-bool ParsedQuicVersion::DoesNotHaveHeadersStream() const {
- return VersionLacksHeadersStream(transport_version);
-}
-
-bool VersionLacksHeadersStream(QuicTransportVersion transport_version) {
- if (GetQuicFlag(FLAGS_quic_headers_stream_id_in_v99) == 0) {
- return false;
- }
- return transport_version == QUIC_VERSION_99;
-}
-
std::ostream& operator<<(std::ostream& os, const ParsedQuicVersion& version) {
os << ParsedQuicVersionToString(version);
return os;
@@ -451,7 +440,6 @@
// Enable necessary flags.
SetQuicFlag(FLAGS_quic_supports_tls_handshake, true);
- SetQuicFlag(FLAGS_quic_headers_stream_id_in_v99, 60);
SetQuicReloadableFlag(quic_simplify_stop_waiting, true);
SetQuicRestartFlag(quic_do_not_override_connection_id, true);
SetQuicRestartFlag(quic_use_allocated_connection_ids, true);
diff --git a/quic/core/quic_versions.h b/quic/core/quic_versions.h
index 72bb32c..1f35f7e 100644
--- a/quic/core/quic_versions.h
+++ b/quic/core/quic_versions.h
@@ -172,9 +172,6 @@
// Returns whether this version supports client connection ID.
bool SupportsClientConnectionIds() const;
-
- // Returns whether this version does not have the Google QUIC headers stream.
- bool DoesNotHaveHeadersStream() const;
};
QUIC_EXPORT_PRIVATE ParsedQuicVersion UnsupportedQuicVersion();
@@ -414,11 +411,6 @@
return transport_version >= QUIC_VERSION_48;
}
-// Returns whether |transport_version| does not have the
-// Google QUIC headers stream.
-QUIC_EXPORT_PRIVATE bool VersionLacksHeadersStream(
- QuicTransportVersion transport_version);
-
// Returns whether |transport_version| makes use of IETF QUIC
// frames or not.
QUIC_EXPORT_PRIVATE inline bool VersionHasIetfQuicFrames(
