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

cc/animation/element_animations_unittest.cc

Index: cc/animation/element_animations_unittest.cc
diff --git a/cc/animation/element_animations_unittest.cc b/cc/animation/element_animations_unittest.cc
index 34107fc5c56bac9e437930be8c6bc59f25f996f2..50b509ffb3da608e09a104ae1d36b569009e5422 100644
--- a/cc/animation/element_animations_unittest.cc
+++ b/cc/animation/element_animations_unittest.cc
@@ -272,7 +272,7 @@ TEST_F(ElementAnimationsTest,
   const int animation1_id = 1;
   std::unique_ptr<Animation> animation_fixed(Animation::Create(
       std::move(curve_fixed), animation1_id, 0, TargetProperty::SCROLL_OFFSET));
-  animations->AddAnimation(std::move(animation_fixed));
+  AddAnimationToElementAnimations(animations.get(), std::move(animation_fixed));
   animations->PushPropertiesTo(animations_impl.get());
   EXPECT_VECTOR2DF_EQ(initial_value,
                       animations_impl->GetAnimationById(animation1_id)
@@ -288,7 +288,7 @@ TEST_F(ElementAnimationsTest,
   const int animation2_id = 2;
   std::unique_ptr<Animation> animation(Animation::Create(
       std::move(curve), animation2_id, 0, TargetProperty::SCROLL_OFFSET));
-  animations->AddAnimation(std::move(animation));
+  AddAnimationToElementAnimations(animations.get(), std::move(animation));
   animations->PushPropertiesTo(animations_impl.get());
   EXPECT_VECTOR2DF_EQ(provider_initial_value,
                       animations_impl->GetAnimationById(animation2_id)
@@ -578,7 +578,8 @@ TEST_F(ElementAnimationsTest, SyncPause) {
 
   // Pause the animation at the middle of the second range so the offset
   // delays animation until the middle of the third range.
-  animations->PauseAnimation(animation_id, TimeDelta::FromSecondsD(1.5));
+  PauseAnimationInElementAnimations(animations.get(), animation_id,
+                                    TimeDelta::FromSecondsD(1.5));
   EXPECT_EQ(Animation::PAUSED,
             animations->GetAnimationById(animation_id)->run_state());
 
@@ -665,6 +666,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 +681,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 +703,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());
 
@@ -745,7 +747,7 @@ TEST_F(ElementAnimationsTest, TrivialTransition) {
       1, TargetProperty::OPACITY));
 
   EXPECT_FALSE(animations->needs_to_start_animations_for_testing());
-  animations->AddAnimation(std::move(to_add));
+  AddAnimationToElementAnimations(animations.get(), std::move(to_add));
   EXPECT_TRUE(animations->needs_to_start_animations_for_testing());
   animations->Animate(kInitialTickTime);
   EXPECT_FALSE(animations->needs_to_start_animations_for_testing());
@@ -785,7 +787,7 @@ TEST_F(ElementAnimationsTest, FilterTransition) {
 
   std::unique_ptr<Animation> animation(
       Animation::Create(std::move(curve), 1, 0, TargetProperty::FILTER));
-  animations->AddAnimation(std::move(animation));
+  AddAnimationToElementAnimations(animations.get(), std::move(animation));
 
   animations->Animate(kInitialTickTime);
   animations->UpdateState(true, events.get());
@@ -834,7 +836,7 @@ TEST_F(ElementAnimationsTest, ScrollOffsetTransition) {
   std::unique_ptr<Animation> animation(
       Animation::Create(std::move(curve), 1, 0, TargetProperty::SCROLL_OFFSET));
   animation->set_needs_synchronized_start_time(true);
-  animations->AddAnimation(std::move(animation));
+  AddAnimationToElementAnimations(animations.get(), std::move(animation));
 
   client_impl_.SetScrollOffsetForAnimation(initial_value);
   animations->PushPropertiesTo(animations_impl.get());
@@ -915,7 +917,7 @@ TEST_F(ElementAnimationsTest, ScrollOffsetTransitionOnImplOnly) {
   std::unique_ptr<Animation> animation(
       Animation::Create(std::move(curve), 1, 0, TargetProperty::SCROLL_OFFSET));
   animation->set_is_impl_only(true);
-  animations_impl->AddAnimation(std::move(animation));
+  AddAnimationToElementAnimations(animations_impl.get(), std::move(animation));
 
   animations_impl->Animate(kInitialTickTime);
   animations_impl->UpdateState(true, events.get());
@@ -1030,7 +1032,7 @@ TEST_F(ElementAnimationsTest, ScrollOffsetTransitionNoImplProvider) {
   std::unique_ptr<Animation> animation(
       Animation::Create(std::move(curve), 1, 0, TargetProperty::SCROLL_OFFSET));
   animation->set_needs_synchronized_start_time(true);
-  animations->AddAnimation(std::move(animation));
+  AddAnimationToElementAnimations(animations.get(), std::move(animation));
 
   client_.SetScrollOffsetForAnimation(initial_value);
   animations->PushPropertiesTo(animations_impl.get());
@@ -1120,13 +1122,13 @@ TEST_F(ElementAnimationsTest, ScrollOffsetRemovalClearsScrollDelta) {
   std::unique_ptr<Animation> animation(Animation::Create(
       std::move(curve), animation_id, 0, TargetProperty::SCROLL_OFFSET));
   animation->set_needs_synchronized_start_time(true);
-  animations->AddAnimation(std::move(animation));
+  AddAnimationToElementAnimations(animations.get(), std::move(animation));
   animations->PushPropertiesTo(animations_impl.get());
   animations_impl->ActivateAnimations();
   EXPECT_FALSE(animations->scroll_offset_animation_was_interrupted());
   EXPECT_FALSE(animations_impl->scroll_offset_animation_was_interrupted());
 
-  animations->RemoveAnimation(animation_id);
+  RemoveAnimationFromElementAnimations(animations.get(), animation_id);
   EXPECT_TRUE(animations->scroll_offset_animation_was_interrupted());
 
   animations->PushPropertiesTo(animations_impl.get());
@@ -1143,13 +1145,13 @@ TEST_F(ElementAnimationsTest, ScrollOffsetRemovalClearsScrollDelta) {
   animation = Animation::Create(std::move(curve), animation_id, 0,
                                 TargetProperty::SCROLL_OFFSET);
   animation->set_needs_synchronized_start_time(true);
-  animations->AddAnimation(std::move(animation));
+  AddAnimationToElementAnimations(animations.get(), std::move(animation));
   animations->PushPropertiesTo(animations_impl.get());
   animations_impl->ActivateAnimations();
   EXPECT_FALSE(animations->scroll_offset_animation_was_interrupted());
   EXPECT_FALSE(animations_impl->scroll_offset_animation_was_interrupted());
 
-  animations->RemoveAnimation(animation_id);
+  RemoveAnimationFromElementAnimations(animations.get(), animation_id);
   EXPECT_TRUE(animations->scroll_offset_animation_was_interrupted());
 
   animations->PushPropertiesTo(animations_impl.get());
@@ -1168,7 +1170,7 @@ TEST_F(ElementAnimationsTest, ScrollOffsetRemovalClearsScrollDelta) {
   EXPECT_FALSE(animations->scroll_offset_animation_was_interrupted());
   EXPECT_FALSE(animations_impl->scroll_offset_animation_was_interrupted());
 
-  animations->RemoveAnimation(animation_id);
+  RemoveAnimationFromElementAnimations(animations.get(), animation_id);
   EXPECT_FALSE(animations->scroll_offset_animation_was_interrupted());
 
   animations->PushPropertiesTo(animations_impl.get());
@@ -1185,7 +1187,7 @@ TEST_F(ElementAnimationsTest, ScrollOffsetRemovalClearsScrollDelta) {
   EXPECT_FALSE(animations->scroll_offset_animation_was_interrupted());
   EXPECT_FALSE(animations_impl->scroll_offset_animation_was_interrupted());
 
-  animations->RemoveAnimation(animation_id);
+  RemoveAnimationFromElementAnimations(animations.get(), animation_id);
   EXPECT_FALSE(animations->scroll_offset_animation_was_interrupted());
 
   animations->PushPropertiesTo(animations_impl.get());
@@ -1223,7 +1225,7 @@ TEST_F(ElementAnimationsTest,
   std::unique_ptr<Animation> to_add(
       Animation::Create(std::move(curve), 1, 0, TargetProperty::SCROLL_OFFSET));
   to_add->set_is_impl_only(true);
-  animations_impl->AddAnimation(std::move(to_add));
+  AddAnimationToElementAnimations(animations_impl.get(), std::move(to_add));
 
   EXPECT_FALSE(delegate.started());
   EXPECT_FALSE(delegate.finished());
@@ -1301,7 +1303,7 @@ TEST_F(ElementAnimationsTest,
 
   // We should pause at the first keyframe indefinitely waiting for that
   // animation to start.
-  animations->AddAnimation(std::move(to_add));
+  AddAnimationToElementAnimations(animations.get(), std::move(to_add));
   animations->Animate(kInitialTickTime);
   animations->UpdateState(true, events.get());
   EXPECT_TRUE(animations->HasActiveAnimation());
@@ -1388,7 +1390,7 @@ TEST_F(ElementAnimationsTest, Interrupt) {
       std::unique_ptr<AnimationCurve>(new FakeFloatTransition(1.0, 1.f, 0.5f)),
       2, TargetProperty::OPACITY));
   animations->AbortAnimations(TargetProperty::OPACITY);
-  animations->AddAnimation(std::move(to_add));
+  AddAnimationToElementAnimations(animations.get(), std::move(to_add));
 
   // Since the previous animation was aborted, the new animation should start
   // right in this call to animate.
@@ -1494,7 +1496,7 @@ TEST_F(ElementAnimationsTest, TrivialLooping) {
       std::unique_ptr<AnimationCurve>(new FakeFloatTransition(1.0, 0.f, 1.f)),
       1, TargetProperty::OPACITY));
   to_add->set_iterations(3);
-  animations->AddAnimation(std::move(to_add));
+  AddAnimationToElementAnimations(animations.get(), std::move(to_add));
 
   animations->Animate(kInitialTickTime);
   animations->UpdateState(true, events.get());
@@ -1540,7 +1542,7 @@ TEST_F(ElementAnimationsTest, InfiniteLooping) {
       std::unique_ptr<AnimationCurve>(new FakeFloatTransition(1.0, 0.f, 1.f)),
       1, TargetProperty::OPACITY));
   to_add->set_iterations(-1);
-  animations->AddAnimation(std::move(to_add));
+  AddAnimationToElementAnimations(animations.get(), std::move(to_add));
 
   animations->Animate(kInitialTickTime);
   animations->UpdateState(true, events.get());
@@ -1677,7 +1679,8 @@ TEST_F(ElementAnimationsTest, PushUpdatesWhenSynchronizedStartTimeNeeded) {
       std::unique_ptr<AnimationCurve>(new FakeFloatTransition(2.0, 0.f, 1.f)),
       0, TargetProperty::OPACITY));
   to_add->set_needs_synchronized_start_time(true);
-  animations->AddAnimation(std::move(to_add));
+  AddAnimationToElementAnimations(animations.get(), std::move(to_add));
+  // AddAnimationToElementAnimations(animations.get(), std::move(to_add));
 
   animations->Animate(kInitialTickTime);
   animations->UpdateState(true, events.get());
@@ -1687,6 +1690,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();
 
@@ -1709,7 +1713,7 @@ TEST_F(ElementAnimationsTest, SkipUpdateState) {
       std::unique_ptr<AnimationCurve>(new FakeTransformTransition(1)), 1,
       TargetProperty::TRANSFORM));
   first_animation->set_is_controlling_instance_for_test(true);
-  animations->AddAnimation(std::move(first_animation));
+  AddAnimationToElementAnimations(animations.get(), std::move(first_animation));
 
   animations->Animate(kInitialTickTime);
   animations->UpdateState(true, events.get());
@@ -1718,7 +1722,8 @@ TEST_F(ElementAnimationsTest, SkipUpdateState) {
       std::unique_ptr<AnimationCurve>(new FakeFloatTransition(1.0, 0.f, 1.f)),
       2, TargetProperty::OPACITY));
   second_animation->set_is_controlling_instance_for_test(true);
-  animations->AddAnimation(std::move(second_animation));
+  AddAnimationToElementAnimations(animations.get(),
+                                  std::move(second_animation));
 
   // Animate but don't UpdateState.
   animations->Animate(kInitialTickTime + TimeDelta::FromMilliseconds(1000));
@@ -1829,7 +1834,7 @@ TEST_F(ElementAnimationsTest, TransformAnimationBounds) {
 
   std::unique_ptr<Animation> animation(
       Animation::Create(std::move(curve1), 1, 1, TargetProperty::TRANSFORM));
-  animations_impl->AddAnimation(std::move(animation));
+  AddAnimationToElementAnimations(animations_impl.get(), std::move(animation));
 
   std::unique_ptr<KeyframedTransformAnimationCurve> curve2(
       KeyframedTransformAnimationCurve::Create());
@@ -1843,7 +1848,7 @@ TEST_F(ElementAnimationsTest, TransformAnimationBounds) {
 
   animation =
       Animation::Create(std::move(curve2), 2, 2, TargetProperty::TRANSFORM);
-  animations_impl->AddAnimation(std::move(animation));
+  AddAnimationToElementAnimations(animations_impl.get(), std::move(animation));
 
   gfx::BoxF box(1.f, 2.f, -1.f, 3.f, 4.f, 5.f);
   gfx::BoxF bounds;
@@ -1879,7 +1884,7 @@ TEST_F(ElementAnimationsTest, TransformAnimationBounds) {
       base::TimeDelta::FromSecondsD(1.0), operations3, nullptr));
   animation =
       Animation::Create(std::move(curve3), 3, 3, TargetProperty::TRANSFORM);
-  animations_impl->AddAnimation(std::move(animation));
+  AddAnimationToElementAnimations(animations_impl.get(), std::move(animation));
   EXPECT_FALSE(animations_impl->TransformAnimationBoundsForBox(box, &bounds));
 }
 
@@ -1947,18 +1952,20 @@ TEST_F(ElementAnimationsTest, MainThreadAbortedAnimationGetsDeleted) {
   animations_impl->ActivateAnimations();
   EXPECT_TRUE(animations_impl->GetAnimationById(animation_id));
 
-  animations->AbortAnimations(TargetProperty::OPACITY);
+  AbortAnimationsInElementAnimations(animations.get(), TargetProperty::OPACITY,
+                                     false);
   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 +1994,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 +2014,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());
 
@@ -2045,26 +2052,26 @@ TEST_F(ElementAnimationsTest, ImplThreadTakeoverAnimationGetsDeleted) {
       std::move(curve), animation_id, 0, TargetProperty::SCROLL_OFFSET));
   animation->set_start_time(TicksFromSecondsF(123));
   animation->set_is_impl_only(true);
-  animations_impl->AddAnimation(std::move(animation));
+  AddAnimationToElementAnimations(animations_impl.get(), std::move(animation));
 
   animations->PushPropertiesTo(animations_impl.get());
   animations_impl->ActivateAnimations();
   EXPECT_TRUE(animations_impl->GetAnimationById(animation_id));
 
   const bool needs_completion = true;
-  animations_impl->AbortAnimations(TargetProperty::SCROLL_OFFSET,
-                                   needs_completion);
+  AbortAnimationsInElementAnimations(
+      animations_impl.get(), TargetProperty::SCROLL_OFFSET, needs_completion);
   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);
@@ -2077,7 +2084,10 @@ TEST_F(ElementAnimationsTest, ImplThreadTakeoverAnimationGetsDeleted) {
 
   animations->NotifyAnimationTakeover(events->events_[0]);
   EXPECT_TRUE(delegate.takeover());
+  // TODO(ymalik): How does it work?
+  animations->SetNeedsPushProperties();

[loyso (OOO), 2016/08/22 07:48:27]

It notifies blink about takover and then suddenly expects the animation to be
deleted.
What code causes this deletion in real case?

[ymalik, 2016/08/22 14:40:01]

The purpose of the Takeover notification is to continue an impl-only scroll
offset animation on the MT. We need this for cases where a main thread scrolling
reason gets added while an impl-only scroll offset animation is in progress (see
crbug.com/581875).

On the impl-side, an animation is put into the ABORTED_BUT_NEEDS_COMPLETION
state
(https://cs.chromium.org/chromium/src/cc/animation/element_animations.cc?rcl=0...).
Then, it is marked for deletion during the next call to
ElementAnimation::UpdateState
(https://cs.chromium.org/chromium/src/cc/animation/element_animations.cc?rcl=0...).

On the mt-side, when we get a Takeover message, the old animation curve is
passed on to blink's ScrollAnimator, and we finish the animation on the MT.

 
+  EXPECT_TRUE(animations->needs_push_properties());
   animations->PushPropertiesTo(animations_impl.get());
   animations_impl->ActivateAnimations();
   EXPECT_FALSE(animations->GetAnimationById(animation_id));
@@ -2102,13 +2112,15 @@ TEST_F(ElementAnimationsTest, FinishedEventsForGroup) {
       std::unique_ptr<AnimationCurve>(new FakeTransformTransition(2.0)), 1,
       group_id, TargetProperty::TRANSFORM));
   first_animation->set_is_controlling_instance_for_test(true);
-  animations_impl->AddAnimation(std::move(first_animation));
+  AddAnimationToElementAnimations(animations_impl.get(),
+                                  std::move(first_animation));
 
   std::unique_ptr<Animation> second_animation(Animation::Create(
       std::unique_ptr<AnimationCurve>(new FakeFloatTransition(1.0, 0.f, 1.f)),
       2, group_id, TargetProperty::OPACITY));
   second_animation->set_is_controlling_instance_for_test(true);
-  animations_impl->AddAnimation(std::move(second_animation));
+  AddAnimationToElementAnimations(animations_impl.get(),
+                                  std::move(second_animation));
 
   animations_impl->Animate(kInitialTickTime);
   animations_impl->UpdateState(true, events.get());
@@ -2158,13 +2170,15 @@ TEST_F(ElementAnimationsTest, FinishedAndAbortedEventsForGroup) {
       std::unique_ptr<AnimationCurve>(new FakeTransformTransition(1.0)), 1,
       TargetProperty::TRANSFORM));
   first_animation->set_is_controlling_instance_for_test(true);
-  animations_impl->AddAnimation(std::move(first_animation));
+  AddAnimationToElementAnimations(animations_impl.get(),
+                                  std::move(first_animation));
 
   std::unique_ptr<Animation> second_animation(CreateAnimation(
       std::unique_ptr<AnimationCurve>(new FakeFloatTransition(1.0, 0.f, 1.f)),
       1, TargetProperty::OPACITY));
   second_animation->set_is_controlling_instance_for_test(true);
-  animations_impl->AddAnimation(std::move(second_animation));
+  AddAnimationToElementAnimations(animations_impl.get(),
+                                  std::move(second_animation));
 
   animations_impl->Animate(kInitialTickTime);
   animations_impl->UpdateState(true, events.get());
@@ -2218,7 +2232,7 @@ TEST_F(ElementAnimationsTest, HasAnimationThatAffectsScale) {
 
   std::unique_ptr<Animation> animation(
       Animation::Create(std::move(curve1), 2, 2, TargetProperty::TRANSFORM));
-  animations_impl->AddAnimation(std::move(animation));
+  AddAnimationToElementAnimations(animations_impl.get(), std::move(animation));
 
   // Translations don't affect scale.
   EXPECT_FALSE(animations_impl->HasAnimationThatAffectsScale());
@@ -2235,7 +2249,7 @@ TEST_F(ElementAnimationsTest, HasAnimationThatAffectsScale) {
 
   animation =
       Animation::Create(std::move(curve2), 3, 3, TargetProperty::TRANSFORM);
-  animations_impl->AddAnimation(std::move(animation));
+  AddAnimationToElementAnimations(animations_impl.get(), std::move(animation));
 
   EXPECT_TRUE(animations_impl->HasAnimationThatAffectsScale());
 
@@ -2281,7 +2295,7 @@ TEST_F(ElementAnimationsTest, HasOnlyTranslationTransforms) {
 
   std::unique_ptr<Animation> animation(
       Animation::Create(std::move(curve1), 2, 2, TargetProperty::TRANSFORM));
-  animations_impl->AddAnimation(std::move(animation));
+  AddAnimationToElementAnimations(animations_impl.get(), std::move(animation));
 
   // The only transform animation we've added is a translation.
   EXPECT_TRUE(
@@ -2302,7 +2316,7 @@ TEST_F(ElementAnimationsTest, HasOnlyTranslationTransforms) {
   animation =
       Animation::Create(std::move(curve2), 3, 3, TargetProperty::TRANSFORM);
   animation->set_affects_active_elements(false);
-  animations_impl->AddAnimation(std::move(animation));
+  AddAnimationToElementAnimations(animations_impl.get(), std::move(animation));
 
   // A scale animation is not a translation.
   EXPECT_FALSE(
@@ -2353,7 +2367,7 @@ TEST_F(ElementAnimationsTest, AnimationStartScale) {
   std::unique_ptr<Animation> animation(
       Animation::Create(std::move(curve1), 1, 1, TargetProperty::TRANSFORM));
   animation->set_affects_active_elements(false);
-  animations_impl->AddAnimation(std::move(animation));
+  AddAnimationToElementAnimations(animations_impl.get(), std::move(animation));
 
   float start_scale = 0.f;
   EXPECT_TRUE(animations_impl->AnimationStartScale(ElementListType::PENDING,
@@ -2381,14 +2395,14 @@ TEST_F(ElementAnimationsTest, AnimationStartScale) {
   curve2->AddKeyframe(TransformKeyframe::Create(
       base::TimeDelta::FromSecondsD(1.0), operations3, nullptr));
 
-  animations_impl->RemoveAnimation(1);
+  RemoveAnimationFromElementAnimations(animations_impl.get(), 1);
   animation =
       Animation::Create(std::move(curve2), 2, 2, TargetProperty::TRANSFORM);
 
   // Reverse Direction
   animation->set_direction(Animation::Direction::REVERSE);
   animation->set_affects_active_elements(false);
-  animations_impl->AddAnimation(std::move(animation));
+  AddAnimationToElementAnimations(animations_impl.get(), std::move(animation));
 
   std::unique_ptr<KeyframedTransformAnimationCurve> curve3(
       KeyframedTransformAnimationCurve::Create());
@@ -2404,7 +2418,7 @@ TEST_F(ElementAnimationsTest, AnimationStartScale) {
   animation =
       Animation::Create(std::move(curve3), 3, 3, TargetProperty::TRANSFORM);
   animation->set_affects_active_elements(false);
-  animations_impl->AddAnimation(std::move(animation));
+  AddAnimationToElementAnimations(animations_impl.get(), std::move(animation));
 
   EXPECT_TRUE(animations_impl->AnimationStartScale(ElementListType::PENDING,
                                                    &start_scale));
@@ -2462,7 +2476,7 @@ TEST_F(ElementAnimationsTest, MaximumTargetScale) {
   std::unique_ptr<Animation> animation(
       Animation::Create(std::move(curve1), 1, 1, TargetProperty::TRANSFORM));
   animation->set_affects_active_elements(false);
-  animations_impl->AddAnimation(std::move(animation));
+  AddAnimationToElementAnimations(animations_impl.get(), std::move(animation));
 
   EXPECT_TRUE(animations_impl->MaximumTargetScale(ElementListType::PENDING,
                                                   &max_scale));
@@ -2492,7 +2506,7 @@ TEST_F(ElementAnimationsTest, MaximumTargetScale) {
   animation =
       Animation::Create(std::move(curve2), 2, 2, TargetProperty::TRANSFORM);
   animation->set_affects_active_elements(false);
-  animations_impl->AddAnimation(std::move(animation));
+  AddAnimationToElementAnimations(animations_impl.get(), std::move(animation));
 
   EXPECT_TRUE(animations_impl->MaximumTargetScale(ElementListType::PENDING,
                                                   &max_scale));
@@ -2522,7 +2536,7 @@ TEST_F(ElementAnimationsTest, MaximumTargetScale) {
   animation =
       Animation::Create(std::move(curve3), 3, 3, TargetProperty::TRANSFORM);
   animation->set_affects_active_elements(false);
-  animations_impl->AddAnimation(std::move(animation));
+  AddAnimationToElementAnimations(animations_impl.get(), std::move(animation));
 
   EXPECT_FALSE(animations_impl->MaximumTargetScale(ElementListType::PENDING,
                                                    &max_scale));
@@ -2572,7 +2586,8 @@ TEST_F(ElementAnimationsTest, MaximumTargetScaleWithDirection) {
   std::unique_ptr<Animation> animation_owned(
       Animation::Create(std::move(curve1), 1, 1, TargetProperty::TRANSFORM));
   Animation* animation = animation_owned.get();
-  animations_impl->AddAnimation(std::move(animation_owned));
+  AddAnimationToElementAnimations(animations_impl.get(),
+                                  std::move(animation_owned));
 
   float max_scale = 0.f;
 
@@ -2887,7 +2902,7 @@ TEST_F(ElementAnimationsTest, ObserverNotifiedWhenTransformAnimationChanges) {
   animations->NotifyAnimationStarted(events->events_[0]);
   events->events_.clear();
 
-  animations->RemoveAnimation(animation_id);
+  RemoveAnimationFromElementAnimations(animations.get(), animation_id);
   EXPECT_FALSE(client_.GetHasPotentialTransformAnimation(
       element_id_, ElementListType::ACTIVE));
   EXPECT_FALSE(client_.GetTransformIsCurrentlyAnimating(
@@ -3105,7 +3120,7 @@ TEST_F(ElementAnimationsTest, ObserverNotifiedWhenOpacityAnimationChanges) {
   animations->NotifyAnimationStarted(events->events_[0]);
   events->events_.clear();
 
-  animations->RemoveAnimation(animation_id);
+  RemoveAnimationFromElementAnimations(animations.get(), animation_id);
   EXPECT_FALSE(client_.GetHasPotentialOpacityAnimation(
       element_id_, ElementListType::ACTIVE));
   EXPECT_FALSE(client_.GetOpacityIsCurrentlyAnimating(element_id_,
@@ -3322,7 +3337,7 @@ TEST_F(ElementAnimationsTest, ObserverNotifiedWhenFilterAnimationChanges) {
   animations->NotifyAnimationStarted(events->events_[0]);
   events->events_.clear();
 
-  animations->RemoveAnimation(animation_id);
+  RemoveAnimationFromElementAnimations(animations.get(), animation_id);
   EXPECT_FALSE(client_.GetHasPotentialFilterAnimation(element_id_,
                                                       ElementListType::ACTIVE));
   EXPECT_FALSE(client_.GetFilterIsCurrentlyAnimating(element_id_,
@@ -3482,7 +3497,8 @@ TEST_F(ElementAnimationsTest, PushedDeletedAnimationWaitsForActivation) {
                   ->affects_active_elements());
 
   // Delete the animation on the main-thread animations.
-  animations->RemoveAnimation(
+  RemoveAnimationFromElementAnimations(
+      animations.get(),
       animations->GetAnimation(TargetProperty::OPACITY)->id());
   animations->PushPropertiesTo(animations_impl.get());
 
@@ -3529,7 +3545,8 @@ TEST_F(ElementAnimationsTest, StartAnimationsAffectingDifferentObservers) {
 
   // Remove the first animation from the main-thread animations, and add a
   // new animation affecting the same property.
-  animations->RemoveAnimation(
+  RemoveAnimationFromElementAnimations(
+      animations.get(),
       animations->GetAnimation(TargetProperty::OPACITY)->id());
   const int second_animation_id = AddOpacityTransitionToElementAnimations(
       animations.get(), 1, 1.f, 0.5f, true);
@@ -3600,7 +3617,7 @@ TEST_F(ElementAnimationsTest, TestIsCurrentlyAnimatingProperty) {
       1, TargetProperty::OPACITY));
   animation->set_affects_active_elements(false);
 
-  animations->AddAnimation(std::move(animation));
+  AddAnimationToElementAnimations(animations.get(), std::move(animation));
   animations->Animate(kInitialTickTime);
   EXPECT_TRUE(animations->IsCurrentlyAnimatingProperty(
       TargetProperty::OPACITY, ElementListType::PENDING));
@@ -3674,7 +3691,7 @@ TEST_F(ElementAnimationsTest, TestIsAnimatingPropertyTimeOffsetFillMode) {
   animation->set_time_offset(TimeDelta::FromMilliseconds(-2000));
   animation->set_affects_active_elements(false);
 
-  animations->AddAnimation(std::move(animation));
+  AddAnimationToElementAnimations(animations.get(), std::move(animation));
 
   animations->Animate(kInitialTickTime);