cc: Mirror LiveTilesRect and tiles between active and recycled trees.

When the live tiles rect changes on the active tiling, we also remove
tiles on the recycled tiling to avoid unshared tiles in the recycle
tree. But when a tile is created on the active tree, we should also
share the tile to the recycle tree. Since we're creating tiles, we
also need to update the live tiles rect so tiles do not exist outside
of it. So now the live tiles rect is simply updated on both trees at
once.

This ensures that on the next commit, if the picture layer did not
get any invalidations, and doesn't push properties, it still has all
the tiles in its live tiles rect so we can do ready-to-activate checks
correctly.

Secondly, when activating, if any tiles were present on the pending
tree but are not on the active tree (can happen due to missing
recordings on the active tree), then share those tiles to the active
tree during activation.

This patch fixes DCHECKs occuring in PictureLayerTiling's
CloneTilesAndPropertiesFrom() where the pending and active tiling were
not ending up with the same number of tiles.

R=enne, vmpstr
BUG=387116

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

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

cc/layers/picture_layer_impl.cc

@@ -664,6 +664,8 @@ scoped_refptr<Tile> PictureLayerImpl::CreateTile(PictureLayerTiling* tiling,
 const Region* PictureLayerImpl::GetPendingInvalidation() {
   if (layer_tree_impl()->IsPendingTree())
     return &invalidation_;
+  if (layer_tree_impl()->IsRecycleTree())
+    return nullptr;
   DCHECK(layer_tree_impl()->IsActiveTree());
   if (PictureLayerImpl* twin_layer = GetPendingOrActiveTwinLayer())
     return &twin_layer->invalidation_;

cc/layers/picture_layer_impl_unittest.cc

@@ -4637,6 +4637,66 @@ TEST_F(PictureLayerImplTest, ChangeInViewportAllowsTilingUpdates) {
   EXPECT_TRUE(pending_layer_->AllTilesRequiredForActivationAreReadyToDraw());
 }
 
+TEST_F(PictureLayerImplTest, CloneMissingRecordings) {
+  gfx::Size tile_size(100, 100);
+  gfx::Size layer_bounds(400, 400);
+
+  scoped_refptr<FakePicturePileImpl> filled_pile =
+      FakePicturePileImpl::CreateFilledPile(tile_size, layer_bounds);
+  scoped_refptr<FakePicturePileImpl> partial_pile =
+      FakePicturePileImpl::CreateEmptyPile(tile_size, layer_bounds);
+  for (int i = 1; i < partial_pile->tiling().num_tiles_x(); ++i) {
+    for (int j = 1; j < partial_pile->tiling().num_tiles_y(); ++j)
+      partial_pile->AddRecordingAt(i, j);
+  }
+
+  SetupPendingTreeWithFixedTileSize(filled_pile, tile_size, Region());
+  ActivateTree();
+
+  PictureLayerTiling* pending_tiling = old_pending_layer_->HighResTiling();
+  PictureLayerTiling* active_tiling = active_layer_->HighResTiling();
+
+  // We should have all tiles in both tile sets.
+  EXPECT_EQ(5u * 5u, pending_tiling->AllTilesForTesting().size());
+  EXPECT_EQ(5u * 5u, active_tiling->AllTilesForTesting().size());
+
+  // Now put a partially-recorded pile on the pending tree (and invalidate
+  // everything, since the main thread PicturePile will invalidate dropped
+  // recordings). This will cause us to be missing some tiles.
+  SetupPendingTreeWithFixedTileSize(partial_pile, tile_size,
+                                    Region(gfx::Rect(layer_bounds)));
+  EXPECT_EQ(3u * 3u, pending_tiling->AllTilesForTesting().size());
+  EXPECT_FALSE(pending_tiling->TileAt(0, 0));
+  EXPECT_FALSE(pending_tiling->TileAt(1, 1));
+  EXPECT_TRUE(pending_tiling->TileAt(2, 2));
+
+  // Active is not affected yet.
+  EXPECT_EQ(5u * 5u, active_tiling->AllTilesForTesting().size());
+
+  // Activate the tree. The same tiles go missing on the active tree.
+  ActivateTree();
+  EXPECT_EQ(3u * 3u, active_tiling->AllTilesForTesting().size());
+  EXPECT_FALSE(active_tiling->TileAt(0, 0));
+  EXPECT_FALSE(active_tiling->TileAt(1, 1));
+  EXPECT_TRUE(active_tiling->TileAt(2, 2));
+
+  // Now put a full recording on the pending tree again. We'll get all our tiles
+  // back.
+  SetupPendingTreeWithFixedTileSize(filled_pile, tile_size,
+                                    Region(gfx::Rect(layer_bounds)));
+  EXPECT_EQ(5u * 5u, pending_tiling->AllTilesForTesting().size());
+
+  // Active is not affected yet.
+  EXPECT_EQ(3u * 3u, active_tiling->AllTilesForTesting().size());
+
+  // Activate the tree. The tiles are created and shared on the active tree.
+  ActivateTree();
+  EXPECT_EQ(5u * 5u, active_tiling->AllTilesForTesting().size());
+  EXPECT_TRUE(active_tiling->TileAt(0, 0)->is_shared());
+  EXPECT_TRUE(active_tiling->TileAt(1, 1)->is_shared());
+  EXPECT_TRUE(active_tiling->TileAt(2, 2)->is_shared());
+}
+
 class TileSizeSettings : public ImplSidePaintingSettings {
  public:
   TileSizeSettings() {

cc/resources/picture_layer_tiling.cc

@@ -88,7 +88,11 @@ PictureLayerTiling::~PictureLayerTiling() {
 
 Tile* PictureLayerTiling::CreateTile(int i,
                                      int j,
-                                     const PictureLayerTiling* twin_tiling) {
+                                     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);
   TileMapKey key(i, j);
   DCHECK(tiles_.find(key) == tiles_.end());
 
@@ -121,6 +125,13 @@ Tile* PictureLayerTiling::CreateTile(int i,
     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();
 }
@@ -128,6 +139,10 @@ Tile* PictureLayerTiling::CreateTile(int i,
 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);
@@ -137,10 +152,10 @@ void PictureLayerTiling::CreateMissingTilesInLiveTilesRect() {
     TileMap::iterator find = tiles_.find(key);
     if (find != tiles_.end())
       continue;
-    CreateTile(key.first, key.second, twin_tiling);
+    CreateTile(key.first, key.second, twin_tiling, null_recycled_twin);
   }
 
-  VerifyLiveTilesRect();
+  VerifyLiveTilesRect(false);
 }
 
 void PictureLayerTiling::CloneTilesAndPropertiesFrom(
@@ -170,13 +185,22 @@ void PictureLayerTiling::CloneTilesAndPropertiesFrom(
   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);
+    CreateTile(key.first, key.second, &twin_tiling, null_recycled_twin);
+  }
+
+  // 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);
   }
 
   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_,
@@ -245,17 +269,17 @@ void PictureLayerTiling::Resize(const gfx::Size& new_layer_bounds) {
 
   // There is no recycled twin since this is run on the pending tiling
   // during commit, and on the active tree during activate.
-  PictureLayerTiling* recycled_twin = NULL;
-  DCHECK_EQ(recycled_twin, client_->GetRecycledTwinTiling(this));
+  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, recycled_twin);
+      RemoveTileAt(i, j, null_recycled_twin);
   }
   for (int i = before_left; i <= after_right; ++i) {
     for (int j = after_bottom + 1; j <= before_bottom; ++j)
-      RemoveTileAt(i, j, recycled_twin);
+      RemoveTileAt(i, j, null_recycled_twin);
   }
 
   // If the layer grew, the live_tiles_rect_ is not changed, but a new row
@@ -265,12 +289,12 @@ void PictureLayerTiling::Resize(const gfx::Size& new_layer_bounds) {
   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);
+      CreateTile(after_right, j, twin_tiling, null_recycled_twin);
   }
   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);
+      CreateTile(i, after_bottom, twin_tiling, null_recycled_twin);
   }
 }
 
@@ -304,7 +328,7 @@ void PictureLayerTiling::Invalidate(const Region& layer_invalidation) {
          ++iter) {
       // There is no recycled twin for the pending tree during commit, or for
       // the active tree during activation.
-      PictureLayerTiling* null_recycled_twin = NULL;
+      PictureLayerTiling* null_recycled_twin = nullptr;
       DCHECK_EQ(null_recycled_twin, client_->GetRecycledTwinTiling(this));
       if (RemoveTileAt(iter.index_x(), iter.index_y(), null_recycled_twin))
         new_tile_keys.push_back(iter.index());
@@ -314,9 +338,14 @@ void PictureLayerTiling::Invalidate(const Region& layer_invalidation) {
   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.
-    const PictureLayerTiling* twin_tiling = nullptr;
+    // 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, twin_tiling);
+      CreateTile(new_tile_keys[i].first, new_tile_keys[i].second,
+                 null_twin_tiling, null_recycled_twin);
     }
   }
 }
@@ -328,7 +357,7 @@ void PictureLayerTiling::SetRasterSource(
   // 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();
+  VerifyLiveTilesRect(false);
 }
 
 PictureLayerTiling::CoverageIterator::CoverageIterator()
@@ -490,8 +519,8 @@ bool PictureLayerTiling::RemoveTileAt(int i,
   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);
+    // Recycled twin does not also have a recycled twin, so pass null.
+    recycled_twin->RemoveTileAt(i, j, nullptr);
   }
   return true;
 }
@@ -502,7 +531,7 @@ void PictureLayerTiling::Reset() {
   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, NULL);
+      recycled_twin->RemoveTileAt(it->first.first, it->first.second, nullptr);
   }
   tiles_.clear();
 }
@@ -642,8 +671,9 @@ void PictureLayerTiling::SetLiveTilesRect(
   if (live_tiles_rect_ == new_live_tiles_rect)
     return;
 
-  // Iterate to delete all tiles outside of our new live_tiles rect.
   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_,
                                            new_live_tiles_rect);
@@ -662,16 +692,20 @@ void PictureLayerTiling::SetLiveTilesRect(
        iter;
        ++iter) {
     TileMapKey key(iter.index());
-    CreateTile(key.first, key.second, twin_tiling);
+    CreateTile(key.first, key.second, twin_tiling, recycled_twin);
   }
 
   live_tiles_rect_ = new_live_tiles_rect;
-  VerifyLiveTilesRect();
+  VerifyLiveTilesRect(false);
+  if (recycled_twin) {
+    recycled_twin->live_tiles_rect_ = live_tiles_rect_;
+    recycled_twin->VerifyLiveTilesRect(true);
+  }
 }
 
-void PictureLayerTiling::VerifyLiveTilesRect() {
+void PictureLayerTiling::VerifyLiveTilesRect(bool is_on_recycle_tree) const {
 #if DCHECK_IS_ON
-  for (TileMap::iterator it = tiles_.begin(); it != tiles_.end(); ++it) {
+  for (auto it = tiles_.begin(); it != tiles_.end(); ++it) {
     if (!it->second.get())
       continue;
     DCHECK(it->first.first < tiling_data_.num_tiles_x())
@@ -688,6 +722,7 @@ void PictureLayerTiling::VerifyLiveTilesRect() {
         << " tile bounds "
         << tiling_data_.TileBounds(it->first.first, it->first.second).ToString()
         << " live_tiles_rect " << live_tiles_rect_.ToString();
+    DCHECK_IMPLIES(is_on_recycle_tree, it->second->is_shared());
   }
 #endif
 }

cc/resources/picture_layer_tiling.h

@@ -287,8 +287,11 @@ class CC_EXPORT PictureLayerTiling {
                      float skewport_target_time_in_seconds,
                      int skewport_extrapolation_limit_in_content_pixels);
   void SetLiveTilesRect(const gfx::Rect& live_tiles_rect);
-  void VerifyLiveTilesRect();
-  Tile* CreateTile(int i, int j, const PictureLayerTiling* twin_tiling);
+  void VerifyLiveTilesRect(bool is_on_recycle_tree) const;
+  Tile* CreateTile(int i,
+                   int j,
+                   const PictureLayerTiling* twin_tiling,
+                   PictureLayerTiling* recycled_twin);
   // Returns true if the Tile existed and was removed from the tiling.
   bool RemoveTileAt(int i, int j, PictureLayerTiling* recycled_twin);
 

cc/resources/picture_layer_tiling_set.cc

@@ -90,7 +90,9 @@ void PictureLayerTilingSet::UpdateTilingsToCurrentRasterSource(
     tiling->Resize(layer_bounds);
     tiling->Invalidate(layer_invalidation);
     tiling->SetRasterSource(raster_source);
-    // TODO(danakj): Is this needed anymore? Won't they already exist?
+    // This is needed for cases where the live tiles rect didn't change but
+    // recordings exist in the raster source that did not exist on the last
+    // raster source.
     tiling->CreateMissingTilesInLiveTilesRect();
 
     // If |twin_set| is present, use the resolutions from there. Otherwise leave

cc/resources/picture_layer_tiling_unittest.cc

@@ -2013,9 +2013,9 @@ TEST(PictureLayerTilingTest, RecycledTilesCleared) {
   active_tiling->ComputeTilePriorityRects(gfx::Rect(0, 0, 100, 100), 1.0f, 3.0,
                                           Occlusion());
 
-  // Ensure that we now have a tile here, but the recycle tiling does not.
+  // Ensure that we now have a tile here on both tilings again.
   EXPECT_TRUE(active_tiling->TileAt(0, 0));
-  EXPECT_FALSE(recycle_tiling->TileAt(0, 0));
+  EXPECT_TRUE(recycle_tiling->TileAt(0, 0));
 }
 
 TEST(PictureLayerTilingTest, RecycledTilesClearedOnReset) {

cc/trees/layer_tree_host_unittest_picture.cc

@@ -191,5 +191,131 @@ class LayerTreeHostPictureTestResizeViewportWithGpuRaster
 SINGLE_AND_MULTI_THREAD_IMPL_TEST_F(
     LayerTreeHostPictureTestResizeViewportWithGpuRaster);
 
+class LayerTreeHostPictureTestChangeLiveTilesRectWithRecycleTree
+    : public LayerTreeHostPictureTest {
+  void SetupTree() override {
+    frame_ = 0;
+    did_post_commit_ = false;
+
+    scoped_refptr<Layer> root = Layer::Create();
+    root->SetBounds(gfx::Size(100, 100));
+
+    // The layer is big enough that the live tiles rect won't cover the full
+    // layer.
+    client_.set_fill_with_nonsolid_color(true);
+    picture_ = FakePictureLayer::Create(&client_);
+    picture_->SetBounds(gfx::Size(100, 100000));
+    root->AddChild(picture_);
+
+    layer_tree_host()->SetRootLayer(root);
+    LayerTreeHostPictureTest::SetupTree();
+  }
+
+  void BeginTest() override { PostSetNeedsCommitToMainThread(); }
+
+  void DrawLayersOnThread(LayerTreeHostImpl* impl) override {
+    LayerImpl* child = impl->active_tree()->root_layer()->children()[0];
+    FakePictureLayerImpl* picture_impl =
+        static_cast<FakePictureLayerImpl*>(child);
+    FakePictureLayerImpl* recycled_impl = static_cast<FakePictureLayerImpl*>(
+        picture_impl->GetRecycledTwinLayer());
+
+    switch (++frame_) {
+      case 1: {
+        PictureLayerTiling* tiling = picture_impl->HighResTiling();
+        PictureLayerTiling* recycled_tiling = recycled_impl->HighResTiling();
+        int num_tiles_y = tiling->TilingDataForTesting().num_tiles_y();
+
+        // There should be tiles at the top of the picture layer but not at the
+        // bottom.
+        EXPECT_TRUE(tiling->TileAt(0, 0));
+        EXPECT_FALSE(tiling->TileAt(0, num_tiles_y));
+
+        // The recycled tiling matches it.
+        EXPECT_TRUE(recycled_tiling->TileAt(0, 0));
+        EXPECT_FALSE(recycled_tiling->TileAt(0, num_tiles_y));
+
+        // The live tiles rect matches on the recycled tree.
+        EXPECT_EQ(tiling->live_tiles_rect(),
+                  recycled_tiling->live_tiles_rect());
+
+        // Make the bottom of the layer visible.
+        picture_impl->SetPosition(gfx::PointF(0.f, -100000.f + 100.f));
+        impl->SetNeedsRedraw();
+        break;
+      }
+      case 2: {
+        PictureLayerTiling* tiling = picture_impl->HighResTiling();
+        PictureLayerTiling* recycled_tiling = recycled_impl->HighResTiling();
+
+        // There not be tiles at the top of the layer now.
+        EXPECT_FALSE(tiling->TileAt(0, 0));
+
+        // The recycled twin tiling should not have unshared tiles at the top
+        // either.
+        EXPECT_FALSE(recycled_tiling->TileAt(0, 0));
+
+        // The live tiles rect matches on the recycled tree.
+        EXPECT_EQ(tiling->live_tiles_rect(),
+                  recycled_tiling->live_tiles_rect());
+
+        // Make the top of the layer visible again.
+        picture_impl->SetPosition(gfx::PointF());
+        impl->SetNeedsRedraw();
+        break;
+      }
+      case 3: {
+        PictureLayerTiling* tiling = picture_impl->HighResTiling();
+        PictureLayerTiling* recycled_tiling = recycled_impl->HighResTiling();
+        int num_tiles_y = tiling->TilingDataForTesting().num_tiles_y();
+
+        // There should be tiles at the top of the picture layer again.
+        EXPECT_TRUE(tiling->TileAt(0, 0));
+        EXPECT_FALSE(tiling->TileAt(0, num_tiles_y));
+
+        // The recycled tiling should also have tiles at the top.
+        EXPECT_TRUE(recycled_tiling->TileAt(0, 0));
+        EXPECT_FALSE(recycled_tiling->TileAt(0, num_tiles_y));
+
+        // The live tiles rect matches on the recycled tree.
+        EXPECT_EQ(tiling->live_tiles_rect(),
+                  recycled_tiling->live_tiles_rect());
+
+        // Make a new main frame without changing the picture layer at all, so
+        // it won't need to update or push properties.
+        did_post_commit_ = true;
+        PostSetNeedsCommitToMainThread();
+        break;
+      }
+    }
+  }
+
+  void WillActivateTreeOnThread(LayerTreeHostImpl* impl) override {
+    LayerImpl* child = impl->sync_tree()->root_layer()->children()[0];
+    FakePictureLayerImpl* picture_impl =
+        static_cast<FakePictureLayerImpl*>(child);
+    PictureLayerTiling* tiling = picture_impl->HighResTiling();
+    int num_tiles_y = tiling->TilingDataForTesting().num_tiles_y();
+
+    // The pending layer should always have tiles at the top of it each commit.
+    // The tile is part of the required for activation set so it should exist.
+    EXPECT_TRUE(tiling->TileAt(0, 0));
+    EXPECT_FALSE(tiling->TileAt(0, num_tiles_y));
+
+    if (did_post_commit_)
+      EndTest();
+  }
+
+  void AfterTest() override {}
+
+  int frame_;
+  bool did_post_commit_;
+  FakeContentLayerClient client_;
+  scoped_refptr<FakePictureLayer> picture_;
+};
+
+SINGLE_AND_MULTI_THREAD_IMPL_TEST_F(
+    LayerTreeHostPictureTestChangeLiveTilesRectWithRecycleTree);
+
 }  // namespace
 }  // namespace cc