Remove PriorityWriteScheduler::root_stream_id_. In practice, PriorityWriteScheduler is always used with spdy::StreamPrecedence objects that have `is_spdy3_priority == true`, and the QUICHE_BUGs on `root_stream_id_` are never triggered. PiperOrigin-RevId: 499839152
diff --git a/quiche/http2/core/priority_write_scheduler.h b/quiche/http2/core/priority_write_scheduler.h index 7335545..6f7b92f 100644 --- a/quiche/http2/core/priority_write_scheduler.h +++ b/quiche/http2/core/priority_write_scheduler.h
@@ -45,28 +45,8 @@ public: using typename WriteScheduler<StreamIdType>::StreamPrecedenceType; - // Creates scheduler with no streams. - PriorityWriteScheduler() : PriorityWriteScheduler(spdy::kHttp2RootStreamId) {} - explicit PriorityWriteScheduler(StreamIdType root_stream_id) - : root_stream_id_(root_stream_id) {} - void RegisterStream(StreamIdType stream_id, const StreamPrecedenceType& precedence) override { - // TODO(mpw): verify |precedence.is_spdy3_priority() == true| once - // Http2PriorityWriteScheduler enabled for HTTP/2. - - // parent_id not used here, but may as well validate it. However, - // parent_id may legitimately not be registered yet--see b/15676312. - StreamIdType parent_id = precedence.parent_id(); - QUICHE_DVLOG_IF( - 1, parent_id != root_stream_id_ && !StreamRegistered(parent_id)) - << "Parent stream " << parent_id << " not registered"; - - if (stream_id == root_stream_id_) { - QUICHE_BUG(spdy_bug_19_1) - << "Stream " << root_stream_id_ << " already registered"; - return; - } auto stream_info = std::make_unique<StreamInfo>( StreamInfo{precedence.spdy3_priority(), stream_id, false}); bool inserted = @@ -107,16 +87,6 @@ void UpdateStreamPrecedence(StreamIdType stream_id, const StreamPrecedenceType& precedence) override { - // TODO(mpw): verify |precedence.is_spdy3_priority() == true| once - // Http2PriorityWriteScheduler enabled for HTTP/2. - - // parent_id not used here, but may as well validate it. However, - // parent_id may legitimately not be registered yet--see b/15676312. - StreamIdType parent_id = precedence.parent_id(); - QUICHE_DVLOG_IF( - 1, parent_id != root_stream_id_ && !StreamRegistered(parent_id)) - << "Parent stream " << parent_id << " not registered"; - auto it = stream_infos_.find(stream_id); if (it == stream_infos_.end()) { // TODO(mpw): add to stream_infos_ on demand--see b/15676312. @@ -331,7 +301,6 @@ PriorityInfo priority_infos_[spdy::kV3LowestPriority + 1]; // StreamInfos for all registered streams. StreamInfoMap stream_infos_; - StreamIdType root_stream_id_; }; } // namespace http2
diff --git a/quiche/http2/core/priority_write_scheduler_test.cc b/quiche/http2/core/priority_write_scheduler_test.cc index 9be325f..e94e522 100644 --- a/quiche/http2/core/priority_write_scheduler_test.cc +++ b/quiche/http2/core/priority_write_scheduler_test.cc
@@ -51,16 +51,14 @@ EXPECT_TRUE(scheduler_.StreamRegistered(1)); EXPECT_EQ(1u, scheduler_.NumRegisteredStreams()); - // Root stream counts as already registered. - EXPECT_QUICHE_BUG( - scheduler_.RegisterStream(kHttp2RootStreamId, SpdyStreamPrecedence(1)), - "Stream 0 already registered"); - // Try redundant registrations. EXPECT_QUICHE_BUG(scheduler_.RegisterStream(1, SpdyStreamPrecedence(1)), "Stream 1 already registered"); + EXPECT_EQ(1u, scheduler_.NumRegisteredStreams()); + EXPECT_QUICHE_BUG(scheduler_.RegisterStream(1, SpdyStreamPrecedence(2)), "Stream 1 already registered"); + EXPECT_EQ(1u, scheduler_.NumRegisteredStreams()); scheduler_.RegisterStream(2, SpdyStreamPrecedence(3)); EXPECT_EQ(2u, scheduler_.NumRegisteredStreams()); @@ -76,6 +74,7 @@ // Try redundant unregistration. EXPECT_QUICHE_BUG(scheduler_.UnregisterStream(1), "Stream 1 not registered"); EXPECT_QUICHE_BUG(scheduler_.UnregisterStream(2), "Stream 2 not registered"); + EXPECT_EQ(0u, scheduler_.NumRegisteredStreams()); } TEST_F(PriorityWriteSchedulerTest, RegisterStreamWithHttp2StreamDependency) {
diff --git a/quiche/quic/core/quic_session.cc b/quiche/quic/core/quic_session.cc index 898a869..7f3312e 100644 --- a/quiche/quic/core/quic_session.cc +++ b/quiche/quic/core/quic_session.cc
@@ -73,7 +73,6 @@ : connection_(connection), perspective_(connection->perspective()), visitor_(owner), - write_blocked_streams_(connection->transport_version()), config_(config), stream_id_manager_(perspective(), connection->transport_version(), kDefaultMaxStreamsPerConnection,
diff --git a/quiche/quic/core/quic_write_blocked_list.cc b/quiche/quic/core/quic_write_blocked_list.cc index d54ccca..9e53c94 100644 --- a/quiche/quic/core/quic_write_blocked_list.cc +++ b/quiche/quic/core/quic_write_blocked_list.cc
@@ -9,17 +9,11 @@ namespace quic { -QuicWriteBlockedList::QuicWriteBlockedList(QuicTransportVersion version) - : priority_write_scheduler_(QuicVersionUsesCryptoFrames(version) - ? std::numeric_limits<QuicStreamId>::max() - : 0), - last_priority_popped_(0) { +QuicWriteBlockedList::QuicWriteBlockedList() : last_priority_popped_(0) { memset(batch_write_stream_id_, 0, sizeof(batch_write_stream_id_)); memset(bytes_left_for_batch_write_, 0, sizeof(bytes_left_for_batch_write_)); } -QuicWriteBlockedList::~QuicWriteBlockedList() {} - bool QuicWriteBlockedList::ShouldYield(QuicStreamId id) const { for (const auto& stream : static_stream_collection_) { if (stream.id == id) {
diff --git a/quiche/quic/core/quic_write_blocked_list.h b/quiche/quic/core/quic_write_blocked_list.h index 30eb337..f0f86a8 100644 --- a/quiche/quic/core/quic_write_blocked_list.h +++ b/quiche/quic/core/quic_write_blocked_list.h
@@ -25,10 +25,10 @@ // Crypto stream > Headers stream > Data streams by requested priority. class QUIC_EXPORT_PRIVATE QuicWriteBlockedList { public: - explicit QuicWriteBlockedList(QuicTransportVersion version); + explicit QuicWriteBlockedList(); QuicWriteBlockedList(const QuicWriteBlockedList&) = delete; QuicWriteBlockedList& operator=(const QuicWriteBlockedList&) = delete; - ~QuicWriteBlockedList(); + ~QuicWriteBlockedList() = default; bool HasWriteBlockedDataStreams() const { return priority_write_scheduler_.HasReadyStreams();
diff --git a/quiche/quic/core/quic_write_blocked_list_test.cc b/quiche/quic/core/quic_write_blocked_list_test.cc index ba02a08..c04b891 100644 --- a/quiche/quic/core/quic_write_blocked_list_test.cc +++ b/quiche/quic/core/quic_write_blocked_list_test.cc
@@ -20,10 +20,6 @@ constexpr bool kNotIncremental = false; class QuicWriteBlockedListTest : public QuicTestWithParam<bool> { - public: - QuicWriteBlockedListTest() - : write_blocked_list_(AllSupportedVersions()[0].transport_version) {} - protected: QuicWriteBlockedList write_blocked_list_; };