Implement ShouldProcessPendingStreamImmediately() with IsEncryptionEstablished() in QuicSession and stop overriding it everywhere. This should be a no-op for HTTP/3 and Qbone as their original implementations has the same return value as IsEncryptionEstablished() at their call site. PiperOrigin-RevId: 571957559
diff --git a/quiche/quic/core/http/quic_receive_control_stream_test.cc b/quiche/quic/core/http/quic_receive_control_stream_test.cc index af7e5d4..d448fe1 100644 --- a/quiche/quic/core/http/quic_receive_control_stream_test.cc +++ b/quiche/quic/core/http/quic_receive_control_stream_test.cc
@@ -88,6 +88,10 @@ session_(connection_) { EXPECT_CALL(session_, OnCongestionWindowChange(_)).Times(AnyNumber()); session_.Initialize(); + EXPECT_CALL( + static_cast<const MockQuicCryptoStream&>(*session_.GetCryptoStream()), + encryption_established()) + .WillRepeatedly(testing::Return(true)); QuicStreamId id = perspective() == Perspective::IS_SERVER ? GetNthClientInitiatedUnidirectionalStreamId( session_.transport_version(), 3)
diff --git a/quiche/quic/core/http/quic_spdy_session_test.cc b/quiche/quic/core/http/quic_spdy_session_test.cc index 2d368c2..07bd3aa 100644 --- a/quiche/quic/core/http/quic_spdy_session_test.cc +++ b/quiche/quic/core/http/quic_spdy_session_test.cc
@@ -1357,6 +1357,7 @@ QuicStreamId id; // Initialize HTTP/3 control stream. if (VersionUsesHttp3(transport_version())) { + CompleteHandshake(); id = GetNthClientInitiatedUnidirectionalStreamId(transport_version(), 3); char type[] = {kControlStream}; @@ -1381,6 +1382,7 @@ std::string error_message; // Initialize HTTP/3 control stream. if (VersionUsesHttp3(transport_version())) { + CompleteHandshake(); id = GetNthClientInitiatedUnidirectionalStreamId(transport_version(), 3); char type[] = {kControlStream}; @@ -1784,6 +1786,7 @@ return; } + CompleteHandshake(); EXPECT_EQ(0u, QuicSessionPeer::GetNumOpenDynamicStreams(&session_)); QuicStreamId stream_id1 = GetNthServerInitiatedUnidirectionalStreamId(transport_version(), 0); @@ -1796,7 +1799,7 @@ if (!VersionUsesHttp3(transport_version())) { return; } - + CompleteHandshake(); QuicStreamId stream_id = QuicUtils::GetFirstUnidirectionalStreamId( transport_version(), Perspective::IS_SERVER); @@ -1928,6 +1931,7 @@ return; } + CompleteHandshake(); EXPECT_EQ(0u, QuicSessionPeer::GetNumOpenDynamicStreams(&session_)); // Push unidirectional stream is type 0x01. @@ -1946,6 +1950,7 @@ return; } + CompleteHandshake(); EXPECT_EQ(0u, QuicSessionPeer::GetNumOpenDynamicStreams(&session_)); // Push unidirectional stream is type 0x01. @@ -2146,6 +2151,8 @@ StrictMock<MockHttp3DebugVisitor> debug_visitor; session_.set_debug_visitor(&debug_visitor); + EXPECT_CALL(debug_visitor, OnSettingsFrameSent(_)); + CompleteHandshake(); // Create control stream. QuicStreamId receive_control_stream_id = @@ -2214,6 +2221,7 @@ return; } + CompleteHandshake(); StrictMock<MockHttp3DebugVisitor> debug_visitor; session_.set_debug_visitor(&debug_visitor); @@ -2261,6 +2269,7 @@ return; } + CompleteHandshake(); StrictMock<MockHttp3DebugVisitor> debug_visitor; session_.set_debug_visitor(&debug_visitor); @@ -2479,6 +2488,7 @@ if (!VersionUsesHttp3(transport_version())) { return; } + CompleteHandshake(); // Use an arbitrary stream id. QuicStreamId stream_id = GetNthClientInitiatedUnidirectionalStreamId(transport_version(), 3); @@ -2649,6 +2659,7 @@ return; } + CompleteHandshake(); const QuicStreamId stream_id = GetNthServerInitiatedUnidirectionalStreamId(transport_version(), 0); ASSERT_TRUE(session_.UsesPendingStreamForFrame(STREAM_FRAME, stream_id)); @@ -2677,6 +2688,7 @@ return; } + CompleteHandshake(); const QuicStreamId stream_id = GetNthServerInitiatedUnidirectionalStreamId(transport_version(), 0); ASSERT_TRUE(session_.UsesPendingStreamForFrame(STREAM_FRAME, stream_id)); @@ -2694,6 +2706,7 @@ return; } + CompleteHandshake(); StrictMock<MockHttp3DebugVisitor> debug_visitor; session_.set_debug_visitor(&debug_visitor); @@ -2767,6 +2780,7 @@ return; } + CompleteHandshake(); std::string data = absl::HexStringToBytes( "02" // Encoder stream. "00"); // Duplicate entry 0, but no entries exist. @@ -2788,6 +2802,7 @@ return; } + CompleteHandshake(); std::string data = absl::HexStringToBytes( "03" // Decoder stream. "00"); // Insert Count Increment with forbidden increment value of zero. @@ -2843,6 +2858,7 @@ return; } + CompleteHandshake(); StrictMock<MockHttp3DebugVisitor> debug_visitor; session_.set_debug_visitor(&debug_visitor); @@ -2956,6 +2972,7 @@ if (!VersionUsesHttp3(transport_version())) { return; } + CompleteHandshake(); struct { char type; @@ -3045,6 +3062,7 @@ return; } + CompleteHandshake(); StrictMock<MockHttp3DebugVisitor> debug_visitor; session_.set_debug_visitor(&debug_visitor); @@ -3170,6 +3188,7 @@ return; } + CompleteHandshake(); StrictMock<MockHttp3DebugVisitor> debug_visitor; session_.set_debug_visitor(&debug_visitor); @@ -3277,6 +3296,7 @@ return; } + CompleteHandshake(); QpackEncoder* qpack_encoder = session_.qpack_encoder(); EXPECT_EQ(0u, qpack_encoder->MaximumDynamicTableCapacity()); EXPECT_EQ(0u, qpack_encoder->maximum_blocked_streams()); @@ -3345,6 +3365,7 @@ return; } + CompleteHandshake(); QpackEncoder* qpack_encoder = session_.qpack_encoder(); EXPECT_EQ(0u, qpack_encoder->MaximumDynamicTableCapacity()); @@ -3408,6 +3429,7 @@ if (!version().UsesHttp3()) { return; } + CompleteHandshake(); session_.set_local_http_datagram_support(local_support); // HTTP/3 datagrams aren't supported before SETTINGS are received. EXPECT_FALSE(session_.SupportsH3Datagram());
diff --git a/quiche/quic/core/qpack/qpack_receive_stream_test.cc b/quiche/quic/core/qpack/qpack_receive_stream_test.cc index 4f94802..e7560fa 100644 --- a/quiche/quic/core/qpack/qpack_receive_stream_test.cc +++ b/quiche/quic/core/qpack/qpack_receive_stream_test.cc
@@ -56,6 +56,10 @@ session_(connection_) { EXPECT_CALL(session_, OnCongestionWindowChange(_)).Times(AnyNumber()); session_.Initialize(); + EXPECT_CALL( + static_cast<const MockQuicCryptoStream&>(*session_.GetCryptoStream()), + encryption_established()) + .WillRepeatedly(testing::Return(true)); QuicStreamId id = perspective() == Perspective::IS_SERVER ? GetNthClientInitiatedUnidirectionalStreamId( session_.transport_version(), 3)
diff --git a/quiche/quic/core/quic_session.cc b/quiche/quic/core/quic_session.cc index f86e722..b57d3b1 100644 --- a/quiche/quic/core/quic_session.cc +++ b/quiche/quic/core/quic_session.cc
@@ -2733,5 +2733,9 @@ } } +bool QuicSession::ShouldProcessPendingStreamImmediately() const { + return IsEncryptionEstablished(); +} + #undef ENDPOINT // undef for jumbo builds } // namespace quic
diff --git a/quiche/quic/core/quic_session.h b/quiche/quic/core/quic_session.h index f347153..76600f5 100644 --- a/quiche/quic/core/quic_session.h +++ b/quiche/quic/core/quic_session.h
@@ -736,7 +736,7 @@ // Returns true if a pending stream should be converted to a real stream after // a corresponding STREAM_FRAME is received. - virtual bool ShouldProcessPendingStreamImmediately() const { return true; } + bool ShouldProcessPendingStreamImmediately() const; spdy::SpdyPriority GetSpdyPriorityofStream(QuicStreamId stream_id) const { return write_blocked_streams_->GetPriorityOfStream(stream_id)
diff --git a/quiche/quic/core/quic_session_test.cc b/quiche/quic/core/quic_session_test.cc index 3b90633..929c844 100644 --- a/quiche/quic/core/quic_session_test.cc +++ b/quiche/quic/core/quic_session_test.cc
@@ -416,19 +416,10 @@ } } - bool ShouldProcessPendingStreamImmediately() const override { - return process_pending_stream_immediately_; - } - void set_uses_pending_streams(bool uses_pending_streams) { uses_pending_streams_ = uses_pending_streams; } - void set_process_pending_stream_immediately( - bool process_pending_stream_immediately) { - process_pending_stream_immediately_ = process_pending_stream_immediately; - } - int num_incoming_streams_created() const { return num_incoming_streams_created_; } @@ -445,7 +436,6 @@ bool writev_consumes_all_data_; bool uses_pending_streams_; - bool process_pending_stream_immediately_ = true; QuicFrame save_frame_; int num_incoming_streams_created_; }; @@ -1832,8 +1822,8 @@ if (!VersionUsesHttp3(transport_version())) { return; } + CompleteHandshake(); session_.set_uses_pending_streams(true); - session_.set_process_pending_stream_immediately(true); QuicStreamId stream_id = QuicUtils::GetFirstUnidirectionalStreamId( transport_version(), Perspective::IS_CLIENT); @@ -1853,7 +1843,6 @@ return; } session_.set_uses_pending_streams(true); - session_.set_process_pending_stream_immediately(false); QuicStreamId stream_id = QuicUtils::GetFirstUnidirectionalStreamId( transport_version(), Perspective::IS_CLIENT); @@ -1891,7 +1880,6 @@ return; } session_.set_uses_pending_streams(true); - session_.set_process_pending_stream_immediately(false); QuicStreamId stream_id = QuicUtils::GetFirstUnidirectionalStreamId( transport_version(), Perspective::IS_CLIENT); @@ -1936,23 +1924,32 @@ EXPECT_EQ(0u, QuicSessionPeer::GetNumOpenDynamicStreams(&session_)); } -TEST_P(QuicSessionTestServer, OnFinPendingStreams) { +TEST_P(QuicSessionTestServer, OnFinPendingStreamsReadUnidirectional) { if (!VersionUsesHttp3(transport_version())) { return; } + CompleteHandshake(); session_.set_uses_pending_streams(true); - session_.set_process_pending_stream_immediately(true); QuicStreamId stream_id = QuicUtils::GetFirstUnidirectionalStreamId( transport_version(), Perspective::IS_CLIENT); QuicStreamFrame data(stream_id, true, 0, ""); session_.OnStreamFrame(data); + // The pending stream will be immediately converted to a normal unidirectional + // stream, but because its FIN has been received, it should be closed + // immediately. EXPECT_FALSE(QuicSessionPeer::GetPendingStream(&session_, stream_id)); EXPECT_EQ(0, session_.num_incoming_streams_created()); EXPECT_EQ(0u, QuicSessionPeer::GetNumOpenDynamicStreams(&session_)); + EXPECT_EQ(nullptr, QuicSessionPeer::GetStream(&session_, stream_id)); +} - session_.set_process_pending_stream_immediately(false); +TEST_P(QuicSessionTestServer, OnFinPendingStreamsBidirectional) { + if (!VersionUsesHttp3(transport_version())) { + return; + } + session_.set_uses_pending_streams(true); // Bidirectional pending stream remains after Fin is received. // Bidirectional stream is buffered. QuicStreamId bidirectional_stream_id = @@ -2002,7 +1999,6 @@ } session_.set_uses_pending_streams(true); - session_.set_process_pending_stream_immediately(false); QuicStreamId stream_id = QuicUtils::GetFirstBidirectionalStreamId( transport_version(), Perspective::IS_CLIENT); QuicStreamFrame data(stream_id, true, 10, absl::string_view("HT")); @@ -2050,7 +2046,6 @@ } session_.set_uses_pending_streams(true); - session_.set_process_pending_stream_immediately(false); QuicStreamId stream_id = QuicUtils::GetFirstBidirectionalStreamId( transport_version(), Perspective::IS_CLIENT); QuicStreamFrame data(stream_id, true, 0, absl::string_view("HT"));
diff --git a/quiche/quic/test_tools/quic_test_utils.cc b/quiche/quic/test_tools/quic_test_utils.cc index c79537d..52bd115 100644 --- a/quiche/quic/test_tools/quic_test_utils.cc +++ b/quiche/quic/test_tools/quic_test_utils.cc
@@ -593,7 +593,8 @@ connection->supported_versions(), /*num_expected_unidirectional_static_streams = */ 0) { if (create_mock_crypto_stream) { - crypto_stream_ = std::make_unique<MockQuicCryptoStream>(this); + crypto_stream_ = + std::make_unique<testing::NiceMock<MockQuicCryptoStream>>(this); } ON_CALL(*this, WritevData(_, _, _, _, _, _)) .WillByDefault(testing::Return(QuicConsumedData(0, false))); @@ -638,8 +639,6 @@ return ssl_early_data_unknown; } -bool MockQuicCryptoStream::encryption_established() const { return false; } - bool MockQuicCryptoStream::one_rtt_keys_available() const { return false; } const QuicCryptoNegotiatedParameters&
diff --git a/quiche/quic/test_tools/quic_test_utils.h b/quiche/quic/test_tools/quic_test_utils.h index 5fd0df1..20567e3 100644 --- a/quiche/quic/test_tools/quic_test_utils.h +++ b/quiche/quic/test_tools/quic_test_utils.h
@@ -834,8 +834,9 @@ ~MockQuicCryptoStream() override; + MOCK_METHOD(bool, encryption_established, (), (const)); + ssl_early_data_reason_t EarlyDataReason() const override; - bool encryption_established() const override; bool one_rtt_keys_available() const override; const QuicCryptoNegotiatedParameters& crypto_negotiated_params() const override;