cc: Handle retroactive BeginFrames in the Scheduler.

cc/scheduler/scheduler.cc

Index: cc/scheduler/scheduler.cc
diff --git a/cc/scheduler/scheduler.cc b/cc/scheduler/scheduler.cc
index ce80784c638429f94b9b80aeab283adec3293689..94cc1c5221f2c7548a839b62c66a0a02af2d4e17 100644
--- a/cc/scheduler/scheduler.cc
+++ b/cc/scheduler/scheduler.cc
@@ -23,13 +23,14 @@ Scheduler::Scheduler(
       client_(client),
       layer_tree_host_id_(layer_tree_host_id),
       impl_task_runner_(impl_task_runner),
-      last_set_needs_begin_impl_frame_(false),
+      last_set_needs_begin_frame_(false),
+      begin_retro_frame_posted_(false),
       state_machine_(scheduler_settings),
       inside_process_scheduled_actions_(false),
       inside_action_(SchedulerStateMachine::ACTION_NONE),
       weak_factory_(this) {
   DCHECK(client_);
-  DCHECK(!state_machine_.BeginImplFrameNeeded());
+  DCHECK(!state_machine_.BeginFrameNeeded());
   if (settings_.main_frame_before_activation_enabled) {
     DCHECK(settings_.main_frame_before_draw_enabled);
   }
@@ -116,13 +117,14 @@ void Scheduler::DidManageTiles() {
 void Scheduler::DidLoseOutputSurface() {
   TRACE_EVENT0("cc", "Scheduler::DidLoseOutputSurface");
   state_machine_.DidLoseOutputSurface();
-  last_set_needs_begin_impl_frame_ = false;
+  last_set_needs_begin_frame_ = false;
+  begin_retro_frame_args_.clear();
   ProcessScheduledActions();
 }
 
 void Scheduler::DidCreateAndInitializeOutputSurface() {
   TRACE_EVENT0("cc", "Scheduler::DidCreateAndInitializeOutputSurface");
-  DCHECK(!last_set_needs_begin_impl_frame_);
+  DCHECK(!last_set_needs_begin_frame_);
   DCHECK(begin_impl_frame_deadline_closure_.IsCancelled());
   state_machine_.DidCreateAndInitializeOutputSurface();
   ProcessScheduledActions();
@@ -134,49 +136,58 @@ void Scheduler::NotifyBeginMainFrameStarted() {
 }
 
 base::TimeTicks Scheduler::AnticipatedDrawTime() const {
-  if (!last_set_needs_begin_impl_frame_ ||
-      last_begin_impl_frame_args_.interval <= base::TimeDelta())
+  if (!last_set_needs_begin_frame_ ||
+      begin_impl_frame_args_.interval <= base::TimeDelta())
     return base::TimeTicks();
 
   base::TimeTicks now = gfx::FrameTime::Now();
-  base::TimeTicks timebase = std::max(last_begin_impl_frame_args_.frame_time,
-                                      last_begin_impl_frame_args_.deadline);
-  int64 intervals =
-      1 + ((now - timebase) / last_begin_impl_frame_args_.interval);
-  return timebase + (last_begin_impl_frame_args_.interval * intervals);
+  base::TimeTicks timebase = std::max(begin_impl_frame_args_.frame_time,
+                                      begin_impl_frame_args_.deadline);
+  int64 intervals = 1 + ((now - timebase) / begin_impl_frame_args_.interval);
+  return timebase + (begin_impl_frame_args_.interval * intervals);
 }
 
 base::TimeTicks Scheduler::LastBeginImplFrameTime() {
-  return last_begin_impl_frame_args_.frame_time;
+  return begin_impl_frame_args_.frame_time;
 }
 
-void Scheduler::SetupNextBeginImplFrameIfNeeded() {
-  bool needs_begin_impl_frame =
-      state_machine_.BeginImplFrameNeeded();
+void Scheduler::SetupNextBeginFrameIfNeeded() {
+  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_impl_frame =
-      // Always request the BeginImplFrame immediately if it wasn't needed
-      // before.
-      (needs_begin_impl_frame && !last_set_needs_begin_impl_frame_) ||
-      // We always need to explicitly request our next BeginImplFrame.
-      at_end_of_deadline;
+  bool should_call_set_needs_begin_frame =
+      // Always request the BeginFrame immediately if it wasn't needed before.
+      (needs_begin_frame && !last_set_needs_begin_frame_) ||
+      // Only stop requesting the BeginFrame at the end of a deadline.
+      ((!needs_begin_frame && last_set_needs_begin_frame_) &&
+       at_end_of_deadline);
 
-  if (should_call_set_needs_begin_impl_frame) {
-    client_->SetNeedsBeginImplFrame(needs_begin_impl_frame);
-    last_set_needs_begin_impl_frame_ = needs_begin_impl_frame;
+  if (should_call_set_needs_begin_frame) {
+    client_->SetNeedsBeginFrame(needs_begin_frame);
+    last_set_needs_begin_frame_ = needs_begin_frame;
+  }
+
+  // Handle retroactive BeginFrames.
+  if (needs_begin_frame && !begin_retro_frame_posted_ &&
+      !begin_retro_frame_args_.empty() &&
+      state_machine_.begin_impl_frame_state() ==
+          SchedulerStateMachine::BEGIN_IMPL_FRAME_STATE_IDLE) {
+    begin_retro_frame_posted_ = true;
+    impl_task_runner_->PostTask(
+        FROM_HERE,
+        base::Bind(&Scheduler::BeginRetroFrame, weak_factory_.GetWeakPtr()));
   }
 
   bool needs_advance_commit_state_timer = false;
   // Setup PollForAnticipatedDrawTriggers if we need to monitor state but
-  // aren't expecting any more BeginImplFrames. This should only be needed by
-  // the synchronous compositor when BeginImplFrameNeeded is false.
+  // 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_.SupportsProactiveBeginImplFrame());
-    DCHECK(!needs_begin_impl_frame);
+    DCHECK(!state_machine_.SupportsProactiveBeginFrame());
+    DCHECK(!needs_begin_frame);
     if (poll_for_draw_triggers_closure_.IsCancelled()) {
       poll_for_draw_triggers_closure_.Reset(
           base::Bind(&Scheduler::PollForAnticipatedDrawTriggers,
@@ -184,17 +195,17 @@ void Scheduler::SetupNextBeginImplFrameIfNeeded() {
       impl_task_runner_->PostDelayedTask(
           FROM_HERE,
           poll_for_draw_triggers_closure_.callback(),
-          last_begin_impl_frame_args_.interval);
+          begin_impl_frame_args_.interval);
     }
   } else {
     poll_for_draw_triggers_closure_.Cancel();
 
     // At this point we'd prefer to advance through the commit flow by
     // drawing a frame, however it's possible that the frame rate controller
-    // will not give us a BeginImplFrame until the commit completes.  See
+    // will not give us a BeginFrame until the commit completes.  See
     // crbug.com/317430 for an example of a swap ack being held on commit. Thus
     // we set a repeating timer to poll on ProcessScheduledActions until we
-    // successfully reach BeginImplFrame. Synchronous compositor does not use
+    // successfully reach BeginFrame. Synchronous compositor does not use
     // frame rate controller or have the circular wait in the bug.
     if (IsBeginMainFrameSentOrStarted() &&
         !settings_.using_synchronous_renderer_compositor) {
@@ -203,13 +214,12 @@ void Scheduler::SetupNextBeginImplFrameIfNeeded() {
   }
   if (needs_advance_commit_state_timer !=
       advance_commit_state_timer_.IsRunning()) {
-    if (needs_advance_commit_state_timer &&
-        last_begin_impl_frame_args_.IsValid()) {
+    if (needs_advance_commit_state_timer && begin_impl_frame_args_.IsValid()) {
       // Since we'd rather get a BeginImplFrame by the normal mechanism, we
       // set the interval to twice the interval from the previous frame.
       advance_commit_state_timer_.Start(
           FROM_HERE,
-          last_begin_impl_frame_args_.interval * 2,
+          begin_impl_frame_args_.interval * 2,
           base::Bind(&Scheduler::ProcessScheduledActions,
                      base::Unretained(this)));
     } else {
@@ -218,22 +228,58 @@ void Scheduler::SetupNextBeginImplFrameIfNeeded() {
   }
 }
 
+void Scheduler::BeginFrame(const BeginFrameArgs& args) {
+  if (last_set_needs_begin_frame_ && begin_retro_frame_args_.empty() &&
+      state_machine_.begin_impl_frame_state() ==
+          SchedulerStateMachine::BEGIN_IMPL_FRAME_STATE_IDLE) {
+    BeginImplFrame(args);
+  } else {
+    begin_retro_frame_args_.push_back(args);
+    TRACE_EVENT_INSTANT0(
+        "cc", "Scheduler::BeginFrame deferred", TRACE_EVENT_SCOPE_THREAD);
+  }
+}
+
+void Scheduler::BeginRetroFrame() {
+  TRACE_EVENT0("cc", "Scheduler::BeginRetroFrame");
+  DCHECK(!begin_retro_frame_args_.empty());
+
+  // Discard expired BeginRetroFrames
+  base::TimeTicks now = gfx::FrameTime::Now();
+  while (!begin_retro_frame_args_.empty() &&
+         now <

[brianderson, 2014/04/02 16:57:49]

Yay for unit tests. This should have been a >. Fixed in most recent patch.

+             AdjustedBeginImplFrameDeadline(begin_retro_frame_args_.front())) {
+    begin_retro_frame_args_.pop_front();
+  }
+
+  if (begin_retro_frame_args_.empty()) {
+    TRACE_EVENT_INSTANT0(
+        "cc", "Scheduler::BeginRetroFrames expired", TRACE_EVENT_SCOPE_THREAD);
+  } else {
+    BeginImplFrame(begin_retro_frame_args_.front());
+    begin_retro_frame_args_.pop_front();
+  }
+
+  begin_retro_frame_posted_ = false;
+}
+
 void Scheduler::BeginImplFrame(const BeginFrameArgs& args) {
   TRACE_EVENT0("cc", "Scheduler::BeginImplFrame");
   DCHECK(state_machine_.begin_impl_frame_state() ==
          SchedulerStateMachine::BEGIN_IMPL_FRAME_STATE_IDLE);
   DCHECK(state_machine_.HasInitializedOutputSurface());
 
-  last_begin_impl_frame_args_ = args;
-  last_begin_impl_frame_args_.deadline -= client_->DrawDurationEstimate();
-
   if (!state_machine_.smoothness_takes_priority() &&
       state_machine_.MainThreadIsInHighLatencyMode() &&
       CanCommitAndActivateBeforeDeadline()) {
     state_machine_.SetSkipNextBeginMainFrameToReduceLatency();
   }
 
-  state_machine_.OnBeginImplFrame(last_begin_impl_frame_args_);
+  begin_impl_frame_args_ = args;
+  begin_impl_frame_args_.deadline -= client_->DrawDurationEstimate();
+
+  client_->WillBeginImplFrame(begin_impl_frame_args_);
+  state_machine_.OnBeginImplFrame(begin_impl_frame_args_);
   devtools_instrumentation::DidBeginFrame(layer_tree_host_id_);
 
   ProcessScheduledActions();
@@ -242,11 +288,11 @@ void Scheduler::BeginImplFrame(const BeginFrameArgs& args) {
     return;
 
   state_machine_.OnBeginImplFrameDeadlinePending();
-  base::TimeTicks adjusted_deadline = AdjustedBeginImplFrameDeadline();
-  ScheduleBeginImplFrameDeadline(adjusted_deadline);
+  ScheduleBeginImplFrameDeadline(AdjustedBeginImplFrameDeadline(args));
 }
 
-base::TimeTicks Scheduler::AdjustedBeginImplFrameDeadline() const {
+base::TimeTicks Scheduler::AdjustedBeginImplFrameDeadline(
+    const BeginFrameArgs& args) const {
   if (settings_.using_synchronous_renderer_compositor) {
     // The synchronous compositor needs to draw right away.
     return base::TimeTicks();
@@ -256,7 +302,7 @@ base::TimeTicks Scheduler::AdjustedBeginImplFrameDeadline() const {
   } else if (state_machine_.needs_redraw()) {
     // We have an animation or fast input path on the impl thread that wants
     // to draw, so don't wait too long for a new active tree.
-    return last_begin_impl_frame_args_.deadline;
+    return args.deadline - client_->DrawDurationEstimate();
   } else {
     // The impl thread doesn't have anything it wants to draw and we are just
     // waiting for a new active tree, so post the deadline for the next
@@ -265,8 +311,7 @@ base::TimeTicks Scheduler::AdjustedBeginImplFrameDeadline() const {
     // BeginImplFrame.
     // TODO(brianderson): Handle long deadlines (that are past the next frame's
     // frame time) properly instead of using this hack.
-    return last_begin_impl_frame_args_.frame_time +
-           last_begin_impl_frame_args_.interval;
+    return args.frame_time + args.interval;
   }
 }
 
@@ -398,7 +443,7 @@ void Scheduler::ProcessScheduledActions() {
     }
   } while (action != SchedulerStateMachine::ACTION_NONE);
 
-  SetupNextBeginImplFrameIfNeeded();
+  SetupNextBeginFrameIfNeeded();
   client_->DidAnticipatedDrawTimeChange(AnticipatedDrawTime());
 
   if (state_machine_.ShouldTriggerBeginImplFrameDeadlineEarly()) {
@@ -435,19 +480,19 @@ bool Scheduler::CanCommitAndActivateBeforeDeadline() const {
   // Check if the main thread computation and commit can be finished before the
   // impl thread's deadline.
   base::TimeTicks estimated_draw_time =
-      last_begin_impl_frame_args_.frame_time +
+      begin_impl_frame_args_.frame_time +
       client_->BeginMainFrameToCommitDurationEstimate() +
       client_->CommitToActivateDurationEstimate();
 
-  TRACE_EVENT2(TRACE_DISABLED_BY_DEFAULT("cc.debug.scheduler"),
-               "CanCommitAndActivateBeforeDeadline",
-               "time_left_after_drawing_ms",
-               (last_begin_impl_frame_args_.deadline - estimated_draw_time)
-                   .InMillisecondsF(),
-               "state",
-               TracedValue::FromValue(StateAsValue().release()));
+  TRACE_EVENT2(
+      TRACE_DISABLED_BY_DEFAULT("cc.debug.scheduler"),
+      "CanCommitAndActivateBeforeDeadline",
+      "time_left_after_drawing_ms",
+      (begin_impl_frame_args_.deadline - estimated_draw_time).InMillisecondsF(),
+      "state",
+      TracedValue::FromValue(StateAsValue().release()));
 
-  return estimated_draw_time < last_begin_impl_frame_args_.deadline;
+  return estimated_draw_time < begin_impl_frame_args_.deadline;
 }
 
 bool Scheduler::IsBeginMainFrameSentOrStarted() const {