More deep-const-correctness for base::SharedMemoryMapping subtypes.

The code touched here wasn't consistent in its use of const, but
generally used it in a shallow-const fashion where even vending write
access to a block of memory could be done via a const method.

This causes problems when trying to enforce stricter lifetime checks on
writable spans, because either the code gets confused about whether the
accesses are read-only (it tries to look for the constness of the
returned pointers and can't figure out what to do if things don't
match everywhere) or it thinks something unsafe or non-sane is happening
(write access to rvalues makes no sense, for example, but read access
might in the context of a short-lived call).

Instead consistently model deep constness, which is compliant with
Chrome's style rules on const and fixes all these issues. Mostly, this
means changing const members/ref args to non-const ones.

Bug: 372381413
Change-Id: I2735c52fbf0f32b813055cbb46f7c15b09eb025f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5939406
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Owners-Override: Daniel Cheng <dcheng@chromium.org>
Auto-Submit: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Fred Shih <ffred@chromium.org>
Reviewed-by: Alex Gough <ajgo@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1373012}

base/memory/shared_memory_mapping.h

@@ -215,7 +215,8 @@ class BASE_EXPORT WritableSharedMemoryMapping : public SharedMemoryMapping {
   // Use `span(mapping)` to make a span of `uint8_t`, `GetMemoryAs<T>()` to
   // access the memory as a single `T` or `GetMemoryAsSpan<T>()` to access it as
   // an array of `T`.
-  uint8_t* data() const { return mapped_memory().data(); }
+  uint8_t* data() { return mapped_memory().data(); }
+  const uint8_t* data() const { return mapped_memory().data(); }
 
   // Iterate memory as bytes up to the end of its logical size.
   iterator begin() {
@@ -240,7 +241,8 @@ class BASE_EXPORT WritableSharedMemoryMapping : public SharedMemoryMapping {
   // of `uint8_t`, `GetMemoryAs<T>()` to access the memory as a single `T`, or
   // `GetMemoryAsSpan<T>()` to access it as an array of `T` or `data()` for an
   // unbounded pointer.
-  void* memory() const { return data(); }
+  void* memory() { return data(); }
+  const void* memory() const { return data(); }
 
   // Returns a pointer to a page-aligned T if the mapping is valid and large
   // enough to contain a T, or nullptr otherwise.

base/trace_event/process_memory_dump_unittest.cc

@@ -60,7 +60,7 @@ void Unmap(void* addr, size_t size) {
 }
 
 std::optional<size_t> CountResidentBytesInSharedMemory(
-    const WritableSharedMemoryMapping& mapping) {
+    WritableSharedMemoryMapping& mapping) {
   // SAFETY: We need the actual mapped memory size here. There's no public
   // method to get this as a span, so we need to construct it unsafely. The
   // mapped_size() is larger than `mem.size()` but represents the actual memory

cc/resources/cross_thread_shared_bitmap.h

@@ -5,6 +5,8 @@
 #ifndef CC_RESOURCES_CROSS_THREAD_SHARED_BITMAP_H_
 #define CC_RESOURCES_CROSS_THREAD_SHARED_BITMAP_H_
 
+#include <utility>
+
 #include "base/memory/read_only_shared_memory_region.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/shared_memory_mapping.h"
@@ -34,7 +36,8 @@ class CC_EXPORT CrossThreadSharedBitmap
   const base::ReadOnlySharedMemoryRegion& shared_region() const {
     return region_;
   }
-  void* memory() const {
+  void* memory() { return const_cast<void*>(std::as_const(*this).memory()); }
+  const void* memory() const {
     // TODO(crbug.com/355003196): This returns an unsafe unbounded pointer. The
     // return type here should be changed to a span, then return span(mapping_).
     return mapping_.data();

components/nacl/loader/nacl_listener.h

@@ -53,9 +53,7 @@ class NaClListener : public IPC::Listener {
   }
 #endif
 
-  void* crash_info_shmem_memory() const {
-    return crash_info_shmem_mapping_.memory();
-  }
+  void* crash_info_shmem_memory() { return crash_info_shmem_mapping_.memory(); }
 
   NaClTrustedListener* trusted_listener() const {
     return trusted_listener_.get();

components/visitedlink/browser/partitioned_visitedlink_writer.cc

@@ -772,7 +772,7 @@ std::optional<uint64_t> PartitionedVisitedLinkWriter::GetOrAddOriginSalt(
 // static
 VisitedLinkCommon::Fingerprint*
 PartitionedVisitedLinkWriter::GetHashTableFromMapping(
-    const base::WritableSharedMemoryMapping& hash_table_mapping) {
+    base::WritableSharedMemoryMapping& hash_table_mapping) {
   DCHECK(hash_table_mapping.IsValid());
   // Our table pointer is just the data immediately following the header.
   return reinterpret_cast<Fingerprint*>(

components/visitedlink/browser/partitioned_visitedlink_writer.h

@@ -241,7 +241,7 @@ class PartitionedVisitedLinkWriter : public VisitedLinkCommon {
   // Returns a pointer to the start of the hash table, given the mapping
   // containing the hash table.
   static Fingerprint* GetHashTableFromMapping(
-      const base::WritableSharedMemoryMapping& hash_table_mapping);
+      base::WritableSharedMemoryMapping& hash_table_mapping);
 
   // Returns the default table size. It can be overridden in unit tests.
   uint32_t DefaultTableSize() const;

components/visitedlink/browser/visitedlink_writer.cc

@@ -1207,7 +1207,7 @@ void VisitedLinkWriter::TableBuilder::OnCompleteMainThread() {
 
 // static
 VisitedLinkCommon::Fingerprint* VisitedLinkWriter::GetHashTableFromMapping(
-    const base::WritableSharedMemoryMapping& hash_table_mapping) {
+    base::WritableSharedMemoryMapping& hash_table_mapping) {
   DCHECK(hash_table_mapping.IsValid());
   // Our table pointer is just the data immediately following the header.
   return reinterpret_cast<Fingerprint*>(

components/visitedlink/browser/visitedlink_writer.h

@@ -403,7 +403,7 @@ class VisitedLinkWriter : public VisitedLinkCommon {
   // Returns a pointer to the start of the hash table, given the mapping
   // containing the hash table.
   static Fingerprint* GetHashTableFromMapping(
-      const base::WritableSharedMemoryMapping& hash_table_mapping);
+      base::WritableSharedMemoryMapping& hash_table_mapping);
 
   // Reference to the browser context that this object belongs to
   // (it knows the path to where the data is stored)

content/browser/webrtc/webrtc_video_capture_service_browsertest.cc

@@ -323,7 +323,7 @@ class SharedMemoryDeviceExerciser : public VirtualDeviceExerciser,
     info->metadata = metadata;
     info->strides = strides_.Clone();
 
-    const base::WritableSharedMemoryMapping& outgoing_buffer =
+    base::WritableSharedMemoryMapping& outgoing_buffer =
         outgoing_buffer_id_to_buffer_map_.at(buffer_id);
 
     static int frame_count = 0;

gpu/command_buffer/common/buffer.cc

@@ -45,7 +45,7 @@ MemoryBufferBacking::MemoryBufferBacking(uint32_t size, uint32_t alignment)
 
 MemoryBufferBacking::~MemoryBufferBacking() = default;
 
-void* MemoryBufferBacking::GetMemory() const {
+const void* MemoryBufferBacking::GetMemory() const {
   return alignment_ > 0 ? base::bits::AlignUp(memory_.get(), alignment_)
                         : memory_.get();
 }
@@ -74,7 +74,7 @@ base::UnguessableToken SharedMemoryBufferBacking::GetGUID() const {
   return shared_memory_region_.GetGUID();
 }
 
-void* SharedMemoryBufferBacking::GetMemory() const {
+const void* SharedMemoryBufferBacking::GetMemory() const {
   return shared_memory_mapping_.memory();
 }
 

gpu/command_buffer/common/buffer.h

@@ -9,6 +9,7 @@
 #include <stdint.h>
 
 #include <memory>
+#include <utility>
 
 #include "base/memory/raw_ptr.h"
 #include "base/memory/ref_counted.h"
@@ -23,7 +24,10 @@ class GPU_EXPORT BufferBacking {
   virtual ~BufferBacking() = default;
   virtual const base::UnsafeSharedMemoryRegion& shared_memory_region() const;
   virtual base::UnguessableToken GetGUID() const;
-  virtual void* GetMemory() const = 0;
+  void* GetMemory() {
+    return const_cast<void*>(std::as_const(*this).GetMemory());
+  }
+  virtual const void* GetMemory() const = 0;
   virtual uint32_t GetSize() const = 0;
 };
 
@@ -35,7 +39,7 @@ class GPU_EXPORT MemoryBufferBacking : public BufferBacking {
   MemoryBufferBacking& operator=(const MemoryBufferBacking&) = delete;
 
   ~MemoryBufferBacking() override;
-  void* GetMemory() const override;
+  const void* GetMemory() const override;
   uint32_t GetSize() const override;
 
  private:
@@ -58,7 +62,7 @@ class GPU_EXPORT SharedMemoryBufferBacking : public BufferBacking {
   ~SharedMemoryBufferBacking() override;
   const base::UnsafeSharedMemoryRegion& shared_memory_region() const override;
   base::UnguessableToken GetGUID() const override;
-  void* GetMemory() const override;
+  const void* GetMemory() const override;
   uint32_t GetSize() const override;
 
  private:
@@ -75,7 +79,8 @@ class GPU_EXPORT Buffer : public base::RefCountedThreadSafe<Buffer> {
   Buffer& operator=(const Buffer&) = delete;
 
   BufferBacking* backing() const { return backing_.get(); }
-  void* memory() const { return memory_; }
+  void* memory() { return memory_; }
+  const void* memory() const { return memory_; }
   uint32_t size() const { return size_; }
 
   // Returns nullptr if the address overflows the memory.

gpu/ipc/client/command_buffer_proxy_impl.cc

@@ -672,8 +672,9 @@ void CommandBufferProxyImpl::TryUpdateStateDontReportError() {
     shared_state()->Read(&last_state_);
 }
 
-gpu::CommandBufferSharedState* CommandBufferProxyImpl::shared_state() const {
-  return reinterpret_cast<gpu::CommandBufferSharedState*>(
+const gpu::CommandBufferSharedState* CommandBufferProxyImpl::shared_state()
+    const {
+  return reinterpret_cast<const gpu::CommandBufferSharedState*>(
       shared_state_mapping_.memory());
 }
 

gpu/ipc/client/command_buffer_proxy_impl.h

@@ -234,7 +234,11 @@ class GPU_EXPORT CommandBufferProxyImpl : public gpu::CommandBuffer,
   void DisconnectChannel();
 
   // The shared memory area used to update state.
-  gpu::CommandBufferSharedState* shared_state() const;
+  gpu::CommandBufferSharedState* shared_state() {
+    return const_cast<gpu::CommandBufferSharedState*>(
+        std::as_const(*this).shared_state());
+  }
+  const gpu::CommandBufferSharedState* shared_state() const;
 
   base::HistogramBase* GetUMAHistogramEnsureWorkVisibleDuration();
 

media/base/user_input_monitor.cc

@@ -25,7 +25,7 @@ uint32_t ReadKeyPressMonitorCount(
 }
 
 void WriteKeyPressMonitorCount(
-    const base::WritableSharedMemoryMapping& writable_mapping,
+    base::WritableSharedMemoryMapping& writable_mapping,
     uint32_t count) {
   if (!writable_mapping.IsValid())
     return;

media/base/user_input_monitor.h

@@ -26,7 +26,7 @@ namespace media {
 uint32_t MEDIA_EXPORT
 ReadKeyPressMonitorCount(const base::ReadOnlySharedMemoryMapping& shmem);
 void MEDIA_EXPORT
-WriteKeyPressMonitorCount(const base::WritableSharedMemoryMapping& shmem,
+WriteKeyPressMonitorCount(base::WritableSharedMemoryMapping& shmem,
                           uint32_t count);
 
 // Base class for audio:: and media:: UserInputMonitor implementations.

media/base/user_input_monitor_linux.cc

@@ -16,7 +16,7 @@ namespace media {
 namespace {
 
 using WriteKeyPressCallback =
-    base::RepeatingCallback<void(const base::WritableSharedMemoryMapping& shmem,
+    base::RepeatingCallback<void(base::WritableSharedMemoryMapping& shmem,
                                  uint32_t count)>;
 
 // Provides a unified interface for using user input monitors of unrelated

media/gpu/mac/vt_video_encode_accelerator_mac.mm

@@ -397,7 +397,7 @@ struct VTVideoEncodeAccelerator::BitstreamBufferRef {
   BitstreamBufferRef& operator=(const BitstreamBufferRef&) = delete;
 
   const int32_t id;
-  const base::WritableSharedMemoryMapping mapping;
+  base::WritableSharedMemoryMapping mapping;
   const size_t size;
 };
 

media/gpu/windows/media_foundation_video_encode_accelerator_win.cc

@@ -863,7 +863,7 @@ struct MediaFoundationVideoEncodeAccelerator::BitstreamBufferRef {
   BitstreamBufferRef& operator=(const BitstreamBufferRef&) = delete;
 
   const int32_t id;
-  const base::WritableSharedMemoryMapping mapping;
+  base::WritableSharedMemoryMapping mapping;
   const size_t size;
 };
 

media/mojo/services/mojo_cdm_allocator.cc

@@ -76,7 +76,7 @@ class MojoCdmBuffer final : public cdm::Buffer {
 
   uint32_t Size() const final { return size_; }
 
-  const base::MappedReadOnlyRegion& Region() const { return *mapped_region_; }
+  base::MappedReadOnlyRegion& Region() { return *mapped_region_; }
   std::unique_ptr<base::MappedReadOnlyRegion> TakeRegion() {
     return std::move(mapped_region_);
   }
@@ -236,8 +236,8 @@ void MojoCdmAllocator::AddRegionToAvailableMap(
   available_regions_.insert({capacity, std::move(mapped_region)});
 }
 
-const base::MappedReadOnlyRegion& MojoCdmAllocator::GetRegionForTesting(
-    cdm::Buffer* buffer) const {
+base::MappedReadOnlyRegion& MojoCdmAllocator::GetRegionForTesting(
+    cdm::Buffer* buffer) {
   MojoCdmBuffer* mojo_buffer = static_cast<MojoCdmBuffer*>(buffer);
   return mojo_buffer->Region();
 }

media/mojo/services/mojo_cdm_allocator.h

@@ -54,8 +54,7 @@ class MEDIA_MOJO_EXPORT MojoCdmAllocator final : public CdmAllocator {
 
   // Returns the base::MappedReadOnlyRegion for a cdm::Buffer allocated by this
   // class.
-  const base::MappedReadOnlyRegion& GetRegionForTesting(
-      cdm::Buffer* buffer) const;
+  base::MappedReadOnlyRegion& GetRegionForTesting(cdm::Buffer* buffer);
 
   // Returns the number of buffers in |available_regions_|.
   size_t GetAvailableRegionCountForTesting();

media/mojo/services/mojo_cdm_allocator_unittest.cc

@@ -36,7 +36,7 @@ class MojoCdmAllocatorTest : public testing::Test {
     return allocator_.CreateCdmVideoFrame();
   }
 
-  const base::MappedReadOnlyRegion& GetRegion(cdm::Buffer* buffer) {
+  base::MappedReadOnlyRegion& GetRegion(cdm::Buffer* buffer) {
     return allocator_.GetRegionForTesting(buffer);
   }
 

media/video/video_encode_accelerator_adapter.cc

@@ -1095,7 +1095,7 @@ VideoEncodeAcceleratorAdapter::PrepareCpuFrame(
                               : src_frame;
   auto shared_frame = VideoFrame::WrapExternalData(
       PIXEL_FORMAT_I420, dest_coded_size, dest_visible_rect,
-      dest_visible_rect.size(), static_cast<uint8_t*>(mapping->memory()),
+      dest_visible_rect.size(), static_cast<const uint8_t*>(mapping->memory()),
       mapping->size(), src_frame->timestamp());
 
   if (!shared_frame || !mapped_src_frame)

ppapi/proxy/ppapi_command_buffer_proxy.cc

@@ -301,7 +301,7 @@ void PpapiCommandBufferProxy::TryUpdateState() {
     shared_state()->Read(&last_state_);
 }
 
-gpu::CommandBufferSharedState* PpapiCommandBufferProxy::shared_state() const {
+gpu::CommandBufferSharedState* PpapiCommandBufferProxy::shared_state() {
   return reinterpret_cast<gpu::CommandBufferSharedState*>(
       shared_state_mapping_.memory());
 }

ppapi/proxy/ppapi_command_buffer_proxy.h

@@ -92,7 +92,7 @@ class PPAPI_PROXY_EXPORT PpapiCommandBufferProxy : public gpu::CommandBuffer,
   void TryUpdateState();
 
   // The shared memory area used to update state.
-  gpu::CommandBufferSharedState* shared_state() const;
+  gpu::CommandBufferSharedState* shared_state();
 
   void FlushInternal();
 

services/audio/input_sync_writer.cc

@@ -376,7 +376,7 @@ bool InputSyncWriter::SignalDataWrittenAndUpdateCounters() {
 }
 
 media::AudioInputBuffer* InputSyncWriter::GetSharedInputBuffer(
-    uint32_t segment_id) const {
+    uint32_t segment_id) {
   uint8_t* ptr = static_cast<uint8_t*>(shared_memory_mapping_.memory());
   CHECK_LT(segment_id, audio_buses_.size());
   ptr += segment_id * shared_memory_segment_size_;

services/audio/input_sync_writer.h

@@ -103,7 +103,7 @@ class InputSyncWriter final : public InputController::SyncWriter {
   // false if failure.
   bool SignalDataWrittenAndUpdateCounters();
 
-  media::AudioInputBuffer* GetSharedInputBuffer(uint32_t segment_id) const;
+  media::AudioInputBuffer* GetSharedInputBuffer(uint32_t segment_id);
 
   const base::RepeatingCallback<void(const std::string&)> log_callback_;
 
@@ -112,7 +112,7 @@ class InputSyncWriter final : public InputController::SyncWriter {
 
   // Shared memory for audio data and associated metadata.
   base::ReadOnlySharedMemoryRegion shared_memory_region_;
-  const base::WritableSharedMemoryMapping shared_memory_mapping_;
+  base::WritableSharedMemoryMapping shared_memory_mapping_;
 
   // The size in bytes of a single audio segment in the shared memory.
   const uint32_t shared_memory_segment_size_;

services/audio/sync_reader_unittest.cc

@@ -76,7 +76,7 @@ TEST_P(SyncReaderBitstreamTest, BitstreamBufferOverflow_DoesNotWriteOOB) {
   auto socket = std::make_unique<base::CancelableSyncSocket>();
   SyncReader reader(base::BindRepeating(&NoLog), params, socket.get());
   ASSERT_TRUE(reader.IsValid());
-  const base::WritableSharedMemoryMapping shmem =
+  base::WritableSharedMemoryMapping shmem =
       reader.TakeSharedMemoryRegion().Map();
   ASSERT_TRUE(shmem.IsValid());
   auto* const buffer = shmem.GetMemoryAs<media::AudioOutputBuffer>();

third_party/blink/renderer/platform/peerconnection/rtc_video_encoder.cc

@@ -189,7 +189,7 @@ class RefCountedWritableSharedMemoryMapping
       const RefCountedWritableSharedMemoryMapping&) = delete;
 
   const unsigned char* front() const {
-    return static_cast<unsigned char*>(mapping_.memory());
+    return static_cast<const unsigned char*>(mapping_.memory());
   }
   unsigned char* front() {
     return static_cast<unsigned char*>(mapping_.memory());
@@ -200,7 +200,7 @@ class RefCountedWritableSharedMemoryMapping
   friend class ThreadSafeRefCounted<RefCountedWritableSharedMemoryMapping>;
   ~RefCountedWritableSharedMemoryMapping() = default;
 
-  const base::WritableSharedMemoryMapping mapping_;
+  base::WritableSharedMemoryMapping mapping_;
 };
 
 class EncodedDataWrapper : public webrtc::EncodedImageBufferInterface {

ui/base/x/x11_user_input_monitor.h

@@ -24,8 +24,9 @@ class COMPONENT_EXPORT(UI_BASE_X) XUserInputMonitor
     : public base::CurrentThread::DestructionObserver,
       public x11::EventObserver {
  public:
-  using WriteKeyPressCallback = base::RepeatingCallback<
-      void(const base::WritableSharedMemoryMapping& shmem, uint32_t count)>;
+  using WriteKeyPressCallback =
+      base::RepeatingCallback<void(base::WritableSharedMemoryMapping& shmem,
+                                   uint32_t count)>;
 
   explicit XUserInputMonitor(
       const scoped_refptr<base::SingleThreadTaskRunner>& io_task_runner);

ui/ozone/public/platform_user_input_monitor.h

@@ -22,8 +22,9 @@ class COMPONENT_EXPORT(OZONE_BASE) PlatformUserInputMonitor {
  public:
   // Via this callback, the platform implementation of the monitor gets the
   // WriteKeyPressMonitorCount defined in media/base/user_input_monitor.h.
-  using WriteKeyPressCallback = base::RepeatingCallback<
-      void(const base::WritableSharedMemoryMapping& shmem, uint32_t count)>;
+  using WriteKeyPressCallback =
+      base::RepeatingCallback<void(base::WritableSharedMemoryMapping& shmem,
+                                   uint32_t count)>;
 
   PlatformUserInputMonitor();
   PlatformUserInputMonitor(const PlatformUserInputMonitor&) = delete;

ui/surface/transport_dib.cc

@@ -93,7 +93,7 @@ bool TransportDIB::Map() {
   return true;
 }
 
-void* TransportDIB::memory() const {
+const void* TransportDIB::memory() const {
   return shm_mapping_.IsValid() ? shm_mapping_.memory() : nullptr;
 }
 

ui/surface/transport_dib.h

@@ -6,7 +6,9 @@
 #define UI_SURFACE_TRANSPORT_DIB_H_
 
 #include <stddef.h>
+
 #include <memory>
+#include <utility>
 
 #include "base/memory/shared_memory_mapping.h"
 #include "base/memory/unsafe_shared_memory_region.h"
@@ -55,7 +57,8 @@ class SURFACE_EXPORT TransportDIB {
   bool Map();
 
   // Return a pointer to the shared memory.
-  void* memory() const;
+  void* memory() { return const_cast<void*>(std::as_const(*this).memory()); }
+  const void* memory() const;
 
   // Return the maximum size of the shared memory. This is not the amount of
   // data which is valid, you have to know that via other means, this is simply