Add logic to ResourceProvider to correctly lock GpuMemoryBuffer Resources.

cc/output/gl_renderer.cc

Index: cc/output/gl_renderer.cc
diff --git a/cc/output/gl_renderer.cc b/cc/output/gl_renderer.cc
index 1c33fe663f15adfcab6193ea2b8b2809165fce21..6a996da78204782969a40dc986c18faebf623450 100644
--- a/cc/output/gl_renderer.cc
+++ b/cc/output/gl_renderer.cc
@@ -394,7 +394,6 @@ GLRenderer::~GLRenderer() {
     pending_async_read_pixels_.pop_back();
   }
 
-  swapped_overlay_resources_.clear();
   CleanupSharedObjects();
 }
 
@@ -2717,24 +2716,34 @@ void GLRenderer::SwapBuffers(const CompositorFrameMetadata& metadata) {
     }
     compositor_frame.gl_frame_data->sub_buffer_rect = swap_buffer_rect_;
   }
-  output_surface_->SwapBuffers(&compositor_frame);
 
-  // We always hold onto resources until an extra frame has swapped, to make
-  // sure we don't update the buffer while it's being scanned out.
-  swapped_overlay_resources_.push_back(std::move(pending_overlay_resources_));
+  swapping_overlay_resources_.push_back(std::move(pending_overlay_resources_));
   pending_overlay_resources_.clear();
-  if (!settings_->release_overlay_resources_on_swap_complete &&

[ccameron, 2016/05/17 22:40:25]

This "!settings_->release_overlay_resources_on_swap_complete" logic is used by
Chrome OS ... GLRendererWithOverlaysTest.ResourcesExportedAndReturnedWithDelay
should be testing this.

[erikchen, 2016/05/17 23:22:33]

I get it. You wanted me to reuse the *overlay_resources data structures, but not
necessarily the logic.

The logic *does* turn out to be reusable, but it causes the phrase
"release_overlay_resources_on_swap_complete" to not be obviously accurate.  I've
chosen to take this route.

The alternative is to introduce another setting, or perhaps make this setting an
enum.

-      swapped_overlay_resources_.size() > 2) {
-    swapped_overlay_resources_.pop_front();
-  }
+
+  output_surface_->SwapBuffers(&compositor_frame);
+
   swap_buffer_rect_ = gfx::Rect();
 }
 
 void GLRenderer::SwapBuffersComplete() {
-  if (settings_->release_overlay_resources_on_swap_complete &&
-      !swapped_overlay_resources_.empty()) {
-    swapped_overlay_resources_.pop_front();
+  if (swapping_overlay_resources_.empty()) {
+    ReleaseOverlayResources();

[ccameron, 2016/05/17 22:40:25]

This is easier to read without the early-out.

   if (settings_->release_overlay_resources_on_swap_complete &&
+    for (OverlayResourceLock& lock : swapped_overlay_resources_.front()) {
+      swapped_and_acked_overlay_resources_[lock->GetResourceId()] =
+          std::move(lock);
+    }
     swapped_overlay_resources_.pop_front();
   }

[erikchen, 2016/05/17 23:22:33]

Done.

+    return;
+  }
+
+  OverlayResourceLockList& list = swapping_overlay_resources_[0];
+  for (OverlayResourceLock& lock : list) {
+    ResourceId resource_id = lock->GetResourceId();
+
+    // Theoretically, there should be a DCHECK here that asserts that the
+    // resource is not already in swapped_and_acked_overlay_resources_. However,
+    // there are compositor consumers that reuse resources without waiting for
+    // them to be returned.

[ccameron, 2016/05/17 22:40:25]

This comment isn't right -- the renderer is not allowed to write to the resource
while the read lock is out, but it is expected to specify the same resource
frame-after-frame.

[erikchen, 2016/05/17 23:22:33]

got it.

+    swapped_and_acked_overlay_resources_[resource_id] = std::move(lock);
   }
+  list.clear();
+  swapping_overlay_resources_.erase(swapping_overlay_resources_.begin());
+  ReleaseOverlayResources();

[ccameron, 2016/05/17 22:40:25]

I'd just move the ReleaseOverlayResources code here inline.

[erikchen, 2016/05/17 23:22:33]

Done.

 }
 
 void GLRenderer::EnforceMemoryPolicy() {
@@ -3604,9 +3613,9 @@ void GLRenderer::ScheduleCALayers(DrawingFrame* frame) {
     unsigned texture_id = 0;
     if (ca_layer_overlay.contents_resource_id) {
       pending_overlay_resources_.push_back(
-          base::WrapUnique(new ResourceProvider::ScopedReadLockGL(
+          base::WrapUnique(new ResourceProvider::ScopedReadLockGpuMemoryBuffer(
               resource_provider_, ca_layer_overlay.contents_resource_id)));
-      texture_id = pending_overlay_resources_.back()->texture_id();
+      texture_id = pending_overlay_resources_.back()->GetTextureId();
     }
     GLfloat contents_rect[4] = {
         ca_layer_overlay.contents_rect.x(), ca_layer_overlay.contents_rect.y(),
@@ -3647,9 +3656,9 @@ void GLRenderer::ScheduleOverlays(DrawingFrame* frame) {
       DCHECK(texture_id || IsContextLost());
     } else {
       pending_overlay_resources_.push_back(
-          base::WrapUnique(new ResourceProvider::ScopedReadLockGL(
+          base::WrapUnique(new ResourceProvider::ScopedReadLockGpuMemoryBuffer(
               resource_provider_, overlay.resource_id)));
-      texture_id = pending_overlay_resources_.back()->texture_id();
+      texture_id = pending_overlay_resources_.back()->GetTextureId();
     }
 
     context_support_->ScheduleOverlayPlane(
@@ -3658,4 +3667,17 @@ void GLRenderer::ScheduleOverlays(DrawingFrame* frame) {
   }
 }
 
+void GLRenderer::ReleaseOverlayResources() {
+  auto it = swapped_and_acked_overlay_resources_.begin();
+  while (it != swapped_and_acked_overlay_resources_.end()) {
+    if (it->second->GetGpuMemoryBuffer() &&
+        it->second->GetGpuMemoryBuffer()->IsInUseByMacOSWindowServer()) {
+      ++it;
+      continue;
+    }
+
+    it = swapped_and_acked_overlay_resources_.erase(it);
+  }
+}
+
 }  // namespace cc