gfe-relnote: Allow sending CRYPTO frames without full padding. Protected by QUIC_VERSION_99 PiperOrigin-RevId: 247997458 Change-Id: I5439247d841c3298a77f7c914e527186f83e3f72
diff --git a/quic/core/quic_dispatcher.cc b/quic/core/quic_dispatcher.cc index fa6fda7..ea2450d 100644 --- a/quic/core/quic_dispatcher.cc +++ b/quic/core/quic_dispatcher.cc
@@ -178,9 +178,9 @@ } offset += frame.stream_frame.data_length; } else { - if (!creator_.ConsumeCryptoData(ENCRYPTION_INITIAL, - reject.length() - offset, offset, - NOT_RETRANSMISSION, &frame)) { + if (!creator_.ConsumeCryptoData( + ENCRYPTION_INITIAL, reject.length() - offset, offset, + /*needs_full_padding=*/true, NOT_RETRANSMISSION, &frame)) { QUIC_BUG << "Unable to consume crypto data into an empty packet."; return; }
diff --git a/quic/core/quic_packet_creator.cc b/quic/core/quic_packet_creator.cc index 8199307..dddc8e7 100644 --- a/quic/core/quic_packet_creator.cc +++ b/quic/core/quic_packet_creator.cc
@@ -161,6 +161,7 @@ bool QuicPacketCreator::ConsumeCryptoData(EncryptionLevel level, size_t write_length, QuicStreamOffset offset, + bool needs_full_padding, TransmissionType transmission_type, QuicFrame* frame) { if (!CreateCryptoFrame(level, write_length, offset, frame)) { @@ -172,7 +173,9 @@ // // TODO(nharper): Check what the IETF drafts say about padding out initial // messages and change this as appropriate. - needs_full_padding_ = true; + if (needs_full_padding) { + needs_full_padding_ = true; + } return AddFrame(*frame, /*save_retransmittable_frames*/ true, transmission_type); }
diff --git a/quic/core/quic_packet_creator.h b/quic/core/quic_packet_creator.h index 367ca11..bceeae9 100644 --- a/quic/core/quic_packet_creator.h +++ b/quic/core/quic_packet_creator.h
@@ -112,6 +112,7 @@ bool ConsumeCryptoData(EncryptionLevel level, size_t write_length, QuicStreamOffset offset, + bool needs_full_padding, TransmissionType transmission_type, QuicFrame* frame);
diff --git a/quic/core/quic_packet_creator_test.cc b/quic/core/quic_packet_creator_test.cc index bd212de..637960f 100644 --- a/quic/core/quic_packet_creator_test.cc +++ b/quic/core/quic_packet_creator_test.cc
@@ -601,6 +601,7 @@ std::string data = "crypto data"; QuicFrame frame; ASSERT_TRUE(creator_.ConsumeCryptoData(ENCRYPTION_INITIAL, data.length(), 0, + /*needs_full_padding=*/true, NOT_RETRANSMISSION, &frame)); EXPECT_EQ(frame.crypto_frame->data_length, data.length()); EXPECT_TRUE(creator_.HasPendingFrames()); @@ -745,9 +746,9 @@ EXPECT_LT(0u, bytes_consumed); } else { producer_.SaveCryptoData(ENCRYPTION_INITIAL, kOffset, data); - ASSERT_TRUE(creator_.ConsumeCryptoData(ENCRYPTION_INITIAL, data.length(), - kOffset, NOT_RETRANSMISSION, - &frame)); + ASSERT_TRUE(creator_.ConsumeCryptoData( + ENCRYPTION_INITIAL, data.length(), kOffset, + /*needs_full_padding=*/true, NOT_RETRANSMISSION, &frame)); size_t bytes_consumed = frame.crypto_frame->data_length; EXPECT_LT(0u, bytes_consumed); }
diff --git a/quic/core/quic_packet_generator.cc b/quic/core/quic_packet_generator.cc index b90880e..ee15df0 100644 --- a/quic/core/quic_packet_generator.cc +++ b/quic/core/quic_packet_generator.cc
@@ -111,7 +111,8 @@ QuicFrame frame; if (!packet_creator_.ConsumeCryptoData( level, write_length - total_bytes_consumed, - offset + total_bytes_consumed, next_transmission_type_, &frame)) { + offset + total_bytes_consumed, fully_pad_crypto_handshake_packets_, + next_transmission_type_, &frame)) { // The only pending data in the packet is non-retransmittable frames. I'm // assuming here that they won't occupy so much of the packet that a // CRYPTO frame won't fit.
diff --git a/quic/core/quic_packet_generator_test.cc b/quic/core/quic_packet_generator_test.cc index 43c3a7f..1d66439 100644 --- a/quic/core/quic_packet_generator_test.cc +++ b/quic/core/quic_packet_generator_test.cc
@@ -614,16 +614,29 @@ EXPECT_CALL(delegate_, OnSerializedPacket(_)) .WillOnce(Invoke(this, &QuicPacketGeneratorTest::SavePacket)); - MakeIOVector("foo", &iov_); - QuicConsumedData consumed = generator_.ConsumeData( - QuicUtils::GetCryptoStreamId(framer_.transport_version()), &iov_, 1u, - iov_.iov_len, 0, NO_FIN); - EXPECT_EQ(3u, consumed.bytes_consumed); + std::string data = "foo"; + MakeIOVector(data, &iov_); + size_t bytes_consumed = 0; + if (QuicVersionUsesCryptoFrames(framer_.transport_version())) { + bytes_consumed = generator_.ConsumeCryptoData(ENCRYPTION_INITIAL, data, 0); + } else { + bytes_consumed = + generator_ + .ConsumeData( + QuicUtils::GetCryptoStreamId(framer_.transport_version()), + &iov_, 1u, iov_.iov_len, 0, NO_FIN) + .bytes_consumed; + } + EXPECT_EQ(3u, bytes_consumed); EXPECT_FALSE(generator_.HasQueuedFrames()); EXPECT_FALSE(generator_.HasRetransmittableFrames()); PacketContents contents; - contents.num_stream_frames = 1; + if (QuicVersionUsesCryptoFrames(framer_.transport_version())) { + contents.num_crypto_frames = 1; + } else { + contents.num_stream_frames = 1; + } contents.num_padding_frames = 0; CheckPacketContains(contents, 0); @@ -631,7 +644,13 @@ // Packet is not fully padded, but we want to future packets to be larger. ASSERT_EQ(kDefaultMaxPacketSize, generator_.GetCurrentMaxPacketLength()); - EXPECT_EQ(27, packets_[0].encrypted_length); + size_t expected_packet_length = 27; + if (QuicVersionUsesCryptoFrames(framer_.transport_version())) { + // The framing of CRYPTO frames is slightly different than that of stream + // frames, so the expected packet length differs slightly. + expected_packet_length = 28; + } + EXPECT_EQ(expected_packet_length, packets_[0].encrypted_length); } TEST_F(QuicPacketGeneratorTest, ConsumeData_EmptyData) {