Make sure QPACK stream types are sent along with SETTINGS in one packet.
gfe-relnote: v99 only, not protected.
Merge intruction: I wrote https://chromium-review.googlesource.com/c/chromium/src/+/1764346 to modify TestPakcetMaker so that v99 tests can be run again.
PiperOrigin-RevId: 264867694
Change-Id: I4d2453abb2f608d39f705c268ff7da265c69e22f
diff --git a/quic/core/http/quic_send_control_stream.cc b/quic/core/http/quic_send_control_stream.cc
index ab02ae9..f395912 100644
--- a/quic/core/http/quic_send_control_stream.cc
+++ b/quic/core/http/quic_send_control_stream.cc
@@ -26,7 +26,7 @@
ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
}
-void QuicSendControlStream::SendSettingsFrame() {
+void QuicSendControlStream::MaybeSendSettingsFrame() {
if (settings_sent_) {
return;
}
@@ -56,9 +56,7 @@
void QuicSendControlStream::WritePriority(const PriorityFrame& priority) {
QuicConnection::ScopedPacketFlusher flusher(session()->connection());
- if (!settings_sent_) {
- SendSettingsFrame();
- }
+ MaybeSendSettingsFrame();
std::unique_ptr<char[]> buffer;
QuicByteCount frame_length =
encoder_.SerializePriorityFrame(priority, &buffer);
diff --git a/quic/core/http/quic_send_control_stream.h b/quic/core/http/quic_send_control_stream.h
index b38ae95..e18fd27 100644
--- a/quic/core/http/quic_send_control_stream.h
+++ b/quic/core/http/quic_send_control_stream.h
@@ -30,9 +30,9 @@
// closed before connection.
void OnStreamReset(const QuicRstStreamFrame& frame) override;
- // Consult the Spdy session to construct Settings frame and sends it on this
- // stream. Settings frame must be the first frame sent on this stream.
- void SendSettingsFrame();
+ // Send SETTINGS frame if it hasn't been sent yet. Settings frame must be the
+ // first frame sent on this stream.
+ void MaybeSendSettingsFrame();
// Construct a MAX_PUSH_ID frame and send it on this stream.
void SendMaxPushIdFrame(PushId max_push_id);
diff --git a/quic/core/http/quic_send_control_stream_test.cc b/quic/core/http/quic_send_control_stream_test.cc
index 722ac86..fc64574 100644
--- a/quic/core/http/quic_send_control_stream_test.cc
+++ b/quic/core/http/quic_send_control_stream_test.cc
@@ -83,10 +83,10 @@
EXPECT_CALL(session_, WritevData(_, _, 1, _, _));
EXPECT_CALL(session_, WritevData(_, _, _, _, _));
- send_control_stream_->SendSettingsFrame();
+ send_control_stream_->MaybeSendSettingsFrame();
// No data should be written the sencond time SendSettingsFrame() is called.
- send_control_stream_->SendSettingsFrame();
+ send_control_stream_->MaybeSendSettingsFrame();
}
TEST_P(QuicSendControlStreamTest, WritePriorityBeforeSettings) {
diff --git a/quic/core/http/quic_spdy_session.cc b/quic/core/http/quic_spdy_session.cc
index c869459..dd9b4f4 100644
--- a/quic/core/http/quic_spdy_session.cc
+++ b/quic/core/http/quic_spdy_session.cc
@@ -512,6 +512,8 @@
DCHECK(perspective() == Perspective::IS_CLIENT)
<< "Server must not send priority";
+ QuicConnection::ScopedPacketFlusher flusher(connection());
+ SendMaxHeaderListSize(max_inbound_header_list_size_);
send_control_stream_->WritePriority(priority);
}
@@ -555,11 +557,12 @@
void QuicSpdySession::SendMaxHeaderListSize(size_t value) {
if (VersionHasStreamType(connection()->transport_version())) {
- send_control_stream_->SendSettingsFrame();
+ QuicConnection::ScopedPacketFlusher flusher(connection());
+ send_control_stream_->MaybeSendSettingsFrame();
// TODO(renjietang): Remove this once stream id manager can take dynamically
// created HTTP/3 unidirectional streams.
- qpack_encoder_send_stream_->SendStreamType();
- qpack_decoder_send_stream_->SendStreamType();
+ qpack_encoder_send_stream_->MaybeSendStreamType();
+ qpack_decoder_send_stream_->MaybeSendStreamType();
return;
}
SpdySettingsIR settings_frame;
diff --git a/quic/core/http/quic_spdy_stream_test.cc b/quic/core/http/quic_spdy_stream_test.cc
index 8b1c922..3798461 100644
--- a/quic/core/http/quic_spdy_stream_test.cc
+++ b/quic/core/http/quic_spdy_stream_test.cc
@@ -1247,6 +1247,14 @@
EXPECT_CALL(*session_, WritevData(send_control_stream,
send_control_stream->id(), _, _, _))
.Times(3);
+ auto qpack_encoder_stream =
+ QuicSpdySessionPeer::GetQpackEncoderSendStream(session_.get());
+ EXPECT_CALL(*session_, WritevData(qpack_encoder_stream,
+ qpack_encoder_stream->id(), 1, 0, _));
+ auto qpack_decoder_stream =
+ QuicSpdySessionPeer::GetQpackDecoderSendStream(session_.get());
+ EXPECT_CALL(*session_, WritevData(qpack_decoder_stream,
+ qpack_decoder_stream->id(), 1, 0, _));
}
// Write the initial headers, without a FIN.
diff --git a/quic/core/qpack/qpack_send_stream.cc b/quic/core/qpack/qpack_send_stream.cc
index b978985..50fe687 100644
--- a/quic/core/qpack/qpack_send_stream.cc
+++ b/quic/core/qpack/qpack_send_stream.cc
@@ -25,11 +25,11 @@
void QpackSendStream::WriteStreamData(QuicStringPiece data) {
QuicConnection::ScopedPacketFlusher flusher(session()->connection());
- SendStreamType();
+ MaybeSendStreamType();
WriteOrBufferData(data, false, nullptr);
}
-void QpackSendStream::SendStreamType() {
+void QpackSendStream::MaybeSendStreamType() {
if (!stream_type_sent_) {
char type[sizeof(http3_stream_type_)];
QuicDataWriter writer(QUIC_ARRAYSIZE(type), type);
diff --git a/quic/core/qpack/qpack_send_stream.h b/quic/core/qpack/qpack_send_stream.h
index e7e4be3..09c6020 100644
--- a/quic/core/qpack/qpack_send_stream.h
+++ b/quic/core/qpack/qpack_send_stream.h
@@ -44,7 +44,7 @@
// TODO(b/112770235): Remove this method once QuicStreamIdManager supports
// creating HTTP/3 unidirectional streams dynamically.
- void SendStreamType();
+ void MaybeSendStreamType();
private:
const uint64_t http3_stream_type_;
diff --git a/quic/core/qpack/qpack_send_stream_test.cc b/quic/core/qpack/qpack_send_stream_test.cc
index 5f82024..c297b56 100644
--- a/quic/core/qpack/qpack_send_stream_test.cc
+++ b/quic/core/qpack/qpack_send_stream_test.cc
@@ -96,7 +96,7 @@
EXPECT_CALL(session_, WritevData(_, _, data.length(), _, _));
qpack_send_stream_->WriteStreamData(QuicStringPiece(data));
EXPECT_CALL(session_, WritevData(_, _, _, _, _)).Times(0);
- qpack_send_stream_->SendStreamType();
+ qpack_send_stream_->MaybeSendStreamType();
}
TEST_P(QpackSendStreamTest, ResetQpackStream) {
