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

11 files changed, 77 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
diff --git a/cc/test/fake_layer_tree_host_impl_client.h b/cc/test/fake_layer_tree_host_impl_client.h
index 708bbcf..b9c1fa9 100644
--- a/cc/test/fake_layer_tree_host_impl_client.h
+++ b/cc/test/fake_layer_tree_host_impl_client.h
@@ -36,6 +36,7 @@ class FakeLayerTreeHostImplClient : public LayerTreeHostImplClient {
   virtual void RequestScrollbarAnimationOnImplThread(base::TimeDelta)
       OVERRIDE {}
   virtual void DidActivatePendingTree() OVERRIDE {}
+  virtual void DidManageTiles() OVERRIDE {}
 };
 
 }  // namespace cc
diff --git a/cc/trees/layer_tree_host_impl.h b/cc/trees/layer_tree_host_impl.h
index 83b92c8..3345d15 100644
--- a/cc/trees/layer_tree_host_impl.h
+++ b/cc/trees/layer_tree_host_impl.h
@@ -79,6 +79,7 @@ class LayerTreeHostImplClient {
   virtual void RenewTreePriority() = 0;
   virtual void RequestScrollbarAnimationOnImplThread(base::TimeDelta delay) = 0;
   virtual void DidActivatePendingTree() = 0;
+  virtual void DidManageTiles() = 0;
 
  protected:
   virtual ~LayerTreeHostImplClient() {}
diff --git a/cc/trees/layer_tree_host_impl_unittest.cc b/cc/trees/layer_tree_host_impl_unittest.cc
index c2ec456..0e69b72 100644
--- a/cc/trees/layer_tree_host_impl_unittest.cc
+++ b/cc/trees/layer_tree_host_impl_unittest.cc
@@ -144,6 +144,7 @@ class LayerTreeHostImplTest : public testing::Test,
   virtual void RequestScrollbarAnimationOnImplThread(base::TimeDelta delay)
       OVERRIDE { requested_scrollbar_animation_delay_ = delay; }
   virtual void DidActivatePendingTree() OVERRIDE {}
+  virtual void DidManageTiles() OVERRIDE {}
 
   void set_reduce_memory_result(bool reduce_memory_result) {
     reduce_memory_result_ = reduce_memory_result;
diff --git a/cc/trees/single_thread_proxy.h b/cc/trees/single_thread_proxy.h
index 843be729..e5b706f 100644
--- a/cc/trees/single_thread_proxy.h
+++ b/cc/trees/single_thread_proxy.h
@@ -73,6 +73,7 @@ class SingleThreadProxy : public Proxy, LayerTreeHostImplClient {
   virtual void RequestScrollbarAnimationOnImplThread(base::TimeDelta delay)
       OVERRIDE {}
   virtual void DidActivatePendingTree() OVERRIDE {}
+  virtual void DidManageTiles() OVERRIDE {}
 
   // Called by the legacy path where RenderWidget does the scheduling.
   void CompositeImmediately(base::TimeTicks frame_begin_time);
diff --git a/cc/trees/thread_proxy.cc b/cc/trees/thread_proxy.cc
index 4f2f488..b25f8c2 100644
--- a/cc/trees/thread_proxy.cc
+++ b/cc/trees/thread_proxy.cc
@@ -1591,4 +1591,9 @@ void ThreadProxy::DidActivatePendingTree() {
       base::TimeTicks::HighResNow() - commit_complete_time_);
 }
 
+void ThreadProxy::DidManageTiles() {
+  DCHECK(IsImplThread());
+  scheduler_on_impl_thread_->DidManageTiles();
+}
+
 }  // namespace cc
diff --git a/cc/trees/thread_proxy.h b/cc/trees/thread_proxy.h
index 3a213b9..0bd1f4b 100644
--- a/cc/trees/thread_proxy.h
+++ b/cc/trees/thread_proxy.h
@@ -90,6 +90,7 @@ class ThreadProxy : public Proxy,
   virtual void RequestScrollbarAnimationOnImplThread(base::TimeDelta delay)
       OVERRIDE;
   virtual void DidActivatePendingTree() OVERRIDE;
+  virtual void DidManageTiles() OVERRIDE;
 
   // SchedulerClient implementation
   virtual void SetNeedsBeginImplFrame(bool enable) OVERRIDE;