ozone: unify GpuMemoryBufferFactoryOzoneNativePixmap in content/

ui/gl/gl_image_ozone_native_pixmap.h

+// Copyright 2015 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.
+
+#ifndef UI_GL_GL_IMAGE_OZONE_NATIVE_PIXMAP_H_
+#define UI_GL_GL_IMAGE_OZONE_NATIVE_PIXMAP_H_
+
+#include "ui/gfx/gpu_memory_buffer.h"
+#include "ui/gl/gl_image_linux_dma_buffer.h"
+#include "ui/ozone/public/native_pixmap.h"
+
+namespace gfx {
+
+class GL_EXPORT GLImageOzoneNativePixmap : public GLImageLinuxDMABuffer {

[reveman, 2015/07/28 19:11:17]

It's confusing to inherit from GLImageLinuxDMABuffer when this is not always
backed by a linux DMA buf. Can you inherit from GLImageEGL instead and move what
you need from GLImageLinuxDMABuffer to the GLImageOzoneNativePixmap class. I
think you can remove the GLImageLinuxDMABuffer class as it's not used by anyone
else.

[dshwang, 2015/07/29 13:23:09]

Good point! This class inherits GLImageEGL and remove GLImageLinuxDMABuffer

+ public:
+  GLImageOzoneNativePixmap(const Size& size, unsigned internalformat);
+  bool Initialize(scoped_refptr<ui::NativePixmap> pixmap,

[reveman, 2015/07/28 19:11:17]

nit: blank line after ctor and ui::NativePixmap* instead of scoped_refptr to
avoid extra refcount churn

[dshwang, 2015/07/29 13:23:09]

blank line after ctor: Done.
ui::NativePixmap* instead of scoped_refptr: I think scoped_refptr is better
because scoped_refptr has intention that this class keeps the pixmap reference
like how this class work. This method is not hot-spot, so I take readability
rather than optimization.

[reveman, 2015/07/29 14:16:41]

We don't use scoped_refptr for that purpose in chromium. Please change to
ui::NativePixmap* as that's the most common in content/ and consistently used
within the GLImage framework. The alternative to avoid refcount churn is "const
scoped_refptr<ui::NativePixmap>& pixmap" but raw pointer is preferred here.

[dshwang, 2015/07/29 15:15:04]

I see. Done.

+                  GpuMemoryBuffer::Format format);
+
+  void Destroy(bool have_context) override;

[reveman, 2015/07/28 19:11:17]

Above this line, add:
// Overridden from GLImage:

[dshwang, 2015/07/29 13:23:10]

Done.

+  bool ScheduleOverlayPlane(AcceleratedWidget widget,
+                            int z_order,
+                            OverlayTransform transform,
+                            const Rect& bounds_rect,
+                            const RectF& crop_rect) override;
+
+ protected:
+  ~GLImageOzoneNativePixmap() override;
+
+ private:
+  scoped_refptr<ui::NativePixmap> pixmap_;
+};
+
+}  // namespace gfx
+
+#endif  // UI_GL_GL_IMAGE_OZONE_NATIVE_PIXMAP_H_