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); }