gfe-relnote: Remove the dependency from QuicMemSliceSpan to QuicStreamSendBuffer and QuicMessageFrame. Refactor only, not protected.
Currently, the (conceptually low level) QuicMemSliceSpan depends on (conceptually high level) QuicStreamSendBuffer and QuicMessageFrame classes, via QuicMemSliceSpan::SaveMemSlicesInSendBuffer and QuicMemSliceSpan::SaveMemSlicesAsMessageData. This change replace those functions by a new QuicMemSliceSpan::ConsumeAll method, which takes a callback instead of directly referring to QuicStreamSendBuffer and QuicMessageFrame.
PiperOrigin-RevId: 240908280
Change-Id: I83c87037586252c1335f3f1e198e6b62915e5698
diff --git a/quic/core/frames/quic_message_frame.cc b/quic/core/frames/quic_message_frame.cc
index c0de272..196f6e9 100644
--- a/quic/core/frames/quic_message_frame.cc
+++ b/quic/core/frames/quic_message_frame.cc
@@ -6,6 +6,7 @@
#include "net/third_party/quiche/src/quic/core/quic_constants.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_logging.h"
+#include "net/third_party/quiche/src/quic/platform/api/quic_mem_slice.h"
namespace quic {
@@ -15,6 +16,15 @@
QuicMessageFrame::QuicMessageFrame(QuicMessageId message_id)
: message_id(message_id), data(nullptr), message_length(0) {}
+QuicMessageFrame::QuicMessageFrame(QuicMessageId message_id,
+ QuicMemSliceSpan span)
+ : message_id(message_id), data(nullptr), message_length(0) {
+ span.ConsumeAll([&](QuicMemSlice slice) {
+ message_length += slice.length();
+ message_data.push_back(std::move(slice));
+ });
+}
+
QuicMessageFrame::QuicMessageFrame(const char* data, QuicPacketLength length)
: message_id(0), data(data), message_length(length) {}
diff --git a/quic/core/frames/quic_message_frame.h b/quic/core/frames/quic_message_frame.h
index 4accff2..78dafe3 100644
--- a/quic/core/frames/quic_message_frame.h
+++ b/quic/core/frames/quic_message_frame.h
@@ -9,6 +9,7 @@
#include "net/third_party/quiche/src/quic/platform/api/quic_containers.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_export.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_mem_slice.h"
+#include "net/third_party/quiche/src/quic/platform/api/quic_mem_slice_span.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_string_piece.h"
namespace quic {
@@ -18,6 +19,7 @@
struct QUIC_EXPORT_PRIVATE QuicMessageFrame {
QuicMessageFrame();
explicit QuicMessageFrame(QuicMessageId message_id);
+ QuicMessageFrame(QuicMessageId message_id, QuicMemSliceSpan span);
QuicMessageFrame(const char* data, QuicPacketLength length);
QuicMessageFrame(const QuicMessageFrame& other) = delete;
diff --git a/quic/core/quic_framer_test.cc b/quic/core/quic_framer_test.cc
index 2985059..9f0e700 100644
--- a/quic/core/quic_framer_test.cc
+++ b/quic/core/quic_framer_test.cc
@@ -8432,11 +8432,8 @@
header.packet_number = kPacketNumber;
QuicMemSliceStorage storage(nullptr, 0, nullptr, 0);
- QuicMessageFrame frame(1);
- MakeSpan(&allocator_, "message", &storage).SaveMemSlicesAsMessageData(&frame);
- QuicMessageFrame frame2(2);
- MakeSpan(&allocator_, "message2", &storage)
- .SaveMemSlicesAsMessageData(&frame2);
+ QuicMessageFrame frame(1, MakeSpan(&allocator_, "message", &storage));
+ QuicMessageFrame frame2(2, MakeSpan(&allocator_, "message2", &storage));
QuicFrames frames = {QuicFrame(&frame), QuicFrame(&frame2)};
// clang-format off
diff --git a/quic/core/quic_packet_creator_test.cc b/quic/core/quic_packet_creator_test.cc
index 66b92b9..45f2651 100644
--- a/quic/core/quic_packet_creator_test.cc
+++ b/quic/core/quic_packet_creator_test.cc
@@ -1616,23 +1616,21 @@
EXPECT_TRUE(
creator_.HasRoomForMessageFrame(creator_.GetLargestMessagePayload()));
std::string message(creator_.GetLargestMessagePayload(), 'a');
- QuicMessageFrame* message_frame = new QuicMessageFrame(1);
- MakeSpan(&allocator_, message, &storage)
- .SaveMemSlicesAsMessageData(message_frame);
+ QuicMessageFrame* message_frame =
+ new QuicMessageFrame(1, MakeSpan(&allocator_, message, &storage));
EXPECT_TRUE(
creator_.AddSavedFrame(QuicFrame(message_frame), NOT_RETRANSMISSION));
EXPECT_TRUE(creator_.HasPendingFrames());
creator_.Flush();
- QuicMessageFrame* frame2 = new QuicMessageFrame(2);
- MakeSpan(&allocator_, "message", &storage).SaveMemSlicesAsMessageData(frame2);
+ QuicMessageFrame* frame2 =
+ new QuicMessageFrame(2, MakeSpan(&allocator_, "message", &storage));
EXPECT_TRUE(creator_.AddSavedFrame(QuicFrame(frame2), NOT_RETRANSMISSION));
EXPECT_TRUE(creator_.HasPendingFrames());
// Verify if a new frame is added, 1 byte message length will be added.
EXPECT_EQ(1u, creator_.ExpansionOnNewFrame());
- QuicMessageFrame* frame3 = new QuicMessageFrame(3);
- MakeSpan(&allocator_, "message2", &storage)
- .SaveMemSlicesAsMessageData(frame3);
+ QuicMessageFrame* frame3 =
+ new QuicMessageFrame(3, MakeSpan(&allocator_, "message2", &storage));
EXPECT_TRUE(creator_.AddSavedFrame(QuicFrame(frame3), NOT_RETRANSMISSION));
EXPECT_EQ(1u, creator_.ExpansionOnNewFrame());
creator_.Flush();
@@ -1643,16 +1641,15 @@
client_framer_.transport_version(), Perspective::IS_CLIENT);
EXPECT_TRUE(creator_.ConsumeData(stream_id, &iov_, 1u, iov_.iov_len, 0u, 0u,
false, false, NOT_RETRANSMISSION, &frame));
- QuicMessageFrame* frame4 = new QuicMessageFrame(4);
- MakeSpan(&allocator_, "message", &storage).SaveMemSlicesAsMessageData(frame4);
+ QuicMessageFrame* frame4 =
+ new QuicMessageFrame(4, MakeSpan(&allocator_, "message", &storage));
EXPECT_TRUE(creator_.AddSavedFrame(QuicFrame(frame4), NOT_RETRANSMISSION));
EXPECT_TRUE(creator_.HasPendingFrames());
// Verify there is not enough room for largest payload.
EXPECT_FALSE(
creator_.HasRoomForMessageFrame(creator_.GetLargestMessagePayload()));
// Add largest message will causes the flush of the stream frame.
- QuicMessageFrame frame5(5);
- MakeSpan(&allocator_, message, &storage).SaveMemSlicesAsMessageData(&frame5);
+ QuicMessageFrame frame5(5, MakeSpan(&allocator_, message, &storage));
EXPECT_FALSE(creator_.AddSavedFrame(QuicFrame(&frame5), NOT_RETRANSMISSION));
EXPECT_FALSE(creator_.HasPendingFrames());
}
@@ -1667,10 +1664,10 @@
// Test all possible size of message frames.
for (size_t message_size = 0;
message_size <= creator_.GetLargestMessagePayload(); ++message_size) {
- QuicMessageFrame* frame = new QuicMessageFrame(0);
- MakeSpan(&allocator_, QuicStringPiece(message_buffer.data(), message_size),
- &storage)
- .SaveMemSlicesAsMessageData(frame);
+ QuicMessageFrame* frame = new QuicMessageFrame(
+ 0, MakeSpan(&allocator_,
+ QuicStringPiece(message_buffer.data(), message_size),
+ &storage));
EXPECT_TRUE(creator_.AddSavedFrame(QuicFrame(frame), NOT_RETRANSMISSION));
EXPECT_TRUE(creator_.HasPendingFrames());
diff --git a/quic/core/quic_packet_generator.cc b/quic/core/quic_packet_generator.cc
index c2b3d48..257ca9b 100644
--- a/quic/core/quic_packet_generator.cc
+++ b/quic/core/quic_packet_generator.cc
@@ -479,8 +479,7 @@
if (!packet_creator_.HasRoomForMessageFrame(message_length)) {
packet_creator_.Flush();
}
- QuicMessageFrame* frame = new QuicMessageFrame(message_id);
- message.SaveMemSlicesAsMessageData(frame);
+ QuicMessageFrame* frame = new QuicMessageFrame(message_id, message);
const bool success =
packet_creator_.AddSavedFrame(QuicFrame(frame), next_transmission_type_);
if (!success) {
diff --git a/quic/core/quic_stream.cc b/quic/core/quic_stream.cc
index ef621fe..a3a63cd 100644
--- a/quic/core/quic_stream.cc
+++ b/quic/core/quic_stream.cc
@@ -601,8 +601,7 @@
if (!span.empty()) {
// Buffer all data if buffered data size is below limit.
QuicStreamOffset offset = send_buffer_.stream_offset();
- consumed_data.bytes_consumed =
- span.SaveMemSlicesInSendBuffer(&send_buffer_);
+ consumed_data.bytes_consumed = send_buffer_.SaveMemSliceSpan(span);
if (offset > send_buffer_.stream_offset() ||
kMaxStreamLength < send_buffer_.stream_offset()) {
QUIC_BUG << "Write too many data via stream " << id_;
diff --git a/quic/core/quic_stream_send_buffer.cc b/quic/core/quic_stream_send_buffer.cc
index 5b66786..ead192d 100644
--- a/quic/core/quic_stream_send_buffer.cc
+++ b/quic/core/quic_stream_send_buffer.cc
@@ -82,6 +82,11 @@
stream_offset_ += length;
}
+QuicByteCount QuicStreamSendBuffer::SaveMemSliceSpan(QuicMemSliceSpan span) {
+ return span.ConsumeAll(
+ [&](QuicMemSlice slice) { SaveMemSlice(std::move(slice)); });
+}
+
void QuicStreamSendBuffer::OnStreamDataConsumed(size_t bytes_consumed) {
stream_bytes_written_ += bytes_consumed;
stream_bytes_outstanding_ += bytes_consumed;
diff --git a/quic/core/quic_stream_send_buffer.h b/quic/core/quic_stream_send_buffer.h
index 4bcac34..e34514b 100644
--- a/quic/core/quic_stream_send_buffer.h
+++ b/quic/core/quic_stream_send_buffer.h
@@ -7,9 +7,11 @@
#include "net/third_party/quiche/src/quic/core/frames/quic_stream_frame.h"
#include "net/third_party/quiche/src/quic/core/quic_interval_set.h"
+#include "net/third_party/quiche/src/quic/core/quic_types.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_containers.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_iovec.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_mem_slice.h"
+#include "net/third_party/quiche/src/quic/platform/api/quic_mem_slice_span.h"
namespace quic {
@@ -72,6 +74,9 @@
// Save |slice| to send buffer.
void SaveMemSlice(QuicMemSlice slice);
+ // Save all slices in |span| to send buffer. Return total bytes saved.
+ QuicByteCount SaveMemSliceSpan(QuicMemSliceSpan span);
+
// Called when |bytes_consumed| bytes has been consumed by the stream.
void OnStreamDataConsumed(size_t bytes_consumed);
diff --git a/quic/core/quic_stream_send_buffer_test.cc b/quic/core/quic_stream_send_buffer_test.cc
index 23a71fd..57e2bd1 100644
--- a/quic/core/quic_stream_send_buffer_test.cc
+++ b/quic/core/quic_stream_send_buffer_test.cc
@@ -12,6 +12,7 @@
#include "net/third_party/quiche/src/quic/platform/api/quic_expect_bug.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_flags.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_test.h"
+#include "net/third_party/quiche/src/quic/platform/api/quic_test_mem_slice_vector.h"
#include "net/third_party/quiche/src/quic/test_tools/quic_stream_send_buffer_peer.h"
#include "net/third_party/quiche/src/quic/test_tools/quic_test_utils.h"
@@ -287,6 +288,38 @@
QuicStreamSendBufferPeer::CurrentWriteSlice(&send_buffer_)->offset);
}
+TEST_F(QuicStreamSendBufferTest, SaveMemSliceSpan) {
+ SimpleBufferAllocator allocator;
+ QuicStreamSendBuffer send_buffer(&allocator);
+
+ char data[1024];
+ std::vector<std::pair<char*, size_t>> buffers;
+ for (size_t i = 0; i < 10; ++i) {
+ buffers.push_back(std::make_pair(data, 1024));
+ }
+ QuicTestMemSliceVector vector(buffers);
+
+ EXPECT_EQ(10 * 1024u, send_buffer.SaveMemSliceSpan(vector.span()));
+ EXPECT_EQ(10u, send_buffer.size());
+}
+
+TEST_F(QuicStreamSendBufferTest, SaveEmptyMemSliceSpan) {
+ SimpleBufferAllocator allocator;
+ QuicStreamSendBuffer send_buffer(&allocator);
+
+ char data[1024];
+ std::vector<std::pair<char*, size_t>> buffers;
+ for (size_t i = 0; i < 10; ++i) {
+ buffers.push_back(std::make_pair(data, 1024));
+ }
+ buffers.push_back(std::make_pair(nullptr, 0));
+ QuicTestMemSliceVector vector(buffers);
+
+ EXPECT_EQ(10 * 1024u, send_buffer.SaveMemSliceSpan(vector.span()));
+ // Verify the empty slice does not get saved.
+ EXPECT_EQ(10u, send_buffer.size());
+}
+
} // namespace
} // namespace test
} // namespace quic
