Rewrite QuicSpdyStreamBodyBuffer's consumed byte tracking algorithm.
This is motivated by having to consume unknown frames (in a later CL), see
cr/258682203 for the end goal.
There is some behavioral change as a side effect of the new algorithm: DATA
frame headers are consumed immediately, not only when some of the corresponding
payload is consumed. This hinges on the previously undocumented property of HttpDecoder that in only calls Visitor::On*FrameStart() methods after the entire frame header is processed. This behavioral change necessitates minor updates to tests.
I plan four more CLs after this: one to move HEADERS frame header and
payload consumption calculations from QuicSpdyStream to
QuicSpdyStreamBodyBuffer; one to consume unknown frames; one to rename
QuicSpdyStreamBodyBuffer to QuicSpdyStreamBodyManager or something similar;
and one to remove Http3FrameLengths if by that point it's entirely unused.
gfe-relnote: n/a, change to QUIC v99-only code. Protected by existing disabled gfe2_reloadable_flag_quic_enable_version_99.
PiperOrigin-RevId: 259987101
Change-Id: Id8a734fb36466b3502373097faba2c6c81c793de
diff --git a/quic/core/http/http_decoder.h b/quic/core/http/http_decoder.h
index 99c4d10..80ba385 100644
--- a/quic/core/http/http_decoder.h
+++ b/quic/core/http/http_decoder.h
@@ -49,6 +49,9 @@
// All the following methods return true to continue decoding,
// and false to pause it.
+ // On*FrameStart() methods are called after the frame header is completely
+ // processed. At that point it is safe to consume
+ // |frame_length.header_length| bytes.
// Called when a PRIORITY frame has been received.
// |frame_length| contains PRIORITY frame length and payload length.
diff --git a/quic/core/http/quic_spdy_stream.cc b/quic/core/http/quic_spdy_stream.cc
index 97991e8..aee3ba5 100644
--- a/quic/core/http/quic_spdy_stream.cc
+++ b/quic/core/http/quic_spdy_stream.cc
@@ -769,7 +769,9 @@
return false;
}
- body_buffer_.OnDataHeader(frame_lengths);
+ sequencer()->MarkConsumed(
+ body_buffer_.OnDataHeader(frame_lengths.header_length));
+
return true;
}
@@ -777,6 +779,7 @@
DCHECK(VersionHasDataFrameHeader(transport_version()));
body_buffer_.OnDataPayload(payload);
+
return true;
}
diff --git a/quic/core/http/quic_spdy_stream_body_buffer.cc b/quic/core/http/quic_spdy_stream_body_buffer.cc
index c29c766..2ae72b5 100644
--- a/quic/core/http/quic_spdy_stream_body_buffer.cc
+++ b/quic/core/http/quic_spdy_stream_body_buffer.cc
@@ -11,89 +11,78 @@
namespace quic {
QuicSpdyStreamBodyBuffer::QuicSpdyStreamBodyBuffer()
- : bytes_remaining_(0),
- total_body_bytes_readable_(0),
- total_body_bytes_received_(0),
- total_payload_lengths_(0) {}
+ : total_body_bytes_received_(0) {}
-void QuicSpdyStreamBodyBuffer::OnDataHeader(Http3FrameLengths frame_lengths) {
- frame_meta_.push_back(frame_lengths);
- total_payload_lengths_ += frame_lengths.payload_length;
+size_t QuicSpdyStreamBodyBuffer::OnDataHeader(QuicByteCount length) {
+ DCHECK_NE(0u, length);
+
+ if (fragments_.empty()) {
+ // DATA frame header can be consumed immediately, because all previously
+ // received DATA frame payload bytes have been read.
+ return length;
+ }
+
+ // DATA frame header length will be consumed after last fragment is read.
+ fragments_.back().trailing_consumable_bytes += length;
+ return 0;
}
void QuicSpdyStreamBodyBuffer::OnDataPayload(QuicStringPiece payload) {
DCHECK(!payload.empty());
- bodies_.push_back(payload);
+
+ fragments_.push_back({payload, 0});
total_body_bytes_received_ += payload.length();
- total_body_bytes_readable_ += payload.length();
- DCHECK_LE(total_body_bytes_received_, total_payload_lengths_);
}
size_t QuicSpdyStreamBodyBuffer::OnBodyConsumed(size_t num_bytes) {
- // Check if the stream has enough decoded data.
- if (num_bytes > total_body_bytes_readable_) {
- QUIC_BUG << "Invalid argument to OnBodyConsumed."
- << " expect to consume: " << num_bytes
- << ", but not enough bytes available. "
- << "Total bytes readable are: " << total_body_bytes_readable_;
- return 0;
- }
- // Discard references in the stream before the sequencer marks them consumed.
- size_t remaining = num_bytes;
- while (remaining > 0) {
- if (bodies_.empty()) {
- QUIC_BUG << "Failed to consume because body buffer is empty.";
+ QuicByteCount bytes_to_consume = 0;
+ size_t remaining_bytes = num_bytes;
+
+ while (remaining_bytes > 0) {
+ if (fragments_.empty()) {
+ QUIC_BUG << "Not enough available body to consume.";
return 0;
}
- auto body = bodies_.front();
- bodies_.pop_front();
- if (body.length() <= remaining) {
- remaining -= body.length();
- } else {
- body = body.substr(remaining, body.length() - remaining);
- bodies_.push_front(body);
- remaining = 0;
- }
- }
- // Consume headers.
- size_t bytes_to_consume = 0;
- while (bytes_remaining_ < num_bytes) {
- if (frame_meta_.empty()) {
- QUIC_BUG << "Faild to consume because frame header buffer is empty.";
- return 0;
- }
- auto meta = frame_meta_.front();
- frame_meta_.pop_front();
- bytes_remaining_ += meta.payload_length;
- bytes_to_consume += meta.header_length;
- }
- bytes_to_consume += num_bytes;
+ Fragment& fragment = fragments_.front();
+ const QuicStringPiece body = fragment.body;
- // Update accountings.
- bytes_remaining_ -= num_bytes;
- total_body_bytes_readable_ -= num_bytes;
+ if (body.length() > remaining_bytes) {
+ // Consume leading |remaining_bytes| bytes of body.
+ bytes_to_consume += remaining_bytes;
+ fragment.body = body.substr(remaining_bytes);
+ return bytes_to_consume;
+ }
+
+ // Consume entire fragment and the following
+ // |trailing_consumable_bytes| bytes.
+ remaining_bytes -= body.length();
+ bytes_to_consume += body.length() + fragment.trailing_consumable_bytes;
+ fragments_.pop_front();
+ }
return bytes_to_consume;
}
int QuicSpdyStreamBodyBuffer::PeekBody(iovec* iov, size_t iov_len) const {
- DCHECK(iov != nullptr);
+ DCHECK(iov);
DCHECK_GT(iov_len, 0u);
- if (bodies_.empty()) {
+ // TODO(bnc): Is this really necessary?
+ if (fragments_.empty()) {
iov[0].iov_base = nullptr;
iov[0].iov_len = 0;
return 0;
}
- // Fill iovs with references from the stream.
+
size_t iov_filled = 0;
- while (iov_filled < bodies_.size() && iov_filled < iov_len) {
- QuicStringPiece body = bodies_[iov_filled];
+ while (iov_filled < fragments_.size() && iov_filled < iov_len) {
+ QuicStringPiece body = fragments_[iov_filled].body;
iov[iov_filled].iov_base = const_cast<char*>(body.data());
iov[iov_filled].iov_len = body.size();
iov_filled++;
}
+
return iov_filled;
}
@@ -101,32 +90,50 @@
size_t iov_len,
size_t* total_bytes_read) {
*total_bytes_read = 0;
- QuicByteCount total_remaining = total_body_bytes_readable_;
+ QuicByteCount bytes_to_consume = 0;
+
+ // The index of iovec to write to.
size_t index = 0;
- size_t src_offset = 0;
- for (size_t i = 0; i < iov_len && total_remaining > 0; ++i) {
- char* dest = reinterpret_cast<char*>(iov[i].iov_base);
- size_t dest_remaining = iov[i].iov_len;
- while (dest_remaining > 0 && total_remaining > 0) {
- auto body = bodies_[index];
- size_t bytes_to_copy =
- std::min<size_t>(body.length() - src_offset, dest_remaining);
- memcpy(dest, body.substr(src_offset, bytes_to_copy).data(),
- bytes_to_copy);
+ // Address to write to within current iovec.
+ char* dest = reinterpret_cast<char*>(iov[index].iov_base);
+ // Remaining space in current iovec.
+ size_t dest_remaining = iov[index].iov_len;
+
+ while (!fragments_.empty()) {
+ Fragment& fragment = fragments_.front();
+ const QuicStringPiece body = fragment.body;
+
+ const size_t bytes_to_copy =
+ std::min<size_t>(body.length(), dest_remaining);
+ memcpy(dest, body.data(), bytes_to_copy);
+ bytes_to_consume += bytes_to_copy;
+ *total_bytes_read += bytes_to_copy;
+
+ if (bytes_to_copy == body.length()) {
+ // Entire fragment read.
+ bytes_to_consume += fragment.trailing_consumable_bytes;
+ fragments_.pop_front();
+ } else {
+ // Consume leading |bytes_to_copy| bytes of body.
+ fragment.body = body.substr(bytes_to_copy);
+ }
+
+ if (bytes_to_copy == dest_remaining) {
+ // Current iovec full.
+ ++index;
+ if (index == iov_len) {
+ break;
+ }
+ dest = reinterpret_cast<char*>(iov[index].iov_base);
+ dest_remaining = iov[index].iov_len;
+ } else {
+ // Advance destination parameters within this iovec.
dest += bytes_to_copy;
dest_remaining -= bytes_to_copy;
- *total_bytes_read += bytes_to_copy;
- total_remaining -= bytes_to_copy;
- if (bytes_to_copy < body.length() - src_offset) {
- src_offset += bytes_to_copy;
- } else {
- index++;
- src_offset = 0;
- }
}
}
- return OnBodyConsumed(*total_bytes_read);
+ return bytes_to_consume;
}
} // namespace quic
diff --git a/quic/core/http/quic_spdy_stream_body_buffer.h b/quic/core/http/quic_spdy_stream_body_buffer.h
index a37f3bf..77c17b8 100644
--- a/quic/core/http/quic_spdy_stream_body_buffer.h
+++ b/quic/core/http/quic_spdy_stream_body_buffer.h
@@ -5,72 +5,81 @@
#ifndef QUICHE_QUIC_CORE_HTTP_QUIC_SPDY_STREAM_BODY_BUFFER_H_
#define QUICHE_QUIC_CORE_HTTP_QUIC_SPDY_STREAM_BODY_BUFFER_H_
-#include "net/third_party/quiche/src/quic/core/http/http_decoder.h"
+#include "net/third_party/quiche/src/quic/core/quic_constants.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_bug_tracker.h"
#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_iovec.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_macros.h"
+#include "net/third_party/quiche/src/quic/platform/api/quic_string_piece.h"
namespace quic {
-class QuicStreamSequencer;
-
-// Buffer decoded body for QuicSpdyStream. It also talks to the sequencer to
-// consume data.
+// "Body" means DATA frame payload.
+// QuicSpdyStreamBodyBuffer does two things: it keeps references to body
+// fragments (owned by QuicStreamSequencer) and offers methods to read them; and
+// calculates the total number of bytes (including DATA frame headers) the
+// caller needs to mark consumed (with QuicStreamSequencer) whenever DATA frame
+// headers are received or body bytes are read.
+// TODO(bnc): Rename to QuicSpdyStreamBodyManager or similar.
class QUIC_EXPORT_PRIVATE QuicSpdyStreamBodyBuffer {
public:
QuicSpdyStreamBodyBuffer();
~QuicSpdyStreamBodyBuffer() = default;
- // Add metadata of the frame to accountings.
- // Called when QuicSpdyStream receives data frame header.
- void OnDataHeader(Http3FrameLengths frame_lengths);
+ // Called when DATA frame header bytes are received. |length| must be
+ // positive. Returns number of bytes the caller shall mark consumed, which
+ // might be zero.
+ QUIC_MUST_USE_RESULT size_t OnDataHeader(QuicByteCount length);
- // Add new data payload to buffer.
- // Called when QuicSpdyStream received data payload.
- // Data pointed by payload must be alive until consumed by
- // QuicStreamSequencer::MarkConsumed().
+ // Called when DATA frame payload is received. |payload| is added to
+ // |fragments_|. The data pointed to by |payload| must be kept alive until an
+ // OnBodyConsumed() or ReadBody() call consumes it. |payload| must not be
+ // empty.
void OnDataPayload(QuicStringPiece payload);
- // Internally marks |num_bytes| of DATA frame payload consumed, and returns
- // the number of bytes that the caller should mark consumed with the
- // sequencer, including DATA frame header bytes, if any.
+ // Internally marks |num_bytes| of DATA frame payload consumed. |num_bytes|
+ // might be zero. Returns the number of bytes that the caller should mark
+ // consumed with the sequencer, which is the sum of |num_bytes| for payload
+ // and additional DATA frame header bytes, if any.
QUIC_MUST_USE_RESULT size_t OnBodyConsumed(size_t num_bytes);
- // Fill up to |iov_len| with bodies available in buffer. No data is consumed.
- // |iov|.iov_base will point to data in the buffer, and |iov|.iov_len will
- // be set to the underlying data length accordingly.
- // Returns the number of iov used.
+ // Set up to |iov_len| elements of iov[] to point to available bodies: each
+ // iov[i].iov_base will point to a body fragment, and iov[i].iov_len will be
+ // set to its length. No data is copied, no data is consumed. Returns the
+ // number of iov set.
int PeekBody(iovec* iov, size_t iov_len) const;
- // Copies from buffer into |iov| up to |iov_len|, and consume data in
- // sequencer. |iov.iov_base| and |iov.iov_len| are preassigned and will not be
- // changed. |*total_bytes_read| is set to the number of bytes read. Returns
- // the number of bytes that should be marked consumed with the sequencer.
+ // Copies data from available bodies into at most |iov_len| elements of iov[].
+ // Internally consumes copied payload bytes as well as all interleaving and
+ // immediately following DATA frame header bytes. |iov.iov_base| and
+ // |iov.iov_len| are preassigned and will not be changed. Returns the total
+ // number of bytes the caller shall mark consumed. Sets
+ // |*total_bytes_read| to the total number of DATA payload bytes read.
QUIC_MUST_USE_RESULT size_t ReadBody(const struct iovec* iov,
size_t iov_len,
size_t* total_bytes_read);
- bool HasBytesToRead() const { return !bodies_.empty(); }
+ bool HasBytesToRead() const { return !fragments_.empty(); }
uint64_t total_body_bytes_received() const {
return total_body_bytes_received_;
}
private:
- // Storage for decoded data.
- QuicDeque<QuicStringPiece> bodies_;
- // Storage for header lengths.
- QuicDeque<Http3FrameLengths> frame_meta_;
- // Bytes in the first available data frame that are not consumed yet.
- QuicByteCount bytes_remaining_;
- // Total available body data in the stream.
- QuicByteCount total_body_bytes_readable_;
- // Total bytes read from the stream excluding headers.
+ // A Fragment instance represents a body fragment with a count of bytes
+ // received afterwards but before the next body fragment that can be marked
+ // consumed as soon as all of the body fragment is read.
+ struct Fragment {
+ // |body| must not be empty.
+ QuicStringPiece body;
+ // Might be zero.
+ QuicByteCount trailing_consumable_bytes;
+ };
+ // Queue of DATA frame payload fragments and byte counts.
+ QuicDeque<Fragment> fragments_;
+ // Total body bytes received.
QuicByteCount total_body_bytes_received_;
- // Total length of payloads tracked by frame_meta_.
- QuicByteCount total_payload_lengths_;
};
} // namespace quic
diff --git a/quic/core/http/quic_spdy_stream_body_buffer_test.cc b/quic/core/http/quic_spdy_stream_body_buffer_test.cc
index d5e8161..7c4787d 100644
--- a/quic/core/http/quic_spdy_stream_body_buffer_test.cc
+++ b/quic/core/http/quic_spdy_stream_body_buffer_test.cc
@@ -4,8 +4,11 @@
#include "net/third_party/quiche/src/quic/core/http/quic_spdy_stream_body_buffer.h"
+#include <algorithm>
+#include <numeric>
#include <string>
+#include "net/third_party/quiche/src/quic/platform/api/quic_arraysize.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_expect_bug.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_logging.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_test.h"
@@ -21,139 +24,255 @@
QuicSpdyStreamBodyBuffer body_buffer_;
};
-TEST_F(QuicSpdyStreamBodyBufferTest, ReceiveBodies) {
- std::string body(1024, 'a');
+TEST_F(QuicSpdyStreamBodyBufferTest, HasBytesToRead) {
EXPECT_FALSE(body_buffer_.HasBytesToRead());
- body_buffer_.OnDataHeader(Http3FrameLengths(3, 1024));
- body_buffer_.OnDataPayload(body);
- EXPECT_EQ(1024u, body_buffer_.total_body_bytes_received());
- EXPECT_TRUE(body_buffer_.HasBytesToRead());
-}
+ EXPECT_EQ(0u, body_buffer_.total_body_bytes_received());
-TEST_F(QuicSpdyStreamBodyBufferTest, PeekBody) {
- std::string body(1024, 'a');
- body_buffer_.OnDataHeader(Http3FrameLengths(3, 1024));
- body_buffer_.OnDataPayload(body);
- EXPECT_EQ(1024u, body_buffer_.total_body_bytes_received());
- iovec vec;
- EXPECT_EQ(1, body_buffer_.PeekBody(&vec, 1));
- EXPECT_EQ(1024u, vec.iov_len);
- EXPECT_EQ(body,
- QuicStringPiece(static_cast<const char*>(vec.iov_base), 1024));
-}
-
-// Buffer only receives 1 frame. Stream consumes less or equal than a frame.
-TEST_F(QuicSpdyStreamBodyBufferTest, MarkConsumedPartialSingleFrame) {
- testing::InSequence seq;
- std::string body(1024, 'a');
const QuicByteCount header_length = 3;
- Http3FrameLengths lengths(header_length, 1024);
- body_buffer_.OnDataHeader(lengths);
- body_buffer_.OnDataPayload(body);
- EXPECT_EQ(header_length + 1024, body_buffer_.OnBodyConsumed(1024));
-}
+ EXPECT_EQ(header_length, body_buffer_.OnDataHeader(header_length));
-// Buffer received 2 frames. Stream consumes multiple times.
-TEST_F(QuicSpdyStreamBodyBufferTest, MarkConsumedMultipleFrames) {
- testing::InSequence seq;
- // 1st frame.
- std::string body1(1024, 'a');
- const QuicByteCount header_length1 = 2;
- Http3FrameLengths lengths1(header_length1, 1024);
- body_buffer_.OnDataHeader(lengths1);
- body_buffer_.OnDataPayload(body1);
+ EXPECT_FALSE(body_buffer_.HasBytesToRead());
+ EXPECT_EQ(0u, body_buffer_.total_body_bytes_received());
- // 2nd frame.
- std::string body2(2048, 'b');
- const QuicByteCount header_length2 = 4;
- Http3FrameLengths lengths2(header_length2, 2048);
- body_buffer_.OnDataHeader(lengths2);
- body_buffer_.OnDataPayload(body2);
-
- EXPECT_EQ(header_length1 + 512, body_buffer_.OnBodyConsumed(512));
- EXPECT_EQ(header_length2 + 2048, body_buffer_.OnBodyConsumed(2048));
- EXPECT_EQ(512u, body_buffer_.OnBodyConsumed(512));
-}
-
-TEST_F(QuicSpdyStreamBodyBufferTest, MarkConsumedMoreThanBuffered) {
std::string body(1024, 'a');
- Http3FrameLengths lengths(3, 1024);
- body_buffer_.OnDataHeader(lengths);
+ body_buffer_.OnDataPayload(body);
+
+ EXPECT_TRUE(body_buffer_.HasBytesToRead());
+ EXPECT_EQ(1024u, body_buffer_.total_body_bytes_received());
+}
+
+TEST_F(QuicSpdyStreamBodyBufferTest, ConsumeMoreThanAvailable) {
+ std::string body(1024, 'a');
body_buffer_.OnDataPayload(body);
size_t bytes_to_consume = 0;
- EXPECT_QUIC_BUG(
- bytes_to_consume = body_buffer_.OnBodyConsumed(2048),
- "Invalid argument to OnBodyConsumed. expect to consume: 2048, but not "
- "enough bytes available. Total bytes readable are: 1024");
+ EXPECT_QUIC_BUG(bytes_to_consume = body_buffer_.OnBodyConsumed(2048),
+ "Not enough available body to consume.");
EXPECT_EQ(0u, bytes_to_consume);
}
-// Buffer receives 1 frame. Stream read from the buffer.
-TEST_F(QuicSpdyStreamBodyBufferTest, ReadSingleBody) {
- testing::InSequence seq;
- std::string body(1024, 'a');
- const QuicByteCount header_length = 2;
- Http3FrameLengths lengths(header_length, 1024);
- body_buffer_.OnDataHeader(lengths);
- body_buffer_.OnDataPayload(body);
+struct {
+ std::vector<QuicByteCount> frame_header_lengths;
+ std::vector<const char*> frame_payloads;
+ std::vector<QuicByteCount> body_bytes_to_read;
+ std::vector<QuicByteCount> expected_return_values;
+} const kOnBodyConsumedTestData[] = {
+ // One frame consumed in one call.
+ {{2}, {"foobar"}, {6}, {6}},
+ // Two frames consumed in one call.
+ {{3, 5}, {"foobar", "baz"}, {9}, {14}},
+ // One frame consumed in two calls.
+ {{2}, {"foobar"}, {4, 2}, {4, 2}},
+ // Two frames consumed in two calls matching frame boundaries.
+ {{3, 5}, {"foobar", "baz"}, {6, 3}, {11, 3}},
+ // Two frames consumed in two calls,
+ // the first call only consuming part of the first frame.
+ {{3, 5}, {"foobar", "baz"}, {5, 4}, {5, 9}},
+ // Two frames consumed in two calls,
+ // the first call consuming the entire first frame and part of the second.
+ {{3, 5}, {"foobar", "baz"}, {7, 2}, {12, 2}},
+};
- char base[1024];
- iovec iov = {&base[0], 1024};
- size_t total_bytes_read = 0;
- EXPECT_EQ(header_length + 1024,
- body_buffer_.ReadBody(&iov, 1, &total_bytes_read));
- EXPECT_EQ(1024u, total_bytes_read);
- EXPECT_EQ(1024u, iov.iov_len);
- EXPECT_EQ(body,
- QuicStringPiece(static_cast<const char*>(iov.iov_base), 1024));
+TEST_F(QuicSpdyStreamBodyBufferTest, OnBodyConsumed) {
+ for (size_t test_case_index = 0;
+ test_case_index < QUIC_ARRAYSIZE(kOnBodyConsumedTestData);
+ ++test_case_index) {
+ const std::vector<QuicByteCount>& frame_header_lengths =
+ kOnBodyConsumedTestData[test_case_index].frame_header_lengths;
+ const std::vector<const char*>& frame_payloads =
+ kOnBodyConsumedTestData[test_case_index].frame_payloads;
+ const std::vector<QuicByteCount>& body_bytes_to_read =
+ kOnBodyConsumedTestData[test_case_index].body_bytes_to_read;
+ const std::vector<QuicByteCount>& expected_return_values =
+ kOnBodyConsumedTestData[test_case_index].expected_return_values;
+
+ for (size_t frame_index = 0; frame_index < frame_header_lengths.size();
+ ++frame_index) {
+ // Frame header of first frame can immediately be consumed, but not the
+ // other frames. Each test case start with an empty
+ // QuicSpdyStreamBodyBuffer.
+ EXPECT_EQ(frame_index == 0 ? frame_header_lengths[frame_index] : 0u,
+ body_buffer_.OnDataHeader(frame_header_lengths[frame_index]));
+ body_buffer_.OnDataPayload(frame_payloads[frame_index]);
+ }
+
+ for (size_t call_index = 0; call_index < body_bytes_to_read.size();
+ ++call_index) {
+ EXPECT_EQ(expected_return_values[call_index],
+ body_buffer_.OnBodyConsumed(body_bytes_to_read[call_index]));
+ }
+
+ EXPECT_FALSE(body_buffer_.HasBytesToRead());
+ }
}
-// Buffer receives 2 frames, stream read from the buffer multiple times.
-TEST_F(QuicSpdyStreamBodyBufferTest, ReadMultipleBody) {
- testing::InSequence seq;
- // 1st frame.
- std::string body1(1024, 'a');
- const QuicByteCount header_length1 = 2;
- Http3FrameLengths lengths1(header_length1, 1024);
- body_buffer_.OnDataHeader(lengths1);
- body_buffer_.OnDataPayload(body1);
+struct {
+ std::vector<QuicByteCount> frame_header_lengths;
+ std::vector<const char*> frame_payloads;
+ size_t iov_len;
+} const kPeekBodyTestData[] = {
+ // No frames, more iovecs than frames.
+ {{}, {}, 1},
+ // One frame, same number of iovecs.
+ {{3}, {"foobar"}, 1},
+ // One frame, more iovecs than frames.
+ {{3}, {"foobar"}, 2},
+ // Two frames, fewer iovecs than frames.
+ {{3, 5}, {"foobar", "baz"}, 1},
+ // Two frames, same number of iovecs.
+ {{3, 5}, {"foobar", "baz"}, 2},
+ // Two frames, more iovecs than frames.
+ {{3, 5}, {"foobar", "baz"}, 3},
+};
- // 2nd frame.
- std::string body2(2048, 'b');
- const QuicByteCount header_length2 = 4;
- Http3FrameLengths lengths2(header_length2, 2048);
- body_buffer_.OnDataHeader(lengths2);
- body_buffer_.OnDataPayload(body2);
+TEST_F(QuicSpdyStreamBodyBufferTest, PeekBody) {
+ for (size_t test_case_index = 0;
+ test_case_index < QUIC_ARRAYSIZE(kPeekBodyTestData); ++test_case_index) {
+ const std::vector<QuicByteCount>& frame_header_lengths =
+ kPeekBodyTestData[test_case_index].frame_header_lengths;
+ const std::vector<const char*>& frame_payloads =
+ kPeekBodyTestData[test_case_index].frame_payloads;
+ size_t iov_len = kPeekBodyTestData[test_case_index].iov_len;
- // First read of 512 bytes.
- char base[512];
- iovec iov = {&base[0], 512};
- size_t total_bytes_read = 0;
- EXPECT_EQ(header_length1 + 512,
- body_buffer_.ReadBody(&iov, 1, &total_bytes_read));
- EXPECT_EQ(512u, total_bytes_read);
- EXPECT_EQ(512u, iov.iov_len);
- EXPECT_EQ(body1.substr(0, 512),
- QuicStringPiece(static_cast<const char*>(iov.iov_base), 512));
+ QuicSpdyStreamBodyBuffer body_buffer;
- // Second read of 2048 bytes.
- char base2[2048];
- iovec iov2 = {&base2[0], 2048};
- EXPECT_EQ(header_length2 + 2048,
- body_buffer_.ReadBody(&iov2, 1, &total_bytes_read));
- EXPECT_EQ(2048u, total_bytes_read);
- EXPECT_EQ(2048u, iov2.iov_len);
- EXPECT_EQ(body1.substr(512, 512) + body2.substr(0, 1536),
- QuicStringPiece(static_cast<const char*>(iov2.iov_base), 2048));
+ for (size_t frame_index = 0; frame_index < frame_header_lengths.size();
+ ++frame_index) {
+ // Frame header of first frame can immediately be consumed, but not the
+ // other frames. Each test case uses a new QuicSpdyStreamBodyBuffer
+ // instance.
+ EXPECT_EQ(frame_index == 0 ? frame_header_lengths[frame_index] : 0u,
+ body_buffer.OnDataHeader(frame_header_lengths[frame_index]));
+ body_buffer.OnDataPayload(frame_payloads[frame_index]);
+ }
- // Third read of the rest 512 bytes.
- char base3[512];
- iovec iov3 = {&base3[0], 512};
- EXPECT_EQ(512u, body_buffer_.ReadBody(&iov3, 1, &total_bytes_read));
- EXPECT_EQ(512u, total_bytes_read);
- EXPECT_EQ(512u, iov3.iov_len);
- EXPECT_EQ(body2.substr(1536, 512),
- QuicStringPiece(static_cast<const char*>(iov3.iov_base), 512));
+ std::vector<iovec> iovecs;
+ iovecs.resize(iov_len);
+ size_t iovs_filled = std::min(frame_payloads.size(), iov_len);
+ ASSERT_EQ(iovs_filled,
+ static_cast<size_t>(body_buffer.PeekBody(&iovecs[0], iov_len)));
+ for (size_t iovec_index = 0; iovec_index < iovs_filled; ++iovec_index) {
+ EXPECT_EQ(frame_payloads[iovec_index],
+ QuicStringPiece(
+ static_cast<const char*>(iovecs[iovec_index].iov_base),
+ iovecs[iovec_index].iov_len));
+ }
+ }
+}
+
+struct {
+ std::vector<QuicByteCount> frame_header_lengths;
+ std::vector<const char*> frame_payloads;
+ std::vector<std::vector<QuicByteCount>> iov_lengths;
+ std::vector<QuicByteCount> expected_total_bytes_read;
+ std::vector<QuicByteCount> expected_return_values;
+} const kReadBodyTestData[] = {
+ // One frame, one read with smaller iovec.
+ {{4}, {"foo"}, {{2}}, {2}, {2}},
+ // One frame, one read with same size iovec.
+ {{4}, {"foo"}, {{3}}, {3}, {3}},
+ // One frame, one read with larger iovec.
+ {{4}, {"foo"}, {{5}}, {3}, {3}},
+ // One frame, one read with two iovecs, smaller total size.
+ {{4}, {"foobar"}, {{2, 3}}, {5}, {5}},
+ // One frame, one read with two iovecs, same total size.
+ {{4}, {"foobar"}, {{2, 4}}, {6}, {6}},
+ // One frame, one read with two iovecs, larger total size in last iovec.
+ {{4}, {"foobar"}, {{2, 6}}, {6}, {6}},
+ // One frame, one read with extra iovecs, body ends at iovec boundary.
+ {{4}, {"foobar"}, {{2, 4, 4, 3}}, {6}, {6}},
+ // One frame, one read with extra iovecs, body ends not at iovec boundary.
+ {{4}, {"foobar"}, {{2, 7, 4, 3}}, {6}, {6}},
+ // One frame, two reads with two iovecs each, smaller total size.
+ {{4}, {"foobarbaz"}, {{2, 1}, {3, 2}}, {3, 5}, {3, 5}},
+ // One frame, two reads with two iovecs each, same total size.
+ {{4}, {"foobarbaz"}, {{2, 1}, {4, 2}}, {3, 6}, {3, 6}},
+ // One frame, two reads with two iovecs each, larger total size.
+ {{4}, {"foobarbaz"}, {{2, 1}, {4, 10}}, {3, 6}, {3, 6}},
+ // Two frames, one read with smaller iovec.
+ {{4, 3}, {"foobar", "baz"}, {{8}}, {8}, {11}},
+ // Two frames, one read with same size iovec.
+ {{4, 3}, {"foobar", "baz"}, {{9}}, {9}, {12}},
+ // Two frames, one read with larger iovec.
+ {{4, 3}, {"foobar", "baz"}, {{10}}, {9}, {12}},
+ // Two frames, one read with two iovecs, smaller total size.
+ {{4, 3}, {"foobar", "baz"}, {{4, 3}}, {7}, {10}},
+ // Two frames, one read with two iovecs, same total size.
+ {{4, 3}, {"foobar", "baz"}, {{4, 5}}, {9}, {12}},
+ // Two frames, one read with two iovecs, larger total size in last iovec.
+ {{4, 3}, {"foobar", "baz"}, {{4, 6}}, {9}, {12}},
+ // Two frames, one read with extra iovecs, body ends at iovec boundary.
+ {{4, 3}, {"foobar", "baz"}, {{4, 6, 4, 3}}, {9}, {12}},
+ // Two frames, one read with extra iovecs, body ends not at iovec boundary.
+ {{4, 3}, {"foobar", "baz"}, {{4, 7, 4, 3}}, {9}, {12}},
+ // Two frames, two reads with two iovecs each, reads end on frame boundary.
+ {{4, 3}, {"foobar", "baz"}, {{2, 4}, {2, 1}}, {6, 3}, {9, 3}},
+ // Three frames, three reads, extra iovecs, no iovec ends on frame boundary.
+ {{4, 3, 6},
+ {"foobar", "bazquux", "qux"},
+ {{4, 3}, {2, 3}, {5, 3}},
+ {7, 5, 4},
+ {10, 5, 10}},
+};
+
+TEST_F(QuicSpdyStreamBodyBufferTest, ReadBody) {
+ for (size_t test_case_index = 0;
+ test_case_index < QUIC_ARRAYSIZE(kReadBodyTestData); ++test_case_index) {
+ const std::vector<QuicByteCount>& frame_header_lengths =
+ kReadBodyTestData[test_case_index].frame_header_lengths;
+ const std::vector<const char*>& frame_payloads =
+ kReadBodyTestData[test_case_index].frame_payloads;
+ const std::vector<std::vector<QuicByteCount>>& iov_lengths =
+ kReadBodyTestData[test_case_index].iov_lengths;
+ const std::vector<QuicByteCount>& expected_total_bytes_read =
+ kReadBodyTestData[test_case_index].expected_total_bytes_read;
+ const std::vector<QuicByteCount>& expected_return_values =
+ kReadBodyTestData[test_case_index].expected_return_values;
+
+ QuicSpdyStreamBodyBuffer body_buffer;
+
+ std::string received_body;
+
+ for (size_t frame_index = 0; frame_index < frame_header_lengths.size();
+ ++frame_index) {
+ // Frame header of first frame can immediately be consumed, but not the
+ // other frames. Each test case uses a new QuicSpdyStreamBodyBuffer
+ // instance.
+ EXPECT_EQ(frame_index == 0 ? frame_header_lengths[frame_index] : 0u,
+ body_buffer.OnDataHeader(frame_header_lengths[frame_index]));
+ body_buffer.OnDataPayload(frame_payloads[frame_index]);
+ received_body.append(frame_payloads[frame_index]);
+ }
+
+ std::string read_body;
+
+ for (size_t call_index = 0; call_index < iov_lengths.size(); ++call_index) {
+ // Allocate single buffer for iovecs.
+ size_t total_iov_length = std::accumulate(iov_lengths[call_index].begin(),
+ iov_lengths[call_index].end(),
+ static_cast<size_t>(0));
+ std::string buffer(total_iov_length, 'z');
+
+ // Construct iovecs pointing to contiguous areas in the buffer.
+ std::vector<iovec> iovecs;
+ size_t offset = 0;
+ for (size_t iov_length : iov_lengths[call_index]) {
+ CHECK(offset + iov_length <= buffer.size());
+ iovecs.push_back({&buffer[offset], iov_length});
+ offset += iov_length;
+ }
+
+ // Make sure |total_bytes_read| differs from |expected_total_bytes_read|.
+ size_t total_bytes_read = expected_total_bytes_read[call_index] + 12;
+ EXPECT_EQ(
+ expected_return_values[call_index],
+ body_buffer.ReadBody(&iovecs[0], iovecs.size(), &total_bytes_read));
+ read_body.append(buffer.substr(0, total_bytes_read));
+ }
+
+ EXPECT_EQ(received_body.substr(0, read_body.size()), read_body);
+ EXPECT_EQ(read_body.size() < received_body.size(),
+ body_buffer.HasBytesToRead());
+ }
}
} // anonymous namespace
diff --git a/quic/core/http/quic_spdy_stream_test.cc b/quic/core/http/quic_spdy_stream_test.cc
index f9777ec..ddfb651 100644
--- a/quic/core/http/quic_spdy_stream_test.cc
+++ b/quic/core/http/quic_spdy_stream_test.cc
@@ -2104,16 +2104,18 @@
// DATA frame.
QuicStringPiece data_payload(kDataFramePayload);
std::string data_frame = DataFrame(data_payload);
+ QuicByteCount data_frame_header_length =
+ data_frame.size() - data_payload.size();
- // DATA frame is not consumed because payload has to be buffered.
- // TODO(bnc): Consume frame header as soon as possible.
+ // DATA frame header is consumed.
+ // DATA frame payload is not consumed because payload has to be buffered.
OnStreamFrame(data_frame);
- EXPECT_EQ(0u, NewlyConsumedBytes());
+ EXPECT_EQ(data_frame_header_length, NewlyConsumedBytes());
// Consume all but last byte of data.
EXPECT_EQ(data_payload.substr(0, data_payload.size() - 1),
ReadFromStream(data_payload.size() - 1));
- EXPECT_EQ(data_frame.size() - 1, NewlyConsumedBytes());
+ EXPECT_EQ(data_payload.size() - 1, NewlyConsumedBytes());
// Trailing HEADERS frame with QPACK encoded
// single header field "custom-key: custom-value".
