Deprecate --gfe2_restart_flag_http2_parse_priority_update_frame. PiperOrigin-RevId: 360234035 Change-Id: I1ce9538d2ae9b660bd39de7c2b5988082916da9b
diff --git a/http2/decoder/http2_frame_decoder.cc b/http2/decoder/http2_frame_decoder.cc index 449d649..9509c0c 100644 --- a/http2/decoder/http2_frame_decoder.cc +++ b/http2/decoder/http2_frame_decoder.cc
@@ -158,12 +158,7 @@ break; case Http2FrameType::PRIORITY_UPDATE: - if (GetHttp2RestartFlag(http2_parse_priority_update_frame)) { - HTTP2_RESTART_FLAG_COUNT_N(http2_parse_priority_update_frame, 1, 2); - status = StartDecodingPriorityUpdatePayload(&subset); - } else { - status = StartDecodingUnknownPayload(&subset); - } + status = StartDecodingPriorityUpdatePayload(&subset); break; default: @@ -237,12 +232,7 @@ break; case Http2FrameType::PRIORITY_UPDATE: - if (GetHttp2RestartFlag(http2_parse_priority_update_frame)) { - HTTP2_RESTART_FLAG_COUNT_N(http2_parse_priority_update_frame, 2, 2); - status = ResumeDecodingPriorityUpdatePayload(&subset); - } else { - status = ResumeDecodingUnknownPayload(&subset); - } + status = ResumeDecodingPriorityUpdatePayload(&subset); break; default:
diff --git a/http2/decoder/http2_frame_decoder_test.cc b/http2/decoder/http2_frame_decoder_test.cc index 0b0b415..b09a5d0 100644 --- a/http2/decoder/http2_frame_decoder_test.cc +++ b/http2/decoder/http2_frame_decoder_test.cc
@@ -558,16 +558,9 @@ }; Http2FrameHeader header(7, Http2FrameType::PRIORITY_UPDATE, 0, 0); - if (GetHttp2RestartFlag(http2_parse_priority_update_frame)) { - FrameParts expected(header, "abc"); - expected.SetOptPriorityUpdate(Http2PriorityUpdateFields{5}); - EXPECT_TRUE(DecodePayloadAndValidateSeveralWays(kFrameData, expected)); - } else { - FrameParts expected(header, absl::string_view("\x00\x00\x00\x05" - "abc", - 7)); - EXPECT_TRUE(DecodePayloadAndValidateSeveralWays(kFrameData, expected)); - } + FrameParts expected(header, "abc"); + expected.SetOptPriorityUpdate(Http2PriorityUpdateFields{5}); + EXPECT_TRUE(DecodePayloadAndValidateSeveralWays(kFrameData, expected)); } TEST_F(Http2FrameDecoderTest, UnknownPayload) {
diff --git a/http2/decoder/payload_decoders/priority_update_payload_decoder_test.cc b/http2/decoder/payload_decoders/priority_update_payload_decoder_test.cc index 39d0afc..41b7dc8 100644 --- a/http2/decoder/payload_decoders/priority_update_payload_decoder_test.cc +++ b/http2/decoder/payload_decoders/priority_update_payload_decoder_test.cc
@@ -61,31 +61,14 @@ } }; -// Avoid initialization of test class when flag is false, because base class -// method AbstractPayloadDecoderTest::SetUp() crashes if -// IsSupportedHttp2FrameType(PRIORITY_UPDATE) returns false. -std::vector<bool> GetTestParams() { - if (GetHttp2RestartFlag(http2_parse_priority_update_frame)) { - return {true}; // Actual Boolean value is ignored. - } else { - return {}; - } -} - class PriorityUpdatePayloadDecoderTest : public AbstractPayloadDecoderTest<PriorityUpdatePayloadDecoder, PriorityUpdatePayloadDecoderPeer, - Listener>, - public ::testing::WithParamInterface<bool> {}; - -INSTANTIATE_TEST_SUITE_P(MaybeRunTest, - PriorityUpdatePayloadDecoderTest, - ::testing::ValuesIn(GetTestParams())); -GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(PriorityUpdatePayloadDecoderTest); + Listener> {}; // Confirm we get an error if the payload is not long enough to hold // Http2PriorityUpdateFields. -TEST_P(PriorityUpdatePayloadDecoderTest, Truncated) { +TEST_F(PriorityUpdatePayloadDecoderTest, Truncated) { auto approve_size = [](size_t size) { return size != Http2PriorityUpdateFields::EncodedSize(); }; @@ -98,9 +81,9 @@ : public AbstractPayloadDecoderTest<PriorityUpdatePayloadDecoder, PriorityUpdatePayloadDecoderPeer, Listener>, - public ::testing::WithParamInterface<std::tuple<uint32_t, bool>> { + public ::testing::WithParamInterface<uint32_t> { protected: - PriorityUpdatePayloadLengthTests() : length_(std::get<0>(GetParam())) { + PriorityUpdatePayloadLengthTests() : length_(GetParam()) { HTTP2_VLOG(1) << "################ length_=" << length_ << " ################"; } @@ -108,12 +91,9 @@ const uint32_t length_; }; -INSTANTIATE_TEST_SUITE_P( - VariousLengths, - PriorityUpdatePayloadLengthTests, - ::testing::Combine(::testing::Values(0, 1, 2, 3, 4, 5, 6), - ::testing::ValuesIn(GetTestParams()))); -GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(PriorityUpdatePayloadLengthTests); +INSTANTIATE_TEST_SUITE_P(VariousLengths, + PriorityUpdatePayloadLengthTests, + ::testing::Values(0, 1, 2, 3, 4, 5, 6)); TEST_P(PriorityUpdatePayloadLengthTests, ValidLength) { Http2PriorityUpdateFields priority_update;
diff --git a/http2/http2_constants.h b/http2/http2_constants.h index cd05eb4..65e55db 100644 --- a/http2/http2_constants.h +++ b/http2/http2_constants.h
@@ -48,12 +48,8 @@ // Is the frame type known/supported? inline bool IsSupportedHttp2FrameType(uint32_t v) { - if (GetHttp2RestartFlag(http2_parse_priority_update_frame) && - v == static_cast<uint32_t>(Http2FrameType::PRIORITY_UPDATE)) { - return true; - } - - return v <= static_cast<uint32_t>(Http2FrameType::ALTSVC); + return v <= static_cast<uint32_t>(Http2FrameType::ALTSVC) || + v == static_cast<uint32_t>(Http2FrameType::PRIORITY_UPDATE); } inline bool IsSupportedHttp2FrameType(Http2FrameType v) { return IsSupportedHttp2FrameType(static_cast<uint32_t>(v));
diff --git a/quic/core/quic_flags_list.h b/quic/core/quic_flags_list.h index f37ccbc..aa5f569 100644 --- a/quic/core/quic_flags_list.h +++ b/quic/core/quic_flags_list.h
@@ -67,7 +67,6 @@ QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_use_write_or_buffer_data_at_level, false) QUIC_FLAG(FLAGS_quic_reloadable_flag_send_quic_fallback_server_config_on_leto_error, false) QUIC_FLAG(FLAGS_quic_restart_flag_dont_fetch_quic_private_keys_from_leto, false) -QUIC_FLAG(FLAGS_quic_restart_flag_http2_parse_priority_update_frame, true) QUIC_FLAG(FLAGS_quic_restart_flag_quic_dispatcher_support_multiple_cid_per_connection_v2, true) QUIC_FLAG(FLAGS_quic_restart_flag_quic_enable_zero_rtt_for_tls_v2, true) QUIC_FLAG(FLAGS_quic_restart_flag_quic_offload_pacing_to_usps2, false)
diff --git a/spdy/core/spdy_framer_test.cc b/spdy/core/spdy_framer_test.cc index 96b9f4c..d7fb6a7 100644 --- a/spdy/core/spdy_framer_test.cc +++ b/spdy/core/spdy_framer_test.cc
@@ -4570,12 +4570,7 @@ testing::StrictMock<test::MockSpdyFramerVisitor> visitor; deframer_.set_visitor(&visitor); - if (GetHttp2RestartFlag(http2_parse_priority_update_frame)) { - EXPECT_CALL(visitor, OnPriorityUpdate(3, "foo")); - } else { - EXPECT_CALL(visitor, OnUnknownFrame(0, _)).WillOnce(testing::Return(true)); - } - + EXPECT_CALL(visitor, OnPriorityUpdate(3, "foo")); deframer_.ProcessInput(kFrameData, sizeof(kFrameData)); EXPECT_FALSE(deframer_.HasError()); } @@ -4592,12 +4587,7 @@ testing::StrictMock<test::MockSpdyFramerVisitor> visitor; deframer_.set_visitor(&visitor); - if (GetHttp2RestartFlag(http2_parse_priority_update_frame)) { - EXPECT_CALL(visitor, OnPriorityUpdate(3, "")); - } else { - EXPECT_CALL(visitor, OnUnknownFrame(0, _)).WillOnce(testing::Return(true)); - } - + EXPECT_CALL(visitor, OnPriorityUpdate(3, "")); deframer_.ProcessInput(kFrameData, sizeof(kFrameData)); EXPECT_FALSE(deframer_.HasError()); } @@ -4613,17 +4603,10 @@ testing::StrictMock<test::MockSpdyFramerVisitor> visitor; deframer_.set_visitor(&visitor); - if (GetHttp2RestartFlag(http2_parse_priority_update_frame)) { - EXPECT_CALL( - visitor, - OnError(Http2DecoderAdapter::SPDY_INVALID_CONTROL_FRAME_SIZE, _)); - deframer_.ProcessInput(kFrameData, sizeof(kFrameData)); - EXPECT_TRUE(deframer_.HasError()); - } else { - EXPECT_CALL(visitor, OnUnknownFrame(0, _)).WillOnce(testing::Return(true)); - deframer_.ProcessInput(kFrameData, sizeof(kFrameData)); - EXPECT_FALSE(deframer_.HasError()); - } + EXPECT_CALL(visitor, + OnError(Http2DecoderAdapter::SPDY_INVALID_CONTROL_FRAME_SIZE, _)); + deframer_.ProcessInput(kFrameData, sizeof(kFrameData)); + EXPECT_TRUE(deframer_.HasError()); } TEST_P(SpdyFramerTest, PriorityUpdateFrameWithShortPayload) { @@ -4639,17 +4622,10 @@ testing::StrictMock<test::MockSpdyFramerVisitor> visitor; deframer_.set_visitor(&visitor); - if (GetHttp2RestartFlag(http2_parse_priority_update_frame)) { - EXPECT_CALL( - visitor, - OnError(Http2DecoderAdapter::SPDY_INVALID_CONTROL_FRAME_SIZE, _)); - deframer_.ProcessInput(kFrameData, sizeof(kFrameData)); - EXPECT_TRUE(deframer_.HasError()); - } else { - EXPECT_CALL(visitor, OnUnknownFrame(0, _)).WillOnce(testing::Return(true)); - deframer_.ProcessInput(kFrameData, sizeof(kFrameData)); - EXPECT_FALSE(deframer_.HasError()); - } + EXPECT_CALL(visitor, + OnError(Http2DecoderAdapter::SPDY_INVALID_CONTROL_FRAME_SIZE, _)); + deframer_.ProcessInput(kFrameData, sizeof(kFrameData)); + EXPECT_TRUE(deframer_.HasError()); } TEST_P(SpdyFramerTest, PriorityUpdateFrameOnIncorrectStream) { @@ -4664,16 +4640,9 @@ testing::StrictMock<test::MockSpdyFramerVisitor> visitor; deframer_.set_visitor(&visitor); - if (GetHttp2RestartFlag(http2_parse_priority_update_frame)) { - EXPECT_CALL(visitor, - OnError(Http2DecoderAdapter::SPDY_INVALID_STREAM_ID, _)); - deframer_.ProcessInput(kFrameData, sizeof(kFrameData)); - EXPECT_TRUE(deframer_.HasError()); - } else { - EXPECT_CALL(visitor, OnUnknownFrame(1, _)).WillOnce(testing::Return(true)); - deframer_.ProcessInput(kFrameData, sizeof(kFrameData)); - EXPECT_FALSE(deframer_.HasError()); - } + EXPECT_CALL(visitor, OnError(Http2DecoderAdapter::SPDY_INVALID_STREAM_ID, _)); + deframer_.ProcessInput(kFrameData, sizeof(kFrameData)); + EXPECT_TRUE(deframer_.HasError()); } TEST_P(SpdyFramerTest, PriorityUpdateFramePrioritizingIncorrectStream) { @@ -4688,16 +4657,9 @@ testing::StrictMock<test::MockSpdyFramerVisitor> visitor; deframer_.set_visitor(&visitor); - if (GetHttp2RestartFlag(http2_parse_priority_update_frame)) { - EXPECT_CALL(visitor, - OnError(Http2DecoderAdapter::SPDY_INVALID_STREAM_ID, _)); - deframer_.ProcessInput(kFrameData, sizeof(kFrameData)); - EXPECT_TRUE(deframer_.HasError()); - } else { - EXPECT_CALL(visitor, OnUnknownFrame(0, _)).WillOnce(testing::Return(true)); - deframer_.ProcessInput(kFrameData, sizeof(kFrameData)); - EXPECT_FALSE(deframer_.HasError()); - } + EXPECT_CALL(visitor, OnError(Http2DecoderAdapter::SPDY_INVALID_STREAM_ID, _)); + deframer_.ProcessInput(kFrameData, sizeof(kFrameData)); + EXPECT_TRUE(deframer_.HasError()); } // Tests handling of PRIORITY frames.