Finish spanifying IOBuffer.

This replaces the protected `size_` and `data_` fields with a single
private `span_` field. It also updates the few remaining subclasses
that accessed the old fields directly not to do so.

Fixed: 396621713
Change-Id: I87c365439033eca7637678ca868c0db52f43757d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6287588
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Commit-Queue: mmenke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1425823}

net/base/io_buffer.cc

@@ -26,25 +26,22 @@ void IOBuffer::AssertValidBufferSize(size_t size) {
 
 IOBuffer::IOBuffer() = default;
 
-IOBuffer::IOBuffer(base::span<char> data)
-    : data_(data.data()), size_(data.size()) {
-  AssertValidBufferSize(size_);
-}
+IOBuffer::IOBuffer(base::span<char> span)
+    : IOBuffer(base::as_writable_bytes(span)) {}
 
-IOBuffer::IOBuffer(base::span<uint8_t> data)
-    : IOBuffer(base::as_writable_chars(data)) {}
+IOBuffer::IOBuffer(base::span<uint8_t> span) : span_(span) {
+  AssertValidBufferSize(span_.size());
+}
 
 IOBuffer::~IOBuffer() = default;
 
 void IOBuffer::SetSpan(base::span<uint8_t> span) {
   AssertValidBufferSize(span.size());
-  data_ = base::as_writable_chars(span).data();
-  size_ = span.size();
+  span_ = span;
 }
 
 void IOBuffer::ClearSpan() {
-  data_ = nullptr;
-  size_ = 0;
+  span_ = base::span<uint8_t>();
 }
 
 IOBufferWithSize::IOBufferWithSize() = default;
@@ -105,7 +102,7 @@ void DrainableIOBuffer::SetOffset(int bytes) {
   CHECK_GE(bytes, 0);
   // Length from the start of `base_` to the end of the buffer passed in to the
   // constructor isn't stored anywhere, so need to calculate it.
-  int length = size_ + used_;
+  int length = size() + used_;
   CHECK_LE(bytes, length);
   used_ = bytes;
   SetSpan(base_->span().subspan(base::checked_cast<size_t>(used_),
@@ -149,7 +146,7 @@ void GrowableIOBuffer::set_offset(int offset) {
 
 void GrowableIOBuffer::DidConsume(int bytes) {
   CHECK_LE(0, bytes);
-  CHECK_LE(bytes, size_);
+  CHECK_LE(bytes, size());
   set_offset(offset_ + bytes);
 }
 

net/base/io_buffer.h

@@ -15,7 +15,7 @@
 #include "base/containers/heap_array.h"
 #include "base/containers/span.h"
 #include "base/memory/free_deleter.h"
-#include "base/memory/raw_ptr.h"
+#include "base/memory/raw_span.h"
 #include "base/memory/ref_counted.h"
 #include "net/base/net_export.h"
 
@@ -89,21 +89,21 @@ class NET_EXPORT IOBuffer : public base::RefCountedThreadSafe<IOBuffer> {
   // take an IOBuffer also take a size indicated the number of IOBuffer bytes to
   // use from the start of bytes(). That number must be no more than the size()
   // of the passed in IOBuffer.
-  int size() const { return size_; }
-
-  char* data() { return data_; }
-  const char* data() const { return data_; }
-
-  uint8_t* bytes() { return reinterpret_cast<uint8_t*>(data()); }
-  const uint8_t* bytes() const {
-    return reinterpret_cast<const uint8_t*>(data());
+  int size() const {
+    // SetSpan() ensures this fits in an int.
+    return static_cast<int>(span_.size());
   }
 
-  base::span<uint8_t> span() {
-    return UNSAFE_TODO(base::span(bytes(), static_cast<size_t>(size_)));
-  }
+  char* data() { return reinterpret_cast<char*>(bytes()); }
+  const char* data() const { return reinterpret_cast<const char*>(bytes()); }
+
+  uint8_t* bytes() { return span_.data(); }
+  const uint8_t* bytes() const { return span_.data(); }
+
+  base::span<uint8_t> span() { return span_; }
   base::span<const uint8_t> span() const {
-    return UNSAFE_TODO(base::span(bytes(), static_cast<size_t>(size_)));
+    // Converts a const base::span<uint8_t> to a base::span<const uint8_t>.
+    return base::as_byte_span(span_);
   }
 
  protected:
@@ -112,16 +112,12 @@ class NET_EXPORT IOBuffer : public base::RefCountedThreadSafe<IOBuffer> {
   static void AssertValidBufferSize(size_t size);
 
   IOBuffer();
-  explicit IOBuffer(base::span<char> data);
-  explicit IOBuffer(base::span<uint8_t> data);
+  explicit IOBuffer(base::span<char> span);
+  explicit IOBuffer(base::span<uint8_t> span);
 
   virtual ~IOBuffer();
 
-  // Sets `data_` and `size_` based on span. CHECKs `size_` isn't too big to fit
-  // in an int.
-  //
-  // TODO(https://crbug.com/396621713): Replace `data_` and `size_` with a
-  // private `raw_span`, with this function being the only way to modify it.
+  // Sets `span_` to `span`. CHECKs if its size is too big to fit in an int.
   void SetSpan(base::span<uint8_t> span);
 
   // Like SetSpan(base::span<uint8_t>()), but without a size check. Particularly
@@ -129,8 +125,8 @@ class NET_EXPORT IOBuffer : public base::RefCountedThreadSafe<IOBuffer> {
   // reference checks.
   void ClearSpan();
 
-  raw_ptr<char, AllowPtrArithmetic> data_ = nullptr;
-  int size_ = 0;
+ private:
+  base::raw_span<uint8_t> span_;
 };
 
 // Class which owns its buffer and manages its destruction.

net/server/http_connection.cc

@@ -126,7 +126,6 @@ bool HttpConnection::QueuedWriteIOBuffer::Append(const std::string& data) {
 
   // If new data is the first pending data, updates data_.
   if (pending_data_.size() == 1) {
-    data_ = const_cast<char*>(pending_data_.front()->data());
     SetSpan(base::as_writable_bytes(base::span(*pending_data_.front())));
   }
   return true;

services/network/public/cpp/net_adapters.cc

@@ -62,7 +62,7 @@ NetToMojoIOBuffer::NetToMojoIOBuffer(
 NetToMojoIOBuffer::~NetToMojoIOBuffer() {
   // Avoid dangling ptr should this destructor remove the last reference
   // to `pending_buffer_`.
-  data_ = nullptr;
+  ClearSpan();
 }
 
 MojoToNetPendingBuffer::MojoToNetPendingBuffer(
@@ -118,7 +118,7 @@ MojoToNetIOBuffer::~MojoToNetIOBuffer() {
 
   // Prevent dangling ptr should this destructor remove the last reference
   // to `pending_buffer_`.
-  data_ = nullptr;
+  ClearSpan();
 }
 
 }  // namespace network

services/network/slop_bucket.cc

@@ -458,8 +458,7 @@ class SlopBucket::ChunkIOBuffer final : public net::IOBuffer {
     // that can be stored in a size_t.
     filled_up_to_ += bytes;
     CHECK_LE(filled_up_to_, chunk_size);
-    data_ += bytes;
-    size_ -= bytes;
+    SetSpan(span().subspan(bytes));
   }
 
   // Notes that `bytes` bytes have been written to the mojo data pipe and should
@@ -495,7 +494,7 @@ class SlopBucket::ChunkIOBuffer final : public net::IOBuffer {
  private:
   ~ChunkIOBuffer() override {
     // Prevent the base class from trying to free `data_`.
-    data_ = nullptr;
+    ClearSpan();
     // Return the chunk to the pool.
     Manager::Get().ReleaseChunk(std::move(base_));
   }