Change default QuicStream priority when using IETF QUIC.
This allows removal of MaybeSendPriorityUpdateFrame() call in
QuicSpdyStream::WriteHeaders(): if priority is default, no need to send
PRIORITY_UPDATE; if SetPriority() has been called, then PRIORITY_UPDATE has
already been sent.
gfe-relnote: n/a, change to QUIC v99-only code. Protected by existing disabled gfe2_reloadable_flag_quic_enable_version_99.
PiperOrigin-RevId: 292745471
Change-Id: Ic9fff95e6a1650a91449a659fbe156265f2b9cac
diff --git a/quic/core/http/quic_spdy_session_test.cc b/quic/core/http/quic_spdy_session_test.cc
index 11c5fda..b813d43 100644
--- a/quic/core/http/quic_spdy_session_test.cc
+++ b/quic/core/http/quic_spdy_session_test.cc
@@ -2172,7 +2172,7 @@
struct PriorityUpdateFrame priority_update1;
priority_update1.prioritized_element_type = REQUEST_STREAM;
priority_update1.prioritized_element_id = stream_id1;
- priority_update1.priority_field_value = "u=1";
+ priority_update1.priority_field_value = "u=2";
std::string serialized_priority_update1 =
SerializePriorityUpdateFrame(priority_update1);
QuicStreamFrame data3(receive_control_stream_id,
@@ -2181,9 +2181,10 @@
// PRIORITY_UPDATE frame arrives after stream creation.
TestStream* stream1 = session_.CreateIncomingStream(stream_id1);
- EXPECT_EQ(3u, stream1->precedence().spdy3_priority());
+ EXPECT_EQ(QuicStream::kDefaultUrgency,
+ stream1->precedence().spdy3_priority());
session_.OnStreamFrame(data3);
- EXPECT_EQ(1u, stream1->precedence().spdy3_priority());
+ EXPECT_EQ(2u, stream1->precedence().spdy3_priority());
// PRIORITY_UPDATE frame for second request stream.
const QuicStreamId stream_id2 = GetNthClientInitiatedBidirectionalId(1);
diff --git a/quic/core/http/quic_spdy_stream.cc b/quic/core/http/quic_spdy_stream.cc
index b393ee3..89748a9 100644
--- a/quic/core/http/quic_spdy_stream.cc
+++ b/quic/core/http/quic_spdy_stream.cc
@@ -281,8 +281,6 @@
false, nullptr);
}
- MaybeSendPriorityUpdateFrame();
-
size_t bytes_written =
WriteHeadersImpl(std::move(header_block), fin, std::move(ack_listener));
if (!VersionUsesHttp3(transport_version()) && fin) {
diff --git a/quic/core/http/quic_spdy_stream.h b/quic/core/http/quic_spdy_stream.h
index c75a268..923ff7c 100644
--- a/quic/core/http/quic_spdy_stream.h
+++ b/quic/core/http/quic_spdy_stream.h
@@ -43,12 +43,6 @@
: public QuicStream,
public QpackDecodedHeadersAccumulator::Visitor {
public:
- // The default value of urgency when using the priority extension defined at
- // https://httpwg.org/http-extensions/draft-ietf-httpbis-priority.html#default.
- // This is not the default priority of a stream, rather the default priority a
- // server thinks a stream has until it receives a PRIORITY_UPDATE frame.
- static const int kDefaultUrgency = 1;
-
// Visitor receives callbacks from the stream.
class QUIC_EXPORT_PRIVATE Visitor {
public:
diff --git a/quic/core/http/quic_spdy_stream_test.cc b/quic/core/http/quic_spdy_stream_test.cc
index 5d0a162..9f7e894 100644
--- a/quic/core/http/quic_spdy_stream_test.cc
+++ b/quic/core/http/quic_spdy_stream_test.cc
@@ -1311,34 +1311,6 @@
EXPECT_TRUE(stream_->fin_sent());
}
-TEST_P(QuicSpdyStreamTest, SendPriorityUpdate) {
- if (!UsesHttp3()) {
- return;
- }
-
- InitializeWithPerspective(kShouldProcessData, Perspective::IS_CLIENT);
-
- // Four writes on the request stream: HEADERS frame header and payload both
- // for headers and trailers.
- EXPECT_CALL(*session_, WritevData(stream_, stream_->id(), _, _, _)).Times(4);
- // PRIORITY_UPDATE frame on the control stream.
- auto send_control_stream =
- QuicSpdySessionPeer::GetSendControlStream(session_.get());
- EXPECT_CALL(*session_, WritevData(send_control_stream,
- send_control_stream->id(), _, _, _));
-
- // Write the initial headers, without a FIN.
- EXPECT_CALL(*stream_, WriteHeadersMock(false));
- stream_->WriteHeaders(SpdyHeaderBlock(), /*fin=*/false, nullptr);
-
- // Writing trailers implicitly sends a FIN.
- SpdyHeaderBlock trailers;
- trailers["trailer key"] = "trailer value";
- EXPECT_CALL(*stream_, WriteHeadersMock(true));
- stream_->WriteTrailers(std::move(trailers), nullptr);
- EXPECT_TRUE(stream_->fin_sent());
-}
-
TEST_P(QuicSpdyStreamTest, DoNotSendPriorityUpdateWithDefaultUrgency) {
if (!UsesHttp3()) {
return;
@@ -1350,10 +1322,8 @@
// for headers and trailers.
EXPECT_CALL(*session_, WritevData(stream_, stream_->id(), _, _, _)).Times(4);
- stream_->SetPriority(
- spdy::SpdyStreamPrecedence(QuicSpdyStream::kDefaultUrgency));
-
- // No PRIORITY_UPDATE frames on the control stream.
+ // No PRIORITY_UPDATE frames on the control stream,
+ // because the stream has default priority.
auto send_control_stream =
QuicSpdySessionPeer::GetSendControlStream(session_.get());
EXPECT_CALL(*session_, WritevData(send_control_stream,
@@ -1382,17 +1352,13 @@
// Two writes on the request stream: HEADERS frame header and payload.
EXPECT_CALL(*session_, WritevData(stream_, stream_->id(), _, _, _)).Times(2);
EXPECT_CALL(*stream_, WriteHeadersMock(false));
+ stream_->WriteHeaders(SpdyHeaderBlock(), /*fin=*/false, nullptr);
+
// PRIORITY_UPDATE frame on the control stream.
auto send_control_stream =
QuicSpdySessionPeer::GetSendControlStream(session_.get());
EXPECT_CALL(*session_, WritevData(send_control_stream,
send_control_stream->id(), _, _, _));
- stream_->WriteHeaders(SpdyHeaderBlock(), /*fin=*/false, nullptr);
- testing::Mock::VerifyAndClearExpectations(session_.get());
-
- // Another PRIORITY_UPDATE frame.
- EXPECT_CALL(*session_, WritevData(send_control_stream,
- send_control_stream->id(), _, _, _));
stream_->SetPriority(spdy::SpdyStreamPrecedence(kV3HighestPriority));
}
diff --git a/quic/core/quic_stream.cc b/quic/core/quic_stream.cc
index bcdf0d9..f2c7c88 100644
--- a/quic/core/quic_stream.cc
+++ b/quic/core/quic_stream.cc
@@ -106,6 +106,9 @@
// static
const SpdyPriority QuicStream::kDefaultPriority;
+// static
+const int QuicStream::kDefaultUrgency;
+
PendingStream::PendingStream(QuicStreamId id, QuicSession* session)
: id_(id),
session_(session),
@@ -334,12 +337,7 @@
: sequencer_(std::move(sequencer)),
id_(id),
session_(session),
- precedence_(
- session->use_http2_priority_write_scheduler()
- ? spdy::SpdyStreamPrecedence(0,
- spdy::kHttp2DefaultStreamWeight,
- false)
- : spdy::SpdyStreamPrecedence(kDefaultPriority)),
+ precedence_(CalculateDefaultPriority(session)),
stream_bytes_read_(stream_bytes_read),
stream_error_(QUIC_STREAM_NO_ERROR),
connection_error_(QUIC_NO_ERROR),
@@ -1193,4 +1191,19 @@
session_->SendStopSending(code, id_);
}
+// static
+spdy::SpdyStreamPrecedence QuicStream::CalculateDefaultPriority(
+ const QuicSession* session) {
+ if (VersionUsesHttp3(session->transport_version())) {
+ return spdy::SpdyStreamPrecedence(QuicStream::kDefaultUrgency);
+ }
+
+ if (session->use_http2_priority_write_scheduler()) {
+ return spdy::SpdyStreamPrecedence(0, spdy::kHttp2DefaultStreamWeight,
+ false);
+ }
+
+ return spdy::SpdyStreamPrecedence(QuicStream::kDefaultPriority);
+}
+
} // namespace quic
diff --git a/quic/core/quic_stream.h b/quic/core/quic_stream.h
index bd680f6..0688641 100644
--- a/quic/core/quic_stream.h
+++ b/quic/core/quic_stream.h
@@ -114,11 +114,15 @@
// This is somewhat arbitrary. It's possible, but unlikely, we will either
// fail to set a priority client-side, or cancel a stream before stripping the
// priority from the wire server-side. In either case, start out with a
- // priority in the middle.
+ // priority in the middle in case of Google QUIC.
static const spdy::SpdyPriority kDefaultPriority = 3;
static_assert(kDefaultPriority ==
(spdy::kV3LowestPriority + spdy::kV3HighestPriority) / 2,
"Unexpected value of kDefaultPriority");
+ // On the other hand, when using IETF QUIC, use the default value defined by
+ // the priority extension at
+ // https://httpwg.org/http-extensions/draft-ietf-httpbis-priority.html#default.
+ static const int kDefaultUrgency = 1;
// Creates a new stream with stream_id |id| associated with |session|. If
// |is_static| is true, then the stream will be given precedence
@@ -353,6 +357,9 @@
// Returns true if the stream is static.
bool is_static() const { return is_static_; }
+ static spdy::SpdyStreamPrecedence CalculateDefaultPriority(
+ const QuicSession* session);
+
protected:
// Close the read side of the socket. May cause the stream to be closed.
// Subclasses and consumers should use StopReading to terminate reading early
