Don't set did_draw_in_last_frame_ in AbortDraw

In webview, frames with no damage sometimes call AbortDraw to prevent
draws. These aborts should not set did_draw_in_last_frame_ or the
scheduler will request new BeginFrames when it doesn't need to.

This CL stops setting did_draw_in_last_frame_ in AbortDraw while still
setting it in WillDraw.

Bug: 687695
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: I84c39a5e06d578ea92558d81ce2fad5dc423cf75
Reviewed-on: https://chromium-review.googlesource.com/933292
Commit-Queue: James Wallace-Lee <jamwalla@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542229}

cc/scheduler/scheduler_state_machine.cc

@@ -816,7 +816,6 @@ void SchedulerStateMachine::WillDrawInternal() {
 
   did_draw_ = true;
   active_tree_needs_first_draw_ = false;
-  did_draw_in_last_frame_ = true;
   last_frame_number_draw_performed_ = current_frame_number_;
 
   if (forced_redraw_state_ == ForcedRedrawOnTimeoutState::WAITING_FOR_DRAW)
@@ -863,6 +862,10 @@ void SchedulerStateMachine::DidDrawInternal(DrawResult draw_result) {
 void SchedulerStateMachine::WillDraw() {
   DCHECK(!did_draw_);
   WillDrawInternal();
+  // 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;
 }
 
 void SchedulerStateMachine::DidDraw(DrawResult draw_result) {

cc/scheduler/scheduler_unittest.cc

@@ -2555,7 +2555,10 @@ TEST_F(SchedulerTest, ScheduledActionActivateAfterBeginFrameSourcePaused) {
   task_runner().RunPendingTasks();  // Run posted deadline.
 
   // Sync tree should be forced to activate.
-  EXPECT_ACTIONS("ScheduledActionActivateSyncTree");
+  // Pausing the begin frame source aborts the draw. Then
+  // ProactiveBeginFrameWanted is no longer true, so the scheduler stops
+  // listening for begin frames.
+  EXPECT_ACTIONS("ScheduledActionActivateSyncTree", "RemoveObserver(this)");
 }
 
 // Tests to ensure frame sources can be successfully changed while drawing.
@@ -2977,7 +2980,10 @@ TEST_F(SchedulerTest, AbortEarlyIfNoDamage) {
   task_runner().RunPendingTasks();  // Run posted deadline.
   // Should not try to schedule a draw. (ScheduledActionDrawIfPossible should
   // not appear.)
-  EXPECT_ACTIONS("AddObserver(this)", "WillBeginImplFrame");
+  // When the frame is aborted, the scheduler does not ask for a proactive begin
+  // frame, so stop listening for begin frames.
+  EXPECT_ACTIONS("AddObserver(this)", "WillBeginImplFrame",
+                 "RemoveObserver(this)");
   EXPECT_EQ(0, client_->num_draws());
 
   scheduler_->SetNeedsRedraw();