Use new shmem API in PPB_ImageData_Impl

The goal is to eliminate usage of deprecated base::SharedMemory.

The shared memory region backing this 'canvas' is writable by
the plugin process (PPB_ImageData_Proxy) and its hosting renderer
(PPB_ImageData_Impl). Both processes must keep writable shared
memory handles because PPB_ImageData_API interface exposes
GetSharedMemory() method.

Hence move the usage of base::SharedMemory into
base::UnsafeSharedMemoryRegion.

Bug: 795291
Change-Id: I4766061db7f0a1e2bedfcb9459490acee6c8b2ef
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1586039
Commit-Queue: Alex Ilin <alexilin@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Bill Budge <bbudge@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Auto-Submit: Alex Ilin <alexilin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#659109}

base/memory/platform_shared_memory_region.h

@@ -36,7 +36,11 @@ namespace subtle {
 // Helper structs to keep two descriptors on POSIX. It's needed to support
 // ConvertToReadOnly().
 struct BASE_EXPORT FDPair {
+  // The main shared memory descriptor that is used for mapping. May be either
+  // writable or read-only, depending on region's mode.
   int fd;
+  // The read-only descriptor, valid only in kWritable mode. Replaces |fd| when
+  // a region is converted to read-only.
   int readonly_fd;
 };
 

base/memory/read_only_shared_memory_region.h

@@ -102,15 +102,6 @@ class BASE_EXPORT ReadOnlySharedMemoryRegion {
     return handle_.GetGUID();
   }
 
- private:
-  FRIEND_TEST_ALL_PREFIXES(FieldTrialListTest,
-                           SerializeSharedMemoryRegionMetadata);
-  friend class FieldTrialList;
-  friend class SharedMemoryHooks;
-
-  explicit ReadOnlySharedMemoryRegion(
-      subtle::PlatformSharedMemoryRegion handle);
-
   // Returns a platform shared memory handle. |this| remains the owner of the
   // handle.
   subtle::PlatformSharedMemoryRegion::PlatformHandle GetPlatformHandle() const {
@@ -118,6 +109,12 @@ class BASE_EXPORT ReadOnlySharedMemoryRegion {
     return handle_.GetPlatformHandle();
   }
 
+ private:
+  friend class SharedMemoryHooks;
+
+  explicit ReadOnlySharedMemoryRegion(
+      subtle::PlatformSharedMemoryRegion handle);
+
   static void set_create_hook(CreateFunction* hook) { create_hook_ = hook; }
 
   static CreateFunction* create_hook_;

base/memory/unsafe_shared_memory_region.h

@@ -109,14 +109,6 @@ class BASE_EXPORT UnsafeSharedMemoryRegion {
     return handle_.GetGUID();
   }
 
- private:
-  FRIEND_TEST_ALL_PREFIXES(DiscardableSharedMemoryTest,
-                           LockShouldFailIfPlatformLockPagesFails);
-  friend class DiscardableSharedMemory;
-  friend class SharedMemoryHooks;
-
-  explicit UnsafeSharedMemoryRegion(subtle::PlatformSharedMemoryRegion handle);
-
   // Returns a platform shared memory handle. |this| remains the owner of the
   // handle.
   subtle::PlatformSharedMemoryRegion::PlatformHandle GetPlatformHandle() const {
@@ -124,6 +116,11 @@ class BASE_EXPORT UnsafeSharedMemoryRegion {
     return handle_.GetPlatformHandle();
   }
 
+ private:
+  friend class SharedMemoryHooks;
+
+  explicit UnsafeSharedMemoryRegion(subtle::PlatformSharedMemoryRegion handle);
+
   static void set_create_hook(CreateFunction* hook) { create_hook_ = hook; }
 
   static CreateFunction* create_hook_;

base/memory/writable_shared_memory_region.h

@@ -22,6 +22,10 @@ namespace base {
 // ReadOnlySharedMemoryRegion. However, unlike ReadOnlySharedMemoryRegion and
 // UnsafeSharedMemoryRegion, ownership of this region (while writable) is unique
 // and may only be transferred, not duplicated.
+//
+// Unlike ReadOnlySharedMemoryRegion and UnsafeSharedMemoryRegion,
+// WritableSharedMemoryRegion doesn't provide GetPlatformHandle() method to
+// ensure that the region is never duplicated while writable.
 class BASE_EXPORT WritableSharedMemoryRegion {
  public:
   using MappingType = WritableSharedMemoryMapping;

content/browser/native_file_system/file_system_chooser_browsertest.cc

@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/scoped_feature_list.h"

content/browser/network_service_browsertest.cc

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "base/bind.h"
+#include "base/files/file_util.h"
 #include "base/memory/ref_counted_memory.h"
 #include "base/test/bind_test_util.h"
 #include "base/test/scoped_feature_list.h"

content/renderer/pepper/ppb_image_data_impl.cc

@@ -12,6 +12,7 @@
 #include "content/common/pepper_file_util.h"
 #include "content/common/view_messages.h"
 #include "content/renderer/render_thread_impl.h"
+#include "mojo/public/cpp/base/shared_memory_utils.h"
 #include "ppapi/c/pp_errors.h"
 #include "ppapi/c/pp_instance.h"
 #include "ppapi/c/pp_resource.h"
@@ -107,9 +108,9 @@ void* PPB_ImageData_Impl::Map() { return backend_->Map(); }
 
 void PPB_ImageData_Impl::Unmap() { backend_->Unmap(); }
 
-int32_t PPB_ImageData_Impl::GetSharedMemory(base::SharedMemory** shm,
-                                            uint32_t* byte_count) {
-  return backend_->GetSharedMemory(shm, byte_count);
+int32_t PPB_ImageData_Impl::GetSharedMemoryRegion(
+    base::UnsafeSharedMemoryRegion** region) {
+  return backend_->GetSharedMemoryRegion(region);
 }
 
 SkCanvas* PPB_ImageData_Impl::GetCanvas() { return backend_->GetCanvas(); }
@@ -135,25 +136,16 @@ bool ImageDataPlatformBackend::Init(PPB_ImageData_Impl* impl,
                                     int width,
                                     int height,
                                     bool init_to_zero) {
-  // TODO(brettw) use init_to_zero when we implement caching.
+  // TODO(brettw): use init_to_zero when we implement caching.
   width_ = width;
   height_ = height;
   uint32_t buffer_size = width_ * height_ * 4;
-  std::unique_ptr<base::SharedMemory> shared_memory =
-      RenderThread::Get()->HostAllocateSharedMemoryBuffer(buffer_size);
-  if (!shared_memory)
+  base::UnsafeSharedMemoryRegion region =
+      mojo::CreateUnsafeSharedMemoryRegion(buffer_size);
+  if (!region.IsValid())
     return false;
 
-  // The TransportDIB is always backed by shared memory, so give the shared
-  // memory handle to it.
-  base::SharedMemoryHandle handle = shared_memory->handle().Duplicate();
-  shared_memory->Unmap();
-  shared_memory->Close();
-  if (!handle.IsValid())
-    return false;
-
-  dib_.reset(TransportDIB::CreateWithHandle(handle));
-
+  dib_ = TransportDIB::CreateWithHandle(std::move(region));
   return !!dib_;
 }
 
@@ -188,10 +180,9 @@ void ImageDataPlatformBackend::Unmap() {
   // in the future to save some memory.
 }
 
-int32_t ImageDataPlatformBackend::GetSharedMemory(base::SharedMemory** shm,
-                                                  uint32_t* byte_count) {
-  *byte_count = dib_->size();
-  *shm = dib_->shared_memory();
+int32_t ImageDataPlatformBackend::GetSharedMemoryRegion(
+    base::UnsafeSharedMemoryRegion** region) {
+  *region = dib_->shared_memory_region();
   return PP_OK;
 }
 
@@ -224,41 +215,42 @@ bool ImageDataSimpleBackend::Init(PPB_ImageData_Impl* impl,
                                   bool init_to_zero) {
   skia_bitmap_.setInfo(
       SkImageInfo::MakeN32Premul(impl->width(), impl->height()));
-  shared_memory_.reset(
-      RenderThread::Get()
-          ->HostAllocateSharedMemoryBuffer(skia_bitmap_.computeByteSize())
-          .release());
-  return !!shared_memory_.get();
+  shm_region_ =
+      mojo::CreateUnsafeSharedMemoryRegion(skia_bitmap_.computeByteSize());
+  return shm_region_.IsValid();
 }
 
-bool ImageDataSimpleBackend::IsMapped() const { return map_count_ > 0; }
+bool ImageDataSimpleBackend::IsMapped() const {
+  return shm_mapping_.IsValid();
+}
 
 TransportDIB* ImageDataSimpleBackend::GetTransportDIB() const {
   return nullptr;
 }
 
 void* ImageDataSimpleBackend::Map() {
-  DCHECK(shared_memory_.get());
+  DCHECK(shm_region_.IsValid());
   if (map_count_++ == 0) {
-    shared_memory_->Map(skia_bitmap_.computeByteSize());
-    skia_bitmap_.setPixels(shared_memory_->memory());
+    shm_mapping_ = shm_region_.Map();
+    if (!shm_mapping_.IsValid())
+      return nullptr;
+
+    skia_bitmap_.setPixels(shm_mapping_.memory());
     // Our platform bitmaps are set to opaque by default, which we don't want.
     skia_bitmap_.setAlphaType(kPremul_SkAlphaType);
     skia_canvas_ = std::make_unique<SkCanvas>(skia_bitmap_);
-    return skia_bitmap_.getAddr32(0, 0);
   }
-  return shared_memory_->memory();
+  return skia_bitmap_.isNull() ? nullptr : skia_bitmap_.getAddr32(0, 0);
 }
 
 void ImageDataSimpleBackend::Unmap() {
   if (--map_count_ == 0)
-    shared_memory_->Unmap();
+    shm_mapping_ = base::WritableSharedMemoryMapping();
 }
 
-int32_t ImageDataSimpleBackend::GetSharedMemory(base::SharedMemory** shm,
-                                                uint32_t* byte_count) {
-  *byte_count = skia_bitmap_.computeByteSize();
-  *shm = shared_memory_.get();
+int32_t ImageDataSimpleBackend::GetSharedMemoryRegion(
+    base::UnsafeSharedMemoryRegion** region) {
+  *region = &shm_region_;
   return PP_OK;
 }
 

content/renderer/pepper/ppb_image_data_impl.h

@@ -10,7 +10,8 @@
 #include <memory>
 
 #include "base/macros.h"
-#include "base/memory/shared_memory.h"
+#include "base/memory/shared_memory_mapping.h"
+#include "base/memory/unsafe_shared_memory_region.h"
 #include "content/common/content_export.h"
 #include "ppapi/c/ppb_image_data.h"
 #include "ppapi/shared_impl/ppb_image_data_shared.h"
@@ -21,10 +22,6 @@
 class SkCanvas;
 class TransportDIB;
 
-namespace base {
-class SharedMemory;
-}
-
 namespace content {
 
 class CONTENT_EXPORT PPB_ImageData_Impl
@@ -48,8 +45,8 @@ class CONTENT_EXPORT PPB_ImageData_Impl
     virtual TransportDIB* GetTransportDIB() const = 0;
     virtual void* Map() = 0;
     virtual void Unmap() = 0;
-    virtual int32_t GetSharedMemory(base::SharedMemory** shm,
-                                    uint32_t* byte_count) = 0;
+    virtual int32_t GetSharedMemoryRegion(
+        base::UnsafeSharedMemoryRegion** region) = 0;
     virtual SkCanvas* GetCanvas() = 0;
     virtual SkBitmap GetMappedBitmap() const = 0;
   };
@@ -93,8 +90,8 @@ class CONTENT_EXPORT PPB_ImageData_Impl
   PP_Bool Describe(PP_ImageDataDesc* desc) override;
   void* Map() override;
   void Unmap() override;
-  int32_t GetSharedMemory(base::SharedMemory** shm,
-                          uint32_t* byte_count) override;
+  int32_t GetSharedMemoryRegion(
+      base::UnsafeSharedMemoryRegion** region) override;
   SkCanvas* GetCanvas() override;
   void SetIsCandidateForReuse() override;
 
@@ -132,8 +129,8 @@ class ImageDataPlatformBackend : public PPB_ImageData_Impl::Backend {
   TransportDIB* GetTransportDIB() const override;
   void* Map() override;
   void Unmap() override;
-  int32_t GetSharedMemory(base::SharedMemory** shm,
-                          uint32_t* byte_count) override;
+  int32_t GetSharedMemoryRegion(
+      base::UnsafeSharedMemoryRegion** region) override;
   SkCanvas* GetCanvas() override;
   SkBitmap GetMappedBitmap() const override;
 
@@ -165,13 +162,14 @@ class ImageDataSimpleBackend : public PPB_ImageData_Impl::Backend {
   TransportDIB* GetTransportDIB() const override;
   void* Map() override;
   void Unmap() override;
-  int32_t GetSharedMemory(base::SharedMemory** shm,
-                          uint32_t* byte_count) override;
+  int32_t GetSharedMemoryRegion(
+      base::UnsafeSharedMemoryRegion** region) override;
   SkCanvas* GetCanvas() override;
   SkBitmap GetMappedBitmap() const override;
 
  private:
-  std::unique_ptr<base::SharedMemory> shared_memory_;
+  base::UnsafeSharedMemoryRegion shm_region_;
+  base::WritableSharedMemoryMapping shm_mapping_;
   // skia_bitmap_ is backed by shared_memory_.
   SkBitmap skia_bitmap_;
   std::unique_ptr<SkCanvas> skia_canvas_;

ppapi/proxy/ppapi_messages.h

@@ -1057,7 +1057,7 @@ IPC_SYNC_MESSAGE_ROUTED4_3(PpapiHostMsg_PPBImageData_CreatePlatform,
                            PP_Bool /* init_to_zero */,
                            ppapi::HostResource /* result_resource */,
                            PP_ImageDataDesc /* image_data_desc */,
-                           ppapi::proxy::ImageHandle /* result */)
+                           ppapi::proxy::SerializedHandle /* result */)
 IPC_SYNC_MESSAGE_ROUTED4_3(PpapiHostMsg_PPBImageData_CreateSimple,
                            PP_Instance /* instance */,
                            int32_t /* format */,

ppapi/proxy/ppb_image_data_proxy.cc

@@ -12,6 +12,7 @@
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/macros.h"
+#include "base/memory/shared_memory_mapping.h"
 #include "base/memory/singleton.h"
 #include "base/memory/weak_ptr.h"
 #include "build/build_config.h"
@@ -105,7 +106,7 @@ struct ImageDataCacheEntry {
 
   base::TimeTicks added_time;
 
-  // Set to true when the renderer tells us that it's OK to re-use this iamge.
+  // Set to true when the renderer tells us that it's OK to re-use this image.
   bool usable;
 
   scoped_refptr<ImageData> image;
@@ -346,8 +347,8 @@ PP_Bool ImageData::Describe(PP_ImageDataDesc* desc) {
   return PP_TRUE;
 }
 
-int32_t ImageData::GetSharedMemory(base::SharedMemory** /* shm */,
-                                   uint32_t* /* byte_count */) {
+int32_t ImageData::GetSharedMemoryRegion(
+    base::UnsafeSharedMemoryRegion** /* region */) {
   // Not supported in the proxy (this method is for actually implementing the
   // proxy in the host).
   return PP_ERROR_NOACCESS;
@@ -369,30 +370,30 @@ void ImageData::RecycleToPlugin(bool zero_contents) {
 // PlatformImageData -----------------------------------------------------------
 
 #if !defined(OS_NACL)
-PlatformImageData::PlatformImageData(const HostResource& resource,
-                                     const PP_ImageDataDesc& desc,
-                                     ImageHandle handle)
+PlatformImageData::PlatformImageData(
+    const HostResource& resource,
+    const PP_ImageDataDesc& desc,
+    base::UnsafeSharedMemoryRegion image_region)
     : ImageData(resource, PPB_ImageData_Shared::PLATFORM, desc) {
 #if defined(OS_WIN)
-  transport_dib_.reset(TransportDIB::CreateWithHandle(handle));
+  transport_dib_ = TransportDIB::CreateWithHandle(std::move(image_region));
 #else
-  transport_dib_.reset(TransportDIB::Map(handle));
+  transport_dib_ = TransportDIB::Map(std::move(image_region));
 #endif  // defined(OS_WIN)
 }
 
-PlatformImageData::~PlatformImageData() {
-}
+PlatformImageData::~PlatformImageData() = default;
 
 void* PlatformImageData::Map() {
   if (!mapped_canvas_.get()) {
     if (!transport_dib_.get())
-      return NULL;
+      return nullptr;
 
     const bool is_opaque = false;
     mapped_canvas_ = transport_dib_->GetPlatformCanvas(
         desc_.size.width, desc_.size.height, is_opaque);
     if (!mapped_canvas_.get())
-      return NULL;
+      return nullptr;
   }
   SkPixmap pixmap;
   skia::GetWritablePixels(mapped_canvas_.get(), &pixmap);
@@ -408,40 +409,33 @@ void PlatformImageData::Unmap() {
 SkCanvas* PlatformImageData::GetCanvas() {
   return mapped_canvas_.get();
 }
-
-// static
-ImageHandle PlatformImageData::NullHandle() {
-  return ImageHandle();
-}
 #endif  // !defined(OS_NACL)
 
 // SimpleImageData -------------------------------------------------------------
 
 SimpleImageData::SimpleImageData(const HostResource& resource,
                                  const PP_ImageDataDesc& desc,
-                                 const base::SharedMemoryHandle& handle)
+                                 base::UnsafeSharedMemoryRegion region)
     : ImageData(resource, PPB_ImageData_Shared::SIMPLE, desc),
-      shm_(handle, false /* read_only */),
+      shm_region_(std::move(region)),
       size_(desc.size.width * desc.size.height * 4),
-      map_count_(0) {
-}
+      map_count_(0) {}
 
-SimpleImageData::~SimpleImageData() {
-}
+SimpleImageData::~SimpleImageData() = default;
 
 void* SimpleImageData::Map() {
   if (map_count_++ == 0)
-    shm_.Map(size_);
-  return shm_.memory();
+    shm_mapping_ = shm_region_.MapAt(0, size_);
+  return shm_mapping_.IsValid() ? shm_mapping_.memory() : nullptr;
 }
 
 void SimpleImageData::Unmap() {
   if (--map_count_ == 0)
-    shm_.Unmap();
+    shm_mapping_ = base::WritableSharedMemoryMapping();
 }
 
 SkCanvas* SimpleImageData::GetCanvas() {
-  return NULL;  // No canvas available.
+  return nullptr;  // No canvas available.
 }
 
 // PPB_ImageData_Proxy ---------------------------------------------------------
@@ -478,28 +472,35 @@ PP_Resource PPB_ImageData_Proxy::CreateProxyResource(
   PP_ImageDataDesc desc;
   switch (type) {
     case PPB_ImageData_Shared::SIMPLE: {
-      ppapi::proxy::SerializedHandle image_handle_wrapper;
+      ppapi::proxy::SerializedHandle image_handle;
       dispatcher->Send(new PpapiHostMsg_PPBImageData_CreateSimple(
-          kApiID, instance, format, size, init_to_zero,
-          &result, &desc, &image_handle_wrapper));
-      if (image_handle_wrapper.is_shmem()) {
-        base::SharedMemoryHandle image_handle = image_handle_wrapper.shmem();
+          kApiID, instance, format, size, init_to_zero, &result, &desc,
+          &image_handle));
+      if (image_handle.is_shmem_region()) {
+        base::UnsafeSharedMemoryRegion image_region =
+            base::UnsafeSharedMemoryRegion::Deserialize(
+                image_handle.TakeSharedMemoryRegion());
         if (!result.is_null()) {
-          return
-              (new SimpleImageData(result, desc, image_handle))->GetReference();
+          return (new SimpleImageData(result, desc, std::move(image_region)))
+              ->GetReference();
         }
       }
       break;
     }
     case PPB_ImageData_Shared::PLATFORM: {
 #if !defined(OS_NACL)
-      ImageHandle image_handle = PlatformImageData::NullHandle();
+      ppapi::proxy::SerializedHandle image_handle;
       dispatcher->Send(new PpapiHostMsg_PPBImageData_CreatePlatform(
-          kApiID, instance, format, size, init_to_zero,
-          &result, &desc, &image_handle));
-      if (!result.is_null()) {
-        return
-            (new PlatformImageData(result, desc, image_handle))->GetReference();
+          kApiID, instance, format, size, init_to_zero, &result, &desc,
+          &image_handle));
+      if (image_handle.is_shmem_region()) {
+        base::UnsafeSharedMemoryRegion image_region =
+            base::UnsafeSharedMemoryRegion::Deserialize(
+                image_handle.TakeSharedMemoryRegion());
+        if (!result.is_null()) {
+          return (new PlatformImageData(result, desc, std::move(image_region)))
+              ->GetReference();
+        }
       }
 #else
       // PlatformImageData shouldn't be created in untrusted code.
@@ -538,8 +539,7 @@ PP_Resource PPB_ImageData_Proxy::CreateImageData(
     const PP_Size& size,
     bool init_to_zero,
     PP_ImageDataDesc* desc,
-    base::SharedMemoryHandle* image_handle,
-    uint32_t* byte_count) {
+    base::UnsafeSharedMemoryRegion* image_region) {
   HostDispatcher* dispatcher = HostDispatcher::GetForInstance(instance);
   if (!dispatcher)
     return 0;
@@ -576,15 +576,14 @@ PP_Resource PPB_ImageData_Proxy::CreateImageData(
     return 0;
   }
 
-  base::SharedMemory* local_shm;
-  if (enter_resource.object()->GetSharedMemory(&local_shm, byte_count) !=
-      PP_OK) {
+  base::UnsafeSharedMemoryRegion* local_shm;
+  if (enter_resource.object()->GetSharedMemoryRegion(&local_shm) != PP_OK) {
     DVLOG(1) << "CreateImageData failed: could not GetSharedMemory";
     return 0;
   }
 
-  *image_handle =
-      dispatcher->ShareSharedMemoryHandleWithRemote(local_shm->handle());
+  *image_region =
+      dispatcher->ShareUnsafeSharedMemoryRegionWithRemote(*local_shm);
   return resource.Release();
 }
 
@@ -595,22 +594,23 @@ void PPB_ImageData_Proxy::OnHostMsgCreatePlatform(
     PP_Bool init_to_zero,
     HostResource* result,
     PP_ImageDataDesc* desc,
-    ImageHandle* result_image_handle) {
+    ppapi::proxy::SerializedHandle* result_image_handle) {
   // Clear |desc| so we don't send uninitialized memory to the plugin.
   // https://crbug.com/391023.
   *desc = PP_ImageDataDesc();
-  base::SharedMemoryHandle image_handle;
-  uint32_t byte_count;
+  base::UnsafeSharedMemoryRegion image_region;
   PP_Resource resource =
-      CreateImageData(instance,
-                      PPB_ImageData_Shared::PLATFORM,
-                      static_cast<PP_ImageDataFormat>(format),
-                      size,
-                      true /* init_to_zero */,
-                      desc, &image_handle, &byte_count);
+      CreateImageData(instance, PPB_ImageData_Shared::PLATFORM,
+                      static_cast<PP_ImageDataFormat>(format), size,
+                      true /* init_to_zero */, desc, &image_region);
   result->SetHostResource(instance, resource);
-  *result_image_handle =
-      resource ? image_handle : PlatformImageData::NullHandle();
+  if (resource) {
+    result_image_handle->set_shmem_region(
+        base::UnsafeSharedMemoryRegion::TakeHandleForSerialization(
+            std::move(image_region)));
+  } else {
+    result_image_handle->set_null_shmem_region();
+  }
 }
 
 void PPB_ImageData_Proxy::OnHostMsgCreateSimple(
@@ -624,21 +624,18 @@ void PPB_ImageData_Proxy::OnHostMsgCreateSimple(
   // Clear |desc| so we don't send uninitialized memory to the plugin.
   // https://crbug.com/391023.
   *desc = PP_ImageDataDesc();
-  base::SharedMemoryHandle image_handle;
-  uint32_t byte_count;
+  base::UnsafeSharedMemoryRegion image_region;
   PP_Resource resource =
-      CreateImageData(instance,
-                      PPB_ImageData_Shared::SIMPLE,
-                      static_cast<PP_ImageDataFormat>(format),
-                      size,
-                      true /* init_to_zero */,
-                      desc, &image_handle, &byte_count);
-
+      CreateImageData(instance, PPB_ImageData_Shared::SIMPLE,
+                      static_cast<PP_ImageDataFormat>(format), size,
+                      true /* init_to_zero */, desc, &image_region);
   result->SetHostResource(instance, resource);
   if (resource) {
-    result_image_handle->set_shmem(image_handle, byte_count);
+    result_image_handle->set_shmem_region(
+        base::UnsafeSharedMemoryRegion::TakeHandleForSerialization(
+            std::move(image_region)));
   } else {
-    result_image_handle->set_null_shmem();
+    result_image_handle->set_null_shmem_region();
   }
 }
 #endif  // !defined(OS_NACL)

ppapi/proxy/ppb_image_data_proxy.h

@@ -10,7 +10,7 @@
 #include <memory>
 
 #include "base/macros.h"
-#include "base/memory/shared_memory.h"
+#include "base/memory/unsafe_shared_memory_region.h"
 #include "build/build_config.h"
 #include "ipc/ipc_platform_file.h"
 #include "ppapi/c/pp_bool.h"
@@ -55,8 +55,8 @@ class PPAPI_PROXY_EXPORT ImageData : public ppapi::Resource,
 
   // PPB_ImageData API.
   PP_Bool Describe(PP_ImageDataDesc* desc) override;
-  int32_t GetSharedMemory(base::SharedMemory** shm,
-                          uint32_t* byte_count) override;
+  int32_t GetSharedMemoryRegion(
+      base::UnsafeSharedMemoryRegion** region) override;
   void SetIsCandidateForReuse() override;
 
   PPB_ImageData_Shared::ImageDataType type() const { return type_; }
@@ -81,14 +81,14 @@ class PPAPI_PROXY_EXPORT ImageData : public ppapi::Resource,
 };
 
 // PlatformImageData is a full featured image data resource which can access
-// the underlying platform-specific canvas and ImageHandle. This can't be used
-// by NaCl apps.
+// the underlying platform-specific canvas and |image_region|. This can't be
+// used by NaCl apps.
 #if !defined(OS_NACL)
 class PPAPI_PROXY_EXPORT PlatformImageData : public ImageData {
  public:
   PlatformImageData(const ppapi::HostResource& resource,
                     const PP_ImageDataDesc& desc,
-                    ImageHandle handle);
+                    base::UnsafeSharedMemoryRegion image_region);
   ~PlatformImageData() override;
 
   // PPB_ImageData API.
@@ -96,8 +96,6 @@ class PPAPI_PROXY_EXPORT PlatformImageData : public ImageData {
   void Unmap() override;
   SkCanvas* GetCanvas() override;
 
-  static ImageHandle NullHandle();
-
  private:
   std::unique_ptr<TransportDIB> transport_dib_;
 
@@ -115,7 +113,7 @@ class PPAPI_PROXY_EXPORT SimpleImageData : public ImageData {
  public:
   SimpleImageData(const ppapi::HostResource& resource,
                   const PP_ImageDataDesc& desc,
-                  const base::SharedMemoryHandle& handle);
+                  base::UnsafeSharedMemoryRegion region);
   ~SimpleImageData() override;
 
   // PPB_ImageData API.
@@ -124,7 +122,8 @@ class PPAPI_PROXY_EXPORT SimpleImageData : public ImageData {
   SkCanvas* GetCanvas() override;
 
  private:
-  base::SharedMemory shm_;
+  base::UnsafeSharedMemoryRegion shm_region_;
+  base::WritableSharedMemoryMapping shm_mapping_;
   uint32_t size_;
   int map_count_;
 
@@ -151,7 +150,8 @@ class PPB_ImageData_Proxy : public InterfaceProxy {
   // On failure, will return invalid resource (0). On success it will return a
   // valid resource and the out params will be written.
   // |desc| contains the result of Describe.
-  // |image_handle| and |byte_count| contain the result of GetSharedMemory.
+  // |image_region| and |byte_count| contain the result of
+  // GetSharedMemoryRegion.
   // NOTE: if |init_to_zero| is false, you should write over the entire image
   // to avoid leaking sensitive data to a less privileged process.
   PPAPI_PROXY_EXPORT static PP_Resource CreateImageData(
@@ -161,8 +161,7 @@ class PPB_ImageData_Proxy : public InterfaceProxy {
       const PP_Size& size,
       bool init_to_zero,
       PP_ImageDataDesc* desc,
-      base::SharedMemoryHandle* image_handle,
-      uint32_t* byte_count);
+      base::UnsafeSharedMemoryRegion* image_region);
 
   static const ApiID kApiID = API_ID_PPB_IMAGE_DATA;
 
@@ -175,7 +174,7 @@ class PPB_ImageData_Proxy : public InterfaceProxy {
       PP_Bool init_to_zero,
       HostResource* result,
       PP_ImageDataDesc* desc,
-      ImageHandle* result_image_handle);
+      ppapi::proxy::SerializedHandle* result_image_handle);
   void OnHostMsgCreateSimple(
       PP_Instance instance,
       int32_t format,

ppapi/proxy/serialized_structs.h

@@ -133,9 +133,6 @@ struct PPB_AudioEncodeParameters {
   PP_HardwareAcceleration acceleration;
 };
 
-// TODO(raymes): Make ImageHandle compatible with SerializedHandle.
-typedef base::SharedMemoryHandle ImageHandle;
-
 }  // namespace proxy
 }  // namespace ppapi
 

ppapi/thunk/ppb_image_data_api.h

@@ -13,7 +13,7 @@
 class SkCanvas;
 
 namespace base {
-class SharedMemory;
+class UnsafeSharedMemoryRegion;
 }  // namespace base
 
 namespace ppapi {
@@ -28,8 +28,8 @@ class PPB_ImageData_API {
   virtual void Unmap() = 0;
 
   // Trusted inteface.
-  virtual int32_t GetSharedMemory(base::SharedMemory** shm,
-                                  uint32_t* byte_count) = 0;
+  virtual int32_t GetSharedMemoryRegion(
+      base::UnsafeSharedMemoryRegion** region) = 0;
 
   // Get the canvas that backs this ImageData, if there is one.
   // The canvas will be NULL:

ui/surface/BUILD.gn

@@ -10,7 +10,6 @@ jumbo_component("surface") {
     "surface_export.h",
     "transport_dib.cc",
     "transport_dib.h",
-    "transport_dib_win.cc",
   ]
 
   configs += [ "//third_party/khronos:khronos_headers" ]
@@ -25,8 +24,4 @@ jumbo_component("surface") {
     "//ui/gfx/geometry",
     "//ui/gl",
   ]
-
-  if (is_posix) {
-    sources += [ "transport_dib_posix.cc" ]
-  }
 }

ui/surface/transport_dib.cc

@@ -4,23 +4,105 @@
 
 #include "ui/surface/transport_dib.h"
 
-#include <limits.h>
+#include <stddef.h>
 
+#include <memory>
+
+#include "base/logging.h"
+#include "base/memory/ptr_util.h"
+#include "base/memory/shared_memory_mapping.h"
+#include "base/memory/unsafe_shared_memory_region.h"
+#include "base/numerics/checked_math.h"
+#include "build/build_config.h"
 #include "skia/ext/platform_canvas.h"
 
+TransportDIB::TransportDIB(base::UnsafeSharedMemoryRegion region)
+    : shm_region_(std::move(region)) {}
+
+TransportDIB::~TransportDIB() = default;
+
 // static
-bool TransportDIB::VerifyCanvasSize(int w, int h) {
-  static const size_t kMaxSize = static_cast<size_t>(INT_MAX);
-  const size_t one_stride = skia::PlatformCanvasStrideForWidth(1);
-  const size_t stride = skia::PlatformCanvasStrideForWidth(w);
-  if (w <= 0 || h <= 0 || static_cast<size_t>(w) > (kMaxSize / one_stride) ||
-      static_cast<size_t>(h) > (kMaxSize / stride)) {
+std::unique_ptr<TransportDIB> TransportDIB::Map(
+    base::UnsafeSharedMemoryRegion region) {
+  std::unique_ptr<TransportDIB> dib = CreateWithHandle(std::move(region));
+  if (!dib->Map())
+    return nullptr;
+  return dib;
+}
+
+// static
+std::unique_ptr<TransportDIB> TransportDIB::CreateWithHandle(
+    base::UnsafeSharedMemoryRegion region) {
+  return base::WrapUnique(new TransportDIB(std::move(region)));
+}
+
+std::unique_ptr<SkCanvas> TransportDIB::GetPlatformCanvas(int w,
+                                                          int h,
+                                                          bool opaque) {
+  if (!shm_region_.IsValid())
+    return nullptr;
+
+#if defined(OS_WIN)
+  // This DIB already mapped the file into this process, but PlatformCanvas
+  // will map it again.
+  DCHECK(!memory()) << "Mapped file twice in the same process.";
+
+  // We can't check the canvas size before mapping, but it's safe because
+  // Windows will fail to map the section if the dimensions of the canvas
+  // are too large.
+  std::unique_ptr<SkCanvas> canvas =
+      skia::CreatePlatformCanvasWithSharedSection(
+          w, h, opaque, shm_region_.GetPlatformHandle(),
+          skia::RETURN_NULL_ON_FAILURE);
+
+  // Calculate the size for the memory region backing the canvas.
+  if (canvas)
+    size_ = skia::PlatformCanvasStrideForWidth(w) * h;
+
+  return canvas;
+#else
+  if ((!memory() && !Map()) || !VerifyCanvasSize(w, h))
+    return nullptr;
+  return skia::CreatePlatformCanvasWithPixels(w, h, opaque,
+                                              static_cast<uint8_t*>(memory()),
+                                              skia::RETURN_NULL_ON_FAILURE);
+#endif
+}
+
+bool TransportDIB::Map() {
+  if (!shm_region_.IsValid())
+    return false;
+
+  if (memory())
+    return true;
+
+  shm_mapping_ = shm_region_.Map();
+  if (!shm_mapping_.IsValid()) {
+    PLOG(ERROR) << "Failed to map transport DIB";
     return false;
   }
 
-  return (stride * h) <= size_;
+  size_ = shm_mapping_.size();
+  return true;
+}
+
+void* TransportDIB::memory() const {
+  return shm_mapping_.IsValid() ? shm_mapping_.memory() : nullptr;
+}
+
+base::UnsafeSharedMemoryRegion* TransportDIB::shared_memory_region() {
+  return &shm_region_;
 }
 
-base::SharedMemory* TransportDIB::shared_memory() {
-  return &shared_memory_;
+// static
+bool TransportDIB::VerifyCanvasSize(int w, int h) {
+  if (w <= 0 || h <= 0)
+    return false;
+
+  const size_t stride = skia::PlatformCanvasStrideForWidth(w);
+  size_t canvas_size;
+  if (!base::CheckMul(h, stride).AssignIfValid(&canvas_size))
+    return false;
+
+  return canvas_size <= size_;
 }

ui/surface/transport_dib.h

@@ -6,10 +6,11 @@
 #define UI_SURFACE_TRANSPORT_DIB_H_
 
 #include <stddef.h>
-#include <stdint.h>
+#include <memory>
 
 #include "base/macros.h"
-#include "base/memory/shared_memory.h"
+#include "base/memory/shared_memory_mapping.h"
+#include "base/memory/unsafe_shared_memory_region.h"
 #include "build/build_config.h"
 #include "ui/surface/surface_export.h"
 
@@ -17,8 +18,6 @@
 #include <windows.h>
 #endif
 
-#include <memory>
-
 class SkCanvas;
 
 // -----------------------------------------------------------------------------
@@ -30,20 +29,15 @@ class SURFACE_EXPORT TransportDIB {
  public:
   ~TransportDIB();
 
-  // A Handle is the type which can be sent over the wire so that the remote
-  // side can map the transport DIB.
-  typedef base::SharedMemoryHandle Handle;
-
-  // Map the referenced transport DIB.  The caller owns the returned object.
-  // Returns NULL on failure.
-  static TransportDIB* Map(Handle transport_dib);
+  // Creates and maps a new TransportDIB with a shared memory region.
+  // Returns nullptr on failure.
+  static std::unique_ptr<TransportDIB> Map(
+      base::UnsafeSharedMemoryRegion region);
 
-  // Create a new |TransportDIB| with a handle to the shared memory. This
-  // always returns a valid pointer. The DIB is not mapped.
-  static TransportDIB* CreateWithHandle(Handle handle);
-
-  // Returns true if the handle is valid.
-  static bool is_valid_handle(Handle dib);
+  // Creates a new TransportDIB with a shared memory region. This always returns
+  // a valid pointer. The DIB is not mapped.
+  static std::unique_ptr<TransportDIB> CreateWithHandle(
+      base::UnsafeSharedMemoryRegion region);
 
   // Returns a canvas using the memory of this TransportDIB. The returned
   // pointer will be owned by the caller. The bitmap will be of the given size,
@@ -70,18 +64,19 @@ class SURFACE_EXPORT TransportDIB {
   // the maximum amount that /could/ be valid.
   size_t size() const { return size_; }
 
-  // Returns a pointer to the SharedMemory object that backs the transport dib.
-  base::SharedMemory* shared_memory();
+  // Returns a pointer to the UnsafeSharedMemoryRegion object that backs the
+  // transport dib.
+  base::UnsafeSharedMemoryRegion* shared_memory_region();
 
  private:
-  TransportDIB();
-
   // Verifies that the dib can hold a canvas of the requested dimensions.
   bool VerifyCanvasSize(int w, int h);
 
-  explicit TransportDIB(base::SharedMemoryHandle dib);
-  base::SharedMemory shared_memory_;
-  size_t size_;  // length, in bytes
+  explicit TransportDIB(base::UnsafeSharedMemoryRegion region);
+
+  base::UnsafeSharedMemoryRegion shm_region_;
+  base::WritableSharedMemoryMapping shm_mapping_;
+  size_t size_ = 0;
 
   DISALLOW_COPY_AND_ASSIGN(TransportDIB);
 };

ui/surface/transport_dib_posix.cc

-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "ui/surface/transport_dib.h"
-
-#include <stddef.h>
-#include <stdint.h>
-#include <sys/stat.h>
-#include <unistd.h>
-
-#include <memory>
-
-#include "base/logging.h"
-#include "base/memory/shared_memory.h"
-#include "build/build_config.h"
-#include "skia/ext/platform_canvas.h"
-
-TransportDIB::TransportDIB()
-    : size_(0) {
-}
-
-TransportDIB::TransportDIB(TransportDIB::Handle dib)
-    : shared_memory_(dib, false /* read write */),
-      size_(0) {
-}
-
-TransportDIB::~TransportDIB() {
-}
-
-// static
-TransportDIB* TransportDIB::Map(Handle handle) {
-  std::unique_ptr<TransportDIB> dib(CreateWithHandle(handle));
-  if (!dib->Map())
-    return NULL;
-  return dib.release();
-}
-
-// static
-TransportDIB* TransportDIB::CreateWithHandle(Handle handle) {
-  return new TransportDIB(handle);
-}
-
-// static
-bool TransportDIB::is_valid_handle(Handle dib) {
-  return base::SharedMemory::IsHandleValid(dib);
-}
-
-std::unique_ptr<SkCanvas> TransportDIB::GetPlatformCanvas(int w,
-                                                          int h,
-                                                          bool opaque) {
-  if ((!memory() && !Map()) || !VerifyCanvasSize(w, h))
-    return NULL;
-  return skia::CreatePlatformCanvasWithPixels(w, h, opaque,
-                                              static_cast<uint8_t*>(memory()),
-                                              skia::RETURN_NULL_ON_FAILURE);
-}
-
-bool TransportDIB::Map() {
-  if (!is_valid_handle(shared_memory_.handle()))
-    return false;
-#if defined(OS_ANDROID)
-  if (!shared_memory_.Map(0))
-    return false;
-  size_ = shared_memory_.mapped_size();
-#else
-  if (memory())
-    return true;
-
-  size_t size = shared_memory_.handle().GetSize();
-  if (!shared_memory_.Map(size))
-    return false;
-
-  size_ = size;
-#endif
-  return true;
-}
-
-void* TransportDIB::memory() const {
-  return shared_memory_.memory();
-}

ui/surface/transport_dib_win.cc

-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "ui/surface/transport_dib.h"
-
-#include <windows.h>
-#include <stddef.h>
-#include <stdint.h>
-
-#include <limits>
-#include <memory>
-
-#include "base/logging.h"
-#include "base/system/sys_info.h"
-#include "skia/ext/platform_canvas.h"
-
-TransportDIB::TransportDIB()
-    : size_(0) {
-}
-
-TransportDIB::~TransportDIB() {
-}
-
-TransportDIB::TransportDIB(base::SharedMemoryHandle handle)
-    : shared_memory_(handle, false /* read write */), size_(0) {}
-
-// static
-TransportDIB* TransportDIB::Map(Handle handle) {
-  std::unique_ptr<TransportDIB> dib(CreateWithHandle(handle));
-  if (!dib->Map())
-    return NULL;
-  return dib.release();
-}
-
-// static
-TransportDIB* TransportDIB::CreateWithHandle(Handle handle) {
-  return new TransportDIB(handle);
-}
-
-// static
-bool TransportDIB::is_valid_handle(Handle dib) {
-  return dib.IsValid();
-}
-
-std::unique_ptr<SkCanvas> TransportDIB::GetPlatformCanvas(int w,
-                                                          int h,
-                                                          bool opaque) {
-  // This DIB already mapped the file into this process, but PlatformCanvas
-  // will map it again.
-  DCHECK(!memory()) << "Mapped file twice in the same process.";
-
-  // We can't check the canvas size before mapping, but it's safe because
-  // Windows will fail to map the section if the dimensions of the canvas
-  // are too large.
-  std::unique_ptr<SkCanvas> canvas =
-      skia::CreatePlatformCanvasWithSharedSection(
-          w, h, opaque, shared_memory_.handle().GetHandle(),
-          skia::RETURN_NULL_ON_FAILURE);
-
-  // Calculate the size for the memory region backing the canvas.
-  if (canvas)
-    size_ = skia::PlatformCanvasStrideForWidth(w) * h;
-
-  return canvas;
-}
-
-bool TransportDIB::Map() {
-  if (!is_valid_handle(shared_memory_.handle()))
-    return false;
-  if (memory())
-    return true;
-
-  if (!shared_memory_.Map(0 /* map whole shared memory segment */)) {
-    LOG(ERROR) << "Failed to map transport DIB"
-               << " handle:" << shared_memory_.handle().GetHandle()
-               << " error:" << ::GetLastError();
-    return false;
-  }
-
-  size_ = shared_memory_.mapped_size();
-  return true;
-}
-
-void* TransportDIB::memory() const {
-  return shared_memory_.memory();
-}