gfx: Fix sending native ozone pixmaps from InProcessCommandBuffer.

The file-descriptors in NativePixmapHandle need to be dup'ed before then
can be sent over mojo IPC from InProcessCommandBuffer. Introduce a
convenience function CloneHandleForIPC() so that it can be used by both
GpuChannelHost and InProcessCommandBuffer.

BUG=643746
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2531963002
Cr-Commit-Position: refs/heads/master@{#434884}

gpu/ipc/client/command_buffer_proxy_impl.cc

@@ -462,10 +462,9 @@ int32_t CommandBufferProxyImpl::CreateImage(ClientBuffer buffer,
   // This handle is owned by the GPU process and must be passed to it or it
   // will leak. In otherwords, do not early out on error between here and the
   // sending of the CreateImage IPC below.
-  bool requires_sync_token = false;
   gfx::GpuMemoryBufferHandle handle =
-      channel_->ShareGpuMemoryBufferToGpuProcess(gpu_memory_buffer->GetHandle(),
-                                                 &requires_sync_token);
+      gfx::CloneHandleForIPC(gpu_memory_buffer->GetHandle());
+  bool requires_sync_token = handle.type == gfx::IO_SURFACE_BUFFER;
 
   uint64_t image_fence_sync = 0;
   if (requires_sync_token) {

gpu/ipc/client/gpu_channel_host.cc

@@ -220,7 +220,7 @@ void GpuChannelHost::RemoveRoute(int route_id) {
 }
 
 base::SharedMemoryHandle GpuChannelHost::ShareToGpuProcess(
-    base::SharedMemoryHandle source_handle) {
+    const base::SharedMemoryHandle& source_handle) {
   if (IsLost())
     return base::SharedMemory::NULLHandle();
 
@@ -232,52 +232,6 @@ int32_t GpuChannelHost::ReserveTransferBufferId() {
   return g_next_transfer_buffer_id.GetNext() + 1;
 }
 
-gfx::GpuMemoryBufferHandle GpuChannelHost::ShareGpuMemoryBufferToGpuProcess(
-    const gfx::GpuMemoryBufferHandle& source_handle,
-    bool* requires_sync_point) {
-  switch (source_handle.type) {
-    case gfx::SHARED_MEMORY_BUFFER: {
-      gfx::GpuMemoryBufferHandle handle;
-      handle.type = gfx::SHARED_MEMORY_BUFFER;
-      handle.handle = ShareToGpuProcess(source_handle.handle);
-      handle.offset = source_handle.offset;
-      handle.stride = source_handle.stride;
-      *requires_sync_point = false;
-      return handle;
-    }
-#if defined(USE_OZONE)
-    case gfx::OZONE_NATIVE_PIXMAP: {
-      std::vector<base::ScopedFD> scoped_fds;
-      for (auto& fd : source_handle.native_pixmap_handle.fds) {
-        base::ScopedFD scoped_fd(HANDLE_EINTR(dup(fd.fd)));
-        if (!scoped_fd.is_valid()) {
-          PLOG(ERROR) << "dup";
-          return gfx::GpuMemoryBufferHandle();
-        }
-        scoped_fds.emplace_back(std::move(scoped_fd));
-      }
-      gfx::GpuMemoryBufferHandle handle;
-      handle.type = gfx::OZONE_NATIVE_PIXMAP;
-      handle.id = source_handle.id;
-      for (auto& scoped_fd : scoped_fds) {
-        handle.native_pixmap_handle.fds.emplace_back(scoped_fd.release(),
-                                                     true /* auto_close */);
-      }
-      handle.native_pixmap_handle.planes =
-          source_handle.native_pixmap_handle.planes;
-      *requires_sync_point = false;
-      return handle;
-    }
-#endif
-    case gfx::IO_SURFACE_BUFFER:
-      *requires_sync_point = true;
-      return source_handle;
-    default:
-      NOTREACHED();
-      return gfx::GpuMemoryBufferHandle();
-  }
-}
-
 int32_t GpuChannelHost::ReserveImageId() {
   return next_image_id_.GetNext();
 }

gpu/ipc/client/gpu_channel_host.h

@@ -139,18 +139,11 @@ class GPU_EXPORT GpuChannelHost
   // GPU process. The caller is responsible for ensuring it is closed. Returns
   // an invalid handle on failure.
   base::SharedMemoryHandle ShareToGpuProcess(
-      base::SharedMemoryHandle source_handle);
+      const base::SharedMemoryHandle& source_handle);
 
   // Reserve one unused transfer buffer ID.
   int32_t ReserveTransferBufferId();
 
-  // Returns a GPU memory buffer handle to the buffer that can be sent via
-  // IPC to the GPU process. The caller is responsible for ensuring it is
-  // closed. Returns an invalid handle on failure.
-  gfx::GpuMemoryBufferHandle ShareGpuMemoryBufferToGpuProcess(
-      const gfx::GpuMemoryBufferHandle& source_handle,
-      bool* requires_sync_point);
-
   // Reserve one unused image ID.
   int32_t ReserveImageId();
 

gpu/ipc/in_process_command_buffer.cc

@@ -108,34 +108,6 @@ class ScopedEvent {
   base::WaitableEvent* event_;
 };
 
-base::SharedMemoryHandle ShareToGpuThread(
-    base::SharedMemoryHandle source_handle) {
-  return base::SharedMemory::DuplicateHandle(source_handle);
-}
-
-gfx::GpuMemoryBufferHandle ShareGpuMemoryBufferToGpuThread(
-    const gfx::GpuMemoryBufferHandle& source_handle,
-    bool* requires_sync_point) {
-  switch (source_handle.type) {
-    case gfx::SHARED_MEMORY_BUFFER: {
-      gfx::GpuMemoryBufferHandle handle;
-      handle.type = gfx::SHARED_MEMORY_BUFFER;
-      handle.handle = ShareToGpuThread(source_handle.handle);
-      handle.offset = source_handle.offset;
-      handle.stride = source_handle.stride;
-      *requires_sync_point = false;
-      return handle;
-    }
-    case gfx::IO_SURFACE_BUFFER:
-    case gfx::OZONE_NATIVE_PIXMAP:
-      *requires_sync_point = true;
-      return source_handle;
-    default:
-      NOTREACHED();
-      return gfx::GpuMemoryBufferHandle();
-  }
-}
-
 scoped_refptr<InProcessCommandBuffer::Service> GetInitialService(
     const scoped_refptr<InProcessCommandBuffer::Service>& service) {
   if (service)
@@ -745,9 +717,9 @@ int32_t InProcessCommandBuffer::CreateImage(ClientBuffer buffer,
   // This handle is owned by the GPU thread and must be passed to it or it
   // will leak. In otherwords, do not early out on error between here and the
   // queuing of the CreateImage task below.
-  bool requires_sync_point = false;
-  gfx::GpuMemoryBufferHandle handle = ShareGpuMemoryBufferToGpuThread(
-      gpu_memory_buffer->GetHandle(), &requires_sync_point);
+  gfx::GpuMemoryBufferHandle handle =
+      gfx::CloneHandleForIPC(gpu_memory_buffer->GetHandle());
+  bool requires_sync_point = handle.type == gfx::IO_SURFACE_BUFFER;
 
   uint64_t fence_sync = 0;
   if (requires_sync_point) {

ui/gfx/gpu_memory_buffer.cc

@@ -18,4 +18,34 @@ GpuMemoryBufferHandle::~GpuMemoryBufferHandle() {}
 void GpuMemoryBuffer::SetColorSpaceForScanout(
     const gfx::ColorSpace& color_space) {}
 
+GpuMemoryBufferHandle CloneHandleForIPC(
+    const GpuMemoryBufferHandle& source_handle) {
+  switch (source_handle.type) {
+    case gfx::EMPTY_BUFFER:
+      NOTREACHED();
+      return source_handle;
+    case gfx::SHARED_MEMORY_BUFFER: {
+      gfx::GpuMemoryBufferHandle handle;
+      handle.type = gfx::SHARED_MEMORY_BUFFER;
+      handle.handle = base::SharedMemory::DuplicateHandle(source_handle.handle);
+      handle.offset = source_handle.offset;
+      handle.stride = source_handle.stride;
+      return handle;
+    }
+    case gfx::OZONE_NATIVE_PIXMAP: {
+      gfx::GpuMemoryBufferHandle handle;
+      handle.type = gfx::OZONE_NATIVE_PIXMAP;
+      handle.id = source_handle.id;
+#if defined(USE_OZONE)
+      handle.native_pixmap_handle =
+          gfx::CloneHandleForIPC(source_handle.native_pixmap_handle);
+#endif
+      return handle;
+    }
+    case gfx::IO_SURFACE_BUFFER:
+      return source_handle;
+  }
+  return gfx::GpuMemoryBufferHandle();
+}
+
 }  // namespace gfx

ui/gfx/gpu_memory_buffer.h

@@ -99,6 +99,12 @@ class GFX_EXPORT GpuMemoryBuffer {
   virtual ClientBuffer AsClientBuffer() = 0;
 };
 
+// Returns an instance of |handle| which can be sent over IPC. This duplicates
+// the file-handles as appropriate, so that the IPC code take ownership of them,
+// without invalidating |handle| itself.
+GFX_EXPORT GpuMemoryBufferHandle
+CloneHandleForIPC(const GpuMemoryBufferHandle& handle);
+
 }  // namespace gfx
 
 #endif  // UI_GFX_GPU_MEMORY_BUFFER_H_

ui/gfx/native_pixmap_handle.cc

@@ -2,10 +2,12 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include <stdint.h>
-
 #include "ui/gfx/native_pixmap_handle.h"
 
+#if defined(USE_OZONE)
+#include "base/posix/eintr_wrapper.h"
+#endif
+
 namespace gfx {
 
 NativePixmapPlane::NativePixmapPlane()
@@ -27,4 +29,23 @@ NativePixmapHandle::NativePixmapHandle(const NativePixmapHandle& other) =
 
 NativePixmapHandle::~NativePixmapHandle() {}
 
+#if defined(USE_OZONE)
+NativePixmapHandle CloneHandleForIPC(const NativePixmapHandle& handle) {
+  NativePixmapHandle clone;
+  std::vector<base::ScopedFD> scoped_fds;
+  for (auto& fd : handle.fds) {
+    base::ScopedFD scoped_fd(HANDLE_EINTR(dup(fd.fd)));
+    if (!scoped_fd.is_valid()) {
+      PLOG(ERROR) << "dup";
+      return NativePixmapHandle();
+    }
+    scoped_fds.emplace_back(std::move(scoped_fd));
+  }
+  for (auto& scoped_fd : scoped_fds)
+    clone.fds.emplace_back(scoped_fd.release(), true /* auto_close */);
+  clone.planes = handle.planes;
+  return clone;
+}
+#endif  // defined(USE_OZONE)
+
 }  // namespace gfx

ui/gfx/native_pixmap_handle.h

@@ -6,6 +6,8 @@
 #define UI_GFX_NATIVE_PIXMAP_HANDLE_H_
 
 #include <stddef.h>
+#include <stdint.h>
+
 #include <vector>
 
 #include "ui/gfx/gfx_export.h"
@@ -50,6 +52,13 @@ struct GFX_EXPORT NativePixmapHandle {
   std::vector<NativePixmapPlane> planes;
 };
 
+#if defined(USE_OZONE)
+// Returns an instance of |handle| which can be sent over IPC. This duplicates
+// the file-handles, so that the IPC code take ownership of them, without
+// invalidating |handle|.
+NativePixmapHandle CloneHandleForIPC(const NativePixmapHandle& handle);
+#endif
+
 }  // namespace gfx
 
 #endif  // UI_GFX_NATIVE_PIXMAP_HANDLE_H_