Remove stream pointer from QuicSession::WritevData.
The session only uses the pointer to access stream_bytes_written. It could simply be replaced by a |is_retransmission| boolean. This change makes the boundary of streams and sessions clearer.
gfe-relnote: no behavior change, not protected.
PiperOrigin-RevId: 296504335
Change-Id: I349bdadadd923e9dcb5b03231ce351e4f1a25c96
diff --git a/quic/core/http/quic_headers_stream_test.cc b/quic/core/http/quic_headers_stream_test.cc
index 68bbadf..4337318 100644
--- a/quic/core/http/quic_headers_stream_test.cc
+++ b/quic/core/http/quic_headers_stream_test.cc
@@ -286,11 +286,10 @@
SpdyPriority priority,
bool is_request) {
// Write the headers and capture the outgoing data
- EXPECT_CALL(session_, WritevData(headers_stream_,
- QuicUtils::GetHeadersStreamId(
+ EXPECT_CALL(session_, WritevData(QuicUtils::GetHeadersStreamId(
connection_->transport_version()),
- _, _, NO_FIN))
- .WillOnce(WithArgs<2>(Invoke(this, &QuicHeadersStreamTest::SaveIov)));
+ _, _, NO_FIN, _))
+ .WillOnce(WithArgs<1>(Invoke(this, &QuicHeadersStreamTest::SaveIov)));
QuicSpdySessionPeer::WriteHeadersOnHeadersStream(
&session_, stream_id, headers_.Clone(), fin,
spdy::SpdyStreamPrecedence(priority), nullptr);
@@ -411,11 +410,10 @@
QuicStreamId promised_stream_id = NextPromisedStreamId();
if (perspective() == Perspective::IS_SERVER) {
// Write the headers and capture the outgoing data
- EXPECT_CALL(session_, WritevData(headers_stream_,
- QuicUtils::GetHeadersStreamId(
+ EXPECT_CALL(session_, WritevData(QuicUtils::GetHeadersStreamId(
connection_->transport_version()),
- _, _, NO_FIN))
- .WillOnce(WithArgs<2>(Invoke(this, &QuicHeadersStreamTest::SaveIov)));
+ _, _, NO_FIN, _))
+ .WillOnce(WithArgs<1>(Invoke(this, &QuicHeadersStreamTest::SaveIov)));
session_.WritePushPromise(stream_id, promised_stream_id,
headers_.Clone());
@@ -827,11 +825,10 @@
}
TEST_P(QuicHeadersStreamTest, AckSentData) {
- EXPECT_CALL(session_, WritevData(headers_stream_,
- QuicUtils::GetHeadersStreamId(
+ EXPECT_CALL(session_, WritevData(QuicUtils::GetHeadersStreamId(
connection_->transport_version()),
- _, _, NO_FIN))
- .WillRepeatedly(Invoke(MockQuicSession::ConsumeData));
+ _, _, NO_FIN, _))
+ .WillRepeatedly(Invoke(&session_, &MockQuicSpdySession::ConsumeData));
InSequence s;
QuicReferenceCountedPointer<MockAckListener> ack_listener1(
new MockAckListener());
@@ -897,11 +894,10 @@
TEST_P(QuicHeadersStreamTest, FrameContainsMultipleHeaders) {
// In this test, a stream frame can contain multiple headers.
- EXPECT_CALL(session_, WritevData(headers_stream_,
- QuicUtils::GetHeadersStreamId(
+ EXPECT_CALL(session_, WritevData(QuicUtils::GetHeadersStreamId(
connection_->transport_version()),
- _, _, NO_FIN))
- .WillRepeatedly(Invoke(MockQuicSession::ConsumeData));
+ _, _, NO_FIN, _))
+ .WillRepeatedly(Invoke(&session_, &MockQuicSpdySession::ConsumeData));
InSequence s;
QuicReferenceCountedPointer<MockAckListener> ack_listener1(
new MockAckListener());
@@ -948,11 +944,10 @@
}
TEST_P(QuicHeadersStreamTest, HeadersGetAckedMultipleTimes) {
- EXPECT_CALL(session_, WritevData(headers_stream_,
- QuicUtils::GetHeadersStreamId(
+ EXPECT_CALL(session_, WritevData(QuicUtils::GetHeadersStreamId(
connection_->transport_version()),
- _, _, NO_FIN))
- .WillRepeatedly(Invoke(MockQuicSession::ConsumeData));
+ _, _, NO_FIN, _))
+ .WillRepeatedly(Invoke(&session_, &MockQuicSpdySession::ConsumeData));
InSequence s;
QuicReferenceCountedPointer<MockAckListener> ack_listener1(
new MockAckListener());
diff --git a/quic/core/http/quic_send_control_stream_test.cc b/quic/core/http/quic_send_control_stream_test.cc
index 1bc10bf..49c5dea 100644
--- a/quic/core/http/quic_send_control_stream_test.cc
+++ b/quic/core/http/quic_send_control_stream_test.cc
@@ -74,7 +74,7 @@
SupportedVersions(GetParam().version))),
session_(connection_) {
ON_CALL(session_, WritevData(_, _, _, _, _))
- .WillByDefault(Invoke(MockQuicSession::ConsumeData));
+ .WillByDefault(Invoke(&session_, &MockQuicSpdySession::ConsumeData));
}
void Initialize() {
@@ -132,20 +132,22 @@
// A lambda to save and consume stream data when QuicSession::WritevData() is
// called.
- auto save_write_data = [&writer](QuicStream* stream, QuicStreamId /*id*/,
- size_t write_length, QuicStreamOffset offset,
- StreamSendingState /*state*/) {
- stream->WriteStreamData(offset, write_length, &writer);
+ auto save_write_data = [&writer, this](QuicStreamId /*id*/,
+ size_t write_length,
+ QuicStreamOffset offset,
+ StreamSendingState /*state*/,
+ bool /*is_retransmission*/) {
+ send_control_stream_->WriteStreamData(offset, write_length, &writer);
return QuicConsumedData(/* bytes_consumed = */ write_length,
/* fin_consumed = */ false);
};
- EXPECT_CALL(session_, WritevData(send_control_stream_, _, 1, _, _))
+ EXPECT_CALL(session_, WritevData(send_control_stream_->id(), 1, _, _, _))
.WillOnce(Invoke(save_write_data));
- EXPECT_CALL(session_, WritevData(send_control_stream_, _,
- expected_write_data.size() - 5, _, _))
+ EXPECT_CALL(session_, WritevData(send_control_stream_->id(),
+ expected_write_data.size() - 5, _, _, _))
.WillOnce(Invoke(save_write_data));
- EXPECT_CALL(session_, WritevData(send_control_stream_, _, 4, _, _))
+ EXPECT_CALL(session_, WritevData(send_control_stream_->id(), 4, _, _, _))
.WillOnce(Invoke(save_write_data));
send_control_stream_->MaybeSendSettingsFrame();
@@ -157,8 +159,9 @@
Initialize();
testing::InSequence s;
- EXPECT_CALL(session_, WritevData(send_control_stream_, _, 1, _, _));
- EXPECT_CALL(session_, WritevData(send_control_stream_, _, _, _, _)).Times(2);
+ EXPECT_CALL(session_, WritevData(send_control_stream_->id(), 1, _, _, _));
+ EXPECT_CALL(session_, WritevData(send_control_stream_->id(), _, _, _, _))
+ .Times(2);
send_control_stream_->MaybeSendSettingsFrame();
// No data should be written the second time MaybeSendSettingsFrame() is
@@ -173,11 +176,12 @@
// The first write will trigger the control stream to write stream type, a
// SETTINGS frame, and a greased frame before the PRIORITY_UPDATE frame.
- EXPECT_CALL(session_, WritevData(send_control_stream_, _, _, _, _)).Times(4);
+ EXPECT_CALL(session_, WritevData(send_control_stream_->id(), _, _, _, _))
+ .Times(4);
PriorityUpdateFrame frame;
send_control_stream_->WritePriorityUpdate(frame);
- EXPECT_CALL(session_, WritevData(send_control_stream_, _, _, _, _));
+ EXPECT_CALL(session_, WritevData(send_control_stream_->id(), _, _, _, _));
send_control_stream_->WritePriorityUpdate(frame);
}
diff --git a/quic/core/http/quic_spdy_client_session_test.cc b/quic/core/http/quic_spdy_client_session_test.cc
index 2ebcbed..01e9fd8 100644
--- a/quic/core/http/quic_spdy_client_session_test.cc
+++ b/quic/core/http/quic_spdy_client_session_test.cc
@@ -232,7 +232,7 @@
// Verify that no data may be send on existing streams.
char data[] = "hello world";
QuicConsumedData consumed = session_->WritevData(
- stream, stream->id(), QUICHE_ARRAYSIZE(data), 0, NO_FIN);
+ stream->id(), QUICHE_ARRAYSIZE(data), 0, NO_FIN, false);
EXPECT_FALSE(consumed.fin_consumed);
EXPECT_EQ(0u, consumed.bytes_consumed);
}
diff --git a/quic/core/http/quic_spdy_session_test.cc b/quic/core/http/quic_spdy_session_test.cc
index fc2f653..bff8bbc 100644
--- a/quic/core/http/quic_spdy_session_test.cc
+++ b/quic/core/http/quic_spdy_session_test.cc
@@ -267,19 +267,16 @@
return QuicSpdySession::GetOrCreateStream(stream_id);
}
- QuicConsumedData WritevData(QuicStream* stream,
- QuicStreamId id,
+ QuicConsumedData WritevData(QuicStreamId id,
size_t write_length,
QuicStreamOffset offset,
- StreamSendingState state) override {
+ StreamSendingState state,
+ bool is_retransmission) override {
bool fin = state != NO_FIN;
QuicConsumedData consumed(write_length, fin);
if (!writev_consumes_all_data_) {
- consumed =
- QuicSession::WritevData(stream, id, write_length, offset, state);
- }
- if (fin && consumed.fin_consumed) {
- stream->set_fin_sent(true);
+ consumed = QuicSession::WritevData(id, write_length, offset, state,
+ is_retransmission);
}
QuicSessionPeer::GetWriteBlockedStreams(this)->UpdateBytesForStream(
id, consumed.bytes_consumed);
@@ -299,7 +296,7 @@
}
MakeIOVector("not empty", &iov);
QuicStreamPeer::SendBuffer(stream).SaveStreamData(&iov, 1, 0, 9);
- QuicConsumedData consumed = WritevData(stream, stream->id(), 9, 0, FIN);
+ QuicConsumedData consumed = WritevData(stream->id(), 9, 0, FIN, false);
QuicStreamPeer::SendBuffer(stream).OnStreamDataConsumed(
consumed.bytes_consumed);
return consumed;
@@ -307,7 +304,7 @@
QuicConsumedData SendLargeFakeData(QuicStream* stream, int bytes) {
DCHECK(writev_consumes_all_data_);
- return WritevData(stream, stream->id(), bytes, 0, FIN);
+ return WritevData(stream->id(), bytes, 0, FIN, false);
}
using QuicSession::closed_streams;
diff --git a/quic/core/http/quic_spdy_stream_test.cc b/quic/core/http/quic_spdy_stream_test.cc
index e68091d..1fca1db 100644
--- a/quic/core/http/quic_spdy_stream_test.cc
+++ b/quic/core/http/quic_spdy_stream_test.cc
@@ -203,7 +203,8 @@
session_ = std::make_unique<StrictMock<MockQuicSpdySession>>(connection_);
session_->Initialize();
ON_CALL(*session_, WritevData(_, _, _, _, _))
- .WillByDefault(Invoke(MockQuicSession::ConsumeData));
+ .WillByDefault(
+ Invoke(session_.get(), &MockQuicSpdySession::ConsumeData));
stream_ =
new StrictMock<TestStream>(GetNthClientInitiatedBidirectionalId(0),
@@ -236,17 +237,16 @@
}
auto send_control_stream =
QuicSpdySessionPeer::GetSendControlStream(session_.get());
- EXPECT_CALL(*session_, WritevData(send_control_stream,
- send_control_stream->id(), _, _, _))
+ EXPECT_CALL(*session_, WritevData(send_control_stream->id(), _, _, _, _))
.Times(num_control_stream_writes);
auto qpack_decoder_stream =
QuicSpdySessionPeer::GetQpackDecoderSendStream(session_.get());
- EXPECT_CALL(*session_, WritevData(qpack_decoder_stream,
- qpack_decoder_stream->id(), 1, 0, _));
+ EXPECT_CALL(*session_,
+ WritevData(qpack_decoder_stream->id(), 1, 0, _, _));
auto qpack_encoder_stream =
QuicSpdySessionPeer::GetQpackEncoderSendStream(session_.get());
- EXPECT_CALL(*session_, WritevData(qpack_encoder_stream,
- qpack_encoder_stream->id(), 1, 0, _));
+ EXPECT_CALL(*session_,
+ WritevData(qpack_encoder_stream->id(), 1, 0, _, _));
}
static_cast<QuicSession*>(session_.get())
->SetDefaultEncryptionLevel(ENCRYPTION_ZERO_RTT);
@@ -769,7 +769,7 @@
const uint64_t kHeaderLength = UsesHttp3() ? 2 : 0;
if (UsesHttp3()) {
- EXPECT_CALL(*session_, WritevData(_, _, kHeaderLength, _, NO_FIN));
+ EXPECT_CALL(*session_, WritevData(_, kHeaderLength, _, NO_FIN, _));
}
EXPECT_CALL(*session_, WritevData(_, _, _, _, _))
.WillOnce(Return(QuicConsumedData(kWindow - kHeaderLength, true)));
@@ -1047,7 +1047,7 @@
EXPECT_CALL(*connection_,
SendBlocked(GetNthClientInitiatedBidirectionalId(0)))
.Times(0);
- EXPECT_CALL(*session_, WritevData(_, _, 0, _, FIN));
+ EXPECT_CALL(*session_, WritevData(_, 0, _, FIN, _));
stream_->WriteOrBufferBody(body, fin);
}
@@ -1290,8 +1290,7 @@
// In this case, TestStream::WriteHeadersImpl() does not prevent writes.
// Four writes on the request stream: HEADERS frame header and payload both
// for headers and trailers.
- EXPECT_CALL(*session_, WritevData(stream_, stream_->id(), _, _, _))
- .Times(4);
+ EXPECT_CALL(*session_, WritevData(stream_->id(), _, _, _, _)).Times(4);
}
// Write the initial headers, without a FIN.
@@ -1315,14 +1314,13 @@
// Four writes on the request stream: HEADERS frame header and payload both
// for headers and trailers.
- EXPECT_CALL(*session_, WritevData(stream_, stream_->id(), _, _, _)).Times(4);
+ EXPECT_CALL(*session_, WritevData(stream_->id(), _, _, _, _)).Times(4);
// No PRIORITY_UPDATE frames on the control stream,
// because the stream has default priority.
auto send_control_stream =
QuicSpdySessionPeer::GetSendControlStream(session_.get());
- EXPECT_CALL(*session_, WritevData(send_control_stream,
- send_control_stream->id(), _, _, _))
+ EXPECT_CALL(*session_, WritevData(send_control_stream->id(), _, _, _, _))
.Times(0);
// Write the initial headers, without a FIN.
@@ -1345,15 +1343,14 @@
InitializeWithPerspective(kShouldProcessData, Perspective::IS_CLIENT);
// Two writes on the request stream: HEADERS frame header and payload.
- EXPECT_CALL(*session_, WritevData(stream_, stream_->id(), _, _, _)).Times(2);
+ EXPECT_CALL(*session_, WritevData(stream_->id(), _, _, _, _)).Times(2);
EXPECT_CALL(*stream_, WriteHeadersMock(false));
stream_->WriteHeaders(SpdyHeaderBlock(), /*fin=*/false, nullptr);
// PRIORITY_UPDATE frame on the control stream.
auto send_control_stream =
QuicSpdySessionPeer::GetSendControlStream(session_.get());
- EXPECT_CALL(*session_, WritevData(send_control_stream,
- send_control_stream->id(), _, _, _));
+ EXPECT_CALL(*session_, WritevData(send_control_stream->id(), _, _, _, _));
stream_->SetPriority(spdy::SpdyStreamPrecedence(kV3HighestPriority));
}
@@ -1368,15 +1365,14 @@
// is called, before HEADERS frame is sent.
auto send_control_stream =
QuicSpdySessionPeer::GetSendControlStream(session_.get());
- EXPECT_CALL(*session_, WritevData(send_control_stream,
- send_control_stream->id(), _, _, _));
+ EXPECT_CALL(*session_, WritevData(send_control_stream->id(), _, _, _, _));
stream_->SetPriority(spdy::SpdyStreamPrecedence(kV3HighestPriority));
testing::Mock::VerifyAndClearExpectations(session_.get());
// Two writes on the request stream: HEADERS frame header and payload.
// PRIORITY_UPDATE frame is not sent this time, because one is already sent.
- EXPECT_CALL(*session_, WritevData(stream_, stream_->id(), _, _, _)).Times(2);
+ EXPECT_CALL(*session_, WritevData(stream_->id(), _, _, _, _)).Times(2);
EXPECT_CALL(*stream_, WriteHeadersMock(true));
stream_->WriteHeaders(SpdyHeaderBlock(), /*fin=*/true, nullptr);
}
@@ -1389,8 +1385,7 @@
if (UsesHttp3()) {
// In this case, TestStream::WriteHeadersImpl() does not prevent writes.
// HEADERS frame header and payload on the request stream.
- EXPECT_CALL(*session_, WritevData(stream_, stream_->id(), _, _, _))
- .Times(2);
+ EXPECT_CALL(*session_, WritevData(stream_->id(), _, _, _, _)).Times(2);
}
// Write the initial headers.
@@ -1433,7 +1428,7 @@
// Expect data being written on the stream. In addition to that, headers are
// also written on the stream in case of IETF QUIC.
- EXPECT_CALL(*session_, WritevData(stream_, stream_->id(), _, _, _))
+ EXPECT_CALL(*session_, WritevData(stream_->id(), _, _, _, _))
.Times(AtLeast(1));
// Write the initial headers.
@@ -1472,9 +1467,9 @@
// Write non-zero body data, but only consume partially, ensuring queueing.
const int kBodySize = 1 * 1024; // 1 kB
if (UsesHttp3()) {
- EXPECT_CALL(*session_, WritevData(_, _, 3, _, NO_FIN));
+ EXPECT_CALL(*session_, WritevData(_, 3, _, NO_FIN, _));
}
- EXPECT_CALL(*session_, WritevData(_, _, kBodySize, _, NO_FIN))
+ EXPECT_CALL(*session_, WritevData(_, kBodySize, _, NO_FIN, _))
.WillOnce(Return(QuicConsumedData(kBodySize - 1, false)));
stream_->WriteOrBufferBody(std::string(kBodySize, 'x'), false);
EXPECT_EQ(1u, stream_->BufferedDataBytes());
@@ -1487,7 +1482,7 @@
EXPECT_FALSE(stream_->write_side_closed());
// Writing the queued bytes will close the write side of the stream.
- EXPECT_CALL(*session_, WritevData(_, _, 1, _, NO_FIN));
+ EXPECT_CALL(*session_, WritevData(_, 1, _, NO_FIN, _));
stream_->OnCanWrite();
EXPECT_TRUE(stream_->write_side_closed());
}
@@ -1595,9 +1590,9 @@
Initialize(kShouldProcessData);
if (UsesHttp3()) {
- EXPECT_CALL(*session_, WritevData(_, _, 2, _, NO_FIN));
+ EXPECT_CALL(*session_, WritevData(_, 2, _, NO_FIN, _));
}
- EXPECT_CALL(*session_, WritevData(_, _, 4, _, FIN));
+ EXPECT_CALL(*session_, WritevData(_, 4, _, FIN, _));
stream_->WriteOrBufferBody("data", true);
stream_->OnPriorityFrame(spdy::SpdyStreamPrecedence(kV3HighestPriority));
EXPECT_EQ(spdy::SpdyStreamPrecedence(kV3HighestPriority),
@@ -2029,8 +2024,7 @@
QuicSpdySessionPeer::GetQpackDecoderSendStream(session_.get());
// The stream byte will be written in the first byte.
- EXPECT_CALL(*session_, WritevData(decoder_send_stream,
- decoder_send_stream->id(), _, _, _));
+ EXPECT_CALL(*session_, WritevData(decoder_send_stream->id(), _, _, _, _));
// Deliver dynamic table entry to decoder.
session_->qpack_decoder()->OnInsertWithoutNameReference("foo", "bar");
@@ -2052,8 +2046,7 @@
headers.length(), data));
EXPECT_EQ(kDataFramePayload, stream_->data());
- EXPECT_CALL(*session_, WritevData(decoder_send_stream,
- decoder_send_stream->id(), _, _, _));
+ EXPECT_CALL(*session_, WritevData(decoder_send_stream->id(), _, _, _, _));
// Deliver second dynamic table entry to decoder.
session_->qpack_decoder()->OnInsertWithoutNameReference("trailing", "foobar");
@@ -2094,8 +2087,7 @@
QuicSpdySessionPeer::GetQpackDecoderSendStream(session_.get());
// The stream byte will be written in the first byte.
- EXPECT_CALL(*session_, WritevData(decoder_send_stream,
- decoder_send_stream->id(), _, _, _));
+ EXPECT_CALL(*session_, WritevData(decoder_send_stream->id(), _, _, _, _));
// Deliver dynamic table entry to decoder.
session_->qpack_decoder()->OnInsertWithoutNameReference("foo", "bar");
EXPECT_TRUE(stream_->headers_decompressed());
@@ -2120,8 +2112,7 @@
// Decoding is blocked because dynamic table entry has not been received yet.
EXPECT_FALSE(stream_->trailers_decompressed());
- EXPECT_CALL(*session_, WritevData(decoder_send_stream,
- decoder_send_stream->id(), _, _, _));
+ EXPECT_CALL(*session_, WritevData(decoder_send_stream->id(), _, _, _, _));
// Deliver second dynamic table entry to decoder.
session_->qpack_decoder()->OnInsertWithoutNameReference("trailing", "foobar");
EXPECT_TRUE(stream_->trailers_decompressed());
@@ -2215,8 +2206,7 @@
QuicSpdySessionPeer::GetQpackDecoderSendStream(session_.get());
// The stream byte will be written in the first byte.
- EXPECT_CALL(*session_, WritevData(decoder_send_stream,
- decoder_send_stream->id(), _, _, _));
+ EXPECT_CALL(*session_, WritevData(decoder_send_stream->id(), _, _, _, _));
// Deliver dynamic table entry to decoder.
session_->qpack_decoder()->OnInsertWithoutNameReference("foo", "bar");
EXPECT_TRUE(stream_->headers_decompressed());
@@ -2645,8 +2635,7 @@
auto qpack_decoder_stream =
QuicSpdySessionPeer::GetQpackDecoderSendStream(session_.get());
- EXPECT_CALL(*session_, WritevData(qpack_decoder_stream,
- qpack_decoder_stream->id(), 1, 1, _));
+ EXPECT_CALL(*session_, WritevData(qpack_decoder_stream->id(), 1, 1, _, _));
EXPECT_CALL(*session_,
SendRstStream(stream_->id(), QUIC_STREAM_CANCELLED, 0));
@@ -2664,8 +2653,7 @@
auto qpack_decoder_stream =
QuicSpdySessionPeer::GetQpackDecoderSendStream(session_.get());
- EXPECT_CALL(*session_, WritevData(qpack_decoder_stream,
- qpack_decoder_stream->id(), 1, 1, _));
+ EXPECT_CALL(*session_, WritevData(qpack_decoder_stream->id(), 1, 1, _, _));
stream_->OnStreamReset(QuicRstStreamFrame(
kInvalidControlFrameId, stream_->id(), QUIC_STREAM_CANCELLED, 0));
