cc: Heuristic for Renderer latency recovery

cc/scheduler/scheduler_unittest.cc

Index: cc/scheduler/scheduler_unittest.cc
diff --git a/cc/scheduler/scheduler_unittest.cc b/cc/scheduler/scheduler_unittest.cc
index 29505e5a9b76197de307feae951fb44f0565c5c0..9346ad598f175b36bd7353f2f8fbe55402e7ac12 100644
--- a/cc/scheduler/scheduler_unittest.cc
+++ b/cc/scheduler/scheduler_unittest.cc
@@ -246,6 +246,16 @@ class SchedulerTest : public testing::Test {
         fake_compositor_timing_history.Pass());
     DCHECK(scheduler_);
     client_->set_scheduler(scheduler_.get());
+
+    // Use large estimates by default to avoid latency recovery
+    // in most tests.
+    base::TimeDelta one_second = base::TimeDelta::FromSeconds(1);
+    fake_compositor_timing_history_->SetDrawDurationEstimate(one_second);
+    fake_compositor_timing_history_->SetBeginMainFrameToCommitDurationEstimate(
+        one_second);
+    fake_compositor_timing_history_->SetCommitToReadyToActivateDurationEstimate(
+        one_second);
+
     return scheduler_.get();
   }
 
@@ -389,6 +399,12 @@ class SchedulerTest : public testing::Test {
       int64 commit_to_activate_estimate_in_ms,
       bool impl_latency_takes_priority,
       bool should_send_begin_main_frame);
+  void ImplFrameInHighLatency(int64 draw_estimate_in_ms,
+                              int64 begin_main_frame_to_commit_estimate_in_ms,
+                              int64 commit_to_activate_estimate_in_ms,
+                              bool has_impl_side_updates,
+                              bool swap_ack_before_deadline,
+                              bool should_draw_and_swap_in_high_latency);
   void BeginFramesNotFromClient(bool use_external_begin_frame_source,
                                 bool throttle_frame_production);
   void BeginFramesNotFromClient_SwapThrottled(
@@ -1335,29 +1351,360 @@ void SchedulerTest::MainFrameInHighLatencyMode(
 TEST_F(SchedulerTest,
        SkipMainFrameIfHighLatencyAndCanCommitAndActivateBeforeDeadline) {
   // Set up client so that estimates indicate that we can commit and activate
-  // before the deadline (~8ms by default).
+  // before the deadline (~11ms by default).
   EXPECT_SCOPED(MainFrameInHighLatencyMode(1, 1, false, false));
 }
 
 TEST_F(SchedulerTest, NotSkipMainFrameIfHighLatencyAndCanCommitTooLong) {
   // Set up client so that estimates indicate that the commit cannot finish
-  // before the deadline (~8ms by default).
+  // before the deadline (~11ms by default).
   EXPECT_SCOPED(MainFrameInHighLatencyMode(10, 1, false, true));
 }
 
 TEST_F(SchedulerTest, NotSkipMainFrameIfHighLatencyAndCanActivateTooLong) {
   // Set up client so that estimates indicate that the activate cannot finish
-  // before the deadline (~8ms by default).
+  // before the deadline (~11ms by default).
   EXPECT_SCOPED(MainFrameInHighLatencyMode(1, 10, false, true));
 }
 
 TEST_F(SchedulerTest, NotSkipMainFrameInPreferImplLatencyMode) {
   // Set up client so that estimates indicate that we can commit and activate
-  // before the deadline (~8ms by default), but also enable impl latency takes
+  // before the deadline (~11ms by default), but also enable impl latency takes
   // priority mode.
   EXPECT_SCOPED(MainFrameInHighLatencyMode(1, 1, true, true));
 }
 
+void SchedulerTest::ImplFrameInHighLatency(
+    int64 draw_estimate_in_ms,
+    int64 begin_main_frame_to_commit_estimate_in_ms,
+    int64 commit_to_activate_estimate_in_ms,
+    bool has_impl_side_updates,
+    bool swap_ack_before_deadline,
+    bool should_draw_and_swap_in_high_latency) {
+  // Set up client with specified estimates.
+  scheduler_settings_.use_external_begin_frame_source = true;
+  SetUpScheduler(true);
+
+  fake_compositor_timing_history_->SetDrawDurationEstimate(
+      base::TimeDelta::FromMilliseconds(draw_estimate_in_ms));
+  fake_compositor_timing_history_->SetBeginMainFrameToCommitDurationEstimate(
+      base::TimeDelta::FromMilliseconds(
+          begin_main_frame_to_commit_estimate_in_ms));
+  fake_compositor_timing_history_->SetCommitToReadyToActivateDurationEstimate(
+      base::TimeDelta::FromMilliseconds(commit_to_activate_estimate_in_ms));
+
+  // To get into a high latency state, this test disables automatic swap acks.
+  scheduler_->SetMaxSwapsPending(1);
+  client_->SetAutomaticSwapAck(false);
+
+  // Draw and swap for first BeginFrame
+  client_->Reset();
+  scheduler_->SetNeedsCommit();
+  if (has_impl_side_updates) {
+    scheduler_->SetNeedsRedraw();
+    EXPECT_FALSE(scheduler_->MainThreadIsInHighLatencyMode());
+    SendNextBeginFrame();
+    EXPECT_ACTION("SetNeedsBeginFrames(true)", client_, 0, 4);
+    EXPECT_ACTION("WillBeginImplFrame", client_, 1, 4);
+    EXPECT_ACTION("ScheduledActionAnimate", client_, 2, 4);
+    EXPECT_ACTION("ScheduledActionSendBeginMainFrame", client_, 3, 4);
+  } else {
+    EXPECT_FALSE(scheduler_->MainThreadIsInHighLatencyMode());
+    SendNextBeginFrame();
+    EXPECT_ACTION("SetNeedsBeginFrames(true)", client_, 0, 3);
+    EXPECT_ACTION("WillBeginImplFrame", client_, 1, 3);
+    EXPECT_ACTION("ScheduledActionSendBeginMainFrame", client_, 2, 3);
+  }
+
+  client_->Reset();
+  scheduler_->NotifyBeginMainFrameStarted();
+  scheduler_->NotifyReadyToCommit();
+  scheduler_->NotifyReadyToActivate();
+  EXPECT_FALSE(scheduler_->MainThreadIsInHighLatencyMode());
+  task_runner().RunTasksWhile(client_->ImplFrameDeadlinePending(true));
+  EXPECT_ACTION("ScheduledActionCommit", client_, 0, 4);
+  EXPECT_ACTION("ScheduledActionActivateSyncTree", client_, 1, 4);
+  EXPECT_ACTION("ScheduledActionAnimate", client_, 2, 4);
+  EXPECT_ACTION("ScheduledActionDrawAndSwapIfPossible", client_, 3, 4);
+
+  // Not calling scheduler_->DidSwapBuffersComplete() until after next frame
+  // puts the impl thread in high latency mode.
+  client_->Reset();
+  scheduler_->SetNeedsCommit();
+  if (has_impl_side_updates)
+    scheduler_->SetNeedsRedraw();
+  EXPECT_FALSE(scheduler_->MainThreadIsInHighLatencyMode());
+  SendNextBeginFrame();
+  if (should_draw_and_swap_in_high_latency) {

[mithro-old, 2015/07/06 11:58:43]

I don't quite understand what is going on here in this if statement chain.

[brianderson, 2015/07/07 02:04:49]

I renamed should_draw_and_swap_in_high_latency to
expect_begin_impl_frame_in_high_latency, which will hopefully reduce the
confusion. (The old variable name was a carryover from the initial patch set.)

Or do you think the test should be split up with fewer options in each test?

[mithro-old, 2015/07/07 07:35:38]

I think you should split the giant SchedulerTest::ImplFrameInHighLatency
function into a bunch of smaller functions which you call from the test code. 

Generally the more "if statements" in a test, the more likely the test is hard
to understand and easy to miss what is going on.

+    if (has_impl_side_updates) {
+      EXPECT_ACTION("WillBeginImplFrame", client_, 0, 2);
+      EXPECT_ACTION("ScheduledActionAnimate", client_, 1, 2);
+    } else {
+      EXPECT_SINGLE_ACTION("WillBeginImplFrame", client_);
+    }
+    EXPECT_TRUE(scheduler_->BeginImplFrameDeadlinePending());
+  } else {
+    EXPECT_NO_ACTION(client_);
+    EXPECT_FALSE(scheduler_->BeginImplFrameDeadlinePending());
+  }
+  EXPECT_FALSE(scheduler_->MainThreadIsInHighLatencyMode());
+
+  client_->Reset();
+  if (should_draw_and_swap_in_high_latency) {
+    DCHECK(swap_ack_before_deadline);
+    scheduler_->DidSwapBuffersComplete();
+    scheduler_->NotifyBeginMainFrameStarted();
+    scheduler_->NotifyReadyToCommit();
+    scheduler_->NotifyReadyToActivate();
+    task_runner().RunTasksWhile(client_->ImplFrameDeadlinePending(true));
+  } else if (swap_ack_before_deadline) {
+    scheduler_->DidSwapBuffersComplete();
+    task_runner().RunTasksWhile(client_->ImplFrameDeadlinePending(true));
+  } else {
+    task_runner().RunTasksWhile(client_->ImplFrameDeadlinePending(true));
+    scheduler_->DidSwapBuffersComplete();
+  }
+
+  // Verify that we recovered immediately if we were supposed to and
+  // that we didn't if weren't.
+  if (should_draw_and_swap_in_high_latency) {
+    EXPECT_ACTION("ScheduledActionSendBeginMainFrame", client_, 0, 5);
+    EXPECT_ACTION("ScheduledActionCommit", client_, 1, 5);
+    EXPECT_ACTION("ScheduledActionActivateSyncTree", client_, 2, 5);
+    EXPECT_ACTION("ScheduledActionAnimate", client_, 3, 5);
+    EXPECT_ACTION("ScheduledActionDrawAndSwapIfPossible", client_, 4, 5);
+    scheduler_->DidSwapBuffersComplete();
+  } else {
+    EXPECT_NO_ACTION(client_);
+  }
+
+  // Verify that we:
+  // 1) Don't skip two frames in a row.
+  // 2) Continue normal operation if we didn't skip a frame.
+  // 3) Make forward progress even when we aren't expecting another swap ack.
+  client_->Reset();
+  EXPECT_FALSE(scheduler_->MainThreadIsInHighLatencyMode());
+  scheduler_->SetNeedsCommit();
+  if (has_impl_side_updates) {
+    scheduler_->SetNeedsRedraw();
+    SendNextBeginFrame();
+    EXPECT_ACTION("WillBeginImplFrame", client_, 0, 3);
+    EXPECT_ACTION("ScheduledActionAnimate", client_, 1, 3);
+    EXPECT_ACTION("ScheduledActionSendBeginMainFrame", client_, 2, 3);
+  } else {
+    SendNextBeginFrame();
+    EXPECT_ACTION("WillBeginImplFrame", client_, 0, 2);
+    EXPECT_ACTION("ScheduledActionSendBeginMainFrame", client_, 1, 2);
+  }
+
+  client_->Reset();
+  scheduler_->NotifyBeginMainFrameStarted();
+  scheduler_->NotifyReadyToCommit();
+  scheduler_->NotifyReadyToActivate();
+  task_runner().RunTasksWhile(client_->ImplFrameDeadlinePending(true));
+  EXPECT_ACTION("ScheduledActionCommit", client_, 0, 4);
+  EXPECT_ACTION("ScheduledActionActivateSyncTree", client_, 1, 4);
+  EXPECT_ACTION("ScheduledActionAnimate", client_, 2, 4);
+  EXPECT_ACTION("ScheduledActionDrawAndSwapIfPossible", client_, 3, 4);
+}
+
+TEST_F(SchedulerTest,
+       SkipSwapIfHighLatencyAndCanDrawBeforeDeadline_SwapAckThenDeadline) {
+  // Use estimates that indicate commit and activate can finish before the
+  // deadline.
+  int64 draw_estimate_in_ms = 1;
+  int64 begin_main_frame_to_commit_estimate_in_ms = 1;
+  int64 commit_to_activate_estimate_in_ms = 1;
+  bool has_impl_side_updates = true;
+  bool swap_ack_before_deadline = true;
+  bool should_draw_and_swap_in_high_latency = false;
+  EXPECT_SCOPED(ImplFrameInHighLatency(
+      draw_estimate_in_ms, begin_main_frame_to_commit_estimate_in_ms,
+      commit_to_activate_estimate_in_ms, has_impl_side_updates,
+      swap_ack_before_deadline, should_draw_and_swap_in_high_latency));
+}
+
+TEST_F(SchedulerTest,
+       SkipSwapIfHighLatencyAndCanDrawBeforeDeadline_DeadlineThenSwapAck) {
+  // Use estimates that indicate commit and activate can finish before the
+  // deadline.
+  int64 draw_estimate_in_ms = 1;
+  int64 begin_main_frame_to_commit_estimate_in_ms = 1;
+  int64 commit_to_activate_estimate_in_ms = 1;
+  bool has_impl_side_updates = true;
+  bool swap_ack_before_deadline = false;
+  bool should_draw_and_swap_in_high_latency = false;
+  EXPECT_SCOPED(ImplFrameInHighLatency(
+      draw_estimate_in_ms, begin_main_frame_to_commit_estimate_in_ms,
+      commit_to_activate_estimate_in_ms, has_impl_side_updates,
+      swap_ack_before_deadline, should_draw_and_swap_in_high_latency));
+}
+
+TEST_F(SchedulerTest, NotSkipSwapIfHighLatencyAndCanDrawTooLong) {
+  // Use estimates that indicate draw cannot finish before the deadline.
+  int64 draw_estimate_in_ms = 20;
+  int64 begin_main_frame_to_commit_estimate_in_ms = 1;
+  int64 commit_to_activate_estimate_in_ms = 1;
+  bool has_impl_side_updates = true;
+  bool swap_ack_before_deadline = true;
+  bool should_draw_and_swap_in_high_latency = true;
+  EXPECT_SCOPED(ImplFrameInHighLatency(
+      draw_estimate_in_ms, begin_main_frame_to_commit_estimate_in_ms,
+      commit_to_activate_estimate_in_ms, has_impl_side_updates,
+      swap_ack_before_deadline, should_draw_and_swap_in_high_latency));
+}
+
+TEST_F(SchedulerTest, NotSkipSwapIfHighLatencyAndCanActivateTooLong) {
+  // Use estimates that indicate commit and activate cannot finish before the
+  // deadline.
+  int64 draw_estimate_in_ms = 1;
+  int64 begin_main_frame_to_commit_estimate_in_ms = 8;
+  int64 commit_to_activate_estimate_in_ms = 8;
+  bool has_impl_side_updates = false;
+  bool swap_ack_before_deadline = true;
+  bool should_draw_and_swap_in_high_latency = true;
+  EXPECT_SCOPED(ImplFrameInHighLatency(
+      draw_estimate_in_ms, begin_main_frame_to_commit_estimate_in_ms,
+      commit_to_activate_estimate_in_ms, has_impl_side_updates,
+      swap_ack_before_deadline, should_draw_and_swap_in_high_latency));
+}
+
+TEST_F(SchedulerTest, MainAndSwapInHighLatencyMode) {
+  // Set up client with custom estimates.
+  // This test starts off with expensive estimates to prevent latency recovery
+  // initially, the lowers the estimates to enable it once both the main
+  // and impl threads are in a high latency mode.
+  scheduler_settings_.use_external_begin_frame_source = true;
+  SetUpScheduler(true);
+
+  auto slow_duration = base::TimeDelta::FromMilliseconds(10);
+  fake_compositor_timing_history_->SetDrawDurationEstimate(slow_duration);
+  fake_compositor_timing_history_->SetBeginMainFrameToCommitDurationEstimate(
+      slow_duration);
+  fake_compositor_timing_history_->SetCommitToReadyToActivateDurationEstimate(
+      slow_duration);
+
+  // To get into a high latency state, this test disables automatic swap acks.
+  scheduler_->SetMaxSwapsPending(1);
+  client_->SetAutomaticSwapAck(false);
+
+  // Impl thread hits deadline before commit finishes to make
+  // MainThreadIsInHighLatencyMode true
+  client_->Reset();
+  scheduler_->SetNeedsCommit();
+  EXPECT_FALSE(scheduler_->MainThreadIsInHighLatencyMode());
+  EXPECT_SCOPED(AdvanceFrame());
+  EXPECT_FALSE(scheduler_->MainThreadIsInHighLatencyMode());
+  task_runner().RunTasksWhile(client_->ImplFrameDeadlinePending(true));
+  EXPECT_TRUE(scheduler_->MainThreadIsInHighLatencyMode());
+  scheduler_->NotifyBeginMainFrameStarted();
+  scheduler_->NotifyReadyToCommit();
+  scheduler_->NotifyReadyToActivate();
+  EXPECT_TRUE(scheduler_->MainThreadIsInHighLatencyMode());
+
+  EXPECT_ACTION("SetNeedsBeginFrames(true)", client_, 0, 5);
+  EXPECT_ACTION("WillBeginImplFrame", client_, 1, 5);
+  EXPECT_ACTION("ScheduledActionSendBeginMainFrame", client_, 2, 5);
+  EXPECT_ACTION("ScheduledActionCommit", client_, 3, 5);
+  EXPECT_ACTION("ScheduledActionActivateSyncTree", client_, 4, 5);
+
+  // Draw and swap for first commit, start second commit.
+  client_->Reset();
+  scheduler_->SetNeedsCommit();
+  EXPECT_TRUE(scheduler_->MainThreadIsInHighLatencyMode());
+  EXPECT_SCOPED(AdvanceFrame());
+  EXPECT_TRUE(scheduler_->MainThreadIsInHighLatencyMode());
+  task_runner().RunTasksWhile(client_->ImplFrameDeadlinePending(true));
+  scheduler_->NotifyBeginMainFrameStarted();
+  scheduler_->NotifyReadyToCommit();
+  scheduler_->NotifyReadyToActivate();
+
+  EXPECT_ACTION("WillBeginImplFrame", client_, 0, 6);
+  EXPECT_ACTION("ScheduledActionAnimate", client_, 1, 6);
+  EXPECT_ACTION("ScheduledActionSendBeginMainFrame", client_, 2, 6);
+  EXPECT_ACTION("ScheduledActionDrawAndSwapIfPossible", client_, 3, 6);
+  EXPECT_ACTION("ScheduledActionCommit", client_, 4, 6);
+  EXPECT_ACTION("ScheduledActionActivateSyncTree", client_, 5, 6);
+
+  // Don't call scheduler_->DidSwapBuffersComplete() until after next frame
+  // to put the impl thread in a high latency mode.
+  client_->Reset();
+  scheduler_->SetNeedsCommit();
+  EXPECT_TRUE(scheduler_->MainThreadIsInHighLatencyMode());
+  EXPECT_SCOPED(AdvanceFrame());
+  EXPECT_TRUE(scheduler_->MainThreadIsInHighLatencyMode());
+  task_runner().RunTasksWhile(client_->ImplFrameDeadlinePending(true));
+  scheduler_->DidSwapBuffersComplete();
+
+  EXPECT_ACTION("WillBeginImplFrame", client_, 0, 2);
+  EXPECT_ACTION("ScheduledActionAnimate", client_, 1, 2);
+  // Note: BeginMainFrame and swap are skipped here because of
+  // swap ack backpressure, not because of latency recovery.
+  EXPECT_FALSE(client_->HasAction("ScheduledActionSendBeginMainFrame"));
+  EXPECT_FALSE(client_->HasAction("ScheduledActionDrawAndSwapIfPossible"));
+
+  // Lower estimates so that the scheduler will attempt latency recovery.
+  auto fast_duration = base::TimeDelta::FromMilliseconds(1);
+  fake_compositor_timing_history_->SetDrawDurationEstimate(fast_duration);
+  fake_compositor_timing_history_->SetBeginMainFrameToCommitDurationEstimate(
+      fast_duration);
+  fake_compositor_timing_history_->SetCommitToReadyToActivateDurationEstimate(
+      fast_duration);
+
+  // Now that both threads are in a high latency mode, make sure we
+  // skip the BeginMainFrame, then the BeginImplFrame, but not both
+  // at the same time.
+
+  // Verify we skip BeginMainFrame first.
+  client_->Reset();
+  // Previous commit request is still outstanding.
+  EXPECT_TRUE(scheduler_->NeedsCommit());
+  EXPECT_TRUE(scheduler_->MainThreadIsInHighLatencyMode());
+  SendNextBeginFrame();
+  task_runner().RunTasksWhile(client_->ImplFrameDeadlinePending(true));
+  EXPECT_FALSE(scheduler_->MainThreadIsInHighLatencyMode());
+
+  EXPECT_ACTION("WillBeginImplFrame", client_, 0, 3);
+  EXPECT_ACTION("ScheduledActionAnimate", client_, 1, 3);
+  EXPECT_ACTION("ScheduledActionDrawAndSwapIfPossible", client_, 2, 3);
+  EXPECT_FALSE(client_->HasAction("ScheduledActionSendBeginMainFrame"));
+
+  // Verify we skip the BeginImplFrame second.
+  client_->Reset();
+  // Previous commit request is still outstanding.
+  EXPECT_TRUE(scheduler_->NeedsCommit());
+  EXPECT_FALSE(scheduler_->MainThreadIsInHighLatencyMode());
+  SendNextBeginFrame();
+  task_runner().RunTasksWhile(client_->ImplFrameDeadlinePending(true));
+  EXPECT_FALSE(scheduler_->MainThreadIsInHighLatencyMode());
+  scheduler_->DidSwapBuffersComplete();
+  EXPECT_FALSE(scheduler_->MainThreadIsInHighLatencyMode());
+
+  EXPECT_NO_ACTION(client_);
+
+  // Then verify we operate in a low latency mode.
+  client_->Reset();
+  // Previous commit request is still outstanding.
+  EXPECT_TRUE(scheduler_->NeedsCommit());
+  EXPECT_FALSE(scheduler_->MainThreadIsInHighLatencyMode());
+  SendNextBeginFrame();
+  EXPECT_FALSE(scheduler_->MainThreadIsInHighLatencyMode());
+  scheduler_->NotifyBeginMainFrameStarted();
+  scheduler_->NotifyReadyToCommit();
+  scheduler_->NotifyReadyToActivate();
+  task_runner().RunTasksWhile(client_->ImplFrameDeadlinePending(true));
+  EXPECT_FALSE(scheduler_->MainThreadIsInHighLatencyMode());
+  scheduler_->DidSwapBuffersComplete();
+  EXPECT_FALSE(scheduler_->MainThreadIsInHighLatencyMode());
+
+  EXPECT_ACTION("WillBeginImplFrame", client_, 0, 6);
+  EXPECT_ACTION("ScheduledActionSendBeginMainFrame", client_, 1, 6);
+  EXPECT_ACTION("ScheduledActionCommit", client_, 2, 6);
+  EXPECT_ACTION("ScheduledActionActivateSyncTree", client_, 3, 6);
+  EXPECT_ACTION("ScheduledActionAnimate", client_, 4, 6);
+  EXPECT_ACTION("ScheduledActionDrawAndSwapIfPossible", client_, 5, 6);
+}
+
 TEST_F(
     SchedulerTest,
     Deadlock_CommitMakesProgressWhileSwapTrottledAndActiveTreeNeedsFirstDraw) {