Modify write_index_ advancement to incorporate coalescing stream frames.
gfe-relnote: protected by gfe2_restart_flag_quic_coalesce_stream_frames_2
PiperOrigin-RevId: 279803164
Change-Id: Idc809c831725b92401b2fd5864379398ddec1bf0
diff --git a/quic/core/quic_packet_creator.cc b/quic/core/quic_packet_creator.cc
index f07394d..8c17421 100644
--- a/quic/core/quic_packet_creator.cc
+++ b/quic/core/quic_packet_creator.cc
@@ -1475,10 +1475,10 @@
return false;
}
- if (GetQuicReloadableFlag(quic_coalesce_stream_frames) &&
+ if (GetQuicRestartFlag(quic_coalesce_stream_frames_2) &&
frame.type == STREAM_FRAME &&
MaybeCoalesceStreamFrame(frame.stream_frame)) {
- QUIC_RELOADABLE_FLAG_COUNT(quic_coalesce_stream_frames);
+ QUIC_RESTART_FLAG_COUNT_N(quic_coalesce_stream_frames_2, 1, 3);
return true;
}
diff --git a/quic/core/quic_packet_creator_test.cc b/quic/core/quic_packet_creator_test.cc
index 1b7deda..da6b5ea 100644
--- a/quic/core/quic_packet_creator_test.cc
+++ b/quic/core/quic_packet_creator_test.cc
@@ -1999,7 +1999,7 @@
if (!GetParam().version_serialization) {
creator_.StopSendingVersion();
}
- SetQuicReloadableFlag(quic_coalesce_stream_frames, true);
+ SetQuicRestartFlag(quic_coalesce_stream_frames_2, true);
const size_t max_plaintext_size =
client_framer_.GetMaxPlaintextSize(creator_.max_packet_length());
EXPECT_FALSE(creator_.HasPendingFrames());
diff --git a/quic/core/quic_stream_send_buffer.cc b/quic/core/quic_stream_send_buffer.cc
index ead192d..158afd8 100644
--- a/quic/core/quic_stream_send_buffer.cc
+++ b/quic/core/quic_stream_send_buffer.cc
@@ -9,6 +9,7 @@
#include "net/third_party/quiche/src/quic/core/quic_stream_send_buffer.h"
#include "net/third_party/quiche/src/quic/core/quic_utils.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_flag_utils.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_flags.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_logging.h"
@@ -95,6 +96,8 @@
bool QuicStreamSendBuffer::WriteStreamData(QuicStreamOffset offset,
QuicByteCount data_length,
QuicDataWriter* writer) {
+ // TODO(renjietang): Remove this variable once quic_coalesce_stream_frames_2
+ // is deprecated.
bool write_index_hit = false;
QuicDeque<BufferedSlice>::iterator slice_it =
write_index_ == -1
@@ -134,15 +137,33 @@
offset += copy_length;
data_length -= copy_length;
- if (write_index_hit && copy_length == available_bytes_in_slice) {
+ if (GetQuicRestartFlag(quic_coalesce_stream_frames_2)) {
+ QUIC_RESTART_FLAG_COUNT_N(quic_coalesce_stream_frames_2, 2, 3);
+ if (write_index_ != -1) {
+ QuicDeque<BufferedSlice>::const_iterator index_slice =
+ buffered_slices_.begin() + write_index_;
+ if (index_slice->offset == slice_it->offset &&
+ copy_length == available_bytes_in_slice) {
+ // The slice pointed by write_index has been fully written, advance
+ // write index.
+ ++write_index_;
+ }
+ }
+ } else if (write_index_hit && copy_length == available_bytes_in_slice) {
// Finished writing all data in current slice, advance write index for
// next write.
++write_index_;
}
}
- if (write_index_hit &&
- static_cast<size_t>(write_index_) == buffered_slices_.size()) {
+ if (GetQuicRestartFlag(quic_coalesce_stream_frames_2)) {
+ QUIC_RESTART_FLAG_COUNT_N(quic_coalesce_stream_frames_2, 3, 3);
+ if (write_index_ != -1 &&
+ static_cast<size_t>(write_index_) == buffered_slices_.size()) {
+ write_index_ = -1;
+ }
+ } else if (write_index_hit &&
+ static_cast<size_t>(write_index_) == buffered_slices_.size()) {
// Already write to the end off buffer.
QUIC_DVLOG(2) << "Finish writing out all buffered data.";
write_index_ = -1;
diff --git a/quic/core/quic_stream_send_buffer_test.cc b/quic/core/quic_stream_send_buffer_test.cc
index 3a5efb5..a6a7ad2 100644
--- a/quic/core/quic_stream_send_buffer_test.cc
+++ b/quic/core/quic_stream_send_buffer_test.cc
@@ -115,6 +115,41 @@
EXPECT_EQ(3840u, send_buffer_.stream_bytes_outstanding());
}
+// Regression test for b/143491027.
+TEST_F(QuicStreamSendBufferTest,
+ WriteStreamDataContainsBothRetransmissionAndNewData) {
+ std::string copy1(1024, 'a');
+ std::string copy2 =
+ std::string(512, 'a') + std::string(256, 'b') + std::string(256, 'c');
+ std::string copy3 = std::string(1024, 'c') + std::string(100, 'd');
+ char buf[6000];
+ QuicDataWriter writer(6000, buf, HOST_BYTE_ORDER);
+ // Write more than one slice.
+ EXPECT_EQ(0, QuicStreamSendBufferPeer::write_index(&send_buffer_));
+ ASSERT_TRUE(send_buffer_.WriteStreamData(0, 1024, &writer));
+ EXPECT_EQ(copy1, QuicStringPiece(buf, 1024));
+ EXPECT_EQ(1, QuicStreamSendBufferPeer::write_index(&send_buffer_));
+
+ // Retransmit the first frame and also send new data.
+ ASSERT_TRUE(send_buffer_.WriteStreamData(0, 2048, &writer));
+ EXPECT_EQ(copy1 + copy2, QuicStringPiece(buf + 1024, 2048));
+
+ // Write new data.
+ if (!GetQuicRestartFlag(quic_coalesce_stream_frames_2)) {
+ EXPECT_EQ(1, QuicStreamSendBufferPeer::write_index(&send_buffer_));
+ EXPECT_DEBUG_DEATH(send_buffer_.WriteStreamData(2048, 50, &writer),
+ "Tried to write data out of sequence.");
+ } else {
+ EXPECT_EQ(2, QuicStreamSendBufferPeer::write_index(&send_buffer_));
+ ASSERT_TRUE(send_buffer_.WriteStreamData(2048, 50, &writer));
+ EXPECT_EQ(std::string(50, 'c'), QuicStringPiece(buf + 1024 + 2048, 50));
+ EXPECT_EQ(2, QuicStreamSendBufferPeer::write_index(&send_buffer_));
+ ASSERT_TRUE(send_buffer_.WriteStreamData(2048, 1124, &writer));
+ EXPECT_EQ(copy3, QuicStringPiece(buf + 1024 + 2048 + 50, 1124));
+ EXPECT_EQ(3, QuicStreamSendBufferPeer::write_index(&send_buffer_));
+ }
+}
+
TEST_F(QuicStreamSendBufferTest, RemoveStreamFrame) {
WriteAllData();
