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) {