Avoid some string copies in net:StringIOBuffer constructor.

The current constructor always copies from a const-ref argument.
Instead, pass by value and move into the member. At worst, we've moved
the copy to the caller, but often we either already have an rvalue
or can move an existing string when calling the constructor.

Remove an odd form of the constructor that might have made sense
prior to movable strings.

-- many unittests remain unchanged, though there might be
   optimizations possible there.

Change-Id: I9bc0e3d980726c8cfe459aac9a78fc4715eb83ac
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4938384
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Juliet Lévesque <julietlevesque@google.com>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Joe Downing <joedow@chromium.org>
Reviewed-by: Joshua Bell <jsbell@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1210933}

ash/quick_pair/message_stream/fake_bluetooth_socket.cc

@@ -4,6 +4,8 @@
 
 #include "ash/quick_pair/message_stream/fake_bluetooth_socket.h"
 
+#include <utility>
+
 #include "base/strings/string_number_conversions.h"
 
 namespace ash {
@@ -43,15 +45,16 @@ void FakeBluetoothSocket::TriggerReceiveCallback() {
   }
 
   std::string buffer_bytes(bytes_.begin(), bytes_.end());
+  const size_t buffer_bytes_size = buffer_bytes.size();
   scoped_refptr<net::IOBuffer> io_buffer =
-      base::MakeRefCounted<net::StringIOBuffer>(buffer_bytes);
+      base::MakeRefCounted<net::StringIOBuffer>(std::move(buffer_bytes));
 
   if (empty_buffer_) {
     io_buffer->data()[0] = '\0';
     empty_buffer_ = false;
   }
   std::move(success_callback_)
-      .Run(/*buffer_size*/ buffer_bytes.size(),
+      .Run(/*buffer_size*/ buffer_bytes_size,
            /*buffer=*/std::move(io_buffer));
 }
 

chrome/test/chromedriver/net/adb_client_socket.cc

@@ -418,7 +418,7 @@ class AdbSendFileSocket : AdbClientSocket {
       buffer.append(payload, payload_length);
 
     scoped_refptr<net::StringIOBuffer> request_buffer =
-        base::MakeRefCounted<net::StringIOBuffer>(buffer);
+        base::MakeRefCounted<net::StringIOBuffer>(std::move(buffer));
 
     auto split_callback = base::SplitOnceCallback(std::move(callback));
     int result = socket_->Write(request_buffer.get(), request_buffer->size(),

chrome/test/chromedriver/net/pipe_connection_posix.cc

@@ -283,8 +283,10 @@ class PipeWriter {
     queued_.insert(queued_.end(), message.c_str(),
                    message.c_str() + message.size() + 1);
     if (!write_buffer_->BytesRemaining()) {
+      const size_t queued_size = queued_.size();
       write_buffer_ = base::MakeRefCounted<net::DrainableIOBuffer>(
-          base::MakeRefCounted<net::StringIOBuffer>(queued_), queued_.size());
+          base::MakeRefCounted<net::StringIOBuffer>(std::move(queued_)),
+          queued_size);
       queued_ = std::string();
       WriteFromBuffer();
     }
@@ -328,8 +330,10 @@ class PipeWriter {
     write_buffer_->DidConsume(rv);
     int sent = WriteFromBuffer();
     if (sent >= 0 && !write_buffer_->BytesRemaining() && !queued_.empty()) {
+      const size_t queued_size = queued_.size();
       write_buffer_ = base::MakeRefCounted<net::DrainableIOBuffer>(
-          base::MakeRefCounted<net::StringIOBuffer>(queued_), queued_.size());
+          base::MakeRefCounted<net::StringIOBuffer>(std::move(queued_)),
+          queued_size);
       queued_ = std::string();
       WriteFromBuffer();
     }

chrome/test/chromedriver/net/websocket.cc

@@ -208,9 +208,10 @@ void WebSocket::ContinueWritingIfNecessary() {
   if (!write_buffer_->BytesRemaining()) {
     if (pending_write_.empty())
       return;
+    const size_t pending_write_length = pending_write_.length();
     write_buffer_ = base::MakeRefCounted<net::DrainableIOBuffer>(
-        base::MakeRefCounted<net::StringIOBuffer>(pending_write_),
-        pending_write_.length());
+        base::MakeRefCounted<net::StringIOBuffer>(std::move(pending_write_)),
+        pending_write_length);
     pending_write_.clear();
   }
   int code = socket_->Write(

content/browser/cache_storage/cache_storage_cache.cc

@@ -1518,8 +1518,8 @@ void CacheStorageCache::MatchAllDidQueryCache(
 void CacheStorageCache::WriteMetadata(disk_cache::Entry* entry,
                                       const proto::CacheMetadata& metadata,
                                       WriteMetadataCallback callback) {
-  std::unique_ptr<std::string> serialized = std::make_unique<std::string>();
-  if (!metadata.SerializeToString(serialized.get())) {
+  std::string serialized;
+  if (!metadata.SerializeToString(&serialized)) {
     std::move(callback).Run(0, -1);
     return;
   }

net/base/io_buffer.cc

@@ -4,6 +4,8 @@
 
 #include "net/base/io_buffer.h"
 
+#include <utility>
+
 #include "base/check_op.h"
 #include "base/numerics/safe_math.h"
 
@@ -52,17 +54,9 @@ IOBufferWithSize::IOBufferWithSize(char* data, size_t size)
 
 IOBufferWithSize::~IOBufferWithSize() = default;
 
-StringIOBuffer::StringIOBuffer(const std::string& s)
-    : IOBuffer(static_cast<char*>(nullptr)), string_data_(s) {
-  AssertValidBufferSize(s.size());
-  data_ = const_cast<char*>(string_data_.data());
-}
-
-StringIOBuffer::StringIOBuffer(std::unique_ptr<std::string> s)
-    : IOBuffer(static_cast<char*>(nullptr)) {
-  AssertValidBufferSize(s->size());
-  string_data_.swap(*s.get());
-  data_ = const_cast<char*>(string_data_.data());
+StringIOBuffer::StringIOBuffer(std::string s) : string_data_(std::move(s)) {
+  AssertValidBufferSize(string_data_.size());
+  data_ = string_data_.data();
 }
 
 StringIOBuffer::~StringIOBuffer() {

net/base/io_buffer.h

@@ -128,8 +128,7 @@ class NET_EXPORT IOBufferWithSize : public IOBuffer {
 // the IOBuffer interface does not provide a proper way to modify it.
 class NET_EXPORT StringIOBuffer : public IOBuffer {
  public:
-  explicit StringIOBuffer(const std::string& s);
-  explicit StringIOBuffer(std::unique_ptr<std::string> s);
+  explicit StringIOBuffer(std::string s);
 
   int size() const { return static_cast<int>(string_data_.size()); }
 

net/http/http_stream_parser.cc

@@ -278,10 +278,11 @@ int HttpStreamParser::SendRequest(
   if (!did_merge) {
     // If we didn't merge the body with the headers, then |request_headers_|
     // contains just the HTTP headers.
+    size_t request_size = request.size();
     scoped_refptr<StringIOBuffer> headers_io_buf =
-        base::MakeRefCounted<StringIOBuffer>(request);
+        base::MakeRefCounted<StringIOBuffer>(std::move(request));
     request_headers_ = base::MakeRefCounted<DrainableIOBuffer>(
-        std::move(headers_io_buf), request.size());
+        std::move(headers_io_buf), request_size);
   }
 
   result = DoLoop(OK);

remoting/host/security_key/security_key_socket.cc

@@ -60,12 +60,11 @@ void SecurityKeySocket::SendResponse(const std::string& response_data) {
   DCHECK(!write_buffer_);
 
   std::string response_length_string = GetResponseLengthAsBytes(response_data);
-  int response_len = response_length_string.size() + response_data.size();
-  std::unique_ptr<std::string> response(
-      new std::string(response_length_string + response_data));
+  std::string response = response_length_string + response_data;
+  const size_t response_size = response.size();
   write_buffer_ = base::MakeRefCounted<net::DrainableIOBuffer>(
       base::MakeRefCounted<net::StringIOBuffer>(std::move(response)),
-      response_len);
+      response_size);
 
   DCHECK(write_buffer_->BytesRemaining());
   DoWrite();

remoting/protocol/ssl_hmac_channel_authenticator.cc

@@ -334,8 +334,10 @@ void SslHmacChannelAuthenticator::OnConnected(int result) {
   }
 
   // Allocate a buffer to write the digest.
+  const size_t auth_bytes_size = auth_bytes.size();
   auth_write_buf_ = base::MakeRefCounted<net::DrainableIOBuffer>(
-      base::MakeRefCounted<net::StringIOBuffer>(auth_bytes), auth_bytes.size());
+      base::MakeRefCounted<net::StringIOBuffer>(std::move(auth_bytes)),
+      auth_bytes_size);
 
   // Read an incoming token.
   auth_read_buf_ = base::MakeRefCounted<net::GrowableIOBuffer>();

storage/browser/blob/blob_reader_unittest.cc

@@ -73,15 +73,13 @@ class FakeFileStreamReader : public FileStreamReader {
  public:
   explicit FakeFileStreamReader(const std::string& contents)
       : buffer_(base::MakeRefCounted<DrainableIOBuffer>(
-            base::MakeRefCounted<net::StringIOBuffer>(
-                std::make_unique<std::string>(contents)),
+            base::MakeRefCounted<net::StringIOBuffer>(contents),
             contents.size())),
         net_error_(net::OK),
         size_(contents.size()) {}
   FakeFileStreamReader(const std::string& contents, uint64_t size)
       : buffer_(base::MakeRefCounted<DrainableIOBuffer>(
-            base::MakeRefCounted<net::StringIOBuffer>(
-                std::make_unique<std::string>(contents)),
+            base::MakeRefCounted<net::StringIOBuffer>(contents),
             contents.size())),
         net_error_(net::OK),
         size_(size) {}