cc: Remove call to SetNeedsBeginFrames on losing output surface.

Let the state machine be in charge of deciding when SetNeedsBeginFrames
is called. This allows us to maintain the invariant that
SetNeedsBeginFrames(false) is only called at the end of the frame i.e.
inside the deadline or when output surface is lost.

BUG=444430

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

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

cc/scheduler/scheduler.cc

@@ -275,8 +275,6 @@ void Scheduler::DidLoseOutputSurface() {
   begin_retro_frame_args_.clear();
   begin_retro_frame_task_.Cancel();
   state_machine_.DidLoseOutputSurface();
-  if (frame_source_->NeedsBeginFrames())
-    frame_source_->SetNeedsBeginFrames(false);
   ProcessScheduledActions();
 }
 
@@ -313,41 +311,29 @@ void Scheduler::SetupNextBeginFrameIfNeeded() {
   if (!task_runner_.get())
     return;
 
-  bool needs_begin_frame = state_machine_.BeginFrameNeeded();
-
-  bool at_end_of_deadline =
-      (state_machine_.begin_impl_frame_state() ==
-       SchedulerStateMachine::BEGIN_IMPL_FRAME_STATE_INSIDE_DEADLINE);
-
-  bool should_call_set_needs_begin_frame =
-      // Always request the BeginFrame immediately if it wasn't needed before.
-      (needs_begin_frame && !frame_source_->NeedsBeginFrames()) ||
-      // Only stop requesting BeginFrames after a deadline.
-      (!needs_begin_frame && frame_source_->NeedsBeginFrames() &&
-       at_end_of_deadline);
-
-  if (should_call_set_needs_begin_frame) {
-    frame_source_->SetNeedsBeginFrames(needs_begin_frame);
+  if (state_machine_.ShouldSetNeedsBeginFrames(
+          frame_source_->NeedsBeginFrames())) {
+    frame_source_->SetNeedsBeginFrames(state_machine_.BeginFrameNeeded());
   }
 
-  if (at_end_of_deadline) {
+  if (state_machine_.begin_impl_frame_state() ==
+      SchedulerStateMachine::BEGIN_IMPL_FRAME_STATE_INSIDE_DEADLINE) {
     frame_source_->DidFinishFrame(begin_retro_frame_args_.size());
   }
 
   PostBeginRetroFrameIfNeeded();
-  SetupPollingMechanisms(needs_begin_frame);
+  SetupPollingMechanisms();
 }
 
 // We may need to poll when we can't rely on BeginFrame to advance certain
 // state or to avoid deadlock.
-void Scheduler::SetupPollingMechanisms(bool needs_begin_frame) {
+void Scheduler::SetupPollingMechanisms() {
   bool needs_advance_commit_state_timer = false;
   // Setup PollForAnticipatedDrawTriggers if we need to monitor state but
   // aren't expecting any more BeginFrames. This should only be needed by
   // the synchronous compositor when BeginFrameNeeded is false.
   if (state_machine_.ShouldPollForAnticipatedDrawTriggers()) {
     DCHECK(!state_machine_.SupportsProactiveBeginFrame());
-    DCHECK(!needs_begin_frame);
     if (poll_for_draw_triggers_task_.IsCancelled()) {
       poll_for_draw_triggers_task_.Reset(poll_for_draw_triggers_closure_);
       base::TimeDelta delay = begin_impl_frame_args_.IsValid()

cc/scheduler/scheduler.h

@@ -223,7 +223,7 @@ class CC_EXPORT Scheduler : public BeginFrameObserverMixIn,
   void RescheduleBeginImplFrameDeadlineIfNeeded();
   void SetupNextBeginFrameIfNeeded();
   void PostBeginRetroFrameIfNeeded();
-  void SetupPollingMechanisms(bool needs_begin_frame);
+  void SetupPollingMechanisms();
   void DrawAndSwapIfPossible();
   void ProcessScheduledActions();
   bool CanCommitAndActivateBeforeDeadline() const;

cc/scheduler/scheduler_state_machine.cc

@@ -696,6 +696,11 @@ bool SchedulerStateMachine::BeginFrameNeededForChildren() const {
 }
 
 bool SchedulerStateMachine::BeginFrameNeeded() const {
+  // We can't handle BeginFrames when output surface isn't initialized.
+  // TODO(brianderson): Support output surface creation inside a BeginFrame.
+  if (!HasInitializedOutputSurface())
+    return false;
+
   if (SupportsProactiveBeginFrame()) {
     return (BeginFrameNeededToAnimateOrDraw() ||
             BeginFrameNeededForChildren() ||
@@ -712,6 +717,29 @@ bool SchedulerStateMachine::BeginFrameNeeded() const {
   return BeginFrameNeededToAnimateOrDraw();
 }
 
+bool SchedulerStateMachine::ShouldSetNeedsBeginFrames(
+    bool frame_source_needs_begin_frames) const {
+  bool needs_begin_frame = BeginFrameNeeded();
+
+  // Never call SetNeedsBeginFrames if the frame source has the right value.
+  if (needs_begin_frame == frame_source_needs_begin_frames)
+    return false;
+
+  // Always request the BeginFrame immediately if it's needed.
+  if (needs_begin_frame)
+    return true;
+
+  // Stop requesting BeginFrames after a deadline.
+  if (begin_impl_frame_state_ == BEGIN_IMPL_FRAME_STATE_INSIDE_DEADLINE)
+    return true;
+
+  // Stop requesting BeginFrames immediately when output surface is lost.
+  if (!HasInitializedOutputSurface())
+    return true;
+
+  return false;
+}
+
 bool SchedulerStateMachine::ShouldPollForAnticipatedDrawTriggers() const {
   // ShouldPollForAnticipatedDrawTriggers is what we use in place of
   // ProactiveBeginFrameWanted when we are using the synchronous
@@ -744,11 +772,6 @@ void SchedulerStateMachine::SetChildrenNeedBeginFrames(
 // These are the cases where we definitely (or almost definitely) have a
 // new frame to animate and/or draw and can draw.
 bool SchedulerStateMachine::BeginFrameNeededToAnimateOrDraw() const {
-  // The output surface is the provider of BeginImplFrames, so we are not going
-  // to get them even if we ask for them.
-  if (!HasInitializedOutputSurface())
-    return false;
-
   // The forced draw respects our normal draw scheduling, so we need to
   // request a BeginImplFrame for it.
   if (forced_redraw_state_ == FORCED_REDRAW_STATE_WAITING_FOR_DRAW)
@@ -762,11 +785,6 @@ bool SchedulerStateMachine::BeginFrameNeededToAnimateOrDraw() const {
 // Proactively requesting the BeginImplFrame helps hide the round trip latency
 // of the SetNeedsBeginFrame request that has to go to the Browser.
 bool SchedulerStateMachine::ProactiveBeginFrameWanted() const {
-  // The output surface is the provider of BeginImplFrames,
-  // so we are not going to get them even if we ask for them.
-  if (!HasInitializedOutputSurface())
-    return false;
-
   // Do not be proactive when invisible.
   if (!visible_)
     return false;

cc/scheduler/scheduler_state_machine.h

@@ -124,6 +124,10 @@ class CC_EXPORT SchedulerStateMachine {
   // to make progress.
   bool BeginFrameNeeded() const;
 
+  // Indicates whether the scheduler should call
+  // SetNeedsBeginFrames(BeginFrameNeeded()) on the frame source.
+  bool ShouldSetNeedsBeginFrames(bool frame_source_needs_begin_frames) const;
+
   // Indicates that we need to independently poll for new state and actions
   // because we can't expect a BeginImplFrame. This is mostly used to avoid
   // drawing repeat frames with the synchronous compositor without dropping

cc/scheduler/scheduler_unittest.cc

@@ -1827,8 +1827,8 @@ void DidLoseOutputSurfaceAfterReadyToCommit(bool impl_side_painting) {
   scheduler->DidLoseOutputSurface();
   if (impl_side_painting) {
     // Sync tree should be forced to activate.
-    EXPECT_ACTION("SetNeedsBeginFrames(false)", client, 0, 2);
-    EXPECT_ACTION("ScheduledActionActivateSyncTree", client, 1, 2);
+    EXPECT_ACTION("ScheduledActionActivateSyncTree", client, 0, 2);
+    EXPECT_ACTION("SetNeedsBeginFrames(false)", client, 1, 2);
   } else {
     EXPECT_SINGLE_ACTION("SetNeedsBeginFrames(false)", client);
   }
@@ -1919,8 +1919,8 @@ TEST(SchedulerTest, DidLoseOutputSurfaceAfterBeginRetroFramePosted) {
   client.Reset();
   EXPECT_FALSE(scheduler->IsBeginRetroFrameArgsEmpty());
   scheduler->DidLoseOutputSurface();
-  EXPECT_ACTION("SetNeedsBeginFrames(false)", client, 0, 2);
-  EXPECT_ACTION("ScheduledActionBeginOutputSurfaceCreation", client, 1, 2);
+  EXPECT_ACTION("ScheduledActionBeginOutputSurfaceCreation", client, 0, 2);
+  EXPECT_ACTION("SetNeedsBeginFrames(false)", client, 1, 2);
   EXPECT_TRUE(scheduler->IsBeginRetroFrameArgsEmpty());
 
   // Posted BeginRetroFrame is aborted.