mus: Get GpuMemoryBuffer from gpu process when possible.

services/ui/common/mus_gpu_memory_buffer_manager.cc

Index: services/ui/common/mus_gpu_memory_buffer_manager.cc
diff --git a/services/ui/common/mus_gpu_memory_buffer_manager.cc b/services/ui/common/mus_gpu_memory_buffer_manager.cc
index eda17128a87cb968ce167c3838b2a190645d5b17..439780e58fce64de9e08d4760ff278707b1e9502 100644
--- a/services/ui/common/mus_gpu_memory_buffer_manager.cc
+++ b/services/ui/common/mus_gpu_memory_buffer_manager.cc
@@ -19,34 +19,49 @@ MusGpuMemoryBufferManager::MusGpuMemoryBufferManager(
 
 MusGpuMemoryBufferManager::~MusGpuMemoryBufferManager() {}
 
-std::unique_ptr<gfx::GpuMemoryBuffer>
-MusGpuMemoryBufferManager::CreateGpuMemoryBuffer(
+gfx::GpuMemoryBufferHandle
+MusGpuMemoryBufferManager::CreateGpuMemoryBufferHandle(
+    gfx::GpuMemoryBufferId id,
+    int client_id,
     const gfx::Size& size,
     gfx::BufferFormat format,
     gfx::BufferUsage usage,
     gpu::SurfaceHandle surface_handle) {
   DCHECK(CalledOnValidThread());
-  gfx::GpuMemoryBufferId id(next_gpu_memory_id_++);
+  if (gpu::GetNativeGpuMemoryBufferType() == gfx::EMPTY_BUFFER)
+    return gfx::GpuMemoryBufferHandle();
   const bool is_native =
       gpu::IsNativeGpuMemoryBufferConfigurationSupported(format, usage);
   if (is_native) {
     gfx::GpuMemoryBufferHandle handle;
-    gpu_service_->CreateGpuMemoryBuffer(id, size, format, usage, client_id_,
+    gpu_service_->CreateGpuMemoryBuffer(id, size, format, usage, client_id,
                                         surface_handle, &handle);
-    if (handle.is_null())
-      return nullptr;
-    return gpu::GpuMemoryBufferImpl::CreateFromHandle(
-        handle, size, format, usage,
-        base::Bind(&MusGpuMemoryBufferManager::DestroyGpuMemoryBuffer,
-                   weak_factory_.GetWeakPtr(), id, client_id_, is_native));
+    if (!handle.is_null())
+      native_buffers_[client_id].insert(handle.id);
+    return handle;
   }
 
   DCHECK(gpu::GpuMemoryBufferImplSharedMemory::IsUsageSupported(usage))
       << static_cast<int>(usage);
-  return gpu::GpuMemoryBufferImplSharedMemory::Create(
-      id, size, format,
+  return gpu::GpuMemoryBufferImplSharedMemory::CreateGpuMemoryBuffer(id, size,
+                                                                     format);
+}
+
+std::unique_ptr<gfx::GpuMemoryBuffer>
+MusGpuMemoryBufferManager::CreateGpuMemoryBuffer(
+    const gfx::Size& size,
+    gfx::BufferFormat format,
+    gfx::BufferUsage usage,
+    gpu::SurfaceHandle surface_handle) {
+  gfx::GpuMemoryBufferId id(next_gpu_memory_id_++);
+  gfx::GpuMemoryBufferHandle handle = CreateGpuMemoryBufferHandle(
+      id, client_id_, size, format, usage, surface_handle);
+  if (handle.is_null())
+    return nullptr;
+  return gpu::GpuMemoryBufferImpl::CreateFromHandle(
+      handle, size, format, usage,
       base::Bind(&MusGpuMemoryBufferManager::DestroyGpuMemoryBuffer,
-                 weak_factory_.GetWeakPtr(), id, client_id_, is_native));
+                 weak_factory_.GetWeakPtr(), id, client_id_));
 }
 
 std::unique_ptr<gfx::GpuMemoryBuffer>
@@ -69,12 +84,14 @@ void MusGpuMemoryBufferManager::SetDestructionSyncToken(
 void MusGpuMemoryBufferManager::DestroyGpuMemoryBuffer(
     gfx::GpuMemoryBufferId id,
     int client_id,
-    bool is_native,
     const gpu::SyncToken& sync_token) {
   DCHECK(CalledOnValidThread());
-  if (is_native) {
-    gpu_service_->DestroyGpuMemoryBuffer(id, client_id, sync_token);
-  }
+  if (!native_buffers_.count(client_id) ||

[reveman, 2016/11/30 17:32:45]

Can we avoid iterating over all buffers? Finding the first is sufficient here.

[sadrul, 2016/11/30 19:54:31]

Not totally sure what you mean here. This is checking whether there's a native
memory buffer associated with (id, client_id).

[reveman, 2016/12/02 19:12:50]

I was referring to count() being less efficient than find() and in this case
find() is sufficient and more properly match the intent of the code. Anyhow,
looks like this count() call is not needed at all in latest patch unless I'm
missing something.

+      !native_buffers_[client_id].erase(id))
+    return;
+  if (native_buffers_[client_id].empty())

[reveman, 2016/11/30 17:32:45]

Can the manager be notified when the client is gone instead of having to handle
this special case here each time we destroy a buffer? Notifying also allows to
to make sure that all native buffers are properly destroyed on the gpu service
side when a client goes away.

[sadrul, 2016/11/30 19:54:31]

Oh good point. I have added the callback to notify this manager when a client
goes away.

But, is it possible for a client to destroy some memory buffer it had previously
allocated without freeing all such memory buffers?

+    native_buffers_.erase(client_id);
+  gpu_service_->DestroyGpuMemoryBuffer(id, client_id, sync_token);
 }
 
 }  // namespace ui