cc: Remove retro frames from scheduler.

cc/scheduler/scheduler.cc

Index: cc/scheduler/scheduler.cc
diff --git a/cc/scheduler/scheduler.cc b/cc/scheduler/scheduler.cc
index e2a531fc04975334761aafa63bde2401c1748a40..01ce0aaf5298d530db9c018cdd3c38f6a90f28ff 100644
--- a/cc/scheduler/scheduler.cc
+++ b/cc/scheduler/scheduler.cc
@@ -64,8 +64,6 @@ Scheduler::Scheduler(
   DCHECK(client_);
   DCHECK(!state_machine_.BeginFrameNeeded());
 
-  begin_retro_frame_closure_ =
-      base::Bind(&Scheduler::BeginRetroFrame, weak_factory_.GetWeakPtr());
   begin_impl_frame_deadline_closure_ = base::Bind(
       &Scheduler::OnBeginImplFrameDeadline, weak_factory_.GetWeakPtr());
 
@@ -202,8 +200,6 @@ void Scheduler::DidPrepareTiles() {
 
 void Scheduler::DidLoseOutputSurface() {
   TRACE_EVENT0("cc", "Scheduler::DidLoseOutputSurface");
-  begin_retro_frame_args_.clear();
-  begin_retro_frame_task_.Cancel();
   state_machine_.DidLoseOutputSurface();
   UpdateCompositorTimingHistoryRecordingEnabled();
   ProcessScheduledActions();
@@ -261,8 +257,6 @@ void Scheduler::SetupNextBeginFrameIfNeeded() {
                                                        false);
     }
   }
-
-  PostBeginRetroFrameIfNeeded();
 }
 
 void Scheduler::OnBeginFrameSourcePausedChanged(bool paused) {
@@ -274,11 +268,19 @@ void Scheduler::OnBeginFrameSourcePausedChanged(bool paused) {
   ProcessScheduledActions();
 }
 
+const BeginFrameArgs& Scheduler::LastUsedBeginFrameArgs() const {
+  return begin_impl_frame_tracker_.DangerousMethodCurrentOrLast();
+}
+
 // BeginFrame is the mechanism that tells us that now is a good time to start
 // making a frame. Usually this means that user input for the frame is complete.
-// If the scheduler is busy, we queue the BeginFrame to be handled later as
-// a BeginRetroFrame.
-bool Scheduler::OnBeginFrameDerivedImpl(const BeginFrameArgs& args) {
+void Scheduler::OnBeginFrame(const BeginFrameArgs& args) {
+  if (!observing_begin_frame_source_ ||
+      state_machine_.begin_impl_frame_state() !=
+          SchedulerStateMachine::BEGIN_IMPL_FRAME_STATE_IDLE) {
+    return;
+  }
+
   TRACE_EVENT1("cc,benchmark", "Scheduler::BeginFrame", "args", args.AsValue());
 
   // Trace this begin frame time through the Chrome stack
@@ -297,35 +299,9 @@ bool Scheduler::OnBeginFrameDerivedImpl(const BeginFrameArgs& args) {
 
   if (settings_.using_synchronous_renderer_compositor) {
     BeginImplFrameSynchronous(adjusted_args);
-    return true;
-  }
-
-  // We have just called SetNeedsBeginFrame(true) and the BeginFrameSource has
-  // sent us the last BeginFrame we have missed. As we might not be able to
-  // actually make rendering for this call, handle it like a "retro frame".
-  // TODO(brainderson): Add a test for this functionality ASAP!
-  if (adjusted_args.type == BeginFrameArgs::MISSED) {

[enne (OOO), 2016/04/18 21:30:38]

The removal of this might break https://codereview.chromium.org/1879833002/.

-    begin_retro_frame_args_.push_back(adjusted_args);
-    PostBeginRetroFrameIfNeeded();
-    return true;
-  }
-
-  bool should_defer_begin_frame =
-      !begin_retro_frame_args_.empty() ||
-      !begin_retro_frame_task_.IsCancelled() ||
-      !observing_begin_frame_source_ ||
-      (state_machine_.begin_impl_frame_state() !=
-       SchedulerStateMachine::BEGIN_IMPL_FRAME_STATE_IDLE);
-
-  if (should_defer_begin_frame) {
-    begin_retro_frame_args_.push_back(adjusted_args);
-    TRACE_EVENT_INSTANT0(
-        "cc", "Scheduler::BeginFrame deferred", TRACE_EVENT_SCOPE_THREAD);
-    // Queuing the frame counts as "using it", so we need to return true.
   } else {
     BeginImplFrameWithDeadline(adjusted_args);
   }
-  return true;
 }
 
 void Scheduler::SetChildrenNeedBeginFrames(bool children_need_begin_frames) {
@@ -353,80 +329,6 @@ void Scheduler::OnDrawForOutputSurface(bool resourceless_software_draw) {
   state_machine_.SetResourcelessSoftareDraw(false);
 }
 
-// BeginRetroFrame is called for BeginFrames that we've deferred because
-// the scheduler was in the middle of processing a previous BeginFrame.
-void Scheduler::BeginRetroFrame() {
-  TRACE_EVENT0("cc,benchmark", "Scheduler::BeginRetroFrame");
-  DCHECK(!settings_.using_synchronous_renderer_compositor);
-  DCHECK(!begin_retro_frame_args_.empty());
-  DCHECK(!begin_retro_frame_task_.IsCancelled());
-  DCHECK_EQ(state_machine_.begin_impl_frame_state(),
-            SchedulerStateMachine::BEGIN_IMPL_FRAME_STATE_IDLE);
-
-  begin_retro_frame_task_.Cancel();
-
-  // Discard expired BeginRetroFrames
-  // Today, we should always end up with at most one un-expired BeginRetroFrame
-  // because deadlines will not be greater than the next frame time. We don't
-  // DCHECK though because some systems don't always have monotonic timestamps.
-  // TODO(brianderson): In the future, long deadlines could result in us not
-  // draining the queue if we don't catch up. If we consistently can't catch
-  // up, our fallback should be to lower our frame rate.
-  base::TimeTicks now = Now();
-
-  while (!begin_retro_frame_args_.empty()) {
-    const BeginFrameArgs& args = begin_retro_frame_args_.front();
-    base::TimeTicks expiration_time = args.deadline;
-    if (now <= expiration_time)
-      break;
-    TRACE_EVENT_INSTANT2(
-        "cc", "Scheduler::BeginRetroFrame discarding", TRACE_EVENT_SCOPE_THREAD,
-        "expiration_time - now", (expiration_time - now).InMillisecondsF(),
-        "BeginFrameArgs", begin_retro_frame_args_.front().AsValue());
-    begin_retro_frame_args_.pop_front();
-    if (begin_frame_source_)
-      begin_frame_source_->DidFinishFrame(begin_retro_frame_args_.size());
-  }
-
-  if (begin_retro_frame_args_.empty()) {
-    TRACE_EVENT_INSTANT0("cc",
-                         "Scheduler::BeginRetroFrames all expired",
-                         TRACE_EVENT_SCOPE_THREAD);
-  } else {
-    BeginFrameArgs front = begin_retro_frame_args_.front();
-    begin_retro_frame_args_.pop_front();
-    BeginImplFrameWithDeadline(front);
-  }
-}
-
-// There could be a race between the posted BeginRetroFrame and a new
-// BeginFrame arriving via the normal mechanism. Scheduler::BeginFrame
-// will check if there is a pending BeginRetroFrame to ensure we handle
-// BeginFrames in FIFO order.
-void Scheduler::PostBeginRetroFrameIfNeeded() {
-  TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("cc.debug.scheduler"),
-               "Scheduler::PostBeginRetroFrameIfNeeded",
-               "state",
-               AsValue());
-  if (!observing_begin_frame_source_)
-    return;
-
-  if (begin_retro_frame_args_.empty() || !begin_retro_frame_task_.IsCancelled())
-    return;
-
-  // begin_retro_frame_args_ should always be empty for the
-  // synchronous compositor.
-  DCHECK(!settings_.using_synchronous_renderer_compositor);
-
-  if (state_machine_.begin_impl_frame_state() !=
-      SchedulerStateMachine::BEGIN_IMPL_FRAME_STATE_IDLE)
-    return;
-
-  begin_retro_frame_task_.Reset(begin_retro_frame_closure_);
-
-  task_runner_->PostTask(FROM_HERE, begin_retro_frame_task_.callback());
-}
-
 void Scheduler::BeginImplFrameWithDeadline(const BeginFrameArgs& args) {
   bool main_thread_is_in_high_latency_mode =
       state_machine_.main_thread_missed_last_deadline();
@@ -478,16 +380,10 @@ void Scheduler::BeginImplFrameWithDeadline(const BeginFrameArgs& args) {
                                       can_activate_before_deadline)) {
     TRACE_EVENT_INSTANT0("cc", "SkipBeginImplFrameToReduceLatency",
                          TRACE_EVENT_SCOPE_THREAD);
-    if (begin_frame_source_)
-      begin_frame_source_->DidFinishFrame(begin_retro_frame_args_.size());
     return;
   }
 
   BeginImplFrame(adjusted_args);
-
-  // The deadline will be scheduled in ProcessScheduledActions.
-  state_machine_.OnBeginImplFrameDeadlinePending();
-  ProcessScheduledActions();
 }
 
 void Scheduler::BeginImplFrameSynchronous(const BeginFrameArgs& args) {
@@ -510,10 +406,12 @@ void Scheduler::FinishImplFrame() {
   state_machine_.OnBeginImplFrameIdle();
   ProcessScheduledActions();
 
+  begin_impl_frame_tracker_.Finish();
+
   client_->DidFinishImplFrame();
+
   if (begin_frame_source_)
-    begin_frame_source_->DidFinishFrame(begin_retro_frame_args_.size());
-  begin_impl_frame_tracker_.Finish();
+    begin_frame_source_->DidFinishFrame(this);
 }
 
 // BeginImplFrame starts a compositor frame that will wait up until a deadline
@@ -661,85 +559,85 @@ void Scheduler::SetDeferCommits(bool defer_commits) {
 void Scheduler::ProcessScheduledActions() {
   // We do not allow ProcessScheduledActions to be recursive.
   // The top-level call will iteratively execute the next action for us anyway.
-  if (inside_process_scheduled_actions_)
-    return;
+  if (!inside_process_scheduled_actions_) {
+    base::AutoReset<bool> mark_inside(&inside_process_scheduled_actions_, true);
+    SchedulerStateMachine::Action action;
+    do {
+      action = state_machine_.NextAction();
+      PerformAction(action);
+      TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("cc.debug.scheduler"),
+                   "SchedulerStateMachine", "state", AsValue());
+    } while (action != SchedulerStateMachine::ACTION_NONE);
+  }
+  SetupNextBeginFrameIfNeeded();
+  ScheduleBeginImplFrameDeadlineIfNeeded();
+}
 
-  base::AutoReset<bool> mark_inside(&inside_process_scheduled_actions_, true);
-
-  SchedulerStateMachine::Action action;
-  do {
-    action = state_machine_.NextAction();
-    TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("cc.debug.scheduler"),
-                 "SchedulerStateMachine",
-                 "state",
-                 AsValue());
-    base::AutoReset<SchedulerStateMachine::Action>
-        mark_inside_action(&inside_action_, action);
-    switch (action) {
-      case SchedulerStateMachine::ACTION_NONE:
-        break;
-      case SchedulerStateMachine::ACTION_SEND_BEGIN_MAIN_FRAME:
-        compositor_timing_history_->WillBeginMainFrame(
-            begin_main_frame_args_.on_critical_path,
-            begin_main_frame_args_.frame_time);
-        state_machine_.WillSendBeginMainFrame();
-        // TODO(brianderson): Pass begin_main_frame_args_ directly to client.
-        client_->ScheduledActionSendBeginMainFrame(begin_main_frame_args_);
-        break;
-      case SchedulerStateMachine::ACTION_COMMIT: {
-        // TODO(robliao): Remove ScopedTracker below once crbug.com/461509 is
-        // fixed.
-        tracked_objects::ScopedTracker tracking_profile4(
-            FROM_HERE_WITH_EXPLICIT_FUNCTION(
-                "461509 Scheduler::ProcessScheduledActions4"));
-        bool commit_has_no_updates = false;
-        state_machine_.WillCommit(commit_has_no_updates);
-        client_->ScheduledActionCommit();
-        break;
-      }
-      case SchedulerStateMachine::ACTION_ACTIVATE_SYNC_TREE:
-        compositor_timing_history_->WillActivate();
-        state_machine_.WillActivate();
-        client_->ScheduledActionActivateSyncTree();
-        compositor_timing_history_->DidActivate();
-        break;
-      case SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE: {
-        // TODO(robliao): Remove ScopedTracker below once crbug.com/461509 is
-        // fixed.
-        tracked_objects::ScopedTracker tracking_profile6(
-            FROM_HERE_WITH_EXPLICIT_FUNCTION(
-                "461509 Scheduler::ProcessScheduledActions6"));
-        DrawAndSwapIfPossible();
-        break;
-      }
-      case SchedulerStateMachine::ACTION_DRAW_AND_SWAP_FORCED:
-        DrawAndSwapForced();
-        break;
-      case SchedulerStateMachine::ACTION_DRAW_AND_SWAP_ABORT: {
-        // No action is actually performed, but this allows the state machine to
-        // drain the pipeline without actually drawing.
-        state_machine_.AbortDrawAndSwap();
-        compositor_timing_history_->DrawAborted();
-        break;
-      }
-      case SchedulerStateMachine::ACTION_BEGIN_OUTPUT_SURFACE_CREATION:
-        state_machine_.WillBeginOutputSurfaceCreation();
-        client_->ScheduledActionBeginOutputSurfaceCreation();
-        break;
-      case SchedulerStateMachine::ACTION_PREPARE_TILES:
-        state_machine_.WillPrepareTiles();
-        client_->ScheduledActionPrepareTiles();
-        break;
-      case SchedulerStateMachine::ACTION_INVALIDATE_OUTPUT_SURFACE: {
-        state_machine_.WillInvalidateOutputSurface();
-        client_->ScheduledActionInvalidateOutputSurface();
-        break;
-      }
+void Scheduler::PerformAction(SchedulerStateMachine::Action action) {
+  base::AutoReset<SchedulerStateMachine::Action> mark_inside_action(
+      &inside_action_, action);
+  switch (action) {
+    case SchedulerStateMachine::ACTION_NONE:
+      break;
+    case SchedulerStateMachine::ACTION_SEND_BEGIN_MAIN_FRAME:
+      compositor_timing_history_->WillBeginMainFrame(
+          begin_main_frame_args_.on_critical_path,
+          begin_main_frame_args_.frame_time);
+      state_machine_.WillSendBeginMainFrame();
+      // TODO(brianderson): Pass begin_main_frame_args_ directly to client.
+      client_->ScheduledActionSendBeginMainFrame(begin_main_frame_args_);
+      break;
+    case SchedulerStateMachine::ACTION_COMMIT: {
+      // TODO(robliao): Remove ScopedTracker below once crbug.com/461509 is
+      // fixed.
+      tracked_objects::ScopedTracker tracking_profile4(
+          FROM_HERE_WITH_EXPLICIT_FUNCTION(
+              "461509 Scheduler::ProcessScheduledActions4"));
+      bool commit_has_no_updates = false;
+      state_machine_.WillCommit(commit_has_no_updates);
+      client_->ScheduledActionCommit();
+      break;
     }
-  } while (action != SchedulerStateMachine::ACTION_NONE);
-
-  ScheduleBeginImplFrameDeadlineIfNeeded();
-  SetupNextBeginFrameIfNeeded();
+    case SchedulerStateMachine::ACTION_ACTIVATE_SYNC_TREE:
+      compositor_timing_history_->WillActivate();
+      state_machine_.WillActivate();
+      client_->ScheduledActionActivateSyncTree();
+      compositor_timing_history_->DidActivate();
+      break;
+    case SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE: {
+      // TODO(robliao): Remove ScopedTracker below once crbug.com/461509 is
+      // fixed.
+      tracked_objects::ScopedTracker tracking_profile6(
+          FROM_HERE_WITH_EXPLICIT_FUNCTION(
+              "461509 Scheduler::ProcessScheduledActions6"));
+      DrawAndSwapIfPossible();
+      break;
+    }
+    case SchedulerStateMachine::ACTION_DRAW_AND_SWAP_FORCED:
+      DrawAndSwapForced();
+      break;
+    case SchedulerStateMachine::ACTION_DRAW_AND_SWAP_ABORT: {
+      // No action is actually performed, but this allows the state machine
+      // to
+      // drain the pipeline without actually drawing.
+      state_machine_.AbortDrawAndSwap();
+      compositor_timing_history_->DrawAborted();
+      break;
+    }
+    case SchedulerStateMachine::ACTION_BEGIN_OUTPUT_SURFACE_CREATION:
+      state_machine_.WillBeginOutputSurfaceCreation();
+      client_->ScheduledActionBeginOutputSurfaceCreation();
+      break;
+    case SchedulerStateMachine::ACTION_PREPARE_TILES:
+      state_machine_.WillPrepareTiles();
+      client_->ScheduledActionPrepareTiles();
+      break;
+    case SchedulerStateMachine::ACTION_INVALIDATE_OUTPUT_SURFACE: {
+      state_machine_.WillInvalidateOutputSurface();
+      client_->ScheduledActionInvalidateOutputSurface();
+      break;
+    }
+  }
 }
 
 std::unique_ptr<base::trace_event::ConvertableToTraceFormat>
@@ -775,10 +673,6 @@ void Scheduler::AsValueInto(base::trace_event::TracedValue* state) const {
                    estimated_parent_draw_time_.InMillisecondsF());
   state->SetBoolean("observing_begin_frame_source",
                     observing_begin_frame_source_);
-  state->SetInteger("begin_retro_frame_args",
-                    static_cast<int>(begin_retro_frame_args_.size()));
-  state->SetBoolean("begin_retro_frame_task",
-                    !begin_retro_frame_task_.IsCancelled());
   state->SetBoolean("begin_impl_frame_deadline_task",
                     !begin_impl_frame_deadline_task_.IsCancelled());
   state->SetString("inside_action",