Change transport dib API to not make copies of SharedMemoryHandle.

I replaced the method handle() with shared_memory(). handle() would make a copy of SharedMemoryHandle, which makes assumptions about the implementation of SharedMemoryHandle. The new method shared_memory() returns a pointer to the Shared Memory object. BUG=466437 Review URL: https://codereview.chromium.org/1145893009 Cr-Commit-Position: refs/heads/master@{#332451}
Denger-Network · Jun 2, 2015 · 739bae4 · 739bae4
1 parent bf622f6
commit 739bae4
Show file tree

Hide file tree

Showing 11 changed files with 43 additions and 45 deletions.
diff --git a/content/renderer/pepper/pepper_compositor_host.cc b/content/renderer/pepper/pepper_compositor_host.cc
@@ -116,20 +116,21 @@ int32_t VerifyCommittedLayer(
       return PP_ERROR_BADARGUMENT;
     }
 
-    base::SharedMemoryHandle handle;
+    base::SharedMemory* shm;
     uint32_t byte_count;
-    if (enter.object()->GetSharedMemory(&handle, &byte_count) != PP_OK)
+    if (enter.object()->GetSharedMemory(&shm, &byte_count) != PP_OK)
       return PP_ERROR_FAILED;
 
 #if defined(OS_WIN)
     base::SharedMemoryHandle shm_handle;
-    if (!::DuplicateHandle(::GetCurrentProcess(), handle, ::GetCurrentProcess(),
-                           &shm_handle, 0, FALSE, DUPLICATE_SAME_ACCESS)) {
+    if (!::DuplicateHandle(::GetCurrentProcess(), shm->handle(),
+                           ::GetCurrentProcess(), &shm_handle, 0, FALSE,
+                           DUPLICATE_SAME_ACCESS)) {
       return PP_ERROR_FAILED;
     }
 #else
     base::SharedMemoryHandle shm_handle =
-        base::SharedMemory::DeepCopyHandle(handle, false);
+        base::SharedMemory::DeepCopyHandle(shm->handle(), false);
 #endif
     image_shm->reset(new base::SharedMemory(shm_handle, true));
     if (!(*image_shm)->Map(desc.stride * desc.size.height)) {

diff --git a/content/renderer/pepper/pepper_video_source_host.cc b/content/renderer/pepper/pepper_video_source_host.cc
@@ -127,8 +127,8 @@ void PepperVideoSourceHost::SendGetFrameReply() {
     // We have already allocated the correct size in shared memory. We need to
     // duplicate the handle for IPC however, which will close down the
     // duplicated handle when it's done.
-    base::SharedMemoryHandle local_handle;
-    if (shared_image_->GetSharedMemory(&local_handle, &byte_count) != PP_OK) {
+    base::SharedMemory* local_shm;
+    if (shared_image_->GetSharedMemory(&local_shm, &byte_count) != PP_OK) {
       SendGetFrameErrorReply(PP_ERROR_FAILED);
       return;
     }
@@ -140,7 +140,8 @@ void PepperVideoSourceHost::SendGetFrameReply() {
       return;
     }
 
-    image_handle = dispatcher->ShareSharedMemoryHandleWithRemote(local_handle);
+    image_handle =
+        dispatcher->ShareSharedMemoryHandleWithRemote(local_shm->handle());
   } else {
     // We need to allocate new shared memory.
     shared_image_ = NULL;  // Release any previous image.

diff --git a/content/renderer/pepper/ppb_image_data_impl.cc b/content/renderer/pepper/ppb_image_data_impl.cc
@@ -105,9 +105,9 @@ void* PPB_ImageData_Impl::Map() { return backend_->Map(); }
 
 void PPB_ImageData_Impl::Unmap() { backend_->Unmap(); }
 
-int32_t PPB_ImageData_Impl::GetSharedMemory(base::SharedMemoryHandle* handle,
+int32_t PPB_ImageData_Impl::GetSharedMemory(base::SharedMemory** shm,
                                             uint32_t* byte_count) {
-  return backend_->GetSharedMemory(handle, byte_count);
+  return backend_->GetSharedMemory(shm, byte_count);
 }
 
 skia::PlatformCanvas* PPB_ImageData_Impl::GetPlatformCanvas() {
@@ -188,11 +188,10 @@ void ImageDataPlatformBackend::Unmap() {
   // in the future to save some memory.
 }
 
-int32_t ImageDataPlatformBackend::GetSharedMemory(
-    base::SharedMemoryHandle* handle,
-    uint32_t* byte_count) {
+int32_t ImageDataPlatformBackend::GetSharedMemory(base::SharedMemory** shm,
+                                                  uint32_t* byte_count) {
   *byte_count = dib_->size();
-  *handle = dib_->handle();
+  *shm = dib_->shared_memory();
   return PP_OK;
 }
 
@@ -250,11 +249,10 @@ void ImageDataSimpleBackend::Unmap() {
     shared_memory_->Unmap();
 }
 
-int32_t ImageDataSimpleBackend::GetSharedMemory(
-    base::SharedMemoryHandle* handle,
-    uint32_t* byte_count) {
+int32_t ImageDataSimpleBackend::GetSharedMemory(base::SharedMemory** shm,
+                                                uint32_t* byte_count) {
   *byte_count = skia_bitmap_.getSize();
-  *handle = shared_memory_->handle();
+  *shm = shared_memory_.get();
   return PP_OK;
 }
 

diff --git a/content/renderer/pepper/ppb_image_data_impl.h b/content/renderer/pepper/ppb_image_data_impl.h
@@ -46,7 +46,7 @@ class CONTENT_EXPORT PPB_ImageData_Impl
     virtual TransportDIB* GetTransportDIB() const = 0;
     virtual void* Map() = 0;
     virtual void Unmap() = 0;
-    virtual int32_t GetSharedMemory(base::SharedMemoryHandle* handle,
+    virtual int32_t GetSharedMemory(base::SharedMemory** shm,
                                     uint32_t* byte_count) = 0;
     virtual SkCanvas* GetPlatformCanvas() = 0;
     virtual SkCanvas* GetCanvas() = 0;
@@ -92,7 +92,7 @@ class CONTENT_EXPORT PPB_ImageData_Impl
   PP_Bool Describe(PP_ImageDataDesc* desc) override;
   void* Map() override;
   void Unmap() override;
-  int32_t GetSharedMemory(base::SharedMemoryHandle* handle,
+  int32_t GetSharedMemory(base::SharedMemory** shm,
                           uint32_t* byte_count) override;
   SkCanvas* GetPlatformCanvas() override;
   SkCanvas* GetCanvas() override;
@@ -128,7 +128,7 @@ class ImageDataPlatformBackend : public PPB_ImageData_Impl::Backend {
   TransportDIB* GetTransportDIB() const override;
   void* Map() override;
   void Unmap() override;
-  int32_t GetSharedMemory(base::SharedMemoryHandle* handle,
+  int32_t GetSharedMemory(base::SharedMemory** shm,
                           uint32_t* byte_count) override;
   SkCanvas* GetPlatformCanvas() override;
   SkCanvas* GetCanvas() override;
@@ -162,7 +162,7 @@ class ImageDataSimpleBackend : public PPB_ImageData_Impl::Backend {
   TransportDIB* GetTransportDIB() const override;
   void* Map() override;
   void Unmap() override;
-  int32_t GetSharedMemory(base::SharedMemoryHandle* handle,
+  int32_t GetSharedMemory(base::SharedMemory** shm,
                           uint32_t* byte_count) override;
   SkCanvas* GetPlatformCanvas() override;
   SkCanvas* GetCanvas() override;

diff --git a/ppapi/proxy/ppb_image_data_proxy.cc b/ppapi/proxy/ppb_image_data_proxy.cc
@@ -347,7 +347,7 @@ PP_Bool ImageData::Describe(PP_ImageDataDesc* desc) {
   return PP_TRUE;
 }
 
-int32_t ImageData::GetSharedMemory(base::SharedMemoryHandle* /* handle */,
+int32_t ImageData::GetSharedMemory(base::SharedMemory** /* shm */,
                                    uint32_t* /* byte_count */) {
   // Not supported in the proxy (this method is for actually implementing the
   // proxy in the host).
@@ -593,14 +593,15 @@ PP_Resource PPB_ImageData_Proxy::CreateImageData(
     return 0;
   }
 
-  base::SharedMemoryHandle local_handle;
-  if (enter_resource.object()->GetSharedMemory(&local_handle, byte_count) !=
+  base::SharedMemory* local_shm;
+  if (enter_resource.object()->GetSharedMemory(&local_shm, byte_count) !=
       PP_OK) {
     DVLOG(1) << "CreateImageData failed: could not GetSharedMemory";
     return 0;
   }
 
-  *image_handle = dispatcher->ShareSharedMemoryHandleWithRemote(local_handle);
+  *image_handle =
+      dispatcher->ShareSharedMemoryHandleWithRemote(local_shm->handle());
   return resource.Release();
 }
 

diff --git a/ppapi/proxy/ppb_image_data_proxy.h b/ppapi/proxy/ppb_image_data_proxy.h
@@ -48,7 +48,7 @@ class PPAPI_PROXY_EXPORT ImageData
 
   // PPB_ImageData API.
   PP_Bool Describe(PP_ImageDataDesc* desc) override;
-  int32_t GetSharedMemory(base::SharedMemoryHandle* handle,
+  int32_t GetSharedMemory(base::SharedMemory** shm,
                           uint32_t* byte_count) override;
   void SetIsCandidateForReuse() override;
 

diff --git a/ppapi/thunk/ppb_image_data_api.h b/ppapi/thunk/ppb_image_data_api.h
@@ -5,12 +5,15 @@
 #ifndef PPAPI_THUNK_PPB_IMAGE_DATA_API_H_
 #define PPAPI_THUNK_PPB_IMAGE_DATA_API_H_
 
-#include "base/memory/shared_memory.h"
 #include "ppapi/c/pp_bool.h"
 #include "ppapi/c/ppb_image_data.h"
 
 class SkCanvas;
 
+namespace base {
+class SharedMemory;
+}  // namespace base
+
 namespace ppapi {
 namespace thunk {
 
@@ -23,7 +26,7 @@ class PPB_ImageData_API {
   virtual void Unmap() = 0;
 
   // Trusted inteface.
-  virtual int32_t GetSharedMemory(base::SharedMemoryHandle* handle,
+  virtual int32_t GetSharedMemory(base::SharedMemory** shm,
                                   uint32_t* byte_count) = 0;
 
   // Get the platform-specific canvas that backs this ImageData, if there is

diff --git a/ui/surface/transport_dib.cc b/ui/surface/transport_dib.cc
@@ -19,3 +19,6 @@ bool TransportDIB::VerifyCanvasSize(int w, int h) {
   return (stride * h) <= size_;
 }
 
+base::SharedMemory* TransportDIB::shared_memory() {
+  return &shared_memory_;
+}
diff --git a/ui/surface/transport_dib.h b/ui/surface/transport_dib.h
@@ -135,9 +135,8 @@ class SURFACE_EXPORT TransportDIB {
   // on the wire.
   Id id() const;
 
-  // Return a handle to the underlying shared memory. This can be sent over the
-  // wire to give this transport DIB to another process.
-  Handle handle() const;
+  // Returns a pointer to the SharedMemory object that backs the transport dib.
+  base::SharedMemory* shared_memory();
 
  private:
   TransportDIB();

diff --git a/ui/surface/transport_dib_posix.cc b/ui/surface/transport_dib_posix.cc
@@ -72,7 +72,7 @@ skia::PlatformCanvas* TransportDIB::GetPlatformCanvas(int w, int h) {
 }
 
 bool TransportDIB::Map() {
-  if (!is_valid_handle(handle()))
+  if (!is_valid_handle(shared_memory_.handle()))
     return false;
 #if defined(OS_ANDROID)
   if (!shared_memory_.Map(0))
@@ -98,12 +98,8 @@ void* TransportDIB::memory() const {
 
 TransportDIB::Id TransportDIB::id() const {
 #if defined(OS_ANDROID)
-  return handle();
+  return shared_memory_.handle();
 #else
   return shared_memory_.id();
 #endif
 }
-
-TransportDIB::Handle TransportDIB::handle() const {
-  return shared_memory_.handle();
-}
diff --git a/ui/surface/transport_dib_win.cc b/ui/surface/transport_dib_win.cc
@@ -72,7 +72,7 @@ skia::PlatformCanvas* TransportDIB::GetPlatformCanvas(int w, int h) {
   // Windows will fail to map the section if the dimensions of the canvas
   // are too large.
   skia::PlatformCanvas* canvas =
-      skia::CreatePlatformCanvas(w, h, true, handle(),
+      skia::CreatePlatformCanvas(w, h, true, shared_memory_.handle(),
                                  skia::RETURN_NULL_ON_FAILURE);
 
   // Calculate the size for the memory region backing the canvas.
@@ -83,7 +83,7 @@ skia::PlatformCanvas* TransportDIB::GetPlatformCanvas(int w, int h) {
 }
 
 bool TransportDIB::Map() {
-  if (!is_valid_handle(handle()))
+  if (!is_valid_handle(shared_memory_.handle()))
     return false;
   if (memory())
     return true;
@@ -103,10 +103,6 @@ void* TransportDIB::memory() const {
   return shared_memory_.memory();
 }
 
-TransportDIB::Handle TransportDIB::handle() const {
-  return shared_memory_.handle();
-}
-
 TransportDIB::Id TransportDIB::id() const {
-  return Id(handle(), sequence_num_);
+  return Id(shared_memory_.handle(), sequence_num_);
 }