cc: Throttle incoming begin frames in scheduler

Unlike PostTask from IO thread to compositor thread in Chrome IPC, mojo
polls for messages on the compositor thread which means it can dequeue a
large number of begin frame messages after the compositor thread has
been busy for some time. All but the last begin frame cancels the
previous begin frame, and is essentially a nop, but it still ticks
animations. When a page has a large number of animations each begin
frame can take a long time and push out other tasks such as tile manager
callbacks stalling the pipeline.

Throttling the begin frames in viz doesn't fully solve the problem
because we have to allow at least two begin frames in flight for
pipelining, and so the client can still process two begin frames back to
back.

Throttling in AsyncLTFS causes issues with LTFS lifetime and ordering
with respect to other messages.

Saving incoming begin frame in scheduler and posting a task works and
ensures that only one begin frame is outstanding at any time.

R=danakj
BUG=782002

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I247a87ad7475d33f878a215ce87056d20482f88c
Reviewed-on: https://chromium-review.googlesource.com/1130082
Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577046}

cc/scheduler/scheduler.h

@@ -193,6 +193,7 @@ class CC_EXPORT Scheduler : public viz::BeginFrameObserverBase {
 
   std::unique_ptr<CompositorTimingHistory> compositor_timing_history_;
 
+  // What the latest deadline was, and when it was scheduled.
   SchedulerStateMachine::BeginImplFrameDeadlineMode
       begin_impl_frame_deadline_mode_ =
           SchedulerStateMachine::BeginImplFrameDeadlineMode::NONE;
@@ -202,9 +203,18 @@ class CC_EXPORT Scheduler : public viz::BeginFrameObserverBase {
   BeginFrameTracker begin_impl_frame_tracker_;
   viz::BeginFrameArgs begin_main_frame_args_;
 
-  base::Closure begin_impl_frame_deadline_closure_;
-  base::CancelableClosure begin_impl_frame_deadline_task_;
-  base::CancelableClosure missed_begin_frame_task_;
+  // Task posted for the deadline or drawing phase of the scheduler. This task
+  // can be rescheduled e.g. when the condition for the deadline is met, it is
+  // scheduled to run immediately.
+  // NOTE: Scheduler weak ptrs are not necessary if CancelableCallback is used.
+  base::CancelableOnceClosure begin_impl_frame_deadline_task_;
+
+  // This is used for queueing begin frames while scheduler is waiting for
+  // previous frame's deadline, or if it's inside ProcessScheduledActions().
+  // Only one such task is posted at any time, but the args are updated as we
+  // get new begin frames.
+  viz::BeginFrameArgs pending_begin_frame_args_;
+  base::CancelableOnceClosure pending_begin_frame_task_;
 
   SchedulerStateMachine state_machine_;
   bool inside_process_scheduled_actions_ = false;
@@ -215,11 +225,32 @@ class CC_EXPORT Scheduler : public viz::BeginFrameObserverBase {
   bool stopped_ = false;
 
  private:
+  // Posts the deadline task if needed by checking
+  // SchedulerStateMachine::CurrentBeginImplFrameDeadlineMode(). This only
+  // happens when the scheduler is processing a begin frame
+  // (BeginImplFrameState::INSIDE_BEGIN_FRAME).
   void ScheduleBeginImplFrameDeadline();
-  void ScheduleBeginImplFrameDeadlineIfNeeded();
+
+  // Starts or stops begin frames as needed by checking
+  // SchedulerStateMachine::BeginFrameNeeded(). This only happens when the
+  // scheduler is not processing a begin frame (BeginImplFrameState::IDLE).
+  void StartOrStopBeginFrames();
+
+  // This will only post a task if the args are valid and there's no existing
+  // task. That implies that we're still expecting begin frames. If begin frames
+  // aren't needed this will be a nop. This only happens when the scheduler is
+  // not processing a begin frame (BeginImplFrameState::IDLE).
+  void PostPendingBeginFrameTask();
+
+  // Use |pending_begin_frame_args_| to begin a new frame like it was received
+  // in OnBeginFrameDerivedImpl().
+  void HandlePendingBeginFrame();
+
+  // Used to drop the pending begin frame before we go idle.
+  void CancelPendingBeginFrameTask();
+
   void BeginImplFrameNotExpectedSoon();
   void BeginMainFrameNotExpectedUntil(base::TimeTicks time);
-  void SetupNextBeginFrameIfNeeded();
   void DrawIfPossible();
   void DrawForced();
   void ProcessScheduledActions();
@@ -235,6 +266,7 @@ class CC_EXPORT Scheduler : public viz::BeginFrameObserverBase {
       base::TimeTicks now) const;
   void AdvanceCommitStateIfPossible();
   bool IsBeginMainFrameSentOrStarted() const;
+
   void BeginImplFrameWithDeadline(const viz::BeginFrameArgs& args);
   void BeginImplFrameSynchronous(const viz::BeginFrameArgs& args);
   void BeginImplFrame(const viz::BeginFrameArgs& args, base::TimeTicks now);
@@ -250,8 +282,6 @@ class CC_EXPORT Scheduler : public viz::BeginFrameObserverBase {
     return inside_action_ == action;
   }
 
-  base::WeakPtrFactory<Scheduler> weak_factory_;
-
   DISALLOW_COPY_AND_ASSIGN(Scheduler);
 };
 

cc/scheduler/scheduler_state_machine.cc

@@ -1076,12 +1076,18 @@ void SchedulerStateMachine::OnBeginImplFrameIdle() {
 
 SchedulerStateMachine::BeginImplFrameDeadlineMode
 SchedulerStateMachine::CurrentBeginImplFrameDeadlineMode() const {
-  if (settings_.using_synchronous_renderer_compositor) {
-    // No deadline for synchronous compositor.
+  const bool outside_begin_frame =
+      begin_impl_frame_state_ != BeginImplFrameState::INSIDE_BEGIN_FRAME;
+  if (settings_.using_synchronous_renderer_compositor || outside_begin_frame) {
+    // No deadline for synchronous compositor, or when outside the begin frame.
     return BeginImplFrameDeadlineMode::NONE;
   } else if (ShouldBlockDeadlineIndefinitely()) {
+    // We do not want to wait for a deadline because we're waiting for full
+    // pipeline to be flushed for headless.
     return BeginImplFrameDeadlineMode::BLOCKED;
   } else if (ShouldTriggerBeginImplFrameDeadlineImmediately()) {
+    // We are ready to draw a new active tree immediately because there's no
+    // commit expected or we're prioritizing active tree latency.
     return BeginImplFrameDeadlineMode::IMMEDIATE;
   } else if (needs_redraw_) {
     // We have an animation or fast input path on the impl thread that wants
@@ -1089,7 +1095,7 @@ SchedulerStateMachine::CurrentBeginImplFrameDeadlineMode() const {
     return BeginImplFrameDeadlineMode::REGULAR;
   } else {
     // The impl thread doesn't have anything it wants to draw and we are just
-    // waiting for a new active tree. In short we are blocked.
+    // waiting for a new active tree.
     return BeginImplFrameDeadlineMode::LATE;
   }
 }

cc/scheduler/scheduler_state_machine.h

@@ -69,12 +69,18 @@ class CC_EXPORT SchedulerStateMachine {
   };
   static const char* BeginImplFrameStateToString(BeginImplFrameState state);
 
+  // The scheduler uses a deadline to wait for main thread updates before
+  // submitting a compositor frame. BeginImplFrameDeadlineMode specifies when
+  // the deadline should run.
   enum class BeginImplFrameDeadlineMode {
-    NONE,
-    IMMEDIATE,
-    REGULAR,
-    LATE,
-    BLOCKED,
+    NONE,  // No deadline should be scheduled e.g. for synchronous compositor.
+    IMMEDIATE,  // Deadline should be scheduled to run immediately.
+    REGULAR,  // Deadline should be scheduled to run at the deadline provided by
+              // in the BeginFrameArgs.
+    LATE,  // Deadline should be scheduled run when the next frame is expected
+           // to arrive.
+    BLOCKED,  // Deadline should be blocked indefinitely until the next frame
+              // arrives.
   };
   static const char* BeginImplFrameDeadlineModeToString(
       BeginImplFrameDeadlineMode mode);
@@ -168,6 +174,8 @@ class CC_EXPORT SchedulerStateMachine {
   BeginImplFrameState begin_impl_frame_state() const {
     return begin_impl_frame_state_;
   }
+
+  // Returns BeginImplFrameDeadlineMode computed based on current state.
   BeginImplFrameDeadlineMode CurrentBeginImplFrameDeadlineMode() const;
 
   // If the main thread didn't manage to produce a new frame in time for the
@@ -320,13 +328,20 @@ class CC_EXPORT SchedulerStateMachine {
   bool BeginFrameNeededForVideo() const;
   bool ProactiveBeginFrameWanted() const;
 
+  // Indicates if we should post the deadline to draw immediately. This is true
+  // when we aren't expecting a commit or activation, or we're prioritizing
+  // active tree draw (see ImplLatencyTakesPriority()).
+  bool ShouldTriggerBeginImplFrameDeadlineImmediately() const;
+
+  // Indicates if we shouldn't schedule a deadline. Used to defer drawing until
+  // the entire pipeline is flushed and active tree is ready to draw for
+  // headless.
+  bool ShouldBlockDeadlineIndefinitely() const;
+
   bool ShouldPerformImplSideInvalidation() const;
   bool CouldCreatePendingTree() const;
   bool ShouldDeferInvalidatingForMainFrame() const;
 
-  bool ShouldTriggerBeginImplFrameDeadlineImmediately() const;
-  bool ShouldBlockDeadlineIndefinitely() const;
-
   bool ShouldAbortCurrentFrame() const;
 
   bool ShouldBeginLayerTreeFrameSinkCreation() const;

cc/scheduler/scheduler_unittest.cc

@@ -511,6 +511,9 @@ class SchedulerTest : public testing::Test {
     if (scheduler_->begin_frame_source() ==
         fake_external_begin_frame_source_.get()) {
       EXPECT_TRUE(scheduler_->begin_frames_expected());
+      // Run the deadline first if we're inside the previous frame.
+      if (client_->IsInsideBeginImplFrame())
+        task_runner_->RunPendingTasks();
       SendNextBeginFrame(animate_only);
     } else {
       task_runner_->RunTasksWhile(client_->FrameHasNotAdvancedCallback());
@@ -3678,10 +3681,11 @@ TEST_F(SchedulerTest, BeginFrameAckForBeginFrameBeforeLastDeadline) {
   client_->Reset();
 
   // Send the next BeginFrame before the previous one's deadline was executed.
-  // This should trigger the previous BeginFrame's deadline synchronously,
-  // during which tiles will be prepared. As a result of that, no further
-  // BeginFrames will be needed, and the new BeginFrame should be dropped.
+  // This should post the previous BeginFrame's deadline, during which tiles
+  // will be prepared. As a result of that, no further BeginFrames will be
+  // needed, and the new BeginFrame should be dropped.
   viz::BeginFrameArgs args = SendNextBeginFrame();
+  task_runner_->RunPendingTasks();  // Run posted deadline.
   EXPECT_ACTIONS("ScheduledActionPrepareTiles", "RemoveObserver(this)");
   EXPECT_FALSE(client_->IsInsideBeginImplFrame());
   EXPECT_FALSE(scheduler_->begin_frames_expected());
@@ -3748,6 +3752,7 @@ TEST_F(SchedulerTest, BeginFrameAckForLateMissedBeginFrame) {
   task_runner_->AdvanceMockTickClock(viz::BeginFrameArgs::DefaultInterval());
   EXPECT_GT(task_runner_->NowTicks(), args.deadline);
   fake_external_begin_frame_source_->TestOnBeginFrame(args);
+  task_runner_->RunPendingTasks();
 
   EXPECT_NO_ACTION();
   EXPECT_FALSE(client_->IsInsideBeginImplFrame());