cc: Allow returning overlay resources upon swap completion

On Mac, the only way to reliably know that overlay resources are no
longer in use is by the function IOSurfaceIsInUse (which is checked by
the functions ResourceProvider::InUseByConsumer and
ResourceProvider::CanLockForWrite).

IOSurfaceIsInUse will start returning true as soon as a CALayer has its
contents set to an IOSurface, which happens during swap in the GPU
process (in particular, when a Mach port for the IOSurface is created
for sending to the WindowServer process), and so the earliest time that
resources may be returned to the renderer process is swap completion.

Repurpose a now-extinct flag to indicate that overlay resources are to
be returned immediately upon swap acknowledgement, and add tests to
ensure this behavior.

BUG=558701
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Review URL: https://codereview.chromium.org/1523063005

Cr-Commit-Position: refs/heads/master@{#368751}

7 files changed, 65 insertions, 93 deletions

diff --git a/cc/output/direct_renderer.h b/cc/output/direct_renderer.h
index 28f2591..cf05af4 100644
--- a/cc/output/direct_renderer.h
+++ b/cc/output/direct_renderer.h
@@ -37,6 +37,7 @@ class CC_EXPORT DirectRenderer : public Renderer {
                  const gfx::Rect& device_viewport_rect,
                  const gfx::Rect& device_clip_rect,
                  bool disable_picture_quad_image_filtering) override;
+  virtual void SwapBuffersComplete() {}
 
   struct CC_EXPORT DrawingFrame {
     DrawingFrame();
diff --git a/cc/output/gl_renderer.cc b/cc/output/gl_renderer.cc
index 4dc802f..acd7bf2 100644
--- a/cc/output/gl_renderer.cc
+++ b/cc/output/gl_renderer.cc
@@ -388,10 +388,7 @@ GLRenderer::~GLRenderer() {
     pending_async_read_pixels_.pop_back();
   }
 
-  previous_swap_overlay_resources_[1].clear();
-  previous_swap_overlay_resources_[0].clear();
-  in_use_overlay_resources_.clear();
-
+  swapped_overlay_resources_.clear();
   CleanupSharedObjects();
 }
 
@@ -2626,22 +2623,24 @@ void GLRenderer::SwapBuffers(const CompositorFrameMetadata& metadata) {
   }
   output_surface_->SwapBuffers(&compositor_frame);
 
-  // We always hold onto resources for an extra frame, to make sure we don't
-  // update the buffer while it's being scanned out. On some platforms, hold
-  // on to resources for an extra frame.
-  if (settings_->delay_releasing_overlay_resources) {
-    previous_swap_overlay_resources_[1].clear();
-    previous_swap_overlay_resources_[1].swap(
-        previous_swap_overlay_resources_[0]);
-  } else {
-    previous_swap_overlay_resources_[0].clear();
+  // 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_));
+  pending_overlay_resources_.clear();
+  if (!settings_->release_overlay_resources_on_swap_complete &&
+      swapped_overlay_resources_.size() > 2) {
+    swapped_overlay_resources_.pop_front();
   }
-  previous_swap_overlay_resources_[0].swap(in_use_overlay_resources_);
-  in_use_overlay_resources_.swap(pending_overlay_resources_);
-
   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();
+  }
+}
+
 void GLRenderer::EnforceMemoryPolicy() {
   if (!visible()) {
     TRACE_EVENT0("cc", "GLRenderer::EnforceMemoryPolicy dropping resources");
diff --git a/cc/output/gl_renderer.h b/cc/output/gl_renderer.h
index 8a7694a..9599b07 100644
--- a/cc/output/gl_renderer.h
+++ b/cc/output/gl_renderer.h
@@ -65,6 +65,7 @@ class CC_EXPORT GLRenderer : public DirectRenderer {
   void Finish() override;
 
   void SwapBuffers(const CompositorFrameMetadata& metadata) override;
+  void SwapBuffersComplete() override;
 
   virtual bool IsContextLost();
 
@@ -264,8 +265,7 @@ class CC_EXPORT GLRenderer : public DirectRenderer {
   using OverlayResourceLockList =
       std::vector<scoped_ptr<ResourceProvider::ScopedReadLockGL>>;
   OverlayResourceLockList pending_overlay_resources_;
-  OverlayResourceLockList in_use_overlay_resources_;
-  OverlayResourceLockList previous_swap_overlay_resources_[2];
+  std::deque<OverlayResourceLockList> swapped_overlay_resources_;
 
   RendererCapabilitiesImpl capabilities_;
 
diff --git a/cc/output/overlay_unittest.cc b/cc/output/overlay_unittest.cc
index 8b2333d..140935c 100644
--- a/cc/output/overlay_unittest.cc
+++ b/cc/output/overlay_unittest.cc
@@ -157,7 +157,10 @@ class OverlayOutputSurface : public OutputSurface {
   }
 
   // OutputSurface implementation
-  void SwapBuffers(CompositorFrame* frame) override;
+  void SwapBuffers(CompositorFrame* frame) override {
+    client_->DidSwapBuffers();
+  }
+  void OnSwapBuffersComplete() override { client_->DidSwapBuffersComplete(); }
 
   void SetOverlayCandidateValidator(OverlayCandidateValidator* validator) {
     overlay_candidate_validator_.reset(validator);
@@ -180,11 +183,6 @@ class OverlayOutputSurface : public OutputSurface {
   bool is_displayed_as_overlay_plane_;
 };
 
-void OverlayOutputSurface::SwapBuffers(CompositorFrame* frame) {
-  client_->DidSwapBuffers();
-  client_->DidSwapBuffersComplete();
-}
-
 scoped_ptr<RenderPass> CreateRenderPass() {
   RenderPassId id(1, 0);
   gfx::Rect output_rect(0, 0, 256, 256);
@@ -1715,7 +1713,18 @@ class GLRendererWithOverlaysTest : public testing::Test {
                                                   resource_provider_.get()));
   }
 
-  void SwapBuffers() { renderer_->SwapBuffers(CompositorFrameMetadata()); }
+  void SwapBuffers() {
+    renderer_->SwapBuffers(CompositorFrameMetadata());
+    output_surface_->OnSwapBuffersComplete();
+    renderer_->SwapBuffersComplete();
+  }
+  void SwapBuffersWithoutComplete() {
+    renderer_->SwapBuffers(CompositorFrameMetadata());
+  }
+  void SwapBuffersComplete() {
+    output_surface_->OnSwapBuffersComplete();
+    renderer_->SwapBuffersComplete();
+  }
 
   RendererSettings settings_;
   FakeOutputSurfaceClient output_surface_client_;
@@ -2018,9 +2027,9 @@ TEST_F(GLRendererWithOverlaysTest, ResourcesExportedAndReturnedWithDelay) {
   Mock::VerifyAndClearExpectations(&scheduler_);
 }
 
-TEST_F(GLRendererWithOverlaysTest, ResourcesExportedAndReturnedWithExtraDelay) {
+TEST_F(GLRendererWithOverlaysTest, ResourcesExportedAndReturnedAtSwapComplete) {
   bool use_validator = true;
-  settings_.delay_releasing_overlay_resources = true;
+  settings_.release_overlay_resources_on_swap_complete = true;
   Init(use_validator);
   renderer_->set_expect_overlays(true);
 
@@ -2059,95 +2068,57 @@ TEST_F(GLRendererWithOverlaysTest, ResourcesExportedAndReturnedWithExtraDelay) {
   overlay3.resource_id = resource3;
   overlay3.plane_z_order = 1;
 
+  // First frame, with no swap completion.
   EXPECT_CALL(scheduler_, Schedule(_, _, _, _, _)).Times(2);
   renderer_->BeginDrawingFrame(&frame1);
   renderer_->FinishDrawingFrame(&frame1);
   EXPECT_TRUE(resource_provider_->InUseByConsumer(resource1));
-  EXPECT_FALSE(resource_provider_->InUseByConsumer(resource2));
-  SwapBuffers();
+  SwapBuffersWithoutComplete();
+  EXPECT_TRUE(resource_provider_->InUseByConsumer(resource1));
   Mock::VerifyAndClearExpectations(&scheduler_);
 
+  // Second frame, with no swap completion.
   EXPECT_CALL(scheduler_, Schedule(_, _, _, _, _)).Times(2);
   renderer_->BeginDrawingFrame(&frame2);
   renderer_->FinishDrawingFrame(&frame2);
   EXPECT_TRUE(resource_provider_->InUseByConsumer(resource1));
   EXPECT_TRUE(resource_provider_->InUseByConsumer(resource2));
-  SwapBuffers();
+  SwapBuffersWithoutComplete();
   EXPECT_TRUE(resource_provider_->InUseByConsumer(resource1));
   EXPECT_TRUE(resource_provider_->InUseByConsumer(resource2));
   Mock::VerifyAndClearExpectations(&scheduler_);
 
+  // Third frame, still with no swap completion (where the resources would
+  // otherwise have been released).
   EXPECT_CALL(scheduler_, Schedule(_, _, _, _, _)).Times(2);
   renderer_->BeginDrawingFrame(&frame3);
   renderer_->FinishDrawingFrame(&frame3);
   EXPECT_TRUE(resource_provider_->InUseByConsumer(resource1));
   EXPECT_TRUE(resource_provider_->InUseByConsumer(resource2));
   EXPECT_TRUE(resource_provider_->InUseByConsumer(resource3));
-  SwapBuffers();
+  SwapBuffersWithoutComplete();
   EXPECT_TRUE(resource_provider_->InUseByConsumer(resource1));
   EXPECT_TRUE(resource_provider_->InUseByConsumer(resource2));
   EXPECT_TRUE(resource_provider_->InUseByConsumer(resource3));
   Mock::VerifyAndClearExpectations(&scheduler_);
 
-  // No overlays, release the resource.
-  EXPECT_CALL(scheduler_, Schedule(_, _, _, _, _)).Times(0);
-  DirectRenderer::DrawingFrame frame_no_overlays;
-  frame_no_overlays.render_passes_in_draw_order = &pass_list;
-  renderer_->set_expect_overlays(false);
-  renderer_->BeginDrawingFrame(&frame_no_overlays);
-  renderer_->FinishDrawingFrame(&frame_no_overlays);
-  EXPECT_TRUE(resource_provider_->InUseByConsumer(resource1));
-  EXPECT_TRUE(resource_provider_->InUseByConsumer(resource2));
-  EXPECT_TRUE(resource_provider_->InUseByConsumer(resource3));
-  SwapBuffers();
+  // This completion corresponds to the first frame.
+  SwapBuffersComplete();
   EXPECT_FALSE(resource_provider_->InUseByConsumer(resource1));
   EXPECT_TRUE(resource_provider_->InUseByConsumer(resource2));
   EXPECT_TRUE(resource_provider_->InUseByConsumer(resource3));
-  Mock::VerifyAndClearExpectations(&scheduler_);
-
-  // Use the same buffer twice.
-  renderer_->set_expect_overlays(true);
-  EXPECT_CALL(scheduler_, Schedule(_, _, _, _, _)).Times(2);
-  renderer_->BeginDrawingFrame(&frame1);
-  renderer_->FinishDrawingFrame(&frame1);
-  EXPECT_TRUE(resource_provider_->InUseByConsumer(resource1));
-  SwapBuffers();
-  Mock::VerifyAndClearExpectations(&scheduler_);
-
-  EXPECT_CALL(scheduler_, Schedule(_, _, _, _, _)).Times(2);
-  renderer_->BeginDrawingFrame(&frame1);
-  renderer_->FinishDrawingFrame(&frame1);
-  EXPECT_TRUE(resource_provider_->InUseByConsumer(resource1));
-  SwapBuffers();
-  EXPECT_TRUE(resource_provider_->InUseByConsumer(resource1));
-  Mock::VerifyAndClearExpectations(&scheduler_);
 
-  EXPECT_CALL(scheduler_, Schedule(_, _, _, _, _)).Times(0);
-  renderer_->set_expect_overlays(false);
-  renderer_->BeginDrawingFrame(&frame_no_overlays);
-  renderer_->FinishDrawingFrame(&frame_no_overlays);
-  EXPECT_TRUE(resource_provider_->InUseByConsumer(resource1));
-  SwapBuffers();
-  EXPECT_TRUE(resource_provider_->InUseByConsumer(resource1));
-  Mock::VerifyAndClearExpectations(&scheduler_);
-
-  EXPECT_CALL(scheduler_, Schedule(_, _, _, _, _)).Times(0);
-  renderer_->set_expect_overlays(false);
-  renderer_->BeginDrawingFrame(&frame_no_overlays);
-  renderer_->FinishDrawingFrame(&frame_no_overlays);
-  EXPECT_TRUE(resource_provider_->InUseByConsumer(resource1));
-  SwapBuffers();
-  EXPECT_TRUE(resource_provider_->InUseByConsumer(resource1));
-  Mock::VerifyAndClearExpectations(&scheduler_);
+  // This completion corresponds to the second frame.
+  SwapBuffersComplete();
+  EXPECT_FALSE(resource_provider_->InUseByConsumer(resource1));
+  EXPECT_FALSE(resource_provider_->InUseByConsumer(resource2));
+  EXPECT_TRUE(resource_provider_->InUseByConsumer(resource3));
 
-  EXPECT_CALL(scheduler_, Schedule(_, _, _, _, _)).Times(0);
-  renderer_->set_expect_overlays(false);
-  renderer_->BeginDrawingFrame(&frame_no_overlays);
-  renderer_->FinishDrawingFrame(&frame_no_overlays);
-  EXPECT_TRUE(resource_provider_->InUseByConsumer(resource1));
-  SwapBuffers();
+  // This completion corresponds to the third frame.
+  SwapBuffersComplete();
   EXPECT_FALSE(resource_provider_->InUseByConsumer(resource1));
-  Mock::VerifyAndClearExpectations(&scheduler_);
+  EXPECT_FALSE(resource_provider_->InUseByConsumer(resource2));
+  EXPECT_FALSE(resource_provider_->InUseByConsumer(resource3));
 }
 
 }  // namespace
diff --git a/cc/output/renderer_settings.cc b/cc/output/renderer_settings.cc
index da79a48..133fb33 100644
--- a/cc/output/renderer_settings.cc
+++ b/cc/output/renderer_settings.cc
@@ -19,7 +19,7 @@ RendererSettings::RendererSettings()
       finish_rendering_on_resize(false),
       should_clear_root_render_pass(true),
       disable_display_vsync(false),
-      delay_releasing_overlay_resources(false),
+      release_overlay_resources_on_swap_complete(false),
       refresh_rate(60.0),
       highp_threshold_min(0),
       use_rgba_4444_textures(false),
@@ -37,8 +37,8 @@ void RendererSettings::ToProtobuf(proto::RendererSettings* proto) const {
   proto->set_finish_rendering_on_resize(finish_rendering_on_resize);
   proto->set_should_clear_root_render_pass(should_clear_root_render_pass);
   proto->set_disable_display_vsync(disable_display_vsync);
-  proto->set_delay_releasing_overlay_resources(
-      delay_releasing_overlay_resources);
+  proto->set_release_overlay_resources_on_swap_complete(
+      release_overlay_resources_on_swap_complete);
   proto->set_refresh_rate(refresh_rate);
   proto->set_highp_threshold_min(highp_threshold_min);
   proto->set_use_rgba_4444_textures(use_rgba_4444_textures);
@@ -54,7 +54,8 @@ void RendererSettings::FromProtobuf(const proto::RendererSettings& proto) {
   finish_rendering_on_resize = proto.finish_rendering_on_resize();
   should_clear_root_render_pass = proto.should_clear_root_render_pass();
   disable_display_vsync = proto.disable_display_vsync();
-  delay_releasing_overlay_resources = proto.delay_releasing_overlay_resources();
+  release_overlay_resources_on_swap_complete =
+      proto.release_overlay_resources_on_swap_complete();
   refresh_rate = proto.refresh_rate();
   highp_threshold_min = proto.highp_threshold_min();
   use_rgba_4444_textures = proto.use_rgba_4444_textures();
@@ -70,8 +71,8 @@ bool RendererSettings::operator==(const RendererSettings& other) const {
          finish_rendering_on_resize == other.finish_rendering_on_resize &&
          should_clear_root_render_pass == other.should_clear_root_render_pass &&
          disable_display_vsync == other.disable_display_vsync &&
-         delay_releasing_overlay_resources ==
-             other.delay_releasing_overlay_resources &&
+         release_overlay_resources_on_swap_complete ==
+             other.release_overlay_resources_on_swap_complete &&
          refresh_rate == other.refresh_rate &&
          highp_threshold_min == other.highp_threshold_min &&
          use_rgba_4444_textures == other.use_rgba_4444_textures &&
diff --git a/cc/output/renderer_settings.h b/cc/output/renderer_settings.h
index 46008fe..657996e 100644
--- a/cc/output/renderer_settings.h
+++ b/cc/output/renderer_settings.h
@@ -27,7 +27,7 @@ class CC_EXPORT RendererSettings {
   bool finish_rendering_on_resize;
   bool should_clear_root_render_pass;
   bool disable_display_vsync;
-  bool delay_releasing_overlay_resources;
+  bool release_overlay_resources_on_swap_complete;
   double refresh_rate;
   int highp_threshold_min;
   bool use_rgba_4444_textures;
diff --git a/cc/output/renderer_settings_unittest.cc b/cc/output/renderer_settings_unittest.cc
index b7febd4..ca318ae 100644
--- a/cc/output/renderer_settings_unittest.cc
+++ b/cc/output/renderer_settings_unittest.cc
@@ -27,7 +27,7 @@ TEST(RendererSettingsTest, AllFieldsFlipped) {
   settings.finish_rendering_on_resize = true;
   settings.should_clear_root_render_pass = false;
   settings.disable_display_vsync = true;
-  settings.delay_releasing_overlay_resources = true;
+  settings.release_overlay_resources_on_swap_complete = true;
   settings.refresh_rate = 6.0;
   settings.highp_threshold_min = 1;
   settings.use_rgba_4444_textures = true;
@@ -45,7 +45,7 @@ TEST(RendererSettingsTest, ArbitraryFieldValues) {
   settings.finish_rendering_on_resize = false;
   settings.should_clear_root_render_pass = false;
   settings.disable_display_vsync = true;
-  settings.delay_releasing_overlay_resources = true;
+  settings.release_overlay_resources_on_swap_complete = true;
   settings.refresh_rate = 999.0;
   settings.highp_threshold_min = 1;
   settings.use_rgba_4444_textures = true;