From 1ae6fa92007cd1423ce1e716fcd22130f3e1dfbe Mon Sep 17 00:00:00 2001 From: bnc Date: Tue, 17 Oct 2023 07:48:23 -0700 Subject: [PATCH] Remove gQUIC push related comments. PiperOrigin-RevId: 574148204 --- quiche/quic/core/http/end_to_end_test.cc | 4 ++-- quiche/quic/core/http/quic_headers_stream.h | 6 +++--- quiche/quic/core/http/quic_spdy_client_session.h | 3 +-- quiche/quic/core/http/quic_spdy_client_session_base.h | 5 ++--- quiche/quic/core/http/quic_spdy_client_session_test.cc | 1 - quiche/quic/core/http/quic_spdy_client_stream.cc | 2 -- quiche/quic/core/http/quic_spdy_session.cc | 1 - quiche/quic/core/http/quic_spdy_stream_test.cc | 2 +- quiche/quic/core/qpack/qpack_blocking_manager.h | 8 ++++---- quiche/quic/core/quic_session.h | 4 +--- quiche/quic/tools/quic_simple_server_session_test.cc | 4 ++-- quiche/quic/tools/quic_spdy_client_base.cc | 2 -- 12 files changed, 16 insertions(+), 26 deletions(-) diff --git a/quiche/quic/core/http/end_to_end_test.cc b/quiche/quic/core/http/end_to_end_test.cc index 48f360456..961214b8e 100644 --- a/quiche/quic/core/http/end_to_end_test.cc +++ b/quiche/quic/core/http/end_to_end_test.cc @@ -4776,8 +4776,8 @@ TEST_P(EndToEndTest, DISABLED_TestHugeResponseWithPacketLoss) { } TEST_P(EndToEndTest, ReleaseHeadersStreamBufferWhenIdle) { - // Tests that when client side has no active request and no waiting - // PUSH_PROMISE, its headers stream's sequencer buffer should be released. + // Tests that when client side has no active request, + // its headers stream's sequencer buffer should be released. ASSERT_TRUE(Initialize()); client_->SendSynchronousRequest("/foo"); if (version_.UsesHttp3()) { diff --git a/quiche/quic/core/http/quic_headers_stream.h b/quiche/quic/core/http/quic_headers_stream.h index ce8010e00..789b0aecd 100644 --- a/quiche/quic/core/http/quic_headers_stream.h +++ b/quiche/quic/core/http/quic_headers_stream.h @@ -22,9 +22,9 @@ namespace test { class QuicHeadersStreamPeer; } // namespace test -// Headers in QUIC are sent as HTTP/2 HEADERS or PUSH_PROMISE frames over a -// reserved stream with the id 3. Each endpoint (client and server) will -// allocate an instance of QuicHeadersStream to send and receive headers. +// Headers in QUIC are sent as HTTP/2 HEADERS frames over a reserved stream with +// the id 3. Each endpoint (client and server) will allocate an instance of +// QuicHeadersStream to send and receive headers. class QUICHE_EXPORT QuicHeadersStream : public QuicStream { public: explicit QuicHeadersStream(QuicSpdySession* session); diff --git a/quiche/quic/core/http/quic_spdy_client_session.h b/quiche/quic/core/http/quic_spdy_client_session.h index 7ed989c9d..1ff353326 100644 --- a/quiche/quic/core/http/quic_spdy_client_session.h +++ b/quiche/quic/core/http/quic_spdy_client_session.h @@ -22,8 +22,7 @@ class QuicServerId; class QUICHE_EXPORT QuicSpdyClientSession : public QuicSpdyClientSessionBase { public: - // Takes ownership of |connection|. Caller retains ownership of - // |promised_by_url|. + // Takes ownership of |connection|. QuicSpdyClientSession(const QuicConfig& config, const ParsedQuicVersionVector& supported_versions, QuicConnection* connection, diff --git a/quiche/quic/core/http/quic_spdy_client_session_base.h b/quiche/quic/core/http/quic_spdy_client_session_base.h index f4a5465a8..afb4dd184 100644 --- a/quiche/quic/core/http/quic_spdy_client_session_base.h +++ b/quiche/quic/core/http/quic_spdy_client_session_base.h @@ -20,8 +20,7 @@ class QUICHE_EXPORT QuicSpdyClientSessionBase : public QuicSpdySession, public QuicCryptoClientStream::ProofHandler { public: - // Takes ownership of |connection|. Caller retains ownership of - // |promised_by_url|. + // Takes ownership of |connection|. QuicSpdyClientSessionBase(QuicConnection* connection, QuicSession::Visitor* visitor, const QuicConfig& config, @@ -37,7 +36,7 @@ class QUICHE_EXPORT QuicSpdyClientSessionBase // Release headers stream's sequencer buffer if it's empty. void OnStreamClosed(QuicStreamId stream_id) override; - // Returns true if there are no active requests and no promised streams. + // Returns true if there are no active requests. bool ShouldReleaseHeadersStreamSequencerBuffer() override; // Override to wait for all received responses to be consumed by application. diff --git a/quiche/quic/core/http/quic_spdy_client_session_test.cc b/quiche/quic/core/http/quic_spdy_client_session_test.cc index 59e1ed36b..5aa479bdd 100644 --- a/quiche/quic/core/http/quic_spdy_client_session_test.cc +++ b/quiche/quic/core/http/quic_spdy_client_session_test.cc @@ -97,7 +97,6 @@ class QuicSpdyClientSessionTest : public QuicTestWithParam { } ~QuicSpdyClientSessionTest() override { - // Session must be destroyed before promised_by_url_ session_.reset(nullptr); } diff --git a/quiche/quic/core/http/quic_spdy_client_stream.cc b/quiche/quic/core/http/quic_spdy_client_stream.cc index 54ed908c0..3dd798684 100644 --- a/quiche/quic/core/http/quic_spdy_client_stream.cc +++ b/quiche/quic/core/http/quic_spdy_client_stream.cc @@ -128,8 +128,6 @@ void QuicSpdyClientStream::OnTrailingHeadersComplete( } void QuicSpdyClientStream::OnBodyAvailable() { - // For push streams, visitor will not be set until the rendezvous - // between server promise and client request is complete. if (visitor() == nullptr) return; while (HasBytesToRead()) { diff --git a/quiche/quic/core/http/quic_spdy_session.cc b/quiche/quic/core/http/quic_spdy_session.cc index d7d4b55f9..3c2db999b 100644 --- a/quiche/quic/core/http/quic_spdy_session.cc +++ b/quiche/quic/core/http/quic_spdy_session.cc @@ -51,7 +51,6 @@ using spdy::SpdyHeadersIR; using spdy::SpdyPingId; using spdy::SpdyPriority; using spdy::SpdyPriorityIR; -using spdy::SpdyPushPromiseIR; using spdy::SpdySerializedFrame; using spdy::SpdySettingsId; using spdy::SpdyStreamId; diff --git a/quiche/quic/core/http/quic_spdy_stream_test.cc b/quiche/quic/core/http/quic_spdy_stream_test.cc index f60bfba4e..3e0b8a191 100644 --- a/quiche/quic/core/http/quic_spdy_stream_test.cc +++ b/quiche/quic/core/http/quic_spdy_stream_test.cc @@ -748,7 +748,7 @@ TEST_P(QuicSpdyStreamTest, Http3FrameError) { Initialize(kShouldProcessData); - // PUSH_PROMISE frame with empty payload is considered invalid. + // PUSH_PROMISE frame is considered invalid. std::string invalid_http3_frame = absl::HexStringToBytes("0500"); QuicStreamFrame stream_frame(stream_->id(), /* fin = */ false, /* offset = */ 0, invalid_http3_frame); diff --git a/quiche/quic/core/qpack/qpack_blocking_manager.h b/quiche/quic/core/qpack/qpack_blocking_manager.h index 0189ad28d..ade3ee601 100644 --- a/quiche/quic/core/qpack/qpack_blocking_manager.h +++ b/quiche/quic/core/qpack/qpack_blocking_manager.h @@ -72,10 +72,10 @@ class QUICHE_EXPORT QpackBlockingManager { friend test::QpackBlockingManagerPeer; // A stream typically has only one header block, except for the rare cases of - // 1xx responses, trailers, or push promises. Even if there are multiple - // header blocks sent on a single stream, they might not be blocked at the - // same time. Use std::list instead of quiche::QuicheCircularDeque because it - // has lower memory footprint when holding few elements. + // 1xx responses and trailers. Even if there are multiple header blocks sent + // on a single stream, they might not be blocked at the same time. Use + // std::list instead of quiche::QuicheCircularDeque because it has lower + // memory footprint when holding few elements. using HeaderBlocksForStream = std::list; using HeaderBlocks = absl::flat_hash_map; diff --git a/quiche/quic/core/quic_session.h b/quiche/quic/core/quic_session.h index 2c2ed0115..71dae7b8d 100644 --- a/quiche/quic/core/quic_session.h +++ b/quiche/quic/core/quic_session.h @@ -760,11 +760,9 @@ class QUICHE_EXPORT QuicSession // Returns true if the stream is a static stream. bool IsStaticStream(QuicStreamId id) const; - // Close connection when receive a frame for a locally-created nonexistent + // Close connection when receiving a frame for a locally-created nonexistent // stream. // Prerequisite: IsClosedStream(stream_id) == false - // Server session might need to override this method to allow server push - // stream to be promised before creating an active stream. virtual void HandleFrameOnNonexistentOutgoingStream(QuicStreamId stream_id); virtual bool MaybeIncreaseLargestPeerStreamId(const QuicStreamId stream_id); diff --git a/quiche/quic/tools/quic_simple_server_session_test.cc b/quiche/quic/tools/quic_simple_server_session_test.cc index b6662d621..a946a2bbf 100644 --- a/quiche/quic/tools/quic_simple_server_session_test.cc +++ b/quiche/quic/tools/quic_simple_server_session_test.cc @@ -443,8 +443,8 @@ TEST_P(QuicSimpleServerSessionTest, CreateOutgoingDynamicStreamUnencrypted) { QuicSessionPeer::GetNumOpenDynamicStreams(session_.get())); } -// Tests that calling GetOrCreateStream() on an outgoing stream not promised yet -// should result close connection. +// Tests that calling GetOrCreateStream() on an outgoing stream should result in +// the connection being closed. TEST_P(QuicSimpleServerSessionTest, GetEvenIncomingError) { const size_t initial_num_open_stream = QuicSessionPeer::GetNumOpenDynamicStreams(session_.get()); diff --git a/quiche/quic/tools/quic_spdy_client_base.cc b/quiche/quic/tools/quic_spdy_client_base.cc index 74b03f1f2..98395340f 100644 --- a/quiche/quic/tools/quic_spdy_client_base.cc +++ b/quiche/quic/tools/quic_spdy_client_base.cc @@ -35,8 +35,6 @@ QuicSpdyClientBase::QuicSpdyClientBase( latest_response_code_(-1) {} QuicSpdyClientBase::~QuicSpdyClientBase() { - // We own the push promise index. We need to explicitly kill - // the session before the push promise index goes out of scope. ResetSession(); }