Remove unnecessary cc::UpdateTickingType from ElementAnimations.

cc::UpdateTickingType::FORCE is only used in two circumstances:

1. From ElementAnimations::InitAffectedElementTypes.
2. From ElementAnimations::ElementRegistered.

Since InitAffectedElementTypes is only called when constructing a new
ElementAnimations, the call can be moved to the constructor such that we
can guarantee the keyframe_effects_list_ is empty and thus remove the
call to UpdateKeyframeEffectsTickingState(FORCE).

When called from ElementRegistered, we are about to set
has_element_in_active_list or has_element_in_pending_list which will
result in the keyframe effect having an element in a list giving the
same behavior that force was being used to provide. The other possible
side effects of force seem unintentional.

Change-Id: I6128793e953ad2e59fbc7657033995292c204f48
Reviewed-on: https://chromium-review.googlesource.com/c/1469070
Commit-Queue: Robert Flack <flackr@chromium.org>
Reviewed-by: Majid Valipour <majidvp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#632337}

cc/animation/animation.cc

@@ -257,7 +257,7 @@ void Animation::UpdateState(bool start_ready_animations,
                             AnimationEvents* events) {
   for (auto& keyframe_effect : keyframe_effects_) {
     keyframe_effect->UpdateState(start_ready_animations, events);
-    keyframe_effect->UpdateTickingState(UpdateTickingType::NORMAL);
+    keyframe_effect->UpdateTickingState();
   }
 }
 
@@ -335,7 +335,7 @@ void Animation::SetNeedsPushProperties() {
 void Animation::ActivateKeyframeEffects() {
   for (auto& keyframe_effect : keyframe_effects_) {
     keyframe_effect->ActivateKeyframeEffects();
-    keyframe_effect->UpdateTickingState(UpdateTickingType::NORMAL);
+    keyframe_effect->UpdateTickingState();
   }
 }
 

cc/animation/animation_host.cc

@@ -138,7 +138,6 @@ void AnimationHost::RegisterKeyframeEffectForElement(
     element_animations = ElementAnimations::Create(this, element_id);
     element_to_animations_map_[element_animations->element_id()] =
         element_animations;
-    element_animations->InitAffectedElementTypes();
   }
 
   DCHECK(element_animations->AnimationHostIs(this));

cc/animation/element_animations.cc

@@ -51,7 +51,9 @@ ElementAnimations::ElementAnimations(AnimationHost* host, ElementId element_id)
       element_id_(element_id),
       has_element_in_active_list_(false),
       has_element_in_pending_list_(false),
-      needs_push_properties_(false) {}
+      needs_push_properties_(false) {
+  InitAffectedElementTypes();
+}
 
 ElementAnimations::~ElementAnimations() = default;
 
@@ -59,8 +61,6 @@ void ElementAnimations::InitAffectedElementTypes() {
   DCHECK(element_id_);
   DCHECK(animation_host_);
 
-  UpdateKeyframeEffectsTickingState(UpdateTickingType::FORCE);
-
   DCHECK(animation_host_->mutator_host_client());
   if (animation_host_->mutator_host_client()->IsElementInList(
           element_id_, ElementListType::ACTIVE)) {
@@ -112,13 +112,15 @@ void ElementAnimations::ElementRegistered(ElementId element_id,
                                           ElementListType list_type) {
   DCHECK_EQ(element_id_, element_id);
 
-  if (!has_element_in_any_list())
-    UpdateKeyframeEffectsTickingState(UpdateTickingType::FORCE);
+  bool had_element_in_any_list = has_element_in_any_list();
 
   if (list_type == ElementListType::ACTIVE)
     set_has_element_in_active_list(true);
   else
     set_has_element_in_pending_list(true);
+
+  if (!had_element_in_any_list)
+    UpdateKeyframeEffectsTickingState();
 }
 
 void ElementAnimations::ElementUnregistered(ElementId element_id,
@@ -162,10 +164,9 @@ void ElementAnimations::PushPropertiesTo(
   element_animations_impl->UpdateClientAnimationState();
 }
 
-void ElementAnimations::UpdateKeyframeEffectsTickingState(
-    UpdateTickingType update_ticking_type) const {
+void ElementAnimations::UpdateKeyframeEffectsTickingState() const {
   for (auto& keyframe_effect : keyframe_effects_list_)
-    keyframe_effect.UpdateTickingState(update_ticking_type);
+    keyframe_effect.UpdateTickingState();
 }
 
 void ElementAnimations::RemoveKeyframeEffectsFromTicking() const {

cc/animation/element_animations.h

@@ -28,8 +28,6 @@ class TransformOperations;
 enum class ElementListType;
 struct AnimationEvent;
 
-enum class UpdateTickingType { NORMAL, FORCE };
-
 // An ElementAnimations owns a list of all KeyframeEffects attached to a single
 // target (represented by an ElementId).
 //
@@ -50,7 +48,6 @@ class CC_ANIMATION_EXPORT ElementAnimations
 
   ElementId element_id() const { return element_id_; }
 
-  void InitAffectedElementTypes();
   void ClearAffectedElementTypes(const PropertyToElementIdMap& element_id_map);
 
   void ElementRegistered(ElementId element_id, ElementListType list_type);
@@ -168,6 +165,8 @@ class CC_ANIMATION_EXPORT ElementAnimations
   ElementAnimations(AnimationHost* host, ElementId element_id);
   ~ElementAnimations() override;
 
+  void InitAffectedElementTypes();
+
   void OnFilterAnimated(ElementListType list_type,
                         const FilterOperations& filters,
                         KeyframeModel* keyframe_model);
@@ -183,8 +182,7 @@ class CC_ANIMATION_EXPORT ElementAnimations
 
   static TargetProperties GetPropertiesMaskForAnimationState();
 
-  void UpdateKeyframeEffectsTickingState(
-      UpdateTickingType update_ticking_type) const;
+  void UpdateKeyframeEffectsTickingState() const;
   void RemoveKeyframeEffectsFromTicking() const;
 
   bool KeyframeModelAffectsActiveElements(KeyframeModel* keyframe_model) const;

cc/animation/keyframe_effect.cc

@@ -203,8 +203,7 @@ void KeyframeEffect::UpdateState(bool start_ready_keyframe_models,
   }
 }
 
-void KeyframeEffect::UpdateTickingState(UpdateTickingType type) {
-  bool force = type == UpdateTickingType::FORCE;
+void KeyframeEffect::UpdateTickingState() {
   if (animation_->has_animation_host()) {
     bool was_ticking = is_ticking_;
     is_ticking_ = HasNonDeletedKeyframeModel();
@@ -212,9 +211,9 @@ void KeyframeEffect::UpdateTickingState(UpdateTickingType type) {
     bool has_element_in_any_list =
         element_animations_->has_element_in_any_list();
 
-    if (is_ticking_ && ((!was_ticking && has_element_in_any_list) || force)) {
+    if (is_ticking_ && !was_ticking && has_element_in_any_list) {
       animation_->AddToTicking();
-    } else if (!is_ticking_ && (was_ticking || force)) {
+    } else if (!is_ticking_ && was_ticking) {
       RemoveFromTicking();
     }
   }
@@ -296,7 +295,7 @@ void KeyframeEffect::RemoveKeyframeModel(int keyframe_model_id) {
   keyframe_models_.erase(keyframe_models_to_remove, keyframe_models_.end());
 
   if (has_bound_element_animations()) {
-    UpdateTickingState(UpdateTickingType::NORMAL);
+    UpdateTickingState();
     if (keyframe_model_removed)
       element_animations_->UpdateClientAnimationState();
     animation_->SetNeedsCommit();
@@ -374,7 +373,7 @@ void KeyframeEffect::KeyframeModelAdded() {
   animation_->SetNeedsCommit();
   needs_to_start_keyframe_models_ = true;
 
-  UpdateTickingState(UpdateTickingType::NORMAL);
+  UpdateTickingState();
   element_animations_->UpdateClientAnimationState();
 }
 
@@ -810,7 +809,7 @@ void KeyframeEffect::PushPropertiesTo(KeyframeEffect* keyframe_effect_impl) {
       scroll_offset_animation_was_interrupted_;
   scroll_offset_animation_was_interrupted_ = false;
 
-  keyframe_effect_impl->UpdateTickingState(UpdateTickingType::NORMAL);
+  keyframe_effect_impl->UpdateTickingState();
 }
 
 void KeyframeEffect::SetAnimation(Animation* animation) {

cc/animation/keyframe_effect.h

@@ -88,7 +88,7 @@ class CC_ANIMATION_EXPORT KeyframeEffect {
   bool is_ticking() const { return is_ticking_; }
 
   void UpdateState(bool start_ready_keyframe_models, AnimationEvents* events);
-  void UpdateTickingState(UpdateTickingType type);
+  void UpdateTickingState();
 
   void Pause(base::TimeDelta pause_offset);