Add constructors for IOBuffer that take the buffer length as a size_t.

Once the consumers have all been updated to work with unsigned buffer lengths, I will remove the signed int versions. For now this change adds some runtime safety checks, and facilitates the ongoing conversion to size_t.

A minority of consumers were calling IOBuffer with something other than "int" or "size_t", and those were updated by this change.

BUG=488553,491315
TBR=michaeln@chromium.org,pfeldman@chromium.org,bradnelson@chromium.org, rockot@chromium.org, dimich@chromium.org

Review URL: https://codereview.chromium.org/1147333003

Cr-Commit-Position: refs/heads/master@{#331485}
(cherry picked from commit 23ba60b3b6e4e6cfa36684ddf621a6afcc6906e4)

Review URL: https://codereview.chromium.org/1230413004 .

Cr-Commit-Position: refs/branch-heads/2403@{#498}
Cr-Branched-From: f54b8097a9c45ed4ad308133d49f05325d6c5070-refs/heads/master@{#330231}

15 files changed, 95 insertions, 29 deletions

diff --git a/chrome/browser/chromeos/file_system_provider/fileapi/file_stream_reader_unittest.cc b/chrome/browser/chromeos/file_system_provider/fileapi/file_stream_reader_unittest.cc
index a3f4d08..2b25c32 100644
--- a/chrome/browser/chromeos/file_system_provider/fileapi/file_stream_reader_unittest.cc
+++ b/chrome/browser/chromeos/file_system_provider/fileapi/file_stream_reader_unittest.cc
@@ -12,6 +12,7 @@
 #include "base/files/scoped_temp_dir.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
+#include "base/numerics/safe_math.h"
 #include "base/run_loop.h"
 #include "chrome/browser/chromeos/file_system_provider/fake_provided_file_system.h"
 #include "chrome/browser/chromeos/file_system_provider/service.h"
@@ -140,8 +141,8 @@ TEST_F(FileSystemProviderFileStreamReader, Read_AllAtOnce) {
   const int64 initial_offset = 0;
   FileStreamReader reader(
       NULL, file_url_, initial_offset, fake_file_->metadata->modification_time);
-  scoped_refptr<net::IOBuffer> io_buffer(
-      new net::IOBuffer(fake_file_->metadata->size));
+  scoped_refptr<net::IOBuffer> io_buffer(new net::IOBuffer(
+      base::CheckedNumeric<size_t>(fake_file_->metadata->size).ValueOrDie()));
 
   const int result =
       reader.Read(io_buffer.get(),
@@ -166,8 +167,8 @@ TEST_F(FileSystemProviderFileStreamReader, Read_WrongFile) {
                           wrong_file_url_,
                           initial_offset,
                           fake_file_->metadata->modification_time);
-  scoped_refptr<net::IOBuffer> io_buffer(
-      new net::IOBuffer(fake_file_->metadata->size));
+  scoped_refptr<net::IOBuffer> io_buffer(new net::IOBuffer(
+      base::CheckedNumeric<size_t>(fake_file_->metadata->size).ValueOrDie()));
 
   const int result =
       reader.Read(io_buffer.get(),
@@ -262,8 +263,8 @@ TEST_F(FileSystemProviderFileStreamReader, Read_ModifiedFile) {
   const int64 initial_offset = 0;
   FileStreamReader reader(NULL, file_url_, initial_offset, base::Time::Max());
 
-  scoped_refptr<net::IOBuffer> io_buffer(
-      new net::IOBuffer(fake_file_->metadata->size));
+  scoped_refptr<net::IOBuffer> io_buffer(new net::IOBuffer(
+      base::CheckedNumeric<size_t>(fake_file_->metadata->size).ValueOrDie()));
   const int result =
       reader.Read(io_buffer.get(),
                   fake_file_->metadata->size,
@@ -282,8 +283,8 @@ TEST_F(FileSystemProviderFileStreamReader, Read_ExpectedModificationTimeNull) {
   const int64 initial_offset = 0;
   FileStreamReader reader(NULL, file_url_, initial_offset, base::Time());
 
-  scoped_refptr<net::IOBuffer> io_buffer(
-      new net::IOBuffer(fake_file_->metadata->size));
+  scoped_refptr<net::IOBuffer> io_buffer(new net::IOBuffer(
+      base::CheckedNumeric<size_t>(fake_file_->metadata->size).ValueOrDie()));
   const int result =
       reader.Read(io_buffer.get(),
                   fake_file_->metadata->size,
diff --git a/chrome/browser/devtools/device/usb/android_usb_device.cc b/chrome/browser/devtools/device/usb/android_usb_device.cc
index 5da4458..fbed997 100644
--- a/chrome/browser/devtools/device/usb/android_usb_device.cc
+++ b/chrome/browser/devtools/device/usb/android_usb_device.cc
@@ -526,7 +526,8 @@ void AndroidUsbDevice::ReadBody(scoped_ptr<AdbMessage> message,
     return;
   }
 
-  scoped_refptr<net::IOBuffer> buffer = new net::IOBuffer(data_length);
+  scoped_refptr<net::IOBuffer> buffer =
+      new net::IOBuffer(static_cast<size_t>(data_length));
   usb_handle_->BulkTransfer(
       device::USB_DIRECTION_INBOUND, inbound_address_, buffer, data_length,
       kUsbTimeout,
diff --git a/components/nacl/browser/pnacl_host.cc b/components/nacl/browser/pnacl_host.cc
index bae7e97d..0df233f 100644
--- a/components/nacl/browser/pnacl_host.cc
+++ b/components/nacl/browser/pnacl_host.cc
@@ -9,6 +9,7 @@
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/logging.h"
+#include "base/numerics/safe_math.h"
 #include "base/task_runner_util.h"
 #include "base/threading/sequenced_worker_pool.h"
 #include "components/nacl/browser/nacl_browser.h"
@@ -417,13 +418,18 @@ scoped_refptr<net::DrainableIOBuffer> PnaclHost::CopyFileToBuffer(
   base::File::Info info;
   scoped_refptr<net::DrainableIOBuffer> buffer;
   bool error = false;
+
+  // TODO(eroman): Maximum size should be changed to size_t once that is
+  // what IOBuffer requires. crbug.com/488553. Also I don't think the
+  // max size should be inclusive here...
   if (!file->GetInfo(&info) ||
       info.size >= std::numeric_limits<int>::max()) {
     PLOG(ERROR) << "File::GetInfo failed";
     error = true;
   } else {
     buffer = new net::DrainableIOBuffer(
-        new net::IOBuffer(static_cast<int>(info.size)), info.size);
+        new net::IOBuffer(base::CheckedNumeric<size_t>(info.size).ValueOrDie()),
+        base::CheckedNumeric<size_t>(info.size).ValueOrDie());
     if (file->Read(0, buffer->data(), buffer->size()) != info.size) {
       PLOG(ERROR) << "CopyFileToBuffer file read failed";
       error = true;
diff --git a/content/browser/appcache/appcache_response.cc b/content/browser/appcache/appcache_response.cc
index 8b7c218..0c0f4bf 100644
--- a/content/browser/appcache/appcache_response.cc
+++ b/content/browser/appcache/appcache_response.cc
@@ -9,6 +9,7 @@
 #include "base/compiler_specific.h"
 #include "base/logging.h"
 #include "base/message_loop/message_loop.h"
+#include "base/numerics/safe_math.h"
 #include "base/pickle.h"
 #include "base/profiler/scoped_tracker.h"
 #include "base/strings/string_util.h"
@@ -272,8 +273,8 @@ void AppCacheResponseReader::OnIOComplete(int result) {
       int64 metadata_size = entry_->GetSize(kResponseMetadataIndex);
       if (metadata_size > 0) {
         reading_metadata_size_ = metadata_size;
-        info_buffer_->http_info->metadata =
-            new net::IOBufferWithSize(metadata_size);
+        info_buffer_->http_info->metadata = new net::IOBufferWithSize(
+            base::CheckedNumeric<size_t>(metadata_size).ValueOrDie());
         ReadRaw(kResponseMetadataIndex, 0,
                 info_buffer_->http_info->metadata.get(), metadata_size);
         return;
diff --git a/content/browser/appcache/view_appcache_internals_job.cc b/content/browser/appcache/view_appcache_internals_job.cc
index 84912b7..57848da 100644
--- a/content/browser/appcache/view_appcache_internals_job.cc
+++ b/content/browser/appcache/view_appcache_internals_job.cc
@@ -13,6 +13,7 @@
 #include "base/i18n/time_formatting.h"
 #include "base/logging.h"
 #include "base/memory/weak_ptr.h"
+#include "base/numerics/safe_math.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
@@ -596,7 +597,8 @@ class ViewEntryJob : public BaseInternalsJob,
     const int64 kLimit = 100 * 1000;
     int64 amount_to_read =
         std::min(kLimit, response_info->response_data_size());
-    response_data_ = new net::IOBuffer(amount_to_read);
+    response_data_ = new net::IOBuffer(
+        base::CheckedNumeric<size_t>(amount_to_read).ValueOrDie());
 
     reader_.reset(appcache_storage_->CreateResponseReader(
         manifest_url_, group_id_, response_id_));
diff --git a/device/hid/hid_connection_mac.cc b/device/hid/hid_connection_mac.cc
index de93cc6..57f56af 100644
--- a/device/hid/hid_connection_mac.cc
+++ b/device/hid/hid_connection_mac.cc
@@ -7,6 +7,7 @@
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/mac/foundation_util.h"
+#include "base/numerics/safe_math.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/stringprintf.h"
 #include "base/thread_task_runner_handle.h"
@@ -135,10 +136,12 @@ void HidConnectionMac::InputReportCallback(void* context,
   scoped_refptr<net::IOBufferWithSize> buffer;
   if (connection->device_info()->has_report_id()) {
     // report_id is already contained in report_bytes
-    buffer = new net::IOBufferWithSize(report_length);
+    buffer = new net::IOBufferWithSize(
+        base::CheckedNumeric<size_t>(report_length).ValueOrDie());
     memcpy(buffer->data(), report_bytes, report_length);
   } else {
-    buffer = new net::IOBufferWithSize(report_length + 1);
+    buffer = new net::IOBufferWithSize(
+        (base::CheckedNumeric<size_t>(report_length) + 1).ValueOrDie());
     buffer->data()[0] = 0;
     memcpy(buffer->data() + 1, report_bytes, report_length);
   }
diff --git a/google_apis/gcm/base/socket_stream.cc b/google_apis/gcm/base/socket_stream.cc
index edf723c..48c7da8 100644
--- a/google_apis/gcm/base/socket_stream.cc
+++ b/google_apis/gcm/base/socket_stream.cc
@@ -15,7 +15,7 @@ namespace {
 
 // TODO(zea): consider having dynamically-sized buffers if this becomes too
 // expensive.
-const uint32 kDefaultBufferSize = 8*1024;
+const size_t kDefaultBufferSize = 8*1024;
 
 }  // namespace
 
diff --git a/net/base/io_buffer.cc b/net/base/io_buffer.cc
index a375381..d722b44 100644
--- a/net/base/io_buffer.cc
+++ b/net/base/io_buffer.cc
@@ -5,15 +5,38 @@
 #include "net/base/io_buffer.h"
 
 #include "base/logging.h"
+#include "base/numerics/safe_math.h"
 
 namespace net {
 
+namespace {
+
+// TODO(eroman): IOBuffer is being converted to require buffer sizes and offsets
+// be specified as "size_t" rather than "int" (crbug.com/488553). To facilitate
+// this move (since LOTS of code needs to be updated), both "size_t" and "int
+// are being accepted. When using "size_t" this function ensures that it can be
+// safely converted to an "int" without truncation.
+void AssertValidBufferSize(size_t size) {
+  base::CheckedNumeric<int>(size).ValueOrDie();
+}
+
+void AssertValidBufferSize(int size) {
+  CHECK_GE(size, 0);
+}
+
+}  // namespace
+
 IOBuffer::IOBuffer()
     : data_(NULL) {
 }
 
 IOBuffer::IOBuffer(int buffer_size) {
-  CHECK_GE(buffer_size, 0);
+  AssertValidBufferSize(buffer_size);
+  data_ = new char[buffer_size];
+}
+
+IOBuffer::IOBuffer(size_t buffer_size) {
+  AssertValidBufferSize(buffer_size);
   data_ = new char[buffer_size];
 }
 
@@ -29,11 +52,22 @@ IOBuffer::~IOBuffer() {
 IOBufferWithSize::IOBufferWithSize(int size)
     : IOBuffer(size),
       size_(size) {
+  AssertValidBufferSize(size);
+}
+
+IOBufferWithSize::IOBufferWithSize(size_t size) : IOBuffer(size), size_(size) {
+  // Note: Size check is done in superclass' constructor.
 }
 
 IOBufferWithSize::IOBufferWithSize(char* data, int size)
     : IOBuffer(data),
       size_(size) {
+  AssertValidBufferSize(size);
+}
+
+IOBufferWithSize::IOBufferWithSize(char* data, size_t size)
+    : IOBuffer(data), size_(size) {
+  AssertValidBufferSize(size);
 }
 
 IOBufferWithSize::~IOBufferWithSize() {
@@ -42,13 +76,13 @@ IOBufferWithSize::~IOBufferWithSize() {
 StringIOBuffer::StringIOBuffer(const std::string& s)
     : IOBuffer(static_cast<char*>(NULL)),
       string_data_(s) {
-  CHECK_LT(s.size(), static_cast<size_t>(INT_MAX));
+  AssertValidBufferSize(s.size());
   data_ = const_cast<char*>(string_data_.data());
 }
 
 StringIOBuffer::StringIOBuffer(scoped_ptr<std::string> s)
     : IOBuffer(static_cast<char*>(NULL)) {
-  CHECK_LT(s->size(), static_cast<size_t>(INT_MAX));
+  AssertValidBufferSize(s->size());
   string_data_.swap(*s.get());
   data_ = const_cast<char*>(string_data_.data());
 }
@@ -64,6 +98,12 @@ DrainableIOBuffer::DrainableIOBuffer(IOBuffer* base, int size)
       base_(base),
       size_(size),
       used_(0) {
+  AssertValidBufferSize(size);
+}
+
+DrainableIOBuffer::DrainableIOBuffer(IOBuffer* base, size_t size)
+    : IOBuffer(base->data()), base_(base), size_(size), used_(0) {
+  AssertValidBufferSize(size);
 }
 
 void DrainableIOBuffer::DidConsume(int bytes) {
diff --git a/net/base/io_buffer.h b/net/base/io_buffer.h
index 04bbc88..83f81a8 100644
--- a/net/base/io_buffer.h
+++ b/net/base/io_buffer.h
@@ -73,7 +73,10 @@ namespace net {
 class NET_EXPORT IOBuffer : public base::RefCountedThreadSafe<IOBuffer> {
  public:
   IOBuffer();
+
+  // TODO(eroman): Deprecated. Use the size_t flavor instead. crbug.com/488553
   explicit IOBuffer(int buffer_size);
+  explicit IOBuffer(size_t buffer_size);
 
   char* data() { return data_; }
 
@@ -95,15 +98,20 @@ class NET_EXPORT IOBuffer : public base::RefCountedThreadSafe<IOBuffer> {
 // argument to IO functions. Please keep using IOBuffer* for API declarations.
 class NET_EXPORT IOBufferWithSize : public IOBuffer {
  public:
+  // TODO(eroman): Deprecated. Use the size_t flavor instead. crbug.com/488553
   explicit IOBufferWithSize(int size);
+  explicit IOBufferWithSize(size_t size);
 
   int size() const { return size_; }
 
  protected:
+  // TODO(eroman): Deprecated. Use the size_t flavor instead. crbug.com/488553
+  IOBufferWithSize(char* data, int size);
+
   // Purpose of this constructor is to give a subclass access to the base class
   // constructor IOBuffer(char*) thus allowing subclass to use underlying
   // memory it does not own.
-  IOBufferWithSize(char* data, int size);
+  IOBufferWithSize(char* data, size_t size);
   ~IOBufferWithSize() override;
 
   int size_;
@@ -143,7 +151,9 @@ 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);
 
   // DidConsume() changes the |data_| pointer so that |data_| always points
   // to the first unconsumed byte.
diff --git a/net/quic/quic_http_stream.cc b/net/quic/quic_http_stream.cc
index ac73d67..cce1512 100644
--- a/net/quic/quic_http_stream.cc
+++ b/net/quic/quic_http_stream.cc
@@ -146,7 +146,8 @@ int QuicHttpStream::SendRequest(const HttpRequestHeaders& request_headers,
     //       request_body_stream_->is_chunked()))
     // Use 10 packets as the body buffer size to give enough space to
     // help ensure we don't often send out partial packets.
-    raw_request_body_buf_ = new IOBufferWithSize(10 * kMaxPacketSize);
+    raw_request_body_buf_ =
+        new IOBufferWithSize(static_cast<size_t>(10 * kMaxPacketSize));
     // The request body buffer is empty at first.
     request_body_buf_ = new DrainableIOBuffer(raw_request_body_buf_.get(), 0);
   }
diff --git a/net/quic/quic_packet_reader.cc b/net/quic/quic_packet_reader.cc
index 5087870c..93451da 100644
--- a/net/quic/quic_packet_reader.cc
+++ b/net/quic/quic_packet_reader.cc
@@ -16,7 +16,7 @@ QuicPacketReader::QuicPacketReader(DatagramClientSocket* socket,
       visitor_(visitor),
       read_pending_(false),
       num_packets_read_(0),
-      read_buffer_(new IOBufferWithSize(kMaxPacketSize)),
+      read_buffer_(new IOBufferWithSize(static_cast<size_t>(kMaxPacketSize))),
       net_log_(net_log),
       weak_factory_(this) {
 }
diff --git a/net/socket/ssl_client_socket_nss.cc b/net/socket/ssl_client_socket_nss.cc
index 7c5264d..c319950 100644
--- a/net/socket/ssl_client_socket_nss.cc
+++ b/net/socket/ssl_client_socket_nss.cc
@@ -1762,7 +1762,7 @@ int SSLClientSocketNSS::Core::BufferSend() {
     // return ERR_ABORTED. See https://crbug.com/381160.
     return ERR_ABORTED;
   }
-  const unsigned int len = len1 + len2;
+  const size_t len = len1 + len2;
 
   int rv = 0;
   if (len) {
diff --git a/net/socket/ssl_server_socket_nss.cc b/net/socket/ssl_server_socket_nss.cc
index 6f505aff..4483c91 100644
--- a/net/socket/ssl_server_socket_nss.cc
+++ b/net/socket/ssl_server_socket_nss.cc
@@ -560,7 +560,7 @@ int SSLServerSocketNSS::BufferSend(void) {
     // The error code itself is ignored, so just return ERR_ABORTED.
     return ERR_ABORTED;
   }
-  const unsigned int len = len1 + len2;
+  const size_t len = len1 + len2;
 
   int rv = 0;
   if (len) {
diff --git a/net/websockets/websocket_deflater.cc b/net/websockets/websocket_deflater.cc
index a4c56bc..9144c01 100644
--- a/net/websockets/websocket_deflater.cc
+++ b/net/websockets/websocket_deflater.cc
@@ -97,10 +97,11 @@ void WebSocketDeflater::PushSyncMark() {
 }
 
 scoped_refptr<IOBufferWithSize> WebSocketDeflater::GetOutput(size_t size) {
+  size_t length_to_copy = std::min(size, buffer_.size());
   std::deque<char>::iterator begin = buffer_.begin();
-  std::deque<char>::iterator end = begin + std::min(size, buffer_.size());
+  std::deque<char>::iterator end = begin + length_to_copy;
 
-  scoped_refptr<IOBufferWithSize> result = new IOBufferWithSize(end - begin);
+  scoped_refptr<IOBufferWithSize> result = new IOBufferWithSize(length_to_copy);
   std::copy(begin, end, result->data());
   buffer_.erase(begin, end);
   return result;
diff --git a/net/websockets/websocket_inflater.cc b/net/websockets/websocket_inflater.cc
index 1d6122b..7767821 100644
--- a/net/websockets/websocket_inflater.cc
+++ b/net/websockets/websocket_inflater.cc
@@ -18,11 +18,11 @@ namespace {
 
 class ShrinkableIOBufferWithSize : public IOBufferWithSize {
  public:
-  explicit ShrinkableIOBufferWithSize(int size)
-      : IOBufferWithSize(size) {}
+  explicit ShrinkableIOBufferWithSize(size_t size) : IOBufferWithSize(size) {}
 
   void Shrink(int new_size) {
-    DCHECK_LE(new_size, size_);
+    CHECK_GE(new_size, 0);
+    CHECK_LE(new_size, size_);
     size_ = new_size;
   }