ozone: Introduce ClientPixmap and ClientPixmapFactory for non-GPU processes.

ui/ozone/public/native_pixmap_client.cc

+// 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.
+
+#include "base/command_line.h"
+#include "base/lazy_instance.h"
+#include "base/logging.h"
+#include "base/trace_event/trace_event.h"
+#include "ui/ozone/platform_object.h"
+#include "ui/ozone/platform_selection.h"
+#include "ui/ozone/public/native_pixmap_client.h"
+#include "ui/ozone/public/ozone_platform.h"
+#include "ui/ozone/public/ozone_switches.h"
+
+namespace ui {
+
+namespace {
+
+NativePixmapClient* g_instance = nullptr;

[spang, 2015/06/05 19:21:54]

I would prefer if this object was managed inside content/common/gpu/...

It doesn't seem like it needs to be shared with anything else.

[reveman, 2015/06/05 20:26:07]

I think IOSurfaceManager and SurfaceTextureManager interfaces are good examples
to follow. Those are abstract classes that live in content/common/

[dshwang, 2015/06/08 11:15:14]

IMO it should stay here. NativePixmapManager needs to be implemented for various
platforms. e.g. gbm
I think we don't want to let content/ know which platform is used. WDYT?

[spang, 2015/06/08 17:47:57]

It's true, we can't move NativePixmapManager implementation.

What I was hoping to move was the instance from the above line (g_instance).
Ideally content would manage its instance(s). We'd expose
NativePixmapManager::Create() rather than NativePixmapManager::GetInstance().

Having it here enforces one instance per address space, but that's not necessary
or even desirable. It may lead to bugs; options like --single-process break when
we do this because then both browser and renderer try to initialize the shared
instance (say, with different copies of the vgem fd).

[dshwang, 2015/06/09 18:37:40]

I understand your concern. However, it's quite complicated problem. I need more
feedback.
If there are a browser process and 3 renderer, You want to create 4
NativePixmapManager instances, even if --single-process is used.
To do it, some unique instance per renderer and browser must hold a
NativePixmapManager instance. e.g. RenderThreadImpl owns a NativePixmapManager
member.
There are two problems
1. NativePixmapManager is ozone specific. RenderThreadImpl will have ozone
specific method with ifdef
2. GpuMemoryBufferImplOzoneNativeBuffer can be used for both browser and
renderer. It would be messy to get right NativePixmapManager instance.

Both are addressable, but code would be ugly. Following code is needed.

--- a/content/child/child_gpu_memory_buffer_manager.cc
+++ b/content/child/child_gpu_memory_buffer_manager.cc
@@ -47,11 +47,18 @@ ChildGpuMemoryBufferManager::AllocateGpuMemoryBuffer(
   if (!success)
     return scoped_ptr<gfx::GpuMemoryBuffer>();
 
+#if defined(USE_OZONE)
+  scoped_ptr<GpuMemoryBufferImpl> buffer(
+      GpuMemoryBufferImplOzoneNativeBuffer::CreateFromHandle(
+          handle, size, format, callback,
+          RenderThreadImpl::current()->GetNativePixmapManager()));
+#else
   scoped_ptr<GpuMemoryBufferImpl> buffer(GpuMemoryBufferImpl::CreateFromHandle(
       handle,
       size,
       format,
       base::Bind(&DeletedGpuMemoryBuffer, sender_, handle.id)));
+#endif
   if (!buffer) {


On the other hands, as considering vgem requirement, it is ok for both browser
and renderer to share singleton instance.
NativePixmapManager initialization will work like as follows:
NativePixmapManager* NativePixmapManager::GetInstance() {
  if (singleton instance exist)
    return the singleton instance;

  create the singleton instance;

  if (browser process) {
    get the vgem fd from DrmDisplayHostManager;
  } else {
    get the vgem fd via IPC;
  }
  return the singleton instance;
}

All NativePixmapManager has to initialize is to ensure vgem fd. In
--single-process mode, Renderer tries to create NativePixmapManager instance but
there is already singleton instance which Browser created. It's ok to share the
singleton instance. I'll add special handling for browser in ozone_platform.cc
in next CL, because browser process needs different initialization.
It will make code clean, but one concern is that we should be aware why
singleton is ok in this case, which seems to make you uneasy.

What do you prefer? I'll go the direction.

+
+}  // namespace
+
+NativePixmapClient::NativePixmapClient() {
+  DCHECK(!g_instance) << "There should only be a single NativePixmapClient.";
+  g_instance = this;
+}
+
+NativePixmapClient::~NativePixmapClient() {
+  DCHECK_EQ(g_instance, this);
+  g_instance = nullptr;
+}
+
+// static
+void NativePixmapClient::InitializeIfNeeded() {
+  if (g_instance)
+    return;
+
+  CreateInstance();
+  g_instance->Initialize();
+}
+
+// static
+NativePixmapClient* NativePixmapClient::GetInstance() {
+  DCHECK(g_instance) << "NativePixmapClient is not initialized";
+  return g_instance;
+}
+
+std::vector<NativePixmapClient::Configuration>
+NativePixmapClient::GetSupportedNativePixmapConfigurations() const {
+  return std::vector<Configuration>();
+}
+
+// static
+void NativePixmapClient::CreateInstance() {

[reveman, 2015/06/05 20:26:07]

why are you not using a normal singleton so that this is thread safe?

[dshwang, 2015/06/08 11:15:14]

Done. nice suggestion.

+  DCHECK(!g_instance);
+  TRACE_EVENT1("ozone", "NativePixmapClient::CreateInstance", "platform",
+               GetOzonePlatformName());
+  scoped_ptr<NativePixmapClient> platform =
+      PlatformObject<NativePixmapClient>::Create();
+
+  // TODO(spang): Currently need to leak this object.
+  NativePixmapClient* pl = platform.release();
+  DCHECK_EQ(g_instance, pl);
+}
+
+void NativePixmapClient::Initialize() {
+}
+
+}  // namespace ui