net: Make net::DrainableIOBuffer constructors take scoped_refptr.

The net::DrainableIOBuffer constructors currently take in a
net::IOBuffer base as a raw pointer, then assign it to a
scoped_refptr<net::IOBuffer> member, thus taking a reference.

This CL turns the raw pointer parameter into a scoped_refptr, and uses
std::move() during the member assignment. So, the reference-increment
operation is pushed from the constructors to the construction callsites.
No new increments are introduced.

The CL also modifies the DrainableIOBuffer construction sites, using
std::move() where possible to remove reference increments.

Bug: 479898
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I95cd0973d5e25c2565ddecfa0e06a3fc15e1f7b9
Reviewed-on: https://chromium-review.googlesource.com/1188955
Commit-Queue: Victor Costan <pwnall@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Nicolas Zea <zea@chromium.org>
Reviewed-by: Cait Phillips <caitkp@chromium.org>
Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587471}

chrome/browser/devtools/device/android_device_manager.cc

@@ -191,9 +191,10 @@ class HttpRequest {
 
     std::string request = base::StrCat(pieces);
     scoped_refptr<net::IOBuffer> base_buffer =
-        new net::IOBuffer(request.size());
+        base::MakeRefCounted<net::IOBuffer>(request.size());
     memcpy(base_buffer->data(), request.data(), request.size());
-    request_ = new net::DrainableIOBuffer(base_buffer.get(), request.size());
+    request_ = base::MakeRefCounted<net::DrainableIOBuffer>(
+        std::move(base_buffer), request.size());
 
     DoSendRequest(net::OK);
   }

chrome/browser/devtools/device/port_forwarding_controller.cc

@@ -220,7 +220,7 @@ class SocketTunnel : public network::mojom::ResolveHostClient {
         base::Bind(
             &SocketTunnel::OnRead, base::Unretained(this), from, to, buffer));
     if (result != net::ERR_IO_PENDING)
-      OnRead(from, to, buffer, result);
+      OnRead(from, to, std::move(buffer), result);
   }
 
   void OnRead(net::StreamSocket* from,
@@ -234,7 +234,7 @@ class SocketTunnel : public network::mojom::ResolveHostClient {
 
     int total = result;
     scoped_refptr<net::DrainableIOBuffer> drainable =
-        new net::DrainableIOBuffer(buffer.get(), total);
+        base::MakeRefCounted<net::DrainableIOBuffer>(std::move(buffer), total);
 
     ++pending_writes_;
     result = to->Write(drainable.get(), total,

chrome/browser/extensions/api/messaging/native_message_process_host.cc

@@ -298,8 +298,11 @@ void NativeMessageProcessHost::DoWrite() {
         !current_write_buffer_->BytesRemaining()) {
       if (write_queue_.empty())
         return;
-      current_write_buffer_ = new net::DrainableIOBuffer(
-          write_queue_.front().get(), write_queue_.front()->size());
+      scoped_refptr<net::IOBufferWithSize> buffer =
+          std::move(write_queue_.front());
+      int buffer_size = buffer->size();
+      current_write_buffer_ = base::MakeRefCounted<net::DrainableIOBuffer>(
+          std::move(buffer), buffer_size);
       write_queue_.pop();
     }
 

chrome/test/chromedriver/net/websocket.cc

@@ -63,8 +63,10 @@ WebSocket::WebSocket(const GURL& url, WebSocketListener* listener)
     : url_(url),
       listener_(listener),
       state_(INITIALIZED),
-      write_buffer_(new net::DrainableIOBuffer(new net::IOBuffer(0), 0)),
-      read_buffer_(new net::IOBufferWithSize(4096)) {}
+      write_buffer_(base::MakeRefCounted<net::DrainableIOBuffer>(
+          base::MakeRefCounted<net::IOBuffer>(0),
+          0)),
+      read_buffer_(base::MakeRefCounted<net::IOBufferWithSize>(4096)) {}
 
 WebSocket::~WebSocket() {
   CHECK(thread_checker_.CalledOnValidThread());
@@ -189,8 +191,8 @@ void WebSocket::ContinueWritingIfNecessary() {
   if (!write_buffer_->BytesRemaining()) {
     if (pending_write_.empty())
       return;
-    write_buffer_ = new net::DrainableIOBuffer(
-        new net::StringIOBuffer(pending_write_),
+    write_buffer_ = base::MakeRefCounted<net::DrainableIOBuffer>(
+        base::MakeRefCounted<net::StringIOBuffer>(pending_write_),
         pending_write_.length());
     pending_write_.clear();
   }

components/cast_channel/cast_transport.cc

@@ -108,8 +108,8 @@ CastTransportImpl::WriteRequest::WriteRequest(
     const net::CompletionCallback& callback)
     : message_namespace(namespace_), callback(callback) {
   VLOG(2) << "WriteRequest size: " << payload.size();
-  io_buffer = new net::DrainableIOBuffer(new net::StringIOBuffer(payload),
-                                         payload.size());
+  io_buffer = base::MakeRefCounted<net::DrainableIOBuffer>(
+      base::MakeRefCounted<net::StringIOBuffer>(payload), payload.size());
 }
 
 CastTransportImpl::WriteRequest::WriteRequest(const WriteRequest& other) =

components/nacl/browser/pnacl_host.cc

@@ -406,8 +406,9 @@ scoped_refptr<net::DrainableIOBuffer> PnaclHost::CopyFileToBuffer(
     return buffer;
   }
 
-  buffer = new net::DrainableIOBuffer(
-      new net::IOBuffer(base::checked_cast<size_t>(file_size)),
+  buffer = base::MakeRefCounted<net::DrainableIOBuffer>(
+      base::MakeRefCounted<net::IOBuffer>(
+          base::checked_cast<size_t>(file_size)),
       base::checked_cast<size_t>(file_size));
   if (file->Read(0, buffer->data(), buffer->size()) != file_size) {
     PLOG(ERROR) << "CopyFileToBuffer file read failed";

components/nacl/browser/pnacl_translation_cache.cc

@@ -246,8 +246,9 @@ void PnaclTranslationCacheEntry::DispatchNext(int rv) {
         step_ = TRANSFER_ENTRY;
         if (is_read_) {
           int bytes_to_transfer = entry_->GetDataSize(1);
-          io_buf_ = new net::DrainableIOBuffer(
-              new net::IOBuffer(bytes_to_transfer), bytes_to_transfer);
+          io_buf_ = base::MakeRefCounted<net::DrainableIOBuffer>(
+              base::MakeRefCounted<net::IOBuffer>(bytes_to_transfer),
+              bytes_to_transfer);
           ReadEntry(0, bytes_to_transfer);
         } else {
           WriteEntry(0, io_buf_->size());

components/nacl/browser/pnacl_translation_cache_unittest.cc

@@ -66,8 +66,9 @@ void PnaclTranslationCacheTest::InitBackend(bool in_mem) {
 void PnaclTranslationCacheTest::StoreNexe(const std::string& key,
                                           const std::string& nexe) {
   net::TestCompletionCallback store_cb;
-  scoped_refptr<net::DrainableIOBuffer> nexe_buf(
-      new net::DrainableIOBuffer(new net::StringIOBuffer(nexe), nexe.size()));
+  scoped_refptr<net::DrainableIOBuffer> nexe_buf =
+      base::MakeRefCounted<net::DrainableIOBuffer>(
+          base::MakeRefCounted<net::StringIOBuffer>(nexe), nexe.size());
   cache_->StoreNexe(key, nexe_buf.get(), store_cb.callback());
   // Using ERR_IO_PENDING here causes the callback to wait for the result
   // which should be harmless even if it returns OK immediately. This is because
@@ -229,9 +230,11 @@ TEST_F(PnaclTranslationCacheTest, StoreLargeOnDisk) {
 
 TEST_F(PnaclTranslationCacheTest, InMemSizeLimit) {
   InitBackend(true);
-  scoped_refptr<net::DrainableIOBuffer> large_buffer(new net::DrainableIOBuffer(
-      new net::StringIOBuffer(std::string(kMaxMemCacheSize + 1, 'a')),
-      kMaxMemCacheSize + 1));
+  scoped_refptr<net::DrainableIOBuffer> large_buffer =
+      base::MakeRefCounted<net::DrainableIOBuffer>(
+          base::MakeRefCounted<net::StringIOBuffer>(
+              std::string(kMaxMemCacheSize + 1, 'a')),
+          kMaxMemCacheSize + 1);
   net::TestCompletionCallback store_cb;
   cache_->StoreNexe(test_key, large_buffer.get(), store_cb.callback());
   EXPECT_EQ(net::ERR_FAILED, store_cb.GetResult(net::ERR_IO_PENDING));

content/browser/devtools/protocol/tethering_handler.cc

@@ -125,7 +125,7 @@ class SocketPump {
 
     int total = result;
     scoped_refptr<net::DrainableIOBuffer> drainable =
-        new net::DrainableIOBuffer(buffer.get(), total);
+        base::MakeRefCounted<net::DrainableIOBuffer>(std::move(buffer), total);
 
     ++pending_writes_;
     result =

content/browser/renderer_host/pepper/pepper_tcp_socket_message_filter.cc

@@ -433,10 +433,10 @@ int32_t PepperTCPSocketMessageFilter::OnMsgWrite(
     return PP_ERROR_BADARGUMENT;
   }
 
-  write_buffer_base_ = new net::IOBuffer(data_size);
+  write_buffer_base_ = base::MakeRefCounted<net::IOBuffer>(data_size);
   memcpy(write_buffer_base_->data(), data.data(), data_size);
-  write_buffer_ =
-      new net::DrainableIOBuffer(write_buffer_base_.get(), data_size);
+  write_buffer_ = base::MakeRefCounted<net::DrainableIOBuffer>(
+      write_buffer_base_, data_size);
   DoWrite(context->MakeReplyMessageContext());
   return PP_OK_COMPLETIONPENDING;
 }

google_apis/gcm/base/socket_stream.cc

@@ -25,9 +25,10 @@ SocketInputStream::SocketInputStream(mojo::ScopedDataPipeConsumerHandle stream)
     : stream_(std::move(stream)),
       stream_watcher_(FROM_HERE, mojo::SimpleWatcher::ArmingPolicy::MANUAL),
       read_size_(0),
-      io_buffer_(new net::IOBuffer(kDefaultBufferSize)),
+      io_buffer_(base::MakeRefCounted<net::IOBuffer>(kDefaultBufferSize)),
       read_buffer_(
-          new net::DrainableIOBuffer(io_buffer_.get(), kDefaultBufferSize)),
+          base::MakeRefCounted<net::DrainableIOBuffer>(io_buffer_,
+                                                       kDefaultBufferSize)),
       next_pos_(0),
       last_error_(net::OK),
       weak_ptr_factory_(this) {

jingle/glue/fake_ssl_client_socket.cc

@@ -65,7 +65,8 @@ static const uint8_t kSslServerHello[] = {
 };
 
 net::DrainableIOBuffer* NewDrainableIOBufferWithSize(int size) {
-  return new net::DrainableIOBuffer(new net::IOBuffer(size), size);
+  return new net::DrainableIOBuffer(base::MakeRefCounted<net::IOBuffer>(size),
+                                    size);
 }
 
 }  // namespace

net/base/file_stream_unittest.cc

@@ -47,8 +47,9 @@ constexpr char kTestData[] = "0123456789";
 constexpr int kTestDataSize = base::size(kTestData) - 1;
 
 // Creates an IOBufferWithSize that contains the kTestDataSize.
-IOBufferWithSize* CreateTestDataBuffer() {
-  IOBufferWithSize* buf = new IOBufferWithSize(kTestDataSize);
+scoped_refptr<IOBufferWithSize> CreateTestDataBuffer() {
+  scoped_refptr<IOBufferWithSize> buf =
+      base::MakeRefCounted<IOBufferWithSize>(kTestDataSize);
   memcpy(buf->data(), kTestData, kTestDataSize);
   return buf;
 }
@@ -351,9 +352,10 @@ TEST_F(FileStreamTest, Write_FromOffset) {
 
   int total_bytes_written = 0;
 
-  scoped_refptr<IOBufferWithSize> buf = CreateTestDataBuffer();
+  scoped_refptr<IOBufferWithSize> buffer = CreateTestDataBuffer();
+  int buffer_size = buffer->size();
   scoped_refptr<DrainableIOBuffer> drainable =
-      base::MakeRefCounted<DrainableIOBuffer>(buf.get(), buf->size());
+      base::MakeRefCounted<DrainableIOBuffer>(std::move(buffer), buffer_size);
   while (total_bytes_written != kTestDataSize) {
     rv = stream.Write(drainable.get(), drainable->BytesRemaining(),
                       callback.callback());
@@ -402,9 +404,10 @@ TEST_F(FileStreamTest, BasicReadWrite) {
 
   int total_bytes_written = 0;
 
-  scoped_refptr<IOBufferWithSize> buf = CreateTestDataBuffer();
+  scoped_refptr<IOBufferWithSize> buffer = CreateTestDataBuffer();
+  int buffer_size = buffer->size();
   scoped_refptr<DrainableIOBuffer> drainable =
-      base::MakeRefCounted<DrainableIOBuffer>(buf.get(), buf->size());
+      base::MakeRefCounted<DrainableIOBuffer>(std::move(buffer), buffer_size);
   while (total_bytes_written != kTestDataSize) {
     rv = stream->Write(drainable.get(), drainable->BytesRemaining(),
                        callback.callback());
@@ -444,9 +447,10 @@ TEST_F(FileStreamTest, BasicWriteRead) {
 
   int total_bytes_written = 0;
 
-  scoped_refptr<IOBufferWithSize> buf = CreateTestDataBuffer();
+  scoped_refptr<IOBufferWithSize> buffer = CreateTestDataBuffer();
+  int buffer_size = buffer->size();
   scoped_refptr<DrainableIOBuffer> drainable =
-      base::MakeRefCounted<DrainableIOBuffer>(buf.get(), buf->size());
+      base::MakeRefCounted<DrainableIOBuffer>(std::move(buffer), buffer_size);
   while (total_bytes_written != kTestDataSize) {
     rv = stream->Write(drainable.get(), drainable->BytesRemaining(),
                        callback.callback());
@@ -505,9 +509,9 @@ class TestWriteReadCompletionCallback {
         total_bytes_written_(total_bytes_written),
         total_bytes_read_(total_bytes_read),
         data_read_(data_read),
-        test_data_(CreateTestDataBuffer()),
-        drainable_(base::MakeRefCounted<DrainableIOBuffer>(test_data_.get(),
-                                                           kTestDataSize)) {}
+        drainable_(
+            base::MakeRefCounted<DrainableIOBuffer>(CreateTestDataBuffer(),
+                                                    kTestDataSize)) {}
 
   int WaitForResult() {
     DCHECK(!waiting_for_result_);
@@ -588,7 +592,6 @@ class TestWriteReadCompletionCallback {
   int* total_bytes_written_;
   int* total_bytes_read_;
   std::string* data_read_;
-  scoped_refptr<IOBufferWithSize> test_data_;
   scoped_refptr<DrainableIOBuffer> drainable_;
 
   DISALLOW_COPY_AND_ASSIGN(TestWriteReadCompletionCallback);
@@ -646,9 +649,9 @@ class TestWriteCloseCompletionCallback {
         waiting_for_result_(false),
         stream_(stream),
         total_bytes_written_(total_bytes_written),
-        test_data_(CreateTestDataBuffer()),
-        drainable_(base::MakeRefCounted<DrainableIOBuffer>(test_data_.get(),
-                                                           kTestDataSize)) {}
+        drainable_(
+            base::MakeRefCounted<DrainableIOBuffer>(CreateTestDataBuffer(),
+                                                    kTestDataSize)) {}
 
   int WaitForResult() {
     DCHECK(!waiting_for_result_);
@@ -696,7 +699,6 @@ class TestWriteCloseCompletionCallback {
   bool waiting_for_result_;
   FileStream* stream_;
   int* total_bytes_written_;
-  scoped_refptr<IOBufferWithSize> test_data_;
   scoped_refptr<DrainableIOBuffer> drainable_;
 
   DISALLOW_COPY_AND_ASSIGN(TestWriteCloseCompletionCallback);

net/base/io_buffer.cc

@@ -92,16 +92,13 @@ StringIOBuffer::~StringIOBuffer() {
   data_ = NULL;
 }
 
-DrainableIOBuffer::DrainableIOBuffer(IOBuffer* base, int size)
-    : IOBuffer(base->data()),
-      base_(base),
-      size_(size),
-      used_(0) {
+DrainableIOBuffer::DrainableIOBuffer(scoped_refptr<IOBuffer> base, int size)
+    : IOBuffer(base->data()), base_(std::move(base)), size_(size), used_(0) {
   AssertValidBufferSize(size);
 }
 
-DrainableIOBuffer::DrainableIOBuffer(IOBuffer* base, size_t size)
-    : IOBuffer(base->data()), base_(base), size_(size), used_(0) {
+DrainableIOBuffer::DrainableIOBuffer(scoped_refptr<IOBuffer> base, size_t size)
+    : IOBuffer(base->data()), base_(std::move(base)), size_(size), used_(0) {
   AssertValidBufferSize(size);
 }
 

net/base/io_buffer.h

@@ -155,8 +155,8 @@ class NET_EXPORT StringIOBuffer : public IOBuffer {
 class NET_EXPORT DrainableIOBuffer : public IOBuffer {
  public:
   // TODO(eroman): Deprecated. Use the size_t flavor instead. crbug.com/488553
-  DrainableIOBuffer(IOBuffer* base, int size);
-  DrainableIOBuffer(IOBuffer* base, size_t size);
+  DrainableIOBuffer(scoped_refptr<IOBuffer> base, int size);
+  DrainableIOBuffer(scoped_refptr<IOBuffer> base, size_t size);
 
   // DidConsume() changes the |data_| pointer so that |data_| always points
   // to the first unconsumed byte.

net/dns/dns_transaction.cc

@@ -652,7 +652,7 @@ class DnsTCPAttempt : public DnsAttempt {
     if (static_cast<int>(query_size) != query_->io_buffer()->size())
       return ERR_FAILED;
     base::WriteBigEndian<uint16_t>(length_buffer_->data(), query_size);
-    buffer_ = base::MakeRefCounted<DrainableIOBuffer>(length_buffer_.get(),
+    buffer_ = base::MakeRefCounted<DrainableIOBuffer>(length_buffer_,
                                                       length_buffer_->size());
     next_state_ = STATE_SEND_LENGTH;
     return OK;
@@ -690,7 +690,7 @@ class DnsTCPAttempt : public DnsAttempt {
           base::BindOnce(&DnsTCPAttempt::OnIOComplete, base::Unretained(this)),
           kTrafficAnnotation);
     }
-    buffer_ = base::MakeRefCounted<DrainableIOBuffer>(length_buffer_.get(),
+    buffer_ = base::MakeRefCounted<DrainableIOBuffer>(length_buffer_,
                                                       length_buffer_->size());
     next_state_ = STATE_READ_LENGTH;
     return OK;

net/filter/filter_source_stream.cc

@@ -141,7 +141,7 @@ int FilterSourceStream::DoReadDataComplete(int result) {
 
   if (result >= OK) {
     drainable_input_buffer_ =
-        base::MakeRefCounted<DrainableIOBuffer>(input_buffer_.get(), result);
+        base::MakeRefCounted<DrainableIOBuffer>(input_buffer_, result);
     next_state_ = STATE_FILTER_DATA;
   }
   if (result <= OK)

net/ftp/ftp_network_transaction.cc

@@ -482,7 +482,7 @@ int FtpNetworkTransaction::SendFtpCommand(const std::string& command,
   write_command_buf_ =
       base::MakeRefCounted<IOBufferWithSize>(command.length() + 2);
   write_buf_ = base::MakeRefCounted<DrainableIOBuffer>(
-      write_command_buf_.get(), write_command_buf_->size());
+      write_command_buf_, write_command_buf_->size());
   memcpy(write_command_buf_->data(), command.data(), command.length());
   memcpy(write_command_buf_->data() + command.length(), kCRLF, 2);
 

net/http/http_stream_parser.cc

@@ -275,7 +275,7 @@ int HttpStreamParser::SendRequest(
     // We'll repurpose |request_headers_| to store the merged headers and
     // body.
     request_headers_ = base::MakeRefCounted<DrainableIOBuffer>(
-        merged_request_headers_and_body.get(), merged_size);
+        merged_request_headers_and_body, merged_size);
 
     memcpy(request_headers_->data(), request.data(), request_headers_length_);
     request_headers_->DidConsume(request_headers_length_);
@@ -308,7 +308,7 @@ int HttpStreamParser::SendRequest(
     scoped_refptr<StringIOBuffer> headers_io_buf =
         base::MakeRefCounted<StringIOBuffer>(request);
     request_headers_ = base::MakeRefCounted<DrainableIOBuffer>(
-        headers_io_buf.get(), headers_io_buf->size());
+        std::move(headers_io_buf), request.size());
   }
 
   result = DoLoop(OK);

net/quic/quic_http_stream.cc

@@ -244,7 +244,7 @@ int QuicHttpStream::SendRequest(const HttpRequestHeaders& request_headers,
                                                 256 * quic::kMaxPacketSize))));
     // The request body buffer is empty at first.
     request_body_buf_ =
-        base::MakeRefCounted<DrainableIOBuffer>(raw_request_body_buf_.get(), 0);
+        base::MakeRefCounted<DrainableIOBuffer>(raw_request_body_buf_, 0);
   }
 
   // Store the response info.
@@ -621,7 +621,7 @@ int QuicHttpStream::DoReadRequestBodyComplete(int rv) {
   }
 
   request_body_buf_ =
-      base::MakeRefCounted<DrainableIOBuffer>(raw_request_body_buf_.get(), rv);
+      base::MakeRefCounted<DrainableIOBuffer>(raw_request_body_buf_, rv);
   if (rv == 0) {  // Reached the end.
     DCHECK(request_body_stream_->IsEOF());
   }

net/server/http_server_unittest.cc

@@ -75,8 +75,8 @@ class TestHttpClient {
   }
 
   void Send(const std::string& data) {
-    write_buffer_ =
-        new DrainableIOBuffer(new StringIOBuffer(data), data.length());
+    write_buffer_ = base::MakeRefCounted<DrainableIOBuffer>(
+        base::MakeRefCounted<StringIOBuffer>(data), data.length());
     Write();
   }