Remove CompositorObserver::OnCompositingEnded()

OnCompistingEnded() is only used in tests. By replacing usages of
OnCompositingEnded() with OnCompositingStarted(), we are able to remove
the call to OnCompositingEnded() in
ui::Compositor::DidReceiveCompositorFrameAck() without making the tests less
effective.

This is the first step in simplifying CompositorFrameAcks.

BUG=671202

Review-Url: https://codereview.chromium.org/2776973004
Cr-Commit-Position: refs/heads/master@{#460409}

ash/mus/non_client_frame_controller_unittest.cc

@@ -120,7 +120,7 @@ TEST_F(NonClientFrameControllerTest, ContentRegionNotDrawnForClient) {
   // Without the window visible, there should be a tile for the wallpaper at
   // (tile_x, tile_y) of size |tile_size|.
   compositor->ScheduleDraw();
-  ui::DrawWaiterForTest::WaitForCompositingEnded(compositor);
+  ui::DrawWaiterForTest::WaitForCompositingStarted(compositor);
   {
     const cc::CompositorFrame& frame = GetLastCompositorFrame();
     ASSERT_EQ(1u, frame.render_pass_list.size());
@@ -133,7 +133,7 @@ TEST_F(NonClientFrameControllerTest, ContentRegionNotDrawnForClient) {
   widget->SetBounds(widget_bound);
   widget->Show();
   compositor->ScheduleDraw();
-  ui::DrawWaiterForTest::WaitForCompositingEnded(compositor);
+  ui::DrawWaiterForTest::WaitForCompositingStarted(compositor);
   {
     // This time, that tile for the wallpaper will not be drawn.
     const cc::CompositorFrame& frame = GetLastCompositorFrame();

content/browser/renderer_host/browser_compositor_view_mac.mm

@@ -86,7 +86,6 @@ void ReleaseSpareCompositors() {
   void OnCompositingDidCommit(ui::Compositor* compositor) override;
   void OnCompositingStarted(ui::Compositor* compositor,
                             base::TimeTicks start_time) override {}
-  void OnCompositingEnded(ui::Compositor* compositor) override {}
   void OnCompositingLockStateChanged(ui::Compositor* compositor) override {}
   void OnCompositingShuttingDown(ui::Compositor* compositor) override {}
 

content/browser/renderer_host/delegated_frame_host.cc

@@ -739,8 +739,6 @@ void DelegatedFrameHost::OnCompositingStarted(ui::Compositor* compositor,
   last_draw_ended_ = start_time;
 }
 
-void DelegatedFrameHost::OnCompositingEnded(ui::Compositor* compositor) {}
-
 void DelegatedFrameHost::OnCompositingLockStateChanged(
     ui::Compositor* compositor) {
   // A compositor lock that is part of a resize lock timed out. We

content/browser/renderer_host/delegated_frame_host.h

@@ -99,7 +99,6 @@ class CONTENT_EXPORT DelegatedFrameHost
   void OnCompositingDidCommit(ui::Compositor* compositor) override;
   void OnCompositingStarted(ui::Compositor* compositor,
                             base::TimeTicks start_time) override;
-  void OnCompositingEnded(ui::Compositor* compositor) override;
   void OnCompositingLockStateChanged(ui::Compositor* compositor) override;
   void OnCompositingShuttingDown(ui::Compositor* compositor) override;
 

ui/compositor/compositor.cc

@@ -82,7 +82,7 @@ Compositor::Compositor(const cc::FrameSinkId& frame_sink_id,
       context_factory_private_(context_factory_private),
       root_layer_(NULL),
       widget_(gfx::kNullAcceleratedWidget),
-      committed_frame_number_(0),
+      activated_frame_count_(0),
       widget_valid_(false),
       compositor_frame_sink_requested_(false),
       frame_sink_id_(frame_sink_id),
@@ -530,9 +530,7 @@ void Compositor::DidCommit() {
 }
 
 void Compositor::DidReceiveCompositorFrameAck() {
-  ++committed_frame_number_;
-  for (auto& observer : observer_list_)
-    observer.OnCompositingEnded(this);
+  ++activated_frame_count_;
 }
 
 void Compositor::DidSubmitCompositorFrame() {

ui/compositor/compositor.h

@@ -387,7 +387,7 @@ class COMPOSITOR_EXPORT Compositor
   }
 
   const cc::FrameSinkId& frame_sink_id() const { return frame_sink_id_; }
-  int committed_frame_number() const { return committed_frame_number_; }
+  int activated_frame_count() const { return activated_frame_count_; }
   float refresh_rate() const { return refresh_rate_; }
 
  private:
@@ -413,7 +413,7 @@ class COMPOSITOR_EXPORT Compositor
 
   gfx::AcceleratedWidget widget_;
   // A sequence number of a current compositor frame for use with metrics.
-  int committed_frame_number_;
+  int activated_frame_count_;
 
   // current VSYNC refresh rate per second.
   float refresh_rate_;

ui/compositor/compositor_observer.h

@@ -29,9 +29,6 @@ class COMPOSITOR_EXPORT CompositorObserver {
   virtual void OnCompositingStarted(Compositor* compositor,
                                     base::TimeTicks start_time) = 0;
 
-  // Called when compositing completes: the present to screen has completed.
-  virtual void OnCompositingEnded(Compositor* compositor) = 0;
-
   // Called when the compositor lock state changes.
   virtual void OnCompositingLockStateChanged(Compositor* compositor) = 0;
 

ui/compositor/compositor_unittest.cc

@@ -114,14 +114,14 @@ TEST_F(CompositorTest, MAYBE_CreateAndReleaseOutputSurface) {
   compositor()->SetScaleAndSize(1.0f, gfx::Size(10, 10));
   DCHECK(compositor()->IsVisible());
   compositor()->ScheduleDraw();
-  DrawWaiterForTest::WaitForCompositingEnded(compositor());
+  DrawWaiterForTest::WaitForCompositingStarted(compositor());
   compositor()->SetVisible(false);
   EXPECT_EQ(gfx::kNullAcceleratedWidget,
             compositor()->ReleaseAcceleratedWidget());
   compositor()->SetAcceleratedWidget(gfx::kNullAcceleratedWidget);
   compositor()->SetVisible(true);
   compositor()->ScheduleDraw();
-  DrawWaiterForTest::WaitForCompositingEnded(compositor());
+  DrawWaiterForTest::WaitForCompositingStarted(compositor());
   compositor()->SetRootLayer(nullptr);
 }
 

ui/compositor/layer.cc

@@ -1084,7 +1084,7 @@ LayerAnimatorCollection* Layer::GetLayerAnimatorCollection() {
 
 int Layer::GetFrameNumber() const {
   const Compositor* compositor = GetCompositor();
-  return compositor ? compositor->committed_frame_number() : 0;
+  return compositor ? compositor->activated_frame_count() : 0;
 }
 
 float Layer::GetRefreshRate() const {

ui/compositor/layer_unittest.cc

@@ -188,7 +188,7 @@ class LayerWithRealCompositorTest : public testing::Test {
   void DrawTree(Layer* root) {
     GetCompositor()->SetRootLayer(root);
     GetCompositor()->ScheduleDraw();
-    WaitForSwap();
+    WaitForDraw();
   }
 
   void ReadPixels(SkBitmap* bitmap) {
@@ -222,10 +222,6 @@ class LayerWithRealCompositorTest : public testing::Test {
     ui::DrawWaiterForTest::WaitForCompositingStarted(GetCompositor());
   }
 
-  void WaitForSwap() {
-    DrawWaiterForTest::WaitForCompositingEnded(GetCompositor());
-  }
-
   void WaitForCommit() {
     ui::DrawWaiterForTest::WaitForCommit(GetCompositor());
   }
@@ -368,12 +364,11 @@ class TestCompositorObserver : public CompositorObserver {
   TestCompositorObserver() = default;
 
   bool committed() const { return committed_; }
-  bool notified() const { return started_ && ended_; }
+  bool started() const { return started_; }
 
   void Reset() {
     committed_ = false;
     started_ = false;
-    ended_ = false;
   }
 
  private:
@@ -386,15 +381,12 @@ class TestCompositorObserver : public CompositorObserver {
     started_ = true;
   }
 
-  void OnCompositingEnded(Compositor* compositor) override { ended_ = true; }
-
   void OnCompositingLockStateChanged(Compositor* compositor) override {}
 
   void OnCompositingShuttingDown(Compositor* compositor) override {}
 
   bool committed_ = false;
   bool started_ = false;
-  bool ended_ = false;
 
   DISALLOW_COPY_AND_ASSIGN(TestCompositorObserver);
 };
@@ -1317,7 +1309,7 @@ TEST_F(LayerWithRealCompositorTest, CompositorObservers) {
   // Explicitly called DrawTree should cause the observers to be notified.
   // NOTE: this call to DrawTree sets l1 to be the compositor's root layer.
   DrawTree(l1.get());
-  EXPECT_TRUE(observer.notified());
+  EXPECT_TRUE(observer.started());
 
   // ScheduleDraw without any visible change should cause a commit.
   observer.Reset();
@@ -1328,26 +1320,26 @@ TEST_F(LayerWithRealCompositorTest, CompositorObservers) {
   // Moving, but not resizing, a layer should alert the observers.
   observer.Reset();
   l2->SetBounds(gfx::Rect(0, 0, 350, 350));
-  WaitForSwap();
-  EXPECT_TRUE(observer.notified());
+  WaitForDraw();
+  EXPECT_TRUE(observer.started());
 
   // So should resizing a layer.
   observer.Reset();
   l2->SetBounds(gfx::Rect(0, 0, 400, 400));
-  WaitForSwap();
-  EXPECT_TRUE(observer.notified());
+  WaitForDraw();
+  EXPECT_TRUE(observer.started());
 
   // Opacity changes should alert the observers.
   observer.Reset();
   l2->SetOpacity(0.5f);
-  WaitForSwap();
-  EXPECT_TRUE(observer.notified());
+  WaitForDraw();
+  EXPECT_TRUE(observer.started());
 
   // So should setting the opacity back.
   observer.Reset();
   l2->SetOpacity(1.0f);
-  WaitForSwap();
-  EXPECT_TRUE(observer.notified());
+  WaitForDraw();
+  EXPECT_TRUE(observer.started());
 
   // Setting the transform of a layer should alert the observers.
   observer.Reset();
@@ -1356,17 +1348,17 @@ TEST_F(LayerWithRealCompositorTest, CompositorObservers) {
   transform.Rotate(90.0);
   transform.Translate(-200.0, -200.0);
   l2->SetTransform(transform);
-  WaitForSwap();
-  EXPECT_TRUE(observer.notified());
+  WaitForDraw();
+  EXPECT_TRUE(observer.started());
 
   GetCompositor()->RemoveObserver(&observer);
 
   // Opacity changes should no longer alert the removed observer.
   observer.Reset();
   l2->SetOpacity(0.5f);
-  WaitForSwap();
+  WaitForDraw();
 
-  EXPECT_FALSE(observer.notified());
+  EXPECT_FALSE(observer.started());
 }
 
 // Checks that modifying the hierarchy correctly affects final composite.
@@ -2179,7 +2171,7 @@ TEST_F(LayerWithRealCompositorTest, CompositorAnimationObserverTest) {
   EXPECT_EQ(0u, animation_observer.animation_step_count());
 
   root->SetOpacity(0.5f);
-  WaitForSwap();
+  WaitForDraw();
   EXPECT_EQ(1u, animation_observer.animation_step_count());
 
   EXPECT_FALSE(animation_observer.shutdown());
@@ -2225,7 +2217,7 @@ TEST_F(LayerWithRealCompositorTest, ReportMetrics) {
   animation_sequence->SetAnimationMetricsReporter(&reporter);
   animator->StartAnimation(animation_sequence);
   while (!reporter.report_called())
-    WaitForSwap();
+    WaitForDraw();
   ResetCompositor();
   // Even though most of the time 100% smooth animations are expected, on the
   // test bots this cannot be guaranteed. Therefore simply check that some

ui/compositor/test/draw_waiter_for_test.cc

@@ -14,11 +14,6 @@ void DrawWaiterForTest::WaitForCompositingStarted(Compositor* compositor) {
   waiter.WaitImpl(compositor);
 }
 
-void DrawWaiterForTest::WaitForCompositingEnded(Compositor* compositor) {
-  DrawWaiterForTest waiter(WAIT_FOR_COMPOSITING_ENDED);
-  waiter.WaitImpl(compositor);
-}
-
 // static
 void DrawWaiterForTest::WaitForCommit(Compositor* compositor) {
   DrawWaiterForTest waiter(WAIT_FOR_COMMIT);
@@ -49,11 +44,6 @@ void DrawWaiterForTest::OnCompositingStarted(Compositor* compositor,
     wait_run_loop_->Quit();
 }
 
-void DrawWaiterForTest::OnCompositingEnded(Compositor* compositor) {
-  if (wait_event_ == WAIT_FOR_COMPOSITING_ENDED)
-    wait_run_loop_->Quit();
-}
-
 void DrawWaiterForTest::OnCompositingLockStateChanged(Compositor* compositor) {}
 
 void DrawWaiterForTest::OnCompositingShuttingDown(Compositor* compositor) {}

ui/compositor/test/draw_waiter_for_test.h

@@ -25,17 +25,13 @@ class DrawWaiterForTest : public CompositorObserver {
   // not to draw.
   static void WaitForCompositingStarted(Compositor* compositor);
 
-  // Waits for a swap to be completed from the compositor.
-  static void WaitForCompositingEnded(Compositor* compositor);
-
   // Waits for a commit instead of a draw.
   static void WaitForCommit(Compositor* compositor);
 
  private:
   enum WaitEvent {
     WAIT_FOR_COMMIT,
     WAIT_FOR_COMPOSITING_STARTED,
-    WAIT_FOR_COMPOSITING_ENDED,
   };
   DrawWaiterForTest(WaitEvent wait_event);
   ~DrawWaiterForTest();
@@ -46,7 +42,6 @@ class DrawWaiterForTest : public CompositorObserver {
   void OnCompositingDidCommit(Compositor* compositor) override;
   void OnCompositingStarted(Compositor* compositor,
                             base::TimeTicks start_time) override;
-  void OnCompositingEnded(Compositor* compositor) override;
   void OnCompositingLockStateChanged(Compositor* compositor) override;
   void OnCompositingShuttingDown(Compositor* compositor) override;
 

ui/snapshot/snapshot_aura_unittest.cc

@@ -126,7 +126,7 @@ class SnapshotAuraTest : public testing::Test {
 
   void WaitForDraw() {
     helper_->host()->compositor()->ScheduleDraw();
-    ui::DrawWaiterForTest::WaitForCompositingEnded(
+    ui::DrawWaiterForTest::WaitForCompositingStarted(
         helper_->host()->compositor());
   }
 

ui/views/controls/scroll_view_unittest.cc

@@ -268,7 +268,6 @@ class WidgetScrollViewTest : public test::WidgetTest,
   }
   void OnCompositingStarted(ui::Compositor* compositor,
                             base::TimeTicks start_time) override {}
-  void OnCompositingEnded(ui::Compositor* compositor) override {}
   void OnCompositingLockStateChanged(ui::Compositor* compositor) override {}
   void OnCompositingShuttingDown(ui::Compositor* compositor) override {}
 

ui/views/view_unittest.cc

@@ -4211,22 +4211,22 @@ TEST_F(ViewLayerTest, DontPaintChildrenWithLayers) {
   widget()->SetContentsView(content_view);
   content_view->SetPaintToLayer();
   GetRootLayer()->GetCompositor()->ScheduleDraw();
-  ui::DrawWaiterForTest::WaitForCompositingEnded(
+  ui::DrawWaiterForTest::WaitForCompositingStarted(
       GetRootLayer()->GetCompositor());
   GetRootLayer()->SchedulePaint(gfx::Rect(0, 0, 10, 10));
   content_view->set_painted(false);
   // content_view no longer has a dirty rect. Paint from the root and make sure
   // PaintTrackingView isn't painted.
   GetRootLayer()->GetCompositor()->ScheduleDraw();
-  ui::DrawWaiterForTest::WaitForCompositingEnded(
+  ui::DrawWaiterForTest::WaitForCompositingStarted(
       GetRootLayer()->GetCompositor());
   EXPECT_FALSE(content_view->painted());
 
   // Make content_view have a dirty rect, paint the layers and make sure
   // PaintTrackingView is painted.
   content_view->layer()->SchedulePaint(gfx::Rect(0, 0, 10, 10));
   GetRootLayer()->GetCompositor()->ScheduleDraw();
-  ui::DrawWaiterForTest::WaitForCompositingEnded(
+  ui::DrawWaiterForTest::WaitForCompositingStarted(
       GetRootLayer()->GetCompositor());
   EXPECT_TRUE(content_view->painted());
 }