Use base::MakeRefCounted instead of new to alloc net::IOBuffer instances.

This CL only handles the difficult cases in //remoting. The easy cases
were tackled by mechanical CLs, such as https://crrev.com/c/1188959 for
//net. Parallel CLs will tackle other difficult cases.

net::IOBuffer is (thread-safe) ref-counted. Asides from improving the
ability to reason about instance ownership locally, creating instances
via base::MakeRefCounted makes it possible to use 1-based ref-counting
in the future (see base/memory/ref_counted.h).

This CL replaces raw IOBuffer pointers with scoped_refptr<net::IOBuffer>
in some APIs. Local inspection of the impacted implementations should
reveal that no extra reference churn was introduced.

Change-Id: I59940c49e6635540a4a42840919ff114b3a1941f
Reviewed-on: https://chromium-review.googlesource.com/1215463Reviewed-by: Gary Kacmarcik <garykac@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590076}

remoting/base/compound_buffer.cc

@@ -12,12 +12,10 @@
 
 namespace remoting {
 
-CompoundBuffer::DataChunk::DataChunk(
-    net::IOBuffer* buffer_value, const char* start_value, int size_value)
-    : buffer(buffer_value),
-      start(start_value),
-      size(size_value) {
-}
+CompoundBuffer::DataChunk::DataChunk(scoped_refptr<net::IOBuffer> buffer,
+                                     const char* start,
+                                     int size)
+    : buffer(std::move(buffer)), start(start), size(size) {}
 
 CompoundBuffer::DataChunk::DataChunk(const DataChunk& other) = default;
 
@@ -36,61 +34,67 @@ void CompoundBuffer::Clear() {
   total_bytes_ = 0;
 }
 
-void CompoundBuffer::Append(net::IOBuffer* buffer,
-                            const char* start, int size) {
+void CompoundBuffer::Append(scoped_refptr<net::IOBuffer> buffer,
+                            const char* start,
+                            int size) {
   // A weak check that the |start| is within |buffer|.
   DCHECK_GE(start, buffer->data());
   DCHECK_GT(size, 0);
 
   CHECK(!locked_);
 
-  chunks_.push_back(DataChunk(buffer, start, size));
+  chunks_.emplace_back(std::move(buffer), start, size);
   total_bytes_ += size;
 }
 
-void CompoundBuffer::Append(net::IOBuffer* buffer, int size) {
-  Append(buffer, buffer->data(), size);
+void CompoundBuffer::Append(scoped_refptr<net::IOBuffer> buffer, int size) {
+  const char* start = buffer->data();
+  Append(std::move(buffer), start, size);
 }
 
 void CompoundBuffer::Append(const CompoundBuffer& buffer) {
   for (DataChunkList::const_iterator it = buffer.chunks_.begin();
        it != buffer.chunks_.end(); ++it) {
-    Append(it->buffer.get(), it->start, it->size);
+    Append(it->buffer, it->start, it->size);
   }
 }
 
-void CompoundBuffer::Prepend(net::IOBuffer* buffer,
-                             const char* start, int size) {
+void CompoundBuffer::Prepend(scoped_refptr<net::IOBuffer> buffer,
+                             const char* start,
+                             int size) {
   // A weak check that the |start| is within |buffer|.
   DCHECK_GE(start, buffer->data());
   DCHECK_GT(size, 0);
 
   CHECK(!locked_);
 
-  chunks_.push_front(DataChunk(buffer, start, size));
+  chunks_.emplace_front(std::move(buffer), start, size);
   total_bytes_ += size;
 }
 
-void CompoundBuffer::Prepend(net::IOBuffer* buffer, int size) {
-  Prepend(buffer, buffer->data(), size);
+void CompoundBuffer::Prepend(scoped_refptr<net::IOBuffer> buffer, int size) {
+  const char* start = buffer->data();
+  Prepend(std::move(buffer), start, size);
 }
 
 void CompoundBuffer::Prepend(const CompoundBuffer& buffer) {
   for (DataChunkList::const_iterator it = buffer.chunks_.begin();
        it != buffer.chunks_.end(); ++it) {
-    Prepend(it->buffer.get(), it->start, it->size);
+    Prepend(it->buffer, it->start, it->size);
   }
 }
 void CompoundBuffer::AppendCopyOf(const char* data, int size) {
-  net::IOBuffer* buffer = new net::IOBuffer(size);
+  scoped_refptr<net::IOBuffer> buffer =
+      base::MakeRefCounted<net::IOBuffer>(size);
   memcpy(buffer->data(), data, size);
-  Append(buffer, size);
+  Append(std::move(buffer), size);
 }
 
 void CompoundBuffer::PrependCopyOf(const char* data, int size) {
-  net::IOBuffer* buffer = new net::IOBuffer(size);
+  scoped_refptr<net::IOBuffer> buffer =
+      base::MakeRefCounted<net::IOBuffer>(size);
   memcpy(buffer->data(), data, size);
-  Prepend(buffer, size);
+  Prepend(std::move(buffer), size);
 }
 
 void CompoundBuffer::CropFront(int bytes) {
@@ -140,8 +144,10 @@ void CompoundBuffer::Lock() {
   locked_ = true;
 }
 
-net::IOBufferWithSize* CompoundBuffer::ToIOBufferWithSize() const {
-  net::IOBufferWithSize* result = new net::IOBufferWithSize(total_bytes_);
+scoped_refptr<net::IOBufferWithSize> CompoundBuffer::ToIOBufferWithSize()
+    const {
+  scoped_refptr<net::IOBufferWithSize> result =
+      base::MakeRefCounted<net::IOBufferWithSize>(total_bytes_);
   CopyTo(result->data(), total_bytes_);
   return result;
 }

remoting/base/compound_buffer.h

@@ -43,11 +43,13 @@ class CompoundBuffer {
   // Adds new chunk to the buffer. |start| defines position of the chunk
   // within the |buffer|. |size| is the size of the chunk that is being
   // added, not size of the |buffer|.
-  void Append(net::IOBuffer* buffer, int size);
-  void Append(net::IOBuffer* buffer, const char* start, int size);
+  void Append(scoped_refptr<net::IOBuffer> buffer, int size);
+  void Append(scoped_refptr<net::IOBuffer> buffer, const char* start, int size);
   void Append(const CompoundBuffer& buffer);
-  void Prepend(net::IOBuffer* buffer, int size);
-  void Prepend(net::IOBuffer* buffer, const char* start, int size);
+  void Prepend(scoped_refptr<net::IOBuffer> buffer, int size);
+  void Prepend(scoped_refptr<net::IOBuffer> buffer,
+               const char* start,
+               int size);
   void Prepend(const CompoundBuffer& buffer);
 
   // Same as above, but creates new IOBuffer and copies the data.
@@ -71,7 +73,7 @@ class CompoundBuffer {
 
   // Creates new IOBufferWithSize object and copies all data into it.
   // Ownership of the result is given to the caller.
-  net::IOBufferWithSize* ToIOBufferWithSize() const;
+  scoped_refptr<net::IOBufferWithSize> ToIOBufferWithSize() const;
 
   // Copies all data into given location.
   void CopyTo(char* data, int data_size) const;
@@ -84,7 +86,7 @@ class CompoundBuffer {
   friend class CompoundBufferInputStream;
 
   struct DataChunk {
-    DataChunk(net::IOBuffer* buffer, const char* start, int size);
+    DataChunk(scoped_refptr<net::IOBuffer> buffer, const char* start, int size);
     DataChunk(const DataChunk& other);
     ~DataChunk();
 

remoting/base/compound_buffer_unittest.cc

@@ -39,7 +39,7 @@ class CompoundBufferTest : public testing::Test {
 
   // Following 5 methods are used with IterateOverPieces().
   void Append(int pos, int size) {
-    target_.Append(data_.get(), data_->data() + pos, size);
+    target_.Append(data_, data_->data() + pos, size);
   }
 
   void AppendCopyOf(int pos, int size) {
@@ -47,7 +47,7 @@ class CompoundBufferTest : public testing::Test {
   }
 
   void Prepend(int pos, int size) {
-    target_.Prepend(data_.get(), data_->data() + kDataSize - pos - size, size);
+    target_.Prepend(data_, data_->data() + kDataSize - pos - size, size);
   }
 
   void PrependCopyOf(int pos, int size) {
@@ -170,7 +170,7 @@ class CompoundBufferTest : public testing::Test {
     const char* data = kTestData.data();
     for (int i = 0; i < segments; ++i) {
       int size = i % 2 == 0 ? 1 : 2;
-      result->Append(new net::WrappedIOBuffer(data), size);
+      result->Append(base::MakeRefCounted<net::WrappedIOBuffer>(data), size);
       data += size;
     }
     result->Lock();

remoting/host/file_proxy_wrapper_linux_unittest.cc

@@ -29,7 +29,8 @@ const std::string& kTestDataThree = "this is the third test string";
 std::unique_ptr<remoting::CompoundBuffer> ToBuffer(const std::string& data) {
   std::unique_ptr<remoting::CompoundBuffer> buffer =
       std::make_unique<remoting::CompoundBuffer>();
-  buffer->Append(new net::WrappedIOBuffer(data.data()), data.size());
+  buffer->Append(base::MakeRefCounted<net::WrappedIOBuffer>(data.data()),
+                 data.size());
   return buffer;
 }
 

remoting/host/file_transfer_message_handler_unittest.cc

@@ -27,7 +27,8 @@ constexpr char kTestFilename[] = "test-file.txt";
 std::unique_ptr<remoting::CompoundBuffer> ToBuffer(const std::string& data) {
   std::unique_ptr<remoting::CompoundBuffer> buffer =
       std::make_unique<remoting::CompoundBuffer>();
-  buffer->Append(new net::WrappedIOBuffer(data.data()), data.size());
+  buffer->Append(base::MakeRefCounted<net::WrappedIOBuffer>(data.data()),
+                 data.size());
   return buffer;
 }
 

remoting/protocol/message_decoder.cc

@@ -24,7 +24,7 @@ MessageDecoder::~MessageDecoder() = default;
 
 void MessageDecoder::AddData(scoped_refptr<net::IOBuffer> data,
                              int data_size) {
-  buffer_.Append(data.get(), data_size);
+  buffer_.Append(std::move(data), data_size);
 }
 
 CompoundBuffer* MessageDecoder::GetNextMessage() {