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

gpu/ipc/client/gpu_memory_buffer_impl_ozone_native_pixmap.cc

Index: gpu/ipc/client/gpu_memory_buffer_impl_ozone_native_pixmap.cc
diff --git a/gpu/ipc/client/gpu_memory_buffer_impl_ozone_native_pixmap.cc b/gpu/ipc/client/gpu_memory_buffer_impl_ozone_native_pixmap.cc
index c676e39d0d5b160c0e7a505da22aafe8d85a8702..f1185ed4b0cba6ad17a5d6438477af7a66278b69 100644
--- a/gpu/ipc/client/gpu_memory_buffer_impl_ozone_native_pixmap.cc
+++ b/gpu/ipc/client/gpu_memory_buffer_impl_ozone_native_pixmap.cc
@@ -29,9 +29,13 @@ GpuMemoryBufferImplOzoneNativePixmap::GpuMemoryBufferImplOzoneNativePixmap(
     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() {}
@@ -44,12 +48,26 @@ GpuMemoryBufferImplOzoneNativePixmap::CreateFromHandle(
     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
@@ -112,6 +130,13 @@ gfx::GpuMemoryBufferHandle GpuMemoryBufferImplOzoneNativePixmap::GetHandle()
   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;
 }