Hook NotifyReadToDraw in SingleThreadProxy and Defer Draw When Needed

For ui::Compositor with impl-side painting, RequireHighResToDraw would
be set to true. Currently during deadline we would try to draw even
when tiles are not ready. Draw would then be aborted inside LTHI.
This CL makes sure that in above case draw would be aborted when tiles
are not ready yet.

BUG=469175
R=brianderson

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

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

9 files changed, 164 insertions, 54 deletions

diff --git a/cc/scheduler/scheduler.cc b/cc/scheduler/scheduler.cc
index 37e7c53..ad5a638 100644
--- a/cc/scheduler/scheduler.cc
+++ b/cc/scheduler/scheduler.cc
@@ -202,8 +202,9 @@ void Scheduler::NotifyReadyToActivate() {
 }
 
 void Scheduler::NotifyReadyToDraw() {
-  // Empty for now, until we take action based on the notification as part of
-  // crbugs 352894, 383157, 421923.
+  // Future work might still needed for crbug.com/352894.
+  state_machine_.NotifyReadyToDraw();
+  ProcessScheduledActions();
 }
 
 void Scheduler::SetThrottleFrameProduction(bool throttle) {
@@ -232,6 +233,11 @@ void Scheduler::SetNeedsPrepareTiles() {
   ProcessScheduledActions();
 }
 
+void Scheduler::SetWaitForReadyToDraw() {
+  state_machine_.SetWaitForReadyToDraw();
+  ProcessScheduledActions();
+}
+
 void Scheduler::SetMaxSwapsPending(int max) {
   state_machine_.SetMaxSwapsPending(max);
 }
@@ -595,6 +601,13 @@ void Scheduler::ScheduleBeginImplFrameDeadline() {
       deadline =
           begin_impl_frame_args_.frame_time + begin_impl_frame_args_.interval;
       break;
+    case SchedulerStateMachine::
+        BEGIN_IMPL_FRAME_DEADLINE_MODE_BLOCKED_ON_READY_TO_DRAW:
+      // We are blocked because we are waiting for ReadyToDraw signal. We would
+      // post deadline after we received ReadyToDraw singal.
+      TRACE_EVENT1("cc", "Scheduler::ScheduleBeginImplFrameDeadline",
+                   "deadline_mode", "blocked_on_ready_to_draw");
+      return;
   }
 
   TRACE_EVENT1(
diff --git a/cc/scheduler/scheduler.h b/cc/scheduler/scheduler.h
index 3a4af5a..54c1511 100644
--- a/cc/scheduler/scheduler.h
+++ b/cc/scheduler/scheduler.h
@@ -117,6 +117,8 @@ class CC_EXPORT Scheduler : public BeginFrameObserverMixIn {
 
   void SetNeedsPrepareTiles();
 
+  void SetWaitForReadyToDraw();
+
   void SetMaxSwapsPending(int max);
   void DidSwapBuffers();
   void DidSwapBuffersComplete();
diff --git a/cc/scheduler/scheduler_state_machine.cc b/cc/scheduler/scheduler_state_machine.cc
index 715d92f..dcd1ac9 100644
--- a/cc/scheduler/scheduler_state_machine.cc
+++ b/cc/scheduler/scheduler_state_machine.cc
@@ -52,7 +52,8 @@ SchedulerStateMachine::SchedulerStateMachine(const SchedulerSettings& settings)
       continuous_painting_(false),
       children_need_begin_frames_(false),
       defer_commits_(false),
-      last_commit_had_no_updates_(false) {
+      last_commit_had_no_updates_(false),
+      wait_for_active_tree_ready_to_draw_(false) {
 }
 
 const char* SchedulerStateMachine::OutputSurfaceStateToString(
@@ -205,6 +206,8 @@ void SchedulerStateMachine::AsValueInto(
                     pending_tree_is_ready_for_activation_);
   state->SetBoolean("active_tree_needs_first_draw",
                     active_tree_needs_first_draw_);
+  state->SetBoolean("wait_for_active_tree_ready_to_draw",
+                    wait_for_active_tree_ready_to_draw_);
   state->SetBoolean("did_create_and_initialize_first_output_surface",
                     did_create_and_initialize_first_output_surface_);
   state->SetBoolean("impl_latency_takes_priority",
@@ -816,7 +819,11 @@ void SchedulerStateMachine::OnBeginImplFrameIdle() {
 
 SchedulerStateMachine::BeginImplFrameDeadlineMode
 SchedulerStateMachine::CurrentBeginImplFrameDeadlineMode() const {
-  if (ShouldTriggerBeginImplFrameDeadlineImmediately()) {
+  if (wait_for_active_tree_ready_to_draw_) {
+    // When we are waiting for ready to draw signal, we do not wait to post a
+    // deadline yet.
+    return BEGIN_IMPL_FRAME_DEADLINE_MODE_BLOCKED_ON_READY_TO_DRAW;
+  } else if (ShouldTriggerBeginImplFrameDeadlineImmediately()) {
     return BEGIN_IMPL_FRAME_DEADLINE_MODE_IMMEDIATE;
   } else if (needs_redraw_ && pending_swaps_ < max_pending_swaps_) {
     // We have an animation or fast input path on the impl thread that wants
@@ -928,6 +935,11 @@ void SchedulerStateMachine::SetNeedsAnimate() {
   needs_animate_ = true;
 }
 
+void SchedulerStateMachine::SetWaitForReadyToDraw() {
+  DCHECK(settings_.impl_side_painting);
+  wait_for_active_tree_ready_to_draw_ = true;
+}
+
 void SchedulerStateMachine::SetNeedsPrepareTiles() {
   if (!needs_prepare_tiles_) {
     TRACE_EVENT0("cc", "SchedulerStateMachine::SetNeedsPrepareTiles");
@@ -1044,6 +1056,7 @@ void SchedulerStateMachine::DidLoseOutputSurface() {
     return;
   output_surface_state_ = OUTPUT_SURFACE_LOST;
   needs_redraw_ = false;
+  wait_for_active_tree_ready_to_draw_ = false;
 }
 
 void SchedulerStateMachine::NotifyReadyToActivate() {
@@ -1051,6 +1064,10 @@ void SchedulerStateMachine::NotifyReadyToActivate() {
     pending_tree_is_ready_for_activation_ = true;
 }
 
+void SchedulerStateMachine::NotifyReadyToDraw() {
+  wait_for_active_tree_ready_to_draw_ = false;
+}
+
 void SchedulerStateMachine::DidCreateAndInitializeOutputSurface() {
   DCHECK_EQ(output_surface_state_, OUTPUT_SURFACE_CREATING);
   output_surface_state_ = OUTPUT_SURFACE_WAITING_FOR_FIRST_COMMIT;
diff --git a/cc/scheduler/scheduler_state_machine.h b/cc/scheduler/scheduler_state_machine.h
index 3914039..f568665 100644
--- a/cc/scheduler/scheduler_state_machine.h
+++ b/cc/scheduler/scheduler_state_machine.h
@@ -66,6 +66,7 @@ class CC_EXPORT SchedulerStateMachine {
     BEGIN_IMPL_FRAME_DEADLINE_MODE_IMMEDIATE,
     BEGIN_IMPL_FRAME_DEADLINE_MODE_REGULAR,
     BEGIN_IMPL_FRAME_DEADLINE_MODE_LATE,
+    BEGIN_IMPL_FRAME_DEADLINE_MODE_BLOCKED_ON_READY_TO_DRAW,
   };
   static const char* BeginImplFrameDeadlineModeToString(
       BeginImplFrameDeadlineMode mode);
@@ -170,6 +171,9 @@ class CC_EXPORT SchedulerStateMachine {
   // PrepareTiles will occur shortly (even if no redraw is required).
   void SetNeedsPrepareTiles();
 
+  // Make deadline wait for ReadyToDraw signal.
+  void SetWaitForReadyToDraw();
+
   // Sets how many swaps can be pending to the OutputSurface.
   void SetMaxSwapsPending(int max);
 
@@ -227,6 +231,9 @@ class CC_EXPORT SchedulerStateMachine {
   // Indicates that the pending tree is ready for activation.
   void NotifyReadyToActivate();
 
+  // Indicates the active tree's visible tiles are ready to be drawn.
+  void NotifyReadyToDraw();
+
   bool has_pending_tree() const { return has_pending_tree_; }
   bool active_tree_needs_first_draw() const {
     return active_tree_needs_first_draw_;
@@ -333,6 +340,7 @@ class CC_EXPORT SchedulerStateMachine {
   bool children_need_begin_frames_;
   bool defer_commits_;
   bool last_commit_had_no_updates_;
+  bool wait_for_active_tree_ready_to_draw_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(SchedulerStateMachine);
diff --git a/cc/scheduler/scheduler_unittest.cc b/cc/scheduler/scheduler_unittest.cc
index 935a80a..eff36a9 100644
--- a/cc/scheduler/scheduler_unittest.cc
+++ b/cc/scheduler/scheduler_unittest.cc
@@ -1124,6 +1124,92 @@ TEST_F(SchedulerTest, TriggerBeginFrameDeadlineEarly) {
   EXPECT_EQ(base::TimeTicks(), client->posted_begin_impl_frame_deadline());
 }
 
+TEST_F(SchedulerTest, WaitForReadyToDrawDoNotPostDeadline) {
+  SchedulerClientNeedsPrepareTilesInDraw* client =
+      new SchedulerClientNeedsPrepareTilesInDraw;
+  scheduler_settings_.use_external_begin_frame_source = true;
+  scheduler_settings_.impl_side_painting = true;
+  SetUpScheduler(make_scoped_ptr(client).Pass(), true);
+
+  // SetNeedsCommit should begin the frame on the next BeginImplFrame.
+  scheduler_->SetNeedsCommit();
+  EXPECT_SINGLE_ACTION("SetNeedsBeginFrames(true)", client_);
+  client_->Reset();
+
+  // Begin new frame.
+  EXPECT_SCOPED(AdvanceFrame());
+  scheduler_->NotifyBeginMainFrameStarted();
+  EXPECT_ACTION("WillBeginImplFrame", client_, 0, 2);
+  EXPECT_ACTION("ScheduledActionSendBeginMainFrame", client_, 1, 2);
+
+  client_->Reset();
+  scheduler_->NotifyReadyToCommit();
+  EXPECT_SINGLE_ACTION("ScheduledActionCommit", client_);
+
+  client_->Reset();
+  scheduler_->NotifyReadyToActivate();
+  EXPECT_SINGLE_ACTION("ScheduledActionActivateSyncTree", client_);
+
+  // Set scheduler to wait for ready to draw. Schedule won't post deadline in
+  // the mode.
+  scheduler_->SetWaitForReadyToDraw();
+  client_->Reset();
+  task_runner().RunPendingTasks();  // Try to run posted deadline.
+  // There is no posted deadline.
+  EXPECT_NO_ACTION(client_);
+
+  // Scheduler received ready to draw signal, and posted deadline.
+  scheduler_->NotifyReadyToDraw();
+  client_->Reset();
+  task_runner().RunPendingTasks();  // Run posted deadline.
+  EXPECT_EQ(1, client_->num_draws());
+  EXPECT_TRUE(client_->HasAction("ScheduledActionDrawAndSwapIfPossible"));
+}
+
+TEST_F(SchedulerTest, WaitForReadyToDrawCancelledWhenLostOutputSurface) {
+  SchedulerClientNeedsPrepareTilesInDraw* client =
+      new SchedulerClientNeedsPrepareTilesInDraw;
+  scheduler_settings_.use_external_begin_frame_source = true;
+  scheduler_settings_.impl_side_painting = true;
+  SetUpScheduler(make_scoped_ptr(client).Pass(), true);
+
+  // SetNeedsCommit should begin the frame on the next BeginImplFrame.
+  scheduler_->SetNeedsCommit();
+  EXPECT_SINGLE_ACTION("SetNeedsBeginFrames(true)", client_);
+  client_->Reset();
+
+  // Begin new frame.
+  EXPECT_SCOPED(AdvanceFrame());
+  scheduler_->NotifyBeginMainFrameStarted();
+  EXPECT_ACTION("WillBeginImplFrame", client_, 0, 2);
+  EXPECT_ACTION("ScheduledActionSendBeginMainFrame", client_, 1, 2);
+
+  client_->Reset();
+  scheduler_->NotifyReadyToCommit();
+  EXPECT_SINGLE_ACTION("ScheduledActionCommit", client_);
+
+  client_->Reset();
+  scheduler_->NotifyReadyToActivate();
+  EXPECT_SINGLE_ACTION("ScheduledActionActivateSyncTree", client_);
+
+  // Set scheduler to wait for ready to draw. Schedule won't post deadline in
+  // the mode.
+  scheduler_->SetWaitForReadyToDraw();
+  client_->Reset();
+  task_runner().RunPendingTasks();  // Try to run posted deadline.
+  // There is no posted deadline.
+  EXPECT_NO_ACTION(client_);
+
+  // Scheduler loses output surface, and stops waiting for ready to draw signal.
+  client_->Reset();
+  scheduler_->DidLoseOutputSurface();
+  EXPECT_TRUE(scheduler_->BeginImplFrameDeadlinePending());
+  task_runner().RunPendingTasks();  // Run posted deadline.
+  EXPECT_ACTION("ScheduledActionBeginOutputSurfaceCreation", client_, 0, 3);
+  EXPECT_ACTION("SetNeedsBeginFrames(false)", client_, 1, 3);
+  EXPECT_ACTION("SendBeginMainFrameNotExpectedSoon", client_, 2, 3);
+}
+
 class SchedulerClientWithFixedEstimates : public FakeSchedulerClient {
  public:
   SchedulerClientWithFixedEstimates(
diff --git a/cc/trees/layer_tree_host_impl.cc b/cc/trees/layer_tree_host_impl.cc
index 00ae766..02af12d 100644
--- a/cc/trees/layer_tree_host_impl.cc
+++ b/cc/trees/layer_tree_host_impl.cc
@@ -316,10 +316,18 @@ void LayerTreeHostImpl::CommitComplete() {
     bool update_lcd_text = true;
     sync_tree()->UpdateDrawProperties(update_lcd_text);
     // Start working on newly created tiles immediately if needed.
-    if (tile_manager_ && tile_priorities_dirty_)
+    if (tile_manager_ && tile_priorities_dirty_) {
       PrepareTiles();
-    else
+    } else {
       NotifyReadyToActivate();
+
+      // Ensure we get ReadyToDraw signal even when PrepareTiles not run. This
+      // is important for SingleThreadProxy and impl-side painting case. For
+      // STP, we commit to active tree and RequiresHighResToDraw, and set
+      // Scheduler to wait for ReadyToDraw signal to avoid Checkerboard.
+      if (proxy_->CommitToActiveTree())
+        NotifyReadyToDraw();
+    }
   } else {
     // If we're not in impl-side painting, the tree is immediately considered
     // active.
@@ -862,17 +870,10 @@ DrawResult LayerTreeHostImpl::CalculateRenderPasses(
   if (have_missing_animated_tiles)
     draw_result = DRAW_ABORTED_CHECKERBOARD_ANIMATIONS;
 
-  // When we have a copy request for a layer, we need to draw even if there
-  // would be animating checkerboards, because failing under those conditions
-  // triggers a new main frame, which may cause the copy request layer to be
-  // destroyed.
-  // TODO(danakj): Leaking scheduler internals into LayerTreeHostImpl here.
-  if (have_copy_request)
-    draw_result = DRAW_SUCCESS;
-
   // When we require high res to draw, abort the draw (almost) always. This does
   // not cause the scheduler to do a main frame, instead it will continue to try
   // drawing until we finally complete, so the copy request will not be lost.
+  // TODO(weiliangc): Remove RequiresHighResToDraw. crbug.com/469175
   if (num_incomplete_tiles || num_missing_tiles) {
     if (RequiresHighResToDraw())
       draw_result = DRAW_ABORTED_MISSING_HIGH_RES_CONTENT;
@@ -937,6 +938,15 @@ DrawResult LayerTreeHostImpl::CalculateRenderPasses(
                    "draw_result", draw_result, "missing tiles",
                    num_missing_tiles);
 
+  // Draw has to be successful to not drop the copy request layer.
+  // When we have a copy request for a layer, we need to draw even if there
+  // would be animating checkerboards, because failing under those conditions
+  // triggers a new main frame, which may cause the copy request layer to be
+  // destroyed.
+  // TODO(weiliangc): Test copy request w/ output surface recreation. Would
+  // trigger this DCHECK.
+  DCHECK_IMPLIES(have_copy_request, draw_result == DRAW_SUCCESS);
+
   return draw_result;
 }
 
diff --git a/cc/trees/layer_tree_host_impl.h b/cc/trees/layer_tree_host_impl.h
index 388de5e..81b811d 100644
--- a/cc/trees/layer_tree_host_impl.h
+++ b/cc/trees/layer_tree_host_impl.h
@@ -506,6 +506,8 @@ class CC_EXPORT LayerTreeHostImpl
   void GetPictureLayerImplPairs(std::vector<PictureLayerImpl::Pair>* layers,
                                 bool need_valid_tile_priorities) const;
 
+  // TODO(weiliangc): Replace RequiresHighResToDraw with scheduler waits for
+  // ReadyToDraw. crbug.com/469175
   void SetRequiresHighResToDraw() { requires_high_res_to_draw_ = true; }
   void ResetRequiresHighResToDraw() { requires_high_res_to_draw_ = false; }
   bool RequiresHighResToDraw() const { return requires_high_res_to_draw_; }
diff --git a/cc/trees/layer_tree_host_impl_unittest.cc b/cc/trees/layer_tree_host_impl_unittest.cc
index 818c97e..f80bf34 100644
--- a/cc/trees/layer_tree_host_impl_unittest.cc
+++ b/cc/trees/layer_tree_host_impl_unittest.cc
@@ -2226,47 +2226,33 @@ TEST_F(LayerTreeHostImplTest, PrepareToDrawSucceedsAndFails) {
       PrepareToDrawSuccessTestCase(DRAW_ABORTED_CHECKERBOARD_ANIMATIONS));
   cases.back().layer_between.has_missing_tile = true;
   cases.back().layer_between.is_animating = true;
-  // 11. Animation with missing tile and copy request after. Must succeed
-  // because the animation checkerboard means we'll get a new frame and the copy
-  // request's layer may be destroyed.
-  cases.push_back(PrepareToDrawSuccessTestCase(DRAW_SUCCESS));
-  cases.back().layer_between.has_missing_tile = true;
-  cases.back().layer_between.is_animating = true;
-  cases.back().layer_after.has_copy_request = true;
-  // 12. Animation with missing tile and copy request before. Must succeed
-  // because the animation checkerboard means we'll get a new frame and the copy
-  // request's layer may be destroyed.
-  cases.push_back(PrepareToDrawSuccessTestCase(DRAW_SUCCESS));
-  cases.back().layer_between.has_missing_tile = true;
-  cases.back().layer_between.is_animating = true;
-  cases.back().layer_before.has_copy_request = true;
-  // 13. Animation with incomplete tile.
+  // 11. Animation with incomplete tile.
   cases.push_back(PrepareToDrawSuccessTestCase(DRAW_SUCCESS));
   cases.back().layer_between.has_incomplete_tile = true;
   cases.back().layer_between.is_animating = true;
 
-  // 14. High res required.
+  // 12. High res required.
   cases.push_back(PrepareToDrawSuccessTestCase(DRAW_SUCCESS));
   cases.back().high_res_required = true;
-  // 15. High res required with incomplete tile.
+  // 13. High res required with incomplete tile.
   cases.push_back(
       PrepareToDrawSuccessTestCase(DRAW_ABORTED_MISSING_HIGH_RES_CONTENT));
   cases.back().high_res_required = true;
   cases.back().layer_between.has_incomplete_tile = true;
-  // 16. High res required with missing tile.
+  // 14. High res required with missing tile.
   cases.push_back(
       PrepareToDrawSuccessTestCase(DRAW_ABORTED_MISSING_HIGH_RES_CONTENT));
   cases.back().high_res_required = true;
   cases.back().layer_between.has_missing_tile = true;
 
-  // 17. High res required is higher priority than animating missing tiles.
+  // 15. High res required is higher priority than animating missing tiles.
   cases.push_back(
       PrepareToDrawSuccessTestCase(DRAW_ABORTED_MISSING_HIGH_RES_CONTENT));
   cases.back().high_res_required = true;
   cases.back().layer_between.has_missing_tile = true;
   cases.back().layer_after.has_missing_tile = true;
   cases.back().layer_after.is_animating = true;
-  // 18. High res required is higher priority than animating missing tiles.
+  // 16. High res required is higher priority than animating missing tiles.
   cases.push_back(
       PrepareToDrawSuccessTestCase(DRAW_ABORTED_MISSING_HIGH_RES_CONTENT));
   cases.back().high_res_required = true;
@@ -2274,25 +2260,6 @@ TEST_F(LayerTreeHostImplTest, PrepareToDrawSucceedsAndFails) {
   cases.back().layer_before.has_missing_tile = true;
   cases.back().layer_before.is_animating = true;
 
-  // 19. High res required is higher priority than copy requests.
-  cases.push_back(
-      PrepareToDrawSuccessTestCase(DRAW_ABORTED_MISSING_HIGH_RES_CONTENT));
-  cases.back().high_res_required = true;
-  cases.back().layer_between.has_missing_tile = true;
-  cases.back().layer_after.has_copy_request = true;
-  // 20. High res required is higher priority than copy requests.
-  cases.push_back(
-      PrepareToDrawSuccessTestCase(DRAW_ABORTED_MISSING_HIGH_RES_CONTENT));
-  cases.back().high_res_required = true;
-  cases.back().layer_between.has_missing_tile = true;
-  cases.back().layer_before.has_copy_request = true;
-  // 21. High res required is higher priority than copy requests.
-  cases.push_back(
-      PrepareToDrawSuccessTestCase(DRAW_ABORTED_MISSING_HIGH_RES_CONTENT));
-  cases.back().high_res_required = true;
-  cases.back().layer_between.has_missing_tile = true;
-  cases.back().layer_between.has_copy_request = true;
-
   host_impl_->active_tree()->SetRootLayer(
       DidDrawCheckLayer::Create(host_impl_->active_tree(), 1));
   DidDrawCheckLayer* root =
diff --git a/cc/trees/single_thread_proxy.cc b/cc/trees/single_thread_proxy.cc
index 3633585..3390193 100644
--- a/cc/trees/single_thread_proxy.cc
+++ b/cc/trees/single_thread_proxy.cc
@@ -422,6 +422,10 @@ void SingleThreadProxy::NotifyReadyToActivate() {
 }
 
 void SingleThreadProxy::NotifyReadyToDraw() {
+  TRACE_EVENT0("cc", "SingleThreadProxy::NotifyReadyToDraw");
+  DebugScopedSetImplThread impl(this);
+  if (scheduler_on_impl_thread_)
+    scheduler_on_impl_thread_->NotifyReadyToDraw();
 }
 
 void SingleThreadProxy::SetNeedsRedrawOnImplThread() {
@@ -488,7 +492,8 @@ void SingleThreadProxy::DidActivateSyncTree() {
   if (layer_tree_host_impl_->settings().impl_side_painting) {
     // This is required because NotifyReadyToActivate gets called immediately
     // after commit since single thread commits directly to the active tree.
-    layer_tree_host_impl_->SetRequiresHighResToDraw();
+    if (scheduler_on_impl_thread_)
+      scheduler_on_impl_thread_->SetWaitForReadyToDraw();
 
     // Synchronously call to CommitComplete. Resetting
     // |commit_blocking_task_runner| would make sure all tasks posted during