Automated g4 rollback of changelist 503440382. *** Reason for rollback *** Unfortunately Chromium still needs this in QuicChromiumClientSession::UnregisterStreamPriority(). *** Original change description *** Remove `is_static` argument from QuicWriteBlockedList::UnregisterStream(). This argument is unnecessary, because QuicWriteBlockedList can easily determine whether the stream has been registered as static or not. *** PiperOrigin-RevId: 504273217
diff --git a/quiche/quic/core/http/quic_server_session_base_test.cc b/quiche/quic/core/http/quic_server_session_base_test.cc index 807f05f..133dcbf 100644 --- a/quiche/quic/core/http/quic_server_session_base_test.cc +++ b/quiche/quic/core/http/quic_server_session_base_test.cc
@@ -525,7 +525,8 @@ if (!VersionUsesHttp3(transport_version())) { session_->UnregisterStreamPriority( - QuicUtils::GetHeadersStreamId(transport_version())); + QuicUtils::GetHeadersStreamId(transport_version()), + /*is_static=*/true); } QuicServerSessionBasePeer::SetCryptoStream(session_.get(), nullptr); MockQuicCryptoServerStream* quic_crypto_stream = nullptr;
diff --git a/quiche/quic/core/quic_session.cc b/quiche/quic/core/quic_session.cc index 4b37c76..a58e572 100644 --- a/quiche/quic/core/quic_session.cc +++ b/quiche/quic/core/quic_session.cc
@@ -1816,8 +1816,8 @@ write_blocked_streams()->RegisterStream(id, is_static, priority); } -void QuicSession::UnregisterStreamPriority(QuicStreamId id) { - write_blocked_streams()->UnregisterStream(id); +void QuicSession::UnregisterStreamPriority(QuicStreamId id, bool is_static) { + write_blocked_streams()->UnregisterStream(id, is_static); } void QuicSession::UpdateStreamPriority(QuicStreamId id,
diff --git a/quiche/quic/core/quic_session.h b/quiche/quic/core/quic_session.h index f59d184..5c97f1a 100644 --- a/quiche/quic/core/quic_session.h +++ b/quiche/quic/core/quic_session.h
@@ -334,7 +334,7 @@ void RegisterStreamPriority(QuicStreamId id, bool is_static, const QuicStreamPriority& priority) override; // Clears priority from the write blocked list. - void UnregisterStreamPriority(QuicStreamId id) override; + void UnregisterStreamPriority(QuicStreamId id, bool is_static) override; // Updates priority on the write blocked list. void UpdateStreamPriority(QuicStreamId id, const QuicStreamPriority& new_priority) override;
diff --git a/quiche/quic/core/quic_stream.cc b/quiche/quic/core/quic_stream.cc index 6a436ff..3fa0d01 100644 --- a/quiche/quic/core/quic_stream.cc +++ b/quiche/quic/core/quic_stream.cc
@@ -390,7 +390,7 @@ << ", fin_outstanding: " << fin_outstanding_; } if (stream_delegate_ != nullptr && type_ != CRYPTO) { - stream_delegate_->UnregisterStreamPriority(id()); + stream_delegate_->UnregisterStreamPriority(id(), is_static_); } }
diff --git a/quiche/quic/core/quic_write_blocked_list.cc b/quiche/quic/core/quic_write_blocked_list.cc index f6d00f2..2497907 100644 --- a/quiche/quic/core/quic_write_blocked_list.cc +++ b/quiche/quic/core/quic_write_blocked_list.cc
@@ -68,8 +68,10 @@ priority_write_scheduler_.RegisterStream(stream_id, priority); } -void QuicWriteBlockedList::UnregisterStream(QuicStreamId stream_id) { - if (static_stream_collection_.Unregister(stream_id)) { +void QuicWriteBlockedList::UnregisterStream(QuicStreamId stream_id, + bool is_static) { + if (is_static) { + static_stream_collection_.Unregister(stream_id); return; } priority_write_scheduler_.UnregisterStream(stream_id); @@ -127,17 +129,17 @@ return false; } -bool QuicWriteBlockedList::StaticStreamCollection::Unregister(QuicStreamId id) { +void QuicWriteBlockedList::StaticStreamCollection::Unregister(QuicStreamId id) { for (auto it = streams_.begin(); it != streams_.end(); ++it) { if (it->id == id) { if (it->is_blocked) { --num_blocked_; } streams_.erase(it); - return true; + return; } } - return false; + QUICHE_DCHECK(false) << "Erasing a non-exist stream with id " << id; } bool QuicWriteBlockedList::StaticStreamCollection::SetBlocked(QuicStreamId id) {
diff --git a/quiche/quic/core/quic_write_blocked_list.h b/quiche/quic/core/quic_write_blocked_list.h index b366eaf..0ad2c2d 100644 --- a/quiche/quic/core/quic_write_blocked_list.h +++ b/quiche/quic/core/quic_write_blocked_list.h
@@ -63,9 +63,7 @@ void RegisterStream(QuicStreamId stream_id, bool is_static_stream, const QuicStreamPriority& priority); - // Unregister a stream. `stream_id` must be registered, either as a static - // stream or as a non-static stream. - void UnregisterStream(QuicStreamId stream_id); + void UnregisterStream(QuicStreamId stream_id, bool is_static); // Updates the stored priority of a stream. Must not be called for static // streams. @@ -126,9 +124,9 @@ // True if |id| is in the collection, regardless of its state. bool IsRegistered(QuicStreamId id) const; - // Remove |id| from the collection. If it is in the blocked state, reduce - // |num_blocked_| by 1. Returns true if |id| was in the collection. - bool Unregister(QuicStreamId id); + // Remove |id| from the collection, if it is in the blocked state, reduce + // |num_blocked_| by 1. + void Unregister(QuicStreamId id); // Set |id| to be blocked. If |id| is not already blocked, increase // |num_blocked_| by 1.
diff --git a/quiche/quic/core/quic_write_blocked_list_test.cc b/quiche/quic/core/quic_write_blocked_list_test.cc index e69e071..1847993 100644 --- a/quiche/quic/core/quic_write_blocked_list_test.cc +++ b/quiche/quic/core/quic_write_blocked_list_test.cc
@@ -6,7 +6,6 @@ #include "quiche/quic/platform/api/quic_test.h" #include "quiche/quic/test_tools/quic_test_utils.h" -#include "quiche/common/platform/api/quiche_expect_bug.h" using spdy::kV3HighestPriority; using spdy::kV3LowestPriority; @@ -54,8 +53,8 @@ write_blocked_list_.RegisterStream(stream_id, is_static_stream, priority); } - void UnregisterStream(QuicStreamId stream_id) { - write_blocked_list_.UnregisterStream(stream_id); + void UnregisterStream(QuicStreamId stream_id, bool is_static) { + write_blocked_list_.UnregisterStream(stream_id, is_static); } void UpdateStreamPriority(QuicStreamId stream_id, @@ -296,8 +295,8 @@ AddStream(1); AddStream(3); - UnregisterStream(23); - UnregisterStream(1); + UnregisterStream(23, kNotStatic); + UnregisterStream(1, kStatic); EXPECT_EQ(3u, PopFront()); EXPECT_EQ(17u, PopFront()); @@ -305,14 +304,6 @@ EXPECT_EQ(40, PopFront()); } -TEST_F(QuicWriteBlockedListTest, UnregisterNotRegisteredStream) { - EXPECT_QUICHE_BUG(UnregisterStream(1), "Stream 1 not registered"); - - RegisterStream(2, kNotStatic, {kV3HighestPriority, kIncremental}); - UnregisterStream(2); - EXPECT_QUICHE_BUG(UnregisterStream(2), "Stream 2 not registered"); -} - TEST_F(QuicWriteBlockedListTest, UpdateStreamPriority) { RegisterStream(40, kNotStatic, {kV3LowestPriority, kNotIncremental}); RegisterStream(23, kNotStatic, {6, kIncremental});
diff --git a/quiche/quic/core/stream_delegate_interface.h b/quiche/quic/core/stream_delegate_interface.h index a5f03f0..b6db350 100644 --- a/quiche/quic/core/stream_delegate_interface.h +++ b/quiche/quic/core/stream_delegate_interface.h
@@ -44,7 +44,7 @@ virtual void RegisterStreamPriority(QuicStreamId id, bool is_static, const QuicStreamPriority& priority) = 0; // Called on stream destruction to clear priority. - virtual void UnregisterStreamPriority(QuicStreamId id) = 0; + virtual void UnregisterStreamPriority(QuicStreamId id, bool is_static) = 0; // Called by the stream on SetPriority to update priority. virtual void UpdateStreamPriority(QuicStreamId id, const QuicStreamPriority& new_priority) = 0;
diff --git a/quiche/quic/tools/quic_simple_server_session_test.cc b/quiche/quic/tools/quic_simple_server_session_test.cc index c30283c..d6eb67f 100644 --- a/quiche/quic/tools/quic_simple_server_session_test.cc +++ b/quiche/quic/tools/quic_simple_server_session_test.cc
@@ -491,7 +491,8 @@ if (!VersionUsesHttp3(transport_version())) { session_->UnregisterStreamPriority( - QuicUtils::GetHeadersStreamId(transport_version())); + QuicUtils::GetHeadersStreamId(transport_version()), + /*is_static=*/true); } // Assume encryption already established. QuicSimpleServerSessionPeer::SetCryptoStream(session_.get(), nullptr);