CC Animation: Introduce some dirty flags to optimize PushProperties on commit

The idea is that we invalidate needs_push_properties flag
from bottom-to-top in the hierarchy of ownership.

It helps us to isolate untouched groups of objects.

BUG=604280
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Committed: https://crrev.com/1ec9dd4962683e1fe12734a6756e825df2db10ca
Review-Url: https://codereview.chromium.org/2261113002
Cr-Original-Commit-Position: refs/heads/master@{#414981}
Cr-Commit-Position: refs/heads/master@{#417132}

cc/animation/animation_host.cc

@@ -43,7 +43,7 @@ AnimationHost::AnimationHost(ThreadInstance thread_instance)
     : mutator_host_client_(nullptr),
       thread_instance_(thread_instance),
       supports_scroll_animations_(false),
-      animation_waiting_for_deletion_(false) {
+      needs_push_properties_(false) {
   if (thread_instance_ == ThreadInstance::IMPL) {
     scroll_offset_animations_impl_ =
         base::MakeUnique<ScrollOffsetAnimationsImpl>(this);
@@ -91,6 +91,7 @@ void AnimationHost::AddAnimationTimeline(
   timeline->SetAnimationHost(this);
   id_to_timeline_map_.insert(
       std::make_pair(timeline->id(), std::move(timeline)));
+  SetNeedsPushProperties();
 }
 
 void AnimationHost::RemoveAnimationTimeline(
@@ -98,6 +99,7 @@ void AnimationHost::RemoveAnimationTimeline(
   DCHECK(timeline->id());
   EraseTimeline(timeline);
   id_to_timeline_map_.erase(timeline->id());
+  SetNeedsPushProperties();
 }
 
 void AnimationHost::RegisterElement(ElementId element_id,
@@ -159,23 +161,31 @@ void AnimationHost::SetMutatorHostClient(MutatorHostClient* client) {
     return;
 
   mutator_host_client_ = client;
+
+  if (needs_push_properties() && mutator_host_client())
+    SetNeedsPushProperties();
 }
 
 void AnimationHost::SetNeedsCommit() {
   DCHECK(mutator_host_client_);
   mutator_host_client_->SetMutatorsNeedCommit();
+  // TODO(loyso): Invalidate property trees only if really needed.
+  mutator_host_client_->SetMutatorsNeedRebuildPropertyTrees();
 }
 
-void AnimationHost::SetNeedsRebuildPropertyTrees() {
-  DCHECK(mutator_host_client_);
-  mutator_host_client_->SetMutatorsNeedRebuildPropertyTrees();
+void AnimationHost::SetNeedsPushProperties() {
+  needs_push_properties_ = true;
 }
 
 void AnimationHost::PushPropertiesTo(AnimationHost* host_impl) {
-  PushTimelinesToImplThread(host_impl);
-  RemoveTimelinesFromImplThread(host_impl);
-  PushPropertiesToImplThread(host_impl);
-  animation_waiting_for_deletion_ = false;
+  if (needs_push_properties_) {
+    needs_push_properties_ = false;
+    PushTimelinesToImplThread(host_impl);
+    RemoveTimelinesFromImplThread(host_impl);
+    PushPropertiesToImplThread(host_impl);
+    // This is redundant but used in tests.
+    host_impl->needs_push_properties_ = false;
+  }
 }
 
 void AnimationHost::PushTimelinesToImplThread(AnimationHost* host_impl) const {
@@ -212,19 +222,24 @@ void AnimationHost::PushPropertiesToImplThread(AnimationHost* host_impl) {
   // to happen before the element animations are synced below.
   for (auto& kv : id_to_timeline_map_) {
     AnimationTimeline* timeline = kv.second.get();
-    AnimationTimeline* timeline_impl =
-        host_impl->GetTimelineById(timeline->id());
-    if (timeline_impl)
-      timeline->PushPropertiesTo(timeline_impl);
+    if (timeline->needs_push_properties()) {
+      AnimationTimeline* timeline_impl =
+          host_impl->GetTimelineById(timeline->id());
+      if (timeline_impl)
+        timeline->PushPropertiesTo(timeline_impl);
+    }
   }
 
   // Sync properties for created ElementAnimations.
   for (auto& kv : element_to_animations_map_) {
     const auto& element_animations = kv.second;
-    auto element_animations_impl =
-        host_impl->GetElementAnimationsForElementId(kv.first);
-    if (element_animations_impl)
-      element_animations->PushPropertiesTo(std::move(element_animations_impl));
+    if (element_animations->needs_push_properties()) {
+      auto element_animations_impl =
+          host_impl->GetElementAnimationsForElementId(kv.first);
+      if (element_animations_impl)
+        element_animations->PushPropertiesTo(
+            std::move(element_animations_impl));
+    }
   }
 
   // Update the impl-only scroll offset animations.
@@ -571,8 +586,4 @@ AnimationHost::all_element_animations_for_testing() const {
   return element_to_animations_map_;
 }
 
-void AnimationHost::OnAnimationWaitingForDeletion() {
-  animation_waiting_for_deletion_ = true;
-}
-
 }  // namespace cc

cc/animation/animation_host.h

@@ -80,7 +80,8 @@ class CC_EXPORT AnimationHost {
   void SetMutatorHostClient(MutatorHostClient* client);
 
   void SetNeedsCommit();
-  void SetNeedsRebuildPropertyTrees();
+  void SetNeedsPushProperties();
+  bool needs_push_properties() const { return needs_push_properties_; }
 
   void PushPropertiesTo(AnimationHost* host_impl);
 
@@ -172,11 +173,6 @@ class CC_EXPORT AnimationHost {
   const ElementToAnimationsMap& active_element_animations_for_testing() const;
   const ElementToAnimationsMap& all_element_animations_for_testing() const;
 
-  bool animation_waiting_for_deletion() const {
-    return animation_waiting_for_deletion_;
-  }
-  void OnAnimationWaitingForDeletion();
-
  private:
   explicit AnimationHost(ThreadInstance thread_instance);
 
@@ -202,7 +198,7 @@ class CC_EXPORT AnimationHost {
   const ThreadInstance thread_instance_;
 
   bool supports_scroll_animations_;
-  bool animation_waiting_for_deletion_;
+  bool needs_push_properties_;
 
   DISALLOW_COPY_AND_ASSIGN(AnimationHost);
 };

cc/animation/animation_host_perftest.cc

@@ -23,6 +23,13 @@ namespace cc {
 
 class AnimationHostPerfTest : public testing::Test {
  protected:
+  AnimationHostPerfTest()
+      : root_layer_impl_(),
+        first_timeline_id_(),
+        last_timeline_id_(),
+        first_player_id_(),
+        last_player_id_() {}
+
   void SetUp() override {
     LayerTreeSettings settings;
     layer_tree_host_ =
@@ -52,12 +59,12 @@ class AnimationHostPerfTest : public testing::Test {
   }
 
   void CreatePlayers(const int num_players) {
-    scoped_refptr<AnimationTimeline> timeline =
+    all_players_timeline_ =
         AnimationTimeline::Create(AnimationIdProvider::NextTimelineId());
-    host()->AddAnimationTimeline(timeline);
+    host()->AddAnimationTimeline(all_players_timeline_);
 
-    const int first_player_id = AnimationIdProvider::NextPlayerId();
-    int last_player_id = first_player_id;
+    first_player_id_ = AnimationIdProvider::NextPlayerId();
+    last_player_id_ = first_player_id_;
 
     for (int i = 0; i < num_players; ++i) {
       scoped_refptr<Layer> layer = Layer::Create();
@@ -65,10 +72,10 @@ class AnimationHostPerfTest : public testing::Test {
       layer->SetElementId(LayerIdToElementIdForTesting(layer->id()));
 
       scoped_refptr<AnimationPlayer> player =
-          AnimationPlayer::Create(last_player_id);
-      last_player_id = AnimationIdProvider::NextPlayerId();
+          AnimationPlayer::Create(last_player_id_);
+      last_player_id_ = AnimationIdProvider::NextPlayerId();
 
-      timeline->AttachPlayer(player);
+      all_players_timeline_->AttachPlayer(player);
       player->AttachElement(layer->element_id());
       EXPECT_TRUE(player->element_animations());
     }
@@ -78,20 +85,20 @@ class AnimationHostPerfTest : public testing::Test {
 
     // Check impl instances created.
     scoped_refptr<AnimationTimeline> timeline_impl =
-        host_impl()->GetTimelineById(timeline->id());
+        host_impl()->GetTimelineById(all_players_timeline_->id());
     EXPECT_TRUE(timeline_impl);
-    for (int i = first_player_id; i < last_player_id; ++i)
+    for (int i = first_player_id_; i < last_player_id_; ++i)
       EXPECT_TRUE(timeline_impl->GetPlayerById(i));
   }
 
   void CreateTimelines(const int num_timelines) {
-    const int first_timeline_id = AnimationIdProvider::NextTimelineId();
-    int last_timeline_id = first_timeline_id;
+    first_timeline_id_ = AnimationIdProvider::NextTimelineId();
+    last_timeline_id_ = first_timeline_id_;
 
     for (int i = 0; i < num_timelines; ++i) {
       scoped_refptr<AnimationTimeline> timeline =
-          AnimationTimeline::Create(last_timeline_id);
-      last_timeline_id = AnimationIdProvider::NextTimelineId();
+          AnimationTimeline::Create(last_timeline_id_);
+      last_timeline_id_ = AnimationIdProvider::NextTimelineId();
       host()->AddAnimationTimeline(timeline);
     }
 
@@ -99,13 +106,26 @@ class AnimationHostPerfTest : public testing::Test {
     layer_tree_host_->CommitAndCreateLayerImplTree();
 
     // Check impl instances created.
-    for (int i = first_timeline_id; i < last_timeline_id; ++i)
+    for (int i = first_timeline_id_; i < last_timeline_id_; ++i)
       EXPECT_TRUE(host_impl()->GetTimelineById(i));
   }
 
+  void SetAllTimelinesNeedPushProperties() const {
+    for (int i = first_timeline_id_; i < last_timeline_id_; ++i)
+      host_impl()->GetTimelineById(i)->SetNeedsPushProperties();
+  }
+
+  void SetAllPlayersNeedPushProperties() const {
+    for (int i = first_player_id_; i < last_player_id_; ++i)
+      all_players_timeline_->GetPlayerById(i)->SetNeedsPushProperties();
+  }
+
   void DoTest() {
     timer_.Reset();
     do {
+      // Invalidate dirty flags.
+      SetAllTimelinesNeedPushProperties();
+      SetAllPlayersNeedPushProperties();
       host()->PushPropertiesTo(host_impl());
       timer_.NextLap();
     } while (!timer_.HasTimeLimitExpired());
@@ -114,12 +134,19 @@ class AnimationHostPerfTest : public testing::Test {
                            "runs/s", true);
   }
 
- protected:
+ private:
   StubLayerTreeHostSingleThreadClient single_thread_client_;
   FakeLayerTreeHostClient fake_client_;
   std::unique_ptr<FakeLayerTreeHost> layer_tree_host_;
   scoped_refptr<Layer> root_layer_;
   LayerImpl* root_layer_impl_;
+  scoped_refptr<AnimationTimeline> all_players_timeline_;
+
+  int first_timeline_id_;
+  int last_timeline_id_;
+
+  int first_player_id_;
+  int last_player_id_;
 
   LapTimer timer_;
   TestTaskGraphRunner task_graph_runner_;

cc/animation/animation_player.cc

@@ -22,7 +22,8 @@ AnimationPlayer::AnimationPlayer(int id)
       animation_timeline_(),
       element_animations_(),
       animation_delegate_(),
-      id_(id) {
+      id_(id),
+      needs_push_properties_(false) {
   DCHECK(id_);
 }
 
@@ -104,15 +105,19 @@ void AnimationPlayer::BindElementAnimations() {
   DCHECK(element_animations_);
 
   // Pass all accumulated animations to ElementAnimations.
-  for (auto& animation : animations_) {
+  for (auto& animation : animations_)
     element_animations_->AddAnimation(std::move(animation));
-  }
+
   if (!animations_.empty())
     SetNeedsCommit();
+  SetNeedsPushProperties();
+
   animations_.clear();
 }
 
 void AnimationPlayer::UnbindElementAnimations() {
+  SetNeedsPushProperties();
+
   element_animations_ = nullptr;
   DCHECK(animations_.empty());
 }
@@ -124,6 +129,7 @@ void AnimationPlayer::AddAnimation(std::unique_ptr<Animation> animation) {
   if (element_animations_) {
     element_animations_->AddAnimation(std::move(animation));
     SetNeedsCommit();
+    SetNeedsPushProperties();
   } else {
     animations_.push_back(std::move(animation));
   }
@@ -134,12 +140,14 @@ void AnimationPlayer::PauseAnimation(int animation_id, double time_offset) {
   element_animations_->PauseAnimation(
       animation_id, base::TimeDelta::FromSecondsD(time_offset));
   SetNeedsCommit();
+  SetNeedsPushProperties();
 }
 
 void AnimationPlayer::RemoveAnimation(int animation_id) {
   if (element_animations_) {
     element_animations_->RemoveAnimation(animation_id);
     SetNeedsCommit();
+    SetNeedsPushProperties();
   } else {
     auto animations_to_remove = std::remove_if(
         animations_.begin(), animations_.end(),
@@ -154,6 +162,7 @@ void AnimationPlayer::AbortAnimation(int animation_id) {
   DCHECK(element_animations_);
   element_animations_->AbortAnimation(animation_id);
   SetNeedsCommit();
+  SetNeedsPushProperties();
 }
 
 void AnimationPlayer::AbortAnimations(TargetProperty::Type target_property,
@@ -161,6 +170,7 @@ void AnimationPlayer::AbortAnimations(TargetProperty::Type target_property,
   if (element_animations_) {
     element_animations_->AbortAnimations(target_property, needs_completion);
     SetNeedsCommit();
+    SetNeedsPushProperties();
   } else {
     auto animations_to_remove = std::remove_if(
         animations_.begin(), animations_.end(),
@@ -172,6 +182,10 @@ void AnimationPlayer::AbortAnimations(TargetProperty::Type target_property,
 }
 
 void AnimationPlayer::PushPropertiesTo(AnimationPlayer* player_impl) {
+  if (!needs_push_properties_)
+    return;
+  needs_push_properties_ = false;
+
   if (element_id_ != player_impl->element_id()) {
     if (player_impl->element_id())
       player_impl->DetachElement();
@@ -207,11 +221,19 @@ void AnimationPlayer::NotifyAnimationAborted(
                                                 group);
 }
 
+void AnimationPlayer::NotifyAnimationWaitingForDeletion() {
+  // We need to purge animations marked for deletion.
+  SetNeedsPushProperties();
+}
+
 void AnimationPlayer::NotifyAnimationTakeover(
     base::TimeTicks monotonic_time,
     TargetProperty::Type target_property,
     double animation_start_time,
     std::unique_ptr<AnimationCurve> curve) {
+  // We need to purge animations marked for deletion on CT.
+  SetNeedsPushProperties();
+
   if (animation_delegate_) {
     DCHECK(curve);
     animation_delegate_->NotifyAnimationTakeover(
@@ -223,7 +245,16 @@ void AnimationPlayer::NotifyAnimationTakeover(
 void AnimationPlayer::SetNeedsCommit() {
   DCHECK(animation_host_);
   animation_host_->SetNeedsCommit();
-  animation_host_->SetNeedsRebuildPropertyTrees();
+}
+
+void AnimationPlayer::SetNeedsPushProperties() {
+  needs_push_properties_ = true;
+
+  DCHECK(animation_timeline_);
+  animation_timeline_->SetNeedsPushProperties();
+
+  DCHECK(element_animations_);
+  element_animations_->SetNeedsPushProperties();
 }
 
 }  // namespace cc

cc/animation/animation_player.h

@@ -82,6 +82,7 @@ class CC_EXPORT AnimationPlayer : public base::RefCounted<AnimationPlayer> {
   void NotifyAnimationAborted(base::TimeTicks monotonic_time,
                               TargetProperty::Type target_property,
                               int group);
+  void NotifyAnimationWaitingForDeletion();
   void NotifyAnimationTakeover(base::TimeTicks monotonic_time,
                                TargetProperty::Type target_property,
                                double animation_start_time,
@@ -92,6 +93,9 @@ class CC_EXPORT AnimationPlayer : public base::RefCounted<AnimationPlayer> {
     return !animations_.empty();
   }
 
+  bool needs_push_properties() const { return needs_push_properties_; }
+  void SetNeedsPushProperties();
+
  private:
   friend class base::RefCounted<AnimationPlayer>;
 
@@ -119,6 +123,7 @@ class CC_EXPORT AnimationPlayer : public base::RefCounted<AnimationPlayer> {
 
   int id_;
   ElementId element_id_;
+  bool needs_push_properties_;
 
   DISALLOW_COPY_AND_ASSIGN(AnimationPlayer);
 };

cc/animation/animation_player_unittest.cc

@@ -24,11 +24,18 @@ class AnimationPlayerTest : public AnimationTimelinesTest {
 // See element_animations_unittest.cc for active/pending observers tests.
 
 TEST_F(AnimationPlayerTest, AttachDetachLayerIfTimelineAttached) {
+  EXPECT_TRUE(CheckPlayerTimelineNeedsPushProperties(false));
   host_->AddAnimationTimeline(timeline_);
+  EXPECT_TRUE(timeline_->needs_push_properties());
+  EXPECT_FALSE(player_->needs_push_properties());
+
   timeline_->AttachPlayer(player_);
   EXPECT_FALSE(player_->element_animations());
   EXPECT_FALSE(player_->element_id());
 
+  EXPECT_TRUE(timeline_->needs_push_properties());
+  EXPECT_FALSE(player_->needs_push_properties());
+
   host_->PushPropertiesTo(host_impl_);
 
   EXPECT_FALSE(GetImplPlayerForLayerId(element_id_));
@@ -37,33 +44,44 @@ TEST_F(AnimationPlayerTest, AttachDetachLayerIfTimelineAttached) {
 
   EXPECT_FALSE(player_impl_->element_animations());
   EXPECT_FALSE(player_impl_->element_id());
+  EXPECT_FALSE(player_->needs_push_properties());
+  EXPECT_FALSE(timeline_->needs_push_properties());
 
   player_->AttachElement(element_id_);
   EXPECT_EQ(player_, GetPlayerForElementId(element_id_));
   EXPECT_TRUE(player_->element_animations());
   EXPECT_EQ(player_->element_id(), element_id_);
+  EXPECT_TRUE(CheckPlayerTimelineNeedsPushProperties(true));
 
   host_->PushPropertiesTo(host_impl_);
 
   EXPECT_EQ(player_impl_, GetImplPlayerForLayerId(element_id_));
   EXPECT_TRUE(player_impl_->element_animations());
   EXPECT_EQ(player_impl_->element_id(), element_id_);
+  EXPECT_TRUE(CheckPlayerTimelineNeedsPushProperties(false));
 
   player_->DetachElement();
   EXPECT_FALSE(GetPlayerForElementId(element_id_));
   EXPECT_FALSE(player_->element_animations());
   EXPECT_FALSE(player_->element_id());
+  EXPECT_TRUE(CheckPlayerTimelineNeedsPushProperties(true));
 
   host_->PushPropertiesTo(host_impl_);
 
   EXPECT_FALSE(GetImplPlayerForLayerId(element_id_));
   EXPECT_FALSE(player_impl_->element_animations());
   EXPECT_FALSE(player_impl_->element_id());
+  EXPECT_TRUE(CheckPlayerTimelineNeedsPushProperties(false));
 
   timeline_->DetachPlayer(player_);
   EXPECT_FALSE(player_->animation_timeline());
   EXPECT_FALSE(player_->element_animations());
   EXPECT_FALSE(player_->element_id());
+  EXPECT_TRUE(timeline_->needs_push_properties());
+  EXPECT_FALSE(player_->needs_push_properties());
+
+  host_->PushPropertiesTo(host_impl_);
+  EXPECT_TRUE(CheckPlayerTimelineNeedsPushProperties(false));
 }
 
 TEST_F(AnimationPlayerTest, AttachDetachTimelineIfLayerAttached) {
@@ -71,18 +89,21 @@ TEST_F(AnimationPlayerTest, AttachDetachTimelineIfLayerAttached) {
 
   EXPECT_FALSE(player_->element_animations());
   EXPECT_FALSE(player_->element_id());
+  EXPECT_FALSE(player_->needs_push_properties());
 
   player_->AttachElement(element_id_);
   EXPECT_FALSE(player_->animation_timeline());
   EXPECT_FALSE(GetPlayerForElementId(element_id_));
   EXPECT_FALSE(player_->element_animations());
   EXPECT_EQ(player_->element_id(), element_id_);
+  EXPECT_FALSE(player_->needs_push_properties());
 
   timeline_->AttachPlayer(player_);
   EXPECT_EQ(timeline_, player_->animation_timeline());
   EXPECT_EQ(player_, GetPlayerForElementId(element_id_));
   EXPECT_TRUE(player_->element_animations());
   EXPECT_EQ(player_->element_id(), element_id_);
+  EXPECT_TRUE(player_->needs_push_properties());
 
   // Removing player from timeline detaches layer.
   timeline_->DetachPlayer(player_);
@@ -90,6 +111,7 @@ TEST_F(AnimationPlayerTest, AttachDetachTimelineIfLayerAttached) {
   EXPECT_FALSE(GetPlayerForElementId(element_id_));
   EXPECT_FALSE(player_->element_animations());
   EXPECT_FALSE(player_->element_id());
+  EXPECT_TRUE(player_->needs_push_properties());
 }
 
 TEST_F(AnimationPlayerTest, PropertiesMutate) {
@@ -100,6 +122,10 @@ TEST_F(AnimationPlayerTest, PropertiesMutate) {
   host_->AddAnimationTimeline(timeline_);
   timeline_->AttachPlayer(player_);
   player_->AttachElement(element_id_);
+  EXPECT_TRUE(CheckPlayerTimelineNeedsPushProperties(true));
+
+  host_->PushPropertiesTo(host_impl_);
+  EXPECT_TRUE(CheckPlayerTimelineNeedsPushProperties(false));
 
   const float start_opacity = .7f;
   const float end_opacity = .3f;
@@ -118,8 +144,10 @@ TEST_F(AnimationPlayerTest, PropertiesMutate) {
                                transform_y);
   AddAnimatedFilterToPlayer(player_.get(), duration, start_brightness,
                             end_brightness);
+  EXPECT_TRUE(CheckPlayerTimelineNeedsPushProperties(true));
 
   host_->PushPropertiesTo(host_impl_);
+  EXPECT_TRUE(CheckPlayerTimelineNeedsPushProperties(false));
 
   EXPECT_FALSE(client_.IsPropertyMutated(element_id_, ElementListType::ACTIVE,
                                          TargetProperty::OPACITY));
@@ -140,9 +168,11 @@ TEST_F(AnimationPlayerTest, PropertiesMutate) {
   base::TimeTicks time;
   time += base::TimeDelta::FromSecondsD(0.1);
   AnimateLayersTransferEvents(time, 3u);
+  EXPECT_TRUE(CheckPlayerTimelineNeedsPushProperties(false));
 
   time += base::TimeDelta::FromSecondsD(duration);
   AnimateLayersTransferEvents(time, 3u);
+  EXPECT_TRUE(CheckPlayerTimelineNeedsPushProperties(false));
 
   client_.ExpectOpacityPropertyMutated(element_id_, ElementListType::ACTIVE,
                                        end_opacity);
@@ -180,8 +210,12 @@ TEST_F(AnimationPlayerTest, AttachTwoPlayersToOneLayer) {
       AnimationPlayer::Create(AnimationIdProvider::NextPlayerId());
 
   host_->AddAnimationTimeline(timeline_);
+
   timeline_->AttachPlayer(player1);
+  EXPECT_TRUE(timeline_->needs_push_properties());
+
   timeline_->AttachPlayer(player2);
+  EXPECT_TRUE(timeline_->needs_push_properties());
 
   player1->set_animation_delegate(&delegate1);
   player2->set_animation_delegate(&delegate2);
@@ -222,12 +256,18 @@ TEST_F(AnimationPlayerTest, AttachTwoPlayersToOneLayer) {
   EXPECT_TRUE(delegate2.started());
   EXPECT_FALSE(delegate2.finished());
 
+  EXPECT_FALSE(player1->needs_push_properties());
+  EXPECT_FALSE(player2->needs_push_properties());
+
   time += base::TimeDelta::FromSecondsD(duration);
   AnimateLayersTransferEvents(time, 2u);
 
   EXPECT_TRUE(delegate1.finished());
   EXPECT_TRUE(delegate2.finished());
 
+  EXPECT_FALSE(player1->needs_push_properties());
+  EXPECT_FALSE(player2->needs_push_properties());
+
   client_.ExpectOpacityPropertyMutated(element_id_, ElementListType::ACTIVE,
                                        end_opacity);
   client_.ExpectTransformPropertyMutated(element_id_, ElementListType::ACTIVE,
@@ -258,11 +298,15 @@ TEST_F(AnimationPlayerTest, AddRemoveAnimationToNonAttachedPlayer) {
   const int opacity_id = AddOpacityTransitionToPlayer(
       player_.get(), duration, start_opacity, end_opacity, false);
 
+  EXPECT_FALSE(player_->needs_push_properties());
+
   host_->AddAnimationTimeline(timeline_);
   timeline_->AttachPlayer(player_);
 
+  EXPECT_FALSE(player_->needs_push_properties());
   EXPECT_FALSE(player_->element_animations());
   player_->RemoveAnimation(filter_id);
+  EXPECT_FALSE(player_->needs_push_properties());
 
   player_->AttachElement(element_id_);
 
@@ -271,6 +315,7 @@ TEST_F(AnimationPlayerTest, AddRemoveAnimationToNonAttachedPlayer) {
                    ->GetAnimationById(filter_id));
   EXPECT_TRUE(player_->element_animations()
                   ->GetAnimationById(opacity_id));
+  EXPECT_TRUE(player_->needs_push_properties());
 
   host_->PushPropertiesTo(host_impl_);
 
@@ -347,6 +392,7 @@ TEST_F(AnimationPlayerTest, SwitchToLayer) {
   EXPECT_EQ(player_impl_, GetImplPlayerForLayerId(element_id_));
   EXPECT_TRUE(player_impl_->element_animations());
   EXPECT_EQ(player_impl_->element_id(), element_id_);
+  EXPECT_TRUE(CheckPlayerTimelineNeedsPushProperties(false));
 
   const ElementId new_element_id(NextTestLayerId(), 0);
   player_->DetachElement();
@@ -355,6 +401,7 @@ TEST_F(AnimationPlayerTest, SwitchToLayer) {
   EXPECT_EQ(player_, GetPlayerForElementId(new_element_id));
   EXPECT_TRUE(player_->element_animations());
   EXPECT_EQ(player_->element_id(), new_element_id);
+  EXPECT_TRUE(CheckPlayerTimelineNeedsPushProperties(true));
 
   host_->PushPropertiesTo(host_impl_);
 

cc/animation/animation_timeline.cc

@@ -6,6 +6,7 @@
 
 #include <algorithm>
 
+#include "cc/animation/animation_host.h"
 #include "cc/animation/animation_player.h"
 
 namespace cc {
@@ -15,8 +16,10 @@ scoped_refptr<AnimationTimeline> AnimationTimeline::Create(int id) {
 }
 
 AnimationTimeline::AnimationTimeline(int id)
-    : id_(id), animation_host_(), is_impl_only_(false) {
-}
+    : id_(id),
+      animation_host_(),
+      needs_push_properties_(false),
+      is_impl_only_(false) {}
 
 AnimationTimeline::~AnimationTimeline() {
   for (auto& kv : id_to_player_map_)
@@ -29,9 +32,14 @@ scoped_refptr<AnimationTimeline> AnimationTimeline::CreateImplInstance() const {
 }
 
 void AnimationTimeline::SetAnimationHost(AnimationHost* animation_host) {
+  if (animation_host_ == animation_host)
+    return;
+
   animation_host_ = animation_host;
   for (auto& kv : id_to_player_map_)
     kv.second->SetAnimationHost(animation_host);
+
+  SetNeedsPushProperties();
 }
 
 void AnimationTimeline::AttachPlayer(scoped_refptr<AnimationPlayer> player) {
@@ -39,12 +47,16 @@ void AnimationTimeline::AttachPlayer(scoped_refptr<AnimationPlayer> player) {
   player->SetAnimationHost(animation_host_);
   player->SetAnimationTimeline(this);
   id_to_player_map_.insert(std::make_pair(player->id(), std::move(player)));
+
+  SetNeedsPushProperties();
 }
 
 void AnimationTimeline::DetachPlayer(scoped_refptr<AnimationPlayer> player) {
   DCHECK(player->id());
   ErasePlayer(player);
   id_to_player_map_.erase(player->id());
+
+  SetNeedsPushProperties();
 }
 
 AnimationPlayer* AnimationTimeline::GetPlayerById(int player_id) const {
@@ -56,12 +68,23 @@ void AnimationTimeline::ClearPlayers() {
   for (auto& kv : id_to_player_map_)
     ErasePlayer(kv.second);
   id_to_player_map_.clear();
+
+  SetNeedsPushProperties();
+}
+
+void AnimationTimeline::SetNeedsPushProperties() {
+  needs_push_properties_ = true;
+  if (animation_host_)
+    animation_host_->SetNeedsPushProperties();
 }
 
 void AnimationTimeline::PushPropertiesTo(AnimationTimeline* timeline_impl) {
-  PushAttachedPlayersToImplThread(timeline_impl);
-  RemoveDetachedPlayersFromImplThread(timeline_impl);
-  PushPropertiesToImplThread(timeline_impl);
+  if (needs_push_properties_) {
+    needs_push_properties_ = false;
+    PushAttachedPlayersToImplThread(timeline_impl);
+    RemoveDetachedPlayersFromImplThread(timeline_impl);
+    PushPropertiesToImplThread(timeline_impl);
+  }
 }
 
 void AnimationTimeline::PushAttachedPlayersToImplThread(
@@ -103,9 +126,11 @@ void AnimationTimeline::PushPropertiesToImplThread(
     AnimationTimeline* timeline_impl) {
   for (auto& kv : id_to_player_map_) {
     AnimationPlayer* player = kv.second.get();
-    AnimationPlayer* player_impl = timeline_impl->GetPlayerById(player->id());
-    if (player_impl)
-      player->PushPropertiesTo(player_impl);
+    if (player->needs_push_properties()) {
+      AnimationPlayer* player_impl = timeline_impl->GetPlayerById(player->id());
+      if (player_impl)
+        player->PushPropertiesTo(player_impl);
+    }
   }
 }
 

cc/animation/animation_timeline.h

@@ -46,6 +46,9 @@ class CC_EXPORT AnimationTimeline : public base::RefCounted<AnimationTimeline> {
 
   AnimationPlayer* GetPlayerById(int player_id) const;
 
+  void SetNeedsPushProperties();
+  bool needs_push_properties() const { return needs_push_properties_; }
+
  private:
   friend class base::RefCounted<AnimationTimeline>;
 
@@ -64,6 +67,7 @@ class CC_EXPORT AnimationTimeline : public base::RefCounted<AnimationTimeline> {
 
   int id_;
   AnimationHost* animation_host_;
+  bool needs_push_properties_;
 
   // Impl-only AnimationTimeline has no main thread instance and lives on
   // it's own.

cc/animation/element_animations.cc

@@ -33,7 +33,8 @@ ElementAnimations::ElementAnimations()
       has_element_in_active_list_(false),
       has_element_in_pending_list_(false),
       needs_to_start_animations_(false),
-      scroll_offset_animation_was_interrupted_(false) {}
+      scroll_offset_animation_was_interrupted_(false),
+      needs_push_properties_(false) {}
 
 ElementAnimations::~ElementAnimations() {}
 
@@ -122,9 +123,18 @@ bool ElementAnimations::IsEmpty() const {
   return !players_list_->might_have_observers();
 }
 
+void ElementAnimations::SetNeedsPushProperties() {
+  needs_push_properties_ = true;
+}
+
 void ElementAnimations::PushPropertiesTo(
     scoped_refptr<ElementAnimations> element_animations_impl) {
   DCHECK_NE(this, element_animations_impl);
+
+  if (!needs_push_properties_)
+    return;
+  needs_push_properties_ = false;
+
   if (!has_any_animation() && !element_animations_impl->has_any_animation())
     return;
   MarkAbortedAnimationsForDeletion(element_animations_impl.get());
@@ -157,6 +167,7 @@ void ElementAnimations::AddAnimation(std::unique_ptr<Animation> animation) {
     default:
       break;
   }
+  SetNeedsPushProperties();
 }
 
 void ElementAnimations::Animate(base::TimeTicks monotonic_time) {
@@ -886,7 +897,7 @@ void ElementAnimations::MarkAnimationsForDeletion(
     }
   }
   if (marked_animations_for_deletions)
-    NotifyClientAnimationWaitingForDeletion();
+    NotifyPlayersAnimationWaitingForDeletion();
 }
 
 void ElementAnimations::MarkAbortedAnimationsForDeletion(
@@ -1065,10 +1076,6 @@ void ElementAnimations::NotifyClientScrollOffsetAnimated(
     OnScrollOffsetAnimated(ElementListType::PENDING, scroll_offset);
 }
 
-void ElementAnimations::NotifyClientAnimationWaitingForDeletion() {
-  OnAnimationWaitingForDeletion();
-}
-
 void ElementAnimations::NotifyClientAnimationChanged(
     TargetProperty::Type property,
     ElementListType list_type,
@@ -1245,6 +1252,7 @@ void ElementAnimations::PauseAnimation(int animation_id,
                                       animations_[i]->time_offset());
     }
   }
+  SetNeedsPushProperties();
 }
 
 void ElementAnimations::RemoveAnimation(int animation_id) {
@@ -1282,6 +1290,8 @@ void ElementAnimations::RemoveAnimation(int animation_id) {
     UpdateClientAnimationState(TargetProperty::OPACITY);
   if (removed_filter_animation)
     UpdateClientAnimationState(TargetProperty::FILTER);
+
+  SetNeedsPushProperties();
 }
 
 void ElementAnimations::AbortAnimation(int animation_id) {
@@ -1299,6 +1309,8 @@ void ElementAnimations::AbortAnimation(int animation_id) {
       }
     }
   }
+
+  SetNeedsPushProperties();
 }
 
 void ElementAnimations::AbortAnimations(TargetProperty::Type target_property,
@@ -1332,6 +1344,8 @@ void ElementAnimations::AbortAnimations(TargetProperty::Type target_property,
         break;
     }
   }
+
+  SetNeedsPushProperties();
 }
 
 Animation* ElementAnimations::GetAnimation(
@@ -1388,13 +1402,6 @@ void ElementAnimations::OnScrollOffsetAnimated(
       element_id(), list_type, scroll_offset);
 }
 
-void ElementAnimations::OnAnimationWaitingForDeletion() {
-  // TODO(loyso): Invalidate AnimationHost::SetNeedsPushProperties here.
-  // But we always do PushProperties in AnimationHost for now. crbug.com/604280
-  DCHECK(animation_host());
-  animation_host()->OnAnimationWaitingForDeletion();
-}
-
 void ElementAnimations::IsAnimatingChanged(ElementListType list_type,
                                            TargetProperty::Type property,
                                            AnimationChangeType change_type,
@@ -1461,6 +1468,13 @@ void ElementAnimations::NotifyPlayersAnimationAborted(
     player->NotifyAnimationAborted(monotonic_time, target_property, group);
 }
 
+void ElementAnimations::NotifyPlayersAnimationWaitingForDeletion() {
+  ElementAnimations::PlayersList::Iterator it(players_list_.get());
+  AnimationPlayer* player;
+  while ((player = it.GetNext()) != nullptr)
+    player->NotifyAnimationWaitingForDeletion();
+}
+
 void ElementAnimations::NotifyPlayersAnimationTakeover(
     base::TimeTicks monotonic_time,
     TargetProperty::Type target_property,

cc/animation/element_animations.h

@@ -178,6 +178,9 @@ class CC_EXPORT ElementAnimations : public base::RefCounted<ElementAnimations> {
     return needs_to_start_animations_;
   }
 
+  void SetNeedsPushProperties();
+  bool needs_push_properties() const { return needs_push_properties_; }
+
  private:
   friend class base::RefCounted<ElementAnimations>;
 
@@ -221,9 +224,6 @@ class CC_EXPORT ElementAnimations : public base::RefCounted<ElementAnimations> {
   void NotifyClientScrollOffsetAnimated(const gfx::ScrollOffset& scroll_offset,
                                         bool notify_active_elements,
                                         bool notify_pending_elements);
-
-  void NotifyClientAnimationWaitingForDeletion();
-
   void NotifyClientAnimationChanged(
       TargetProperty::Type property,
       ElementListType list_type,
@@ -239,7 +239,6 @@ class CC_EXPORT ElementAnimations : public base::RefCounted<ElementAnimations> {
                            const gfx::Transform& transform);
   void OnScrollOffsetAnimated(ElementListType list_type,
                               const gfx::ScrollOffset& scroll_offset);
-  void OnAnimationWaitingForDeletion();
   void IsAnimatingChanged(ElementListType list_type,
                           TargetProperty::Type property,
                           AnimationChangeType change_type,
@@ -255,6 +254,7 @@ class CC_EXPORT ElementAnimations : public base::RefCounted<ElementAnimations> {
   void NotifyPlayersAnimationAborted(base::TimeTicks monotonic_time,
                                      TargetProperty::Type target_property,
                                      int group);
+  void NotifyPlayersAnimationWaitingForDeletion();
   void NotifyPlayersAnimationPropertyUpdate(const AnimationEvent& event);
   void NotifyPlayersAnimationTakeover(base::TimeTicks monotonic_time,
                                       TargetProperty::Type target_property,
@@ -280,6 +280,8 @@ class CC_EXPORT ElementAnimations : public base::RefCounted<ElementAnimations> {
 
   bool scroll_offset_animation_was_interrupted_;
 
+  bool needs_push_properties_;
+
   struct PropertyAnimationState {
     bool currently_running_for_active_elements = false;
     bool currently_running_for_pending_elements = false;

cc/animation/element_animations_unittest.cc

@@ -665,6 +665,7 @@ TEST_F(ElementAnimationsTest, AnimationsAreDeleted) {
                                           false);
   animations->Animate(kInitialTickTime);
   animations->UpdateState(true, nullptr);
+  EXPECT_TRUE(animations->needs_push_properties());
   animations->PushPropertiesTo(animations_impl.get());
   animations_impl->ActivateAnimations();
 
@@ -679,15 +680,15 @@ TEST_F(ElementAnimationsTest, AnimationsAreDeleted) {
   animations->Animate(kInitialTickTime + TimeDelta::FromMilliseconds(1000));
   animations->UpdateState(true, nullptr);
 
-  EXPECT_FALSE(host_->animation_waiting_for_deletion());
-  EXPECT_FALSE(host_impl_->animation_waiting_for_deletion());
+  EXPECT_FALSE(host_->needs_push_properties());
+  EXPECT_FALSE(host_impl_->needs_push_properties());
 
   events = host_impl_->CreateEvents();
   animations_impl->Animate(kInitialTickTime +
                            TimeDelta::FromMilliseconds(2000));
   animations_impl->UpdateState(true, events.get());
 
-  EXPECT_TRUE(host_impl_->animation_waiting_for_deletion());
+  EXPECT_TRUE(host_impl_->needs_push_properties());
 
   // There should be a FINISHED event for the animation.
   EXPECT_EQ(1u, events->events_.size());
@@ -701,7 +702,7 @@ TEST_F(ElementAnimationsTest, AnimationsAreDeleted) {
 
   animations->Animate(kInitialTickTime + TimeDelta::FromMilliseconds(3000));
   animations->UpdateState(true, nullptr);
-  EXPECT_TRUE(host_->animation_waiting_for_deletion());
+  EXPECT_TRUE(host_->needs_push_properties());
 
   animations->PushPropertiesTo(animations_impl.get());
 
@@ -1687,6 +1688,7 @@ TEST_F(ElementAnimationsTest, PushUpdatesWhenSynchronizedStartTimeNeeded) {
   EXPECT_TRUE(active_animation);
   EXPECT_TRUE(active_animation->needs_synchronized_start_time());
 
+  EXPECT_TRUE(animations->needs_push_properties());
   animations->PushPropertiesTo(animations_impl.get());
   animations_impl->ActivateAnimations();
 
@@ -1950,15 +1952,16 @@ TEST_F(ElementAnimationsTest, MainThreadAbortedAnimationGetsDeleted) {
   animations->AbortAnimations(TargetProperty::OPACITY);
   EXPECT_EQ(Animation::ABORTED,
             animations->GetAnimation(TargetProperty::OPACITY)->run_state());
-  EXPECT_FALSE(host_->animation_waiting_for_deletion());
-  EXPECT_FALSE(host_impl_->animation_waiting_for_deletion());
+  EXPECT_FALSE(host_->needs_push_properties());
+  EXPECT_FALSE(host_impl_->needs_push_properties());
 
   animations->Animate(kInitialTickTime);
   animations->UpdateState(true, nullptr);
-  EXPECT_FALSE(host_->animation_waiting_for_deletion());
+  EXPECT_FALSE(host_->needs_push_properties());
   EXPECT_EQ(Animation::ABORTED,
             animations->GetAnimation(TargetProperty::OPACITY)->run_state());
 
+  EXPECT_TRUE(animations->needs_push_properties());
   animations->PushPropertiesTo(animations_impl.get());
   EXPECT_FALSE(animations->GetAnimationById(animation_id));
   EXPECT_FALSE(animations_impl->GetAnimationById(animation_id));
@@ -1987,13 +1990,13 @@ TEST_F(ElementAnimationsTest, ImplThreadAbortedAnimationGetsDeleted) {
   EXPECT_EQ(
       Animation::ABORTED,
       animations_impl->GetAnimation(TargetProperty::OPACITY)->run_state());
-  EXPECT_FALSE(host_->animation_waiting_for_deletion());
-  EXPECT_FALSE(host_impl_->animation_waiting_for_deletion());
+  EXPECT_FALSE(host_->needs_push_properties());
+  EXPECT_FALSE(host_impl_->needs_push_properties());
 
   auto events = host_impl_->CreateEvents();
   animations_impl->Animate(kInitialTickTime);
   animations_impl->UpdateState(true, events.get());
-  EXPECT_TRUE(host_impl_->animation_waiting_for_deletion());
+  EXPECT_TRUE(host_impl_->needs_push_properties());
   EXPECT_EQ(1u, events->events_.size());
   EXPECT_EQ(AnimationEvent::ABORTED, events->events_[0].type);
   EXPECT_EQ(
@@ -2007,7 +2010,7 @@ TEST_F(ElementAnimationsTest, ImplThreadAbortedAnimationGetsDeleted) {
 
   animations->Animate(kInitialTickTime + TimeDelta::FromMilliseconds(500));
   animations->UpdateState(true, nullptr);
-  EXPECT_TRUE(host_->animation_waiting_for_deletion());
+  EXPECT_TRUE(host_->needs_push_properties());
   EXPECT_EQ(Animation::WAITING_FOR_DELETION,
             animations->GetAnimation(TargetProperty::OPACITY)->run_state());
 
@@ -2057,14 +2060,14 @@ TEST_F(ElementAnimationsTest, ImplThreadTakeoverAnimationGetsDeleted) {
   EXPECT_EQ(Animation::ABORTED_BUT_NEEDS_COMPLETION,
             animations_impl->GetAnimation(TargetProperty::SCROLL_OFFSET)
                 ->run_state());
-  EXPECT_FALSE(host_->animation_waiting_for_deletion());
-  EXPECT_FALSE(host_impl_->animation_waiting_for_deletion());
+  EXPECT_FALSE(host_->needs_push_properties());
+  EXPECT_FALSE(host_impl_->needs_push_properties());
 
   auto events = host_impl_->CreateEvents();
   animations_impl->Animate(kInitialTickTime);
   animations_impl->UpdateState(true, events.get());
   EXPECT_TRUE(delegate_impl.finished());
-  EXPECT_TRUE(host_impl_->animation_waiting_for_deletion());
+  EXPECT_TRUE(host_impl_->needs_push_properties());
   EXPECT_EQ(1u, events->events_.size());
   EXPECT_EQ(AnimationEvent::TAKEOVER, events->events_[0].type);
   EXPECT_EQ(123, events->events_[0].animation_start_time);
@@ -2075,9 +2078,16 @@ TEST_F(ElementAnimationsTest, ImplThreadTakeoverAnimationGetsDeleted) {
             animations_impl->GetAnimation(TargetProperty::SCROLL_OFFSET)
                 ->run_state());
 
+  // MT receives the event to take over.
   animations->NotifyAnimationTakeover(events->events_[0]);
   EXPECT_TRUE(delegate.takeover());
 
+  // AnimationPlayer::NotifyAnimationTakeover requests SetNeedsPushProperties
+  // to purge CT animations marked for deletion.
+  EXPECT_TRUE(animations->needs_push_properties());
+
+  // ElementAnimations::PurgeAnimationsMarkedForDeletion call happens only in
+  // ElementAnimations::PushPropertiesTo.
   animations->PushPropertiesTo(animations_impl.get());
   animations_impl->ActivateAnimations();
   EXPECT_FALSE(animations->GetAnimationById(animation_id));

cc/animation/scroll_offset_animations.cc

@@ -34,6 +34,7 @@ void ScrollOffsetAnimations::AddAdjustmentUpdate(ElementId element_id,
   update.adjustment_ += adjustment;
   element_to_update_map_[element_id] = update;
   animation_host_->SetNeedsCommit();
+  animation_host_->SetNeedsPushProperties();
 }
 
 void ScrollOffsetAnimations::AddTakeoverUpdate(ElementId element_id) {
@@ -42,6 +43,7 @@ void ScrollOffsetAnimations::AddTakeoverUpdate(ElementId element_id) {
   update.takeover_ = true;
   element_to_update_map_[element_id] = update;
   animation_host_->SetNeedsCommit();
+  animation_host_->SetNeedsPushProperties();
 }
 
 bool ScrollOffsetAnimations::HasUpdatesForTesting() const {

cc/test/animation_timelines_test_common.cc

@@ -518,4 +518,33 @@ int AnimationTimelinesTest::NextTestLayerId() {
   return next_test_layer_id_;
 }
 
+bool AnimationTimelinesTest::CheckPlayerTimelineNeedsPushProperties(
+    bool needs_push_properties) const {
+  DCHECK(player_);
+  DCHECK(timeline_);
+
+  bool result = true;
+
+  if (player_->needs_push_properties() != needs_push_properties) {
+    ADD_FAILURE() << "player_->needs_push_properties() expected to be "
+                  << needs_push_properties;
+    result = false;
+  }
+  if (timeline_->needs_push_properties() != needs_push_properties) {
+    ADD_FAILURE() << "timeline_->needs_push_properties() expected to be "
+                  << needs_push_properties;
+    result = false;
+  }
+  if (player_->element_animations() &&
+      player_->element_animations()->needs_push_properties() !=
+          needs_push_properties) {
+    ADD_FAILURE() << "player_->element_animations()->needs_push_properties() "
+                     "expected to be "
+                  << needs_push_properties;
+    result = false;
+  }
+
+  return result;
+}
+
 }  // namespace cc

cc/test/animation_timelines_test_common.h

@@ -294,6 +294,8 @@ class AnimationTimelinesTest : public testing::Test {
 
   int NextTestLayerId();
 
+  bool CheckPlayerTimelineNeedsPushProperties(bool needs_push_properties) const;
+
   TestHostClient client_;
   TestHostClient client_impl_;