Deprecate gfe2_restart_flag_quic_opport_bundle_qpack_decoder_data5.
PiperOrigin-RevId: 653047871
diff --git a/quiche/common/quiche_feature_flags_list.h b/quiche/common/quiche_feature_flags_list.h
index a00bbe5..831d27d 100755
--- a/quiche/common/quiche_feature_flags_list.h
+++ b/quiche/common/quiche_feature_flags_list.h
@@ -50,7 +50,6 @@
QUICHE_FLAG(bool, quiche_reloadable_flag_quic_testonly_default_false, false, false, "A testonly reloadable flag that will always default to false.")
QUICHE_FLAG(bool, quiche_reloadable_flag_quic_testonly_default_true, true, true, "A testonly reloadable flag that will always default to true.")
QUICHE_FLAG(bool, quiche_reloadable_flag_quic_use_received_client_addresses_cache, true, true, "If true, use a LRU cache to record client addresses of packets received on server's original address.")
-QUICHE_FLAG(bool, quiche_restart_flag_quic_opport_bundle_qpack_decoder_data5, true, true, "If true, bundle qpack decoder data with other frames opportunistically.")
QUICHE_FLAG(bool, quiche_restart_flag_quic_support_ect1, false, false, "When true, allows sending of QUIC packets marked ECT(1). A different flag (TBD) will actually utilize this capability to send ECT(1).")
QUICHE_FLAG(bool, quiche_restart_flag_quic_support_release_time_for_gso, false, false, "If true, QuicGsoBatchWriter will support release time if it is available and the process has the permission to do so.")
QUICHE_FLAG(bool, quiche_restart_flag_quic_testonly_default_false, false, false, "A testonly restart flag that will always default to false.")
diff --git a/quiche/quic/core/http/end_to_end_test.cc b/quiche/quic/core/http/end_to_end_test.cc
index f0a9d86..61e9d39 100644
--- a/quiche/quic/core/http/end_to_end_test.cc
+++ b/quiche/quic/core/http/end_to_end_test.cc
@@ -165,8 +165,8 @@
connection_id_length));
}
} // End of outer version loop.
- } // End of congestion_control_tag loop.
- } // End of connection_id_length loop.
+ } // End of congestion_control_tag loop.
+ } // End of connection_id_length loop.
// Only run every event loop implementation for one fixed configuration.
for (QuicEventLoopFactory* event_loop : GetAllSupportedEventLoops()) {
@@ -2879,15 +2879,9 @@
client_connection->GetStats().packets_sent;
if (version_.UsesHttp3()) {
- if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- // QPACK decoder instructions and RESET_STREAM and STOP_SENDING frames are
- // sent in a single packet.
- EXPECT_EQ(packets_sent_before + 1, packets_sent_now);
- } else {
- // Make sure 2 packets were sent, one for QPACK instructions, another for
- // RESET_STREAM and STOP_SENDING.
- EXPECT_EQ(packets_sent_before + 2, packets_sent_now);
- }
+ // QPACK decoder instructions and RESET_STREAM and STOP_SENDING frames are
+ // sent in a single packet.
+ EXPECT_EQ(packets_sent_before + 1, packets_sent_now);
}
// WaitForEvents waits 50ms and returns true if there are outstanding
@@ -3062,7 +3056,6 @@
return;
}
override_client_connection_id_length_ = kQuicDefaultConnectionIdLength;
- SetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5, false);
ASSERT_TRUE(Initialize());
SendSynchronousFooRequestAndCheckResponse();
@@ -3072,6 +3065,15 @@
QuicConnection* client_connection = GetClientConnection();
ASSERT_TRUE(client_connection != nullptr);
+ {
+ QuicConnection::ScopedPacketFlusher flusher(client_connection);
+ if (client_connection->SupportsMultiplePacketNumberSpaces()) {
+ client_connection->SendAllPendingAcks();
+ } else {
+ client_connection->SendAck();
+ }
+ }
+
// Migrate socket to a new IP address.
QuicIpAddress host1 = TestLoopback(2);
EXPECT_NE(host0, host1);
diff --git a/quiche/quic/core/http/quic_spdy_session.cc b/quiche/quic/core/http/quic_spdy_session.cc
index 72d7276..edfbb4a 100644
--- a/quiche/quic/core/http/quic_spdy_session.cc
+++ b/quiche/quic/core/http/quic_spdy_session.cc
@@ -867,8 +867,7 @@
}
bool QuicSpdySession::CheckStreamWriteBlocked(QuicStream* stream) const {
- if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5) &&
- qpack_decoder_send_stream_ != nullptr &&
+ if (qpack_decoder_send_stream_ != nullptr &&
stream->id() == qpack_decoder_send_stream_->id()) {
// Decoder data is always bundled opportunistically.
return true;
diff --git a/quiche/quic/core/http/quic_spdy_session_test.cc b/quiche/quic/core/http/quic_spdy_session_test.cc
index 8646ef3..e94a79c 100644
--- a/quiche/quic/core/http/quic_spdy_session_test.cc
+++ b/quiche/quic/core/http/quic_spdy_session_test.cc
@@ -1360,13 +1360,6 @@
OnStreamReset(GetNthClientInitiatedBidirectionalId(0), _));
}
- // In HTTP/3, Qpack stream will send data on stream reset and cause packet to
- // be flushed.
- if (VersionUsesHttp3(transport_version()) &&
- !GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- EXPECT_CALL(*writer_, WritePacket(_, _, _, _, _, _))
- .WillOnce(Return(WriteResult(WRITE_STATUS_OK, 0)));
- }
EXPECT_CALL(*connection_, SendControlFrame(_));
QuicRstStreamFrame rst1(kInvalidControlFrameId,
GetNthClientInitiatedBidirectionalId(0),
@@ -1608,9 +1601,6 @@
// the STOP_SENDING, so set up the EXPECT there.
EXPECT_CALL(*connection_, OnStreamReset(stream->id(), _));
EXPECT_CALL(*connection_, SendControlFrame(_));
- } else if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- EXPECT_CALL(*writer_, WritePacket(_, _, _, _, _, _))
- .WillOnce(Return(WriteResult(WRITE_STATUS_OK, 0)));
}
QuicRstStreamFrame rst_frame(kInvalidControlFrameId, stream->id(),
QUIC_STREAM_CANCELLED, kByteOffset);
diff --git a/quiche/quic/core/http/quic_spdy_stream_test.cc b/quiche/quic/core/http/quic_spdy_stream_test.cc
index 1b44549..2984396 100644
--- a/quiche/quic/core/http/quic_spdy_stream_test.cc
+++ b/quiche/quic/core/http/quic_spdy_stream_test.cc
@@ -590,15 +590,6 @@
stream_->id(),
QuicResetStreamError::FromInternal(QUIC_HEADERS_TOO_LARGE), 0));
- if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- auto qpack_decoder_stream =
- QuicSpdySessionPeer::GetQpackDecoderSendStream(session_.get());
- // Stream type and stream cancellation.
- EXPECT_CALL(*session_,
- WritevData(qpack_decoder_stream->id(), _, _, NO_FIN, _, _))
- .Times(2);
- }
-
stream_->OnStreamFrame(frame);
EXPECT_THAT(stream_->stream_error(), IsStreamError(QUIC_HEADERS_TOO_LARGE));
}
@@ -2250,9 +2241,6 @@
StrictMock<MockHttp3DebugVisitor> debug_visitor;
session_->set_debug_visitor(&debug_visitor);
- auto decoder_send_stream =
- QuicSpdySessionPeer::GetQpackDecoderSendStream(session_.get());
-
// Deliver dynamic table entry to decoder.
session_->qpack_decoder()->OnInsertWithoutNameReference("foo", "bar");
@@ -2264,16 +2252,6 @@
std::string headers = HeadersFrame(encoded_headers);
EXPECT_CALL(debug_visitor,
OnHeadersFrameReceived(stream_->id(), encoded_headers.length()));
- if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- // Decoder stream type.
- EXPECT_CALL(*session_,
- WritevData(decoder_send_stream->id(), /* write_length = */ 1,
- /* offset = */ 0, _, _, _));
- // Header acknowledgement.
- EXPECT_CALL(*session_,
- WritevData(decoder_send_stream->id(), /* write_length = */ 1,
- /* offset = */ 1, _, _, _));
- }
EXPECT_CALL(debug_visitor, OnHeadersDecoded(stream_->id(), _));
stream_->OnStreamFrame(QuicStreamFrame(stream_->id(), false, 0, headers));
@@ -2305,10 +2283,6 @@
EXPECT_CALL(debug_visitor,
OnHeadersFrameReceived(stream_->id(), encoded_trailers.length()));
// Header acknowledgement.
- if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- EXPECT_CALL(*session_,
- WritevData(decoder_send_stream->id(), _, _, _, _, _));
- }
EXPECT_CALL(debug_visitor, OnHeadersDecoded(stream_->id(), _));
stream_->OnStreamFrame(QuicStreamFrame(stream_->id(), true, /* offset = */
headers.length() + data.length(),
@@ -2346,19 +2320,6 @@
EXPECT_FALSE(stream_->headers_decompressed());
EXPECT_EQ(std::nullopt, stream_->header_decoding_delay());
- auto decoder_send_stream =
- QuicSpdySessionPeer::GetQpackDecoderSendStream(session_.get());
-
- if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- // Decoder stream type.
- EXPECT_CALL(*session_,
- WritevData(decoder_send_stream->id(), /* write_length = */ 1,
- /* offset = */ 0, _, _, _));
- // Header acknowledgement.
- EXPECT_CALL(*session_,
- WritevData(decoder_send_stream->id(), /* write_length = */ 1,
- /* offset = */ 1, _, _, _));
- }
EXPECT_CALL(debug_visitor, OnHeadersDecoded(stream_->id(), _));
const QuicTime::Delta delay = QuicTime::Delta::FromSeconds(1);
@@ -2395,11 +2356,6 @@
// Decoding is blocked because dynamic table entry has not been received yet.
EXPECT_FALSE(stream_->trailers_decompressed());
- if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- // Header acknowledgement.
- EXPECT_CALL(*session_,
- WritevData(decoder_send_stream->id(), _, _, _, _, _));
- }
EXPECT_CALL(debug_visitor, OnHeadersDecoded(stream_->id(), _));
// Deliver second dynamic table entry to decoder.
session_->qpack_decoder()->OnInsertWithoutNameReference("trailing", "foobar");
@@ -2432,20 +2388,8 @@
// Decoding is blocked because dynamic table entry has not been received yet.
EXPECT_FALSE(stream_->headers_decompressed());
- auto decoder_send_stream =
- QuicSpdySessionPeer::GetQpackDecoderSendStream(session_.get());
if (GetQuicReloadableFlag(
quic_stop_reading_also_stops_header_decompression)) {
- if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- // Stream type.
- EXPECT_CALL(*session_,
- WritevData(decoder_send_stream->id(), /* write_length = */ 1,
- /* offset = */ 0, _, _, _));
- // Send stream cancellation.
- EXPECT_CALL(*session_,
- WritevData(decoder_send_stream->id(), /* write_length = */ 1,
- /* offset = */ 1, _, _, _));
- }
EXPECT_CALL(debug_visitor, OnHeadersDecoded(stream_->id(), _)).Times(0);
}
// Stop reading from now on. Any buffered compressed headers shouldn't be
@@ -2454,16 +2398,6 @@
if (!GetQuicReloadableFlag(
quic_stop_reading_also_stops_header_decompression)) {
- if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- // Stream type.
- EXPECT_CALL(*session_,
- WritevData(decoder_send_stream->id(), /* write_length = */ 1,
- /* offset = */ 0, _, _, _));
- // Header acknowledgement.
- EXPECT_CALL(*session_,
- WritevData(decoder_send_stream->id(), /* write_length = */ 1,
- /* offset = */ 1, _, _, _));
- }
EXPECT_CALL(debug_visitor, OnHeadersDecoded(stream_->id(), _));
}
// Deliver dynamic table entry to decoder.
@@ -2555,19 +2489,6 @@
// Decoding is blocked because dynamic table entry has not been received yet.
EXPECT_FALSE(stream_->headers_decompressed());
- auto decoder_send_stream =
- QuicSpdySessionPeer::GetQpackDecoderSendStream(session_.get());
-
- if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- // Decoder stream type.
- EXPECT_CALL(*session_,
- WritevData(decoder_send_stream->id(), /* write_length = */ 1,
- /* offset = */ 0, _, _, _));
- // Header acknowledgement.
- EXPECT_CALL(*session_,
- WritevData(decoder_send_stream->id(), /* write_length = */ 1,
- /* offset = */ 1, _, _, _));
- }
// Deliver dynamic table entry to decoder.
session_->qpack_decoder()->OnInsertWithoutNameReference("foo", "bar");
EXPECT_TRUE(stream_->headers_decompressed());
@@ -2632,18 +2553,6 @@
// Decoding is blocked because dynamic table entry has not been received yet.
EXPECT_FALSE(stream_->headers_decompressed());
- if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- // Decoder stream type and stream cancellation instruction.
- auto decoder_send_stream =
- QuicSpdySessionPeer::GetQpackDecoderSendStream(session_.get());
- EXPECT_CALL(*session_,
- WritevData(decoder_send_stream->id(), /* write_length = */ 1,
- /* offset = */ 0, _, _, _));
- EXPECT_CALL(*session_,
- WritevData(decoder_send_stream->id(), /* write_length = */ 1,
- /* offset = */ 1, _, _, _));
- }
-
// Reset stream by this endpoint, for example, due to stream cancellation.
EXPECT_CALL(*session_, MaybeSendStopSendingFrame(
stream_->id(), QuicResetStreamError::FromInternal(
@@ -2683,18 +2592,6 @@
// Decoding is blocked because dynamic table entry has not been received yet.
EXPECT_FALSE(stream_->headers_decompressed());
- if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- // Decoder stream type and stream cancellation instruction.
- auto decoder_send_stream =
- QuicSpdySessionPeer::GetQpackDecoderSendStream(session_.get());
- EXPECT_CALL(*session_,
- WritevData(decoder_send_stream->id(), /* write_length = */ 1,
- /* offset = */ 0, _, _, _));
- EXPECT_CALL(*session_,
- WritevData(decoder_send_stream->id(), /* write_length = */ 1,
- /* offset = */ 1, _, _, _));
- }
-
// OnStreamReset() is called when RESET_STREAM frame is received from peer.
// This aborts header decompression.
stream_->OnStreamReset(QuicRstStreamFrame(
@@ -3042,18 +2939,7 @@
CloseConnection(QUIC_HTTP_INVALID_FRAME_SEQUENCE_ON_SPDY_STREAM,
"Unexpected DATA frame received.",
ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET))
- .WillOnce(InvokeWithoutArgs([this]() {
- auto* qpack_decoder_stream =
- QuicSpdySessionPeer::GetQpackDecoderSendStream(session_.get());
- if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5) &&
- GetQuicReloadableFlag(
- quic_stop_reading_also_stops_header_decompression)) {
- EXPECT_CALL(*session_,
- WritevData(qpack_decoder_stream->id(), _, _, _, _, _))
- .Times(2);
- }
- stream_->StopReading();
- }));
+ .WillOnce(InvokeWithoutArgs([this]() { stream_->StopReading(); }));
std::string data = DataFrame(kDataFramePayload);
stream_->OnStreamFrame(QuicStreamFrame(stream_->id(), false, 0, data));
@@ -3102,18 +2988,7 @@
CloseConnection(QUIC_HTTP_INVALID_FRAME_SEQUENCE_ON_SPDY_STREAM,
"HEADERS frame received after trailing HEADERS.",
ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET))
- .WillOnce(InvokeWithoutArgs([this]() {
- auto* qpack_decoder_stream =
- QuicSpdySessionPeer::GetQpackDecoderSendStream(session_.get());
- if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5) &&
- GetQuicReloadableFlag(
- quic_stop_reading_also_stops_header_decompression)) {
- EXPECT_CALL(*session_,
- WritevData(qpack_decoder_stream->id(), _, _, _, _, _))
- .Times(2);
- }
- stream_->StopReading();
- }));
+ .WillOnce(InvokeWithoutArgs([this]() { stream_->StopReading(); }));
// Receive another HEADERS frame, with no header fields.
std::string trailers2 = HeadersFrame(HttpHeaderBlock());
@@ -3163,18 +3038,7 @@
CloseConnection(QUIC_HTTP_INVALID_FRAME_SEQUENCE_ON_SPDY_STREAM,
"Unexpected DATA frame received.",
ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET))
- .WillOnce(InvokeWithoutArgs([this]() {
- auto* qpack_decoder_stream =
- QuicSpdySessionPeer::GetQpackDecoderSendStream(session_.get());
- if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5) &&
- GetQuicReloadableFlag(
- quic_stop_reading_also_stops_header_decompression)) {
- EXPECT_CALL(*session_,
- WritevData(qpack_decoder_stream->id(), _, _, _, _, _))
- .Times(2);
- }
- stream_->StopReading();
- }));
+ .WillOnce(InvokeWithoutArgs([this]() { stream_->StopReading(); }));
// Receive more data.
std::string data2 = DataFrame("This payload should not be processed.");
@@ -3226,18 +3090,6 @@
Initialize(kShouldProcessData);
- auto qpack_decoder_stream =
- QuicSpdySessionPeer::GetQpackDecoderSendStream(session_.get());
- if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- // Stream type.
- EXPECT_CALL(*session_,
- WritevData(qpack_decoder_stream->id(), /* write_length = */ 1,
- /* offset = */ 0, _, _, _));
- // Stream cancellation.
- EXPECT_CALL(*session_,
- WritevData(qpack_decoder_stream->id(), /* write_length = */ 1,
- /* offset = */ 1, _, _, _));
- }
EXPECT_CALL(*session_, MaybeSendStopSendingFrame(
stream_->id(), QuicResetStreamError::FromInternal(
QUIC_STREAM_CANCELLED)));
@@ -3259,19 +3111,6 @@
Initialize(kShouldProcessData);
- auto qpack_decoder_stream =
- QuicSpdySessionPeer::GetQpackDecoderSendStream(session_.get());
- if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- // Stream type.
- EXPECT_CALL(*session_,
- WritevData(qpack_decoder_stream->id(), /* write_length = */ 1,
- /* offset = */ 0, _, _, _));
- // Stream cancellation.
- EXPECT_CALL(*session_,
- WritevData(qpack_decoder_stream->id(), /* write_length = */ 1,
- /* offset = */ 1, _, _, _));
- }
-
stream_->OnStreamReset(QuicRstStreamFrame(
kInvalidControlFrameId, stream_->id(), QUIC_STREAM_CANCELLED, 0));
}
@@ -3655,16 +3494,6 @@
QuicStreamFrame frame(stream_->id(), /* fin = */ false, 0, data_frame);
stream_->OnStreamFrame(frame);
- if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- // As a result of resetting the stream, stream type and stream cancellation
- // are sent on the QPACK decoder stream.
- auto qpack_decoder_stream =
- QuicSpdySessionPeer::GetQpackDecoderSendStream(session_.get());
- EXPECT_CALL(*session_,
- WritevData(qpack_decoder_stream->id(), _, _, NO_FIN, _, _))
- .Times(2);
- }
-
stream_->OnStreamReset(QuicRstStreamFrame(
kInvalidControlFrameId, stream_->id(), QUIC_STREAM_NO_ERROR, 0));
diff --git a/quiche/quic/core/qpack/qpack_decoded_headers_accumulator_test.cc b/quiche/quic/core/qpack/qpack_decoded_headers_accumulator_test.cc
index cd31cd5..6e2cd4c 100644
--- a/quiche/quic/core/qpack/qpack_decoded_headers_accumulator_test.cc
+++ b/quiche/quic/core/qpack/qpack_decoded_headers_accumulator_test.cc
@@ -191,9 +191,7 @@
EXPECT_CALL(visitor_, OnHeadersDecoded(_, true));
qpack_decoder_.OnInsertWithoutNameReference("foo", "bar");
- if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- qpack_decoder_.FlushDecoderStream();
- }
+ qpack_decoder_.FlushDecoderStream();
}
TEST_F(QpackDecodedHeadersAccumulatorTest, BlockedDecoding) {
@@ -218,9 +216,7 @@
EXPECT_EQ(strlen("foo") + strlen("bar"),
header_list.uncompressed_header_bytes());
EXPECT_EQ(encoded_data.size(), header_list.compressed_header_bytes());
- if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- qpack_decoder_.FlushDecoderStream();
- }
+ qpack_decoder_.FlushDecoderStream();
}
TEST_F(QpackDecodedHeadersAccumulatorTest,
@@ -247,9 +243,7 @@
accumulator_.EndHeaderBlock();
EXPECT_THAT(header_list, ElementsAre(Pair("foo", "bar"), Pair("foo", "bar")));
- if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- qpack_decoder_.FlushDecoderStream();
- }
+ qpack_decoder_.FlushDecoderStream();
}
// Regression test for https://crbug.com/1024263.
diff --git a/quiche/quic/core/qpack/qpack_decoder.cc b/quiche/quic/core/qpack/qpack_decoder.cc
index f128fc9..49c7e81 100644
--- a/quiche/quic/core/qpack/qpack_decoder.cc
+++ b/quiche/quic/core/qpack/qpack_decoder.cc
@@ -32,9 +32,6 @@
void QpackDecoder::OnStreamReset(QuicStreamId stream_id) {
if (header_table_.maximum_dynamic_table_capacity() > 0) {
decoder_stream_sender_.SendStreamCancellation(stream_id);
- if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- decoder_stream_sender_.Flush();
- }
}
}
@@ -68,10 +65,6 @@
header_table_.inserted_entry_count() - known_received_count_);
known_received_count_ = header_table_.inserted_entry_count();
}
-
- if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- decoder_stream_sender_.Flush();
- }
}
void QpackDecoder::OnInsertWithNameReference(bool is_static,
diff --git a/quiche/quic/core/qpack/qpack_decoder_stream_sender.cc b/quiche/quic/core/qpack/qpack_decoder_stream_sender.cc
index 7ebc9a5..d512788 100644
--- a/quiche/quic/core/qpack/qpack_decoder_stream_sender.cc
+++ b/quiche/quic/core/qpack/qpack_decoder_stream_sender.cc
@@ -42,17 +42,12 @@
if (buffer_.empty() || delegate_ == nullptr) {
return;
}
- if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- QUIC_RESTART_FLAG_COUNT_N(quic_opport_bundle_qpack_decoder_data5, 3, 4);
- // Swap buffer_ before calling WriteStreamData, which might result in a
- // reentrant call to `Flush()`.
- std::string copy;
- std::swap(copy, buffer_);
- delegate_->WriteStreamData(copy);
- return;
- }
- delegate_->WriteStreamData(buffer_);
- buffer_.clear();
+
+ // Swap buffer_ before calling WriteStreamData, which might result in a
+ // reentrant call to `Flush()`.
+ std::string copy;
+ std::swap(copy, buffer_);
+ delegate_->WriteStreamData(copy);
}
} // namespace quic
diff --git a/quiche/quic/core/qpack/qpack_decoder_test.cc b/quiche/quic/core/qpack/qpack_decoder_test.cc
index 853ab54..b15ffc5 100644
--- a/quiche/quic/core/qpack/qpack_decoder_test.cc
+++ b/quiche/quic/core/qpack/qpack_decoder_test.cc
@@ -421,17 +421,10 @@
.InSequence(s);
EXPECT_CALL(handler_, OnHeaderDecoded(Eq("foo"), Eq("ZZZ"))).InSequence(s);
EXPECT_CALL(handler_, OnHeaderDecoded(Eq(":method"), Eq("ZZ"))).InSequence(s);
- if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- EXPECT_CALL(decoder_stream_sender_delegate_,
- WriteStreamData(Eq(kHeaderAcknowledgement)))
- .InSequence(s);
- }
EXPECT_CALL(handler_, OnDecodingCompleted()).InSequence(s);
- if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- EXPECT_CALL(decoder_stream_sender_delegate_,
- WriteStreamData(Eq(kHeaderAcknowledgement)))
- .InSequence(s);
- }
+ EXPECT_CALL(decoder_stream_sender_delegate_,
+ WriteStreamData(Eq(kHeaderAcknowledgement)))
+ .InSequence(s);
ASSERT_TRUE(absl::HexStringToBytes(
"0500" // Required Insert Count 4 and Delta Base 0.
@@ -444,9 +437,7 @@
// with value "ZZ".
&input));
DecodeHeaderBlock(input);
- if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- FlushDecoderStream();
- }
+ FlushDecoderStream();
EXPECT_CALL(handler_, OnHeaderDecoded(Eq("foo"), Eq("bar"))).InSequence(s);
EXPECT_CALL(handler_, OnHeaderDecoded(Eq("foo"), Eq("ZZZ"))).InSequence(s);
@@ -454,17 +445,10 @@
.InSequence(s);
EXPECT_CALL(handler_, OnHeaderDecoded(Eq("foo"), Eq("ZZZ"))).InSequence(s);
EXPECT_CALL(handler_, OnHeaderDecoded(Eq(":method"), Eq("ZZ"))).InSequence(s);
- if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- EXPECT_CALL(decoder_stream_sender_delegate_,
- WriteStreamData(Eq(kHeaderAcknowledgement)))
- .InSequence(s);
- }
EXPECT_CALL(handler_, OnDecodingCompleted()).InSequence(s);
- if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- EXPECT_CALL(decoder_stream_sender_delegate_,
- WriteStreamData(Eq(kHeaderAcknowledgement)))
- .InSequence(s);
- }
+ EXPECT_CALL(decoder_stream_sender_delegate_,
+ WriteStreamData(Eq(kHeaderAcknowledgement)))
+ .InSequence(s);
ASSERT_TRUE(absl::HexStringToBytes(
"0502" // Required Insert Count 4 and Delta Base 2.
@@ -477,9 +461,7 @@
// with value "ZZ".
&input));
DecodeHeaderBlock(input);
- if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- FlushDecoderStream();
- }
+ FlushDecoderStream();
EXPECT_CALL(handler_, OnHeaderDecoded(Eq("foo"), Eq("bar"))).InSequence(s);
EXPECT_CALL(handler_, OnHeaderDecoded(Eq("foo"), Eq("ZZZ"))).InSequence(s);
@@ -487,17 +469,10 @@
.InSequence(s);
EXPECT_CALL(handler_, OnHeaderDecoded(Eq("foo"), Eq("ZZZ"))).InSequence(s);
EXPECT_CALL(handler_, OnHeaderDecoded(Eq(":method"), Eq("ZZ"))).InSequence(s);
- if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- EXPECT_CALL(decoder_stream_sender_delegate_,
- WriteStreamData(Eq(kHeaderAcknowledgement)))
- .InSequence(s);
- }
EXPECT_CALL(handler_, OnDecodingCompleted()).InSequence(s);
- if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- EXPECT_CALL(decoder_stream_sender_delegate_,
- WriteStreamData(Eq(kHeaderAcknowledgement)))
- .InSequence(s);
- }
+ EXPECT_CALL(decoder_stream_sender_delegate_,
+ WriteStreamData(Eq(kHeaderAcknowledgement)))
+ .InSequence(s);
ASSERT_TRUE(absl::HexStringToBytes(
"0582" // Required Insert Count 4 and Delta Base 2 with sign bit set.
@@ -510,9 +485,7 @@
// with value "ZZ".
&input));
DecodeHeaderBlock(input);
- if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- FlushDecoderStream();
- }
+ FlushDecoderStream();
}
TEST_P(QpackDecoderTest, DecreasingDynamicTableCapacityEvictsEntries) {
@@ -551,9 +524,7 @@
"80", // Dynamic table entry with relative index 0, absolute index 0.
&input));
DecodeHeaderBlock(input);
- if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- FlushDecoderStream();
- }
+ FlushDecoderStream();
}
TEST_P(QpackDecoderTest, EncoderStreamErrorEntryTooLarge) {
@@ -838,9 +809,7 @@
"80", // Emit dynamic table entry with relative index 0.
&input));
DecodeHeaderBlock(input);
- if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- FlushDecoderStream();
- }
+ FlushDecoderStream();
}
TEST_P(QpackDecoderTest, NonZeroRequiredInsertCountButNoDynamicEntries) {
@@ -1033,9 +1002,7 @@
// Add literal entry with name "foo" and value "bar".
ASSERT_TRUE(absl::HexStringToBytes("6294e703626172", &input));
DecodeEncoderStreamData(input);
- if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- FlushDecoderStream();
- }
+ FlushDecoderStream();
}
TEST_P(QpackDecoderTest, BlockedDecodingUnblockedBeforeEndOfHeaderBlock) {
@@ -1080,9 +1047,7 @@
EXPECT_CALL(decoder_stream_sender_delegate_,
WriteStreamData(Eq(kHeaderAcknowledgement)));
EndDecoding();
- if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- FlushDecoderStream();
- }
+ FlushDecoderStream();
}
// Regression test for https://crbug.com/1024263.
@@ -1148,9 +1113,7 @@
// Insert Count is now 6, reaching Required Insert Count of the header block.
ASSERT_TRUE(absl::HexStringToBytes("6294e70362617a", &input));
DecodeEncoderStreamData(input);
- if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- FlushDecoderStream();
- }
+ FlushDecoderStream();
}
TEST_P(QpackDecoderTest, TooManyBlockedStreams) {
@@ -1200,9 +1163,7 @@
"80", // Dynamic table entry with relative index 0, absolute index 0.
&input));
DecodeHeaderBlock(input);
- if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- FlushDecoderStream();
- }
+ FlushDecoderStream();
}
} // namespace
diff --git a/quiche/quic/core/qpack/qpack_send_stream_test.cc b/quiche/quic/core/qpack/qpack_send_stream_test.cc
index 3315ee1..0ef0624 100644
--- a/quiche/quic/core/qpack/qpack_send_stream_test.cc
+++ b/quiche/quic/core/qpack/qpack_send_stream_test.cc
@@ -133,7 +133,6 @@
}
TEST_P(QpackSendStreamTest, GetSendWindowSizeFromSession) {
- SetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5, true);
EXPECT_NE(session_.GetFlowControlSendWindowSize(qpack_send_stream_->id()),
std::numeric_limits<QuicByteCount>::max());
}
diff --git a/quiche/quic/core/quic_connection.cc b/quiche/quic/core/quic_connection.cc
index cbb2c69..c81536b 100644
--- a/quiche/quic/core/quic_connection.cc
+++ b/quiche/quic/core/quic_connection.cc
@@ -3066,35 +3066,21 @@
void QuicConnection::MaybeBundleOpportunistically(
TransmissionType transmission_type) {
- if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- QUIC_RESTART_FLAG_COUNT_N(quic_opport_bundle_qpack_decoder_data5, 1, 4);
+ const bool should_bundle_ack_frequency =
+ !ack_frequency_sent_ && sent_packet_manager_.CanSendAckFrequency() &&
+ transmission_type == NOT_RETRANSMISSION &&
+ packet_creator_.NextSendingPacketNumber() >=
+ FirstSendingPacketNumber() + kMinReceivedBeforeAckDecimation;
- const bool should_bundle_ack_frequency =
- !ack_frequency_sent_ && sent_packet_manager_.CanSendAckFrequency() &&
- transmission_type == NOT_RETRANSMISSION &&
- packet_creator_.NextSendingPacketNumber() >=
- FirstSendingPacketNumber() + kMinReceivedBeforeAckDecimation;
+ if (should_bundle_ack_frequency) {
+ QUIC_RELOADABLE_FLAG_COUNT_N(quic_can_send_ack_frequency, 3, 3);
+ ack_frequency_sent_ = true;
+ auto frame = sent_packet_manager_.GetUpdatedAckFrequencyFrame();
+ visitor_->SendAckFrequency(frame);
+ }
- if (should_bundle_ack_frequency) {
- QUIC_RELOADABLE_FLAG_COUNT_N(quic_can_send_ack_frequency, 3, 3);
- ack_frequency_sent_ = true;
- auto frame = sent_packet_manager_.GetUpdatedAckFrequencyFrame();
- visitor_->SendAckFrequency(frame);
- }
-
- if (transmission_type == NOT_RETRANSMISSION) {
- visitor_->MaybeBundleOpportunistically();
- }
- } else {
- if (!ack_frequency_sent_ && sent_packet_manager_.CanSendAckFrequency()) {
- if (packet_creator_.NextSendingPacketNumber() >=
- FirstSendingPacketNumber() + kMinReceivedBeforeAckDecimation) {
- QUIC_RELOADABLE_FLAG_COUNT_N(quic_can_send_ack_frequency, 3, 3);
- ack_frequency_sent_ = true;
- auto frame = sent_packet_manager_.GetUpdatedAckFrequencyFrame();
- visitor_->SendAckFrequency(frame);
- }
- }
+ if (transmission_type == NOT_RETRANSMISSION) {
+ visitor_->MaybeBundleOpportunistically();
}
if (packet_creator_.has_ack() || !CanWrite(NO_RETRANSMITTABLE_DATA)) {
@@ -5745,10 +5731,7 @@
uber_received_packet_manager_.GetEarliestAckTimeout();
QUIC_BUG_IF(quic_bug_12714_32, !earliest_ack_timeout.IsInitialized());
MaybeBundleCryptoDataWithAcks();
- if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- QUIC_RESTART_FLAG_COUNT_N(quic_opport_bundle_qpack_decoder_data5, 2, 4);
- visitor_->MaybeBundleOpportunistically();
- }
+ visitor_->MaybeBundleOpportunistically();
earliest_ack_timeout = uber_received_packet_manager_.GetEarliestAckTimeout();
if (!earliest_ack_timeout.IsInitialized()) {
return;
diff --git a/quiche/quic/core/quic_connection_test.cc b/quiche/quic/core/quic_connection_test.cc
index 28bb07c..e040c76 100644
--- a/quiche/quic/core/quic_connection_test.cc
+++ b/quiche/quic/core/quic_connection_test.cc
@@ -704,10 +704,8 @@
EXPECT_CALL(visitor_, OnCongestionWindowChange(_)).Times(AnyNumber());
EXPECT_CALL(visitor_, OnPacketReceived(_, _, _)).Times(AnyNumber());
EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)).Times(AnyNumber());
- if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- EXPECT_CALL(visitor_, MaybeBundleOpportunistically()).Times(AnyNumber());
- EXPECT_CALL(visitor_, GetFlowControlSendWindowSize(_)).Times(AnyNumber());
- }
+ EXPECT_CALL(visitor_, MaybeBundleOpportunistically()).Times(AnyNumber());
+ EXPECT_CALL(visitor_, GetFlowControlSendWindowSize(_)).Times(AnyNumber());
EXPECT_CALL(visitor_, OnOneRttPacketAcknowledged())
.Times(testing::AtMost(1));
EXPECT_CALL(*loss_algorithm_, GetLossTimeout())
diff --git a/quiche/quic/core/quic_packet_creator.cc b/quiche/quic/core/quic_packet_creator.cc
index aecfc73..ca35a07 100644
--- a/quiche/quic/core/quic_packet_creator.cc
+++ b/quiche/quic/core/quic_packet_creator.cc
@@ -1314,11 +1314,6 @@
}
void QuicPacketCreator::MaybeBundleOpportunistically() {
- if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- delegate_->MaybeBundleOpportunistically(next_transmission_type_);
- return;
- }
-
// delegate_->MaybeBundleOpportunistically() may change
// next_transmission_type_ for the bundled data.
const TransmissionType next_transmission_type = next_transmission_type_;
@@ -1337,14 +1332,6 @@
bool has_handshake = QuicUtils::IsCryptoStreamId(transport_version(), id);
const TransmissionType next_transmission_type = next_transmission_type_;
MaybeBundleOpportunistically();
- // TODO(wub): Remove this QUIC_BUG when deprecating
- // quic_opport_bundle_qpack_decoder_data5.
- QUIC_BUG_IF(quic_packet_creator_change_transmission_type,
- next_transmission_type != next_transmission_type_)
- << ENDPOINT
- << "Transmission type changed by bundled data. old transmission type:"
- << next_transmission_type
- << ", new transmission type:" << next_transmission_type_;
// If the data being consumed is subject to flow control, check the flow
// control send window to see if |write_length| exceeds the send window after
// bundling opportunistic data, if so, reduce |write_length| to the send
@@ -1354,11 +1341,9 @@
// - And it's not handshake data. This is always true for ConsumeData because
// the function is not called for handshake data.
const size_t original_write_length = write_length;
- if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5) &&
- next_transmission_type_ == NOT_RETRANSMISSION) {
+ if (next_transmission_type_ == NOT_RETRANSMISSION) {
if (QuicByteCount send_window = delegate_->GetFlowControlSendWindowSize(id);
write_length > send_window) {
- QUIC_RESTART_FLAG_COUNT_N(quic_opport_bundle_qpack_decoder_data5, 4, 4);
QUIC_DLOG(INFO) << ENDPOINT
<< "After bundled data, reducing (old) write_length:"
<< write_length << "to (new) send_window:" << send_window;
diff --git a/quiche/quic/core/quic_packet_creator_test.cc b/quiche/quic/core/quic_packet_creator_test.cc
index b527b09..2d06c6c 100644
--- a/quiche/quic/core/quic_packet_creator_test.cc
+++ b/quiche/quic/core/quic_packet_creator_test.cc
@@ -2932,22 +2932,13 @@
QuicStreamId stream_id = QuicUtils::GetFirstBidirectionalStreamId(
framer_.transport_version(), Perspective::IS_CLIENT);
- if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- EXPECT_CALL(delegate_, GetFlowControlSendWindowSize(stream_id))
- .WillOnce(Return(data.length() - 1));
- } else {
- EXPECT_CALL(delegate_, GetFlowControlSendWindowSize(_)).Times(0);
- }
+ EXPECT_CALL(delegate_, GetFlowControlSendWindowSize(stream_id))
+ .WillOnce(Return(data.length() - 1));
QuicConsumedData consumed = creator_.ConsumeData(stream_id, data, 0u, FIN);
- if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- EXPECT_EQ(consumed.bytes_consumed, data.length() - 1);
- EXPECT_FALSE(consumed.fin_consumed);
- } else {
- EXPECT_EQ(consumed.bytes_consumed, data.length());
- EXPECT_TRUE(consumed.fin_consumed);
- }
+ EXPECT_EQ(consumed.bytes_consumed, data.length() - 1);
+ EXPECT_FALSE(consumed.fin_consumed);
}
// Tests the case that after bundling data, send window is exactly as big as
@@ -2962,12 +2953,8 @@
QuicStreamId stream_id = QuicUtils::GetFirstBidirectionalStreamId(
framer_.transport_version(), Perspective::IS_CLIENT);
- if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) {
- EXPECT_CALL(delegate_, GetFlowControlSendWindowSize(stream_id))
- .WillOnce(Return(data.length()));
- } else {
- EXPECT_CALL(delegate_, GetFlowControlSendWindowSize(_)).Times(0);
- }
+ EXPECT_CALL(delegate_, GetFlowControlSendWindowSize(stream_id))
+ .WillOnce(Return(data.length()));
QuicConsumedData consumed = creator_.ConsumeData(stream_id, data, 0u, FIN);
diff --git a/quiche/quic/core/quic_session.cc b/quiche/quic/core/quic_session.cc
index a8b113b..2ee8651 100644
--- a/quiche/quic/core/quic_session.cc
+++ b/quiche/quic/core/quic_session.cc
@@ -2508,7 +2508,6 @@
}
QuicByteCount QuicSession::GetFlowControlSendWindowSize(QuicStreamId id) {
- QUICHE_DCHECK(GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5));
auto it = stream_map_.find(id);
if (it == stream_map_.end()) {
// No flow control for invalid or inactive stream ids. Returning uint64max
