Add more PRIORITY_UPDATE tests.
Test that if QuicStream::SetPriority() is called with the priority that the
stream already has, then QuicSpdyStream::MaybeSendPriorityUpdateFrame() does not
send a PRIORITY_UPDATE frame. This was prompted by automated mutation testing
commenting on cl/494755193 that removing the line
http://google3/third_party/quic/core/http/quic_spdy_stream.cc;l=595;rcl=491635277
does not cause any test failures. Now it does.
Also add some ParsePriorityFieldValue tests to exercise code for ignoring inner
list dict values, and some tests for operator==() with out-of-bounds urgency
values. Inspired by reviewers' comments and mutation testing findings on
cl/494755193.
Also add QuicSpdySessionTest to exercise parsing the incremental bit. Note that
this is still unused, but at least parsed.
PiperOrigin-RevId: 495008021
diff --git a/quiche/quic/core/http/quic_spdy_session_test.cc b/quiche/quic/core/http/quic_spdy_session_test.cc
index 209b4f8..550ba80 100644
--- a/quiche/quic/core/http/quic_spdy_session_test.cc
+++ b/quiche/quic/core/http/quic_spdy_session_test.cc
@@ -2233,7 +2233,7 @@
// PRIORITY_UPDATE frame for second request stream.
const QuicStreamId stream_id2 = GetNthClientInitiatedBidirectionalId(1);
- PriorityUpdateFrame priority_update2{stream_id2, "u=2"};
+ PriorityUpdateFrame priority_update2{stream_id2, "u=5, i"};
std::string serialized_priority_update2 =
HttpEncoder::SerializePriorityUpdateFrame(priority_update2);
QuicStreamFrame stream_frame3(receive_control_stream_id,
@@ -2246,8 +2246,11 @@
session_.OnStreamFrame(stream_frame3);
// Priority is applied upon stream construction.
TestStream* stream2 = session_.CreateIncomingStream(stream_id2);
- EXPECT_EQ((QuicStreamPriority{2u, QuicStreamPriority::kDefaultIncremental}),
- stream2->priority());
+ if (GetQuicReloadableFlag(quic_priority_update_structured_headers_parser)) {
+ EXPECT_EQ((QuicStreamPriority{5u, true}), stream2->priority());
+ } else {
+ EXPECT_EQ((QuicStreamPriority{5u, false}), stream2->priority());
+ }
}
TEST_P(QuicSpdySessionTestServer, OnInvalidPriorityUpdateFrame) {
diff --git a/quiche/quic/core/http/quic_spdy_stream_test.cc b/quiche/quic/core/http/quic_spdy_stream_test.cc
index 0325538..dbc033e 100644
--- a/quiche/quic/core/http/quic_spdy_stream_test.cc
+++ b/quiche/quic/core/http/quic_spdy_stream_test.cc
@@ -1566,6 +1566,7 @@
EXPECT_CALL(*stream_, WriteHeadersMock(false));
EXPECT_CALL(debug_visitor, OnHeadersFrameSent(stream_->id(), _));
stream_->WriteHeaders(Http2HeaderBlock(), /*fin=*/false, nullptr);
+ testing::Mock::VerifyAndClearExpectations(&debug_visitor);
// PRIORITY_UPDATE frame on the control stream.
auto send_control_stream =
@@ -1573,14 +1574,22 @@
EXPECT_CALL(*session_, WritevData(send_control_stream->id(), _, _, _, _, _));
PriorityUpdateFrame priority_update1{stream_->id(), "u=0"};
EXPECT_CALL(debug_visitor, OnPriorityUpdateFrameSent(priority_update1));
- stream_->SetPriority(QuicStreamPriority{
- kV3HighestPriority, QuicStreamPriority::kDefaultIncremental});
+ const QuicStreamPriority priority1{kV3HighestPriority,
+ QuicStreamPriority::kDefaultIncremental};
+ stream_->SetPriority(priority1);
+ testing::Mock::VerifyAndClearExpectations(&debug_visitor);
// Send another PRIORITY_UPDATE frame with incremental flag set to true.
EXPECT_CALL(*session_, WritevData(send_control_stream->id(), _, _, _, _, _));
PriorityUpdateFrame priority_update2{stream_->id(), "u=2, i"};
EXPECT_CALL(debug_visitor, OnPriorityUpdateFrameSent(priority_update2));
- stream_->SetPriority(QuicStreamPriority{2, true});
+ const QuicStreamPriority priority2{2, true};
+ stream_->SetPriority(priority2);
+ testing::Mock::VerifyAndClearExpectations(&debug_visitor);
+
+ // Calling SetPriority() with the same priority does not trigger sending
+ // another PRIORITY_UPDATE frame.
+ stream_->SetPriority(priority2);
}
TEST_P(QuicSpdyStreamTest, ChangePriorityBeforeWritingHeaders) {
diff --git a/quiche/quic/core/quic_stream_priority.cc b/quiche/quic/core/quic_stream_priority.cc
index 377866a..9729706 100644
--- a/quiche/quic/core/quic_stream_priority.cc
+++ b/quiche/quic/core/quic_stream_priority.cc
@@ -57,6 +57,9 @@
const std::vector<quiche::structured_headers::ParameterizedItem>& member =
value.member;
if (member.size() != 1) {
+ // If `member_is_inner_list` is false above,
+ // then `member` should have exactly one element.
+ QUICHE_BUG(priority_field_value_parsing_internal_error);
continue;
}
diff --git a/quiche/quic/core/quic_stream_priority_test.cc b/quiche/quic/core/quic_stream_priority_test.cc
index 50b6662..1ebbabb 100644
--- a/quiche/quic/core/quic_stream_priority_test.cc
+++ b/quiche/quic/core/quic_stream_priority_test.cc
@@ -21,10 +21,13 @@
QuicStreamPriority::kDefaultIncremental}));
EXPECT_EQ((QuicStreamPriority{5, true}), (QuicStreamPriority{5, true}));
EXPECT_EQ((QuicStreamPriority{2, false}), (QuicStreamPriority{2, false}));
+ EXPECT_EQ((QuicStreamPriority{11, true}), (QuicStreamPriority{11, true}));
EXPECT_NE((QuicStreamPriority{1, true}), (QuicStreamPriority{3, true}));
EXPECT_NE((QuicStreamPriority{4, false}), (QuicStreamPriority{4, true}));
EXPECT_NE((QuicStreamPriority{6, true}), (QuicStreamPriority{2, false}));
+ EXPECT_NE((QuicStreamPriority{12, true}), (QuicStreamPriority{9, true}));
+ EXPECT_NE((QuicStreamPriority{2, false}), (QuicStreamPriority{8, false}));
}
TEST(SerializePriorityFieldValueTest, SerializePriorityFieldValue) {
@@ -102,6 +105,12 @@
// Cannot be parsed as structured headers.
result = ParsePriorityFieldValue("000");
EXPECT_FALSE(result.success);
+
+ // Inner list dictionary values are ignored.
+ result = ParsePriorityFieldValue("a=(1 2), u=1");
+ EXPECT_TRUE(result.success);
+ EXPECT_EQ(1, result.priority.urgency);
+ EXPECT_FALSE(result.priority.incremental);
}
} // namespace quic::test
