cc: Don't ManageTiles twice in one frame

Any external callers of ManageTiles should inform the Scheduler that
ManageTiles happened so that (1) it can clear the "needs manage tiles"
flag and (2) it can avoid doing a second ManageTiles on the same frame.

Other than after commit, where ManageTiles needs to be called
immediately in order to kick off new raster tasks (or determine that no
raster tasks are required to activate the tree), ManageTiles just needs
to be called periodically to keep the raster jobs working on the most
important content.  Delaying the periodic caller to prevent these bad
frames with commits is a worthwhile tradeoff.

R=brianderson@chromium.org
BUG=314882

Review URL: https://codereview.chromium.org/45923005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@233111 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 67 insertions, 0 deletions

diff --git a/cc/scheduler/scheduler.cc b/cc/scheduler/scheduler.cc
index 93b9131..4f2dbf6 100644
--- a/cc/scheduler/scheduler.cc
+++ b/cc/scheduler/scheduler.cc
@@ -101,6 +101,10 @@ void Scheduler::BeginMainFrameAborted(bool did_handle) {
   ProcessScheduledActions();
 }
 
+void Scheduler::DidManageTiles() {
+  state_machine_.DidManageTiles();
+}
+
 void Scheduler::DidLoseOutputSurface() {
   TRACE_EVENT0("cc", "Scheduler::DidLoseOutputSurface");
   last_set_needs_begin_impl_frame_ = false;
diff --git a/cc/scheduler/scheduler.h b/cc/scheduler/scheduler.h
index 1a284f1..e4a8bf3 100644
--- a/cc/scheduler/scheduler.h
+++ b/cc/scheduler/scheduler.h
@@ -91,6 +91,7 @@ class CC_EXPORT Scheduler {
   void FinishCommit();
   void BeginMainFrameAborted(bool did_handle);
 
+  void DidManageTiles();
   void DidLoseOutputSurface();
   void DidCreateAndInitializeOutputSurface();
   bool HasInitializedOutputSurface() const {
diff --git a/cc/scheduler/scheduler_state_machine.cc b/cc/scheduler/scheduler_state_machine.cc
index 0fd0f57..4d9e5e9 100644
--- a/cc/scheduler/scheduler_state_machine.cc
+++ b/cc/scheduler/scheduler_state_machine.cc
@@ -26,6 +26,7 @@ SchedulerStateMachine::SchedulerStateMachine(const SchedulerSettings& settings)
       last_frame_number_swap_performed_(-1),
       last_frame_number_begin_main_frame_sent_(-1),
       last_frame_number_update_visible_tiles_was_called_(-1),
+      last_frame_number_manage_tiles_called_(-1),
       consecutive_failed_draws_(0),
       needs_redraw_(false),
       needs_manage_tiles_(false),
@@ -493,6 +494,12 @@ bool SchedulerStateMachine::ShouldCommit() const {
 }
 
 bool SchedulerStateMachine::ShouldManageTiles() const {
+  // ManageTiles only really needs to be called immediately after commit
+  // and then periodically after that.  Limiting to once per frame prevents
+  // post-commit and post-draw ManageTiles on the same frame.
+  if (last_frame_number_manage_tiles_called_ == current_frame_number_)
+    return false;
+
   // Limiting to once per-frame is not enough, since we only want to
   // manage tiles _after_ draws. Polling for draw triggers and
   // begin-frame are mutually exclusive, so we limit to these two cases.
@@ -1018,6 +1025,11 @@ void SchedulerStateMachine::BeginMainFrameAborted(bool did_handle) {
   }
 }
 
+void SchedulerStateMachine::DidManageTiles() {
+  needs_manage_tiles_ = false;
+  last_frame_number_manage_tiles_called_ = current_frame_number_;
+}
+
 void SchedulerStateMachine::DidLoseOutputSurface() {
   if (output_surface_state_ == OUTPUT_SURFACE_LOST ||
       output_surface_state_ == OUTPUT_SURFACE_CREATING)
diff --git a/cc/scheduler/scheduler_state_machine.h b/cc/scheduler/scheduler_state_machine.h
index 802d06a..50efada 100644
--- a/cc/scheduler/scheduler_state_machine.h
+++ b/cc/scheduler/scheduler_state_machine.h
@@ -219,6 +219,7 @@ class CC_EXPORT SchedulerStateMachine {
 
   bool has_pending_tree() const { return has_pending_tree_; }
 
+  void DidManageTiles();
   void DidLoseOutputSurface();
   void DidCreateAndInitializeOutputSurface();
   bool HasInitializedOutputSurface() const;
@@ -271,6 +272,7 @@ class CC_EXPORT SchedulerStateMachine {
   int last_frame_number_swap_performed_;
   int last_frame_number_begin_main_frame_sent_;
   int last_frame_number_update_visible_tiles_was_called_;
+  int last_frame_number_manage_tiles_called_;
 
   int consecutive_failed_draws_;
   bool needs_redraw_;
diff --git a/cc/scheduler/scheduler_unittest.cc b/cc/scheduler/scheduler_unittest.cc
index c045183..03f3148 100644
--- a/cc/scheduler/scheduler_unittest.cc
+++ b/cc/scheduler/scheduler_unittest.cc
@@ -1080,5 +1080,53 @@ TEST(SchedulerTest, ManageTiles) {
   EXPECT_TRUE(client.HasAction("ScheduledActionManageTiles"));
 }
 
+// Test that ManageTiles only happens once per frame.  If an external caller
+// initiates it, then the state machine should not on that frame.
+TEST(SchedulerTest, ManageTilesOncePerFrame) {
+  FakeSchedulerClient client;
+  SchedulerSettings default_scheduler_settings;
+  Scheduler* scheduler = client.CreateScheduler(default_scheduler_settings);
+  scheduler->SetCanStart();
+  scheduler->SetVisible(true);
+  scheduler->SetCanDraw(true);
+  InitializeOutputSurfaceAndFirstCommit(scheduler);
+
+  // If DidManageTiles during a frame, then ManageTiles should not occur again.
+  scheduler->SetNeedsManageTiles();
+  scheduler->SetNeedsRedraw();
+  client.Reset();
+  scheduler->BeginImplFrame(BeginFrameArgs::CreateForTesting());
+  EXPECT_SINGLE_ACTION("PostBeginImplFrameDeadlineTask", client);
+
+  EXPECT_TRUE(scheduler->ManageTilesPending());
+  scheduler->DidManageTiles();
+  EXPECT_FALSE(scheduler->ManageTilesPending());
+
+  client.Reset();
+  scheduler->OnBeginImplFrameDeadline();
+  EXPECT_EQ(1, client.num_draws());
+  EXPECT_TRUE(client.HasAction("ScheduledActionDrawAndSwapIfPossible"));
+  EXPECT_FALSE(client.HasAction("ScheduledActionManageTiles"));
+  EXPECT_FALSE(scheduler->RedrawPending());
+  EXPECT_FALSE(scheduler->ManageTilesPending());
+
+  // Next frame without DidManageTiles should ManageTiles with draw.
+  scheduler->SetNeedsManageTiles();
+  scheduler->SetNeedsRedraw();
+  client.Reset();
+  scheduler->BeginImplFrame(BeginFrameArgs::CreateForTesting());
+  EXPECT_SINGLE_ACTION("PostBeginImplFrameDeadlineTask", client);
+
+  client.Reset();
+  scheduler->OnBeginImplFrameDeadline();
+  EXPECT_EQ(1, client.num_draws());
+  EXPECT_TRUE(client.HasAction("ScheduledActionDrawAndSwapIfPossible"));
+  EXPECT_TRUE(client.HasAction("ScheduledActionManageTiles"));
+  EXPECT_LT(client.ActionIndex("ScheduledActionDrawAndSwapIfPossible"),
+            client.ActionIndex("ScheduledActionManageTiles"));
+  EXPECT_FALSE(scheduler->RedrawPending());
+  EXPECT_FALSE(scheduler->ManageTilesPending());
+}
+
 }  // namespace
 }  // namespace cc