cc: Clean up tilings in UpdateTiles as well as in AppendQuads.

Sometimes we get into situations where we get frequent UpdateTiles,
but no AppendQuads. As a result we can start accumulating tilings
(during pinch zoom for example), but we never free any of them, since
the code to free tilings happens only during AppendQuads. This patch
remedies this by ensuring that if AppendQuads doesn't clean up the
tilings, and we've added some since last time we ran UpdateTiles,
we clean them up.

R=danakj
BUG=452304

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

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

cc/layers/picture_layer_impl.cc

@@ -271,7 +271,7 @@ void PictureLayerImpl::AppendQuads(RenderPass* render_pass,
 
   // Keep track of the tilings that were used so that tilings that are
   // unused can be considered for removal.
-  std::vector<PictureLayerTiling*> seen_tilings;
+  last_append_quads_tilings_.clear();
 
   // Ignore missing tiles outside of viewport for tile priority. This is
   // normally the same as draw viewport but can be independently overridden by
@@ -398,8 +398,10 @@ void PictureLayerImpl::AppendQuads(RenderPass* render_pass,
     if (iter.resolution() != LOW_RESOLUTION)
       only_used_low_res_last_append_quads_ = false;
 
-    if (seen_tilings.empty() || seen_tilings.back() != iter.CurrentTiling())
-      seen_tilings.push_back(iter.CurrentTiling());
+    if (last_append_quads_tilings_.empty() ||
+        last_append_quads_tilings_.back() != iter.CurrentTiling()) {
+      last_append_quads_tilings_.push_back(iter.CurrentTiling());
+    }
   }
 
   if (missing_tile_count) {
@@ -416,7 +418,7 @@ void PictureLayerImpl::AppendQuads(RenderPass* render_pass,
   // that this is at the expense of doing cause more frequent re-painting. A
   // better scheme would be to maintain a tighter visible_content_rect for the
   // finer tilings.
-  CleanUpTilingsOnActiveLayer(seen_tilings);
+  CleanUpTilingsOnActiveLayer(last_append_quads_tilings_);
 }
 
 bool PictureLayerImpl::UpdateTiles(const Occlusion& occlusion_in_content_space,
@@ -437,6 +439,14 @@ bool PictureLayerImpl::UpdateTiles(const Occlusion& occlusion_in_content_space,
     return false;
   }
 
+  // Remove any non-ideal tilings that were not used last time we generated
+  // quads to save memory and processing time. Note that pending tree should
+  // only have one or two tilings (high and low res), so only clean up the
+  // active layer. This cleans it up here in case AppendQuads didn't run.
+  // If it did run, this would not remove any additional tilings.
+  if (GetTree() == ACTIVE_TREE)
+    CleanUpTilingsOnActiveLayer(last_append_quads_tilings_);
+
   UpdateIdealScales();
 
   if (!raster_contents_scale_ || ShouldAdjustRasterScale()) {
@@ -455,7 +465,6 @@ bool PictureLayerImpl::UpdateTiles(const Occlusion& occlusion_in_content_space,
 
   if (draw_transform_is_animating())
     raster_source_->SetShouldAttemptToUseDistanceFieldText();
-
   return UpdateTilePriorities(occlusion_in_content_space);
 }
 
@@ -997,7 +1006,7 @@ void PictureLayerImpl::RecalculateRasterScales() {
 }
 
 void PictureLayerImpl::CleanUpTilingsOnActiveLayer(
-    std::vector<PictureLayerTiling*> used_tilings) {
+    const std::vector<PictureLayerTiling*>& used_tilings) {
   DCHECK(layer_tree_impl()->IsActiveTree());
   if (tilings_->num_tilings() == 0)
     return;

cc/layers/picture_layer_impl.h

@@ -116,7 +116,7 @@ class CC_EXPORT PictureLayerImpl
   virtual bool ShouldAdjustRasterScale() const;
   virtual void RecalculateRasterScales();
   void CleanUpTilingsOnActiveLayer(
-      std::vector<PictureLayerTiling*> used_tilings);
+      const std::vector<PictureLayerTiling*>& used_tilings);
   float MinimumContentsScale() const;
   float MaximumContentsScale() const;
   void ResetRasterScale();
@@ -173,6 +173,13 @@ class CC_EXPORT PictureLayerImpl
   gfx::Rect visible_rect_for_tile_priority_;
   gfx::Rect viewport_rect_for_tile_priority_in_content_space_;
 
+  // List of tilings that were used last time we appended quads. This can be
+  // used as an optimization not to remove tilings if they are still being
+  // drawn. Note that accessing this vector should only be done in the context
+  // of comparing pointers, since objects pointed to are not guaranteed to
+  // exist.
+  std::vector<PictureLayerTiling*> last_append_quads_tilings_;
+
   DISALLOW_COPY_AND_ASSIGN(PictureLayerImpl);
 };
 

cc/layers/picture_layer_impl_unittest.cc

@@ -793,6 +793,10 @@ TEST_F(PictureLayerImplTest, ClonePartialInvalidation) {
   // Add a non-shared tiling on the active tree.
   PictureLayerTiling* tiling = active_layer_->AddTiling(3.f);
   tiling->CreateAllTilesForTesting();
+
+  // Ensure UpdateTiles won't remove any tilings.
+  active_layer_->MarkAllTilingsUsed();
+
   // Then setup a new pending tree and activate it.
   SetupTreesWithFixedTileSize(pending_pile, active_pile, gfx::Size(50, 50),
                               layer_invalidation);
@@ -1076,6 +1080,9 @@ TEST_F(PictureLayerImplTest, PinchGestureTilings) {
   EXPECT_BOTH_EQ(tilings()->tiling_at(1)->contents_scale(),
                  2.f * low_res_factor);
 
+  // Ensure UpdateTiles won't remove any tilings.
+  active_layer_->MarkAllTilingsUsed();
+
   // Start a pinch gesture.
   host_impl_.PinchGestureBegin();
 
@@ -1090,6 +1097,9 @@ TEST_F(PictureLayerImplTest, PinchGestureTilings) {
   EXPECT_FLOAT_EQ(2.0f * low_res_factor,
                   active_layer_->tilings()->tiling_at(2)->contents_scale());
 
+  // Ensure UpdateTiles won't remove any tilings.
+  active_layer_->MarkAllTilingsUsed();
+
   // Zoom out further, close to our low-res scale factor. We should
   // use that tiling as high-res, and not create a new tiling.
   SetContentsScaleOnBothLayers(low_res_factor * 2.1f, 1.0f,
@@ -1126,6 +1136,9 @@ TEST_F(PictureLayerImplTest, SnappedTilingDuringZoom) {
   EXPECT_FLOAT_EQ(0.0625f,
                   active_layer_->tilings()->tiling_at(1)->contents_scale());
 
+  // Ensure UpdateTiles won't remove any tilings.
+  active_layer_->MarkAllTilingsUsed();
+
   // Start a pinch gesture.
   host_impl_.PinchGestureBegin();
 
@@ -1140,6 +1153,9 @@ TEST_F(PictureLayerImplTest, SnappedTilingDuringZoom) {
   EXPECT_FLOAT_EQ(0.0625,
                   active_layer_->tilings()->tiling_at(2)->contents_scale());
 
+  // Ensure UpdateTiles won't remove any tilings.
+  active_layer_->MarkAllTilingsUsed();
+
   // Zoom out further, close to our low-res scale factor. We should
   // use that tiling as high-res, and not create a new tiling.
   SetContentsScaleOnBothLayers(0.1f, 1.0f, 0.1f, 1.0f, false);
@@ -1179,6 +1195,11 @@ TEST_F(PictureLayerImplTest, CleanUpTilings) {
   EXPECT_EQ(2u, active_layer_->tilings()->num_tilings());
   EXPECT_EQ(1.f, active_layer_->HighResTiling()->contents_scale());
 
+  // Ensure UpdateTiles won't remove any tilings. Note this is unrelated to
+  // |used_tilings| variable, and it's here only to ensure that active_layer_
+  // won't remove tilings before the test has a chance to verify behavior.
+  active_layer_->MarkAllTilingsUsed();
+
   // We only have ideal tilings, so they aren't removed.
   used_tilings.clear();
   active_layer_->CleanUpTilingsOnActiveLayer(used_tilings);
@@ -1211,6 +1232,9 @@ TEST_F(PictureLayerImplTest, CleanUpTilings) {
       1.f * low_res_factor,
       active_layer_->tilings()->tiling_at(3)->contents_scale());
 
+  // Ensure UpdateTiles won't remove any tilings.
+  active_layer_->MarkAllTilingsUsed();
+
   // Mark the non-ideal tilings as used. They won't be removed.
   used_tilings.clear();
   used_tilings.push_back(active_layer_->tilings()->tiling_at(1));
@@ -1303,6 +1327,9 @@ TEST_F(PictureLayerImplTest, DontAddLowResDuringAnimation) {
   EXPECT_BOTH_EQ(LowResTiling()->contents_scale(), low_res_factor);
   EXPECT_BOTH_EQ(num_tilings(), 2u);
 
+  // Ensure UpdateTiles won't remove any tilings.
+  active_layer_->MarkAllTilingsUsed();
+
   // Page scale animation, new high res, but no low res. We still have
   // a tiling at the previous scale, it's just not marked as low res on the
   // active layer. The pending layer drops non-ideal tilings.
@@ -3769,6 +3796,11 @@ TEST_F(NoLowResPictureLayerImplTest, CleanUpTilings) {
   SetContentsScaleOnBothLayers(scale, device_scale, page_scale, 1.f, false);
   ASSERT_EQ(1u, active_layer_->tilings()->num_tilings());
 
+  // Ensure UpdateTiles won't remove any tilings. Note this is unrelated to
+  // |used_tilings| variable, and it's here only to ensure that active_layer_
+  // won't remove tilings before the test has a chance to verify behavior.
+  active_layer_->MarkAllTilingsUsed();
+
   // We only have ideal tilings, so they aren't removed.
   used_tilings.clear();
   active_layer_->CleanUpTilingsOnActiveLayer(used_tilings);
@@ -3797,6 +3829,9 @@ TEST_F(NoLowResPictureLayerImplTest, CleanUpTilings) {
   EXPECT_FLOAT_EQ(1.f,
                   active_layer_->tilings()->tiling_at(1)->contents_scale());
 
+  // Ensure UpdateTiles won't remove any tilings.
+  active_layer_->MarkAllTilingsUsed();
+
   // Mark the non-ideal tilings as used. They won't be removed.
   used_tilings.clear();
   used_tilings.push_back(active_layer_->tilings()->tiling_at(1));

cc/test/fake_picture_layer_impl.h

@@ -121,6 +121,14 @@ class FakePictureLayerImpl : public PictureLayerImpl {
   void ResetAllTilesPriorities();
   PictureLayerTilingSet* GetTilings() { return tilings_.get(); }
 
+  // Add the given tiling as a "used" tiling during AppendQuads. This ensures
+  // that future calls to UpdateTiles don't delete the tiling.
+  void MarkAllTilingsUsed() {
+    last_append_quads_tilings_.clear();
+    for (size_t i = 0; i < tilings_->num_tilings(); ++i)
+      last_append_quads_tilings_.push_back(tilings_->tiling_at(i));
+  }
+
   size_t release_resources_count() const { return release_resources_count_; }
   void reset_release_resources_count() { release_resources_count_ = 0; }