gfe-relnote: Move headers streams out of static stream map. Protected by gfe_reloadable_quic_eliminate_static_stream_map. Following up this CL, crypto stream will be moved out of static_stream_map too. PiperOrigin-RevId: 244429772 Change-Id: I5186bc7ab4e6ee9c9f546a3c9456be489a4ddc26
diff --git a/quic/core/http/quic_spdy_client_session_base.cc b/quic/core/http/quic_spdy_client_session_base.cc index d388d2e..4aa3348 100644 --- a/quic/core/http/quic_spdy_client_session_base.cc +++ b/quic/core/http/quic_spdy_client_session_base.cc
@@ -95,6 +95,13 @@ // It's quite possible to receive headers after a stream has been reset. return; } + if (GetQuicReloadableFlag(quic_eliminate_static_stream_map) && + stream->is_static()) { + connection()->CloseConnection( + QUIC_INVALID_HEADERS_STREAM_DATA, "stream is static", + ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET); + return; + } stream->OnPromiseHeaderList(promised_stream_id, frame_len, header_list); }
diff --git a/quic/core/http/quic_spdy_session.cc b/quic/core/http/quic_spdy_session.cc index bda2e17..d7a5473 100644 --- a/quic/core/http/quic_spdy_session.cc +++ b/quic/core/http/quic_spdy_session.cc
@@ -355,9 +355,15 @@ headers_stream_ = QuicMakeUnique<QuicHeadersStream>((this)); DCHECK_EQ(QuicUtils::GetHeadersStreamId(connection()->transport_version()), headers_stream_->id()); - RegisterStaticStream( - QuicUtils::GetHeadersStreamId(connection()->transport_version()), - headers_stream_.get()); + if (!GetQuicReloadableFlag(quic_eliminate_static_stream_map)) { + RegisterStaticStream( + QuicUtils::GetHeadersStreamId(connection()->transport_version()), + headers_stream_.get()); + } else { + QUIC_RELOADABLE_FLAG_COUNT_N(quic_eliminate_static_stream_map, 7, 9); + unowned_headers_stream_ = headers_stream_.get(); + RegisterStaticStreamNew(std::move(headers_stream_)); + } set_max_uncompressed_header_bytes(max_inbound_header_list_size_); @@ -438,6 +444,14 @@ // It's quite possible to receive headers after a stream has been reset. return; } + if (GetQuicReloadableFlag(quic_eliminate_static_stream_map) && + stream->is_static()) { + QUIC_RELOADABLE_FLAG_COUNT_N(quic_eliminate_static_stream_map, 8, 9); + connection()->CloseConnection( + QUIC_INVALID_HEADERS_STREAM_DATA, "stream is static", + ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET); + return; + } stream->OnStreamHeaderList(fin, frame_len, header_list); } @@ -478,7 +492,7 @@ } SpdyPriorityIR priority_frame(id, parent_stream_id, weight, exclusive); SpdySerializedFrame frame(spdy_framer_.SerializeFrame(priority_frame)); - headers_stream_->WriteOrBufferData( + headers_stream()->WriteOrBufferData( QuicStringPiece(frame.data(), frame.size()), false, nullptr); return frame.size(); } @@ -498,7 +512,7 @@ push_promise.set_fin(false); SpdySerializedFrame frame(spdy_framer_.SerializeFrame(push_promise)); - headers_stream_->WriteOrBufferData( + headers_stream()->WriteOrBufferData( QuicStringPiece(frame.data(), frame.size()), false, nullptr); return frame.size(); } @@ -508,7 +522,7 @@ settings_frame.AddSetting(SETTINGS_MAX_HEADER_LIST_SIZE, value); SpdySerializedFrame frame(spdy_framer_.SerializeFrame(settings_frame)); - headers_stream_->WriteOrBufferData( + headers_stream()->WriteOrBufferData( QuicStringPiece(frame.data(), frame.size()), false, nullptr); return frame.size(); } @@ -567,7 +581,7 @@ headers_frame.set_exclusive(exclusive); } SpdySerializedFrame frame(spdy_framer_.SerializeFrame(headers_frame)); - headers_stream_->WriteOrBufferData( + headers_stream()->WriteOrBufferData( QuicStringPiece(frame.data(), frame.size()), false, std::move(ack_listener)); return frame.size(); @@ -695,8 +709,20 @@ } bool QuicSpdySession::HasActiveRequestStreams() const { - // TODO(renjietang): Exclude static streams. - return !dynamic_streams().empty(); + if (!GetQuicReloadableFlag(quic_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, 9, 9); + if (static_cast<size_t>(dynamic_streams().size()) > + num_incoming_static_streams() + num_outgoing_static_streams()) { + return dynamic_streams().size() - num_incoming_static_streams() - + num_outgoing_static_streams() > + 0; + } + return false; } } // namespace quic
diff --git a/quic/core/http/quic_spdy_session.h b/quic/core/http/quic_spdy_session.h index 0ff1150..9248fa5 100644 --- a/quic/core/http/quic_spdy_session.h +++ b/quic/core/http/quic_spdy_session.h
@@ -139,7 +139,17 @@ QpackEncoder* qpack_encoder(); QpackDecoder* qpack_decoder(); - QuicHeadersStream* headers_stream() { return headers_stream_.get(); } + QuicHeadersStream* headers_stream() { + return GetQuicReloadableFlag(quic_eliminate_static_stream_map) + ? unowned_headers_stream_ + : headers_stream_.get(); + } + + const QuicHeadersStream* headers_stream() const { + return GetQuicReloadableFlag(quic_eliminate_static_stream_map) + ? unowned_headers_stream_ + : headers_stream_.get(); + } bool server_push_enabled() const { return server_push_enabled_; } @@ -262,6 +272,13 @@ // TODO(123528590): Remove this member. std::unique_ptr<QuicHeadersStream> headers_stream_; + // Unowned headers stream pointer that points to the stream + // in dynamic_stream_map. + // TODO(renjietang): Merge this with headers_stream_ and clean up other + // static_stream_map logic when flag eliminate_static_stream_map + // is deprecated. + QuicHeadersStream* unowned_headers_stream_; + // The maximum size of a header block that will be accepted from the peer, // defined per spec as key + value + overhead per field (uncompressed). size_t max_inbound_header_list_size_;
diff --git a/quic/core/http/quic_spdy_session_test.cc b/quic/core/http/quic_spdy_session_test.cc index 04605e1..5726573 100644 --- a/quic/core/http/quic_spdy_session_test.cc +++ b/quic/core/http/quic_spdy_session_test.cc
@@ -896,9 +896,16 @@ // connection flow control blocked. TestCryptoStream* crypto_stream = session_.GetMutableCryptoStream(); EXPECT_CALL(*crypto_stream, OnCanWrite()); - QuicSpdySessionPeer::SetHeadersStream(&session_, nullptr); - TestHeadersStream* headers_stream = new TestHeadersStream(&session_); - QuicSpdySessionPeer::SetHeadersStream(&session_, headers_stream); + TestHeadersStream* headers_stream; + if (!GetQuicReloadableFlag(quic_eliminate_static_stream_map)) { + 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); + } session_.MarkConnectionLevelWriteBlocked( QuicUtils::GetHeadersStreamId(connection_->transport_version())); EXPECT_CALL(*headers_stream, OnCanWrite()); @@ -1597,9 +1604,16 @@ } TEST_P(QuicSpdySessionTestClient, WritePriority) { - QuicSpdySessionPeer::SetHeadersStream(&session_, nullptr); - TestHeadersStream* headers_stream = new TestHeadersStream(&session_); - QuicSpdySessionPeer::SetHeadersStream(&session_, headers_stream); + TestHeadersStream* headers_stream; + if (!GetQuicReloadableFlag(quic_eliminate_static_stream_map)) { + 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); + } // Make packet writer blocked so |headers_stream| will buffer its write data. MockPacketWriter* writer = static_cast<MockPacketWriter*>(