Prevent QuicSession from directly accessing streams' flow controller.
This change provides the following advantages:
1. QuicSession is no longer able to go cross QuicStream to modify its flow controller, and is limited on what it can do with stream's flow controller.
2. QuicStream::IsFlowControlBlocked() can be potentially transitioned to a stream state.
This CL also removes some tests in quic_spdy_session_test that are redundant with quic_session_test.
No behavior change. not protected.
PiperOrigin-RevId: 325899044
Change-Id: I1b676da2736507aec0a4438afdbc596ca274ed13
diff --git a/quic/core/http/quic_spdy_session_test.cc b/quic/core/http/quic_spdy_session_test.cc
index ede4d2c..18d3d75 100644
--- a/quic/core/http/quic_spdy_session_test.cc
+++ b/quic/core/http/quic_spdy_session_test.cc
@@ -1305,12 +1305,12 @@
// Create a stream, and send enough data to make it flow control blocked.
TestStream* stream2 = session_.CreateOutgoingBidirectionalStream();
std::string body(kMinimumFlowControlSendWindow, '.');
- EXPECT_FALSE(stream2->flow_controller()->IsBlocked());
+ EXPECT_FALSE(stream2->IsFlowControlBlocked());
EXPECT_FALSE(session_.IsConnectionFlowControlBlocked());
EXPECT_FALSE(session_.IsStreamFlowControlBlocked());
EXPECT_CALL(*connection_, SendControlFrame(_)).Times(AtLeast(1));
stream2->WriteOrBufferBody(body, false);
- EXPECT_TRUE(stream2->flow_controller()->IsBlocked());
+ EXPECT_TRUE(stream2->IsFlowControlBlocked());
EXPECT_TRUE(session_.IsConnectionFlowControlBlocked());
EXPECT_TRUE(session_.IsStreamFlowControlBlocked());
@@ -1319,7 +1319,7 @@
CompleteHandshake();
EXPECT_TRUE(QuicSessionPeer::IsStreamWriteBlocked(&session_, stream2->id()));
// Stream is now unblocked.
- EXPECT_FALSE(stream2->flow_controller()->IsBlocked());
+ EXPECT_FALSE(stream2->IsFlowControlBlocked());
EXPECT_FALSE(session_.IsConnectionFlowControlBlocked());
EXPECT_FALSE(session_.IsStreamFlowControlBlocked());
}
@@ -1335,18 +1335,18 @@
// contains a larger send window offset, the stream becomes unblocked.
session_.set_writev_consumes_all_data(true);
TestCryptoStream* crypto_stream = session_.GetMutableCryptoStream();
- EXPECT_FALSE(crypto_stream->flow_controller()->IsBlocked());
+ EXPECT_FALSE(crypto_stream->IsFlowControlBlocked());
EXPECT_FALSE(session_.IsConnectionFlowControlBlocked());
EXPECT_FALSE(session_.IsStreamFlowControlBlocked());
QuicHeadersStream* headers_stream =
QuicSpdySessionPeer::GetHeadersStream(&session_);
- EXPECT_FALSE(headers_stream->flow_controller()->IsBlocked());
+ EXPECT_FALSE(headers_stream->IsFlowControlBlocked());
EXPECT_FALSE(session_.IsConnectionFlowControlBlocked());
EXPECT_FALSE(session_.IsStreamFlowControlBlocked());
EXPECT_CALL(*connection_, SendControlFrame(_))
.WillOnce(Invoke(&ClearControlFrame));
- for (QuicStreamId i = 0;
- !crypto_stream->flow_controller()->IsBlocked() && i < 1000u; i++) {
+ for (QuicStreamId i = 0; !crypto_stream->IsFlowControlBlocked() && i < 1000u;
+ i++) {
EXPECT_FALSE(session_.IsConnectionFlowControlBlocked());
EXPECT_FALSE(session_.IsStreamFlowControlBlocked());
QuicStreamOffset offset = crypto_stream->stream_bytes_written();
@@ -1358,8 +1358,8 @@
QuicDataWriter writer(1000, buf, quiche::NETWORK_BYTE_ORDER);
crypto_stream->WriteStreamData(offset, crypto_message.size(), &writer);
}
- EXPECT_TRUE(crypto_stream->flow_controller()->IsBlocked());
- EXPECT_FALSE(headers_stream->flow_controller()->IsBlocked());
+ EXPECT_TRUE(crypto_stream->IsFlowControlBlocked());
+ EXPECT_FALSE(headers_stream->IsFlowControlBlocked());
EXPECT_FALSE(session_.IsConnectionFlowControlBlocked());
EXPECT_TRUE(session_.IsStreamFlowControlBlocked());
EXPECT_FALSE(session_.HasDataToWrite());
@@ -1371,7 +1371,7 @@
EXPECT_TRUE(QuicSessionPeer::IsStreamWriteBlocked(
&session_, QuicUtils::GetCryptoStreamId(transport_version())));
// Stream is now unblocked and will no longer have buffered data.
- EXPECT_FALSE(crypto_stream->flow_controller()->IsBlocked());
+ EXPECT_FALSE(crypto_stream->IsFlowControlBlocked());
EXPECT_FALSE(session_.IsConnectionFlowControlBlocked());
EXPECT_FALSE(session_.IsStreamFlowControlBlocked());
}
@@ -1400,12 +1400,12 @@
// contains a larger send window offset, the stream becomes unblocked.
session_.set_writev_consumes_all_data(true);
TestCryptoStream* crypto_stream = session_.GetMutableCryptoStream();
- EXPECT_FALSE(crypto_stream->flow_controller()->IsBlocked());
+ EXPECT_FALSE(crypto_stream->IsFlowControlBlocked());
EXPECT_FALSE(session_.IsConnectionFlowControlBlocked());
EXPECT_FALSE(session_.IsStreamFlowControlBlocked());
QuicHeadersStream* headers_stream =
QuicSpdySessionPeer::GetHeadersStream(&session_);
- EXPECT_FALSE(headers_stream->flow_controller()->IsBlocked());
+ EXPECT_FALSE(headers_stream->IsFlowControlBlocked());
EXPECT_FALSE(session_.IsConnectionFlowControlBlocked());
EXPECT_FALSE(session_.IsStreamFlowControlBlocked());
QuicStreamId stream_id = 5;
@@ -1414,7 +1414,7 @@
.WillOnce(Invoke(&ClearControlFrame));
SpdyHeaderBlock headers;
SimpleRandom random;
- while (!headers_stream->flow_controller()->IsBlocked() && stream_id < 2000) {
+ while (!headers_stream->IsFlowControlBlocked() && stream_id < 2000) {
EXPECT_FALSE(session_.IsConnectionFlowControlBlocked());
EXPECT_FALSE(session_.IsStreamFlowControlBlocked());
headers["header"] = quiche::QuicheStrCat(
@@ -1430,8 +1430,8 @@
spdy::SpdyStreamPrecedence(0), nullptr);
EXPECT_TRUE(headers_stream->HasBufferedData());
- EXPECT_TRUE(headers_stream->flow_controller()->IsBlocked());
- EXPECT_FALSE(crypto_stream->flow_controller()->IsBlocked());
+ EXPECT_TRUE(headers_stream->IsFlowControlBlocked());
+ EXPECT_FALSE(crypto_stream->IsFlowControlBlocked());
EXPECT_FALSE(session_.IsConnectionFlowControlBlocked());
EXPECT_TRUE(session_.IsStreamFlowControlBlocked());
EXPECT_FALSE(session_.HasDataToWrite());
@@ -1441,7 +1441,7 @@
CompleteHandshake();
// Stream is now unblocked and will no longer have buffered data.
- EXPECT_FALSE(headers_stream->flow_controller()->IsBlocked());
+ EXPECT_FALSE(headers_stream->IsFlowControlBlocked());
EXPECT_FALSE(session_.IsConnectionFlowControlBlocked());
EXPECT_FALSE(session_.IsStreamFlowControlBlocked());
EXPECT_TRUE(headers_stream->HasBufferedData());
@@ -1495,119 +1495,6 @@
EXPECT_EQ(kByteOffset, session_.flow_controller()->bytes_consumed());
}
-TEST_P(QuicSpdySessionTestServer,
- ConnectionFlowControlAccountingFinAndLocalReset) {
- // Test the situation where we receive a FIN on a stream, and before we fully
- // consume all the data from the sequencer buffer we locally RST the stream.
- // The bytes between highest consumed byte, and the final byte offset that we
- // determined when the FIN arrived, should be marked as consumed at the
- // connection level flow controller when the stream is reset.
- TestStream* stream = session_.CreateOutgoingBidirectionalStream();
-
- const QuicStreamOffset kByteOffset =
- kInitialSessionFlowControlWindowForTest / 2 - 1;
- QuicStreamFrame frame(stream->id(), true, kByteOffset, ".");
- session_.OnStreamFrame(frame);
- EXPECT_TRUE(connection_->connected());
-
- EXPECT_EQ(0u, stream->flow_controller()->bytes_consumed());
- EXPECT_EQ(kByteOffset + frame.data_length,
- stream->flow_controller()->highest_received_byte_offset());
-
- // Reset stream locally.
- EXPECT_CALL(*connection_, SendControlFrame(_));
- EXPECT_CALL(*connection_, OnStreamReset(stream->id(), _));
- stream->Reset(QUIC_STREAM_CANCELLED);
- EXPECT_EQ(kByteOffset + frame.data_length,
- session_.flow_controller()->bytes_consumed());
-}
-
-TEST_P(QuicSpdySessionTestServer, ConnectionFlowControlAccountingFinAfterRst) {
- CompleteHandshake();
- EXPECT_CALL(*connection_, SendControlFrame(_))
- .WillRepeatedly(Invoke(&ClearControlFrame));
- // Test that when we RST the stream (and tear down stream state), and then
- // receive a FIN from the peer, we correctly adjust our connection level flow
- // control receive window.
-
- // Connection starts with some non-zero highest received byte offset,
- // due to other active streams.
- const uint64_t kInitialConnectionBytesConsumed = 567;
- const uint64_t kInitialConnectionHighestReceivedOffset = 1234;
- EXPECT_LT(kInitialConnectionBytesConsumed,
- kInitialConnectionHighestReceivedOffset);
- session_.flow_controller()->UpdateHighestReceivedOffset(
- kInitialConnectionHighestReceivedOffset);
- session_.flow_controller()->AddBytesConsumed(kInitialConnectionBytesConsumed);
-
- // Reset our stream: this results in the stream being closed locally.
- TestStream* stream = session_.CreateOutgoingBidirectionalStream();
- if (VersionUsesHttp3(transport_version())) {
- EXPECT_CALL(*writer_, WritePacket(_, _, _, _, _))
- .WillOnce(Return(WriteResult(WRITE_STATUS_OK, 0)));
- }
- EXPECT_CALL(*connection_, OnStreamReset(stream->id(), _));
- stream->Reset(QUIC_STREAM_CANCELLED);
-
- // Now receive a response from the peer with a FIN. We should handle this by
- // adjusting the connection level flow control receive window to take into
- // account the total number of bytes sent by the peer.
- const QuicStreamOffset kByteOffset = 5678;
- std::string body = "hello";
- QuicStreamFrame frame(stream->id(), true, kByteOffset,
- quiche::QuicheStringPiece(body));
- session_.OnStreamFrame(frame);
-
- QuicStreamOffset total_stream_bytes_sent_by_peer =
- kByteOffset + body.length();
- EXPECT_EQ(kInitialConnectionBytesConsumed + total_stream_bytes_sent_by_peer,
- session_.flow_controller()->bytes_consumed());
- EXPECT_EQ(
- kInitialConnectionHighestReceivedOffset + total_stream_bytes_sent_by_peer,
- session_.flow_controller()->highest_received_byte_offset());
-}
-
-TEST_P(QuicSpdySessionTestServer, ConnectionFlowControlAccountingRstAfterRst) {
- CompleteHandshake();
- // Test that when we RST the stream (and tear down stream state), and then
- // receive a RST from the peer, we correctly adjust our connection level flow
- // control receive window.
-
- // Connection starts with some non-zero highest received byte offset,
- // due to other active streams.
- const uint64_t kInitialConnectionBytesConsumed = 567;
- const uint64_t kInitialConnectionHighestReceivedOffset = 1234;
- EXPECT_LT(kInitialConnectionBytesConsumed,
- kInitialConnectionHighestReceivedOffset);
- session_.flow_controller()->UpdateHighestReceivedOffset(
- kInitialConnectionHighestReceivedOffset);
- session_.flow_controller()->AddBytesConsumed(kInitialConnectionBytesConsumed);
-
- // Reset our stream: this results in the stream being closed locally.
- TestStream* stream = session_.CreateOutgoingBidirectionalStream();
- if (VersionUsesHttp3(transport_version())) {
- EXPECT_CALL(*writer_, WritePacket(_, _, _, _, _))
- .WillOnce(Return(WriteResult(WRITE_STATUS_OK, 0)));
- }
- EXPECT_CALL(*connection_, SendControlFrame(_));
- EXPECT_CALL(*connection_, OnStreamReset(stream->id(), _));
- stream->Reset(QUIC_STREAM_CANCELLED);
- EXPECT_TRUE(QuicStreamPeer::read_side_closed(stream));
-
- // Now receive a RST from the peer. We should handle this by adjusting the
- // connection level flow control receive window to take into account the total
- // number of bytes sent by the peer.
- const QuicStreamOffset kByteOffset = 5678;
- QuicRstStreamFrame rst_frame(kInvalidControlFrameId, stream->id(),
- QUIC_STREAM_CANCELLED, kByteOffset);
- session_.OnRstStream(rst_frame);
-
- EXPECT_EQ(kInitialConnectionBytesConsumed + kByteOffset,
- session_.flow_controller()->bytes_consumed());
- EXPECT_EQ(kInitialConnectionHighestReceivedOffset + kByteOffset,
- session_.flow_controller()->highest_received_byte_offset());
-}
-
TEST_P(QuicSpdySessionTestServer, InvalidStreamFlowControlWindowInHandshake) {
if (GetParam().handshake_protocol == PROTOCOL_TLS1_3) {
// IETF Quic doesn't require a minimum flow control window.
@@ -1624,22 +1511,6 @@
session_.OnConfigNegotiated();
}
-TEST_P(QuicSpdySessionTestServer, InvalidSessionFlowControlWindowInHandshake) {
- if (GetParam().handshake_protocol == PROTOCOL_TLS1_3) {
- // IETF Quic doesn't require a minimum flow control window.
- return;
- }
- // Test that receipt of an invalid (< default) session flow control window
- // from the peer results in the connection being torn down.
- const uint32_t kInvalidWindow = kMinimumFlowControlSendWindow - 1;
- QuicConfigPeer::SetReceivedInitialSessionFlowControlWindow(session_.config(),
- kInvalidWindow);
-
- EXPECT_CALL(*connection_,
- CloseConnection(QUIC_FLOW_CONTROL_INVALID_WINDOW, _, _));
- session_.OnConfigNegotiated();
-}
-
TEST_P(QuicSpdySessionTestServer, TooLowUnidirectionalStreamLimitHttp3) {
if (!VersionUsesHttp3(transport_version())) {
return;
@@ -1666,34 +1537,6 @@
session_.flow_controller()));
}
-TEST_P(QuicSpdySessionTestServer, FlowControlWithInvalidFinalOffset) {
- CompleteHandshake();
- // Test that if we receive a stream RST with a highest byte offset that
- // violates flow control, that we close the connection.
- const uint64_t kLargeOffset = kInitialSessionFlowControlWindowForTest + 1;
- EXPECT_CALL(*connection_,
- CloseConnection(QUIC_FLOW_CONTROL_RECEIVED_TOO_MUCH_DATA, _, _))
- .Times(2);
-
- // Check that stream frame + FIN results in connection close.
- TestStream* stream = session_.CreateOutgoingBidirectionalStream();
- if (VersionUsesHttp3(transport_version())) {
- EXPECT_CALL(*writer_, WritePacket(_, _, _, _, _))
- .WillOnce(Return(WriteResult(WRITE_STATUS_OK, 0)));
- }
- EXPECT_CALL(*connection_, SendControlFrame(_));
- EXPECT_CALL(*connection_, OnStreamReset(stream->id(), _));
- stream->Reset(QUIC_STREAM_CANCELLED);
- QuicStreamFrame frame(stream->id(), true, kLargeOffset,
- quiche::QuicheStringPiece());
- session_.OnStreamFrame(frame);
-
- // Check that RST results in connection close.
- QuicRstStreamFrame rst_frame(kInvalidControlFrameId, stream->id(),
- QUIC_STREAM_CANCELLED, kLargeOffset);
- session_.OnRstStream(rst_frame);
-}
-
TEST_P(QuicSpdySessionTestServer, WindowUpdateUnblocksHeadersStream) {
if (VersionUsesHttp3(transport_version())) {
// The test relies on headers stream, which no longer exists in IETF QUIC.
@@ -1706,9 +1549,8 @@
// Set the headers stream to be flow control blocked.
QuicHeadersStream* headers_stream =
QuicSpdySessionPeer::GetHeadersStream(&session_);
- QuicFlowControllerPeer::SetSendWindowOffset(headers_stream->flow_controller(),
- 0);
- EXPECT_TRUE(headers_stream->flow_controller()->IsBlocked());
+ QuicStreamPeer::SetSendWindowOffset(headers_stream, 0);
+ EXPECT_TRUE(headers_stream->IsFlowControlBlocked());
EXPECT_FALSE(session_.IsConnectionFlowControlBlocked());
EXPECT_TRUE(session_.IsStreamFlowControlBlocked());
@@ -1717,7 +1559,7 @@
headers_stream->id(),
2 * kMinimumFlowControlSendWindow);
session_.OnWindowUpdateFrame(window_update_frame);
- EXPECT_FALSE(headers_stream->flow_controller()->IsBlocked());
+ EXPECT_FALSE(headers_stream->IsFlowControlBlocked());
EXPECT_FALSE(session_.IsConnectionFlowControlBlocked());
EXPECT_FALSE(session_.IsStreamFlowControlBlocked());
}
@@ -2048,7 +1890,7 @@
EXPECT_EQ(1u, QuicSessionPeer::GetNumOpenDynamicStreams(&session_));
QuicStream* stream = session_.GetOrCreateStream(stream_id1);
- EXPECT_EQ(1u, stream->flow_controller()->bytes_consumed());
+ EXPECT_EQ(1u, QuicStreamPeer::bytes_consumed(stream));
EXPECT_EQ(1u, session_.flow_controller()->bytes_consumed());
// The same stream type can be encoded differently.
@@ -2060,7 +1902,7 @@
EXPECT_EQ(2u, QuicSessionPeer::GetNumOpenDynamicStreams(&session_));
stream = session_.GetOrCreateStream(stream_id2);
- EXPECT_EQ(4u, stream->flow_controller()->bytes_consumed());
+ EXPECT_EQ(4u, QuicStreamPeer::bytes_consumed(stream));
EXPECT_EQ(5u, session_.flow_controller()->bytes_consumed());
}
@@ -2092,7 +1934,7 @@
session_.OnStreamFrame(data1);
EXPECT_EQ(1u, QuicSessionPeer::GetNumOpenDynamicStreams(&session_));
QuicStream* stream = session_.GetOrCreateStream(stream_id);
- EXPECT_EQ(3u, stream->flow_controller()->highest_received_byte_offset());
+ EXPECT_EQ(3u, stream->highest_received_byte_offset());
}
TEST_P(QuicSpdySessionTestServer, OnStreamFrameLost) {
