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

cc/layers/picture_layer_impl.cc

Index: cc/layers/picture_layer_impl.cc
diff --git a/cc/layers/picture_layer_impl.cc b/cc/layers/picture_layer_impl.cc
index 813bc47877c1f41cd803f2ab5ea4b5c9c45ec01d..1e36b93406b447d86e70d2332020f7b0f8a187f9 100644
--- a/cc/layers/picture_layer_impl.cc
+++ b/cc/layers/picture_layer_impl.cc
@@ -84,7 +84,8 @@ PictureLayerImpl::PictureLayerImpl(LayerTreeImpl* tree_impl,
       was_screen_space_transform_animating_(false),
       only_used_low_res_last_append_quads_(false),
       is_mask_(is_mask),
-      nearest_neighbor_(false) {
+      nearest_neighbor_(false),
+      need_tiling_cleanup_(false) {
   layer_tree_impl()->RegisterPictureLayerImpl(this);
 }
 
@@ -276,7 +277,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
@@ -403,8 +404,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) {
@@ -421,7 +424,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,
@@ -442,6 +445,16 @@ 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. Additionally, only clean up if AppendQuads didn't have

[danakj, 2015/01/27 23:02:28]

This is just an optimization because it wouldn't actually remove anything then
right? Sorry I was hoping for a more "why" than an explanation about
need_tiling_cleanup_, which I think is clear.

+  // a chance to do so, as inidicated by |need_tiling_cleanup_|. That is to say,
+  // if AppendQuads cleaned up tilings, then |need_tiling_cleanup_| would be set
+  // to false.
+  if (need_tiling_cleanup_ && GetTree() == ACTIVE_TREE)
+    CleanUpTilingsOnActiveLayer(last_append_quads_tilings_);
+
   UpdateIdealScales();
 
   if (!raster_contents_scale_ || ShouldAdjustRasterScale()) {
@@ -460,7 +473,6 @@ bool PictureLayerImpl::UpdateTiles(const Occlusion& occlusion_in_content_space,
 
   if (draw_transform_is_animating())
     raster_source_->SetShouldAttemptToUseDistanceFieldText();
-
   return UpdateTilePriorities(occlusion_in_content_space);
 }
 
@@ -779,6 +791,7 @@ PictureLayerTiling* PictureLayerImpl::AddTiling(float contents_scale) {
   DCHECK_GE(contents_scale, MinimumContentsScale());
   DCHECK_LE(contents_scale, MaximumContentsScale());
   DCHECK(raster_source_->HasRecordings());
+  need_tiling_cleanup_ = true;

[danakj, 2015/01/27 23:02:28]

What about when we UpdateRasterSource and clone the tiling from the active tree?
That makes a tiling here but doesn't set need_tiling_cleanup_. But then you
wouldn't want to cleanup after UpdateRasterSource until after AppendQuads had a
chance to go. (I a-little-bit feel like this bool is making things more
complicated than it's worth worrying about? Or maybe we could just set this to
true in UpdateTiles after it attempts to do the cleanup?)

   return tilings_->AddTiling(contents_scale, raster_source_);
 }
 
@@ -992,7 +1005,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;
@@ -1024,6 +1037,7 @@ void PictureLayerImpl::CleanUpTilingsOnActiveLayer(
 
   if (recycled_twin_set && recycled_twin_set->num_tilings() == 0)
     recycled_twin->ResetRasterScale();
+  need_tiling_cleanup_ = false;
 
   DCHECK_GT(tilings_->num_tilings(), 0u);
   SanityCheckTilingState();