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