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;
