gfx: Use union in mojom::GpuMemoryBufferHandle

The Mojo union more closely matches the actual intent of GpuMemoryBufferHandle,
since the handle can store only one of several types of platform handles.

GpuMemoryBufferHandle has |id| member that should be present all the time, so
mojom::GpuMemoryBufferHandle remains a Mojo struct and the union is embedded
into it.

Bug: 863011
Change-Id: I5dc9100392cd0a6f701e93ac4c91ca50e5077967
Reviewed-on: https://chromium-review.googlesource.com/c/1288609
Commit-Queue: Alexandr Ilin <alexilin@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601130}

ui/gfx/mojo/buffer_types.mojom

@@ -91,21 +91,24 @@ struct AHardwareBufferHandle {
   handle<message_pipe> tracking_pipe;
 };
 
-// gfx::GpuMemoryBufferHandle
-struct GpuMemoryBufferHandle {
+union GpuMemoryBufferPlatformHandle {
   // TODO(676224): Use preprocessor to restrict platform-specific members to
   // desired platform.
-  GpuMemoryBufferType type;
-  GpuMemoryBufferId id;
-  mojo_base.mojom.UnsafeSharedMemoryRegion? shared_memory_handle;
-  uint32 offset;
-  uint32 stride;
-  NativePixmapHandle? native_pixmap_handle;
-  handle? mach_port;
+  mojo_base.mojom.UnsafeSharedMemoryRegion shared_memory_handle;
+  NativePixmapHandle native_pixmap_handle;
+  handle mach_port;
 
   [EnableIf=is_win]
-  handle? dxgi_handle;
+  handle dxgi_handle;
 
   [EnableIf=is_android]
-  AHardwareBufferHandle? android_hardware_buffer_handle;
+  AHardwareBufferHandle android_hardware_buffer_handle;
+};
+
+// gfx::GpuMemoryBufferHandle
+struct GpuMemoryBufferHandle {
+  GpuMemoryBufferId id;
+  uint32 offset;
+  uint32 stride;
+  GpuMemoryBufferPlatformHandle? platform_handle;
 };

ui/gfx/mojo/buffer_types_struct_traits.cc

@@ -61,60 +61,43 @@ bool StructTraits<
 #endif
 }
 
-base::UnsafeSharedMemoryRegion
-StructTraits<gfx::mojom::GpuMemoryBufferHandleDataView,
-             gfx::GpuMemoryBufferHandle>::
-    shared_memory_handle(gfx::GpuMemoryBufferHandle& handle) {
-  if (handle.type != gfx::SHARED_MEMORY_BUFFER)
-    return base::UnsafeSharedMemoryRegion();
-  return std::move(handle.region);
-}
-
-const gfx::NativePixmapHandle&
-StructTraits<gfx::mojom::GpuMemoryBufferHandleDataView,
-             gfx::GpuMemoryBufferHandle>::
-    native_pixmap_handle(const gfx::GpuMemoryBufferHandle& handle) {
+gfx::mojom::GpuMemoryBufferPlatformHandlePtr StructTraits<
+    gfx::mojom::GpuMemoryBufferHandleDataView,
+    gfx::GpuMemoryBufferHandle>::platform_handle(gfx::GpuMemoryBufferHandle&
+                                                     handle) {
+  switch (handle.type) {
+    case gfx::EMPTY_BUFFER:
+      break;
+    case gfx::SHARED_MEMORY_BUFFER:
+      return gfx::mojom::GpuMemoryBufferPlatformHandle::NewSharedMemoryHandle(
+          std::move(handle.region));
+    case gfx::NATIVE_PIXMAP: {
 #if defined(OS_LINUX)
-  return handle.native_pixmap_handle;
+      return gfx::mojom::GpuMemoryBufferPlatformHandle::NewNativePixmapHandle(
+          handle.native_pixmap_handle);
 #else
       static base::NoDestructor<gfx::NativePixmapHandle> pixmap_handle;
-  return *pixmap_handle;
+      return gfx::mojom::GpuMemoryBufferPlatformHandle::NewNativePixmapHandle(
+          *pixmap_handle);
 #endif
-}
-
-mojo::ScopedHandle StructTraits<gfx::mojom::GpuMemoryBufferHandleDataView,
-                                gfx::GpuMemoryBufferHandle>::
-    mach_port(const gfx::GpuMemoryBufferHandle& handle) {
+    }
+    case gfx::IO_SURFACE_BUFFER:
 #if defined(OS_MACOSX) && !defined(OS_IOS)
-  if (handle.type != gfx::IO_SURFACE_BUFFER)
-    return mojo::ScopedHandle();
-  return mojo::WrapMachPort(handle.mach_port.get());
+      return gfx::mojom::GpuMemoryBufferPlatformHandle::NewMachPort(
+          mojo::WrapMachPort(handle.mach_port.get()));
 #else
-  return mojo::ScopedHandle();
+      break;
 #endif
-}
-
+    case gfx::DXGI_SHARED_HANDLE:
 #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());
-}
+      return gfx::mojom::GpuMemoryBufferPlatformHandle::NewDxgiHandle(
+          mojo::WrapPlatformFile(handle.dxgi_handle.GetHandle()));
+#else
+      break;
 #endif
-
+    case gfx::ANDROID_HARDWARE_BUFFER: {
 #if defined(OS_ANDROID)
-// static
-gfx::mojom::AHardwareBufferHandlePtr
-StructTraits<gfx::mojom::GpuMemoryBufferHandleDataView,
-             gfx::GpuMemoryBufferHandle>::
-    android_hardware_buffer_handle(gfx::GpuMemoryBufferHandle& handle) {
-  if (handle.type != gfx::ANDROID_HARDWARE_BUFFER)
-    return nullptr;
-
       // We must keep a ref to the AHardwareBuffer alive until the receiver has
       // acquired its own reference. We do this by sending a message pipe handle
       // along with the buffer. When the receiver deserializes (or even if they
@@ -123,77 +106,108 @@ StructTraits<gfx::mojom::GpuMemoryBufferHandleDataView,
       mojo::MessagePipe tracking_pipe;
       auto wrapped_handle = gfx::mojom::AHardwareBufferHandle::New(
           mojo::WrapPlatformFile(
-          handle.android_hardware_buffer.SerializeAsFileDescriptor().release()),
+              handle.android_hardware_buffer.SerializeAsFileDescriptor()
+                  .release()),
           std::move(tracking_pipe.handle0));
 
       // Pass ownership of the input handle to our tracking pipe to keep the AHB
       // alive until it's deserialized.
       mojo::ScopeToMessagePipe(std::move(handle.android_hardware_buffer),
                                std::move(tracking_pipe.handle1));
-  return wrapped_handle;
-}
+      return gfx::mojom::GpuMemoryBufferPlatformHandle::
+          NewAndroidHardwareBufferHandle(std::move(wrapped_handle));
+#else
+      break;
 #endif
+    }
+  }
+
+  return nullptr;
+}
 
 bool StructTraits<gfx::mojom::GpuMemoryBufferHandleDataView,
                   gfx::GpuMemoryBufferHandle>::
     Read(gfx::mojom::GpuMemoryBufferHandleDataView data,
          gfx::GpuMemoryBufferHandle* out) {
-  if (!data.ReadType(&out->type) || !data.ReadId(&out->id))
-    return false;
-
-  if (out->type == gfx::SHARED_MEMORY_BUFFER) {
-    if (!data.ReadSharedMemoryHandle(&out->region))
+  if (!data.ReadId(&out->id))
     return false;
 
   out->offset = data.offset();
   out->stride = data.stride();
+
+  gfx::mojom::GpuMemoryBufferPlatformHandlePtr platform_handle;
+  if (!data.ReadPlatformHandle(&platform_handle)) {
+    return false;
+  }
+
+  if (!platform_handle) {
+    out->type = gfx::EMPTY_BUFFER;
+    return true;
   }
+
+  switch (platform_handle->which()) {
+    case gfx::mojom::GpuMemoryBufferPlatformHandleDataView::Tag::
+        SHARED_MEMORY_HANDLE:
+      out->type = gfx::SHARED_MEMORY_BUFFER;
+      out->region = std::move(platform_handle->get_shared_memory_handle());
+      return true;
+    case gfx::mojom::GpuMemoryBufferPlatformHandleDataView::Tag::
+        NATIVE_PIXMAP_HANDLE:
 #if defined(OS_LINUX)
-  if (out->type == gfx::NATIVE_PIXMAP &&
-      !data.ReadNativePixmapHandle(&out->native_pixmap_handle))
+      out->type = gfx::NATIVE_PIXMAP;
+      out->native_pixmap_handle = platform_handle->get_native_pixmap_handle();
+      return true;
+#else
       return false;
 #endif
+    case gfx::mojom::GpuMemoryBufferPlatformHandleDataView::Tag::MACH_PORT: {
 #if defined(OS_MACOSX) && !defined(OS_IOS)
-  if (out->type == gfx::IO_SURFACE_BUFFER) {
+      out->type = gfx::IO_SURFACE_BUFFER;
       mach_port_t mach_port;
-    MojoResult unwrap_result =
-        mojo::UnwrapMachPort(data.TakeMachPort(), &mach_port);
+      MojoResult unwrap_result = mojo::UnwrapMachPort(
+          std::move(platform_handle->get_mach_port()), &mach_port);
       if (unwrap_result != MOJO_RESULT_OK)
         return false;
       out->mach_port.reset(mach_port);
-  }
+      return true;
+#else
+      return false;
 #endif
+    }
 #if defined(OS_WIN)
-  if (out->type == gfx::DXGI_SHARED_HANDLE) {
+    case gfx::mojom::GpuMemoryBufferPlatformHandleDataView::Tag::DXGI_HANDLE: {
+      out->type = gfx::DXGI_SHARED_HANDLE;
       HANDLE handle;
-    MojoResult unwrap_result =
-        mojo::UnwrapPlatformFile(data.TakeDxgiHandle(), &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->offset = data.offset();
-    out->stride = data.stride();
+      return true;
     }
 #endif
 #if defined(OS_ANDROID)
-  if (out->type == gfx::ANDROID_HARDWARE_BUFFER) {
-    gfx::mojom::AHardwareBufferHandlePtr buffer_handle;
-    if (!data.ReadAndroidHardwareBufferHandle(&buffer_handle) || !buffer_handle)
+    case gfx::mojom::GpuMemoryBufferPlatformHandleDataView::Tag::
+        ANDROID_HARDWARE_BUFFER_HANDLE: {
+      out->type = gfx::ANDROID_HARDWARE_BUFFER;
+      gfx::mojom::AHardwareBufferHandlePtr buffer_handle =
+          std::move(platform_handle->get_android_hardware_buffer_handle());
+      if (!buffer_handle)
         return false;
 
       base::PlatformFile fd;
-    MojoResult unwrap_result =
-        mojo::UnwrapPlatformFile(std::move(buffer_handle->buffer_handle), &fd);
+      MojoResult unwrap_result = mojo::UnwrapPlatformFile(
+          std::move(buffer_handle->buffer_handle), &fd);
       base::ScopedFD scoped_fd(fd);
       if (unwrap_result != MOJO_RESULT_OK || !scoped_fd.is_valid())
         return false;
       out->android_hardware_buffer = base::android::ScopedHardwareBufferHandle::
           DeserializeFromFileDescriptor(std::move(scoped_fd));
-    out->offset = data.offset();
-    out->stride = data.stride();
     }
 #endif
-  return true;
+  }
+
+  return false;
 }
 
 }  // namespace mojo

ui/gfx/mojo/buffer_types_struct_traits.h

@@ -176,54 +176,6 @@ struct StructTraits<gfx::mojom::BufferUsageAndFormatDataView,
                    gfx::BufferUsageAndFormat* out);
 };
 
-template <>
-struct EnumTraits<gfx::mojom::GpuMemoryBufferType, gfx::GpuMemoryBufferType> {
-  static gfx::mojom::GpuMemoryBufferType ToMojom(
-      gfx::GpuMemoryBufferType type) {
-    switch (type) {
-      case gfx::GpuMemoryBufferType::EMPTY_BUFFER:
-        return gfx::mojom::GpuMemoryBufferType::EMPTY_BUFFER;
-      case gfx::GpuMemoryBufferType::SHARED_MEMORY_BUFFER:
-        return gfx::mojom::GpuMemoryBufferType::SHARED_MEMORY_BUFFER;
-      case gfx::GpuMemoryBufferType::IO_SURFACE_BUFFER:
-        return gfx::mojom::GpuMemoryBufferType::IO_SURFACE_BUFFER;
-      case gfx::GpuMemoryBufferType::NATIVE_PIXMAP:
-        return gfx::mojom::GpuMemoryBufferType::NATIVE_PIXMAP;
-      case gfx::GpuMemoryBufferType::DXGI_SHARED_HANDLE:
-        return gfx::mojom::GpuMemoryBufferType::DXGI_SHARED_HANDLE;
-      case gfx::GpuMemoryBufferType::ANDROID_HARDWARE_BUFFER:
-        return gfx::mojom::GpuMemoryBufferType::ANDROID_HARDWARE_BUFFER;
-    }
-    NOTREACHED();
-    return gfx::mojom::GpuMemoryBufferType::EMPTY_BUFFER;
-  }
-
-  static bool FromMojom(gfx::mojom::GpuMemoryBufferType input,
-                        gfx::GpuMemoryBufferType* out) {
-    switch (input) {
-      case gfx::mojom::GpuMemoryBufferType::EMPTY_BUFFER:
-        *out = gfx::GpuMemoryBufferType::EMPTY_BUFFER;
-        return true;
-      case gfx::mojom::GpuMemoryBufferType::SHARED_MEMORY_BUFFER:
-        *out = gfx::GpuMemoryBufferType::SHARED_MEMORY_BUFFER;
-        return true;
-      case gfx::mojom::GpuMemoryBufferType::IO_SURFACE_BUFFER:
-        *out = gfx::GpuMemoryBufferType::IO_SURFACE_BUFFER;
-        return true;
-      case gfx::mojom::GpuMemoryBufferType::NATIVE_PIXMAP:
-        *out = gfx::GpuMemoryBufferType::NATIVE_PIXMAP;
-        return true;
-      case gfx::mojom::GpuMemoryBufferType::DXGI_SHARED_HANDLE:
-        *out = gfx::GpuMemoryBufferType::DXGI_SHARED_HANDLE;
-        return true;
-      case gfx::mojom::GpuMemoryBufferType::ANDROID_HARDWARE_BUFFER:
-        *out = gfx::GpuMemoryBufferType::ANDROID_HARDWARE_BUFFER;
-        return true;
-    }
-    return false;
-  }
-};
-
 template <>
 struct StructTraits<gfx::mojom::GpuMemoryBufferIdDataView,
                     gfx::GpuMemoryBufferId> {
@@ -288,34 +240,17 @@ struct StructTraits<gfx::mojom::NativePixmapHandleDataView,
 template <>
 struct StructTraits<gfx::mojom::GpuMemoryBufferHandleDataView,
                     gfx::GpuMemoryBufferHandle> {
-  static gfx::GpuMemoryBufferType type(
-      const gfx::GpuMemoryBufferHandle& handle) {
-    return handle.type;
-  }
   static gfx::GpuMemoryBufferId id(const gfx::GpuMemoryBufferHandle& handle) {
     return handle.id;
   }
-  static base::UnsafeSharedMemoryRegion shared_memory_handle(
-      gfx::GpuMemoryBufferHandle& handle);
   static uint32_t offset(const gfx::GpuMemoryBufferHandle& handle) {
     return handle.offset;
   }
   static uint32_t stride(const gfx::GpuMemoryBufferHandle& handle) {
     return handle.stride;
   }
-  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
-
-#if defined(OS_ANDROID)
-  static gfx::mojom::AHardwareBufferHandlePtr android_hardware_buffer_handle(
+  static gfx::mojom::GpuMemoryBufferPlatformHandlePtr platform_handle(
       gfx::GpuMemoryBufferHandle& handle);
-#endif
 
   static bool Read(gfx::mojom::GpuMemoryBufferHandleDataView data,
                    gfx::GpuMemoryBufferHandle* handle);