diff options
author | rtenneti <rtenneti@chromium.org> | 2015-10-13 17:07:00 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-10-14 00:08:28 +0000 |
commit | f60c32d8eab385d3ae4e1277c4f77b0c644fe9cd (patch) | |
tree | 1c384491bee36352f44bc611a3c100aedc08024c /net | |
parent | d0653bf3cf98689af2bc85b035c3cb3d8aa0de8f (diff) | |
download | chromium_src-f60c32d8eab385d3ae4e1277c4f77b0c644fe9cd.zip chromium_src-f60c32d8eab385d3ae4e1277c4f77b0c644fe9cd.tar.gz chromium_src-f60c32d8eab385d3ae4e1277c4f77b0c644fe9cd.tar.bz2 |
relnote: Refactor stream buffer allocation out into a scoped_ptr.
No functional change.
This is intended to support free listing the stream buffer allocations.
Merge internal change: 103088826
R=rch@chromium.org
Review URL: https://codereview.chromium.org/1397113003
Cr-Commit-Position: refs/heads/master@{#353917}
Diffstat (limited to 'net')
-rw-r--r-- | net/quic/quic_packet_creator.cc | 17 | ||||
-rw-r--r-- | net/quic/quic_packet_creator.h | 15 | ||||
-rw-r--r-- | net/quic/quic_packet_creator_test.cc | 30 | ||||
-rw-r--r-- | net/quic/quic_packet_generator.cc | 12 | ||||
-rw-r--r-- | net/quic/quic_packet_generator.h | 7 | ||||
-rw-r--r-- | net/quic/quic_protocol.cc | 10 | ||||
-rw-r--r-- | net/quic/quic_protocol.h | 12 |
7 files changed, 59 insertions, 44 deletions
diff --git a/net/quic/quic_packet_creator.cc b/net/quic/quic_packet_creator.cc index 3dcfe6e..18a719a 100644 --- a/net/quic/quic_packet_creator.cc +++ b/net/quic/quic_packet_creator.cc @@ -266,7 +266,7 @@ size_t QuicPacketCreator::CreateStreamFrame(QuicStreamId id, QuicStreamOffset offset, bool fin, QuicFrame* frame, - scoped_ptr<char[]>* buffer) { + UniqueStreamBuffer* buffer) { DCHECK_GT(max_packet_length_, StreamFramePacketOverhead(connection_id_length_, kIncludeVersion, PACKET_6BYTE_PACKET_NUMBER, offset, @@ -294,7 +294,7 @@ size_t QuicPacketCreator::CreateStreamFrame(QuicStreamId id, size_t bytes_consumed = min<size_t>(BytesFree() - min_frame_size, data_size); bool set_fin = fin && bytes_consumed == data_size; // Last frame. - buffer->reset(new char[bytes_consumed]); + *buffer = NewStreamBuffer(bytes_consumed); CopyToBuffer(iov, iov_offset, bytes_consumed, buffer->get()); *frame = QuicFrame(new QuicStreamFrame( id, set_fin, offset, StringPiece(buffer->get(), bytes_consumed))); @@ -417,17 +417,18 @@ bool QuicPacketCreator::AddSavedFrame(const QuicFrame& frame) { /*needs_padding=*/false, nullptr); } -bool QuicPacketCreator::AddSavedFrame(const QuicFrame& frame, char* buffer) { +bool QuicPacketCreator::AddSavedFrame(const QuicFrame& frame, + UniqueStreamBuffer buffer) { return AddFrame(frame, /*save_retransmittable_frames=*/true, - /*needs_padding=*/false, buffer); + /*needs_padding=*/false, buffer.Pass()); } bool QuicPacketCreator::AddPaddedSavedFrame(const QuicFrame& frame, - char* buffer) { + UniqueStreamBuffer buffer) { return AddFrame(frame, /*save_retransmittable_frames=*/true, - /*needs_padding=*/true, buffer); + /*needs_padding=*/true, buffer.Pass()); } SerializedPacket QuicPacketCreator::SerializePacket( @@ -596,7 +597,7 @@ bool QuicPacketCreator::ShouldRetransmit(const QuicFrame& frame) { bool QuicPacketCreator::AddFrame(const QuicFrame& frame, bool save_retransmittable_frames, bool needs_padding, - char* buffer) { + UniqueStreamBuffer buffer) { DVLOG(1) << "Adding frame: " << frame; InFecGroup is_in_fec_group = MaybeUpdateLengthsAndStartFec(); @@ -615,7 +616,7 @@ bool QuicPacketCreator::AddFrame(const QuicFrame& frame, new RetransmittableFrames(encryption_level_)); } queued_frames_.push_back( - queued_retransmittable_frames_->AddFrame(frame, buffer)); + queued_retransmittable_frames_->AddFrame(frame, buffer.Pass())); } else { queued_frames_.push_back(frame); } diff --git a/net/quic/quic_packet_creator.h b/net/quic/quic_packet_creator.h index ee6e35c..618e00b 100644 --- a/net/quic/quic_packet_creator.h +++ b/net/quic/quic_packet_creator.h @@ -89,7 +89,7 @@ class NET_EXPORT_PRIVATE QuicPacketCreator { QuicStreamOffset offset, bool fin, QuicFrame* frame, - scoped_ptr<char[]>* buffer); + UniqueStreamBuffer* buffer); // Serializes all frames into a single packet. All frames must fit into a // single packet. Also, sets the entropy hash of the serialized packet to a @@ -151,13 +151,12 @@ class NET_EXPORT_PRIVATE QuicPacketCreator { // Returns false if the frame doesn't fit into the current packet. bool AddSavedFrame(const QuicFrame& frame); - // Identical to AddSavedFrame, but takes ownership of the buffer if it returns - // true. - bool AddSavedFrame(const QuicFrame& frame, char* buffer); + // Identical to AddSavedFrame, but takes ownership of the buffer. + bool AddSavedFrame(const QuicFrame& frame, UniqueStreamBuffer buffer); - // Identical to AddSavedFrame, but takes ownership of the buffer if it returns - // true, and allows to cause the packet to be padded. - bool AddPaddedSavedFrame(const QuicFrame& frame, char* buffer); + // Identical to AddSavedFrame, but takes ownership of the buffer, and allows + // to cause the packet to be padded. + bool AddPaddedSavedFrame(const QuicFrame& frame, UniqueStreamBuffer buffer); // Serializes all frames which have been added and adds any which should be // retransmitted to |retransmittable_frames| if it's not nullptr. All frames @@ -264,7 +263,7 @@ class NET_EXPORT_PRIVATE QuicPacketCreator { bool AddFrame(const QuicFrame& frame, bool save_retransmittable_frames, bool needs_padding, - char* buffer); + UniqueStreamBuffer buffer); // Adds a padding frame to the current packet only if the current packet // contains a handshake message, and there is sufficient room to fit a diff --git a/net/quic/quic_packet_creator_test.cc b/net/quic/quic_packet_creator_test.cc index 962c1b5..c0565ed 100644 --- a/net/quic/quic_packet_creator_test.cc +++ b/net/quic/quic_packet_creator_test.cc @@ -491,7 +491,7 @@ TEST_P(QuicPacketCreatorTest, ReserializeFramesWithSequenceNumberLength) { TEST_P(QuicPacketCreatorTest, ReserializeFramesWithPadding) { QuicFrame frame; QuicIOVector io_vector(MakeIOVector("fake handshake message data")); - scoped_ptr<char[]> stream_buffer; + UniqueStreamBuffer stream_buffer; creator_.CreateStreamFrame(kCryptoStreamId, io_vector, 0u, 0u, false, &frame, &stream_buffer); RetransmittableFrames frames(ENCRYPTION_NONE); @@ -515,7 +515,7 @@ TEST_P(QuicPacketCreatorTest, ReserializeFramesWithFullPacketAndPadding) { QuicFrame frame; QuicIOVector io_vector(MakeIOVector(data)); - scoped_ptr<char[]> stream_buffer; + UniqueStreamBuffer stream_buffer; creator_.CreateStreamFrame(kCryptoStreamId, io_vector, 0, kOffset, false, &frame, &stream_buffer); RetransmittableFrames frames(ENCRYPTION_NONE); @@ -623,7 +623,7 @@ TEST_P(QuicPacketCreatorTest, SwitchFecOnWithStreamFrameQueued) { // Add a stream frame to the creator. QuicFrame frame; QuicIOVector io_vector(MakeIOVector("test")); - scoped_ptr<char[]> stream_buffer; + UniqueStreamBuffer stream_buffer; size_t consumed = creator_.CreateStreamFrame(1u, io_vector, 0u, 0u, false, &frame, &stream_buffer); EXPECT_EQ(4u, consumed); @@ -654,7 +654,7 @@ TEST_P(QuicPacketCreatorTest, SwitchFecOnWithStreamFrameQueued) { TEST_P(QuicPacketCreatorTest, CreateStreamFrame) { QuicFrame frame; QuicIOVector io_vector(MakeIOVector("test")); - scoped_ptr<char[]> stream_buffer; + UniqueStreamBuffer stream_buffer; size_t consumed = creator_.CreateStreamFrame(1u, io_vector, 0u, 0u, false, &frame, &stream_buffer); EXPECT_EQ(4u, consumed); @@ -666,7 +666,7 @@ TEST_P(QuicPacketCreatorTest, CreateStreamFrame) { TEST_P(QuicPacketCreatorTest, CreateStreamFrameFin) { QuicFrame frame; QuicIOVector io_vector(MakeIOVector("test")); - scoped_ptr<char[]> stream_buffer; + UniqueStreamBuffer stream_buffer; size_t consumed = creator_.CreateStreamFrame(1u, io_vector, 0u, 10u, true, &frame, &stream_buffer); EXPECT_EQ(4u, consumed); @@ -678,7 +678,7 @@ TEST_P(QuicPacketCreatorTest, CreateStreamFrameFin) { TEST_P(QuicPacketCreatorTest, CreateStreamFrameFinOnly) { QuicFrame frame; QuicIOVector io_vector(nullptr, 0, 0); - scoped_ptr<char[]> stream_buffer; + UniqueStreamBuffer stream_buffer; size_t consumed = creator_.CreateStreamFrame(1u, io_vector, 0u, 0u, true, &frame, &stream_buffer); EXPECT_EQ(0u, consumed); @@ -698,7 +698,7 @@ TEST_P(QuicPacketCreatorTest, CreateAllFreeBytesForStreamFrames) { if (should_have_room) { QuicFrame frame; QuicIOVector io_vector(MakeIOVector("testdata")); - scoped_ptr<char[]> stream_buffer; + UniqueStreamBuffer stream_buffer; size_t bytes_consumed = creator_.CreateStreamFrame(kClientDataStreamId1, io_vector, 0u, kOffset, false, &frame, &stream_buffer); @@ -725,7 +725,7 @@ TEST_P(QuicPacketCreatorTest, StreamFrameConsumption) { size_t bytes_free = delta > 0 ? 0 : 0 - delta; QuicFrame frame; QuicIOVector io_vector(MakeIOVector(data)); - scoped_ptr<char[]> stream_buffer; + UniqueStreamBuffer stream_buffer; size_t bytes_consumed = creator_.CreateStreamFrame(kClientDataStreamId1, io_vector, 0u, kOffset, false, &frame, &stream_buffer); @@ -759,7 +759,7 @@ TEST_P(QuicPacketCreatorTest, StreamFrameConsumptionWithFec) { size_t bytes_free = delta > 0 ? 0 : 0 - delta; QuicFrame frame; QuicIOVector io_vector(MakeIOVector(data)); - scoped_ptr<char[]> stream_buffer; + UniqueStreamBuffer stream_buffer; size_t bytes_consumed = creator_.CreateStreamFrame(kClientDataStreamId1, io_vector, 0u, kOffset, false, &frame, &stream_buffer); @@ -794,7 +794,7 @@ TEST_P(QuicPacketCreatorTest, CryptoStreamFramePacketPadding) { QuicFrame frame; QuicIOVector io_vector(MakeIOVector(data)); - scoped_ptr<char[]> stream_buffer; + UniqueStreamBuffer stream_buffer; size_t bytes_consumed = creator_.CreateStreamFrame( kCryptoStreamId, io_vector, 0u, kOffset, false, &frame, &stream_buffer); EXPECT_LT(0u, bytes_consumed); @@ -830,7 +830,7 @@ TEST_P(QuicPacketCreatorTest, NonCryptoStreamFramePacketNonPadding) { QuicFrame frame; QuicIOVector io_vector(MakeIOVector(data)); - scoped_ptr<char[]> stream_buffer; + UniqueStreamBuffer stream_buffer; size_t bytes_consumed = creator_.CreateStreamFrame(kClientDataStreamId1, io_vector, 0u, kOffset, false, &frame, &stream_buffer); @@ -966,7 +966,7 @@ TEST_P(QuicPacketCreatorTest, CreateStreamFrameTooLarge) { QuicFrame frame; const string too_long_payload(payload_length * 2, 'a'); QuicIOVector io_vector(MakeIOVector(too_long_payload)); - scoped_ptr<char[]> stream_buffer; + UniqueStreamBuffer stream_buffer; size_t consumed = creator_.CreateStreamFrame(1u, io_vector, 0u, 0u, true, &frame, &stream_buffer); EXPECT_EQ(payload_length, consumed); @@ -997,7 +997,7 @@ TEST_P(QuicPacketCreatorTest, AddFrameAndSerialize) { QuicFrame frame; QuicIOVector io_vector(MakeIOVector("test")); - scoped_ptr<char[]> stream_buffer; + UniqueStreamBuffer stream_buffer; size_t consumed = creator_.CreateStreamFrame(1u, io_vector, 0u, 0u, false, &frame, &stream_buffer); EXPECT_EQ(4u, consumed); @@ -1058,7 +1058,7 @@ TEST_P(QuicPacketCreatorTest, SerializeTruncatedAckFrameWithLargePacketSize) { // Make sure that an additional stream frame can be added to the packet. QuicFrame stream_frame; QuicIOVector io_vector(MakeIOVector("test")); - scoped_ptr<char[]> stream_buffer; + UniqueStreamBuffer stream_buffer; size_t consumed = creator_.CreateStreamFrame(2u, io_vector, 0u, 0u, false, &stream_frame, &stream_buffer); EXPECT_EQ(4u, consumed); @@ -1204,7 +1204,7 @@ TEST_P(QuicPacketCreatorTest, ResetFecGroupWithQueuedFrames) { // Add a stream frame to the creator. QuicFrame frame; QuicIOVector io_vector(MakeIOVector("test")); - scoped_ptr<char[]> stream_buffer; + UniqueStreamBuffer stream_buffer; size_t consumed = creator_.CreateStreamFrame(1u, io_vector, 0u, 0u, false, &frame, &stream_buffer); EXPECT_EQ(4u, consumed); diff --git a/net/quic/quic_packet_generator.cc b/net/quic/quic_packet_generator.cc index 1c306c4..eed8ec7 100644 --- a/net/quic/quic_packet_generator.cc +++ b/net/quic/quic_packet_generator.cc @@ -168,7 +168,7 @@ QuicConsumedData QuicPacketGenerator::ConsumeData( while (delegate_->ShouldGeneratePacket( HAS_RETRANSMITTABLE_DATA, has_handshake ? IS_HANDSHAKE : NOT_HANDSHAKE)) { QuicFrame frame; - scoped_ptr<char[]> buffer; + UniqueStreamBuffer buffer; size_t bytes_consumed = packet_creator_.CreateStreamFrame( id, iov, total_bytes_consumed, offset + total_bytes_consumed, fin, &frame, &buffer); @@ -179,7 +179,7 @@ QuicConsumedData QuicPacketGenerator::ConsumeData( ack_notifiers_.push_back(notifier); } - if (!AddFrame(frame, buffer.get(), has_handshake)) { + if (!AddFrame(frame, buffer.Pass(), has_handshake)) { LOG(DFATAL) << "Failed to add stream frame."; // Inability to add a STREAM frame creates an unrecoverable hole in a // the stream, so it's best to close the connection. @@ -187,8 +187,6 @@ QuicConsumedData QuicPacketGenerator::ConsumeData( delete notifier; return QuicConsumedData(0, false); } - // When AddFrame succeeds, it takes ownership of the buffer. - ignore_result(buffer.release()); total_bytes_consumed += bytes_consumed; fin_consumed = fin && total_bytes_consumed == iov.total_length; @@ -442,11 +440,11 @@ bool QuicPacketGenerator::AddNextPendingFrame() { } bool QuicPacketGenerator::AddFrame(const QuicFrame& frame, - char* buffer, + UniqueStreamBuffer buffer, bool needs_padding) { bool success = needs_padding - ? packet_creator_.AddPaddedSavedFrame(frame, buffer) - : packet_creator_.AddSavedFrame(frame, buffer); + ? packet_creator_.AddPaddedSavedFrame(frame, buffer.Pass()) + : packet_creator_.AddSavedFrame(frame, buffer.Pass()); if (success && debug_delegate_) { debug_delegate_->OnFrameAddedToPacket(frame); } diff --git a/net/quic/quic_packet_generator.h b/net/quic/quic_packet_generator.h index 923d01e..f782e3c 100644 --- a/net/quic/quic_packet_generator.h +++ b/net/quic/quic_packet_generator.h @@ -258,9 +258,10 @@ class NET_EXPORT_PRIVATE QuicPacketGenerator { bool CanSendWithNextPendingFrameAddition() const; // Add exactly one pending frame, preferring ack frames over control frames. bool AddNextPendingFrame(); - // Adds a frame and takes ownership of the underlying buffer if the addition - // was successful. - bool AddFrame(const QuicFrame& frame, char* buffer, bool needs_padding); + // Adds a frame and takes ownership of the underlying buffer. + bool AddFrame(const QuicFrame& frame, + UniqueStreamBuffer buffer, + bool needs_padding); void SerializeAndSendPacket(); diff --git a/net/quic/quic_protocol.cc b/net/quic/quic_protocol.cc index 5625d6c..d281358 100644 --- a/net/quic/quic_protocol.cc +++ b/net/quic/quic_protocol.cc @@ -91,6 +91,10 @@ QuicPublicResetPacket::QuicPublicResetPacket( const QuicPacketPublicHeader& header) : public_header(header), nonce_proof(0), rejected_packet_number(0) {} +UniqueStreamBuffer NewStreamBuffer(size_t size) { + return UniqueStreamBuffer(new char[size]); +} + QuicStreamFrame::QuicStreamFrame() : stream_id(0), fin(false), offset(0) { } @@ -820,6 +824,8 @@ RetransmittableFrames::~RetransmittableFrames() { DCHECK(false) << "Cannot delete type: " << it->type; } } + // TODO(rtenneti): Delete the for loop once chrome has c++11 library support + // for "std::vector<UniqueStreamBuffer> stream_data_;". for (const char* buffer : stream_data_) { delete[] buffer; } @@ -830,13 +836,13 @@ const QuicFrame& RetransmittableFrames::AddFrame(const QuicFrame& frame) { } const QuicFrame& RetransmittableFrames::AddFrame(const QuicFrame& frame, - char* buffer) { + UniqueStreamBuffer buffer) { if (frame.type == STREAM_FRAME && frame.stream_frame->stream_id == kCryptoStreamId) { has_crypto_handshake_ = IS_HANDSHAKE; } if (buffer != nullptr) { - stream_data_.push_back(buffer); + stream_data_.push_back(buffer.release()); } frames_.push_back(frame); return frames_.back(); diff --git a/net/quic/quic_protocol.h b/net/quic/quic_protocol.h index 800b349..c942b8d 100644 --- a/net/quic/quic_protocol.h +++ b/net/quic/quic_protocol.h @@ -19,6 +19,7 @@ #include "base/basictypes.h" #include "base/containers/hash_tables.h" #include "base/logging.h" +#include "base/memory/scoped_ptr.h" #include "base/strings/string_piece.h" #include "net/base/int128.h" #include "net/base/iovec.h" @@ -696,6 +697,11 @@ struct NET_EXPORT_PRIVATE QuicPingFrame { // frame. struct NET_EXPORT_PRIVATE QuicMtuDiscoveryFrame {}; +typedef scoped_ptr<char[]> UniqueStreamBuffer; + +// Allocates memory of size |size| for a QUIC stream buffer. +UniqueStreamBuffer NewStreamBuffer(size_t size); + struct NET_EXPORT_PRIVATE QuicStreamFrame { QuicStreamFrame(); QuicStreamFrame(const QuicStreamFrame& frame); @@ -1108,7 +1114,7 @@ class NET_EXPORT_PRIVATE RetransmittableFrames { // Takes ownership of the frame inside |frame|. const QuicFrame& AddFrame(const QuicFrame& frame); // Takes ownership of the frame inside |frame| and |buffer|. - const QuicFrame& AddFrame(const QuicFrame& frame, char* buffer); + const QuicFrame& AddFrame(const QuicFrame& frame, UniqueStreamBuffer buffer); // Removes all stream frames associated with |stream_id|. void RemoveFramesForStream(QuicStreamId stream_id); @@ -1132,6 +1138,10 @@ class NET_EXPORT_PRIVATE RetransmittableFrames { IsHandshake has_crypto_handshake_; bool needs_padding_; // Data referenced by the StringPiece of a QuicStreamFrame. + // + // TODO(rtenneti): Change const char* to UniqueStreamBuffer once chrome has + // c++11 library support. + // std::vector<UniqueStreamBuffer> stream_data_; std::vector<const char*> stream_data_; DISALLOW_COPY_AND_ASSIGN(RetransmittableFrames); |