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}

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);
   }

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

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

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);
   }

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();
 

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();

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