cc: Ensure to issue only one NotifiyReadyTo* callback per manage tiles.

This patch ensures that we only issue a single notification that we're
ready to either activate or draw per manage tiles call.

Previously it was possible that we might issue extra notifications
due to tile manager's internal rescheduling.

Comes with a test that fails without the patch and passes with the
patch.

R=reveman, danakj

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

Cr-Commit-Position: refs/heads/master@{#306616}

5 files changed, 92 insertions, 9 deletions

diff --git a/cc/resources/tile_manager.cc b/cc/resources/tile_manager.cc
index d83b824..66f95da 100644
--- a/cc/resources/tile_manager.cc
+++ b/cc/resources/tile_manager.cc
@@ -239,9 +239,11 @@ TileManager::TileManager(
           task_runner_.get(),
           base::Bind(&TileManager::CheckIfReadyToActivate,
                      base::Unretained(this))),
-      ready_to_draw_check_notifier_(task_runner_.get(),
-                                    base::Bind(&TileManager::CheckIfReadyToDraw,
-                                               base::Unretained(this))) {
+      ready_to_draw_check_notifier_(
+          task_runner_.get(),
+          base::Bind(&TileManager::CheckIfReadyToDraw, base::Unretained(this))),
+      did_notify_ready_to_activate_(false),
+      did_notify_ready_to_draw_(false) {
   rasterizer_->SetClient(this);
 }
 
@@ -416,9 +418,12 @@ void TileManager::ManageTiles(const GlobalStateThatImpactsTilePriority& state) {
   TileVector tiles_that_need_to_be_rasterized;
   AssignGpuMemoryToTiles(&tiles_that_need_to_be_rasterized);
 
-  // Finally, schedule rasterizer tasks.
+  // Schedule rasterizer tasks.
   ScheduleTasks(tiles_that_need_to_be_rasterized);
 
+  did_notify_ready_to_activate_ = false;
+  did_notify_ready_to_draw_ = false;
+
   TRACE_EVENT_INSTANT1("cc",
                        "DidManage",
                        TRACE_EVENT_SCOPE_THREAD,
@@ -882,8 +887,13 @@ void TileManager::CheckIfReadyToActivate() {
   rasterizer_->CheckForCompletedTasks();
   did_check_for_completed_tasks_since_last_schedule_tasks_ = true;
 
-  if (IsReadyToActivate())
-    client_->NotifyReadyToActivate();
+  if (did_notify_ready_to_activate_)
+    return;
+  if (!IsReadyToActivate())
+    return;
+
+  client_->NotifyReadyToActivate();
+  did_notify_ready_to_activate_ = true;
 }
 
 void TileManager::CheckIfReadyToDraw() {
@@ -892,8 +902,13 @@ void TileManager::CheckIfReadyToDraw() {
   rasterizer_->CheckForCompletedTasks();
   did_check_for_completed_tasks_since_last_schedule_tasks_ = true;
 
-  if (IsReadyToDraw())
-    client_->NotifyReadyToDraw();
+  if (did_notify_ready_to_draw_)
+    return;
+  if (!IsReadyToDraw())
+    return;
+
+  client_->NotifyReadyToDraw();
+  did_notify_ready_to_draw_ = true;
 }
 
 TileManager::MemoryUsage::MemoryUsage() : memory_bytes_(0), resource_count_(0) {
diff --git a/cc/resources/tile_manager.h b/cc/resources/tile_manager.h
index e94fb7c..8d19520 100644
--- a/cc/resources/tile_manager.h
+++ b/cc/resources/tile_manager.h
@@ -162,6 +162,10 @@ class CC_EXPORT TileManager : public RasterizerClient,
     return tiles;
   }
 
+  void SetScheduledRasterTaskLimitForTesting(size_t limit) {
+    scheduled_raster_task_limit_ = limit;
+  }
+
  protected:
   TileManager(TileManagerClient* client,
               const scoped_refptr<base::SequencedTaskRunner>& task_runner,
@@ -243,7 +247,7 @@ class CC_EXPORT TileManager : public RasterizerClient,
   ResourcePool* resource_pool_;
   Rasterizer* rasterizer_;
   GlobalStateThatImpactsTilePriority global_state_;
-  const size_t scheduled_raster_task_limit_;
+  size_t scheduled_raster_task_limit_;
 
   typedef base::hash_map<Tile::Id, Tile*> TileMap;
   TileMap tiles_;
@@ -282,6 +286,9 @@ class CC_EXPORT TileManager : public RasterizerClient,
   EvictionTilePriorityQueue eviction_priority_queue_;
   bool eviction_priority_queue_is_up_to_date_;
 
+  bool did_notify_ready_to_activate_;
+  bool did_notify_ready_to_draw_;
+
   DISALLOW_COPY_AND_ASSIGN(TileManager);
 };
 
diff --git a/cc/test/layer_tree_test.cc b/cc/test/layer_tree_test.cc
index fb16917..5be1240 100644
--- a/cc/test/layer_tree_test.cc
+++ b/cc/test/layer_tree_test.cc
@@ -153,6 +153,11 @@ class ThreadProxyForTest : public ThreadProxy {
     test_hooks_->ScheduledActionBeginOutputSurfaceCreation();
   }
 
+  void ScheduledActionManageTiles() override {
+    ThreadProxy::ScheduledActionManageTiles();
+    test_hooks_->ScheduledActionManageTiles();
+  }
+
   ThreadProxyForTest(
       TestHooks* test_hooks,
       LayerTreeHost* host,
diff --git a/cc/test/layer_tree_test.h b/cc/test/layer_tree_test.h
index 2914cfc..863100b 100644
--- a/cc/test/layer_tree_test.h
+++ b/cc/test/layer_tree_test.h
@@ -98,6 +98,7 @@ class TestHooks : public AnimationDelegate {
   virtual void ScheduledActionAnimate() {}
   virtual void ScheduledActionCommit() {}
   virtual void ScheduledActionBeginOutputSurfaceCreation() {}
+  virtual void ScheduledActionManageTiles() {}
 
   // Implementation of AnimationDelegate:
   void NotifyAnimationStarted(base::TimeTicks monotonic_time,
diff --git a/cc/trees/layer_tree_host_unittest.cc b/cc/trees/layer_tree_host_unittest.cc
index a1e82f8..712909f 100644
--- a/cc/trees/layer_tree_host_unittest.cc
+++ b/cc/trees/layer_tree_host_unittest.cc
@@ -5887,4 +5887,59 @@ class LayerTreeHostTestContinuousDrawWhenCreatingVisibleTiles
 
 MULTI_THREAD_TEST_F(LayerTreeHostTestContinuousDrawWhenCreatingVisibleTiles);
 
+class LayerTreeHostTestOneActivatePerManageTiles : public LayerTreeHostTest {
+ public:
+  LayerTreeHostTestOneActivatePerManageTiles()
+      : notify_ready_to_activate_count_(0u), scheduled_manage_tiles_count_(0) {}
+
+  void SetupTree() override {
+    client_.set_fill_with_nonsolid_color(true);
+    scoped_refptr<FakePictureLayer> root_layer =
+        FakePictureLayer::Create(&client_);
+    root_layer->SetBounds(gfx::Size(1500, 1500));
+    root_layer->SetIsDrawable(true);
+
+    layer_tree_host()->SetRootLayer(root_layer);
+    LayerTreeHostTest::SetupTree();
+  }
+
+  void BeginTest() override {
+    layer_tree_host()->SetViewportSize(gfx::Size(16, 16));
+    PostSetNeedsCommitToMainThread();
+  }
+
+  void InitializedRendererOnThread(LayerTreeHostImpl* host_impl,
+                                   bool success) override {
+    ASSERT_TRUE(success);
+    host_impl->tile_manager()->SetScheduledRasterTaskLimitForTesting(1);
+  }
+
+  void NotifyReadyToActivateOnThread(LayerTreeHostImpl* impl) override {
+    ++notify_ready_to_activate_count_;
+    EndTestAfterDelayMs(100);
+  }
+
+  void ScheduledActionManageTiles() override {
+    ++scheduled_manage_tiles_count_;
+  }
+
+  void AfterTest() override {
+    // Expect at most a notification for each scheduled manage tiles, plus one
+    // for the initial commit (which doesn't go through scheduled actions).
+    // The reason this is not an equality is because depending on timing, we
+    // might get a manage tiles but not yet get a notification that we're
+    // ready to activate. The intent of a test is to ensure that we don't
+    // get more than one notification per manage tiles, so this is OK.
+    EXPECT_LE(notify_ready_to_activate_count_,
+              1u + scheduled_manage_tiles_count_);
+  }
+
+ protected:
+  FakeContentLayerClient client_;
+  size_t notify_ready_to_activate_count_;
+  size_t scheduled_manage_tiles_count_;
+};
+
+MULTI_THREAD_IMPL_TEST_F(LayerTreeHostTestOneActivatePerManageTiles);
+
 }  // namespace cc