cc: Remove tile sharing from tilings.

cc/resources/picture_layer_tiling.cc

Index: cc/resources/picture_layer_tiling.cc
diff --git a/cc/resources/picture_layer_tiling.cc b/cc/resources/picture_layer_tiling.cc
index 51c8ca735c1b908e410dfc1cc32625fa944727f7..c9618f496b3733d633616da87b2fa0c4d58c9b35 100644
--- a/cc/resources/picture_layer_tiling.cc
+++ b/cc/resources/picture_layer_tiling.cc
@@ -79,8 +79,6 @@ PictureLayerTiling::PictureLayerTiling(
 }
 
 PictureLayerTiling::~PictureLayerTiling() {
-  for (TileMap::const_iterator it = tiles_.begin(); it != tiles_.end(); ++it)
-    it->second->set_shared(false);
 }
 
 // static
@@ -94,13 +92,7 @@ float PictureLayerTiling::CalculateSoonBorderDistance(
       max_dimension * kSoonBorderDistanceViewportPercentage);
 }
 
-Tile* PictureLayerTiling::CreateTile(int i,
-                                     int j,
-                                     const PictureLayerTiling* twin_tiling,
-                                     PictureLayerTiling* recycled_twin) {
-  // Can't have both a (pending or active) twin and a recycled twin tiling.
-  DCHECK_IMPLIES(twin_tiling, !recycled_twin);
-  DCHECK_IMPLIES(recycled_twin, !twin_tiling);
+Tile* PictureLayerTiling::CreateTile(int i, int j) {
   TileMapKey key(i, j);
   DCHECK(tiles_.find(key) == tiles_.end());
 
@@ -108,116 +100,66 @@ Tile* PictureLayerTiling::CreateTile(int i,
   gfx::Rect tile_rect = paint_rect;
   tile_rect.set_size(tiling_data_.max_texture_size());
 
-  // Check our twin for a valid tile.

[enne (OOO), 2015/04/09 17:54:23]

I'm not 100% opposed to different classes for pending and active in the future,
but having the side by side diff makes this a lot easier to see this amazing
code deletion.

[vmpstr, 2015/04/10 20:25:13]

Acknowledged.

-  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_);
-      const Region* invalidation = client_->GetPendingInvalidation();
-      if (!invalidation || !invalidation->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;
-      }
-    }
-  }
-
   if (!raster_source_->CoversRect(tile_rect, contents_scale_))
     return nullptr;
 
-  // Create a new tile because our twin didn't have a valid one.
   scoped_refptr<Tile> tile = client_->CreateTile(contents_scale_, tile_rect);
-  DCHECK(!tile->is_shared());
   tile->set_tiling_index(i, j);
   tiles_[key] = tile;
-
-  if (recycled_twin) {
-    DCHECK(recycled_twin->tiles_.find(key) == recycled_twin->tiles_.end());
-    // Do what recycled_twin->CreateTile() would do.
-    tile->set_shared(true);
-    recycled_twin->tiles_[key] = tile;
-  }
   return tile.get();
 }
 
 void PictureLayerTiling::CreateMissingTilesInLiveTilesRect() {
-  const PictureLayerTiling* twin_tiling =
-      client_->GetPendingOrActiveTwinTiling(this);
-  // There is no recycled twin during commit from the main thread which is when
-  // this occurs.
-  PictureLayerTiling* null_recycled_twin = nullptr;
-  DCHECK_EQ(null_recycled_twin, client_->GetRecycledTwinTiling(this));
   bool include_borders = false;
-  for (TilingData::Iterator iter(
-           &tiling_data_, live_tiles_rect_, include_borders);
-       iter;
-       ++iter) {
+  for (TilingData::Iterator iter(&tiling_data_, live_tiles_rect_,
+                                 include_borders);
+       iter; ++iter) {
     TileMapKey key = iter.index();
     TileMap::iterator find = tiles_.find(key);
     if (find != tiles_.end())
       continue;
-    CreateTile(key.first, key.second, twin_tiling, null_recycled_twin);
-  }
 
+    if (ShouldCreateTileAt(key.first, key.second))
+      CreateTile(key.first, key.second);
+  }
   VerifyLiveTilesRect(false);
 }
 
-void PictureLayerTiling::CloneTilesAndPropertiesFrom(
-    const PictureLayerTiling& twin_tiling) {
-  DCHECK_EQ(&twin_tiling, client_->GetPendingOrActiveTwinTiling(this));
-
-  SetRasterSourceAndResize(twin_tiling.raster_source_);
-  DCHECK_EQ(twin_tiling.contents_scale_, contents_scale_);
-  DCHECK_EQ(twin_tiling.raster_source_, raster_source_);
-  DCHECK_EQ(twin_tiling.tile_size().ToString(), tile_size().ToString());
-
-  resolution_ = twin_tiling.resolution_;
-
-  SetLiveTilesRect(twin_tiling.live_tiles_rect());
-
-  // Recreate unshared tiles.
+void PictureLayerTiling::TakeTilesAndPropertiesFrom(
+    PictureLayerTiling* pending_twin) {
+  SetRasterSourceAndResize(pending_twin->raster_source_);
   std::vector<TileMapKey> to_remove;
-  for (const auto& tile_map_pair : tiles_) {
-    TileMapKey key = tile_map_pair.first;
-    Tile* tile = tile_map_pair.second.get();
-    if (!tile->is_shared())
-      to_remove.push_back(key);
-  }
-  // The recycled twin does not exist since there is a pending twin (which is
-  // |twin_tiling|).
-  PictureLayerTiling* null_recycled_twin = nullptr;
-  DCHECK_EQ(null_recycled_twin, client_->GetRecycledTwinTiling(this));
-  for (const auto& key : to_remove) {
-    RemoveTileAt(key.first, key.second, null_recycled_twin);
-    CreateTile(key.first, key.second, &twin_tiling, null_recycled_twin);
+  // If the new raster source covers a tile, then update the tile's raster
+  // source. Otherwise, the tile has to be removed since it's not fully covered.
+  // TODO(vmpstr): Figure out how to pass "remove a tile" information from

[enne (OOO), 2015/04/09 17:54:24]

Is this TODO an optimization, in that you think checking all these CoversRect
calls are inefficient? I guess in the past we issues invalidations for all of
the missing recordings and that would drop the tiles and force them to not be
recreated.

Now that https://codereview.chromium.org/1061203002 has landed, the recording
rect should be always correct, because there can no longer be holes in it.  You
should just be able to throw away anything outside of it.  Not sure if that's
more efficient or not.

Maybe SetRasterSourceAndResize could do this with the recorded region as well as
the layer bounds? *thinking out loud*

[vmpstr, 2015/04/10 20:25:13]

This is both an optimization and a simplification imo. The reason is that when
we attempt to create a tile on the pending tree, if we fail to do so, we can
just either store something in tile_map or have a separate container that says
something along the lines of "this tile would exist if it was possible to create
it". When activating (in this function), we could iterate that container and
remove those tiles since with the new raster source we know they can't exist. 

The loop here does exactly the same thing, except it checks for every tile if it
is possible to create it with the new raster source. 

Another possible todo here is to not store raster_source on a tile, but on a
tiling and return it out of an iterator. If that happens, then this whole loop
(both if and else parts) can be removed.

+  // pending to active.
+  for (const auto& tile_pair : tiles_) {
+    Tile* tile = tile_pair.second.get();
+    if (raster_source_->CoversRect(tile->content_rect(),

[danakj, 2015/04/08 00:13:06]

Doing scaling math for every tile here is a bit sad, but I guess the above TODO
covers it.

[vmpstr, 2015/04/10 20:25:13]

Yeep.

+                                   tile->contents_scale())) {
+      tile->set_raster_source(raster_source_.get());
+    } else {
+      to_remove.push_back(tile_pair.first);
+    }
   }
+  for (const auto& key : to_remove)
+    RemoveTileAt(key.first, key.second);
 
-  // Create any missing tiles from the |twin_tiling|.
-  for (const auto& tile_map_pair : twin_tiling.tiles_) {
-    TileMapKey key = tile_map_pair.first;
-    Tile* tile = tile_map_pair.second.get();
-    if (!tile->is_shared())
-      CreateTile(key.first, key.second, &twin_tiling, null_recycled_twin);
-  }
+  resolution_ = pending_twin->resolution_;
+  SetLiveTilesRect(pending_twin->live_tiles_rect());
+
+  for (const auto& tile_pair : pending_twin->tiles_)

[danakj, 2015/04/08 00:13:06]

can you // this is copying the tiles_ from the pending twin or something, i was
like wat. or is there a way to make this more clear in the code?

how about insert(pending_twin->tiles_.begin(), pending_twin->tiles_.end())? But
then that does't replace does it. This operator[] per tile seems both hard to
read and performancely problematic, but maybe there is not a better way. What
about swap() and insert?

[vmpstr, 2015/04/10 20:25:13]

I'm not sure what is bad here? Any method (short of swap) would have to do an
equivalent of operator[] at least once, which this does. Swap/insert doesn't
really work for the reasons you mentioned: we have to both create new tiles and
override old tiles. It's also a hash_map, so I don't think it's that bad.

+    tiles_[tile_pair.first] = tile_pair.second;
+  pending_twin->tiles_.clear();
 
-  DCHECK_EQ(twin_tiling.tiles_.size(), tiles_.size());
-#if DCHECK_IS_ON()
-  for (const auto& tile_map_pair : tiles_)
-    DCHECK(tile_map_pair.second->is_shared());
   VerifyLiveTilesRect(false);
-#endif
 
-  UpdateTilePriorityRects(twin_tiling.current_content_to_screen_scale_,
-                          twin_tiling.current_visible_rect_,
-                          twin_tiling.current_skewport_rect_,
-                          twin_tiling.current_soon_border_rect_,
-                          twin_tiling.current_eventually_rect_,
-                          twin_tiling.current_occlusion_in_layer_space_);
+  SetTilePriorityRects(pending_twin->current_content_to_screen_scale_,
+                       pending_twin->current_visible_rect_,
+                       pending_twin->current_skewport_rect_,
+                       pending_twin->current_soon_border_rect_,
+                       pending_twin->current_eventually_rect_,
+                       pending_twin->current_occlusion_in_layer_space_);
 }
 
 void PictureLayerTiling::SetRasterSourceAndResize(
@@ -270,36 +212,41 @@ void PictureLayerTiling::SetRasterSourceAndResize(
 
   // There is no recycled twin since this is run on the pending tiling
   // during commit, and on the active tree during activate.
-  PictureLayerTiling* null_recycled_twin = nullptr;
-  DCHECK_EQ(null_recycled_twin, client_->GetRecycledTwinTiling(this));
-
   // Drop tiles outside the new layer bounds if the layer shrank.
   for (int i = after_right + 1; i <= before_right; ++i) {
     for (int j = before_top; j <= before_bottom; ++j)
-      RemoveTileAt(i, j, null_recycled_twin);
+      RemoveTileAt(i, j);
   }
   for (int i = before_left; i <= after_right; ++i) {
     for (int j = after_bottom + 1; j <= before_bottom; ++j)
-      RemoveTileAt(i, j, null_recycled_twin);
+      RemoveTileAt(i, j);
   }
 
-  // If the layer grew, the live_tiles_rect_ is not changed, but a new row
-  // and/or column of tiles may now exist inside the same live_tiles_rect_.
-  const PictureLayerTiling* twin_tiling =
-      client_->GetPendingOrActiveTwinTiling(this);
   if (after_right > before_right) {
     DCHECK_EQ(after_right, before_right + 1);
-    for (int j = before_top; j <= after_bottom; ++j)
-      CreateTile(after_right, j, twin_tiling, null_recycled_twin);
+    for (int j = before_top; j <= after_bottom; ++j) {
+      if (ShouldCreateTileAt(after_right, j))
+        CreateTile(after_right, j);
+    }
   }
   if (after_bottom > before_bottom) {
     DCHECK_EQ(after_bottom, before_bottom + 1);
-    for (int i = before_left; i <= before_right; ++i)
-      CreateTile(i, after_bottom, twin_tiling, null_recycled_twin);
+    for (int i = before_left; i <= before_right; ++i) {
+      if (ShouldCreateTileAt(i, after_bottom))
+        CreateTile(i, after_bottom);
+    }
   }
 }
 
 void PictureLayerTiling::Invalidate(const Region& layer_invalidation) {
+  // We don't need to invalidate the pending tiling, since
+  // CreateMissingTilesInLiveTilesRect will populate all the tiles that we need.
+  if (client_->GetTree() == PENDING_TREE)
+    return;
+
+  // We only invalidate the active tiling when it's orphaned: it has no pending
+  // twin, so it's slated for removal in the future.
+  DCHECK(!client_->GetPendingOrActiveTwinTiling(this));
   if (live_tiles_rect_.IsEmpty())
     return;
   std::vector<TileMapKey> new_tile_keys;
@@ -323,41 +270,75 @@ void PictureLayerTiling::Invalidate(const Region& layer_invalidation) {
     // Since the content_rect includes border pixels already, don't include
     // borders when iterating to avoid double counting them.
     bool include_borders = false;
-    for (TilingData::Iterator iter(
-             &tiling_data_, content_rect, include_borders);
-         iter;
-         ++iter) {
-      // There is no recycled twin for the pending tree during commit, or for
-      // the active tree during activation.
-      PictureLayerTiling* null_recycled_twin = nullptr;
-      DCHECK_EQ(null_recycled_twin, client_->GetRecycledTwinTiling(this));
-      if (RemoveTileAt(iter.index_x(), iter.index_y(), null_recycled_twin))
+    for (
+        TilingData::Iterator iter(&tiling_data_, content_rect, include_borders);
+        iter; ++iter) {
+      if (RemoveTileAt(iter.index_x(), iter.index_y()))
         new_tile_keys.push_back(iter.index());
     }
   }
 
-  if (!new_tile_keys.empty()) {
-    // During commit from the main thread, invalidations can never be shared
-    // with the active tree since the active tree has different content there.
-    // And when invalidating an active-tree tiling, it means there was no
-    // pending tiling to clone from.
-    const PictureLayerTiling* null_twin_tiling = nullptr;
-    PictureLayerTiling* null_recycled_twin = nullptr;
-    DCHECK_EQ(null_recycled_twin, client_->GetRecycledTwinTiling(this));
-    for (size_t i = 0; i < new_tile_keys.size(); ++i) {
-      CreateTile(new_tile_keys[i].first, new_tile_keys[i].second,
-                 null_twin_tiling, null_recycled_twin);
-    }
-  }
+  for (const auto& key : new_tile_keys)
+    CreateTile(key.first, key.second);
 }
 
 void PictureLayerTiling::SetRasterSourceOnTiles() {
-  // Shared (ie. non-invalidated) tiles on the pending tree are updated to use
-  // the new raster source. When this raster source is activated, the raster
-  // source will remain valid for shared tiles in the active tree.
-  for (TileMap::const_iterator it = tiles_.begin(); it != tiles_.end(); ++it)
-    it->second->set_raster_source(raster_source_);
-  VerifyLiveTilesRect(false);
+  if (client_->GetTree() == PENDING_TREE)
+    return;
+
+  // TODO(vmpstr): Make iterators return the raster source instead.
+  for (auto& tile_pair : tiles_)
+    tile_pair.second->set_raster_source(raster_source_.get());
+}
+
+bool PictureLayerTiling::ShouldCreateTileAt(int i, int j) const {
+  // Active tree should always create a tile.

[enne (OOO), 2015/04/09 17:54:23]

O_O

Can the active tree DCHECK that its raster source covers this rect?

[vmpstr, 2015/04/10 20:25:13]

So this is the reason I split the classes into two initially :) This function
wouldn't exist on the active tree at all, we'd just call CreateTile instead of
if (ShouldCreateTile()) CreateTile(). 

The raster source doesn't have to cover the tile, we're just saying that we
_should_ create a tile here. Whether CreateTile itself succeed or not is
different (that's the one that checks whether we can actually raster the tile).
I could move that check from CreateTile to ShouldCreateTile, but.. I'm a bit
more hesitant on that, since I have to verify that all calls to CreateTile are
protected.

Alternatively, I could move this ShouldCreate into Create and always call
Create, which would early out if !ShouldCreate...

[enne (OOO), 2015/04/10 21:16:28]

Maybe the answer here is just better comments about why the active tree should
always create a tile but the pending tree needs more checks?

[vmpstr, 2015/04/10 22:24:29]

Added a comment.

+  if (client_->GetTree() == ACTIVE_TREE)
+    return true;
+
+  // If the pending tree has no active twin, then it needs to create all tiles.
+  const PictureLayerTiling* active_twin =
+      client_->GetPendingOrActiveTwinTiling(this);
+  if (!active_twin)
+    return true;
+
+  // Pending tree will override the entire active tree if indices don't match.
+  if (!TilingMatchesTileIndices(active_twin))
+    return true;
+
+  gfx::Rect paint_rect = tiling_data_.TileBoundsWithBorder(i, j);
+  gfx::Rect tile_rect = paint_rect;
+  tile_rect.set_size(tiling_data_.max_texture_size());
+
+  // If the active tree can't create a tile, because of its raster source, then
+  // the pending tree should create one.
+  if (!active_twin->raster_source()->CoversRect(tile_rect, contents_scale()))
+    return true;
+
+  const Region* layer_invalidation = client_->GetPendingInvalidation();
+  gfx::Rect layer_rect =
+      gfx::ScaleToEnclosingRect(tile_rect, 1.f / contents_scale());
+
+  // If this tile is invalidated, then the pending tree should create one.
+  if (layer_invalidation && layer_invalidation->Intersects(layer_rect))
+    return true;
+
+  // If the active tree doesn't have a tile here, but it's in the pending tree's
+  // visible rect, then the pending tree should create a tile. This can happen
+  // if the pending visible rect is outside of the active tree's live tiles
+  // rect. In those situations, we need to block activation until we're ready to
+  // display content, which will have to come from the pending tree.
+  if (!active_twin->TileAt(i, j) && current_visible_rect_.Intersects(tile_rect))
+    return true;
+
+  // In all other cases, the pending tree doesn't need to create a tile.
+  return false;
+}
+
+bool PictureLayerTiling::TilingMatchesTileIndices(
+    const PictureLayerTiling* twin) const {
+  return tiling_data_.max_texture_size() ==
+         twin->tiling_data_.max_texture_size();
 }
 
 PictureLayerTiling::CoverageIterator::CoverageIterator()
@@ -499,29 +480,16 @@ gfx::RectF PictureLayerTiling::CoverageIterator::texture_rect() const {
   return texture_rect;
 }
 
-bool PictureLayerTiling::RemoveTileAt(int i,
-                                      int j,
-                                      PictureLayerTiling* recycled_twin) {
+bool PictureLayerTiling::RemoveTileAt(int i, int j) {
   TileMap::iterator found = tiles_.find(TileMapKey(i, j));
   if (found == tiles_.end())
     return false;
-  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, nullptr);
-  }
   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) {
-    it->second->set_shared(false);
-    if (recycled_twin)
-      recycled_twin->RemoveTileAt(it->first.first, it->first.second, nullptr);
-  }
   tiles_.clear();
 }
 
@@ -631,14 +599,14 @@ bool PictureLayerTiling::ComputeTilePriorityRects(
                            visible_rect_in_content_space);
   last_viewport_in_layer_space_ = viewport_in_layer_space;
 
+  SetTilePriorityRects(content_to_screen_scale, visible_rect_in_content_space,
+                       skewport, soon_border_rect, eventually_rect,
+                       occlusion_in_layer_space);
   SetLiveTilesRect(eventually_rect);
-  UpdateTilePriorityRects(
-      content_to_screen_scale, visible_rect_in_content_space, skewport,
-      soon_border_rect, eventually_rect, occlusion_in_layer_space);
   return true;
 }
 
-void PictureLayerTiling::UpdateTilePriorityRects(
+void PictureLayerTiling::SetTilePriorityRects(
     float content_to_screen_scale,
     const gfx::Rect& visible_rect_in_content_space,
     const gfx::Rect& skewport,
@@ -669,36 +637,24 @@ void PictureLayerTiling::SetLiveTilesRect(
   if (live_tiles_rect_ == new_live_tiles_rect)
     return;
 
-  PictureLayerTiling* recycled_twin = client_->GetRecycledTwinTiling(this);
-
   // Iterate to delete all tiles outside of our new live_tiles rect.
-  for (TilingData::DifferenceIterator iter(&tiling_data_,
-                                           live_tiles_rect_,
+  for (TilingData::DifferenceIterator iter(&tiling_data_, live_tiles_rect_,
                                            new_live_tiles_rect);
-       iter;
-       ++iter) {
-    RemoveTileAt(iter.index_x(), iter.index_y(), recycled_twin);
+       iter; ++iter) {
+    RemoveTileAt(iter.index_x(), iter.index_y());
   }
 
-  const PictureLayerTiling* twin_tiling =
-      client_->GetPendingOrActiveTwinTiling(this);
-
   // Iterate to allocate new tiles for all regions with newly exposed area.
-  for (TilingData::DifferenceIterator iter(&tiling_data_,
-                                           new_live_tiles_rect,
+  for (TilingData::DifferenceIterator iter(&tiling_data_, new_live_tiles_rect,
                                            live_tiles_rect_);
-       iter;
-       ++iter) {
+       iter; ++iter) {
     TileMapKey key(iter.index());
-    CreateTile(key.first, key.second, twin_tiling, recycled_twin);
+    if (ShouldCreateTileAt(key.first, key.second))
+      CreateTile(key.first, key.second);
   }
 
   live_tiles_rect_ = new_live_tiles_rect;
   VerifyLiveTilesRect(false);
-  if (recycled_twin) {
-    recycled_twin->live_tiles_rect_ = live_tiles_rect_;
-    recycled_twin->VerifyLiveTilesRect(true);
-  }
 }
 
 void PictureLayerTiling::VerifyLiveTilesRect(bool is_on_recycle_tree) const {
@@ -726,130 +682,115 @@ void PictureLayerTiling::VerifyLiveTilesRect(bool is_on_recycle_tree) const {
 }
 
 bool PictureLayerTiling::IsTileOccluded(const Tile* tile) const {
-  DCHECK(tile);
+  // If this tile is not occluded on this tree, then it is not occluded.
+  if (!IsTileOccludedOnCurrentTree(tile))
+    return false;
+
+  // Otherwise, if this is the pending tree, we're done and the tile is
+  // occluded.
+  if (client_->GetTree() == PENDING_TREE)
+    return true;
 
+  // On the active tree however, we need to check if this tile will be
+  // unoccluded upon activation, in which case it has to be considered
+  // unoccluded.
+  const PictureLayerTiling* pending_twin =
+      client_->GetPendingOrActiveTwinTiling(this);
+  if (pending_twin) {
+    // If there's a pending tile in the same position. Or if the pending twin
+    // would have to be creating all tiles, then we don't need to worry about
+    // occlusion on the twin.
+    if (!TilingMatchesTileIndices(pending_twin) ||
+        pending_twin->TileAt(tile->tiling_i_index(), tile->tiling_j_index())) {
+      return true;
+    }
+    return pending_twin->IsTileOccludedOnCurrentTree(tile);
+  }
+  return true;
+}
+
+bool PictureLayerTiling::IsTileOccludedOnCurrentTree(const Tile* tile) const {
   if (!current_occlusion_in_layer_space_.HasOcclusion())
     return false;
-
   gfx::Rect tile_query_rect =
       gfx::IntersectRects(tile->content_rect(), current_visible_rect_);
-
   // 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())
     return false;
 
   if (contents_scale_ != 1.f) {
     tile_query_rect =
-        gfx::ScaleToEnclosingRect(tile_query_rect, 1.0f / contents_scale_);
+        gfx::ScaleToEnclosingRect(tile_query_rect, 1.f / contents_scale_);
   }
-
   return current_occlusion_in_layer_space_.IsOccluded(tile_query_rect);
 }
 
-bool PictureLayerTiling::IsTileRequiredForActivationIfVisible(
-    const Tile* tile) const {
-  DCHECK_EQ(PENDING_TREE, client_->GetTree());
-
-  // This function assumes that the tile is visible (i.e. in the viewport).  The
-  // caller needs to make sure that this condition is met to ensure we don't
-  // block activation on tiles outside of the viewport.
-
-  // 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;
+bool PictureLayerTiling::IsTileRequiredForActivation(const Tile* tile) const {
+  if (client_->GetTree() == PENDING_TREE) {
+    // If we're not allowed to required tiles for activation, then the tile is

[enne (OOO), 2015/04/09 17:54:23]

*require

+    // not required.
+    if (!can_require_tiles_for_activation_)
+      return false;
+
+    // If this is not a high res tiling, then the tile is not required.
+    if (resolution_ != HIGH_RESOLUTION)
+      return false;
+
+    // If the tile is occluded, then it's not required.
+    if (IsTileOccluded(tile))
+      return false;
+
+    bool tile_in_visible_rect =
+        tile->content_rect().Intersects(current_visible_rect_);
+    if (tile_in_visible_rect && client_->RequiresHighResToDraw())
+      return true;
+
+    // Otherwise, the tile is required if it's in the pending tree's visible

[enne (OOO), 2015/04/09 17:54:23]

I think this comment (and a lot of other ones in this function and some other
longer functions with lots of conditionals like ShouldCreateTileAt) could maybe
benefit from fewer "describing what the code is doing" comments and more "why is
this doing this" comments.  I would honestly probably remove most of the
comments in this function.

[vmpstr, 2015/04/10 20:25:13]

Done.

+    // rect.
+    return tile_in_visible_rect;
+  }
 
-  const PictureLayerTiling* twin_tiling =
+  DCHECK(client_->GetTree() == ACTIVE_TREE);
+  const PictureLayerTiling* pending_twin =
       client_->GetPendingOrActiveTwinTiling(this);
-  if (!twin_tiling)
-    return true;
-
-  if (twin_tiling->raster_source()->GetSize() != raster_source()->GetSize())
-    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)
+  // If we don't have a pending tree, or the pending tree will overwrite the
+  // given tile, then it is not required for activation.
+  if (!pending_twin || !TilingMatchesTileIndices(pending_twin) ||
+      pending_twin->TileAt(tile->tiling_i_index(), tile->tiling_j_index())) {
     return false;
-
-  return true;
+  }
+  // Otherwise, ask the pending twin if this tile is required for activation.
+  return pending_twin->IsTileRequiredForActivation(tile);
 }
 
-bool PictureLayerTiling::IsTileRequiredForDrawIfVisible(
-    const Tile* tile) const {
-  DCHECK_EQ(ACTIVE_TREE, client_->GetTree());
-
-  // This function assumes that the tile is visible (i.e. in the viewport).
+bool PictureLayerTiling::IsTileRequiredForDraw(const Tile* tile) const {
+  if (client_->GetTree() == PENDING_TREE)
+    return false;
 
   if (resolution_ != HIGH_RESOLUTION)
     return false;
 
-  if (IsTileOccluded(tile))
+  bool tile_is_visible = current_visible_rect_.Intersects(tile->content_rect());
+  if (!tile_is_visible)
     return false;
 
+  if (IsTileOccludedOnCurrentTree(tile))
+    return false;
   return true;
 }
 
 void PictureLayerTiling::UpdateTileAndTwinPriority(Tile* tile) const {
+  // TODO(vmpstr): Remove references to tree/twin_tree in tile.
   WhichTree tree = client_->GetTree();
   WhichTree twin_tree = tree == ACTIVE_TREE ? PENDING_TREE : ACTIVE_TREE;
-
   tile->SetPriority(tree, ComputePriorityForTile(tile));
-  UpdateRequiredStateForTile(tile, tree);
-
-  const PictureLayerTiling* twin_tiling =
-      client_->GetPendingOrActiveTwinTiling(this);
-  if (!tile->is_shared() || !twin_tiling) {
-    tile->SetPriority(twin_tree, TilePriority());
-    tile->set_is_occluded(twin_tree, false);
-    if (twin_tree == PENDING_TREE)
-      tile->set_required_for_activation(false);
-    else
-      tile->set_required_for_draw(false);
-    return;
-  }
-
-  tile->SetPriority(twin_tree, twin_tiling->ComputePriorityForTile(tile));
-  twin_tiling->UpdateRequiredStateForTile(tile, twin_tree);
-}
-
-void PictureLayerTiling::UpdateRequiredStateForTile(Tile* tile,
-                                                    WhichTree tree) const {
-  if (tile->priority(tree).priority_bin == TilePriority::NOW) {
-    if (tree == PENDING_TREE) {
-      tile->set_required_for_activation(
-          IsTileRequiredForActivationIfVisible(tile));
-    } else {
-      tile->set_required_for_draw(IsTileRequiredForDrawIfVisible(tile));
-    }
-    tile->set_is_occluded(tree, IsTileOccluded(tile));
-    return;
-  }
-
-  // Non-NOW bin tiles are not required or occluded.
-  if (tree == PENDING_TREE)
-    tile->set_required_for_activation(false);
-  else
-    tile->set_required_for_draw(false);
-  tile->set_is_occluded(tree, false);
+  tile->SetPriority(twin_tree, TilePriority());
+  bool is_tile_occluded = IsTileOccluded(tile);
+  tile->set_is_occluded(tree, is_tile_occluded);
+  tile->set_is_occluded(twin_tree, is_tile_occluded);
+  tile->set_required_for_activation(IsTileRequiredForActivation(tile));
+  tile->set_required_for_draw(IsTileRequiredForDraw(tile));
 }
 
 void PictureLayerTiling::VerifyAllTilesHaveCurrentRasterSource() const {
@@ -874,6 +815,11 @@ TilePriority PictureLayerTiling::ComputePriorityForTile(
     return TilePriority(resolution_, TilePriority::NOW, 0);
   }
 
+  if (max_tile_priority_bin <= TilePriority::SOON &&
+      pending_visible_rect().Intersects(tile_bounds)) {
+    return TilePriority(resolution_, TilePriority::SOON, 0);
+  }
+
   DCHECK_GT(current_content_to_screen_scale_, 0.f);
   float distance_to_visible =
       current_visible_rect_.ManhattanInternalDistance(tile_bounds) *