[cc scheduler] Fix a no-damage notification.

It is possible for a draw to succeed, but a frame to still not be
submitted. This can happen when nothing blocks the draw, but still
no frame was submitted because there was no damage. In such cases,
mark the frame as deliberately dropped, rather than a frame that
was aborted while waiting on the main-thread.

BUG=790761

Change-Id: I410151858f7f2dd48b066cfc0ec05c29aac2e6d1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2144422
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org>
Cr-Commit-Position: refs/heads/master@{#758410}

cc/scheduler/scheduler.cc

@@ -583,7 +583,9 @@ void Scheduler::FinishImplFrame() {
   // ensures that the acks are sent in order.
   if (!state_machine_.did_submit_in_last_frame()) {
     SendDidNotProduceFrame(begin_impl_frame_tracker_.Current(),
-                           FrameSkippedReason::kWaitingOnMain);
+                           state_machine_.draw_succeeded_in_last_frame()
+                               ? FrameSkippedReason::kNoDamage
+                               : FrameSkippedReason::kWaitingOnMain);
   }
 
   begin_impl_frame_tracker_.Finish();

cc/scheduler/scheduler_state_machine.cc

@@ -266,7 +266,7 @@ void SchedulerStateMachine::AsProtozeroInto(
   minor_state->set_video_needs_begin_frames(video_needs_begin_frames_);
   minor_state->set_defer_begin_main_frame(defer_begin_main_frame_);
   minor_state->set_last_commit_had_no_updates(last_commit_had_no_updates_);
-  minor_state->set_did_draw_in_last_frame(did_draw_in_last_frame_);
+  minor_state->set_did_draw_in_last_frame(did_attempt_draw_in_last_frame_);
   minor_state->set_did_submit_in_last_frame(did_submit_in_last_frame_);
   minor_state->set_needs_impl_side_invalidation(needs_impl_side_invalidation_);
   minor_state->set_current_pending_tree_is_impl_side(
@@ -981,10 +981,11 @@ void SchedulerStateMachine::WillDraw() {
   // Set this to true to proactively request a new BeginFrame. We can't set this
   // in WillDrawInternal because AbortDraw calls WillDrawInternal but shouldn't
   // request another frame.
-  did_draw_in_last_frame_ = true;
+  did_attempt_draw_in_last_frame_ = true;
 }
 
 void SchedulerStateMachine::DidDraw(DrawResult draw_result) {
+  draw_succeeded_in_last_frame_ = draw_result == DRAW_SUCCESS;
   DidDrawInternal(draw_result);
 }
 
@@ -1126,7 +1127,7 @@ bool SchedulerStateMachine::ProactiveBeginFrameWanted() const {
   // frame soon. This helps avoid negative glitches in our SetNeedsBeginFrame
   // requests, which may propagate to the BeginImplFrame provider and get
   // sampled at an inopportune time, delaying the next BeginImplFrame.
-  if (did_draw_in_last_frame_)
+  if (did_attempt_draw_in_last_frame_)
     return true;
 
   // If the last commit was aborted because of early out (no updates), we should
@@ -1154,7 +1155,8 @@ void SchedulerStateMachine::OnBeginImplFrame(const viz::BeginFrameId& frame_id,
   last_frame_events_.did_commit_during_frame = did_commit_during_frame_;
 
   last_commit_had_no_updates_ = false;
-  did_draw_in_last_frame_ = false;
+  did_attempt_draw_in_last_frame_ = false;
+  draw_succeeded_in_last_frame_ = false;
   did_submit_in_last_frame_ = false;
   needs_one_begin_impl_frame_ = false;
 

cc/scheduler/scheduler_state_machine.h

@@ -323,6 +323,9 @@ class CC_EXPORT SchedulerStateMachine {
   bool video_needs_begin_frames() const { return video_needs_begin_frames_; }
 
   bool did_submit_in_last_frame() const { return did_submit_in_last_frame_; }
+  bool draw_succeeded_in_last_frame() const {
+    return draw_succeeded_in_last_frame_;
+  }
 
   bool needs_impl_side_invalidation() const {
     return needs_impl_side_invalidation_;
@@ -450,7 +453,8 @@ class CC_EXPORT SchedulerStateMachine {
   bool video_needs_begin_frames_ = false;
   bool last_commit_had_no_updates_ = false;
   bool active_tree_is_ready_to_draw_ = true;
-  bool did_draw_in_last_frame_ = false;
+  bool did_attempt_draw_in_last_frame_ = false;
+  bool draw_succeeded_in_last_frame_ = false;
   bool did_submit_in_last_frame_ = false;
   bool needs_impl_side_invalidation_ = false;
   bool next_invalidation_needs_first_draw_on_activation_ = false;

cc/scheduler/scheduler_unittest.cc

@@ -60,6 +60,7 @@ class FakeSchedulerClient : public SchedulerClient,
     num_draws_ = 0;
     last_begin_main_frame_args_ = viz::BeginFrameArgs();
     last_begin_frame_ack_ = viz::BeginFrameAck();
+    last_frame_skipped_reason_.reset();
   }
 
   void set_scheduler(TestScheduler* scheduler) { scheduler_ = scheduler; }
@@ -130,6 +131,7 @@ class FakeSchedulerClient : public SchedulerClient,
     EXPECT_FALSE(inside_action_);
     base::AutoReset<bool> mark_inside(&inside_action_, true);
     last_begin_frame_ack_ = ack;
+    last_frame_skipped_reason_ = reason;
   }
 
   void WillNotReceiveBeginFrame() override {}
@@ -153,23 +155,26 @@ class FakeSchedulerClient : public SchedulerClient,
     return last_begin_frame_ack_;
   }
 
+  FrameSkippedReason last_frame_skipped_reason() const {
+    return last_frame_skipped_reason_.value();
+  }
+
   DrawResult ScheduledActionDrawIfPossible() override {
     EXPECT_FALSE(inside_action_);
     base::AutoReset<bool> mark_inside(&inside_action_, true);
     PushAction("ScheduledActionDrawIfPossible");
     num_draws_++;
-    DrawResult result =
-        draw_will_happen_ ? DRAW_SUCCESS : DRAW_ABORTED_CHECKERBOARD_ANIMATIONS;
-    bool swap_will_happen =
-        draw_will_happen_ && swap_will_happen_if_draw_happens_;
-    if (swap_will_happen) {
+    if (!draw_will_happen_)
+      return DRAW_ABORTED_CHECKERBOARD_ANIMATIONS;
+
+    if (swap_will_happen_if_draw_happens_) {
       last_begin_frame_ack_ = scheduler_->CurrentBeginFrameAckForActiveTree();
       scheduler_->DidSubmitCompositorFrame(0, EventMetricsSet());
 
       if (automatic_ack_)
         scheduler_->DidReceiveCompositorFrameAck();
     }
-    return result;
+    return DRAW_SUCCESS;
   }
   DrawResult ScheduledActionDrawForced() override {
     EXPECT_FALSE(inside_action_);
@@ -283,6 +288,7 @@ class FakeSchedulerClient : public SchedulerClient,
   std::vector<const char*> actions_;
   TestScheduler* scheduler_ = nullptr;
   base::TimeDelta frame_interval_;
+  base::Optional<FrameSkippedReason> last_frame_skipped_reason_;
 };
 
 enum BeginFrameSourceType {
@@ -1813,6 +1819,8 @@ void SchedulerTest::ImplFrameSkippedAfterLateAck(
     EXPECT_NO_ACTION();
     EXPECT_FALSE(client_->IsInsideBeginImplFrame());
     EXPECT_FALSE(scheduler_->MainThreadMissedLastDeadline());
+    EXPECT_EQ(FrameSkippedReason::kRecoverLatency,
+              client_->last_frame_skipped_reason());
 
     // Verify that we do not perform any actions after we are no longer
     // swap throttled.
@@ -1939,6 +1947,8 @@ TEST_F(SchedulerTest,
     EXPECT_NO_ACTION();
     EXPECT_FALSE(client_->IsInsideBeginImplFrame());
     EXPECT_FALSE(scheduler_->MainThreadMissedLastDeadline());
+    EXPECT_EQ(FrameSkippedReason::kRecoverLatency,
+              client_->last_frame_skipped_reason());
 
     // Verify that we do not perform any actions after we are no longer
     // swap throttled.
@@ -3850,7 +3860,26 @@ TEST_F(SchedulerTest, BeginFrameAckForFinishedImplFrame) {
   has_damage = false;
   EXPECT_EQ(viz::BeginFrameAck(args, has_damage),
             client_->last_begin_frame_ack());
+  EXPECT_EQ(FrameSkippedReason::kWaitingOnMain,
+            client_->last_frame_skipped_reason());
   client_->Reset();
+
+  // Draw succeeds, but 'swap' does not happen (i.e. no frame is submitted).
+  args = SendNextBeginFrame();
+  EXPECT_ACTIONS("WillBeginImplFrame");
+  EXPECT_TRUE(client_->IsInsideBeginImplFrame());
+  EXPECT_TRUE(scheduler_->begin_frames_expected());
+  client_->Reset();
+  client_->SetDrawWillHappen(true);
+  client_->SetSwapWillHappenIfDrawHappens(false);
+  task_runner_->RunPendingTasks();  // Run posted deadline.
+
+  // Draw with no damage.
+  has_damage = false;
+  EXPECT_EQ(viz::BeginFrameAck(args, has_damage),
+            client_->last_begin_frame_ack());
+  EXPECT_EQ(FrameSkippedReason::kNoDamage,
+            client_->last_frame_skipped_reason());
 }
 
 TEST_F(SchedulerTest, BeginFrameAckForSkippedImplFrame) {