Cleanup GpuMemoryBuffer

content/common/gpu/client/command_buffer_proxy_impl.cc

Index: content/common/gpu/client/command_buffer_proxy_impl.cc
diff --git a/content/common/gpu/client/command_buffer_proxy_impl.cc b/content/common/gpu/client/command_buffer_proxy_impl.cc
index f80521dd4d17799cfe7fb49e797f43fe51d2ef7a..e314a627ff95ee9414d618c26d47e811da09a605 100644
--- a/content/common/gpu/client/command_buffer_proxy_impl.cc
+++ b/content/common/gpu/client/command_buffer_proxy_impl.cc
@@ -169,10 +169,14 @@ void CommandBufferProxyImpl::OnSetMemoryAllocation(
 }
 
 void CommandBufferProxyImpl::OnSignalSyncPointAck(uint32 id) {
-  SignalTaskMap::iterator it = signal_tasks_.find(id);
-  DCHECK(it != signal_tasks_.end());
-  base::Closure callback = it->second;
-  signal_tasks_.erase(it);
+  base::Closure callback;
+  {
+    base::AutoLock lock(signal_lock_);

[reveman, 2014/10/22 22:42:47]

Alternatively, we can make sure to only access signal_tasks_ on the thread that
is set up to process messages for this command buffer. Looks like using
MessageLoopProxy::current() at the time the ctor is called would yield the
correct result.

We should probably avoid adding a lock where a lock is currently not needed.

+    SignalTaskMap::iterator it = signal_tasks_.find(id);
+    DCHECK(it != signal_tasks_.end());
+    callback = it->second;
+    signal_tasks_.erase(it);
+  }
   callback.Run();
 }
 
@@ -259,6 +263,37 @@ void CommandBufferProxyImpl::SetSwapBuffersCompletionCallback(
   swap_buffers_completion_callback_ = callback;
 }
 
+void CommandBufferProxyImpl::WaitForPendingGpuMemoryBufferUsageToComplete(
+    const base::Closure& callback) {
+  uint32 sync_point = InsertSyncPoint();
+  SignalSyncPoint(
+      sync_point,
+      base::Bind(&CommandBufferProxyImpl::GpuMemoryBufferSyncPointSignalled,
+                 base::Unretained(this),
+                 sync_point));
+
+  base::AutoLock lock(gpu_memory_buffer_task_lock_);
+  gpu_memory_buffer_tasks_.insert(std::make_pair(sync_point, callback));

[alexst (slow to review), 2014/10/22 20:05:01]

Forgot to add, these are to be called in the destructor. if the proxy goes away.

[reveman, 2014/10/22 22:42:47]

Will the dtor be called on the same thread that called this function?

+}
+
+void CommandBufferProxyImpl::GpuMemoryBufferSyncPointSignalled(
+    uint32 sync_point) {
+  base::Closure callback;
+  {
+    base::AutoLock lock(gpu_memory_buffer_task_lock_);
+    SignalTaskMap::iterator it = gpu_memory_buffer_tasks_.find(sync_point);
+    DCHECK(it != gpu_memory_buffer_tasks_.end());
+    callback = it->second;
+    signal_tasks_.erase(it);
+  }
+  callback.Run();
+}
+
+uint32 CommandBufferProxyImpl::GetNextSignalId() {
+  base::AutoLock lock(signal_lock_);
+  return next_signal_id_++;
+}
+
 void CommandBufferProxyImpl::WaitForTokenInRange(int32 start, int32 end) {
   TRACE_EVENT2("gpu",
                "CommandBufferProxyImpl::WaitForToken",
@@ -463,13 +498,14 @@ void CommandBufferProxyImpl::SignalSyncPoint(uint32 sync_point,
   if (last_state_.error != gpu::error::kNoError)
     return;
 
-  uint32 signal_id = next_signal_id_++;
+  uint32 signal_id = GetNextSignalId();
   if (!Send(new GpuCommandBufferMsg_SignalSyncPoint(route_id_,
                                                     sync_point,
                                                     signal_id))) {
     return;
   }
 
+  base::AutoLock lock(signal_lock_);
   signal_tasks_.insert(std::make_pair(signal_id, callback));
 }
 
@@ -486,13 +522,14 @@ void CommandBufferProxyImpl::SignalQuery(uint32 query,
   // would have to make calls at an astounding rate (300B/s) and even if they
   // could do that, all they would do is to prevent some callbacks from getting
   // called, leading to stalled threads and/or memory leaks.
-  uint32 signal_id = next_signal_id_++;
+  uint32 signal_id = GetNextSignalId();
   if (!Send(new GpuCommandBufferMsg_SignalQuery(route_id_,
                                                 query,
                                                 signal_id))) {
     return;
   }
 
+  base::AutoLock lock(signal_lock_);
   signal_tasks_.insert(std::make_pair(signal_id, callback));
 }