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: 503440382
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 133dcbf..807f05f 100644
--- a/quiche/quic/core/http/quic_server_session_base_test.cc
+++ b/quiche/quic/core/http/quic_server_session_base_test.cc
@@ -525,8 +525,7 @@
if (!VersionUsesHttp3(transport_version())) {
session_->UnregisterStreamPriority(
- QuicUtils::GetHeadersStreamId(transport_version()),
- /*is_static=*/true);
+ QuicUtils::GetHeadersStreamId(transport_version()));
}
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 a58e572..4b37c76 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, bool is_static) {
- write_blocked_streams()->UnregisterStream(id, is_static);
+void QuicSession::UnregisterStreamPriority(QuicStreamId id) {
+ write_blocked_streams()->UnregisterStream(id);
}
void QuicSession::UpdateStreamPriority(QuicStreamId id,
diff --git a/quiche/quic/core/quic_session.h b/quiche/quic/core/quic_session.h
index 5c97f1a..f59d184 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, bool is_static) override;
+ void UnregisterStreamPriority(QuicStreamId id) 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 3fa0d01..6a436ff 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(), is_static_);
+ stream_delegate_->UnregisterStreamPriority(id());
}
}
diff --git a/quiche/quic/core/quic_write_blocked_list.cc b/quiche/quic/core/quic_write_blocked_list.cc
index 2497907..f6d00f2 100644
--- a/quiche/quic/core/quic_write_blocked_list.cc
+++ b/quiche/quic/core/quic_write_blocked_list.cc
@@ -68,10 +68,8 @@
priority_write_scheduler_.RegisterStream(stream_id, priority);
}
-void QuicWriteBlockedList::UnregisterStream(QuicStreamId stream_id,
- bool is_static) {
- if (is_static) {
- static_stream_collection_.Unregister(stream_id);
+void QuicWriteBlockedList::UnregisterStream(QuicStreamId stream_id) {
+ if (static_stream_collection_.Unregister(stream_id)) {
return;
}
priority_write_scheduler_.UnregisterStream(stream_id);
@@ -129,17 +127,17 @@
return false;
}
-void QuicWriteBlockedList::StaticStreamCollection::Unregister(QuicStreamId id) {
+bool 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;
+ return true;
}
}
- QUICHE_DCHECK(false) << "Erasing a non-exist stream with id " << id;
+ return false;
}
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 0ad2c2d..b366eaf 100644
--- a/quiche/quic/core/quic_write_blocked_list.h
+++ b/quiche/quic/core/quic_write_blocked_list.h
@@ -63,7 +63,9 @@
void RegisterStream(QuicStreamId stream_id, bool is_static_stream,
const QuicStreamPriority& priority);
- void UnregisterStream(QuicStreamId stream_id, bool is_static);
+ // Unregister a stream. `stream_id` must be registered, either as a static
+ // stream or as a non-static stream.
+ void UnregisterStream(QuicStreamId stream_id);
// Updates the stored priority of a stream. Must not be called for static
// streams.
@@ -124,9 +126,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.
- void Unregister(QuicStreamId id);
+ // 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);
// 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 1847993..e69e071 100644
--- a/quiche/quic/core/quic_write_blocked_list_test.cc
+++ b/quiche/quic/core/quic_write_blocked_list_test.cc
@@ -6,6 +6,7 @@
#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;
@@ -53,8 +54,8 @@
write_blocked_list_.RegisterStream(stream_id, is_static_stream, priority);
}
- void UnregisterStream(QuicStreamId stream_id, bool is_static) {
- write_blocked_list_.UnregisterStream(stream_id, is_static);
+ void UnregisterStream(QuicStreamId stream_id) {
+ write_blocked_list_.UnregisterStream(stream_id);
}
void UpdateStreamPriority(QuicStreamId stream_id,
@@ -295,8 +296,8 @@
AddStream(1);
AddStream(3);
- UnregisterStream(23, kNotStatic);
- UnregisterStream(1, kStatic);
+ UnregisterStream(23);
+ UnregisterStream(1);
EXPECT_EQ(3u, PopFront());
EXPECT_EQ(17u, PopFront());
@@ -304,6 +305,14 @@
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 b6db350..a5f03f0 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, bool is_static) = 0;
+ virtual void UnregisterStreamPriority(QuicStreamId id) = 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 d6eb67f..c30283c 100644
--- a/quiche/quic/tools/quic_simple_server_session_test.cc
+++ b/quiche/quic/tools/quic_simple_server_session_test.cc
@@ -491,8 +491,7 @@
if (!VersionUsesHttp3(transport_version())) {
session_->UnregisterStreamPriority(
- QuicUtils::GetHeadersStreamId(transport_version()),
- /*is_static=*/true);
+ QuicUtils::GetHeadersStreamId(transport_version()));
}
// Assume encryption already established.
QuicSimpleServerSessionPeer::SetCryptoStream(session_.get(), nullptr);
