Let quic::serializedpacket to own the frames and optionally the encrypted_buffer. no behavior change.
This change removes quic::ClearSerializedPacket() and quic::OwningSerializedPacketPointer. Their functionality have be moved to the destructor of quic::SerializedPacket. To simplify ownership, I changed quic::SerializedPacket to move-only and changed many functions in QuicPacketCreator and QuicConnection to take a SerializedPacket object instead of a pointer.
The optional ownership of encrypted_buffer is expressed using the newly added SerializedPacket.release_encrypted_buffer function. Currently only connectivity probing packets are setting it. In the next flag-protected change, I'll use it to free writer-allocated buffers.
PiperOrigin-RevId: 311381784
Change-Id: Icea678c488c4f2af1397ce82ecdf715b3d9f5407
diff --git a/quic/core/quic_coalesced_packet.cc b/quic/core/quic_coalesced_packet.cc
index 6393f5d..c18369e 100644
--- a/quic/core/quic_coalesced_packet.cc
+++ b/quic/core/quic_coalesced_packet.cc
@@ -90,9 +90,6 @@
for (auto& packet : encrypted_buffers_) {
packet.clear();
}
- if (initial_packet_ != nullptr) {
- ClearSerializedPacket(initial_packet_.get());
- }
initial_packet_ = nullptr;
}
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index 61119ad..1832092 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -2299,7 +2299,7 @@
// Termination packets are encrypted and saved, so don't exit early.
const bool is_termination_packet = IsTerminationPacket(*packet);
QuicPacketNumber packet_number = packet->packet_number;
- QuicPacketLength encrypted_length = packet->encrypted_length;
+ const QuicPacketLength encrypted_length = packet->encrypted_length;
// Termination packets are eventually owned by TimeWaitListManager.
// Others are deleted at the end of this call.
if (is_termination_packet) {
@@ -2422,8 +2422,8 @@
// When MSG_TOO_BIG is returned, the system typically knows what the
// actual MTU is, so there is no need to probe further.
// TODO(wub): Reduce max packet size to a safe default, or the actual MTU.
- QUIC_DVLOG(1) << ENDPOINT << " MTU probe packet too big, size:"
- << packet->encrypted_length
+ QUIC_DVLOG(1) << ENDPOINT
+ << " MTU probe packet too big, size:" << encrypted_length
<< ", long_term_mtu_:" << long_term_mtu_;
mtu_discoverer_.Disable();
mtu_discovery_alarm_->Cancel();
@@ -2487,7 +2487,7 @@
if (EnforceAntiAmplificationLimit()) {
// Include bytes sent even if they are not in flight.
- bytes_sent_before_address_validation_ += packet->encrypted_length;
+ bytes_sent_before_address_validation_ += encrypted_length;
}
const bool in_flight = sent_packet_manager_.OnPacketSent(
@@ -2623,8 +2623,8 @@
return writer_->GetNextWriteLocation(self_address().host(), peer_address());
}
-void QuicConnection::OnSerializedPacket(SerializedPacket* serialized_packet) {
- if (serialized_packet->encrypted_buffer == nullptr) {
+void QuicConnection::OnSerializedPacket(SerializedPacket serialized_packet) {
+ if (serialized_packet.encrypted_buffer == nullptr) {
// We failed to serialize the packet, so close the connection.
// Specify that the close is silent, that no packet be sent, so no infinite
// loop here.
@@ -2643,14 +2643,14 @@
return;
}
- if (serialized_packet->retransmittable_frames.empty()) {
+ if (serialized_packet.retransmittable_frames.empty()) {
// Increment consecutive_num_packets_with_no_retransmittable_frames_ if
// this packet is a new transmission with no retransmittable frames.
++consecutive_num_packets_with_no_retransmittable_frames_;
} else {
consecutive_num_packets_with_no_retransmittable_frames_ = 0;
}
- SendOrQueuePacket(serialized_packet);
+ SendOrQueuePacket(std::move(serialized_packet));
}
void QuicConnection::OnUnrecoverableError(QuicErrorCode error,
@@ -2709,14 +2709,9 @@
kAlarmGranularity);
}
-void QuicConnection::SendOrQueuePacket(SerializedPacket* packet) {
+void QuicConnection::SendOrQueuePacket(SerializedPacket packet) {
// The caller of this function is responsible for checking CanWrite().
- if (packet->encrypted_buffer == nullptr) {
- QUIC_BUG << "packet.encrypted_buffer == nullptr in to SendOrQueuePacket";
- return;
- }
- WritePacket(packet);
- ClearSerializedPacket(packet);
+ WritePacket(&packet);
}
void QuicConnection::OnPingTimeout() {
@@ -3568,7 +3563,7 @@
<< "Sending path probe packet for connection_id = "
<< server_connection_id_;
- OwningSerializedPacketPointer probing_packet;
+ std::unique_ptr<SerializedPacket> probing_packet;
if (!version().HasIetfQuicFrames()) {
// Non-IETF QUIC, generate a padded ping regardless of whether this is a
// request or a response.
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h
index 08f44ff..6586fb2 100644
--- a/quic/core/quic_connection.h
+++ b/quic/core/quic_connection.h
@@ -588,7 +588,7 @@
IsHandshake handshake) override;
const QuicFrames MaybeBundleAckOpportunistically() override;
char* GetPacketBuffer() override;
- void OnSerializedPacket(SerializedPacket* packet) override;
+ void OnSerializedPacket(SerializedPacket packet) override;
void OnUnrecoverableError(QuicErrorCode error,
const std::string& error_details) override;
@@ -971,7 +971,7 @@
// Send a packet to the peer, and takes ownership of the packet if the packet
// cannot be written immediately.
- virtual void SendOrQueuePacket(SerializedPacket* packet);
+ virtual void SendOrQueuePacket(SerializedPacket packet);
// Called after a packet is received from a new effective peer address and is
// decrypted. Starts validation of effective peer's address change. Calls
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index ffe43c6..a5a0a59 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -674,7 +674,7 @@
serialized_packet.retransmittable_frames.push_back(
QuicFrame(QuicPingFrame()));
}
- OnSerializedPacket(&serialized_packet);
+ OnSerializedPacket(std::move(serialized_packet));
}
QuicConsumedData SaveAndSendStreamData(QuicStreamId id,
@@ -1160,6 +1160,15 @@
}
}
+ QuicFrame MakeCryptoFrame() const {
+ if (QuicVersionUsesCryptoFrames(connection_.transport_version())) {
+ return QuicFrame(new QuicCryptoFrame(crypto_frame_));
+ }
+ return QuicFrame(QuicStreamFrame(
+ QuicUtils::GetCryptoStreamId(connection_.transport_version()), false,
+ 0u, quiche::QuicheStringPiece()));
+ }
+
void ProcessFramePacket(QuicFrame frame) {
ProcessFramePacketWithAddresses(frame, kSelfAddress, kPeerAddress);
}
@@ -1506,7 +1515,7 @@
return ConstructPacket(header, frames);
}
- OwningSerializedPacketPointer ConstructProbingPacket() {
+ std::unique_ptr<SerializedPacket> ConstructProbingPacket() {
if (VersionHasIetfQuicFrames(version().transport_version)) {
QuicPathFrameBuffer payload = {
{0xde, 0xad, 0xbe, 0xef, 0xba, 0xdc, 0x0f, 0xfe}};
@@ -1780,17 +1789,13 @@
EXPECT_EQ(Perspective::IS_CLIENT, connection_.perspective());
EXPECT_TRUE(connection_.connected());
- QuicFrame frame;
if (QuicVersionUsesCryptoFrames(connection_.transport_version())) {
- frame = QuicFrame(&crypto_frame_);
EXPECT_CALL(visitor_, OnCryptoFrame(_));
} else {
- frame = QuicFrame(QuicStreamFrame(
- QuicUtils::GetCryptoStreamId(connection_.transport_version()), false,
- 0u, quiche::QuicheStringPiece()));
EXPECT_CALL(visitor_, OnStreamFrame(_));
}
- ProcessFramePacketWithAddresses(frame, kSelfAddress, kPeerAddress);
+ ProcessFramePacketWithAddresses(MakeCryptoFrame(), kSelfAddress,
+ kPeerAddress);
// Cause change in self_address.
QuicIpAddress host;
host.FromString("1.1.1.1");
@@ -1800,7 +1805,8 @@
} else {
EXPECT_CALL(visitor_, OnStreamFrame(_));
}
- ProcessFramePacketWithAddresses(frame, self_address, kPeerAddress);
+ ProcessFramePacketWithAddresses(MakeCryptoFrame(), self_address,
+ kPeerAddress);
EXPECT_TRUE(connection_.connected());
}
@@ -1811,24 +1817,21 @@
EXPECT_EQ(Perspective::IS_SERVER, connection_.perspective());
EXPECT_TRUE(connection_.connected());
- QuicFrame frame;
if (QuicVersionUsesCryptoFrames(connection_.transport_version())) {
- frame = QuicFrame(&crypto_frame_);
EXPECT_CALL(visitor_, OnCryptoFrame(_));
} else {
- frame = QuicFrame(QuicStreamFrame(
- QuicUtils::GetCryptoStreamId(connection_.transport_version()), false,
- 0u, quiche::QuicheStringPiece()));
EXPECT_CALL(visitor_, OnStreamFrame(_));
}
- ProcessFramePacketWithAddresses(frame, kSelfAddress, kPeerAddress);
+ ProcessFramePacketWithAddresses(MakeCryptoFrame(), kSelfAddress,
+ kPeerAddress);
// Cause change in self_address.
QuicIpAddress host;
host.FromString("1.1.1.1");
QuicSocketAddress self_address(host, 123);
EXPECT_CALL(visitor_, AllowSelfAddressChange()).WillOnce(Return(false));
EXPECT_CALL(visitor_, OnConnectionClosed(_, _));
- ProcessFramePacketWithAddresses(frame, self_address, kPeerAddress);
+ ProcessFramePacketWithAddresses(MakeCryptoFrame(), self_address,
+ kPeerAddress);
EXPECT_FALSE(connection_.connected());
TestConnectionCloseQuicErrorCode(QUIC_ERROR_MIGRATING_ADDRESS);
}
@@ -1840,29 +1843,27 @@
EXPECT_EQ(Perspective::IS_SERVER, connection_.perspective());
EXPECT_TRUE(connection_.connected());
- QuicFrame frame;
if (QuicVersionUsesCryptoFrames(connection_.transport_version())) {
- frame = QuicFrame(&crypto_frame_);
EXPECT_CALL(visitor_, OnCryptoFrame(_)).Times(3);
} else {
- frame = QuicFrame(QuicStreamFrame(
- QuicUtils::GetCryptoStreamId(connection_.transport_version()), false,
- 0u, quiche::QuicheStringPiece()));
EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(3);
}
QuicIpAddress host;
host.FromString("1.1.1.1");
QuicSocketAddress self_address1(host, 443);
- ProcessFramePacketWithAddresses(frame, self_address1, kPeerAddress);
+ ProcessFramePacketWithAddresses(MakeCryptoFrame(), self_address1,
+ kPeerAddress);
// Cause self_address change to mapped Ipv4 address.
QuicIpAddress host2;
host2.FromString(quiche::QuicheStrCat(
"::ffff:", connection_.self_address().host().ToString()));
QuicSocketAddress self_address2(host2, connection_.self_address().port());
- ProcessFramePacketWithAddresses(frame, self_address2, kPeerAddress);
+ ProcessFramePacketWithAddresses(MakeCryptoFrame(), self_address2,
+ kPeerAddress);
EXPECT_TRUE(connection_.connected());
// self_address change back to Ipv4 address.
- ProcessFramePacketWithAddresses(frame, self_address1, kPeerAddress);
+ ProcessFramePacketWithAddresses(MakeCryptoFrame(), self_address1,
+ kPeerAddress);
EXPECT_TRUE(connection_.connected());
}
@@ -1877,21 +1878,17 @@
QuicConnectionPeer::SetEffectivePeerAddress(&connection_,
QuicSocketAddress());
- QuicFrame frame;
if (QuicVersionUsesCryptoFrames(connection_.transport_version())) {
- frame = QuicFrame(&crypto_frame_);
EXPECT_CALL(visitor_, OnCryptoFrame(_)).Times(AnyNumber());
} else {
- frame = QuicFrame(QuicStreamFrame(
- QuicUtils::GetCryptoStreamId(connection_.transport_version()), false,
- 0u, quiche::QuicheStringPiece()));
EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(AnyNumber());
}
QuicPacketCreatorPeer::SetPacketNumber(&peer_creator_, 5);
const QuicSocketAddress kNewPeerAddress =
QuicSocketAddress(QuicIpAddress::Loopback6(),
/*port=*/23456);
- ProcessFramePacketWithAddresses(frame, kSelfAddress, kNewPeerAddress);
+ ProcessFramePacketWithAddresses(MakeCryptoFrame(), kSelfAddress,
+ kNewPeerAddress);
EXPECT_EQ(kNewPeerAddress, connection_.peer_address());
EXPECT_EQ(kNewPeerAddress, connection_.effective_peer_address());
@@ -1899,7 +1896,8 @@
QuicPacketCreatorPeer::SetPacketNumber(&peer_creator_, 4);
// This is an old packet, do not migrate.
EXPECT_CALL(visitor_, OnConnectionMigration(PORT_CHANGE)).Times(0);
- ProcessFramePacketWithAddresses(frame, kSelfAddress, kPeerAddress);
+ ProcessFramePacketWithAddresses(MakeCryptoFrame(), kSelfAddress,
+ kPeerAddress);
EXPECT_EQ(kNewPeerAddress, connection_.peer_address());
EXPECT_EQ(kNewPeerAddress, connection_.effective_peer_address());
}
@@ -1917,17 +1915,13 @@
QuicSocketAddress());
EXPECT_FALSE(connection_.effective_peer_address().IsInitialized());
- QuicFrame frame;
if (QuicVersionUsesCryptoFrames(connection_.transport_version())) {
- frame = QuicFrame(&crypto_frame_);
EXPECT_CALL(visitor_, OnCryptoFrame(_)).Times(AnyNumber());
} else {
- frame = QuicFrame(QuicStreamFrame(
- QuicUtils::GetCryptoStreamId(connection_.transport_version()), false,
- 0u, quiche::QuicheStringPiece()));
EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(AnyNumber());
}
- ProcessFramePacketWithAddresses(frame, kSelfAddress, kPeerAddress);
+ ProcessFramePacketWithAddresses(MakeCryptoFrame(), kSelfAddress,
+ kPeerAddress);
EXPECT_EQ(kPeerAddress, connection_.peer_address());
EXPECT_EQ(kPeerAddress, connection_.effective_peer_address());
@@ -1936,7 +1930,8 @@
const QuicSocketAddress kNewPeerAddress =
QuicSocketAddress(QuicIpAddress::Loopback6(), /*port=*/23456);
EXPECT_CALL(visitor_, OnConnectionMigration(PORT_CHANGE)).Times(1);
- ProcessFramePacketWithAddresses(frame, kSelfAddress, kNewPeerAddress);
+ ProcessFramePacketWithAddresses(MakeCryptoFrame(), kSelfAddress,
+ kNewPeerAddress);
EXPECT_EQ(kNewPeerAddress, connection_.peer_address());
EXPECT_EQ(kNewPeerAddress, connection_.effective_peer_address());
}
@@ -1956,17 +1951,13 @@
QuicSocketAddress(QuicIpAddress::Loopback6(), /*port=*/43210);
connection_.ReturnEffectivePeerAddressForNextPacket(kEffectivePeerAddress);
- QuicFrame frame;
if (QuicVersionUsesCryptoFrames(connection_.transport_version())) {
- frame = QuicFrame(&crypto_frame_);
EXPECT_CALL(visitor_, OnCryptoFrame(_)).Times(AnyNumber());
} else {
- frame = QuicFrame(QuicStreamFrame(
- QuicUtils::GetCryptoStreamId(connection_.transport_version()), false,
- 0u, quiche::QuicheStringPiece()));
EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(AnyNumber());
}
- ProcessFramePacketWithAddresses(frame, kSelfAddress, kPeerAddress);
+ ProcessFramePacketWithAddresses(MakeCryptoFrame(), kSelfAddress,
+ kPeerAddress);
EXPECT_EQ(kPeerAddress, connection_.peer_address());
EXPECT_EQ(kEffectivePeerAddress, connection_.effective_peer_address());
@@ -1976,7 +1967,8 @@
QuicSocketAddress(QuicIpAddress::Loopback6(), /*port=*/54321);
connection_.ReturnEffectivePeerAddressForNextPacket(kNewEffectivePeerAddress);
EXPECT_CALL(visitor_, OnConnectionMigration(PORT_CHANGE)).Times(1);
- ProcessFramePacketWithAddresses(frame, kSelfAddress, kPeerAddress);
+ ProcessFramePacketWithAddresses(MakeCryptoFrame(), kSelfAddress,
+ kPeerAddress);
EXPECT_EQ(kPeerAddress, connection_.peer_address());
EXPECT_EQ(kNewEffectivePeerAddress, connection_.effective_peer_address());
@@ -2005,7 +1997,8 @@
connection_.ReturnEffectivePeerAddressForNextPacket(
kNewerEffectivePeerAddress);
EXPECT_CALL(visitor_, OnConnectionMigration(PORT_CHANGE)).Times(1);
- ProcessFramePacketWithAddresses(frame, kSelfAddress, kFinalPeerAddress);
+ ProcessFramePacketWithAddresses(MakeCryptoFrame(), kSelfAddress,
+ kFinalPeerAddress);
EXPECT_EQ(kFinalPeerAddress, connection_.peer_address());
EXPECT_EQ(kNewerEffectivePeerAddress, connection_.effective_peer_address());
EXPECT_EQ(PORT_CHANGE, connection_.active_effective_peer_migration_type());
@@ -2019,7 +2012,8 @@
kNewestEffectivePeerAddress);
EXPECT_CALL(visitor_, OnConnectionMigration(IPV6_TO_IPV4_CHANGE)).Times(1);
EXPECT_CALL(*send_algorithm_, OnConnectionMigration()).Times(1);
- ProcessFramePacketWithAddresses(frame, kSelfAddress, kFinalPeerAddress);
+ ProcessFramePacketWithAddresses(MakeCryptoFrame(), kSelfAddress,
+ kFinalPeerAddress);
EXPECT_EQ(kFinalPeerAddress, connection_.peer_address());
EXPECT_EQ(kNewestEffectivePeerAddress, connection_.effective_peer_address());
EXPECT_EQ(IPV6_TO_IPV4_CHANGE,
@@ -2039,17 +2033,13 @@
QuicSocketAddress());
EXPECT_FALSE(connection_.effective_peer_address().IsInitialized());
- QuicFrame frame;
if (QuicVersionUsesCryptoFrames(connection_.transport_version())) {
- frame = QuicFrame(&crypto_frame_);
EXPECT_CALL(visitor_, OnCryptoFrame(_)).Times(AnyNumber());
} else {
- frame = QuicFrame(QuicStreamFrame(
- QuicUtils::GetCryptoStreamId(connection_.transport_version()), false,
- 0u, quiche::QuicheStringPiece()));
EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(AnyNumber());
}
- ProcessFramePacketWithAddresses(frame, kSelfAddress, kPeerAddress);
+ ProcessFramePacketWithAddresses(MakeCryptoFrame(), kSelfAddress,
+ kPeerAddress);
EXPECT_EQ(kPeerAddress, connection_.peer_address());
EXPECT_EQ(kPeerAddress, connection_.effective_peer_address());
@@ -2058,7 +2048,7 @@
// Process a padded PING or PATH CHALLENGE packet with no peer address change
// on server side will be ignored.
- OwningSerializedPacketPointer probing_packet = ConstructProbingPacket();
+ std::unique_ptr<SerializedPacket> probing_packet = ConstructProbingPacket();
std::unique_ptr<QuicReceivedPacket> received(ConstructReceivedPacket(
QuicEncryptedPacket(probing_packet->encrypted_buffer,
@@ -2141,17 +2131,13 @@
QuicSocketAddress());
EXPECT_FALSE(connection_.effective_peer_address().IsInitialized());
- QuicFrame frame;
if (QuicVersionUsesCryptoFrames(connection_.transport_version())) {
- frame = QuicFrame(&crypto_frame_);
EXPECT_CALL(visitor_, OnCryptoFrame(_)).Times(AnyNumber());
} else {
- frame = QuicFrame(QuicStreamFrame(
- QuicUtils::GetCryptoStreamId(connection_.transport_version()), false,
- 0u, quiche::QuicheStringPiece()));
EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(AnyNumber());
}
- ProcessFramePacketWithAddresses(frame, kSelfAddress, kPeerAddress);
+ ProcessFramePacketWithAddresses(MakeCryptoFrame(), kSelfAddress,
+ kPeerAddress);
EXPECT_EQ(kPeerAddress, connection_.peer_address());
EXPECT_EQ(kPeerAddress, connection_.effective_peer_address());
@@ -2168,7 +2154,7 @@
const QuicSocketAddress kNewPeerAddress =
QuicSocketAddress(QuicIpAddress::Loopback6(), /*port=*/23456);
- OwningSerializedPacketPointer probing_packet = ConstructProbingPacket();
+ std::unique_ptr<SerializedPacket> probing_packet = ConstructProbingPacket();
std::unique_ptr<QuicReceivedPacket> received(ConstructReceivedPacket(
QuicEncryptedPacket(probing_packet->encrypted_buffer,
probing_packet->encrypted_length),
@@ -2186,7 +2172,8 @@
// Process another packet with the old peer address on server side will not
// start peer migration.
EXPECT_CALL(visitor_, OnConnectionMigration(PORT_CHANGE)).Times(0);
- ProcessFramePacketWithAddresses(frame, kSelfAddress, kPeerAddress);
+ ProcessFramePacketWithAddresses(MakeCryptoFrame(), kSelfAddress,
+ kPeerAddress);
EXPECT_EQ(kPeerAddress, connection_.peer_address());
EXPECT_EQ(kPeerAddress, connection_.effective_peer_address());
}
@@ -2205,17 +2192,13 @@
QuicSocketAddress());
EXPECT_FALSE(connection_.effective_peer_address().IsInitialized());
- QuicFrame frame;
if (GetParam().version.UsesCryptoFrames()) {
- frame = QuicFrame(&crypto_frame_);
EXPECT_CALL(visitor_, OnCryptoFrame(_)).Times(AnyNumber());
} else {
- frame = QuicFrame(QuicStreamFrame(
- QuicUtils::GetCryptoStreamId(connection_.transport_version()), false,
- 0u, quiche::QuicheStringPiece()));
EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(AnyNumber());
}
- ProcessFramePacketWithAddresses(frame, kSelfAddress, kPeerAddress);
+ ProcessFramePacketWithAddresses(MakeCryptoFrame(), kSelfAddress,
+ kPeerAddress);
EXPECT_EQ(kPeerAddress, connection_.peer_address());
EXPECT_EQ(kPeerAddress, connection_.effective_peer_address());
@@ -2269,7 +2252,8 @@
} else {
EXPECT_CALL(visitor_, OnConnectionMigration(PORT_CHANGE)).Times(0);
}
- ProcessFramePacketWithAddresses(frame, kSelfAddress, kPeerAddress);
+ ProcessFramePacketWithAddresses(MakeCryptoFrame(), kSelfAddress,
+ kPeerAddress);
EXPECT_EQ(kPeerAddress, connection_.peer_address());
EXPECT_EQ(kPeerAddress, connection_.effective_peer_address());
}
@@ -2287,18 +2271,14 @@
QuicSocketAddress());
EXPECT_FALSE(connection_.effective_peer_address().IsInitialized());
- QuicFrame frame;
if (QuicVersionUsesCryptoFrames(connection_.transport_version())) {
- frame = QuicFrame(&crypto_frame_);
EXPECT_CALL(visitor_, OnCryptoFrame(_)).Times(AnyNumber());
} else {
- frame = QuicFrame(QuicStreamFrame(
- QuicUtils::GetCryptoStreamId(connection_.transport_version()), false,
- 0u, quiche::QuicheStringPiece()));
EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(AnyNumber());
}
QuicPacketCreatorPeer::SetPacketNumber(&peer_creator_, 5);
- ProcessFramePacketWithAddresses(frame, kSelfAddress, kPeerAddress);
+ ProcessFramePacketWithAddresses(MakeCryptoFrame(), kSelfAddress,
+ kPeerAddress);
EXPECT_EQ(kPeerAddress, connection_.peer_address());
EXPECT_EQ(kPeerAddress, connection_.effective_peer_address());
@@ -2320,7 +2300,7 @@
const QuicSocketAddress kNewPeerAddress =
QuicSocketAddress(QuicIpAddress::Loopback6(), /*port=*/23456);
- OwningSerializedPacketPointer probing_packet = ConstructProbingPacket();
+ std::unique_ptr<SerializedPacket> probing_packet = ConstructProbingPacket();
std::unique_ptr<QuicReceivedPacket> received(ConstructReceivedPacket(
QuicEncryptedPacket(probing_packet->encrypted_buffer,
probing_packet->encrypted_length),
@@ -2349,17 +2329,13 @@
QuicSocketAddress());
EXPECT_FALSE(connection_.effective_peer_address().IsInitialized());
- QuicFrame frame;
if (QuicVersionUsesCryptoFrames(connection_.transport_version())) {
- frame = QuicFrame(&crypto_frame_);
EXPECT_CALL(visitor_, OnCryptoFrame(_)).Times(AnyNumber());
} else {
- frame = QuicFrame(QuicStreamFrame(
- QuicUtils::GetCryptoStreamId(connection_.transport_version()), false,
- 0u, quiche::QuicheStringPiece()));
EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(AnyNumber());
}
- ProcessFramePacketWithAddresses(frame, kSelfAddress, kPeerAddress);
+ ProcessFramePacketWithAddresses(MakeCryptoFrame(), kSelfAddress,
+ kPeerAddress);
EXPECT_EQ(kPeerAddress, connection_.peer_address());
EXPECT_EQ(kPeerAddress, connection_.effective_peer_address());
@@ -2377,7 +2353,7 @@
const QuicSocketAddress kNewPeerAddress =
QuicSocketAddress(QuicIpAddress::Loopback6(), /*port=*/23456);
- OwningSerializedPacketPointer probing_packet = ConstructProbingPacket();
+ std::unique_ptr<SerializedPacket> probing_packet = ConstructProbingPacket();
std::unique_ptr<QuicReceivedPacket> received(ConstructReceivedPacket(
QuicEncryptedPacket(probing_packet->encrypted_buffer,
probing_packet->encrypted_length),
@@ -2390,7 +2366,8 @@
// side will start peer migration.
EXPECT_CALL(visitor_, OnConnectionMigration(PORT_CHANGE)).Times(1);
- ProcessFramePacketWithAddresses(frame, kSelfAddress, kNewPeerAddress);
+ ProcessFramePacketWithAddresses(MakeCryptoFrame(), kSelfAddress,
+ kNewPeerAddress);
EXPECT_EQ(kNewPeerAddress, connection_.peer_address());
EXPECT_EQ(kNewPeerAddress, connection_.effective_peer_address());
}
@@ -2408,17 +2385,13 @@
QuicSocketAddress());
EXPECT_FALSE(connection_.effective_peer_address().IsInitialized());
- QuicFrame frame;
if (QuicVersionUsesCryptoFrames(connection_.transport_version())) {
- frame = QuicFrame(&crypto_frame_);
EXPECT_CALL(visitor_, OnCryptoFrame(_)).Times(AnyNumber());
} else {
- frame = QuicFrame(QuicStreamFrame(
- QuicUtils::GetCryptoStreamId(connection_.transport_version()), false,
- 0u, quiche::QuicheStringPiece()));
EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(AnyNumber());
}
- ProcessFramePacketWithAddresses(frame, kSelfAddress, kPeerAddress);
+ ProcessFramePacketWithAddresses(MakeCryptoFrame(), kSelfAddress,
+ kPeerAddress);
EXPECT_EQ(kPeerAddress, connection_.peer_address());
EXPECT_EQ(kPeerAddress, connection_.effective_peer_address());
@@ -2427,7 +2400,7 @@
EXPECT_CALL(visitor_, OnConnectionMigration(PORT_CHANGE)).Times(0);
EXPECT_CALL(visitor_, OnPacketReceived(_, _, false)).Times(1);
- OwningSerializedPacketPointer probing_packet = ConstructProbingPacket();
+ std::unique_ptr<SerializedPacket> probing_packet = ConstructProbingPacket();
std::unique_ptr<QuicReceivedPacket> received(ConstructReceivedPacket(
QuicEncryptedPacket(probing_packet->encrypted_buffer,
probing_packet->encrypted_length),
@@ -2461,17 +2434,13 @@
QuicSocketAddress());
EXPECT_FALSE(connection_.effective_peer_address().IsInitialized());
- QuicFrame frame;
if (QuicVersionUsesCryptoFrames(connection_.transport_version())) {
- frame = QuicFrame(&crypto_frame_);
EXPECT_CALL(visitor_, OnCryptoFrame(_)).Times(AnyNumber());
} else {
- frame = QuicFrame(QuicStreamFrame(
- QuicUtils::GetCryptoStreamId(connection_.transport_version()), false,
- 0u, quiche::QuicheStringPiece()));
EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(AnyNumber());
}
- ProcessFramePacketWithAddresses(frame, kSelfAddress, kPeerAddress);
+ ProcessFramePacketWithAddresses(MakeCryptoFrame(), kSelfAddress,
+ kPeerAddress);
EXPECT_EQ(kPeerAddress, connection_.peer_address());
EXPECT_EQ(kPeerAddress, connection_.effective_peer_address());
@@ -2489,7 +2458,7 @@
const QuicSocketAddress kNewSelfAddress =
QuicSocketAddress(QuicIpAddress::Loopback6(), /*port=*/23456);
- OwningSerializedPacketPointer probing_packet = ConstructProbingPacket();
+ std::unique_ptr<SerializedPacket> probing_packet = ConstructProbingPacket();
std::unique_ptr<QuicReceivedPacket> received(ConstructReceivedPacket(
QuicEncryptedPacket(probing_packet->encrypted_buffer,
probing_packet->encrypted_length),
@@ -2517,17 +2486,13 @@
QuicSocketAddress());
EXPECT_FALSE(connection_.effective_peer_address().IsInitialized());
- QuicFrame frame;
if (QuicVersionUsesCryptoFrames(connection_.transport_version())) {
- frame = QuicFrame(&crypto_frame_);
EXPECT_CALL(visitor_, OnCryptoFrame(_)).Times(AnyNumber());
} else {
- frame = QuicFrame(QuicStreamFrame(
- QuicUtils::GetCryptoStreamId(connection_.transport_version()), false,
- 0u, quiche::QuicheStringPiece()));
EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(AnyNumber());
}
- ProcessFramePacketWithAddresses(frame, kSelfAddress, kPeerAddress);
+ ProcessFramePacketWithAddresses(MakeCryptoFrame(), kSelfAddress,
+ kPeerAddress);
EXPECT_EQ(kPeerAddress, connection_.peer_address());
EXPECT_EQ(kPeerAddress, connection_.effective_peer_address());
@@ -2536,7 +2501,8 @@
const QuicSocketAddress kNewPeerAddress =
QuicSocketAddress(QuicIpAddress::Loopback6(), /*port=*/23456);
EXPECT_CALL(visitor_, OnConnectionMigration(PORT_CHANGE)).Times(0);
- ProcessFramePacketWithAddresses(frame, kSelfAddress, kNewPeerAddress);
+ ProcessFramePacketWithAddresses(MakeCryptoFrame(), kSelfAddress,
+ kNewPeerAddress);
EXPECT_EQ(kNewPeerAddress, connection_.peer_address());
EXPECT_EQ(kNewPeerAddress, connection_.effective_peer_address());
}
@@ -7382,31 +7348,31 @@
EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
- QuicGoAwayFrame goaway;
- goaway.last_good_stream_id = 1;
- goaway.error_code = QUIC_PEER_GOING_AWAY;
- goaway.reason_phrase = "Going away.";
+ QuicGoAwayFrame* goaway = new QuicGoAwayFrame();
+ goaway->last_good_stream_id = 1;
+ goaway->error_code = QUIC_PEER_GOING_AWAY;
+ goaway->reason_phrase = "Going away.";
EXPECT_CALL(visitor_, OnGoAway(_));
- ProcessGoAwayPacket(&goaway);
+ ProcessGoAwayPacket(goaway);
}
TEST_P(QuicConnectionTest, WindowUpdate) {
EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
- QuicWindowUpdateFrame window_update;
- window_update.stream_id = 3;
- window_update.max_data = 1234;
+ QuicWindowUpdateFrame* window_update = new QuicWindowUpdateFrame();
+ window_update->stream_id = 3;
+ window_update->max_data = 1234;
EXPECT_CALL(visitor_, OnWindowUpdateFrame(_));
- ProcessFramePacket(QuicFrame(&window_update));
+ ProcessFramePacket(QuicFrame(window_update));
}
TEST_P(QuicConnectionTest, Blocked) {
EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
- QuicBlockedFrame blocked;
- blocked.stream_id = 3;
+ QuicBlockedFrame* blocked = new QuicBlockedFrame();
+ blocked->stream_id = 3;
EXPECT_CALL(visitor_, OnBlockedFrame(_));
- ProcessFramePacket(QuicFrame(&blocked));
+ ProcessFramePacket(QuicFrame(blocked));
EXPECT_EQ(1u, connection_.GetStats().blocked_frames_received);
EXPECT_EQ(0u, connection_.GetStats().blocked_frames_sent);
}
@@ -7684,11 +7650,11 @@
EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
// Send a WINDOW_UPDATE frame.
- QuicWindowUpdateFrame window_update;
- window_update.stream_id = 3;
- window_update.max_data = 1234;
+ QuicWindowUpdateFrame* window_update = new QuicWindowUpdateFrame();
+ window_update->stream_id = 3;
+ window_update->max_data = 1234;
EXPECT_CALL(visitor_, OnWindowUpdateFrame(_));
- ProcessFramePacket(QuicFrame(&window_update));
+ ProcessFramePacket(QuicFrame(window_update));
// Ensure that this has caused the ACK alarm to be set.
QuicAlarm* ack_alarm = QuicConnectionPeer::GetAckAlarm(&connection_);
@@ -7699,10 +7665,10 @@
EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
// Send a BLOCKED frame.
- QuicBlockedFrame blocked;
- blocked.stream_id = 3;
+ QuicBlockedFrame* blocked = new QuicBlockedFrame();
+ blocked->stream_id = 3;
EXPECT_CALL(visitor_, OnBlockedFrame(_));
- ProcessFramePacket(QuicFrame(&blocked));
+ ProcessFramePacket(QuicFrame(blocked));
// Ensure that this has caused the ACK alarm to be set.
QuicAlarm* ack_alarm = QuicConnectionPeer::GetAckAlarm(&connection_);
@@ -9304,17 +9270,13 @@
return;
}
set_perspective(Perspective::IS_SERVER);
- QuicFrame frame;
if (QuicVersionUsesCryptoFrames(connection_.transport_version())) {
- frame = QuicFrame(&crypto_frame_);
EXPECT_CALL(visitor_, OnCryptoFrame(_)).Times(1);
} else {
- frame = QuicFrame(QuicStreamFrame(
- QuicUtils::GetCryptoStreamId(connection_.transport_version()), false,
- 0u, quiche::QuicheStringPiece()));
EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(1);
}
- ProcessFramePacketWithAddresses(frame, kSelfAddress, kPeerAddress);
+ ProcessFramePacketWithAddresses(MakeCryptoFrame(), kSelfAddress,
+ kPeerAddress);
// Let connection process a Google QUIC packet.
peer_framer_.set_version_for_tests(
@@ -9625,20 +9587,16 @@
/*transport_close_frame_type=*/0));
// Received 2 packets.
- QuicFrame frame;
if (QuicVersionUsesCryptoFrames(connection_.transport_version())) {
- frame = QuicFrame(&crypto_frame_);
EXPECT_CALL(visitor_, OnCryptoFrame(_)).Times(AnyNumber());
} else {
- frame = QuicFrame(QuicStreamFrame(
- QuicUtils::GetCryptoStreamId(connection_.transport_version()), false,
- 0u, quiche::QuicheStringPiece()));
EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(AnyNumber());
}
- ProcessFramePacketWithAddresses(frame, kSelfAddress, kPeerAddress);
+ ProcessFramePacketWithAddresses(MakeCryptoFrame(), kSelfAddress,
+ kPeerAddress);
QuicAlarm* ack_alarm = QuicConnectionPeer::GetAckAlarm(&connection_);
EXPECT_TRUE(ack_alarm->IsSet());
- ProcessFramePacketWithAddresses(QuicFrame(connection_close_frame.get()),
+ ProcessFramePacketWithAddresses(QuicFrame(connection_close_frame.release()),
kSelfAddress, kPeerAddress);
// Verify ack alarm is not set.
EXPECT_FALSE(ack_alarm->IsSet());
diff --git a/quic/core/quic_dispatcher.cc b/quic/core/quic_dispatcher.cc
index ffea9ff..c0d3970 100644
--- a/quic/core/quic_dispatcher.cc
+++ b/quic/core/quic_dispatcher.cc
@@ -61,14 +61,11 @@
~PacketCollector() override = default;
// QuicPacketCreator::DelegateInterface methods:
- void OnSerializedPacket(SerializedPacket* serialized_packet) override {
+ void OnSerializedPacket(SerializedPacket serialized_packet) override {
// Make a copy of the serialized packet to send later.
packets_.emplace_back(
- new QuicEncryptedPacket(CopyBuffer(*serialized_packet),
- serialized_packet->encrypted_length, true));
- serialized_packet->encrypted_buffer = nullptr;
- DeleteFrames(&(serialized_packet->retransmittable_frames));
- serialized_packet->retransmittable_frames.clear();
+ new QuicEncryptedPacket(CopyBuffer(serialized_packet),
+ serialized_packet.encrypted_length, true));
}
char* GetPacketBuffer() override {
@@ -571,35 +568,35 @@
QuicDispatcher::QuicPacketFate QuicDispatcher::ValidityChecks(
const ReceivedPacketInfo& packet_info) {
if (!packet_info.version_flag) {
- // The Android network conformance test contains a UDP test that sends a
- // 12-byte packet with the following format:
- // - 0x0c (public flags: 8-byte connection ID, 1-byte packet number)
- // - randomized 8-byte connection ID
- // - 0x01 (1-byte packet number)
- // - 0x00 (private flags)
- // - 0x07 (PING frame).
- // That packet is invalid and we would normally drop it but in order to
- // unblock this conformance testing we have the following workaround that
- // will be removed once the fixed test is deployed.
- // TODO(b/139691956) Remove this workaround once fixed test is deployed.
- if (!GetQuicReloadableFlag(
- quic_remove_android_conformance_test_workaround) &&
- packet_info.packet.length() == 12 &&
- packet_info.packet.data()[0] == 0x0c &&
- packet_info.packet.data()[9] == 0x01 &&
- packet_info.packet.data()[10] == 0x00 &&
- packet_info.packet.data()[11] == 0x07) {
- QUIC_DLOG(INFO) << "Received Android UDP network conformance test "
- "packet with connection ID "
- << packet_info.destination_connection_id;
- // Respond with a public reset that the test will know how to parse
- // then return kFateDrop to stop processing of this packet.
- time_wait_list_manager()->SendPublicReset(
- packet_info.self_address, packet_info.peer_address,
- packet_info.destination_connection_id,
- /*ietf_quic=*/false, GetPerPacketContext());
- return kFateDrop;
- }
+ // The Android network conformance test contains a UDP test that sends a
+ // 12-byte packet with the following format:
+ // - 0x0c (public flags: 8-byte connection ID, 1-byte packet number)
+ // - randomized 8-byte connection ID
+ // - 0x01 (1-byte packet number)
+ // - 0x00 (private flags)
+ // - 0x07 (PING frame).
+ // That packet is invalid and we would normally drop it but in order to
+ // unblock this conformance testing we have the following workaround that
+ // will be removed once the fixed test is deployed.
+ // TODO(b/139691956) Remove this workaround once fixed test is deployed.
+ if (!GetQuicReloadableFlag(
+ quic_remove_android_conformance_test_workaround) &&
+ packet_info.packet.length() == 12 &&
+ packet_info.packet.data()[0] == 0x0c &&
+ packet_info.packet.data()[9] == 0x01 &&
+ packet_info.packet.data()[10] == 0x00 &&
+ packet_info.packet.data()[11] == 0x07) {
+ QUIC_DLOG(INFO) << "Received Android UDP network conformance test "
+ "packet with connection ID "
+ << packet_info.destination_connection_id;
+ // Respond with a public reset that the test will know how to parse
+ // then return kFateDrop to stop processing of this packet.
+ time_wait_list_manager()->SendPublicReset(
+ packet_info.self_address, packet_info.peer_address,
+ packet_info.destination_connection_id,
+ /*ietf_quic=*/false, GetPerPacketContext());
+ return kFateDrop;
+ }
QUIC_DLOG(INFO)
<< "Packet without version arrived for unknown connection ID "
diff --git a/quic/core/quic_packet_creator.cc b/quic/core/quic_packet_creator.cc
index a9d7076..0ae78d2 100644
--- a/quic/core/quic_packet_creator.cc
+++ b/quic/core/quic_packet_creator.cc
@@ -460,7 +460,7 @@
SerializedPacket packet(std::move(packet_));
ClearPacket();
RemoveSoftMaxPacketLength();
- delegate_->OnSerializedPacket(&packet);
+ delegate_->OnSerializedPacket(std::move(packet));
}
void QuicPacketCreator::ClearPacket() {
@@ -768,7 +768,7 @@
return encrypted;
}
-OwningSerializedPacketPointer
+std::unique_ptr<SerializedPacket>
QuicPacketCreator::SerializeConnectivityProbingPacket() {
QUIC_BUG_IF(VersionHasIetfQuicFrames(framer_->transport_version()))
<< "Must not be version 99 to serialize padded ping connectivity probe";
@@ -791,17 +791,20 @@
kMaxOutgoingPacketSize, buffer.get());
DCHECK(encrypted_length);
- OwningSerializedPacketPointer serialize_packet(new SerializedPacket(
+ std::unique_ptr<SerializedPacket> serialize_packet(new SerializedPacket(
header.packet_number, header.packet_number_length, buffer.release(),
encrypted_length, /*has_ack=*/false, /*has_stop_waiting=*/false));
+ serialize_packet->release_encrypted_buffer = [](const char* p) {
+ delete[] p;
+ };
serialize_packet->encryption_level = packet_.encryption_level;
serialize_packet->transmission_type = NOT_RETRANSMISSION;
return serialize_packet;
}
-OwningSerializedPacketPointer
+std::unique_ptr<SerializedPacket>
QuicPacketCreator::SerializePathChallengeConnectivityProbingPacket(
QuicPathFrameBuffer* payload) {
QUIC_BUG_IF(!VersionHasIetfQuicFrames(framer_->transport_version()))
@@ -827,17 +830,21 @@
kMaxOutgoingPacketSize, buffer.get());
DCHECK(encrypted_length);
- OwningSerializedPacketPointer serialize_packet(new SerializedPacket(
- header.packet_number, header.packet_number_length, buffer.release(),
- encrypted_length, /*has_ack=*/false, /*has_stop_waiting=*/false));
+ std::unique_ptr<SerializedPacket> serialize_packet(
+ new SerializedPacket(header.packet_number, header.packet_number_length,
+ buffer.release(), encrypted_length,
+ /*has_ack=*/false, /*has_stop_waiting=*/false));
+ serialize_packet->release_encrypted_buffer = [](const char* p) {
+ delete[] p;
+ };
serialize_packet->encryption_level = packet_.encryption_level;
serialize_packet->transmission_type = NOT_RETRANSMISSION;
return serialize_packet;
}
-OwningSerializedPacketPointer
+std::unique_ptr<SerializedPacket>
QuicPacketCreator::SerializePathResponseConnectivityProbingPacket(
const QuicCircularDeque<QuicPathFrameBuffer>& payloads,
const bool is_padded) {
@@ -864,10 +871,14 @@
kMaxOutgoingPacketSize, buffer.get());
DCHECK(encrypted_length);
- OwningSerializedPacketPointer serialize_packet(new SerializedPacket(
- header.packet_number, header.packet_number_length, buffer.release(),
- encrypted_length, /*has_ack=*/false, /*has_stop_waiting=*/false));
+ std::unique_ptr<SerializedPacket> serialize_packet(
+ new SerializedPacket(header.packet_number, header.packet_number_length,
+ buffer.release(), encrypted_length,
+ /*has_ack=*/false, /*has_stop_waiting=*/false));
+ serialize_packet->release_encrypted_buffer = [](const char* p) {
+ delete[] p;
+ };
serialize_packet->encryption_level = packet_.encryption_level;
serialize_packet->transmission_type = NOT_RETRANSMISSION;
diff --git a/quic/core/quic_packet_creator.h b/quic/core/quic_packet_creator.h
index fbe046f..7695587 100644
--- a/quic/core/quic_packet_creator.h
+++ b/quic/core/quic_packet_creator.h
@@ -44,10 +44,9 @@
// packet. If return nullptr, QuicPacketCreator will serialize on a stack
// buffer.
virtual char* GetPacketBuffer() = 0;
- // Called when a packet is serialized. Delegate does not take the ownership
- // of |serialized_packet|, but takes ownership of any frames it removes
- // from |packet.retransmittable_frames|.
- virtual void OnSerializedPacket(SerializedPacket* serialized_packet) = 0;
+ // Called when a packet is serialized. Delegate take the ownership of
+ // |serialized_packet|.
+ virtual void OnSerializedPacket(SerializedPacket serialized_packet) = 0;
// Called when an unrecoverable error is encountered.
virtual void OnUnrecoverableError(QuicErrorCode error,
@@ -210,19 +209,20 @@
const ParsedQuicVersionVector& supported_versions);
// Creates a connectivity probing packet for versions prior to version 99.
- OwningSerializedPacketPointer SerializeConnectivityProbingPacket();
+ std::unique_ptr<SerializedPacket> SerializeConnectivityProbingPacket();
// Create connectivity probing request and response packets using PATH
// CHALLENGE and PATH RESPONSE frames, respectively, for version 99/IETF QUIC.
// SerializePathChallengeConnectivityProbingPacket will pad the packet to be
// MTU bytes long.
- OwningSerializedPacketPointer SerializePathChallengeConnectivityProbingPacket(
- QuicPathFrameBuffer* payload);
+ std::unique_ptr<SerializedPacket>
+ SerializePathChallengeConnectivityProbingPacket(QuicPathFrameBuffer* payload);
// If |is_padded| is true then SerializePathResponseConnectivityProbingPacket
// will pad the packet to be MTU bytes long, else it will not pad the packet.
// |payloads| is cleared.
- OwningSerializedPacketPointer SerializePathResponseConnectivityProbingPacket(
+ std::unique_ptr<SerializedPacket>
+ SerializePathResponseConnectivityProbingPacket(
const QuicCircularDeque<QuicPathFrameBuffer>& payloads,
const bool is_padded);
@@ -458,8 +458,8 @@
// Serializes all frames which have been added and adds any which should be
// retransmitted to packet_.retransmittable_frames. All frames must fit into
// a single packet.
- // Fails if |buffer_len| isn't long enough for the encrypted packet.
- void SerializePacket(char* encrypted_buffer, size_t buffer_len);
+ // Fails if |encrypted_buffer_len| isn't long enough for the encrypted packet.
+ void SerializePacket(char* encrypted_buffer, size_t encrypted_buffer_len);
// Called after a new SerialiedPacket is created to call the delegate's
// OnSerializedPacket and reset state.
diff --git a/quic/core/quic_packet_creator_test.cc b/quic/core/quic_packet_creator_test.cc
index b11417d..d1beea7 100644
--- a/quic/core/quic_packet_creator_test.cc
+++ b/quic/core/quic_packet_creator_test.cc
@@ -138,28 +138,16 @@
class QuicPacketCreatorTest : public QuicTestWithParam<TestParams> {
public:
- void ClearSerializedPacketForTests(SerializedPacket* serialized_packet) {
- if (serialized_packet == nullptr) {
- return;
- }
- ClearSerializedPacket(serialized_packet);
+ void ClearSerializedPacketForTests(SerializedPacket /*serialized_packet*/) {
+ // serialized packet self-clears on destruction.
}
- void SaveSerializedPacket(SerializedPacket* serialized_packet) {
- if (serialized_packet == nullptr) {
- return;
- }
- delete[] serialized_packet_.encrypted_buffer;
- serialized_packet_ = *serialized_packet;
- serialized_packet_.encrypted_buffer = CopyBuffer(*serialized_packet);
- serialized_packet->retransmittable_frames.clear();
+ void SaveSerializedPacket(SerializedPacket serialized_packet) {
+ serialized_packet_.reset(CopySerializedPacket(
+ serialized_packet, &allocator_, /*copy_buffer=*/true));
}
- void DeleteSerializedPacket() {
- delete[] serialized_packet_.encrypted_buffer;
- serialized_packet_.encrypted_buffer = nullptr;
- ClearSerializedPacket(&serialized_packet_);
- }
+ void DeleteSerializedPacket() { serialized_packet_ = nullptr; }
protected:
QuicPacketCreatorTest()
@@ -173,8 +161,7 @@
Perspective::IS_CLIENT,
connection_id_.length()),
data_("foo"),
- creator_(connection_id_, &client_framer_, &delegate_, &producer_),
- serialized_packet_(creator_.NoPacket()) {
+ creator_(connection_id_, &client_framer_, &delegate_, &producer_) {
EXPECT_CALL(delegate_, GetPacketBuffer()).WillRepeatedly(Return(nullptr));
creator_.SetEncrypter(ENCRYPTION_INITIAL, std::make_unique<NullEncrypter>(
Perspective::IS_CLIENT));
@@ -208,10 +195,7 @@
}
}
- ~QuicPacketCreatorTest() override {
- delete[] serialized_packet_.encrypted_buffer;
- ClearSerializedPacket(&serialized_packet_);
- }
+ ~QuicPacketCreatorTest() override {}
SerializedPacket SerializeAllFrames(const QuicFrames& frames) {
SerializedPacket packet = QuicPacketCreatorPeer::SerializeAllFrames(
@@ -297,7 +281,7 @@
std::string data_;
struct iovec iov_;
TestPacketCreator creator_;
- SerializedPacket serialized_packet_;
+ std::unique_ptr<SerializedPacket> serialized_packet_;
SimpleDataProducer producer_;
SimpleBufferAllocator allocator_;
};
@@ -354,12 +338,12 @@
}
TEST_P(QuicPacketCreatorTest, SerializeConnectionClose) {
- QuicConnectionCloseFrame frame(creator_.transport_version(), QUIC_NO_ERROR,
- "error",
- /*transport_close_frame_type=*/0);
+ QuicConnectionCloseFrame* frame = new QuicConnectionCloseFrame(
+ creator_.transport_version(), QUIC_NO_ERROR, "error",
+ /*transport_close_frame_type=*/0);
QuicFrames frames;
- frames.push_back(QuicFrame(&frame));
+ frames.push_back(QuicFrame(frame));
SerializedPacket serialized = SerializeAllFrames(frames);
EXPECT_EQ(ENCRYPTION_INITIAL, serialized.encryption_level);
ASSERT_EQ(QuicPacketNumber(1u), serialized.packet_number);
@@ -489,7 +473,7 @@
EXPECT_CALL(delegate_, OnSerializedPacket(_))
.WillOnce(Invoke(this, &QuicPacketCreatorTest::SaveSerializedPacket));
creator_.FlushCurrentPacket();
- ASSERT_TRUE(serialized_packet_.encrypted_buffer);
+ ASSERT_TRUE(serialized_packet_->encrypted_buffer);
DeleteSerializedPacket();
}
}
@@ -537,7 +521,7 @@
EXPECT_LT(0u, bytes_consumed);
}
creator_.FlushCurrentPacket();
- ASSERT_TRUE(serialized_packet_.encrypted_buffer);
+ ASSERT_TRUE(serialized_packet_->encrypted_buffer);
// If there is not enough space in the packet to fit a padding frame
// (1 byte) and to expand the stream frame (another 2 bytes) the packet
// will not be padded.
@@ -546,9 +530,9 @@
!QuicVersionUsesCryptoFrames(client_framer_.transport_version())) ||
client_framer_.version().CanSendCoalescedPackets()) {
EXPECT_EQ(kDefaultMaxPacketSize - bytes_free,
- serialized_packet_.encrypted_length);
+ serialized_packet_->encrypted_length);
} else {
- EXPECT_EQ(kDefaultMaxPacketSize, serialized_packet_.encrypted_length);
+ EXPECT_EQ(kDefaultMaxPacketSize, serialized_packet_->encrypted_length);
}
DeleteSerializedPacket();
}
@@ -578,12 +562,12 @@
size_t bytes_consumed = frame.stream_frame.data_length;
EXPECT_LT(0u, bytes_consumed);
creator_.FlushCurrentPacket();
- ASSERT_TRUE(serialized_packet_.encrypted_buffer);
+ ASSERT_TRUE(serialized_packet_->encrypted_buffer);
if (bytes_free > 0) {
EXPECT_EQ(kDefaultMaxPacketSize - bytes_free,
- serialized_packet_.encrypted_length);
+ serialized_packet_->encrypted_length);
} else {
- EXPECT_EQ(kDefaultMaxPacketSize, serialized_packet_.encrypted_length);
+ EXPECT_EQ(kDefaultMaxPacketSize, serialized_packet_->encrypted_length);
}
DeleteSerializedPacket();
}
@@ -951,7 +935,7 @@
creator_.set_encryption_level(level);
- OwningSerializedPacketPointer encrypted;
+ std::unique_ptr<SerializedPacket> encrypted;
if (VersionHasIetfQuicFrames(creator_.transport_version())) {
QuicPathFrameBuffer payload = {
{0xde, 0xad, 0xbe, 0xef, 0xba, 0xdc, 0x0f, 0xfe}};
@@ -994,7 +978,7 @@
creator_.set_encryption_level(level);
- OwningSerializedPacketPointer encrypted(
+ std::unique_ptr<SerializedPacket> encrypted(
creator_.SerializePathChallengeConnectivityProbingPacket(&payload));
{
InSequence s;
@@ -1027,7 +1011,7 @@
QuicCircularDeque<QuicPathFrameBuffer> payloads;
payloads.push_back(payload0);
- OwningSerializedPacketPointer encrypted(
+ std::unique_ptr<SerializedPacket> encrypted(
creator_.SerializePathResponseConnectivityProbingPacket(payloads,
true));
{
@@ -1061,7 +1045,7 @@
QuicCircularDeque<QuicPathFrameBuffer> payloads;
payloads.push_back(payload0);
- OwningSerializedPacketPointer encrypted(
+ std::unique_ptr<SerializedPacket> encrypted(
creator_.SerializePathResponseConnectivityProbingPacket(payloads,
false));
{
@@ -1096,7 +1080,7 @@
payloads.push_back(payload0);
payloads.push_back(payload1);
- OwningSerializedPacketPointer encrypted(
+ std::unique_ptr<SerializedPacket> encrypted(
creator_.SerializePathResponseConnectivityProbingPacket(payloads,
true));
{
@@ -1133,7 +1117,7 @@
payloads.push_back(payload0);
payloads.push_back(payload1);
- OwningSerializedPacketPointer encrypted(
+ std::unique_ptr<SerializedPacket> encrypted(
creator_.SerializePathResponseConnectivityProbingPacket(payloads,
false));
{
@@ -1171,7 +1155,7 @@
payloads.push_back(payload1);
payloads.push_back(payload2);
- OwningSerializedPacketPointer encrypted(
+ std::unique_ptr<SerializedPacket> encrypted(
creator_.SerializePathResponseConnectivityProbingPacket(payloads,
true));
{
@@ -1211,7 +1195,7 @@
payloads.push_back(payload1);
payloads.push_back(payload2);
- OwningSerializedPacketPointer encrypted(
+ std::unique_ptr<SerializedPacket> encrypted(
creator_.SerializePathResponseConnectivityProbingPacket(payloads,
false));
InSequence s;
@@ -1364,7 +1348,6 @@
}
ProcessPacket(serialized);
EXPECT_EQ(GetParam().version_serialization, header.version_flag);
- DeleteFrames(&frames_);
}
TEST_P(QuicPacketCreatorTest, SerializeFrameShortData) {
@@ -1405,7 +1388,6 @@
}
ProcessPacket(serialized);
EXPECT_EQ(GetParam().version_serialization, header.version_flag);
- DeleteFrames(&frames_);
}
TEST_P(QuicPacketCreatorTest, ConsumeDataLargerThanOneStreamFrame) {
@@ -1491,13 +1473,14 @@
EXPECT_FALSE(creator_.AddFrame(QuicFrame(&ack_frame), NOT_RETRANSMISSION));
// Ensure the packet is successfully created.
- ASSERT_TRUE(serialized_packet_.encrypted_buffer);
- ASSERT_FALSE(serialized_packet_.retransmittable_frames.empty());
- const QuicFrames& retransmittable = serialized_packet_.retransmittable_frames;
+ ASSERT_TRUE(serialized_packet_->encrypted_buffer);
+ ASSERT_FALSE(serialized_packet_->retransmittable_frames.empty());
+ const QuicFrames& retransmittable =
+ serialized_packet_->retransmittable_frames;
ASSERT_EQ(1u, retransmittable.size());
EXPECT_EQ(STREAM_FRAME, retransmittable[0].type);
- EXPECT_TRUE(serialized_packet_.has_ack);
- EXPECT_EQ(QuicPacketNumber(10u), serialized_packet_.largest_acked);
+ EXPECT_TRUE(serialized_packet_->has_ack);
+ EXPECT_EQ(QuicPacketNumber(10u), serialized_packet_->largest_acked);
DeleteSerializedPacket();
EXPECT_FALSE(creator_.HasPendingFrames());
@@ -1536,9 +1519,10 @@
EXPECT_EQ(4u, num_bytes_consumed);
// Ensure the packet is successfully created.
- ASSERT_TRUE(serialized_packet_.encrypted_buffer);
- ASSERT_FALSE(serialized_packet_.retransmittable_frames.empty());
- const QuicFrames& retransmittable = serialized_packet_.retransmittable_frames;
+ ASSERT_TRUE(serialized_packet_->encrypted_buffer);
+ ASSERT_FALSE(serialized_packet_->retransmittable_frames.empty());
+ const QuicFrames& retransmittable =
+ serialized_packet_->retransmittable_frames;
ASSERT_EQ(1u, retransmittable.size());
EXPECT_EQ(STREAM_FRAME, retransmittable[0].type);
DeleteSerializedPacket();
@@ -1568,8 +1552,8 @@
EXPECT_EQ(1u, num_bytes_consumed);
// Check that a packet is created.
- ASSERT_TRUE(serialized_packet_.encrypted_buffer);
- ASSERT_FALSE(serialized_packet_.retransmittable_frames.empty());
+ ASSERT_TRUE(serialized_packet_->encrypted_buffer);
+ ASSERT_FALSE(serialized_packet_->retransmittable_frames.empty());
{
InSequence s;
EXPECT_CALL(framer_visitor_, OnPacket());
@@ -1583,7 +1567,7 @@
}
EXPECT_CALL(framer_visitor_, OnPacketComplete());
}
- ProcessPacket(serialized_packet_);
+ ProcessPacket(*serialized_packet_);
}
TEST_P(QuicPacketCreatorTest, AddUnencryptedStreamDataClosesConnection) {
@@ -1672,7 +1656,7 @@
EXPECT_CALL(framer_visitor_, OnPacketComplete());
}
// Packet only contains padding.
- ProcessPacket(serialized_packet_);
+ ProcessPacket(*serialized_packet_);
}
EXPECT_EQ(0u, creator_.pending_padding_bytes());
}
@@ -1754,9 +1738,8 @@
/*needs_full_padding=*/true, NOT_RETRANSMISSION, &frame));
EXPECT_CALL(delegate_, OnSerializedPacket(_))
- .WillOnce(Invoke([expected_buffer](SerializedPacket* serialized_packet) {
- EXPECT_EQ(expected_buffer, serialized_packet->encrypted_buffer);
- ClearSerializedPacket(serialized_packet);
+ .WillOnce(Invoke([expected_buffer](SerializedPacket serialized_packet) {
+ EXPECT_EQ(expected_buffer, serialized_packet.encrypted_buffer);
}));
creator_.FlushCurrentPacket();
}
@@ -1871,7 +1854,7 @@
EXPECT_CALL(delegate_, OnSerializedPacket(_))
.WillOnce(Invoke(this, &QuicPacketCreatorTest::SaveSerializedPacket));
creator_.FlushCurrentPacket();
- ASSERT_TRUE(serialized_packet_.encrypted_buffer);
+ ASSERT_TRUE(serialized_packet_->encrypted_buffer);
DeleteSerializedPacket();
}
}
@@ -1987,18 +1970,18 @@
.WillOnce(Invoke(this, &QuicPacketCreatorTest::SaveSerializedPacket));
EXPECT_TRUE(creator_.AddFrame(ack_frame, LOSS_RETRANSMISSION));
- ASSERT_FALSE(serialized_packet_.encrypted_buffer);
+ ASSERT_EQ(serialized_packet_, nullptr);
EXPECT_TRUE(creator_.AddFrame(stream_frame, RTO_RETRANSMISSION));
- ASSERT_FALSE(serialized_packet_.encrypted_buffer);
+ ASSERT_EQ(serialized_packet_, nullptr);
EXPECT_TRUE(creator_.AddFrame(padding_frame, TLP_RETRANSMISSION));
creator_.FlushCurrentPacket();
- ASSERT_TRUE(serialized_packet_.encrypted_buffer);
+ ASSERT_TRUE(serialized_packet_->encrypted_buffer);
// The last retransmittable frame on packet is a stream frame, the packet's
// transmission type should be the same as the stream frame's.
- EXPECT_EQ(serialized_packet_.transmission_type, RTO_RETRANSMISSION);
+ EXPECT_EQ(serialized_packet_->transmission_type, RTO_RETRANSMISSION);
DeleteSerializedPacket();
}
@@ -2053,7 +2036,6 @@
quiche::test::CompareCharArraysWithHexError(
"retry token", header.retry_token.data(), header.retry_token.length(),
retry_token_bytes, sizeof(retry_token_bytes));
- DeleteFrames(&frames_);
}
TEST_P(QuicPacketCreatorTest, GetConnectionId) {
@@ -2149,7 +2131,7 @@
EXPECT_CALL(framer_visitor_, OnStreamFrame(_));
EXPECT_CALL(framer_visitor_, OnStreamFrame(_));
EXPECT_CALL(framer_visitor_, OnPacketComplete());
- ProcessPacket(serialized_packet_);
+ ProcessPacket(*serialized_packet_);
}
TEST_P(QuicPacketCreatorTest, SaveNonRetransmittableFrames) {
@@ -2330,7 +2312,7 @@
(),
(override));
MOCK_METHOD(char*, GetPacketBuffer, (), (override));
- MOCK_METHOD(void, OnSerializedPacket, (SerializedPacket*), (override));
+ MOCK_METHOD(void, OnSerializedPacket, (SerializedPacket), (override));
MOCK_METHOD(void,
OnUnrecoverableError,
(QuicErrorCode, const std::string&),
@@ -2498,18 +2480,13 @@
creator_.AttachPacketFlusher();
}
- ~QuicPacketCreatorMultiplePacketsTest() override {
- for (SerializedPacket& packet : packets_) {
- delete[] packet.encrypted_buffer;
- ClearSerializedPacket(&packet);
- }
- }
+ ~QuicPacketCreatorMultiplePacketsTest() override {}
- void SavePacket(SerializedPacket* packet) {
- packet->encrypted_buffer = CopyBuffer(*packet);
- packets_.push_back(*packet);
- packet->encrypted_buffer = nullptr;
- packet->retransmittable_frames.clear();
+ void SavePacket(SerializedPacket packet) {
+ DCHECK(packet.release_encrypted_buffer == nullptr);
+ packet.encrypted_buffer = CopyBuffer(packet);
+ packet.release_encrypted_buffer = [](const char* p) { delete[] p; };
+ packets_.push_back(std::move(packet));
}
protected:
@@ -2994,7 +2971,7 @@
contents.num_stream_frames = 1;
CheckPacketContains(contents, 0);
EXPECT_FALSE(packets_.empty());
- SerializedPacket packet = packets_.back();
+ SerializedPacket& packet = packets_.back();
EXPECT_TRUE(!packet.retransmittable_frames.empty());
EXPECT_EQ(LOSS_RETRANSMISSION, packet.transmission_type);
EXPECT_EQ(STREAM_FRAME, packet.retransmittable_frames.front().type);
@@ -3024,7 +3001,7 @@
contents.num_stream_frames = 1;
CheckPacketContains(contents, 0);
EXPECT_FALSE(packets_.empty());
- SerializedPacket packet = packets_.back();
+ SerializedPacket& packet = packets_.back();
EXPECT_TRUE(!packet.retransmittable_frames.empty());
EXPECT_EQ(STREAM_FRAME, packet.retransmittable_frames.front().type);
const QuicStreamFrame& stream_frame =
@@ -3067,7 +3044,7 @@
EXPECT_FALSE(creator_.HasPendingRetransmittableFrames());
EXPECT_FALSE(packets_.empty());
- SerializedPacket packet = packets_.back();
+ SerializedPacket& packet = packets_.back();
EXPECT_TRUE(!packet.retransmittable_frames.empty());
EXPECT_EQ(STREAM_FRAME, packet.retransmittable_frames.front().type);
const QuicStreamFrame& stream_frame =
@@ -3096,7 +3073,7 @@
EXPECT_FALSE(creator_.HasPendingRetransmittableFrames());
EXPECT_FALSE(packets_.empty());
- SerializedPacket packet = packets_.back();
+ SerializedPacket& packet = packets_.back();
EXPECT_TRUE(!packet.retransmittable_frames.empty());
EXPECT_EQ(STREAM_FRAME, packet.retransmittable_frames.front().type);
const QuicStreamFrame& stream_frame =
@@ -3443,7 +3420,7 @@
GenerateConnectivityProbingPacket) {
delegate_.SetCanWriteAnything();
- OwningSerializedPacketPointer probing_packet;
+ std::unique_ptr<SerializedPacket> probing_packet;
if (VersionHasIetfQuicFrames(framer_.transport_version())) {
QuicPathFrameBuffer payload = {
{0xde, 0xad, 0xbe, 0xef, 0xba, 0xdc, 0x0f, 0xfe}};
diff --git a/quic/core/quic_packets.cc b/quic/core/quic_packets.cc
index 4123c27..69575b9 100644
--- a/quic/core/quic_packets.cc
+++ b/quic/core/quic_packets.cc
@@ -465,15 +465,8 @@
transmission_type(NOT_RETRANSMISSION),
has_ack_frame_copy(false) {}
-SerializedPacket::SerializedPacket(const SerializedPacket& other) = default;
-
-SerializedPacket& SerializedPacket::operator=(const SerializedPacket& other) =
- default;
-
SerializedPacket::SerializedPacket(SerializedPacket&& other)
- : encrypted_buffer(other.encrypted_buffer),
- encrypted_length(other.encrypted_length),
- has_crypto_handshake(other.has_crypto_handshake),
+ : has_crypto_handshake(other.has_crypto_handshake),
num_padding_bytes(other.num_padding_bytes),
packet_number(other.packet_number),
packet_number_length(other.packet_number_length),
@@ -483,23 +476,58 @@
transmission_type(other.transmission_type),
largest_acked(other.largest_acked),
has_ack_frame_copy(other.has_ack_frame_copy) {
- retransmittable_frames.swap(other.retransmittable_frames);
- nonretransmittable_frames.swap(other.nonretransmittable_frames);
+ if (this != &other) {
+ if (release_encrypted_buffer && encrypted_buffer != nullptr) {
+ release_encrypted_buffer(encrypted_buffer);
+ }
+ encrypted_buffer = other.encrypted_buffer;
+ encrypted_length = other.encrypted_length;
+ release_encrypted_buffer = std::move(other.release_encrypted_buffer);
+ other.release_encrypted_buffer = nullptr;
+
+ retransmittable_frames.swap(other.retransmittable_frames);
+ nonretransmittable_frames.swap(other.nonretransmittable_frames);
+ }
}
-SerializedPacket::~SerializedPacket() {}
+SerializedPacket::~SerializedPacket() {
+ if (release_encrypted_buffer && encrypted_buffer != nullptr) {
+ release_encrypted_buffer(encrypted_buffer);
+ }
+
+ if (!retransmittable_frames.empty()) {
+ DeleteFrames(&retransmittable_frames);
+ }
+ for (auto& frame : nonretransmittable_frames) {
+ if (!has_ack_frame_copy && frame.type == ACK_FRAME) {
+ // Do not delete ack frame if the packet does not own a copy of it.
+ continue;
+ }
+ DeleteFrame(&frame);
+ }
+}
SerializedPacket* CopySerializedPacket(const SerializedPacket& serialized,
QuicBufferAllocator* allocator,
bool copy_buffer) {
- SerializedPacket* copy = new SerializedPacket(serialized);
+ SerializedPacket* copy = new SerializedPacket(
+ serialized.packet_number, serialized.packet_number_length,
+ serialized.encrypted_buffer, serialized.encrypted_length,
+ serialized.has_ack, serialized.has_stop_waiting);
+ copy->has_crypto_handshake = serialized.has_crypto_handshake;
+ copy->num_padding_bytes = serialized.num_padding_bytes;
+ copy->encryption_level = serialized.encryption_level;
+ copy->transmission_type = serialized.transmission_type;
+ copy->largest_acked = serialized.largest_acked;
+
if (copy_buffer) {
copy->encrypted_buffer = CopyBuffer(serialized);
+ copy->release_encrypted_buffer = [](const char* p) { delete[] p; };
}
// Copy underlying frames.
copy->retransmittable_frames =
CopyQuicFrames(allocator, serialized.retransmittable_frames);
- copy->nonretransmittable_frames.clear();
+ DCHECK(copy->nonretransmittable_frames.empty());
for (const auto& frame : serialized.nonretransmittable_frames) {
if (frame.type == ACK_FRAME) {
copy->has_ack_frame_copy = true;
@@ -509,27 +537,8 @@
return copy;
}
-void ClearSerializedPacket(SerializedPacket* serialized_packet) {
- if (!serialized_packet->retransmittable_frames.empty()) {
- DeleteFrames(&serialized_packet->retransmittable_frames);
- }
- for (auto& frame : serialized_packet->nonretransmittable_frames) {
- if (!serialized_packet->has_ack_frame_copy && frame.type == ACK_FRAME) {
- // Do not delete ack frame if the packet does not own a copy of it.
- continue;
- }
- DeleteFrame(&frame);
- }
- serialized_packet->nonretransmittable_frames.clear();
- serialized_packet->encrypted_buffer = nullptr;
- serialized_packet->encrypted_length = 0;
- serialized_packet->largest_acked.Clear();
-}
-
char* CopyBuffer(const SerializedPacket& packet) {
- char* dst_buffer = new char[packet.encrypted_length];
- memcpy(dst_buffer, packet.encrypted_buffer, packet.encrypted_length);
- return dst_buffer;
+ return CopyBuffer(packet.encrypted_buffer, packet.encrypted_length);
}
char* CopyBuffer(const char* encrypted_buffer,
diff --git a/quic/core/quic_packets.h b/quic/core/quic_packets.h
index e8e1931..04522e8 100644
--- a/quic/core/quic_packets.h
+++ b/quic/core/quic_packets.h
@@ -357,6 +357,13 @@
bool owns_header_buffer_;
};
+// SerializedPacket contains information of a serialized(encrypted) packet.
+//
+// WARNING:
+//
+// If you add a member field to this class, please make sure it is properly
+// copied in |CopySerializedPacket|.
+//
struct QUIC_EXPORT_PRIVATE SerializedPacket {
SerializedPacket(QuicPacketNumber packet_number,
QuicPacketNumberLength packet_number_length,
@@ -364,14 +371,20 @@
QuicPacketLength encrypted_length,
bool has_ack,
bool has_stop_waiting);
- SerializedPacket(const SerializedPacket& other);
- SerializedPacket& operator=(const SerializedPacket& other);
+
+ // Copy constructor & assignment are deleted. Use |CopySerializedPacket| to
+ // make a copy.
+ SerializedPacket(const SerializedPacket& other) = delete;
+ SerializedPacket& operator=(const SerializedPacket& other) = delete;
SerializedPacket(SerializedPacket&& other);
~SerializedPacket();
- // Not owned.
+ // Not owned if |release_encrypted_buffer| is nullptr. Otherwise it is
+ // released by |release_encrypted_buffer| on destruction.
const char* encrypted_buffer;
QuicPacketLength encrypted_length;
+ std::function<void(const char*)> release_encrypted_buffer;
+
QuicFrames retransmittable_frames;
QuicFrames nonretransmittable_frames;
IsHandshake has_crypto_handshake;
@@ -401,10 +414,6 @@
QuicBufferAllocator* allocator,
bool copy_buffer);
-// Deletes and clears all the frames and the packet from serialized packet.
-QUIC_EXPORT_PRIVATE void ClearSerializedPacket(
- SerializedPacket* serialized_packet);
-
// Allocates a new char[] of size |packet.encrypted_length| and copies in
// |packet.encrypted_buffer|.
QUIC_EXPORT_PRIVATE char* CopyBuffer(const SerializedPacket& packet);
@@ -413,21 +422,6 @@
QUIC_EXPORT_PRIVATE char* CopyBuffer(const char* encrypted_buffer,
QuicPacketLength encrypted_length);
-struct QUIC_EXPORT_PRIVATE SerializedPacketDeleter {
- void operator()(SerializedPacket* packet) {
- if (packet->encrypted_buffer != nullptr) {
- delete[] packet->encrypted_buffer;
- }
- delete packet;
- }
-};
-
-// On destruction, OwningSerializedPacketPointer deletes a packet's (on-heap)
-// encrypted_buffer before deleting the (also on-heap) packet itself.
-// TODO(wub): Maybe delete retransmittable_frames too?
-typedef std::unique_ptr<SerializedPacket, SerializedPacketDeleter>
- OwningSerializedPacketPointer;
-
// Context for an incoming packet.
struct QUIC_EXPORT_PRIVATE QuicPerPacketContext {
virtual ~QuicPerPacketContext() {}
diff --git a/quic/core/quic_packets_test.cc b/quic/core/quic_packets_test.cc
index 1ccaabc..b2bccbe 100644
--- a/quic/core/quic_packets_test.cc
+++ b/quic/core/quic_packets_test.cc
@@ -106,10 +106,6 @@
CopySerializedPacket(packet, &allocator, /*copy_buffer=*/false));
EXPECT_EQ(packet.encrypted_buffer, copy2->encrypted_buffer);
EXPECT_EQ(1000u, copy2->encrypted_length);
- ClearSerializedPacket(&packet);
- delete[] copy->encrypted_buffer;
- ClearSerializedPacket(copy.get());
- ClearSerializedPacket(copy2.get());
}
} // namespace
