cc: Start using raster/eviction iterators.

cc/resources/picture_layer_tiling.cc

 // Copyright 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "cc/resources/picture_layer_tiling.h"
 
 #include <algorithm>
 #include <cmath>
 #include <limits>
 #include <set>
@@ skipping 36 matching lines @@
       return a_is_occluded;
 
     // Or if a is farther away from visible.
     return a_priority.distance_to_visible > b_priority.distance_to_visible;
   }
 
  private:
   TreePriority tree_priority_;
 };
 
-void ReleaseTile(Tile* tile, WhichTree tree) {
-  // Reset priority as tile is ref-counted and might still be used
-  // even though we no longer hold a reference to it here anymore.
-  tile->SetPriority(tree, TilePriority());
-  tile->set_shared(false);
-}
-
 }  // namespace
 
 scoped_ptr<PictureLayerTiling> PictureLayerTiling::Create(
     float contents_scale,
     const gfx::Size& layer_bounds,
     PictureLayerTilingClient* client) {
   return make_scoped_ptr(new PictureLayerTiling(contents_scale,
                                                 layer_bounds,
                                                 client));
 }
 
 PictureLayerTiling::PictureLayerTiling(float contents_scale,
                                        const gfx::Size& layer_bounds,
                                        PictureLayerTilingClient* client)
     : contents_scale_(contents_scale),
       layer_bounds_(layer_bounds),
       resolution_(NON_IDEAL_RESOLUTION),
       client_(client),
       tiling_data_(gfx::Size(), gfx::Size(), true),
       last_impl_frame_time_in_seconds_(0.0),
+      content_to_screen_scale_(1.0f),

[danakj, 2014/10/08 18:46:25]

can this be 0 to indicate it is set before we use it?

[vmpstr, 2014/10/08 22:38:34]

Done.

+      can_require_tiles_for_activation_(true),

[danakj, 2014/10/08 18:46:26]

can this be false? we should set it before we use it right?

[vmpstr, 2014/10/08 22:38:34]

Done.

       has_visible_rect_tiles_(false),
       has_skewport_rect_tiles_(false),
       has_soon_border_rect_tiles_(false),
       has_eventually_rect_tiles_(false),
       eviction_tiles_cache_valid_(false),
       eviction_cache_tree_priority_(SAME_PRIORITY_FOR_BOTH_TREES) {
   gfx::Size content_bounds =
       gfx::ToCeiledSize(gfx::ScaleSize(layer_bounds, contents_scale));
   gfx::Size tile_size = client_->CalculateTileSize(content_bounds);
   if (tile_size.IsEmpty()) {
     layer_bounds_ = gfx::Size();
     content_bounds = gfx::Size();
   }
 
   DCHECK(!gfx::ToFlooredSize(
       gfx::ScaleSize(layer_bounds, contents_scale)).IsEmpty()) <<
       "Tiling created with scale too small as contents become empty." <<
       " Layer bounds: " << layer_bounds.ToString() <<
       " Contents scale: " << contents_scale;
 
   tiling_data_.SetTilingSize(content_bounds);
   tiling_data_.SetMaxTextureSize(tile_size);
 }
 
 PictureLayerTiling::~PictureLayerTiling() {
   for (TileMap::const_iterator it = tiles_.begin(); it != tiles_.end(); ++it)
-    ReleaseTile(it->second.get(), client_->GetTree());
+    it->second->set_shared(false);
 }
 
 void PictureLayerTiling::SetClient(PictureLayerTilingClient* client) {
   client_ = client;
 }
 
 Tile* PictureLayerTiling::CreateTile(int i,
                                      int j,
                                      const PictureLayerTiling* twin_tiling) {
   TileMapKey key(i, j);
   DCHECK(tiles_.find(key) == tiles_.end());
 
   gfx::Rect paint_rect = tiling_data_.TileBoundsWithBorder(i, j);
   gfx::Rect tile_rect = paint_rect;
   tile_rect.set_size(tiling_data_.max_texture_size());
 
   // Check our twin for a valid tile.
   if (twin_tiling &&
       tiling_data_.max_texture_size() ==
       twin_tiling->tiling_data_.max_texture_size()) {
     if (Tile* candidate_tile = twin_tiling->TileAt(i, j)) {
       gfx::Rect rect =
           gfx::ScaleToEnclosingRect(paint_rect, 1.0f / contents_scale_);
       if (!client_->GetInvalidation()->Intersects(rect)) {
         DCHECK(!candidate_tile->is_shared());
+        DCHECK_EQ(i, candidate_tile->tiling_i_index());
+        DCHECK_EQ(j, candidate_tile->tiling_j_index());
         candidate_tile->set_shared(true);
         tiles_[key] = candidate_tile;
         return candidate_tile;
       }
     }
   }
 
   // Create a new tile because our twin didn't have a valid one.
   scoped_refptr<Tile> tile = client_->CreateTile(this, tile_rect);
   if (tile.get()) {
     DCHECK(!tile->is_shared());
+    tile->set_tiling_index(i, j);
     tiles_[key] = tile;
   }
   return tile.get();
 }
 
 void PictureLayerTiling::CreateMissingTilesInLiveTilesRect() {
   const PictureLayerTiling* twin_tiling = client_->GetTwinTiling(this);
   bool include_borders = false;
   for (TilingData::Iterator iter(
            &tiling_data_, live_tiles_rect_, include_borders);
@@ skipping 302 matching lines @@
 gfx::Size PictureLayerTiling::CoverageIterator::texture_size() const {
   return tiling_->tiling_data_.max_texture_size();
 }
 
 bool PictureLayerTiling::RemoveTileAt(int i,
                                       int j,
                                       PictureLayerTiling* recycled_twin) {
   TileMap::iterator found = tiles_.find(TileMapKey(i, j));
   if (found == tiles_.end())
     return false;
-  ReleaseTile(found->second.get(), client_->GetTree());
+  found->second->set_shared(false);
   tiles_.erase(found);
   if (recycled_twin) {
     // Recycled twin does not also have a recycled twin, so pass NULL.
     recycled_twin->RemoveTileAt(i, j, NULL);
   }
   return true;
 }
 
 void PictureLayerTiling::Reset() {
   live_tiles_rect_ = gfx::Rect();
   PictureLayerTiling* recycled_twin = client_->GetRecycledTwinTiling(this);
   for (TileMap::const_iterator it = tiles_.begin(); it != tiles_.end(); ++it) {
-    ReleaseTile(it->second.get(), client_->GetTree());
+    it->second->set_shared(false);
     if (recycled_twin)
       recycled_twin->RemoveTileAt(it->first.first, it->first.second, NULL);
   }
   tiles_.clear();
 }
 
 gfx::Rect PictureLayerTiling::ComputeSkewport(
     double current_frame_time_in_seconds,
     const gfx::Rect& visible_rect_in_content_space) const {
   gfx::Rect skewport = visible_rect_in_content_space;
@@ skipping 34 matching lines @@
                  extrapolation_multiplier * (old_bottom - new_bottom));
 
   // Clip the skewport to |max_skewport|.
   skewport.Intersect(max_skewport);
 
   // Finally, ensure that visible rect is contained in the skewport.
   skewport.Union(visible_rect_in_content_space);
   return skewport;
 }
 
-void PictureLayerTiling::UpdateTilePriorities(
+void PictureLayerTiling::ComputeTilePriorityRects(
     WhichTree tree,
     const gfx::Rect& viewport_in_layer_space,
     float ideal_contents_scale,
     double current_frame_time_in_seconds,
     const Occlusion& occlusion_in_layer_space) {
   if (!NeedsUpdateForFrameAtTime(current_frame_time_in_seconds)) {
     // This should never be zero for the purposes of has_ever_been_updated().
     DCHECK_NE(current_frame_time_in_seconds, 0.0);
     return;
   }
 
   gfx::Rect visible_rect_in_content_space =
       gfx::ScaleToEnclosingRect(viewport_in_layer_space, contents_scale_);
 
   if (tiling_size().IsEmpty()) {
     last_impl_frame_time_in_seconds_ = current_frame_time_in_seconds;
     last_visible_rect_in_content_space_ = visible_rect_in_content_space;
     return;
   }
 
+  // Calculate the skewport.
+  gfx::Rect skewport = ComputeSkewport(current_frame_time_in_seconds,
+                                       visible_rect_in_content_space);
+  DCHECK(skewport.Contains(visible_rect_in_content_space));
+
+  // Calculate the eventually/live tiles rect.
   size_t max_tiles_for_interest_area = client_->GetMaxTilesForInterestArea();
 
   gfx::Size tile_size = tiling_data_.max_texture_size();
   int64 eventually_rect_area =
       max_tiles_for_interest_area * tile_size.width() * tile_size.height();
 
-  gfx::Rect skewport = ComputeSkewport(current_frame_time_in_seconds,
-                                       visible_rect_in_content_space);
-  DCHECK(skewport.Contains(visible_rect_in_content_space));
-
   gfx::Rect eventually_rect =
       ExpandRectEquallyToAreaBoundedBy(visible_rect_in_content_space,
                                        eventually_rect_area,
                                        gfx::Rect(tiling_size()),
                                        &expansion_cache_);
 
   DCHECK(eventually_rect.IsEmpty() ||
          gfx::Rect(tiling_size()).Contains(eventually_rect))
       << "tiling_size: " << tiling_size().ToString()
       << " eventually_rect: " << eventually_rect.ToString();
 
+  // Calculate the soon border rect.
+  content_to_screen_scale_ = ideal_contents_scale / contents_scale_;
+  gfx::Rect soon_border_rect = visible_rect_in_content_space;
+  float border = kSoonBorderDistanceInScreenPixels / content_to_screen_scale_;
+  soon_border_rect.Inset(-border, -border, -border, -border);
+
+  // Update the tiling state.
   SetLiveTilesRect(eventually_rect);
 
   last_impl_frame_time_in_seconds_ = current_frame_time_in_seconds;
   last_visible_rect_in_content_space_ = visible_rect_in_content_space;
 
   eviction_tiles_cache_valid_ = false;
 
-  TilePriority now_priority(resolution_, TilePriority::NOW, 0);
-  float content_to_screen_scale = ideal_contents_scale / contents_scale_;
-
-  // Assign now priority to all visible tiles.
-  bool include_borders = false;
-  has_visible_rect_tiles_ = false;
-  for (TilingData::Iterator iter(
-           &tiling_data_, visible_rect_in_content_space, include_borders);
-       iter;
-       ++iter) {
-    TileMap::iterator find = tiles_.find(iter.index());
-    if (find == tiles_.end())
-      continue;
-    has_visible_rect_tiles_ = true;
-    Tile* tile = find->second.get();
-
-    tile->SetPriority(tree, now_priority);
-
-    // Set whether tile is occluded or not.
-    gfx::Rect tile_query_rect = ScaleToEnclosingRect(
-        IntersectRects(tile->content_rect(), visible_rect_in_content_space),
-        1.0f / contents_scale_);
-    bool is_occluded = occlusion_in_layer_space.IsOccluded(tile_query_rect);
-    tile->set_is_occluded(tree, is_occluded);
-  }
-
-  // Assign soon priority to skewport tiles.
-  has_skewport_rect_tiles_ = false;
-  for (TilingData::DifferenceIterator iter(
-           &tiling_data_, skewport, visible_rect_in_content_space);
-       iter;
-       ++iter) {
-    TileMap::iterator find = tiles_.find(iter.index());
-    if (find == tiles_.end())
-      continue;
-    has_skewport_rect_tiles_ = true;
-    Tile* tile = find->second.get();
-
-    gfx::Rect tile_bounds =
-        tiling_data_.TileBounds(iter.index_x(), iter.index_y());
-
-    float distance_to_visible =
-        visible_rect_in_content_space.ManhattanInternalDistance(tile_bounds) *
-        content_to_screen_scale;
-
-    TilePriority priority(resolution_, TilePriority::SOON, distance_to_visible);
-    tile->SetPriority(tree, priority);
-  }
-
-  // Assign eventually priority to interest rect tiles.
-  has_eventually_rect_tiles_ = false;
-  for (TilingData::DifferenceIterator iter(
-           &tiling_data_, eventually_rect, skewport);
-       iter;
-       ++iter) {
-    TileMap::iterator find = tiles_.find(iter.index());
-    if (find == tiles_.end())
-      continue;
-    has_eventually_rect_tiles_ = true;
-    Tile* tile = find->second.get();
-
-    gfx::Rect tile_bounds =
-        tiling_data_.TileBounds(iter.index_x(), iter.index_y());
-
-    float distance_to_visible =
-        visible_rect_in_content_space.ManhattanInternalDistance(tile_bounds) *
-        content_to_screen_scale;
-    TilePriority priority(
-        resolution_, TilePriority::EVENTUALLY, distance_to_visible);
-    tile->SetPriority(tree, priority);
-  }
-
-  // Upgrade the priority on border tiles to be SOON.
-  gfx::Rect soon_border_rect = visible_rect_in_content_space;
-  float border = kSoonBorderDistanceInScreenPixels / content_to_screen_scale;
-  soon_border_rect.Inset(-border, -border, -border, -border);
-  has_soon_border_rect_tiles_ = false;
-  for (TilingData::DifferenceIterator iter(
-           &tiling_data_, soon_border_rect, skewport);
-       iter;
-       ++iter) {
-    TileMap::iterator find = tiles_.find(iter.index());
-    if (find == tiles_.end())
-      continue;
-    has_soon_border_rect_tiles_ = true;
-    Tile* tile = find->second.get();
-
-    TilePriority priority(resolution_,
-                          TilePriority::SOON,
-                          tile->priority(tree).distance_to_visible);
-    tile->SetPriority(tree, priority);
-  }
-
-  // Update iteration rects.
   current_visible_rect_ = visible_rect_in_content_space;
   current_skewport_rect_ = skewport;
   current_soon_border_rect_ = soon_border_rect;
   current_eventually_rect_ = eventually_rect;
+  current_occlusion_in_layer_space_ = occlusion_in_layer_space;
+
+  // Update has_*_tiles state.
+  gfx::Rect tiling_rect(tiling_size());
+
+  has_visible_rect_tiles_ = tiling_rect.Intersects(current_visible_rect_);
+  has_skewport_rect_tiles_ = tiling_rect.Intersects(current_skewport_rect_);
+  has_soon_border_rect_tiles_ =
+      tiling_rect.Intersects(current_soon_border_rect_);
+  has_eventually_rect_tiles_ = tiling_rect.Intersects(current_eventually_rect_);
 }
 
 void PictureLayerTiling::SetLiveTilesRect(
     const gfx::Rect& new_live_tiles_rect) {
   DCHECK(new_live_tiles_rect.IsEmpty() ||
          gfx::Rect(tiling_size()).Contains(new_live_tiles_rect))
       << "tiling_size: " << tiling_size().ToString()
       << " new_live_tiles_rect: " << new_live_tiles_rect.ToString();
   if (live_tiles_rect_ == new_live_tiles_rect)
     return;
@@ skipping 40 matching lines @@
     DCHECK(tiling_data_.TileBounds(it->first.first, it->first.second)
                .Intersects(live_tiles_rect_))
         << this << " " << it->first.first << "," << it->first.second
         << " tile bounds "
         << tiling_data_.TileBounds(it->first.first, it->first.second).ToString()
         << " live_tiles_rect " << live_tiles_rect_.ToString();
   }
 #endif
 }
 
-void PictureLayerTiling::DidBecomeRecycled() {
-  // DidBecomeActive below will set the active priority for tiles that are
-  // still in the tree. Calling this first on an active tiling that is becoming
-  // recycled takes care of tiles that are no longer in the active tree (eg.
-  // due to a pending invalidation).
-  for (TileMap::const_iterator it = tiles_.begin(); it != tiles_.end(); ++it) {
-    it->second->SetPriority(ACTIVE_TREE, TilePriority());
+bool PictureLayerTiling::IsTileOccluded(const Tile* tile) const {
+  DCHECK(tile);
+  // TODO(vmpstr): Skip this work if occlusion is empty (this needs a bool

[danakj, 2014/10/08 18:46:25]

This bool function exists now right?

[vmpstr, 2014/10/08 22:38:34]

Done.

+  // function on occlusion).
+  gfx::Rect tile_query_rect =
+      IntersectRects(tile->content_rect(), current_visible_rect_);

[danakj, 2014/10/08 18:46:25]

nit: gfx::IntersectRects

[vmpstr, 2014/10/08 22:38:34]

Done.

+
+  if (contents_scale_ != 1.f) {
+    tile_query_rect =
+        ScaleToEnclosingRect(tile_query_rect, 1.0f / contents_scale_);

[danakj, 2014/10/08 18:46:26]

nit: gfx::ScaleToE..

[vmpstr, 2014/10/08 22:38:34]

Done.

   }
+
+  // Explicitly check if the tile is outside the viewport. If so, we need to
+  // return false, since occlusion for this tile is unknown.
+  // TODO(vmpstr): Since the current visible rect is really a viewport in
+  // layer space, we should probably clip tile query rect to tiling bounds
+  // or live tiles rect.
+  if (tile_query_rect.IsEmpty())

[danakj, 2014/10/08 18:46:25]

nit: do this before scaling, scaling shouldn't make it empty, and we don't need
to scale if it is empty

[vmpstr, 2014/10/08 22:38:34]

Done.

+    return false;
+  return current_occlusion_in_layer_space_.IsOccluded(tile_query_rect);
 }
 
-void PictureLayerTiling::DidBecomeActive() {
-  PicturePileImpl* active_pile = client_->GetPile();
-  for (TileMap::const_iterator it = tiles_.begin(); it != tiles_.end(); ++it) {
-    it->second->SetPriority(ACTIVE_TREE, it->second->priority(PENDING_TREE));
-    it->second->SetPriority(PENDING_TREE, TilePriority());
+bool PictureLayerTiling::IsTileRequiredForActivation(const Tile* tile) const {

[danakj, 2014/10/08 18:46:25]

can you add a comment somewhere here saying this is only called for visible
tiles? (or a dcheck for bonus points if that's easy to do)

[vmpstr, 2014/10/08 22:38:34]

Done.

+  DCHECK_EQ(PENDING_TREE, client_->GetTree());
 
-    // Tile holds a ref onto a picture pile. If the tile never gets invalidated
-    // and recreated, then that picture pile ref could exist indefinitely.  To
-    // prevent this, ask the client to update the pile to its own ref.  This
-    // will cause PicturePileImpls to get deleted once the corresponding
-    // PictureLayerImpl and any in flight raster jobs go out of scope.
-    it->second->set_picture_pile(active_pile);
+  // If we are not allowed to mark tiles as required for activation, then don't
+  // do it.
+  if (!can_require_tiles_for_activation_)
+    return false;
+
+  if (resolution_ != HIGH_RESOLUTION)
+    return false;
+
+  if (IsTileOccluded(tile))
+    return false;
+
+  if (client_->RequiresHighResToDraw())
+    return true;
+
+  const PictureLayerTiling* twin_tiling = client_->GetTwinTiling(this);
+  if (!twin_tiling)
+    return true;
+
+  if (twin_tiling->layer_bounds() != layer_bounds())
+    return true;
+
+  if (twin_tiling->current_visible_rect_ != current_visible_rect_)
+    return true;
+
+  Tile* twin_tile =
+      twin_tiling->TileAt(tile->tiling_i_index(), tile->tiling_j_index());
+  // If twin tile is missing, it might not have a recording, so we don't need
+  // this tile to be required for activation.
+  if (!twin_tile)
+    return false;
+
+  return true;
+}
+
+void PictureLayerTiling::UpdateTileAndTwinPriority(Tile* tile) const {
+  UpdateTilePriority(tile);
+
+  const PictureLayerTiling* twin_tiling = client_->GetTwinTiling(this);
+  if (!tile->is_shared() || !twin_tiling) {
+    WhichTree tree = client_->GetTree();
+    WhichTree twin_tree = tree == ACTIVE_TREE ? PENDING_TREE : ACTIVE_TREE;
+    tile->SetPriority(twin_tree, TilePriority());
+    tile->set_is_occluded(twin_tree, false);
+    if (twin_tree == PENDING_TREE)

[danakj, 2014/10/08 18:46:25]

nit: it's probly cheaper to just set_required all the time than to branch, and
it's harmless to do so right?

[vmpstr, 2014/10/08 22:38:34]

Not really harmless in general, since we have to ensure that required for
activation property is only touched from the pending tree... The active tree
might make a wrong decision and since this is a bool on tile, it would override
the previous value that the pending tree has set. As a follow-up when I clean up
tiles, I think I might make this a per tree setting on tile, since then the code
here is easier and we just have to ensure to read off the right thing when we
need it.

+      tile->set_required_for_activation(false);
+    return;
   }
+
+  twin_tiling->UpdateTilePriority(tile);
+}
+
+void PictureLayerTiling::UpdateTilePriority(Tile* tile) const {
+  // TODO(vmpstr): This code should return the priority instead of setting it on
+  // the tile. This should be a part of the change to move tile priority from
+  // tiles into iterators.
+  WhichTree tree = client_->GetTree();
+
+  if (tree == PENDING_TREE)

[danakj, 2014/10/08 18:46:26]

same here, can just set false always?

[vmpstr, 2014/10/08 22:38:34]

We have ensure that we don't set that from the active tree

Same as above.

+    tile->set_required_for_activation(false);
+  tile->set_is_occluded(tree, false);

[danakj, 2014/10/08 18:46:26]

move these below the early out so we don't set these values twice?

[vmpstr, 2014/10/08 22:38:34]

Done.

+
+  DCHECK_EQ(TileAt(tile->tiling_i_index(), tile->tiling_j_index()), tile);
+  gfx::Rect tile_bounds =
+      tiling_data_.TileBounds(tile->tiling_i_index(), tile->tiling_j_index());
+
+  if (current_visible_rect_.Intersects(tile_bounds)) {
+    tile->SetPriority(tree, TilePriority(resolution_, TilePriority::NOW, 0));
+    if (tree == PENDING_TREE)
+      tile->set_required_for_activation(IsTileRequiredForActivation(tile));
+    tile->set_is_occluded(tree, IsTileOccluded(tile));
+    return;
+  }
+
+  float distance_to_visible =
+      current_visible_rect_.ManhattanInternalDistance(tile_bounds) *
+      content_to_screen_scale_;
+
+  if (current_soon_border_rect_.Intersects(tile_bounds) ||
+      current_skewport_rect_.Intersects(tile_bounds)) {
+    tile->SetPriority(
+        tree,
+        TilePriority(resolution_, TilePriority::SOON, distance_to_visible));
+    return;
+  }
+
+  tile->SetPriority(
+      tree,
+      TilePriority(resolution_, TilePriority::EVENTUALLY, distance_to_visible));
 }
 
 void PictureLayerTiling::GetAllTilesForTracing(
     std::set<const Tile*>* tiles) const {
   for (TileMap::const_iterator it = tiles_.begin(); it != tiles_.end(); ++it)
     tiles->insert(it->second.get());
 }
 
 void PictureLayerTiling::AsValueInto(base::debug::TracedValue* state) const {
   state->SetInteger("num_tiles", tiles_.size());
@@ skipping 170 matching lines @@
     return;
 
   eviction_tiles_now_.clear();
   eviction_tiles_now_and_required_for_activation_.clear();
   eviction_tiles_soon_.clear();
   eviction_tiles_soon_and_required_for_activation_.clear();
   eviction_tiles_eventually_.clear();
   eviction_tiles_eventually_and_required_for_activation_.clear();
 
   for (TileMap::iterator it = tiles_.begin(); it != tiles_.end(); ++it) {
-    // TODO(vmpstr): This should update the priority if UpdateTilePriorities
-    // changes not to do this.
     Tile* tile = it->second.get();
+    UpdateTileAndTwinPriority(tile);
     const TilePriority& priority =
         tile->priority_for_tree_priority(tree_priority);
     switch (priority.priority_bin) {
       case TilePriority::EVENTUALLY:
         if (tile->required_for_activation())
           eviction_tiles_eventually_and_required_for_activation_.push_back(
               tile);
         else
           eviction_tiles_eventually_.push_back(tile);
         break;
@@ skipping 54 matching lines @@
       return &eviction_tiles_now_and_required_for_activation_;
   }
   NOTREACHED();
   return &eviction_tiles_eventually_;
 }
 
 PictureLayerTiling::TilingRasterTileIterator::TilingRasterTileIterator()
     : tiling_(NULL), current_tile_(NULL) {}
 
 PictureLayerTiling::TilingRasterTileIterator::TilingRasterTileIterator(
-    PictureLayerTiling* tiling,
-    WhichTree tree)
-    : tiling_(tiling), phase_(VISIBLE_RECT), tree_(tree), current_tile_(NULL) {
+    PictureLayerTiling* tiling)
+    : tiling_(tiling), phase_(VISIBLE_RECT), current_tile_(NULL) {
   if (!tiling_->has_visible_rect_tiles_) {
     AdvancePhase();
     return;
   }
 
   visible_iterator_ = TilingData::Iterator(&tiling_->tiling_data_,
                                            tiling_->current_visible_rect_,
                                            false /* include_borders */);
   if (!visible_iterator_) {
     AdvancePhase();
     return;
   }
 
   current_tile_ =
       tiling_->TileAt(visible_iterator_.index_x(), visible_iterator_.index_y());
-  if (!current_tile_ || !TileNeedsRaster(current_tile_))
+  if (!current_tile_ || !TileNeedsRaster(current_tile_)) {
     ++(*this);
+    return;
+  }
+  tiling_->UpdateTileAndTwinPriority(current_tile_);

[danakj, 2014/10/08 18:46:26]

while iterating you always update the priority of the tile before returning, but
here you don't if the first tile doesn't need raster. should this be consistent?
could this cause bugs?

[vmpstr, 2014/10/08 22:38:34]

The priority is only updated for returned values. That's true of the rest of the
code here as well. For instance, operator++ has something along the lines of

while (!current_tile_ || !TileNeedsRaster(current_tile_)) {
 ...
}

if (current_tile_)
  tiling_->UpdateTileAndTwinPriority(current_tile_);

[danakj, 2014/10/08 23:38:01]

Right, but if the first tile in the iterator didn't need raster, wouldn't we
have a current_tile_ (ie operator* would have a tile) but it's priorities
wouldn't be updated?

[danakj, 2014/10/08 23:39:27]

Or would operator bool be false? I'm not sure why it would?

[vmpstr, 2014/10/09 00:01:42]

operator* can only return a tile that needs raster for this iterator.. That is,
we skip all tiles that don't need raster silently. If we run out of tiles,
current_tile_ becomes NULL, which is what operator bool checks. 

So basically, we can't have a situation where we would return a tile that
doesn't have its priorities updated and at the same time we can't return a tile
that doesn't need raster. All other cases are consistent in that we don't update
the tile priorities... At least that's the design.

[danakj, 2014/10/09 00:03:26]

Oh, ok ++ above would end up with a NULL tile in that case then I guess. OK
thanks!

 }
 
 PictureLayerTiling::TilingRasterTileIterator::~TilingRasterTileIterator() {}
 
 void PictureLayerTiling::TilingRasterTileIterator::AdvancePhase() {
   DCHECK_LT(phase_, EVENTUALLY_RECT);
 
   do {
     phase_ = static_cast<Phase>(phase_ + 1);
     switch (phase_) {
@@ skipping 40 matching lines @@
       if (current_tile_ && TileNeedsRaster(current_tile_))
         break;
       ++spiral_iterator_;
     }
 
     if (!spiral_iterator_ && phase_ == EVENTUALLY_RECT) {
       current_tile_ = NULL;
       break;
     }
   } while (!spiral_iterator_);
+
+  if (current_tile_)
+    tiling_->UpdateTileAndTwinPriority(current_tile_);
 }
 
 PictureLayerTiling::TilingRasterTileIterator&
 PictureLayerTiling::TilingRasterTileIterator::
 operator++() {
   current_tile_ = NULL;
   while (!current_tile_ || !TileNeedsRaster(current_tile_)) {
     std::pair<int, int> next_index;
     switch (phase_) {
       case VISIBLE_RECT:
@@ skipping 17 matching lines @@
         ++spiral_iterator_;
         if (!spiral_iterator_) {
           current_tile_ = NULL;
           return *this;
         }
         next_index = spiral_iterator_.index();
         break;
     }
     current_tile_ = tiling_->TileAt(next_index.first, next_index.second);
   }
+
+  if (current_tile_)
+    tiling_->UpdateTileAndTwinPriority(current_tile_);
   return *this;
 }
 
 PictureLayerTiling::TilingEvictionTileIterator::TilingEvictionTileIterator()
     : eviction_tiles_(NULL), current_eviction_tiles_index_(0u) {
 }
 
 PictureLayerTiling::TilingEvictionTileIterator::TilingEvictionTileIterator(
     PictureLayerTiling* tiling,
     TreePriority tree_priority,
@@ skipping 30 matching lines @@
   DCHECK(*this);
   do {
     ++current_eviction_tiles_index_;
   } while (current_eviction_tiles_index_ != eviction_tiles_->size() &&
            !(*eviction_tiles_)[current_eviction_tiles_index_]->HasResources());
 
   return *this;
 }
 
 }  // namespace cc