Make GpuMemoryBufferHandle::dxgi_handle a base::win::ScopedHandle

In the process, convert the buffer_types.mojom to handle<platform>. This also adds a legacy IPC::ParamTraits<ScopedHandle> to support sending GpuMemoryBufferHandle over legacy IPC. Tbr: flackr@chromium.org Bug: 863011, 710376 Change-Id: I087d9d83edc729b512dbb79806a0b4b2db2b31bd Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2064609 Commit-Queue: Robert Sesek <rsesek@chromium.org> Reviewed-by: Ken Rockot <rockot@google.com> Reviewed-by: Daniele Castagna <dcastagna@chromium.org> Reviewed-by: Michael Thiessen <mthiesse@chromium.org> Cr-Commit-Position: refs/heads/master@{#745586}
Denger-Network · Feb 28, 2020 · 0291066 · 0291066
1 parent 96b03b9
commit 0291066
Show file tree

Hide file tree

Showing 11 changed files with 92 additions and 32 deletions.
diff --git a/chrome/browser/vr/win/graphics_delegate_win.cc b/chrome/browser/vr/win/graphics_delegate_win.cc
@@ -122,8 +122,7 @@ mojo::PlatformHandle GraphicsDelegateWin::GetTexture() {
     return {};
 
   gfx::GpuMemoryBufferHandle gpu_handle = gpu_memory_buffer_->CloneHandle();
-  return mojo::PlatformHandle(
-      base::win::ScopedHandle(gpu_handle.dxgi_handle.GetHandle()));
+  return mojo::PlatformHandle(std::move(gpu_handle.dxgi_handle));
 }
 
 gfx::RectF GraphicsDelegateWin::GetLeft() {

diff --git a/gpu/ipc/common/gpu_memory_buffer_impl_dxgi.cc b/gpu/ipc/common/gpu_memory_buffer_impl_dxgi.cc
@@ -19,16 +19,15 @@ namespace gpu {
 GpuMemoryBufferImplDXGI::~GpuMemoryBufferImplDXGI() {}
 
 std::unique_ptr<GpuMemoryBufferImplDXGI>
-GpuMemoryBufferImplDXGI::CreateFromHandle(
-    const gfx::GpuMemoryBufferHandle& handle,
-    const gfx::Size& size,
-    gfx::BufferFormat format,
-    gfx::BufferUsage usage,
-    DestructionCallback callback) {
+GpuMemoryBufferImplDXGI::CreateFromHandle(gfx::GpuMemoryBufferHandle handle,
+                                          const gfx::Size& size,
+                                          gfx::BufferFormat format,
+                                          gfx::BufferUsage usage,
+                                          DestructionCallback callback) {
   DCHECK(handle.dxgi_handle.IsValid());
-  return base::WrapUnique(new GpuMemoryBufferImplDXGI(
-      handle.id, size, format, std::move(callback),
-      base::win::ScopedHandle(handle.dxgi_handle.GetHandle())));
+  return base::WrapUnique(
+      new GpuMemoryBufferImplDXGI(handle.id, size, format, std::move(callback),
+                                  std::move(handle.dxgi_handle)));
 }
 
 base::OnceClosure GpuMemoryBufferImplDXGI::AllocateForTesting(
@@ -78,7 +77,7 @@ base::OnceClosure GpuMemoryBufferImplDXGI::AllocateForTesting(
   DCHECK(SUCCEEDED(hr));
 
   gfx::GpuMemoryBufferId kBufferId(1);
-  handle->dxgi_handle = IPC::PlatformFileForTransit(texture_handle);
+  handle->dxgi_handle.Set(texture_handle);
   handle->type = gfx::DXGI_SHARED_HANDLE;
   handle->id = kBufferId;
   return base::DoNothing();
@@ -115,7 +114,7 @@ gfx::GpuMemoryBufferHandle GpuMemoryBufferImplDXGI::CloneHandle() const {
                         &duplicated_handle, 0, FALSE, DUPLICATE_SAME_ACCESS);
   if (!result)
     DPLOG(ERROR) << "Failed to duplicate DXGI resource handle.";
-  handle.dxgi_handle = IPC::PlatformFileForTransit(duplicated_handle);
+  handle.dxgi_handle.Set(duplicated_handle);
   return handle;
 }
 

diff --git a/gpu/ipc/common/gpu_memory_buffer_impl_dxgi.h b/gpu/ipc/common/gpu_memory_buffer_impl_dxgi.h
@@ -26,7 +26,7 @@ class GPU_EXPORT GpuMemoryBufferImplDXGI : public GpuMemoryBufferImpl {
       gfx::DXGI_SHARED_HANDLE;
 
   static std::unique_ptr<GpuMemoryBufferImplDXGI> CreateFromHandle(
-      const gfx::GpuMemoryBufferHandle& handle,
+      gfx::GpuMemoryBufferHandle handle,
       const gfx::Size& size,
       gfx::BufferFormat format,
       gfx::BufferUsage usage,

diff --git a/gpu/ipc/service/gpu_memory_buffer_factory_dxgi.cc b/gpu/ipc/service/gpu_memory_buffer_factory_dxgi.cc
@@ -82,7 +82,7 @@ gfx::GpuMemoryBufferHandle GpuMemoryBufferFactoryDXGI::CreateGpuMemoryBuffer(
   if (!BufferSizeForBufferFormatChecked(size, format, &buffer_size))
     return handle;
 
-  handle.dxgi_handle = IPC::PlatformFileForTransit(texture_handle);
+  handle.dxgi_handle.Set(texture_handle);
   handle.type = gfx::DXGI_SHARED_HANDLE;
   handle.id = id;
 
@@ -107,10 +107,8 @@ GpuMemoryBufferFactoryDXGI::CreateImageForGpuMemoryBuffer(
   if (handle.type != gfx::DXGI_SHARED_HANDLE)
     return nullptr;
   // Transfer ownership of handle to GLImageDXGI.
-  base::win::ScopedHandle handle_owner;
-  handle_owner.Set(handle.dxgi_handle.GetHandle());
   auto image = base::MakeRefCounted<gl::GLImageDXGI>(size, nullptr);
-  if (!image->InitializeHandle(std::move(handle_owner), 0, format))
+  if (!image->InitializeHandle(std::move(handle.dxgi_handle), 0, format))
     return nullptr;
   return image;
 }

diff --git a/ipc/ipc_message_utils.cc b/ipc/ipc_message_utils.cc
@@ -624,6 +624,43 @@ void ParamTraits<base::ScopedFD>::Log(const param_type& p, std::string* l) {
 }
 #endif  // defined(OS_POSIX) || defined(OS_FUCHSIA)
 
+#if defined(OS_WIN)
+void ParamTraits<base::win::ScopedHandle>::Write(base::Pickle* m,
+                                                 const param_type& p) {
+  const bool valid = p.IsValid();
+  WriteParam(m, valid);
+  if (!valid)
+    return;
+
+  HandleWin handle(p.Get());
+  WriteParam(m, handle);
+}
+
+bool ParamTraits<base::win::ScopedHandle>::Read(const base::Pickle* m,
+                                                base::PickleIterator* iter,
+                                                param_type* r) {
+  r->Close();
+
+  bool valid;
+  if (!ReadParam(m, iter, &valid))
+    return false;
+  if (!valid)
+    return true;
+
+  HandleWin handle;
+  if (!ReadParam(m, iter, &handle))
+    return false;
+
+  r->Set(handle.get_handle());
+  return true;
+}
+
+void ParamTraits<base::win::ScopedHandle>::Log(const param_type& p,
+                                               std::string* l) {
+  l->append(base::StringPrintf("ScopedHandle(%p)", p.Get()));
+}
+#endif  // defined(OS_WIN)
+
 #if defined(OS_FUCHSIA)
 void ParamTraits<zx::vmo>::Write(base::Pickle* m, const param_type& p) {
   // This serialization must be kept in sync with

diff --git a/ipc/ipc_message_utils.h b/ipc/ipc_message_utils.h
@@ -607,6 +607,18 @@ struct COMPONENT_EXPORT(IPC) ParamTraits<base::ScopedFD> {
 
 #endif  // defined(OS_POSIX) || defined(OS_FUCHSIA)
 
+#if defined(OS_WIN)
+template <>
+struct COMPONENT_EXPORT(IPC) ParamTraits<base::win::ScopedHandle> {
+  using param_type = base::win::ScopedHandle;
+  static void Write(base::Pickle* m, const param_type& p);
+  static bool Read(const base::Pickle* m,
+                   base::PickleIterator* iter,
+                   param_type* r);
+  static void Log(const param_type& p, std::string* l);
+};
+#endif
+
 #if defined(OS_FUCHSIA)
 template <>
 struct COMPONENT_EXPORT(IPC) ParamTraits<zx::vmo> {

diff --git a/ipc/ipc_message_utils_unittest.cc b/ipc/ipc_message_utils_unittest.cc
@@ -13,10 +13,15 @@
 #include "base/memory/ptr_util.h"
 #include "base/test/test_shared_memory_util.h"
 #include "base/unguessable_token.h"
+#include "build/build_config.h"
 #include "ipc/ipc_channel_handle.h"
 #include "ipc/ipc_message.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+#if defined(OS_WIN)
+#include <windows.h>
+#endif
+
 namespace IPC {
 namespace {
 
@@ -230,5 +235,23 @@ TEST(IPCMessageUtilsTest, StrongAlias) {
   EXPECT_EQ(input, output);
 }
 
+#if defined(OS_WIN)
+TEST(IPCMessageUtilsTest, ScopedHandle) {
+  HANDLE raw_dupe_handle;
+  ASSERT_TRUE(::DuplicateHandle(::GetCurrentProcess(), ::GetCurrentProcess(),
+                                ::GetCurrentProcess(), &raw_dupe_handle, 0,
+                                FALSE, DUPLICATE_SAME_ACCESS));
+  base::win::ScopedHandle dupe_handle(raw_dupe_handle);
+
+  Message message(0, 0, Message::PRIORITY_LOW);
+  WriteParam(&message, dupe_handle);
+
+  base::PickleIterator iter(message);
+  base::win::ScopedHandle read_handle;
+  EXPECT_TRUE(ReadParam(&message, &iter, &read_handle));
+  EXPECT_TRUE(read_handle.IsValid());
+}
+#endif  // defined(OS_WIN)
+
 }  // namespace
 }  // namespace IPC
diff --git a/third_party/blink/renderer/platform/graphics/gpu/xr_frame_transport.cc b/third_party/blink/renderer/platform/graphics/gpu/xr_frame_transport.cc
@@ -129,8 +129,7 @@ void XRFrameTransport::FrameSubmit(
     // passed over IPC.
     gfx::GpuMemoryBufferHandle gpu_handle = gpu_memory_buffer->CloneHandle();
     vr_presentation_provider->SubmitFrameWithTextureHandle(
-        vr_frame_id, mojo::PlatformHandle(base::win::ScopedHandle(
-                         gpu_handle.dxgi_handle.GetHandle())));
+        vr_frame_id, mojo::PlatformHandle(std::move(gpu_handle.dxgi_handle)));
 #else
     NOTIMPLEMENTED();
 #endif

diff --git a/ui/gfx/gpu_memory_buffer.h b/ui/gfx/gpu_memory_buffer.h
@@ -20,7 +20,7 @@
 #elif defined(OS_MACOSX) && !defined(OS_IOS)
 #include "ui/gfx/mac/io_surface.h"
 #elif defined(OS_WIN)
-#include "ipc/ipc_platform_file.h"  // nogncheck
+#include "base/win/scoped_handle.h"
 #elif defined(OS_ANDROID)
 #include "base/android/scoped_hardware_buffer_handle.h"
 #endif
@@ -70,8 +70,7 @@ struct GFX_EXPORT GpuMemoryBufferHandle {
 #elif defined(OS_MACOSX) && !defined(OS_IOS)
   ScopedRefCountedIOSurfaceMachPort mach_port;
 #elif defined(OS_WIN)
-  // TODO(crbug.com/863011): convert this to a scoped handle.
-  IPC::PlatformFileForTransit dxgi_handle;
+  base::win::ScopedHandle dxgi_handle;
 #elif defined(OS_ANDROID)
   base::android::ScopedHardwareBufferHandle android_hardware_buffer;
 #endif

diff --git a/ui/gfx/mojom/buffer_types.mojom b/ui/gfx/mojom/buffer_types.mojom
@@ -101,7 +101,7 @@ union GpuMemoryBufferPlatformHandle {
   handle<platform> mach_port;
 
   [EnableIf=is_win]
-  handle dxgi_handle;
+  handle<platform> dxgi_handle;
 
   [EnableIf=is_android]
   AHardwareBufferHandle android_hardware_buffer_handle;

diff --git a/ui/gfx/mojom/buffer_types_mojom_traits.cc b/ui/gfx/mojom/buffer_types_mojom_traits.cc
@@ -6,7 +6,6 @@
 
 #include "build/build_config.h"
 #include "mojo/public/cpp/base/shared_memory_mojom_traits.h"
-#include "mojo/public/cpp/system/platform_handle.h"
 
 #if defined(OS_ANDROID)
 #include "base/android/scoped_hardware_buffer_handle.h"
@@ -105,7 +104,7 @@ gfx::mojom::GpuMemoryBufferPlatformHandlePtr StructTraits<
 #if defined(OS_WIN)
       DCHECK(handle.dxgi_handle.IsValid());
       return gfx::mojom::GpuMemoryBufferPlatformHandle::NewDxgiHandle(
-          mojo::WrapPlatformFile(handle.dxgi_handle.GetHandle()));
+          mojo::PlatformHandle(std::move(handle.dxgi_handle)));
 #else
       break;
 #endif
@@ -182,12 +181,7 @@ bool StructTraits<gfx::mojom::GpuMemoryBufferHandleDataView,
 #elif defined(OS_WIN)
     case gfx::mojom::GpuMemoryBufferPlatformHandleDataView::Tag::DXGI_HANDLE: {
       out->type = gfx::DXGI_SHARED_HANDLE;
-      HANDLE handle;
-      MojoResult unwrap_result = mojo::UnwrapPlatformFile(
-          std::move(platform_handle->get_dxgi_handle()), &handle);
-      if (unwrap_result != MOJO_RESULT_OK)
-        return false;
-      out->dxgi_handle = IPC::PlatformFileForTransit(handle);
+      out->dxgi_handle = platform_handle->get_dxgi_handle().TakeHandle();
       return true;
     }
 #elif defined(OS_ANDROID)