gpu: Remove Create/DeleteImage IPC by adding an X11_PIXMAP_BUFFER GpuMemoryBuffer type.

gpu/command_buffer/service/in_process_command_buffer.cc

Index: gpu/command_buffer/service/in_process_command_buffer.cc
diff --git a/gpu/command_buffer/service/in_process_command_buffer.cc b/gpu/command_buffer/service/in_process_command_buffer.cc
index 79cb6c0dd92fcbaa8db3a5240cc2e6e1fa4d1d3b..4deed2ed8a9cf54135d4cfd579f2a8408b6bbc41 100644
--- a/gpu/command_buffer/service/in_process_command_buffer.cc
+++ b/gpu/command_buffer/service/in_process_command_buffer.cc
@@ -24,7 +24,6 @@
 #include "base/sequence_checker.h"
 #include "base/synchronization/condition_variable.h"
 #include "base/threading/thread.h"
-#include "gpu/command_buffer/client/gpu_memory_buffer_factory.h"
 #include "gpu/command_buffer/service/command_buffer_service.h"
 #include "gpu/command_buffer/service/context_group.h"
 #include "gpu/command_buffer/service/gl_context_virtual.h"
@@ -47,8 +46,6 @@ namespace gpu {
 
 namespace {
 
-static GpuMemoryBufferFactory* g_gpu_memory_buffer_factory = NULL;
-
 template <typename T>
 static void RunTaskWithResult(base::Callback<T(void)> task,
                               T* result,
@@ -297,11 +294,8 @@ bool InProcessCommandBuffer::Initialize(
       base::Bind(&RunTaskWithResult<bool>, init_task, &result, &completion));
   completion.Wait();
 
-  if (result) {
+  if (result)
     capabilities_ = capabilities;
-    capabilities_.map_image =
-        capabilities_.map_image && g_gpu_memory_buffer_factory;
-  }
   return result;
 }
 
@@ -343,7 +337,6 @@ bool InProcessCommandBuffer::InitializeOnGpuThread(
           ? params.context_group->decoder_->GetContextGroup()
           : new gles2::ContextGroup(NULL,
                                     NULL,
-                                    NULL,
                                     service_->shader_translator_cache(),
                                     NULL,
                                     bind_generates_resource)));
@@ -415,9 +408,7 @@ bool InProcessCommandBuffer::InitializeOnGpuThread(
   }
   *params.capabilities = decoder_->GetCapabilities();
 
-  gpu_control_.reset(
-      new GpuControlService(decoder_->GetContextGroup()->image_manager(),
-                            decoder_->GetQueryManager()));
+  gpu_control_.reset(new GpuControlService(decoder_->GetQueryManager()));
 
   if (!params.is_offscreen) {
     decoder_->SetResizeCallback(base::Bind(
@@ -587,14 +578,14 @@ scoped_refptr<Buffer> InProcessCommandBuffer::CreateTransferBuffer(size_t size,
 void InProcessCommandBuffer::DestroyTransferBuffer(int32 id) {
   CheckSequencedThread();
   base::Closure task =
-      base::Bind(&InProcessCommandBuffer::DestroyTransferBufferOnGputhread,
+      base::Bind(&InProcessCommandBuffer::DestroyTransferBufferOnGpuThread,
                  base::Unretained(this),
                  id);
 
   QueueTask(task);
 }
 
-void InProcessCommandBuffer::DestroyTransferBufferOnGputhread(int32 id) {
+void InProcessCommandBuffer::DestroyTransferBufferOnGpuThread(int32 id) {
   base::AutoLock lock(command_buffer_lock_);
   command_buffer_->DestroyTransferBuffer(id);
 }
@@ -612,22 +603,23 @@ gfx::GpuMemoryBuffer* InProcessCommandBuffer::CreateGpuMemoryBuffer(
   CheckSequencedThread();
 
   *id = -1;
-  linked_ptr<gfx::GpuMemoryBuffer> buffer =
-      make_linked_ptr(g_gpu_memory_buffer_factory->CreateGpuMemoryBuffer(
-          width, height, internalformat, usage));
+
+  // TODO: Use content::GpuMemoryBufferImpl::Create() to create this instance.

[no sievers, 2014/07/01 22:49:07]

And for this we have to somehow unify the code in
android_webview/browser/gpu_memory_buffer_factory_impl.cc with
content/common/gpu/gpu_memory_buffer_factory_android.cc.

The main reason for this being under android_webview/ is probably
g_gl_draw_functions. We can change it to whatever works with this patch. For
example, we could override the whole factory (or just the one creator function -
everything else could be wrapped in callbacks) at runtime. 

[reveman, 2014/07/02 14:47:54]

I'm not too worried about unifying them, I'll figure out a clean way to do that.

The problem is that the GpuMemoryBufferImpl code lives in content/ while
InProcessCommandBuffer (unlike CommandBufferProxy) code lives in gpu/. I'm not
sure how to best bridge that. A few ideas:

- Move InProcessCommandBuffer to content/

- Implement GpuControl in content/ instead of by InProcessCommandBuffer

- Add a Create/DestroyGpuMemoryBuffer function to GpuControlService and allow
GpuControlService to be implemented in content/

- Move all GpuMemoryBufferImpl and GpuMemoryBufferFactory code to gpu/ instead
of content/ (I'm not a fan of this approach as the code needs to do client ID
validation for secure multi-process usage which I think is content/ specific)

[no sievers, 2014/07/03 00:20:42]

I like the latter two ideas better. Having things in content/ limits how things
can be used, incl. in testing. For example, there might also be some value in
having GpuMemoryBuffer live inside gpu/ so it can be tested, let's say, with
gl_tests. Do you think we can maybe just move the check for cross-process
validation to some other place/call-site in content/?

+  linked_ptr<gfx::GpuMemoryBuffer> buffer;
   if (!buffer.get())
     return NULL;
 
   static int32 next_id = 1;
   *id = next_id++;
 
-  base::Closure task = base::Bind(&GpuControlService::RegisterGpuMemoryBuffer,
-                                  base::Unretained(gpu_control_.get()),
-                                  *id,
-                                  buffer->GetHandle(),
-                                  width,
-                                  height,
-                                  internalformat);
+  base::Closure task =
+      base::Bind(&InProcessCommandBuffer::RegisterGpuMemoryBufferOnGpuThread,
+                 base::Unretained(this),
+                 *id,
+                 buffer->GetHandle(),
+                 width,
+                 height,
+                 internalformat);
 
   QueueTask(task);
 
@@ -635,18 +627,51 @@ gfx::GpuMemoryBuffer* InProcessCommandBuffer::CreateGpuMemoryBuffer(
   return buffer.get();
 }
 
+void InProcessCommandBuffer::RegisterGpuMemoryBufferOnGpuThread(
+    int32 id,
+    const gfx::GpuMemoryBufferHandle& handle,
+    size_t width,
+    size_t height,
+    unsigned internalformat) {
+  // TODO: Use content::GpuMemoryBufferFactory::CreateImageForGpuMemoryBuffer()

[no sievers, 2014/07/01 22:49:07]

So this is not a problem or is it? Looks like the factory already supports
ANDROID_NATIVE_BUFFER, and we'd just have to create a factory instance here.

[reveman, 2014/07/02 14:47:54]

Same problem here. GpuMemoryBufferFactory lives in content/ for what I think is
a good reason.

+  // to create this instance.
+  scoped_refptr<gfx::GLImage> image;
+  if (!image)
+    return;
+
+  // For Android specific workaround.
+  gles2::ContextGroup* context_group = decoder_->GetContextGroup();
+  if (context_group->feature_info()->workarounds().release_image_after_use)
+    image->SetReleaseAfterUse();
+
+  if (decoder_) {
+    gpu::gles2::ImageManager* image_manager = decoder_->GetImageManager();
+    if (image_manager)
+      image_manager->AddImage(image, id);
+  }
+}
+
 void InProcessCommandBuffer::DestroyGpuMemoryBuffer(int32 id) {
   CheckSequencedThread();
   GpuMemoryBufferMap::iterator it = gpu_memory_buffers_.find(id);
   if (it != gpu_memory_buffers_.end())
     gpu_memory_buffers_.erase(it);
-  base::Closure task = base::Bind(&GpuControlService::UnregisterGpuMemoryBuffer,
-                                  base::Unretained(gpu_control_.get()),
-                                  id);
+  base::Closure task =
+      base::Bind(&InProcessCommandBuffer::UnregisterGpuMemoryBufferOnGpuThread,
+                 base::Unretained(this),
+                 id);
 
   QueueTask(task);
 }
 
+void InProcessCommandBuffer::UnregisterGpuMemoryBufferOnGpuThread(int32 id) {
+  if (decoder_) {
+    gpu::gles2::ImageManager* image_manager = decoder_->GetImageManager();
+    if (image_manager)
+      image_manager->RemoveImage(id);
+  }
+}
+
 uint32 InProcessCommandBuffer::InsertSyncPoint() {
   uint32 sync_point = g_sync_point_manager.Get().GenerateSyncPoint();
   QueueTask(base::Bind(&InProcessCommandBuffer::RetireSyncPointOnGpuThread,
@@ -773,10 +798,4 @@ InProcessCommandBuffer::GetSurfaceTexture(uint32 stream_id) {
 }
 #endif
 
-// static
-void InProcessCommandBuffer::SetGpuMemoryBufferFactory(
-    GpuMemoryBufferFactory* factory) {
-  g_gpu_memory_buffer_factory = factory;
-}
-
 }  // namespace gpu