Remove media::UnalignedSharedMemory.

This was previously needed since //base's shmem implementation required
mapping at page-aligned offsets. However, several different places
throughout Chrome needed non-page-aligned offsets, so the //base API
supports it directly now.

For the most part, the changes in this CL follow as a natural result of
removing media::UnalignedSharedMemory usage from media::BitstreamBuffer
and media::DecoderBuffer. media::UnalignedSharedMemory internally
contained the shmem region and potentially the shmem mapping as well.
Usage has been split apart into region and mapping as appropriate.

Many parts of the code used internal implementation details of base's
shmem; these have been updated to use the non-subtle versions of the API
as much as possible, though there are many ARC++ and/or CrOS endpoints
that serialize raw Mojo handles or fds directly that cannot easily be
updated.

Bug: 795291, 849207, 1323416
Change-Id: Icbaa854930aac9f5409283a70a7ab6da48b60ad0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3632832
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Reviewed-by: Miguel Casas-Sanchez <mcasas@chromium.org>
Reviewed-by: Alex Gough <ajgo@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1002338}

ash/components/arc/video_accelerator/gpu_arc_video_decode_accelerator.cc

@@ -12,6 +12,8 @@
 #include "ash/components/arc/video_accelerator/protected_buffer_manager.h"
 #include "base/bind.h"
 #include "base/files/scoped_file.h"
+#include "base/memory/platform_shared_memory_region.h"
+#include "base/memory/unsafe_shared_memory_region.h"
 #include "base/metrics/histogram_functions.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/posix/eintr_wrapper.h"
@@ -526,13 +528,13 @@ void GpuArcVideoDecodeAccelerator::Decode(
   DCHECK(secure_mode_);
   DCHECK(!*secure_mode_);
   ContinueDecode(std::move(bitstream_buffer), std::move(handle_fd),
-                 base::subtle::PlatformSharedMemoryRegion());
+                 base::UnsafeSharedMemoryRegion());
 }
 
 void GpuArcVideoDecodeAccelerator::ContinueDecode(
     mojom::BitstreamBufferPtr bitstream_buffer,
     base::ScopedFD handle_fd,
-    base::subtle::PlatformSharedMemoryRegion shm_region) {
+    base::UnsafeSharedMemoryRegion shm_region) {
   DVLOGF(4);
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
   if (!vda_) {
@@ -583,10 +585,11 @@ void GpuArcVideoDecodeAccelerator::ContinueDecode(
       return;
     }
     DCHECK(!shm_region.IsValid());
-    shm_region = base::subtle::PlatformSharedMemoryRegion::Take(
-        std::move(handle_fd),
-        base::subtle::PlatformSharedMemoryRegion::Mode::kUnsafe, handle_size,
-        base::UnguessableToken::Create());
+    shm_region = base::UnsafeSharedMemoryRegion::Deserialize(
+        base::subtle::PlatformSharedMemoryRegion::Take(
+            std::move(handle_fd),
+            base::subtle::PlatformSharedMemoryRegion::Mode::kUnsafe,
+            handle_size, base::UnguessableToken::Create()));
   }
 
   if (!shm_region.IsValid()) {

ash/components/arc/video_accelerator/gpu_arc_video_decode_accelerator.h

@@ -13,6 +13,7 @@
 #include "ash/components/arc/mojom/video_decode_accelerator.mojom.h"
 #include "base/callback_forward.h"
 #include "base/files/scoped_file.h"
+#include "base/memory/unsafe_shared_memory_region.h"
 #include "base/threading/thread_checker.h"
 #include "gpu/config/gpu_driver_bug_workarounds.h"
 #include "gpu/config/gpu_preferences.h"
@@ -110,7 +111,7 @@ class GpuArcVideoDecodeAccelerator
   // directly.
   void ContinueDecode(mojom::BitstreamBufferPtr bitstream_buffer,
                       base::ScopedFD handle_fd,
-                      base::subtle::PlatformSharedMemoryRegion shm_region);
+                      base::UnsafeSharedMemoryRegion shm_region);
 
   // Posted as a task after getting the result of the first query to the
   // |protected_buffer_manager_| in order to resume decode tasks that were

ash/components/arc/video_accelerator/gpu_arc_video_decoder.cc

@@ -12,6 +12,8 @@
 #include "ash/components/arc/video_accelerator/protected_buffer_manager.h"
 #include "base/bind.h"
 #include "base/files/scoped_file.h"
+#include "base/memory/platform_shared_memory_region.h"
+#include "base/memory/unsafe_shared_memory_region.h"
 #include "base/metrics/histogram_functions.h"
 #include "base/posix/eintr_wrapper.h"
 #include "base/threading/sequenced_task_runner_handle.h"
@@ -395,7 +397,7 @@ scoped_refptr<media::DecoderBuffer> GpuArcVideoDecoder::CreateDecoderBuffer(
     uint32_t bytes_used) {
   // TODO(b/189278506) integrate additional memory buffer verification for
   // protected buffers (see crrev.com/3306795).
-  base::subtle::PlatformSharedMemoryRegion shm_region;
+  base::UnsafeSharedMemoryRegion shm_region;
   if (*secure_mode_) {
     // Use protected shared memory associated with the given file descriptor.
     shm_region = protected_buffer_manager_->GetProtectedSharedMemoryRegionFor(
@@ -410,9 +412,11 @@ scoped_refptr<media::DecoderBuffer> GpuArcVideoDecoder::CreateDecoderBuffer(
       VLOGF(1) << "Failed to get size for fd";
       return nullptr;
     }
-    shm_region = base::subtle::PlatformSharedMemoryRegion::Take(
-        std::move(fd), base::subtle::PlatformSharedMemoryRegion::Mode::kUnsafe,
-        size, base::UnguessableToken::Create());
+    shm_region = base::UnsafeSharedMemoryRegion::Deserialize(
+        base::subtle::PlatformSharedMemoryRegion::Take(
+            std::move(fd),
+            base::subtle::PlatformSharedMemoryRegion::Mode::kUnsafe, size,
+            base::UnguessableToken::Create()));
     if (!shm_region.IsValid()) {
       VLOGF(1) << "Cannot take file descriptor based shared memory";
       return nullptr;

ash/components/arc/video_accelerator/gpu_arc_video_encode_accelerator.cc

@@ -245,9 +245,11 @@ void GpuArcVideoEncodeAccelerator::UseBitstreamBuffer(
   // rather than pulling out the fd. https://crbug.com/713763.
   // TODO(rockot): Pass through a real size rather than |0|.
   base::UnguessableToken guid = base::UnguessableToken::Create();
-  auto shm_region = base::subtle::PlatformSharedMemoryRegion::Take(
-      std::move(fd), base::subtle::PlatformSharedMemoryRegion::Mode::kUnsafe,
-      shmem_size, guid);
+  auto shm_region = base::UnsafeSharedMemoryRegion::Deserialize(
+      base::subtle::PlatformSharedMemoryRegion::Take(
+          std::move(fd),
+          base::subtle::PlatformSharedMemoryRegion::Mode::kUnsafe, shmem_size,
+          guid));
   if (!shm_region.IsValid()) {
     client_->NotifyError(Error::kInvalidArgumentError);
     return;

ash/components/arc/video_accelerator/oop_arc_video_accelerator_factory.cc

@@ -8,12 +8,14 @@
 #include "ash/components/arc/video_accelerator/gpu_arc_video_decode_accelerator.h"
 #include "ash/components/arc/video_accelerator/gpu_arc_video_decoder.h"
 #include "ash/components/arc/video_accelerator/protected_buffer_manager.h"
+#include "base/memory/unsafe_shared_memory_region.h"
 #include "gpu/config/gpu_driver_bug_workarounds.h"
 #include "gpu/config/gpu_preferences.h"
 #include "media/base/bind_to_current_loop.h"
 #include "media/gpu/macros.h"
 #include "mojo/public/cpp/bindings/callback_helpers.h"
 #include "mojo/public/cpp/bindings/self_owned_receiver.h"
+#include "mojo/public/cpp/system/platform_handle.h"
 
 namespace arc {
 
@@ -67,13 +69,12 @@ class MojoProtectedBufferManager : public DecoderProtectedBufferManager {
       GetProtectedSharedMemoryRegionForResponseCB response_cb,
       mojo::ScopedSharedBufferHandle shared_memory_mojo_handle) {
     if (!shared_memory_mojo_handle.is_valid()) {
-      return std::move(response_cb)
-          .Run(base::subtle::PlatformSharedMemoryRegion());
+      return std::move(response_cb).Run(base::UnsafeSharedMemoryRegion());
     }
 
     // TODO(b/195769334): does anything need to be validated here?
     std::move(response_cb)
-        .Run(mojo::UnwrapPlatformSharedMemoryRegion(
+        .Run(mojo::UnwrapUnsafeSharedMemoryRegion(
             std::move(shared_memory_mojo_handle)));
   }
 

ash/components/arc/video_accelerator/protected_buffer_manager.cc

@@ -10,7 +10,6 @@
 #include "base/bind.h"
 #include "base/bits.h"
 #include "base/logging.h"
-#include "base/memory/platform_shared_memory_region.h"
 #include "base/memory/unsafe_shared_memory_region.h"
 #include "base/posix/eintr_wrapper.h"
 #include "base/system/sys_info.h"
@@ -47,8 +46,7 @@ class ProtectedBufferManager::ProtectedBuffer {
   // Downcasting methods to return duplicated handles to the underlying
   // protected buffers for each buffer type, or empty/null handles if not
   // applicable.
-  virtual base::subtle::PlatformSharedMemoryRegion
-  DuplicatePlatformSharedMemoryRegion() const {
+  virtual base::UnsafeSharedMemoryRegion DuplicateSharedMemoryRegion() const {
     return {};
   }
   virtual gfx::NativePixmapHandle DuplicateNativePixmapHandle() const {
@@ -79,15 +77,14 @@ class ProtectedBufferManager::ProtectedSharedMemory
       scoped_refptr<gfx::NativePixmap> dummy_handle,
       size_t size);
 
-  base::subtle::PlatformSharedMemoryRegion DuplicatePlatformSharedMemoryRegion()
-      const override {
+  base::UnsafeSharedMemoryRegion DuplicateSharedMemoryRegion() const override {
     return region_.Duplicate();
   }
 
  private:
   explicit ProtectedSharedMemory(scoped_refptr<gfx::NativePixmap> dummy_handle);
 
-  base::subtle::PlatformSharedMemoryRegion region_;
+  base::UnsafeSharedMemoryRegion region_;
 };
 
 ProtectedBufferManager::ProtectedSharedMemory::ProtectedSharedMemory(
@@ -113,9 +110,7 @@ ProtectedBufferManager::ProtectedSharedMemory::Create(
     return nullptr;
   }
 
-  protected_shmem->region_ =
-      base::UnsafeSharedMemoryRegion::TakeHandleForSerialization(
-          std::move(shmem_region));
+  protected_shmem->region_ = std::move(shmem_region);
   return protected_shmem;
 }
 
@@ -399,7 +394,7 @@ void ProtectedBufferManager::ReleaseAllProtectedBuffers(uint64_t allocator_id) {
   allocator_to_buffers_map_.erase(allocator_id);
 }
 
-base::subtle::PlatformSharedMemoryRegion
+base::UnsafeSharedMemoryRegion
 ProtectedBufferManager::GetProtectedSharedMemoryRegionFor(
     base::ScopedFD dummy_fd) {
   uint32_t id = 0;
@@ -412,7 +407,7 @@ ProtectedBufferManager::GetProtectedSharedMemoryRegionFor(
   if (iter == buffer_map_.end())
     return {};
 
-  return iter->second->DuplicatePlatformSharedMemoryRegion();
+  return iter->second->DuplicateSharedMemoryRegion();
 }
 
 gfx::NativePixmapHandle

ash/components/arc/video_accelerator/protected_buffer_manager.h

@@ -10,8 +10,8 @@
 #include <set>
 
 #include "base/files/scoped_file.h"
-#include "base/memory/platform_shared_memory_region.h"
 #include "base/memory/ref_counted.h"
+#include "base/memory/unsafe_shared_memory_region.h"
 #include "base/memory/weak_ptr.h"
 #include "base/synchronization/lock.h"
 #include "base/thread_annotations.h"
@@ -34,7 +34,7 @@ class DecoderProtectedBufferManager
   // after use. |response_cb| is called on the calling sequence and may be
   // called before this method returns.
   using GetProtectedSharedMemoryRegionForResponseCB =
-      base::OnceCallback<void(base::subtle::PlatformSharedMemoryRegion)>;
+      base::OnceCallback<void(base::UnsafeSharedMemoryRegion)>;
   virtual void GetProtectedSharedMemoryRegionFor(
       base::ScopedFD dummy_fd,
       GetProtectedSharedMemoryRegionForResponseCB response_cb) = 0;
@@ -70,10 +70,10 @@ class ProtectedBufferManager : public DecoderProtectedBufferManager {
   CreateProtectedBufferAllocator(
       scoped_refptr<ProtectedBufferManager> protected_buffer_manager);
 
-  // Return a duplicated PlatformSharedMemoryRegion associated with the
+  // Return a duplicated UnsafeSharedMemoryRegion associated with the
   // |dummy_fd|, if one exists, or an invalid handle otherwise. The client is
   // responsible for closing the handle after use.
-  base::subtle::PlatformSharedMemoryRegion GetProtectedSharedMemoryRegionFor(
+  base::UnsafeSharedMemoryRegion GetProtectedSharedMemoryRegionFor(
       base::ScopedFD dummy_fd);
 
   // Return a duplicated NativePixmapHandle associated with the |dummy_fd|,

ash/components/arc/video_accelerator/protected_buffer_manager_proxy.cc

@@ -6,6 +6,8 @@
 
 #include "ash/components/arc/video_accelerator/arc_video_accelerator_util.h"
 #include "ash/components/arc/video_accelerator/protected_buffer_manager.h"
+#include "base/memory/platform_shared_memory_region.h"
+#include "base/memory/unsafe_shared_memory_region.h"
 #include "media/gpu/macros.h"
 #include "mojo/public/cpp/system/platform_handle.h"
 
@@ -35,8 +37,14 @@ void GpuArcProtectedBufferManagerProxy::
     return;
   }
 
-  // This ScopedFDPair dance is chromeos-specific.
-  base::subtle::ScopedFDPair fd_pair = region.PassPlatformHandle();
+  // Due to POSIX limitations, the shmem platform handle consists of a pair of
+  // a writable FD and a read-only FD. Since GetProtectedSharedMemoryRegionFor()
+  // returns a base::UnsafeSharedMemoryRegion, only the writable FD should be
+  // present.
+  base::subtle::PlatformSharedMemoryRegion platform_region =
+      base::UnsafeSharedMemoryRegion::TakeHandleForSerialization(
+          std::move(region));
+  base::subtle::ScopedFDPair fd_pair = platform_region.PassPlatformHandle();
   std::move(callback).Run(mojo::WrapPlatformFile(std::move(fd_pair.fd)));
 }
 
@@ -45,16 +53,12 @@ void GpuArcProtectedBufferManagerProxy::GetProtectedSharedMemoryFromHandle(
     GetProtectedSharedMemoryFromHandleCallback callback) {
   base::ScopedFD unwrapped_fd = UnwrapFdFromMojoHandle(std::move(dummy_handle));
 
-  auto region_platform_handle =
+  base::UnsafeSharedMemoryRegion unsafe_shared_memory_region =
       protected_buffer_manager_->GetProtectedSharedMemoryRegionFor(
           std::move(unwrapped_fd));
-  if (!region_platform_handle.IsValid())
+  if (!unsafe_shared_memory_region.IsValid())
     return std::move(callback).Run(mojo::ScopedSharedBufferHandle());
 
-  base::UnsafeSharedMemoryRegion unsafe_shared_memory_region =
-      base::UnsafeSharedMemoryRegion::Deserialize(
-          std::move(region_platform_handle));
-  CHECK(unsafe_shared_memory_region.IsValid());
   std::move(callback).Run(mojo::WrapUnsafeSharedMemoryRegion(
       std::move(unsafe_shared_memory_region)));
 }

components/chromeos_camera/common/mjpeg_decode_accelerator_mojom_traits.cc

@@ -5,7 +5,10 @@
 #include "components/chromeos_camera/common/mjpeg_decode_accelerator_mojom_traits.h"
 
 #include "base/check.h"
+#include "base/memory/platform_shared_memory_region.h"
+#include "base/memory/unsafe_shared_memory_region.h"
 #include "base/notreached.h"
+#include "base/numerics/checked_math.h"
 #include "base/time/time.h"
 #include "media/base/ipc/media_param_traits_macros.h"
 #include "mojo/public/cpp/base/time_mojom_traits.h"
@@ -69,7 +72,9 @@ bool EnumTraits<chromeos_camera::mojom::DecodeError,
 mojo::ScopedSharedBufferHandle StructTraits<
     chromeos_camera::mojom::BitstreamBufferDataView,
     media::BitstreamBuffer>::memory_handle(media::BitstreamBuffer& input) {
-  base::subtle::PlatformSharedMemoryRegion input_region = input.TakeRegion();
+  base::subtle::PlatformSharedMemoryRegion input_region =
+      base::UnsafeSharedMemoryRegion::TakeHandleForSerialization(
+          input.TakeRegion());
   DCHECK(input_region.IsValid()) << "Bad BitstreamBuffer handle";
 
   // TODO(https://crbug.com/793446): Split BitstreamBuffers into ReadOnly and
@@ -105,14 +110,18 @@ bool StructTraits<chromeos_camera::mojom::BitstreamBufferDataView,
   if (!handle.is_valid())
     return false;
 
-  auto memory_region =
-      mojo::UnwrapPlatformSharedMemoryRegion(std::move(handle));
-  if (!memory_region.IsValid())
+  auto region = base::UnsafeSharedMemoryRegion::Deserialize(
+      mojo::UnwrapPlatformSharedMemoryRegion(std::move(handle)));
+  if (!region.IsValid())
     return false;
 
-  media::BitstreamBuffer bitstream_buffer(
-      input.id(), std::move(memory_region), input.size(),
-      base::checked_cast<off_t>(input.offset()), timestamp);
+  auto offset = base::MakeCheckedNum(input.offset()).Cast<uint64_t>();
+  if (!offset.IsValid())
+    return false;
+
+  media::BitstreamBuffer bitstream_buffer(input.id(), std::move(region),
+                                          input.size(), offset.ValueOrDie(),
+                                          timestamp);
   if (key_id.size()) {
     // Note that BitstreamBuffer currently ignores how each buffer is
     // encrypted and uses the settings from the Audio/VideoDecoderConfig.

components/chromeos_camera/fake_mjpeg_decode_accelerator.cc

@@ -8,10 +8,11 @@
 
 #include "base/bind.h"
 #include "base/logging.h"
+#include "base/memory/shared_memory_mapping.h"
+#include "base/memory/unsafe_shared_memory_region.h"
 #include "base/task/single_thread_task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "media/base/bind_to_current_loop.h"
-#include "media/base/unaligned_shared_memory.h"
 #include "media/base/video_frame.h"
 #include "media/base/video_types.h"
 
@@ -60,10 +61,10 @@ void FakeMjpegDecodeAccelerator::Decode(
     scoped_refptr<media::VideoFrame> video_frame) {
   DCHECK(io_task_runner_->BelongsToCurrentThread());
 
-  auto src_shm = std::make_unique<media::UnalignedSharedMemory>(
-      bitstream_buffer.TakeRegion(), bitstream_buffer.size(),
-      false /* read_only */);
-  if (!src_shm->MapAt(bitstream_buffer.offset(), bitstream_buffer.size())) {
+  base::UnsafeSharedMemoryRegion src_shm_region = bitstream_buffer.TakeRegion();
+  base::WritableSharedMemoryMapping src_shm_mapping =
+      src_shm_region.MapAt(bitstream_buffer.offset(), bitstream_buffer.size());
+  if (!src_shm_mapping.IsValid()) {
     DLOG(ERROR) << "Unable to map shared memory in FakeMjpegDecodeAccelerator";
     NotifyError(bitstream_buffer.id(),
                 MjpegDecodeAccelerator::UNREADABLE_INPUT);
@@ -75,7 +76,7 @@ void FakeMjpegDecodeAccelerator::Decode(
       FROM_HERE,
       base::BindOnce(&FakeMjpegDecodeAccelerator::DecodeOnDecoderThread,
                      base::Unretained(this), bitstream_buffer.id(),
-                     std::move(video_frame), std::move(src_shm)));
+                     std::move(video_frame), std::move(src_shm_mapping)));
 }
 
 void FakeMjpegDecodeAccelerator::Decode(
@@ -90,7 +91,7 @@ void FakeMjpegDecodeAccelerator::Decode(
 void FakeMjpegDecodeAccelerator::DecodeOnDecoderThread(
     int32_t task_id,
     scoped_refptr<media::VideoFrame> video_frame,
-    std::unique_ptr<media::UnalignedSharedMemory> src_shm) {
+    base::WritableSharedMemoryMapping src_shm_mapping) {
   DCHECK(decoder_task_runner_->BelongsToCurrentThread());
 
   // Do not actually decode the Jpeg data.

components/chromeos_camera/fake_mjpeg_decode_accelerator.h

@@ -7,8 +7,7 @@
 
 #include <stdint.h>
 
-#include <memory>
-
+#include "base/memory/shared_memory_mapping.h"
 #include "base/memory/weak_ptr.h"
 #include "base/threading/thread.h"
 #include "components/chromeos_camera/mjpeg_decode_accelerator.h"
@@ -47,10 +46,9 @@ class FakeMjpegDecodeAccelerator : public MjpegDecodeAccelerator {
   bool IsSupported() override;
 
  private:
-  void DecodeOnDecoderThread(
-      int32_t task_id,
-      scoped_refptr<media::VideoFrame> video_frame,
-      std::unique_ptr<media::UnalignedSharedMemory> src_shm);
+  void DecodeOnDecoderThread(int32_t task_id,
+                             scoped_refptr<media::VideoFrame> video_frame,
+                             base::WritableSharedMemoryMapping src_shm_mapping);
   void NotifyError(int32_t task_id, Error error);
   void NotifyErrorOnClientThread(int32_t task_id, Error error);
   void OnDecodeDoneOnClientThread(int32_t task_id);