cc: Don't require push properties for picture layers

PictureLayer now only needs push properties if its pile has changed.  In
order to make this possible, some of the non-pushing post-commit work
had to be deferred so that every layer that got committed (even if it
didn't get pushed) would go through it.  This is lazily done with a
boolean flag in PictureLayerImpl, with some DCHECKs for sanity about its
state.

As every layer on the pending tree that will get drawn will be
CalcDrawEtc'd and get UpdateTilePriorities first, this seems a safe
place to put this lazy check.

R=danakj@chromium.org
BUG=259402

Review URL: https://chromiumcodereview.appspot.com/19540014

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@214533 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 59 insertions, 79 deletions

diff --git a/cc/layers/picture_layer.cc b/cc/layers/picture_layer.cc
index 5438a8c..2e7a90b 100644
--- a/cc/layers/picture_layer.cc
+++ b/cc/layers/picture_layer.cc
@@ -36,25 +36,14 @@ scoped_ptr<LayerImpl> PictureLayer::CreateLayerImpl(LayerTreeImpl* tree_impl) {
 
 void PictureLayer::PushPropertiesTo(LayerImpl* base_layer) {
   Layer::PushPropertiesTo(base_layer);
-
   PictureLayerImpl* layer_impl = static_cast<PictureLayerImpl*>(base_layer);
-  // This should be first so others can use it.
-  layer_impl->UpdateTwinLayer();
 
   layer_impl->SetIsMask(is_mask_);
-  layer_impl->CreateTilingSetIfNeeded();
   // Unlike other properties, invalidation must always be set on layer_impl.
   // See PictureLayerImpl::PushPropertiesTo for more details.
   layer_impl->invalidation_.Clear();
   layer_impl->invalidation_.Swap(&pile_invalidation_);
   layer_impl->pile_ = PicturePileImpl::CreateFromOther(pile_.get());
-  layer_impl->SyncFromActiveLayer();
-
-  // PictureLayer must push properties every frame.
-  // TODO(danakj): If we can avoid requiring to do CreateTilingSetIfNeeded() and
-  // SyncFromActiveLayer() on every commit then this could go away, maybe
-  // conditionally. crbug.com/259402
-  needs_push_properties_ = true;
 }
 
 void PictureLayer::SetLayerTreeHost(LayerTreeHost* host) {
@@ -112,11 +101,14 @@ bool PictureLayer::Update(ResourceUpdateQueue*,
                                pile_invalidation_,
                                visible_layer_rect,
                                rendering_stats_instrumentation());
-  if (!updated) {
+  if (updated) {
+    SetNeedsPushProperties();
+  } else {
     // If this invalidation did not affect the pile, then it can be cleared as
     // an optimization.
     pile_invalidation_.Clear();
   }
+
   return updated;
 }
 
diff --git a/cc/layers/picture_layer_impl.cc b/cc/layers/picture_layer_impl.cc
index da9cf91..6b4bc58 100644
--- a/cc/layers/picture_layer_impl.cc
+++ b/cc/layers/picture_layer_impl.cc
@@ -45,8 +45,8 @@ PictureLayerImpl::PictureLayerImpl(LayerTreeImpl* tree_impl, int id)
       raster_contents_scale_(0.f),
       low_res_raster_contents_scale_(0.f),
       raster_source_scale_was_animating_(false),
-      is_using_lcd_text_(tree_impl->settings().can_use_lcd_text) {
-}
+      is_using_lcd_text_(tree_impl->settings().can_use_lcd_text),
+      needs_post_commit_initialization_(true) {}
 
 PictureLayerImpl::~PictureLayerImpl() {
 }
@@ -60,13 +60,11 @@ scoped_ptr<LayerImpl> PictureLayerImpl::CreateLayerImpl(
   return PictureLayerImpl::Create(tree_impl, id()).PassAs<LayerImpl>();
 }
 
-void PictureLayerImpl::CreateTilingSetIfNeeded() {
-  DCHECK(layer_tree_impl()->IsPendingTree());
-  if (!tilings_)
-    tilings_.reset(new PictureLayerTilingSet(this, bounds()));
-}
-
 void PictureLayerImpl::PushPropertiesTo(LayerImpl* base_layer) {
+  // It's possible this layer was never drawn or updated (e.g. because it was
+  // a descendant of an opacity 0 layer).
+  DoPostCommitInitializationIfNeeded();
+
   LayerImpl::PushPropertiesTo(base_layer);
 
   PictureLayerImpl* layer_impl = static_cast<PictureLayerImpl*>(base_layer);
@@ -78,8 +76,9 @@ void PictureLayerImpl::PushPropertiesTo(LayerImpl* base_layer) {
 
   layer_impl->SetIsMask(is_mask_);
   layer_impl->pile_ = pile_;
-  pile_ = NULL;
 
+  // Tilings would be expensive to push, so we swap.  This optimization requires
+  // an extra invalidation in SyncFromActiveLayer.
   layer_impl->tilings_.swap(tilings_);
   layer_impl->tilings_->SetClient(layer_impl);
   if (tilings_)
@@ -92,19 +91,19 @@ void PictureLayerImpl::PushPropertiesTo(LayerImpl* base_layer) {
   layer_impl->low_res_raster_contents_scale_ = low_res_raster_contents_scale_;
 
   layer_impl->UpdateLCDTextStatus(is_using_lcd_text_);
+  layer_impl->needs_post_commit_initialization_ = false;
 
-  // As an optimization, don't make a copy of this potentially complex region,
-  // and swap it directly from the pending to the active layer.  In general, any
-  // property pushed to a LayerImpl continues to live on that LayerImpl.
-  // However, invalidation is the difference between two main thread frames, so
-  // it no longer makes sense once the pending tree gets recycled.  It will
-  // always get pushed during PictureLayer::PushPropertiesTo.
+  // The invalidation on this soon-to-be-recycled layer must be cleared to
+  // mirror clearing the invalidation in PictureLayer's version of this function
+  // in case push properties is skipped.
   layer_impl->invalidation_.Swap(&invalidation_);
   invalidation_.Clear();
+  needs_post_commit_initialization_ = true;
 }
 
 void PictureLayerImpl::AppendQuads(QuadSink* quad_sink,
                                    AppendQuadsData* append_quads_data) {
+  DCHECK(!needs_post_commit_initialization_);
   gfx::Rect rect(visible_content_rect());
   gfx::Rect content_rect(content_bounds());
 
@@ -296,6 +295,7 @@ void PictureLayerImpl::AppendQuads(QuadSink* quad_sink,
 }
 
 void PictureLayerImpl::UpdateTilePriorities() {
+  DCHECK(!needs_post_commit_initialization_);
   if (!tilings_->num_tilings())
     return;
 
@@ -380,6 +380,7 @@ void PictureLayerImpl::CalculateContentsScale(
     float* contents_scale_x,
     float* contents_scale_y,
     gfx::Size* content_bounds) {
+  DoPostCommitInitializationIfNeeded();
   if (!CanHaveTilings()) {
     ideal_page_scale_ = page_scale_factor;
     ideal_device_scale_ = device_scale_factor;
@@ -525,13 +526,6 @@ gfx::Size PictureLayerImpl::CalculateTileSize(
   return default_tile_size;
 }
 
-void PictureLayerImpl::SyncFromActiveLayer() {
-  DCHECK(layer_tree_impl()->IsPendingTree());
-
-  if (twin_layer_)
-    SyncFromActiveLayer(twin_layer_);
-}
-
 void PictureLayerImpl::SyncFromActiveLayer(const PictureLayerImpl* other) {
   UpdateLCDTextStatus(other->is_using_lcd_text_);
 
@@ -597,15 +591,6 @@ void PictureLayerImpl::SyncTiling(
     UpdateTilePriorities();
 }
 
-void PictureLayerImpl::UpdateTwinLayer() {
-  DCHECK(layer_tree_impl()->IsPendingTree());
-
-  twin_layer_ = static_cast<PictureLayerImpl*>(
-      layer_tree_impl()->FindActiveTreeLayerById(id()));
-  if (twin_layer_)
-    twin_layer_->twin_layer_ = this;
-}
-
 void PictureLayerImpl::SetIsMask(bool is_mask) {
   if (is_mask_ == is_mask)
     return;
@@ -724,6 +709,25 @@ void PictureLayerImpl::MarkVisibleResourcesAsRequired() const {
   }
 }
 
+void PictureLayerImpl::DoPostCommitInitialization() {
+  DCHECK(needs_post_commit_initialization_);
+  DCHECK(layer_tree_impl()->IsPendingTree());
+
+  if (!tilings_)
+    tilings_.reset(new PictureLayerTilingSet(this, bounds()));
+
+  DCHECK(!twin_layer_);
+  twin_layer_ = static_cast<PictureLayerImpl*>(
+      layer_tree_impl()->FindActiveTreeLayerById(id()));
+  if (twin_layer_) {
+    DCHECK(!twin_layer_->twin_layer_);
+    twin_layer_->twin_layer_ = this;
+    SyncFromActiveLayer(twin_layer_);
+  }
+
+  needs_post_commit_initialization_ = false;
+}
+
 PictureLayerTiling* PictureLayerImpl::AddTiling(float contents_scale) {
   DCHECK(CanHaveTilingWithScale(contents_scale)) <<
       "contents_scale: " << contents_scale;
@@ -775,6 +779,7 @@ void PictureLayerImpl::ManageTilings(bool animating_transform_to_screen) {
   DCHECK(ideal_device_scale_);
   DCHECK(ideal_source_scale_);
   DCHECK(CanHaveTilings());
+  DCHECK(!needs_post_commit_initialization_);
 
   bool change_target_tiling =
       raster_page_scale_ == 0.f ||
diff --git a/cc/layers/picture_layer_impl.h b/cc/layers/picture_layer_impl.h
index 25cc5d0..42efa72 100644
--- a/cc/layers/picture_layer_impl.h
+++ b/cc/layers/picture_layer_impl.h
@@ -62,11 +62,7 @@ class CC_EXPORT PictureLayerImpl
       const PictureLayerTiling* tiling) OVERRIDE;
 
   // PushPropertiesTo active tree => pending tree.
-  void SyncFromActiveLayer();
   void SyncTiling(const PictureLayerTiling* tiling);
-  void UpdateTwinLayer();
-
-  void CreateTilingSetIfNeeded();
 
   // Mask-related functions
   void SetIsMask(bool is_mask);
@@ -92,6 +88,11 @@ class CC_EXPORT PictureLayerImpl
   void UpdateLCDTextStatus(bool new_status);
   void ResetRasterScale();
   void MarkVisibleResourcesAsRequired() const;
+  void DoPostCommitInitializationIfNeeded() {
+    if (needs_post_commit_initialization_)
+      DoPostCommitInitialization();
+  }
+  void DoPostCommitInitialization();
 
   bool CanHaveTilings() const;
   bool CanHaveTilingWithScale(float contents_scale) const;
@@ -124,6 +125,7 @@ class CC_EXPORT PictureLayerImpl
 
   bool raster_source_scale_was_animating_;
   bool is_using_lcd_text_;
+  bool needs_post_commit_initialization_;
 
   friend class PictureLayer;
   DISALLOW_COPY_AND_ASSIGN(PictureLayerImpl);
diff --git a/cc/layers/picture_layer_impl_unittest.cc b/cc/layers/picture_layer_impl_unittest.cc
index 855fa56..88d179f 100644
--- a/cc/layers/picture_layer_impl_unittest.cc
+++ b/cc/layers/picture_layer_impl_unittest.cc
@@ -69,7 +69,6 @@ class PictureLayerImplTest : public testing::Test {
         host_impl_.active_tree()->LayerById(id_));
 
     SetupPendingTree(pending_pile);
-    pending_layer_->UpdateTwinLayer();
   }
 
   void AddDefaultTilingsWithInvalidation(const Region& invalidation) {
@@ -79,7 +78,6 @@ class PictureLayerImplTest : public testing::Test {
     for (size_t i = 0; i < active_layer_->tilings()->num_tilings(); ++i)
       active_layer_->tilings()->tiling_at(i)->CreateAllTilesForTesting();
     pending_layer_->set_invalidation(invalidation);
-    pending_layer_->SyncFromActiveLayer();
     for (size_t i = 0; i < pending_layer_->tilings()->num_tilings(); ++i)
       pending_layer_->tilings()->tiling_at(i)->CreateAllTilesForTesting();
   }
@@ -98,6 +96,7 @@ class PictureLayerImplTest : public testing::Test {
 
     pending_layer_ = static_cast<FakePictureLayerImpl*>(
         host_impl_.pending_tree()->LayerById(id_));
+    pending_layer_->DoPostCommitInitializationIfNeeded();
   }
 
   static void VerifyAllTilesExistAndHavePile(
diff --git a/cc/test/fake_picture_layer_impl.cc b/cc/test/fake_picture_layer_impl.cc
index dd383a6..bdf40a5 100644
--- a/cc/test/fake_picture_layer_impl.cc
+++ b/cc/test/fake_picture_layer_impl.cc
@@ -14,7 +14,6 @@ FakePictureLayerImpl::FakePictureLayerImpl(
       append_quads_count_(0) {
   pile_ = pile;
   SetBounds(pile_->size());
-  CreateTilingSetIfNeeded();
 }
 
 FakePictureLayerImpl::FakePictureLayerImpl(LayerTreeImpl* tree_impl, int id)
diff --git a/cc/test/fake_picture_layer_impl.h b/cc/test/fake_picture_layer_impl.h
index a0c7829..248437b 100644
--- a/cc/test/fake_picture_layer_impl.h
+++ b/cc/test/fake_picture_layer_impl.h
@@ -32,6 +32,7 @@ class FakePictureLayerImpl : public PictureLayerImpl {
   using PictureLayerImpl::CleanUpTilingsOnActiveLayer;
   using PictureLayerImpl::CanHaveTilings;
   using PictureLayerImpl::MarkVisibleResourcesAsRequired;
+  using PictureLayerImpl::DoPostCommitInitializationIfNeeded;
 
   PictureLayerTiling* HighResTiling() const;
   PictureLayerTiling* LowResTiling() const;
diff --git a/cc/trees/layer_tree_host_unittest.cc b/cc/trees/layer_tree_host_unittest.cc
index e9435f7..df4643a 100644
--- a/cc/trees/layer_tree_host_unittest.cc
+++ b/cc/trees/layer_tree_host_unittest.cc
@@ -3245,19 +3245,13 @@ class LayerTreeHostTestLayersPushProperties : public LayerTreeHostTest {
     child_ = PushPropertiesCountingLayer::Create();
     child2_ = PushPropertiesCountingLayer::Create();
     grandchild_ = PushPropertiesCountingLayer::Create();
-
-    if (layer_tree_host()->settings().impl_side_painting)
-      leaf_picture_layer_ = FakePictureLayer::Create(&client_);
-    else
-      leaf_content_layer_ = FakeContentLayer::Create(&client_);
+    leaf_scrollbar_layer_ =
+        FakeScrollbarLayer::Create(false, false, root_->id());
 
     root_->AddChild(child_);
     root_->AddChild(child2_);
     child_->AddChild(grandchild_);
-    if (leaf_picture_layer_)
-      child2_->AddChild(leaf_picture_layer_);
-    if (leaf_content_layer_)
-      child2_->AddChild(leaf_content_layer_);
+    child2_->AddChild(leaf_scrollbar_layer_);
 
     other_root_ = PushPropertiesCountingLayer::Create();
 
@@ -3276,16 +3270,10 @@ class LayerTreeHostTestLayersPushProperties : public LayerTreeHostTest {
               child2_->push_properties_count());
     EXPECT_EQ(expected_push_properties_other_root_,
               other_root_->push_properties_count());
-    if (leaf_content_layer_) {
-      EXPECT_EQ(expected_push_properties_leaf_layer_,
-                leaf_content_layer_->push_properties_count());
-    }
-    if (leaf_picture_layer_) {
-      EXPECT_EQ(expected_push_properties_leaf_layer_,
-                leaf_picture_layer_->push_properties_count());
-    }
+    EXPECT_EQ(expected_push_properties_leaf_layer_,
+              leaf_scrollbar_layer_->push_properties_count());
 
-    // The content/picture layer always needs to be pushed.
+    // The scrollbar layer always needs to be pushed.
     if (root_->layer_tree_host()) {
       EXPECT_TRUE(root_->descendant_needs_push_properties());
       EXPECT_FALSE(root_->needs_push_properties());
@@ -3294,13 +3282,9 @@ class LayerTreeHostTestLayersPushProperties : public LayerTreeHostTest {
       EXPECT_TRUE(child2_->descendant_needs_push_properties());
       EXPECT_FALSE(child2_->needs_push_properties());
     }
-    if (leaf_content_layer_.get() && leaf_content_layer_->layer_tree_host()) {
-      EXPECT_FALSE(leaf_content_layer_->descendant_needs_push_properties());
-      EXPECT_TRUE(leaf_content_layer_->needs_push_properties());
-    }
-    if (leaf_picture_layer_.get() && leaf_picture_layer_->layer_tree_host()) {
-      EXPECT_FALSE(leaf_picture_layer_->descendant_needs_push_properties());
-      EXPECT_TRUE(leaf_picture_layer_->needs_push_properties());
+    if (leaf_scrollbar_layer_->layer_tree_host()) {
+      EXPECT_FALSE(leaf_scrollbar_layer_->descendant_needs_push_properties());
+      EXPECT_TRUE(leaf_scrollbar_layer_->needs_push_properties());
     }
 
     // child_ and grandchild_ don't persist their need to push properties.
@@ -3411,8 +3395,7 @@ class LayerTreeHostTestLayersPushProperties : public LayerTreeHostTest {
 
     // Content/Picture layers require PushProperties every commit that they are
     // in the tree.
-    if ((leaf_content_layer_.get() && leaf_content_layer_->layer_tree_host()) ||
-        (leaf_picture_layer_.get() && leaf_picture_layer_->layer_tree_host()))
+    if (leaf_scrollbar_layer_->layer_tree_host())
       ++expected_push_properties_leaf_layer_;
   }
 
@@ -3425,8 +3408,7 @@ class LayerTreeHostTestLayersPushProperties : public LayerTreeHostTest {
   scoped_refptr<PushPropertiesCountingLayer> child2_;
   scoped_refptr<PushPropertiesCountingLayer> grandchild_;
   scoped_refptr<PushPropertiesCountingLayer> other_root_;
-  scoped_refptr<FakeContentLayer> leaf_content_layer_;
-  scoped_refptr<FakePictureLayer> leaf_picture_layer_;
+  scoped_refptr<FakeScrollbarLayer> leaf_scrollbar_layer_;
   size_t expected_push_properties_root_;
   size_t expected_push_properties_child_;
   size_t expected_push_properties_child2_;