gpu: Export/import buffers created by Ozone GMB factory.

gpu/ipc/client/gpu_memory_buffer_impl_ozone_native_pixmap.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "gpu/ipc/client/gpu_memory_buffer_impl_ozone_native_pixmap.h"
 
 #include <utility>
 
 #include "base/memory/ptr_util.h"
 #include "gpu/ipc/common/gpu_memory_buffer_support.h"
@@ skipping 11 matching lines @@
   // returns and reference count drops to 0.
 }
 
 }  // namespace
 
 GpuMemoryBufferImplOzoneNativePixmap::GpuMemoryBufferImplOzoneNativePixmap(
     gfx::GpuMemoryBufferId id,
     const gfx::Size& size,
     gfx::BufferFormat format,
     const DestructionCallback& callback,
-    std::unique_ptr<ui::ClientNativePixmap> pixmap)
+    std::unique_ptr<ui::ClientNativePixmap> pixmap,
+    const std::vector<std::pair<int, int>>& strides_and_offsets,
+    base::ScopedFD&& fd)

[mdempsky, 2016/06/17 19:38:19]

nit: I think just "base::ScopedFD fd" is preferable here?

[spang, 2016/06/17 20:00:25]

+1

[reveman, 2016/06/17 20:34:02]

I assume this is for consistency with existing code in chromium? Initial patch
had "base::ScopedFD fd" but dcastagna@ asked me to use a rvalue reference here
to make it more clear that it's being passed. Either way is fine with me but I
changed to "base::ScopedFD fd" in latest patch as the majority seems to prefer
that.

[spang, 2016/06/17 20:52:31]

Not just consistency, this usage is against the style guide.

"Use rvalue references only to define move constructors and move assignment
operators"

(This is a constructor but it isn't the move constructor)

Accepting a move constructed argument by value is the approved way.

     : GpuMemoryBufferImpl(id, size, format, callback),
       pixmap_(std::move(pixmap)),
+      strides_and_offsets_(strides_and_offsets),
+      fd_(std::move(fd)),
       data_(nullptr) {}
 
 GpuMemoryBufferImplOzoneNativePixmap::~GpuMemoryBufferImplOzoneNativePixmap() {}
 
 // static
 std::unique_ptr<GpuMemoryBufferImplOzoneNativePixmap>
 GpuMemoryBufferImplOzoneNativePixmap::CreateFromHandle(
     const gfx::GpuMemoryBufferHandle& handle,
     const gfx::Size& size,
     gfx::BufferFormat format,
     gfx::BufferUsage usage,
     const DestructionCallback& callback) {
+  DCHECK_EQ(handle.native_pixmap_handle.fds.size(), 1u);
+  base::ScopedFD scoped_fd(handle.native_pixmap_handle.fds[0].fd);
+
+  // GpuMemoryBufferImpl needs the FD to implement GetHandle() but
+  // ui::ClientNativePixmapFactory::ImportFromHandle is expected to take
+  // ownership of the FD passed in the handle so we have to dup it here in
+  // order to pass a valid FD to the GpuMemoryBufferImpl ctor.
+  gfx::NativePixmapHandle native_pixmap_handle;
+  native_pixmap_handle.fds.emplace_back(HANDLE_EINTR(dup(scoped_fd.get())),

[spang, 2016/06/17 20:00:25]

dup might fail with EMFILE.. I think we should be checking for this instead of
pushing -1 into the vector.

[reveman, 2016/06/17 20:34:02]

Done.

+                                        true);
+  native_pixmap_handle.strides_and_offsets =
+      handle.native_pixmap_handle.strides_and_offsets;
   std::unique_ptr<ui::ClientNativePixmap> native_pixmap =
       ui::ClientNativePixmapFactory::GetInstance()->ImportFromHandle(
-          handle.native_pixmap_handle, size, usage);
+          native_pixmap_handle, size, usage);
   DCHECK(native_pixmap);
+
   return base::WrapUnique(new GpuMemoryBufferImplOzoneNativePixmap(
-      handle.id, size, format, callback, std::move(native_pixmap)));
+      handle.id, size, format, callback, std::move(native_pixmap),
+      handle.native_pixmap_handle.strides_and_offsets, std::move(scoped_fd)));
 }
 
 // static
 bool GpuMemoryBufferImplOzoneNativePixmap::IsConfigurationSupported(
     gfx::BufferFormat format,
     gfx::BufferUsage usage) {
   return gpu::IsNativeGpuMemoryBufferConfigurationSupported(format, usage);
 }
 
 // static
@@ skipping 42 matching lines @@
   int stride;
   pixmap_->GetStride(&stride);
   return stride;
 }
 
 gfx::GpuMemoryBufferHandle GpuMemoryBufferImplOzoneNativePixmap::GetHandle()
     const {
   gfx::GpuMemoryBufferHandle handle;
   handle.type = gfx::OZONE_NATIVE_PIXMAP;
   handle.id = id_;
+  base::ScopedFD scoped_fd(HANDLE_EINTR(dup(fd_.get())));
+  if (!scoped_fd.is_valid()) {
+    PLOG(ERROR) << "dup";
+    return gfx::GpuMemoryBufferHandle();
+  }
+  handle.native_pixmap_handle.fds.emplace_back(scoped_fd.release(), true);
+  handle.native_pixmap_handle.strides_and_offsets = strides_and_offsets_;
   return handle;
 }
 
 }  // namespace gpu