gfe-relnote: deprecate gfe2_reloadable_flag_quic_eliminate_static_stream_map_3.
Note that static_stream_map still exists. More clean up will come in follow up CLs.
PiperOrigin-RevId: 257075373
Change-Id: I39fe8e561f5575ed491829cac3d528e7c1d04bf8
diff --git a/quic/core/http/quic_spdy_session.cc b/quic/core/http/quic_spdy_session.cc
index 0cd5f90..0c26de7 100644
--- a/quic/core/http/quic_spdy_session.cc
+++ b/quic/core/http/quic_spdy_session.cc
@@ -344,10 +344,9 @@
static_cast<QuicSpdyStream*>(kv.second.get())->ClearSession();
}
for (auto const& kv : dynamic_streams()) {
- if (eliminate_static_stream_map() && kv.second->is_static()) {
- continue;
+ if (!kv.second->is_static()) {
+ static_cast<QuicSpdyStream*>(kv.second.get())->ClearSession();
}
- static_cast<QuicSpdyStream*>(kv.second.get())->ClearSession();
}
}
@@ -375,19 +374,12 @@
headers_stream_ = QuicMakeUnique<QuicHeadersStream>((this));
DCHECK_EQ(QuicUtils::GetHeadersStreamId(connection()->transport_version()),
headers_stream_->id());
- if (!eliminate_static_stream_map()) {
- RegisterStaticStream(
- QuicUtils::GetHeadersStreamId(connection()->transport_version()),
- headers_stream_.get());
- } else {
- QUIC_RELOADABLE_FLAG_COUNT_N(quic_eliminate_static_stream_map_3, 7, 17);
- unowned_headers_stream_ = headers_stream_.get();
- RegisterStaticStreamNew(std::move(headers_stream_),
- /*stream_already_counted = */ false);
- }
- if (VersionHasStreamType(connection()->transport_version()) &&
- eliminate_static_stream_map()) {
+ unowned_headers_stream_ = headers_stream_.get();
+ RegisterStaticStreamNew(std::move(headers_stream_),
+ /*stream_already_counted = */ false);
+
+ if (VersionHasStreamType(connection()->transport_version())) {
auto send_control = QuicMakeUnique<QuicSendControlStream>(
GetNextOutgoingUnidirectionalStreamId(), this,
max_inbound_header_list_size_);
@@ -737,13 +729,9 @@
}
bool QuicSpdySession::HasActiveRequestStreams() const {
- if (!eliminate_static_stream_map()) {
- return !dynamic_streams().empty();
- }
// In the case where session is destructed by calling
// dynamic_streams().clear(), we will have incorrect accounting here.
// TODO(renjietang): Modify destructors and make this a DCHECK.
- QUIC_RELOADABLE_FLAG_COUNT_N(quic_eliminate_static_stream_map_3, 9, 17);
if (static_cast<size_t>(dynamic_streams().size()) >
num_incoming_static_streams() + num_outgoing_static_streams()) {
return dynamic_streams().size() - num_incoming_static_streams() -
diff --git a/quic/core/http/quic_spdy_session.h b/quic/core/http/quic_spdy_session.h
index a0ddac4..93f121c 100644
--- a/quic/core/http/quic_spdy_session.h
+++ b/quic/core/http/quic_spdy_session.h
@@ -147,14 +147,10 @@
QpackEncoder* qpack_encoder();
QpackDecoder* qpack_decoder();
- QuicHeadersStream* headers_stream() {
- return eliminate_static_stream_map() ? unowned_headers_stream_
- : headers_stream_.get();
- }
+ QuicHeadersStream* headers_stream() { return unowned_headers_stream_; }
const QuicHeadersStream* headers_stream() const {
- return eliminate_static_stream_map() ? unowned_headers_stream_
- : headers_stream_.get();
+ return unowned_headers_stream_;
}
bool server_push_enabled() const { return server_push_enabled_; }
diff --git a/quic/core/http/quic_spdy_session_test.cc b/quic/core/http/quic_spdy_session_test.cc
index a9540ea..4b450e9 100644
--- a/quic/core/http/quic_spdy_session_test.cc
+++ b/quic/core/http/quic_spdy_session_test.cc
@@ -927,16 +927,10 @@
EXPECT_CALL(*crypto_stream, OnCanWrite());
}
TestHeadersStream* headers_stream;
- if (!GetQuicReloadableFlag(quic_eliminate_static_stream_map_3) &&
- !QuicVersionUsesCryptoFrames(connection_->transport_version())) {
- QuicSpdySessionPeer::SetHeadersStream(&session_, nullptr);
- headers_stream = new TestHeadersStream(&session_);
- QuicSpdySessionPeer::SetHeadersStream(&session_, headers_stream);
- } else {
- QuicSpdySessionPeer::SetUnownedHeadersStream(&session_, nullptr);
- headers_stream = new TestHeadersStream(&session_);
- QuicSpdySessionPeer::SetUnownedHeadersStream(&session_, headers_stream);
- }
+
+ QuicSpdySessionPeer::SetUnownedHeadersStream(&session_, nullptr);
+ headers_stream = new TestHeadersStream(&session_);
+ QuicSpdySessionPeer::SetUnownedHeadersStream(&session_, headers_stream);
session_.MarkConnectionLevelWriteBlocked(
QuicUtils::GetHeadersStreamId(connection_->transport_version()));
EXPECT_CALL(*headers_stream, OnCanWrite());
@@ -1747,16 +1741,9 @@
TEST_P(QuicSpdySessionTestClient, WritePriority) {
TestHeadersStream* headers_stream;
- if (!GetQuicReloadableFlag(quic_eliminate_static_stream_map_3) &&
- !QuicVersionUsesCryptoFrames(connection_->transport_version())) {
- QuicSpdySessionPeer::SetHeadersStream(&session_, nullptr);
- headers_stream = new TestHeadersStream(&session_);
- QuicSpdySessionPeer::SetHeadersStream(&session_, headers_stream);
- } else {
- QuicSpdySessionPeer::SetUnownedHeadersStream(&session_, nullptr);
- headers_stream = new TestHeadersStream(&session_);
- QuicSpdySessionPeer::SetUnownedHeadersStream(&session_, headers_stream);
- }
+ QuicSpdySessionPeer::SetUnownedHeadersStream(&session_, nullptr);
+ headers_stream = new TestHeadersStream(&session_);
+ QuicSpdySessionPeer::SetUnownedHeadersStream(&session_, headers_stream);
// Make packet writer blocked so |headers_stream| will buffer its write data.
MockPacketWriter* writer = static_cast<MockPacketWriter*>(
@@ -2089,8 +2076,7 @@
}
TEST_P(QuicSpdySessionTestServer, ReceiveControlStream) {
- if (!VersionHasStreamType(transport_version()) ||
- !GetQuicReloadableFlag(quic_eliminate_static_stream_map_3)) {
+ if (!VersionHasStreamType(transport_version())) {
return;
}
// Use a arbitrary stream id.
@@ -2115,8 +2101,7 @@
}
TEST_P(QuicSpdySessionTestServer, ReceiveControlStreamOutOfOrderDelivery) {
- if (!VersionHasStreamType(transport_version()) ||
- !GetQuicReloadableFlag(quic_eliminate_static_stream_map_3)) {
+ if (!VersionHasStreamType(transport_version())) {
return;
}
// Use an arbitrary stream id.
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc
index 829b3d3..dd9839f 100644
--- a/quic/core/quic_session.cc
+++ b/quic/core/quic_session.cc
@@ -88,10 +88,7 @@
control_frame_manager_(this),
last_message_id_(0),
closed_streams_clean_up_alarm_(nullptr),
- supported_versions_(supported_versions),
- eliminate_static_stream_map_(
- GetQuicReloadableFlag(quic_eliminate_static_stream_map_3) ||
- QuicVersionUsesCryptoFrames(connection->transport_version())) {
+ supported_versions_(supported_versions) {
closed_streams_clean_up_alarm_ =
QuicWrapUnique<QuicAlarm>(connection_->alarm_factory()->CreateAlarm(
new ClosedStreamsCleanUpDelegate(this)));
@@ -109,18 +106,12 @@
DCHECK_EQ(QuicUtils::GetCryptoStreamId(connection_->transport_version()),
GetMutableCryptoStream()->id());
- if (!eliminate_static_stream_map_) {
- RegisterStaticStream(
- QuicUtils::GetCryptoStreamId(connection_->transport_version()),
- GetMutableCryptoStream());
- } else {
- QUIC_RELOADABLE_FLAG_COUNT_N(quic_eliminate_static_stream_map_3, 10, 17);
- QuicStreamId id =
- QuicUtils::GetCryptoStreamId(connection_->transport_version());
- largest_static_stream_id_ = std::max(id, largest_static_stream_id_);
- if (VersionHasIetfQuicFrames(connection_->transport_version())) {
- v99_streamid_manager_.RegisterStaticStream(id, false);
- }
+
+ QuicStreamId id =
+ QuicUtils::GetCryptoStreamId(connection_->transport_version());
+ largest_static_stream_id_ = std::max(id, largest_static_stream_id_);
+ if (VersionHasIetfQuicFrames(connection_->transport_version())) {
+ v99_streamid_manager_.RegisterStaticStream(id, false);
}
}
@@ -145,7 +136,6 @@
void QuicSession::RegisterStaticStreamNew(std::unique_ptr<QuicStream> stream,
bool stream_already_counted) {
- DCHECK(eliminate_static_stream_map_);
QuicStreamId stream_id = stream->id();
dynamic_stream_map_[stream_id] = std::move(stream);
if (VersionHasIetfQuicFrames(connection_->transport_version())) {
@@ -224,8 +214,7 @@
}
return;
}
- if (eliminate_static_stream_map_ && frame.fin && stream->is_static()) {
- QUIC_RELOADABLE_FLAG_COUNT_N(quic_eliminate_static_stream_map_3, 1, 17);
+ if (frame.fin && stream->is_static()) {
connection()->CloseConnection(
QUIC_INVALID_STREAM_ID, "Attempt to close a static stream",
ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
@@ -312,8 +301,7 @@
return true;
}
- if (eliminate_static_stream_map_ && stream->is_static()) {
- QUIC_RELOADABLE_FLAG_COUNT_N(quic_eliminate_static_stream_map_3, 2, 17);
+ if (stream->is_static()) {
QUIC_DVLOG(1) << ENDPOINT
<< "Received STOP_SENDING for a static stream, id: "
<< stream_id << " Closing connection";
@@ -387,8 +375,7 @@
HandleRstOnValidNonexistentStream(frame);
return; // Errors are handled by GetOrCreateStream.
}
- if (eliminate_static_stream_map_ && stream->is_static()) {
- QUIC_RELOADABLE_FLAG_COUNT_N(quic_eliminate_static_stream_map_3, 3, 17);
+ if (stream->is_static()) {
connection()->CloseConnection(
QUIC_INVALID_STREAM_ID, "Attempt to reset a static stream",
ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
@@ -433,35 +420,20 @@
error_ = frame.quic_error_code;
}
- if (!eliminate_static_stream_map_) {
- while (!dynamic_stream_map_.empty()) {
- DynamicStreamMap::iterator it = dynamic_stream_map_.begin();
- QuicStreamId id = it->first;
- it->second->OnConnectionClosed(frame.quic_error_code, source);
- // The stream should call CloseStream as part of OnConnectionClosed.
- if (dynamic_stream_map_.find(id) != dynamic_stream_map_.end()) {
- QUIC_BUG << ENDPOINT << "Stream " << id
- << " failed to close under OnConnectionClosed";
- CloseStream(id);
- }
+ // Copy all non static streams in a new map for the ease of deleting.
+ QuicSmallMap<QuicStreamId, QuicStream*, 10> non_static_streams;
+ for (const auto& it : dynamic_stream_map_) {
+ if (!it.second->is_static()) {
+ non_static_streams[it.first] = it.second.get();
}
- } else {
- QUIC_RELOADABLE_FLAG_COUNT_N(quic_eliminate_static_stream_map_3, 4, 17);
- // Copy all non static streams in a new map for the ease of deleting.
- QuicSmallMap<QuicStreamId, QuicStream*, 10> non_static_streams;
- for (const auto& it : dynamic_stream_map_) {
- if (!it.second->is_static()) {
- non_static_streams[it.first] = it.second.get();
- }
- }
- for (const auto& it : non_static_streams) {
- QuicStreamId id = it.first;
- it.second->OnConnectionClosed(frame.quic_error_code, source);
- if (dynamic_stream_map_.find(id) != dynamic_stream_map_.end()) {
- QUIC_BUG << ENDPOINT << "Stream " << id
- << " failed to close under OnConnectionClosed";
- CloseStream(id);
- }
+ }
+ for (const auto& it : non_static_streams) {
+ QuicStreamId id = it.first;
+ it.second->OnConnectionClosed(frame.quic_error_code, source);
+ if (dynamic_stream_map_.find(id) != dynamic_stream_map_.end()) {
+ QUIC_BUG << ENDPOINT << "Stream " << id
+ << " failed to close under OnConnectionClosed";
+ CloseStream(id);
}
}
@@ -787,8 +759,7 @@
DynamicStreamMap::iterator it = dynamic_stream_map_.find(id);
if (it != dynamic_stream_map_.end()) {
- if (eliminate_static_stream_map_ && it->second->is_static()) {
- QUIC_RELOADABLE_FLAG_COUNT_N(quic_eliminate_static_stream_map_3, 5, 17);
+ if (it->second->is_static()) {
QUIC_DVLOG(1) << ENDPOINT
<< "Try to send rst for a static stream, id: " << id
<< " Closing connection";
@@ -862,8 +833,7 @@
return;
}
QuicStream* stream = it->second.get();
- if (eliminate_static_stream_map_ && stream->is_static()) {
- QUIC_RELOADABLE_FLAG_COUNT_N(quic_eliminate_static_stream_map_3, 6, 17);
+ if (stream->is_static()) {
QUIC_DVLOG(1) << ENDPOINT
<< "Try to close a static stream, id: " << stream_id
<< " Closing connection";
@@ -1116,9 +1086,7 @@
for (auto const& kv : dynamic_stream_map_) {
kv.second->flow_controller()->UpdateReceiveWindowSize(stream_window);
}
- if (eliminate_static_stream_map_ &&
- !QuicVersionUsesCryptoFrames(connection_->transport_version())) {
- QUIC_RELOADABLE_FLAG_COUNT_N(quic_eliminate_static_stream_map_3, 11, 17);
+ if (!QuicVersionUsesCryptoFrames(connection_->transport_version())) {
GetMutableCryptoStream()->flow_controller()->UpdateReceiveWindowSize(
stream_window);
}
@@ -1167,9 +1135,7 @@
for (auto const& kv : dynamic_stream_map_) {
kv.second->UpdateSendWindowOffset(new_window);
}
- if (eliminate_static_stream_map_ &&
- !QuicVersionUsesCryptoFrames(connection_->transport_version())) {
- QUIC_RELOADABLE_FLAG_COUNT_N(quic_eliminate_static_stream_map_3, 12, 17);
+ if (!QuicVersionUsesCryptoFrames(connection_->transport_version())) {
GetMutableCryptoStream()->UpdateSendWindowOffset(new_window);
}
}
@@ -1292,10 +1258,8 @@
QuicStream* QuicSession::GetOrCreateStream(const QuicStreamId stream_id) {
DCHECK(!QuicContainsKey(pending_stream_map_, stream_id));
- if (eliminate_static_stream_map_ &&
- QuicUtils::IsCryptoStreamId(connection_->transport_version(),
+ if (QuicUtils::IsCryptoStreamId(connection_->transport_version(),
stream_id)) {
- QUIC_RELOADABLE_FLAG_COUNT_N(quic_eliminate_static_stream_map_3, 13, 17);
return GetMutableCryptoStream();
}
StaticStreamMap::iterator it = static_stream_map_.find(stream_id);
@@ -1436,15 +1400,11 @@
}
bool QuicSession::IsStaticStream(QuicStreamId id) const {
- if (eliminate_static_stream_map()) {
- auto it = dynamic_stream_map_.find(id);
- if (it == dynamic_stream_map_.end()) {
- return false;
- }
- return it->second->is_static();
+ auto it = dynamic_stream_map_.find(id);
+ if (it == dynamic_stream_map_.end()) {
+ return false;
}
-
- return QuicContainsKey(static_streams(), id);
+ return it->second->is_static();
}
size_t QuicSession::GetNumOpenIncomingStreams() const {
@@ -1532,10 +1492,8 @@
return true;
}
}
- if (eliminate_static_stream_map_ &&
- !QuicVersionUsesCryptoFrames(connection_->transport_version()) &&
+ if (!QuicVersionUsesCryptoFrames(connection_->transport_version()) &&
GetMutableCryptoStream()->flow_controller()->IsBlocked()) {
- QUIC_RELOADABLE_FLAG_COUNT_N(quic_eliminate_static_stream_map_3, 14, 17);
return true;
}
return false;
@@ -1629,9 +1587,7 @@
return zombie_stream->second.get();
}
- if (eliminate_static_stream_map_ &&
- QuicUtils::IsCryptoStreamId(connection_->transport_version(), id)) {
- QUIC_RELOADABLE_FLAG_COUNT_N(quic_eliminate_static_stream_map_3, 15, 17);
+ if (QuicUtils::IsCryptoStreamId(connection_->transport_version(), id)) {
return const_cast<QuicCryptoStream*>(GetCryptoStream());
}
diff --git a/quic/core/quic_session.h b/quic/core/quic_session.h
index 5a0e9dd..3325506 100644
--- a/quic/core/quic_session.h
+++ b/quic/core/quic_session.h
@@ -570,10 +570,6 @@
return false;
}
- bool eliminate_static_stream_map() const {
- return eliminate_static_stream_map_;
- }
-
private:
friend class test::QuicSessionPeer;
@@ -731,9 +727,6 @@
// Supported version list used by the crypto handshake only. Please note, this
// list may be a superset of the connection framer's supported versions.
ParsedQuicVersionVector supported_versions_;
-
- // Latched value of quic_eliminate_static_stream_map.
- const bool eliminate_static_stream_map_;
};
} // namespace quic
diff --git a/quic/core/quic_session_test.cc b/quic/core/quic_session_test.cc
index 6188b05..44f5869 100644
--- a/quic/core/quic_session_test.cc
+++ b/quic/core/quic_session_test.cc
@@ -1298,13 +1298,8 @@
QuicUtils::GetHeadersStreamId(connection_->transport_version());
std::unique_ptr<TestStream> fake_headers_stream = QuicMakeUnique<TestStream>(
headers_stream_id, &session_, /*is_static*/ true, BIDIRECTIONAL);
- if (GetQuicReloadableFlag(quic_eliminate_static_stream_map_3)) {
- QuicSessionPeer::RegisterStaticStreamNew(&session_,
- std::move(fake_headers_stream));
- } else {
- QuicSessionPeer::RegisterStaticStream(&session_, headers_stream_id,
- fake_headers_stream.get());
- }
+ QuicSessionPeer::RegisterStaticStreamNew(&session_,
+ std::move(fake_headers_stream));
// Send two bytes of payload.
QuicStreamFrame data1(headers_stream_id, true, 0, QuicStringPiece("HT"));
EXPECT_CALL(*connection_,
@@ -1322,13 +1317,8 @@
QuicUtils::GetHeadersStreamId(connection_->transport_version());
std::unique_ptr<TestStream> fake_headers_stream = QuicMakeUnique<TestStream>(
headers_stream_id, &session_, /*is_static*/ true, BIDIRECTIONAL);
- if (GetQuicReloadableFlag(quic_eliminate_static_stream_map_3)) {
- QuicSessionPeer::RegisterStaticStreamNew(&session_,
- std::move(fake_headers_stream));
- } else {
- QuicSessionPeer::RegisterStaticStream(&session_, headers_stream_id,
- fake_headers_stream.get());
- }
+ QuicSessionPeer::RegisterStaticStreamNew(&session_,
+ std::move(fake_headers_stream));
// Send two bytes of payload.
QuicRstStreamFrame rst1(kInvalidControlFrameId, headers_stream_id,
QUIC_ERROR_PROCESSING_STREAM, 0);
@@ -2512,13 +2502,8 @@
QuicStreamId stream_id = 0;
std::unique_ptr<TestStream> fake_static_stream = QuicMakeUnique<TestStream>(
stream_id, &session_, /*is_static*/ true, BIDIRECTIONAL);
- if (GetQuicReloadableFlag(quic_eliminate_static_stream_map_3)) {
- QuicSessionPeer::RegisterStaticStreamNew(&session_,
- std::move(fake_static_stream));
- } else {
- QuicSessionPeer::RegisterStaticStream(&session_, stream_id,
- fake_static_stream.get());
- }
+ QuicSessionPeer::RegisterStaticStreamNew(&session_,
+ std::move(fake_static_stream));
// Check that a stream id in the static stream map is ignored.
// Note that the notion of a static stream is Google-specific.
QuicStopSendingFrame frame(1, stream_id, 123);
diff --git a/quic/core/quic_versions.cc b/quic/core/quic_versions.cc
index d9e89ff..66b8ad6 100644
--- a/quic/core/quic_versions.cc
+++ b/quic/core/quic_versions.cc
@@ -452,7 +452,6 @@
// Enable necessary flags.
SetQuicFlag(FLAGS_quic_supports_tls_handshake, true);
SetQuicFlag(FLAGS_quic_headers_stream_id_in_v99, 60);
- SetQuicReloadableFlag(quic_eliminate_static_stream_map_3, true);
SetQuicRestartFlag(quic_do_not_override_connection_id, true);
SetQuicRestartFlag(quic_use_allocated_connection_ids, true);
}
