Add multi-process GpuMemoryBuffer framework.

content/common/gpu/client/command_buffer_proxy_impl.cc

 // Copyright (c) 2012 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 "content/common/gpu/client/command_buffer_proxy_impl.h"
 
 #include "base/callback.h"
 #include "base/debug/trace_event.h"
 #include "base/logging.h"
 #include "base/memory/shared_memory.h"
@@ skipping 349 matching lines @@
   NOTREACHED();
 }
 
 void CommandBufferProxyImpl::SetContextLostReason(
     gpu::error::ContextLostReason reason) {
   // Not implemented in proxy.
   NOTREACHED();
 }
 
 bool CommandBufferProxyImpl::SupportsGpuMemoryBuffer() {
-  return false;
+  return true;

[piman, 2013/10/21 21:57:00]

Won't this force impl-side painting to go through this path, which removes all
scheduling of texture uploads? Isn't that a bad thing?

[reveman, 2013/10/22 16:26:11]

It's disabled by default. You need to use --enable-map-image for this to be
used.

 }
 
 gfx::GpuMemoryBuffer* CommandBufferProxyImpl::CreateGpuMemoryBuffer(
     size_t width,
     size_t height,
     unsigned internalformat,
     int32* id) {
-  NOTREACHED();
-  return NULL;
+  *id = -1;
+
+  if (last_state_.error != gpu::error::kNoError)
+    return NULL;
+
+  int32 new_id = channel_->ReserveGpuMemoryBufferId();
+  DCHECK(gpu_memory_buffers_.find(new_id) == gpu_memory_buffers_.end());
+
+  scoped_ptr<gfx::GpuMemoryBuffer> gpu_memory_buffer(
+      channel_->factory()->AllocateGpuMemoryBuffer(width,
+                                                   height,
+                                                   internalformat));
+  if (!gpu_memory_buffer)
+    return NULL;
+

[piman, 2013/10/21 21:57:00]

nit: can we DCHECK the handle type here?

[reveman, 2013/10/22 16:26:11]

Hm, I'd prefer to keep this code type agnostic. Can you explain a bit more why
you think this is necessary? ShareGpuMemoryBufferToGpuProcess already has a
NOTREACHED() for invalid types.

+  // This handle is owned by the GPU process and must be passed to it or it
+  // will leak. In otherwords, do not early out on error between here and the
+  // sending of the RegisterGpuMemoryBuffer IPC below.
+  gfx::GpuMemoryBufferHandle handle =
+      channel_->ShareGpuMemoryBufferToGpuProcess(
+          gpu_memory_buffer->GetHandle());
+
+  if (!Send(new GpuCommandBufferMsg_RegisterGpuMemoryBuffer(
+                route_id_,
+                new_id,
+                handle,
+                width,
+                height,
+                internalformat))) {
+    return NULL;
+  }
+
+  *id = new_id;
+  gpu_memory_buffers_[new_id] = gpu_memory_buffer.get();
+  return gpu_memory_buffer.release();

[piman, 2013/10/21 21:57:00]

nit: these 2 lines would be a bit clearer if written something like:

gpu_memory_buffers_[new_id] = gpu_memory_buffer.release();
return gpu_memory_buffers_[new_id];

You're transferring ownership to the gpu_memory_buffers_ map. As you have it
written, it looks as if you're transferring ownership to the caller.

Have you considered using linked_ptr instead, that can be put it in a map?

[reveman, 2013/10/22 16:26:11]

switched to this in latest patch.
I think that's a good idea but for consistency we should also change the
TransferBufferMap at the same time and as this will touch a few files I prefer
to do it as a follow up.

 }
 
 void CommandBufferProxyImpl::DestroyGpuMemoryBuffer(int32 id) {
-  NOTREACHED();
+  if (last_state_.error != gpu::error::kNoError)
+    return;
+
+  // Remove the gpu memory buffer from the client side cache.
+  GpuMemoryBufferMap::iterator it = gpu_memory_buffers_.find(id);
+  if (it != gpu_memory_buffers_.end()) {
+    delete it->second;
+    gpu_memory_buffers_.erase(it);
+  }
+
+  Send(new GpuCommandBufferMsg_DestroyGpuMemoryBuffer(route_id_, id));
 }
 
 int CommandBufferProxyImpl::GetRouteID() const {
   return route_id_;
 }
 
 bool CommandBufferProxyImpl::Echo(const base::Closure& callback) {
   if (last_state_.error != gpu::error::kNoError) {
     return false;
   }
@@ skipping 164 matching lines @@
 void CommandBufferProxyImpl::SendManagedMemoryStats(
     const GpuManagedMemoryStats& stats) {
   if (last_state_.error != gpu::error::kNoError)
     return;
 
   Send(new GpuCommandBufferMsg_SendClientManagedMemoryStats(route_id_,
                                                             stats));
 }
 
 }  // namespace content