Split active animation interpolation storage between standard and custom properties

This change refactors the storage of active animation interpolations
during a style resolve update by whether it targets a custom property
or a standard property. This makes it consistent with active transition
interpolations.

This change is necessary to process custom property animations separately
from standard properties in future patches.

This patch introduces no changes in behaviour.

BUG=671904

Review-Url: https://codereview.chromium.org/2831273002
Cr-Commit-Position: refs/heads/master@{#476179}

third_party/WebKit/Source/core/animation/css/CSSAnimationUpdate.h

@@ -104,8 +104,10 @@ class CSSAnimationUpdate final {
     new_animations_ = update.NewAnimations();
     animations_with_updates_ = update.AnimationsWithUpdates();
     new_transitions_ = update.NewTransitions();
-    active_interpolations_for_animations_ =
-        update.ActiveInterpolationsForAnimations();
+    active_interpolations_for_custom_animations_ =
+        update.ActiveInterpolationsForCustomAnimations();
+    active_interpolations_for_standard_animations_ =
+        update.ActiveInterpolationsForStandardAnimations();
     active_interpolations_for_custom_transitions_ =
         update.ActiveInterpolationsForCustomTransitions();
     active_interpolations_for_standard_transitions_ =
@@ -122,7 +124,8 @@ class CSSAnimationUpdate final {
     new_animations_.clear();
     animations_with_updates_.clear();
     new_transitions_.clear();
-    active_interpolations_for_animations_.clear();
+    active_interpolations_for_custom_animations_.clear();
+    active_interpolations_for_standard_animations_.clear();
     active_interpolations_for_custom_transitions_.clear();
     active_interpolations_for_standard_transitions_.clear();
     cancelled_animation_indices_.clear();
@@ -236,9 +239,13 @@ class CSSAnimationUpdate final {
     return finished_transitions_;
   }
 
-  void AdoptActiveInterpolationsForAnimations(
+  void AdoptActiveInterpolationsForCustomAnimations(
       ActiveInterpolationsMap& new_map) {
-    new_map.swap(active_interpolations_for_animations_);
+    new_map.swap(active_interpolations_for_custom_animations_);
+  }
+  void AdoptActiveInterpolationsForStandardAnimations(
+      ActiveInterpolationsMap& new_map) {
+    new_map.swap(active_interpolations_for_standard_animations_);
   }
   void AdoptActiveInterpolationsForCustomTransitions(
       ActiveInterpolationsMap& new_map) {
@@ -248,8 +255,19 @@ class CSSAnimationUpdate final {
       ActiveInterpolationsMap& new_map) {
     new_map.swap(active_interpolations_for_standard_transitions_);
   }
-  const ActiveInterpolationsMap& ActiveInterpolationsForAnimations() const {
-    return active_interpolations_for_animations_;
+  const ActiveInterpolationsMap& ActiveInterpolationsForCustomAnimations()
+      const {
+    return active_interpolations_for_custom_animations_;
+  }
+  ActiveInterpolationsMap& ActiveInterpolationsForCustomAnimations() {
+    return active_interpolations_for_custom_animations_;
+  }
+  const ActiveInterpolationsMap& ActiveInterpolationsForStandardAnimations()
+      const {
+    return active_interpolations_for_standard_animations_;
+  }
+  ActiveInterpolationsMap& ActiveInterpolationsForStandardAnimations() {
+    return active_interpolations_for_standard_animations_;
   }
   const ActiveInterpolationsMap& ActiveInterpolationsForCustomTransitions()
       const {
@@ -259,9 +277,6 @@ class CSSAnimationUpdate final {
       const {
     return active_interpolations_for_standard_transitions_;
   }
-  ActiveInterpolationsMap& ActiveInterpolationsForAnimations() {
-    return active_interpolations_for_animations_;
-  }
 
   bool IsEmpty() const {
     return new_animations_.IsEmpty() &&
@@ -271,7 +286,8 @@ class CSSAnimationUpdate final {
            animations_with_updates_.IsEmpty() && new_transitions_.IsEmpty() &&
            cancelled_transitions_.IsEmpty() &&
            finished_transitions_.IsEmpty() &&
-           active_interpolations_for_animations_.IsEmpty() &&
+           active_interpolations_for_custom_animations_.IsEmpty() &&
+           active_interpolations_for_standard_animations_.IsEmpty() &&
            active_interpolations_for_custom_transitions_.IsEmpty() &&
            active_interpolations_for_standard_transitions_.IsEmpty() &&
            updated_compositor_keyframes_.IsEmpty();
@@ -301,7 +317,8 @@ class CSSAnimationUpdate final {
   HashSet<PropertyHandle> cancelled_transitions_;
   HashSet<PropertyHandle> finished_transitions_;
 
-  ActiveInterpolationsMap active_interpolations_for_animations_;
+  ActiveInterpolationsMap active_interpolations_for_custom_animations_;
+  ActiveInterpolationsMap active_interpolations_for_standard_animations_;
   ActiveInterpolationsMap active_interpolations_for_custom_transitions_;
   ActiveInterpolationsMap active_interpolations_for_standard_transitions_;
 

third_party/WebKit/Source/core/animation/css/CSSAnimations.cpp

@@ -433,12 +433,15 @@ void CSSAnimations::SnapshotCompositorKeyframes(
 }
 
 void CSSAnimations::MaybeApplyPendingUpdate(Element* element) {
-  previous_active_interpolations_for_animations_.clear();
+  previous_active_interpolations_for_custom_animations_.clear();
+  previous_active_interpolations_for_standard_animations_.clear();
   if (pending_update_.IsEmpty())
     return;
 
-  previous_active_interpolations_for_animations_.swap(
-      pending_update_.ActiveInterpolationsForAnimations());
+  previous_active_interpolations_for_custom_animations_.swap(
+      pending_update_.ActiveInterpolationsForCustomAnimations());
+  previous_active_interpolations_for_standard_animations_.swap(
+      pending_update_.ActiveInterpolationsForStandardAnimations());
 
   // FIXME: cancelling, pausing, unpausing animations all query
   // compositingState, which is not necessarily up to date here
@@ -636,12 +639,23 @@ void CSSAnimations::CalculateTransitionUpdateForProperty(
   // FIXME: We should transition if an !important property changes even when an
   // animation is running, but this is a bit hard to do with the current
   // applyMatchedProperties system.
-  if (state.update.ActiveInterpolationsForAnimations().Contains(property) ||
-      (state.animating_element->GetElementAnimations() &&
-       state.animating_element->GetElementAnimations()
-           ->CssAnimations()
-           .previous_active_interpolations_for_animations_.Contains(
-               property))) {
+  if (property.IsCSSCustomProperty()) {
+    if (state.update.ActiveInterpolationsForCustomAnimations().Contains(
+            property) ||
+        (state.animating_element->GetElementAnimations() &&
+         state.animating_element->GetElementAnimations()
+             ->CssAnimations()
+             .previous_active_interpolations_for_custom_animations_.Contains(
+                 property))) {
+      return;
+    }
+  } else if (state.update.ActiveInterpolationsForStandardAnimations().Contains(
+                 property) ||
+             (state.animating_element->GetElementAnimations() &&
+              state.animating_element->GetElementAnimations()
+                  ->CssAnimations()
+                  .previous_active_interpolations_for_standard_animations_
+                  .Contains(property))) {
     return;
   }
 
@@ -935,6 +949,10 @@ void CSSAnimations::Cancel() {
   ClearPendingUpdate();
 }
 
+static bool IsCustomPropertyHandle(const PropertyHandle& property) {
+  return property.IsCSSCustomProperty();
+}
+
 // TODO(alancutter): CSS properties and presentation attributes may have
 // identical effects. By grouping them in the same set we introduce a bug where
 // arbitrary hash iteration will determine the order the apply in and thus which
@@ -942,9 +960,28 @@ void CSSAnimations::Cancel() {
 // the case of effect collisions.
 // Example: Both 'color' and 'svg-color' set the color on ComputedStyle but are
 // considered distinct properties in the ActiveInterpolationsMap.
-static bool IsStylePropertyHandle(const PropertyHandle& property_handle) {
-  return property_handle.IsCSSProperty() ||
-         property_handle.IsPresentationAttribute();
+static bool IsStandardPropertyHandle(const PropertyHandle& property) {
+  return (property.IsCSSProperty() && !property.IsCSSCustomProperty()) ||
+         property.IsPresentationAttribute();
+}
+
+static void AdoptActiveAnimationInterpolations(
+    EffectStack* effect_stack,
+    CSSAnimationUpdate& update,
+    const HeapVector<Member<const InertEffect>>* new_animations,
+    const HeapHashSet<Member<const Animation>>* suppressed_animations) {
+  ActiveInterpolationsMap custom_interpolations(
+      EffectStack::ActiveInterpolations(
+          effect_stack, new_animations, suppressed_animations,
+          KeyframeEffectReadOnly::kDefaultPriority, IsCustomPropertyHandle));
+  update.AdoptActiveInterpolationsForCustomAnimations(custom_interpolations);
+
+  ActiveInterpolationsMap standard_interpolations(
+      EffectStack::ActiveInterpolations(
+          effect_stack, new_animations, suppressed_animations,
+          KeyframeEffectReadOnly::kDefaultPriority, IsStandardPropertyHandle));
+  update.AdoptActiveInterpolationsForStandardAnimations(
+      standard_interpolations);
 }
 
 void CSSAnimations::CalculateAnimationActiveInterpolations(
@@ -957,12 +994,7 @@ void CSSAnimations::CalculateAnimationActiveInterpolations(
 
   if (update.NewAnimations().IsEmpty() &&
       update.SuppressedAnimations().IsEmpty()) {
-    ActiveInterpolationsMap active_interpolations_for_animations(
-        EffectStack::ActiveInterpolations(
-            effect_stack, nullptr, nullptr,
-            KeyframeEffectReadOnly::kDefaultPriority, IsStylePropertyHandle));
-    update.AdoptActiveInterpolationsForAnimations(
-        active_interpolations_for_animations);
+    AdoptActiveAnimationInterpolations(effect_stack, update, nullptr, nullptr);
     return;
   }
 
@@ -974,29 +1006,17 @@ void CSSAnimations::CalculateAnimationActiveInterpolations(
   for (const auto& updated_animation : update.AnimationsWithUpdates())
     new_effects.push_back(updated_animation.effect);
 
-  ActiveInterpolationsMap active_interpolations_for_animations(
-      EffectStack::ActiveInterpolations(
-          effect_stack, &new_effects, &update.SuppressedAnimations(),
-          KeyframeEffectReadOnly::kDefaultPriority, IsStylePropertyHandle));
-  update.AdoptActiveInterpolationsForAnimations(
-      active_interpolations_for_animations);
-}
-
-static bool IsCustomStylePropertyHandle(const PropertyHandle& property) {
-  return property.IsCSSCustomProperty();
-}
-
-static bool IsStandardStylePropertyHandle(const PropertyHandle& property) {
-  return IsStylePropertyHandle(property) && !property.IsCSSCustomProperty();
+  AdoptActiveAnimationInterpolations(effect_stack, update, &new_effects,
+                                     &update.SuppressedAnimations());
 }
 
-static EffectStack::PropertyHandleFilter StylePropertyFilter(
+static EffectStack::PropertyHandleFilter PropertyFilter(
     CSSAnimations::PropertyPass property_pass) {
   if (property_pass == CSSAnimations::PropertyPass::kCustom) {
-    return IsCustomStylePropertyHandle;
+    return IsCustomPropertyHandle;
   }
   DCHECK_EQ(property_pass, CSSAnimations::PropertyPass::kStandard);
-  return IsStandardStylePropertyHandle;
+  return IsStandardPropertyHandle;
 }
 
 void CSSAnimations::CalculateTransitionActiveInterpolations(
@@ -1014,7 +1034,7 @@ void CSSAnimations::CalculateTransitionActiveInterpolations(
     active_interpolations_for_transitions = EffectStack::ActiveInterpolations(
         effect_stack, nullptr, nullptr,
         KeyframeEffectReadOnly::kTransitionPriority,
-        StylePropertyFilter(property_pass));
+        PropertyFilter(property_pass));
   } else {
     HeapVector<Member<const InertEffect>> new_transitions;
     for (const auto& entry : update.NewTransitions())
@@ -1035,14 +1055,18 @@ void CSSAnimations::CalculateTransitionActiveInterpolations(
     active_interpolations_for_transitions = EffectStack::ActiveInterpolations(
         effect_stack, &new_transitions, &cancelled_animations,
         KeyframeEffectReadOnly::kTransitionPriority,
-        StylePropertyFilter(property_pass));
+        PropertyFilter(property_pass));
   }
 
+  const ActiveInterpolationsMap& animations =
+      property_pass == PropertyPass::kCustom
+          ? update.ActiveInterpolationsForCustomAnimations()
+          : update.ActiveInterpolationsForStandardAnimations();
   // Properties being animated by animations don't get values from transitions
   // applied.
-  if (!update.ActiveInterpolationsForAnimations().IsEmpty() &&
+  if (!animations.IsEmpty() &&
       !active_interpolations_for_transitions.IsEmpty()) {
-    for (const auto& entry : update.ActiveInterpolationsForAnimations())
+    for (const auto& entry : animations)
       active_interpolations_for_transitions.erase(entry.key);
   }
 
@@ -1218,11 +1242,6 @@ bool CSSAnimations::IsAffectedByKeyframesFromScope(
   return ToShadowRoot(tree_scope.RootNode()).host() == element;
 }
 
-bool CSSAnimations::IsCustomPropertyHandle(const PropertyHandle& property) {
-  return property.IsCSSProperty() &&
-         property.CssProperty() == CSSPropertyVariable;
-}
-
 bool CSSAnimations::IsAnimatingCustomProperties(
     const ElementAnimations* element_animations) {
   return element_animations &&

third_party/WebKit/Source/core/animation/css/CSSAnimations.h

@@ -64,7 +64,6 @@ class CSSAnimations final {
   static bool IsAnimationAffectingProperty(CSSPropertyID);
   static bool IsAffectedByKeyframesFromScope(const Element&, const TreeScope&);
   static bool IsAnimatingCustomProperties(const ElementAnimations*);
-  static bool IsCustomPropertyHandle(const PropertyHandle&);
   static void CalculateAnimationUpdate(CSSAnimationUpdate&,
                                        const Element* animating_element,
                                        Element&,
@@ -157,7 +156,9 @@ class CSSAnimations final {
 
   CSSAnimationUpdate pending_update_;
 
-  ActiveInterpolationsMap previous_active_interpolations_for_animations_;
+  ActiveInterpolationsMap previous_active_interpolations_for_custom_animations_;
+  ActiveInterpolationsMap
+      previous_active_interpolations_for_standard_animations_;
 
   struct TransitionUpdateState {
     STACK_ALLOCATED();

third_party/WebKit/Source/core/css/resolver/StyleResolver.cpp

@@ -1170,21 +1170,22 @@ bool StyleResolver::ApplyAnimatedStandardProperties(
     state.SetApplyPropertyToVisitedLinkStyle(true);
   }
 
-  const ActiveInterpolationsMap& active_interpolations_map_for_animations =
-      state.AnimationUpdate().ActiveInterpolationsForAnimations();
+  const ActiveInterpolationsMap&
+      active_interpolations_map_for_standard_animations =
+          state.AnimationUpdate().ActiveInterpolationsForStandardAnimations();
   const ActiveInterpolationsMap&
       active_interpolations_map_for_standard_transitions =
           state.AnimationUpdate().ActiveInterpolationsForStandardTransitions();
   // TODO(crbug.com/644148): Apply animations on custom properties.
   ApplyAnimatedProperties<kHighPropertyPriority>(
-      state, active_interpolations_map_for_animations);
+      state, active_interpolations_map_for_standard_animations);
   ApplyAnimatedProperties<kHighPropertyPriority>(
       state, active_interpolations_map_for_standard_transitions);
 
   UpdateFont(state);
 
   ApplyAnimatedProperties<kLowPropertyPriority>(
-      state, active_interpolations_map_for_animations);
+      state, active_interpolations_map_for_standard_animations);
   ApplyAnimatedProperties<kLowPropertyPriority>(
       state, active_interpolations_map_for_standard_transitions);
 
@@ -1229,6 +1230,7 @@ void StyleResolver::ApplyAnimatedProperties(
     CSSPropertyID property = entry.key.IsCSSProperty()
                                  ? entry.key.CssProperty()
                                  : entry.key.PresentationAttribute();
+    DCHECK_EQ(entry.key.IsCSSCustomProperty(), priority == kResolveVariables);
     if (!CSSPropertyPriorityData<priority>::PropertyHasPriority(property))
       continue;
     const Interpolation& interpolation = *entry.value.front();
@@ -1751,7 +1753,8 @@ void StyleResolver::ApplyCustomProperties(StyleResolverState& state,
       needs_apply_pass);
   if (apply_animations == kIncludeAnimations) {
     ApplyAnimatedProperties<kResolveVariables>(
-        state, state.AnimationUpdate().ActiveInterpolationsForAnimations());
+        state,
+        state.AnimationUpdate().ActiveInterpolationsForCustomAnimations());
     ApplyAnimatedProperties<kResolveVariables>(
         state,
         state.AnimationUpdate().ActiveInterpolationsForCustomTransitions());
@@ -1770,7 +1773,8 @@ void StyleResolver::ApplyCustomProperties(StyleResolverState& state,
           needs_apply_pass);
       if (apply_animations == kIncludeAnimations) {
         ApplyAnimatedProperties<kResolveVariables>(
-            state, state.AnimationUpdate().ActiveInterpolationsForAnimations());
+            state,
+            state.AnimationUpdate().ActiveInterpolationsForCustomAnimations());
         ApplyAnimatedProperties<kResolveVariables>(
             state,
             state.AnimationUpdate().ActiveInterpolationsForCustomTransitions());
@@ -1815,17 +1819,12 @@ void StyleResolver::CalculateAnimationUpdate(StyleResolverState& state,
     return;
   }
   if (!state.AnimationUpdate()
+           .ActiveInterpolationsForCustomAnimations()
+           .IsEmpty() ||
+      !state.AnimationUpdate()
            .ActiveInterpolationsForCustomTransitions()
            .IsEmpty()) {
     state.SetIsAnimatingCustomProperties(true);
-    return;
-  }
-  for (const auto& property_handle :
-       state.AnimationUpdate().ActiveInterpolationsForAnimations().Keys()) {
-    if (CSSAnimations::IsCustomPropertyHandle(property_handle)) {
-      state.SetIsAnimatingCustomProperties(true);
-      return;
-    }
   }
 }