Don't use SharedMemory[Handle] to hold a DXGI resource handle

Impending changes to Mojo IPC will require that handles serialized
as shared memory handles are actually shared memory handles. DXGI
resources are not, but are being transported using
GpuMemoryBufferHandle's shared_memory field.

This introduces a Windows-only GpuMemoryBufferHandle field for a
generic HANDLE to be used for DXGI resources, as well as a corresponding
mojom field for the same purpose.

There should be no net effect on the behavior of the system wrt handle
lifetime or usage.


Bug: 826213
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I5d510e0ebdb60d4872fc428a95ce681dd18369dc
Reviewed-on: https://chromium-review.googlesource.com/1011600Reviewed-by: Kenneth Russell <kbr@chromium.org>
Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org>
Reviewed-by: Bill Orr <billorr@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: David Reveman <reveman@chromium.org>
Commit-Queue: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551231}

gpu/ipc/common/gpu_memory_buffer_impl_dxgi.cc

@@ -25,9 +25,10 @@ GpuMemoryBufferImplDXGI::CreateFromHandle(
     gfx::BufferFormat format,
     gfx::BufferUsage usage,
     const DestructionCallback& callback) {
-  DCHECK(base::SharedMemory::IsHandleValid(handle.handle));
-  return base::WrapUnique(new GpuMemoryBufferImplDXGI(handle.id, size, format,
-                                                      callback, handle.handle));
+  DCHECK(handle.dxgi_handle.IsValid());
+  return base::WrapUnique(new GpuMemoryBufferImplDXGI(
+      handle.id, size, format, callback,
+      base::win::ScopedHandle(handle.dxgi_handle.GetHandle())));
 }
 
 base::Closure GpuMemoryBufferImplDXGI::AllocateForTesting(
@@ -77,8 +78,7 @@ base::Closure GpuMemoryBufferImplDXGI::AllocateForTesting(
   DCHECK(SUCCEEDED(hr));
 
   gfx::GpuMemoryBufferId kBufferId(1);
-  handle->handle = base::SharedMemoryHandle(texture_handle, 0,
-                                            base::UnguessableToken::Create());
+  handle->dxgi_handle = IPC::PlatformFileForTransit(texture_handle);
   handle->type = gfx::DXGI_SHARED_HANDLE;
   handle->id = kBufferId;
   return base::DoNothing();
@@ -104,7 +104,7 @@ gfx::GpuMemoryBufferHandle GpuMemoryBufferImplDXGI::GetHandle() const {
   handle.id = id_;
   handle.offset = 0;
   handle.stride = stride(0);
-  handle.handle = shared_memory_.handle();
+  handle.dxgi_handle = IPC::PlatformFileForTransit(dxgi_handle_.Get());
   return handle;
 }
 
@@ -113,8 +113,8 @@ GpuMemoryBufferImplDXGI::GpuMemoryBufferImplDXGI(
     const gfx::Size& size,
     gfx::BufferFormat format,
     const DestructionCallback& callback,
-    const base::SharedMemoryHandle& dxgi_handle)
+    base::win::ScopedHandle dxgi_handle)
     : GpuMemoryBufferImpl(id, size, format, callback),
-      shared_memory_(dxgi_handle, false) {}
+      dxgi_handle_(std::move(dxgi_handle)) {}
 
 }  // namespace gpu

gpu/ipc/common/gpu_memory_buffer_impl_dxgi.h

@@ -48,9 +48,9 @@ class GPU_EXPORT GpuMemoryBufferImplDXGI : public GpuMemoryBufferImpl {
                           const gfx::Size& size,
                           gfx::BufferFormat format,
                           const DestructionCallback& callback,
-                          const base::SharedMemoryHandle& dxgi_handle);
+                          base::win::ScopedHandle dxgi_handle);
 
-  base::SharedMemory shared_memory_;
+  base::win::ScopedHandle dxgi_handle_;
 
   DISALLOW_COPY_AND_ASSIGN(GpuMemoryBufferImplDXGI);
 };

gpu/ipc/service/gpu_memory_buffer_factory_dxgi.cc

@@ -82,8 +82,7 @@ gfx::GpuMemoryBufferHandle GpuMemoryBufferFactoryDXGI::CreateGpuMemoryBuffer(
   if (!BufferSizeForBufferFormatChecked(size, format, &buffer_size))
     return handle;
 
-  handle.handle = base::SharedMemoryHandle(texture_handle, buffer_size,
-                                           base::UnguessableToken::Create());
+  handle.dxgi_handle = IPC::PlatformFileForTransit(texture_handle);
   handle.type = gfx::DXGI_SHARED_HANDLE;
   handle.id = id;
 
@@ -110,7 +109,7 @@ GpuMemoryBufferFactoryDXGI::CreateImageForGpuMemoryBuffer(
     return nullptr;
   // Transfer ownership of handle to GLImageDXGIHandle.
   base::win::ScopedHandle handle_owner;
-  handle_owner.Set(handle.handle.GetHandle());
+  handle_owner.Set(handle.dxgi_handle.GetHandle());
   auto image = base::MakeRefCounted<gl::GLImageDXGIHandle>(size, 0, format);
   if (!image->Initialize(std::move(handle_owner)))
     return nullptr;

third_party/blink/renderer/platform/graphics/gpu/xr_frame_transport.cc

@@ -134,7 +134,8 @@ void XRFrameTransport::FrameSubmit(
       gfx::GpuMemoryBufferHandle gpu_handle =
           CloneHandleForIPC(gpu_memory_buffer->GetHandle());
       vr_presentation_provider->SubmitFrameWithTextureHandle(
-          vr_frame_id, mojo::WrapPlatformFile(gpu_handle.handle.GetHandle()));
+          vr_frame_id,
+          mojo::WrapPlatformFile(gpu_handle.dxgi_handle.GetHandle()));
     }
 #else
     NOTIMPLEMENTED();

ui/gfx/BUILD.gn

@@ -574,6 +574,10 @@ source_set("memory_buffer_sources") {
 
     public_deps += [ "//ui/gfx:color_space" ]
   }
+
+  if (is_win) {
+    public_deps += [ "//ipc:message_support" ]
+  }
 }
 
 # TODO(ccameron): This can be moved into a separate source_set.

ui/gfx/gpu_memory_buffer.cc

@@ -4,8 +4,13 @@
 
 #include "ui/gfx/gpu_memory_buffer.h"
 
+#include "base/process/process_handle.h"
 #include "ui/gfx/generic_shared_memory_id.h"
 
+#if defined(OS_WIN)
+#include <windows.h>
+#endif
+
 namespace gfx {
 
 GpuMemoryBufferHandle::GpuMemoryBufferHandle() : type(EMPTY_BUFFER), id(0) {}
@@ -13,7 +18,7 @@ GpuMemoryBufferHandle::GpuMemoryBufferHandle() : type(EMPTY_BUFFER), id(0) {}
 GpuMemoryBufferHandle::GpuMemoryBufferHandle(
     const GpuMemoryBufferHandle& other) = default;
 
-GpuMemoryBufferHandle::~GpuMemoryBufferHandle() {}
+GpuMemoryBufferHandle::~GpuMemoryBufferHandle() = default;
 
 void GpuMemoryBuffer::SetColorSpace(const gfx::ColorSpace& color_space) {}
 
@@ -55,7 +60,16 @@ GpuMemoryBufferHandle CloneHandleForIPC(
       gfx::GpuMemoryBufferHandle handle;
       handle.type = gfx::DXGI_SHARED_HANDLE;
       handle.id = source_handle.id;
-      handle.handle = base::SharedMemory::DuplicateHandle(source_handle.handle);
+#if defined(OS_WIN)
+      base::ProcessHandle process = ::GetCurrentProcess();
+      HANDLE duplicated_handle;
+      BOOL result = ::DuplicateHandle(
+          process, source_handle.dxgi_handle.GetHandle(), process,
+          &duplicated_handle, 0, FALSE, DUPLICATE_SAME_ACCESS);
+      if (!result)
+        DPLOG(ERROR) << "Failed to duplicate DXGI resource handle.";
+      handle.dxgi_handle = IPC::PlatformFileForTransit(duplicated_handle);
+#endif
       return handle;
   }
   return gfx::GpuMemoryBufferHandle();

ui/gfx/gpu_memory_buffer.h

@@ -20,6 +20,8 @@
 #include "ui/gfx/native_pixmap_handle.h"
 #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
 #endif
 
 extern "C" typedef struct _ClientBuffer* ClientBuffer;
@@ -54,6 +56,8 @@ struct GFX_EXPORT GpuMemoryBufferHandle {
   NativePixmapHandle native_pixmap_handle;
 #elif defined(OS_MACOSX) && !defined(OS_IOS)
   ScopedRefCountedIOSurfaceMachPort mach_port;
+#elif defined(OS_WIN)
+  IPC::PlatformFileForTransit dxgi_handle;
 #endif
 };
 

ui/gfx/ipc/gfx_param_traits_macros.h

@@ -55,6 +55,8 @@ IPC_STRUCT_TRAITS_BEGIN(gfx::GpuMemoryBufferHandle)
   IPC_STRUCT_TRAITS_MEMBER(native_pixmap_handle)
 #elif defined(OS_MACOSX)
   IPC_STRUCT_TRAITS_MEMBER(mach_port)
+#elif defined(OS_WIN)
+  IPC_STRUCT_TRAITS_MEMBER(dxgi_handle)
 #endif
 IPC_STRUCT_TRAITS_END()
 

ui/gfx/mojo/buffer_types.mojom

@@ -90,4 +90,7 @@ struct GpuMemoryBufferHandle {
   uint32 stride;
   NativePixmapHandle? native_pixmap_handle;
   handle? mach_port;
+
+  [EnableIf=is_win]
+  handle? dxgi_handle;
 };

ui/gfx/mojo/buffer_types_struct_traits.cc

@@ -55,7 +55,6 @@ StructTraits<gfx::mojom::GpuMemoryBufferHandleDataView,
              gfx::GpuMemoryBufferHandle>::
     shared_memory_handle(const gfx::GpuMemoryBufferHandle& handle) {
   if (handle.type != gfx::SHARED_MEMORY_BUFFER &&
-      handle.type != gfx::DXGI_SHARED_HANDLE &&
       handle.type != gfx::ANDROID_HARDWARE_BUFFER)
     return mojo::ScopedSharedBufferHandle();
   return mojo::WrapSharedMemoryHandle(
@@ -87,6 +86,18 @@ mojo::ScopedHandle StructTraits<gfx::mojom::GpuMemoryBufferHandleDataView,
 #endif
 }
 
+#if defined(OS_WIN)
+// static
+mojo::ScopedHandle StructTraits<gfx::mojom::GpuMemoryBufferHandleDataView,
+                                gfx::GpuMemoryBufferHandle>::
+    dxgi_handle(const gfx::GpuMemoryBufferHandle& handle) {
+  if (handle.type != gfx::DXGI_SHARED_HANDLE)
+    return mojo::ScopedHandle();
+  DCHECK(handle.dxgi_handle.IsValid());
+  return mojo::WrapPlatformFile(handle.dxgi_handle.GetHandle());
+}
+#endif
+
 bool StructTraits<gfx::mojom::GpuMemoryBufferHandleDataView,
                   gfx::GpuMemoryBufferHandle>::
     Read(gfx::mojom::GpuMemoryBufferHandleDataView data,
@@ -95,7 +106,6 @@ bool StructTraits<gfx::mojom::GpuMemoryBufferHandleDataView,
     return false;
 
   if (out->type == gfx::SHARED_MEMORY_BUFFER ||
-      out->type == gfx::DXGI_SHARED_HANDLE ||
       out->type == gfx::ANDROID_HARDWARE_BUFFER) {
     mojo::ScopedSharedBufferHandle handle = data.TakeSharedMemoryHandle();
     if (handle.is_valid()) {
@@ -122,6 +132,18 @@ bool StructTraits<gfx::mojom::GpuMemoryBufferHandleDataView,
       return false;
     out->mach_port.reset(mach_port);
   }
+#endif
+#if defined(OS_WIN)
+  if (out->type == gfx::DXGI_SHARED_HANDLE) {
+    HANDLE handle;
+    MojoResult unwrap_result =
+        mojo::UnwrapPlatformFile(data.TakeDxgiHandle(), &handle);
+    if (unwrap_result != MOJO_RESULT_OK)
+      return false;
+    out->dxgi_handle = IPC::PlatformFileForTransit(handle);
+    out->offset = data.offset();
+    out->stride = data.stride();
+  }
 #endif
   return true;
 }

ui/gfx/mojo/buffer_types_struct_traits.h

@@ -5,6 +5,7 @@
 #ifndef UI_GFX_MOJO_BUFFER_TYPES_STRUCT_TRAITS_H_
 #define UI_GFX_MOJO_BUFFER_TYPES_STRUCT_TRAITS_H_
 
+#include "build/build_config.h"
 #include "ui/gfx/buffer_types.h"
 #include "ui/gfx/mojo/buffer_types.mojom.h"
 
@@ -325,6 +326,12 @@ struct StructTraits<gfx::mojom::GpuMemoryBufferHandleDataView,
   static const gfx::NativePixmapHandle& native_pixmap_handle(
       const gfx::GpuMemoryBufferHandle& handle);
   static mojo::ScopedHandle mach_port(const gfx::GpuMemoryBufferHandle& handle);
+
+#if defined(OS_WIN)
+  static mojo::ScopedHandle dxgi_handle(
+      const gfx::GpuMemoryBufferHandle& handle);
+#endif
+
   static bool Read(gfx::mojom::GpuMemoryBufferHandleDataView data,
                    gfx::GpuMemoryBufferHandle* handle);
 };