[web-animations] keyframes should be recomputed when the "currentcolo…

…r" value is used

https://bugs.webkit.org/show_bug.cgi?id=251491

Reviewed by Antti Koivisto.

Keyframes can set any number of color-related properties to use `currentcolor`. We need
to recompute keyframes if the value to which `currentcolor` would resolve changes during
an animation.

To that end, we keep track of properties set to `currentcolor` on keyframes, and recompute
the keyframes if we find that `RenderStyle::color()` has changed during style resolution.

In the case where one of those properties is the `color` property, the relevant value is
not the style's value, but the parent style's value. In this case, which is bound to be
rather rare, we elect to always recompute keyframes.

* LayoutTests/imported/w3c/web-platform-tests/css/css-animations/responsive/column-rule-color-001-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/web-animations/responsive/to-color-change-expected.txt:
* Source/WebCore/animation/KeyframeEffect.cpp:
(WebCore::KeyframeEffect::updateBlendingKeyframes):
(WebCore::KeyframeEffect::computeCSSAnimationBlendingKeyframes):
(WebCore::KeyframeEffect::hasPropertySetToCurrentColor const):
(WebCore::KeyframeEffect::hasColorSetToCurrentColor const):
* Source/WebCore/animation/KeyframeEffect.h:
* Source/WebCore/animation/KeyframeEffectStack.cpp:
(WebCore::KeyframeEffectStack::applyKeyframeEffects):
* Source/WebCore/style/StyleResolver.cpp:
(WebCore::Style::Resolver::keyframeStylesForAnimation):
* Source/WebCore/style/StyleResolver.h:

Canonical link: https://commits.webkit.org/259736@main

LayoutTests/imported/w3c/web-platform-tests/css/css-animations/responsive/column-rule-color-001-expected.txt

@@ -1,4 +1,4 @@
 
-FAIL column-rule-color responds to currentColor changes assert_equals: expected "rgb(0, 30, 30)" but got "rgb(30, 30, 0)"
+PASS column-rule-color responds to currentColor changes
 PASS column-rule-color responds to inherited changes
 

LayoutTests/imported/w3c/web-platform-tests/web-animations/responsive/to-color-change-expected.txt

@@ -7,8 +7,8 @@ PASS Border color responsive to currentColor changes again
 PASS Outline color responsive to currentColor changes
 PASS Stroke color responsive to currentColor changes
 PASS Text decoration color responsive to currentColor changes
-FAIL Color responsive to parent currentColor changes assert_equals: expected "rgb(34, 51, 34)" but got "rgb(0, 51, 51)"
-FAIL Color responsive to repeated parent currentColor changes assert_equals: expected "rgb(102, 17, 51)" but got "rgb(0, 17, 153)"
+PASS Color responsive to parent currentColor changes
+PASS Color responsive to repeated parent currentColor changes
 PASS Color animations do not expose visited status
 PASS Color animations do not expose parent visited status
 PASS Color animations respond to inherited changes

Source/WebCore/animation/KeyframeEffect.cpp

@@ -883,6 +883,7 @@ void KeyframeEffect::updateBlendingKeyframes(RenderStyle& elementStyle, const St
     auto& styleResolver = m_target->styleResolver();
 
     m_inheritedProperties.clear();
+    m_currentColorProperties.clear();
     m_containsCSSVariableReferences = false;
 
     for (auto& keyframe : m_parsedKeyframes) {
@@ -909,8 +910,13 @@ void KeyframeEffect::updateBlendingKeyframes(RenderStyle& elementStyle, const St
 
         for (auto property : keyframeRule->properties()) {
             if (auto* cssValue = property.value()) {
-                if (cssValue->isPrimitiveValue() && downcast<CSSPrimitiveValue>(cssValue)->valueID() == CSSValueInherit)
-                    m_inheritedProperties.add(property.id());
+                if (cssValue->isPrimitiveValue()) {
+                    auto valueId = downcast<CSSPrimitiveValue>(*cssValue).valueID();
+                    if (valueId == CSSValueInherit)
+                        m_inheritedProperties.add(property.id());
+                    else if (valueId == CSSValueCurrentcolor)
+                        m_currentColorProperties.add(property.id());
+                }
             }
         }
 
@@ -1055,7 +1061,7 @@ void KeyframeEffect::computeCSSAnimationBlendingKeyframes(const RenderStyle& una
 
     KeyframeList keyframeList(AtomString { backingAnimation.name().string });
     if (auto* styleScope = Style::Scope::forOrdinal(*m_target, backingAnimation.nameStyleScopeOrdinal()))
-        styleScope->resolver().keyframeStylesForAnimation(*m_target, unanimatedStyle, resolutionContext, keyframeList, m_containsCSSVariableReferences, m_inheritedProperties);
+        styleScope->resolver().keyframeStylesForAnimation(*m_target, unanimatedStyle, resolutionContext, keyframeList, m_containsCSSVariableReferences, m_inheritedProperties, m_currentColorProperties);
 
     // Ensure resource loads for all the frames.
     for (auto& keyframe : keyframeList) {
@@ -2391,4 +2397,14 @@ void KeyframeEffect::lastStyleChangeEventStyleDidChange(const RenderStyle* previ
         abilityToBeAcceleratedDidChange();
 }
 
+bool KeyframeEffect::hasPropertySetToCurrentColor() const
+{
+    return !m_currentColorProperties.isEmpty();
+}
+
+bool KeyframeEffect::hasColorSetToCurrentColor() const
+{
+    return m_currentColorProperties.contains(CSSPropertyColor);
+}
+
 } // namespace WebCore

Source/WebCore/animation/KeyframeEffect.h

@@ -180,6 +180,8 @@ class KeyframeEffect : public AnimationEffect, public CSSPropertyBlendingClient
 
     bool containsCSSVariableReferences() const { return m_containsCSSVariableReferences; }
     bool hasExplicitlyInheritedKeyframeProperty() const { return m_hasExplicitlyInheritedKeyframeProperty; }
+    bool hasPropertySetToCurrentColor() const;
+    bool hasColorSetToCurrentColor() const;
 
 private:
     KeyframeEffect(Element*, PseudoId);
@@ -243,6 +245,7 @@ class KeyframeEffect : public AnimationEffect, public CSSPropertyBlendingClient
     KeyframeList m_blendingKeyframes { emptyAtom() };
     HashSet<AnimatableProperty> m_animatedProperties;
     HashSet<CSSPropertyID> m_inheritedProperties;
+    HashSet<CSSPropertyID> m_currentColorProperties;
     Vector<ParsedKeyframe> m_parsedKeyframes;
     Vector<AcceleratedAction> m_pendingAcceleratedActions;
     RefPtr<Element> m_target;

Source/WebCore/animation/KeyframeEffectStack.cpp

@@ -187,8 +187,18 @@ OptionSet<AnimationImpact> KeyframeEffectStack::applyKeyframeEffects(RenderStyle
             return false;
         };
 
+        auto currentColorPropertyChanged = [&]() {
+            // If the "color" property itself is set to "currentcolor" on a keyframe, we always recompute keyframes.
+            if (effect->hasColorSetToCurrentColor())
+                return true;
+            // For all other color-related properties set to "currentcolor" on a keyframe, it's sufficient to check
+            // whether the value "color" resolves to has changed since the last style resolution.
+            return effect->hasPropertySetToCurrentColor() && previousLastStyleChangeEventStyle
+                && previousLastStyleChangeEventStyle->color() != targetStyle.color();
+        };
+
         auto logicalPropertyDidChange = propertyAffectingLogicalPropertiesChanged && effect->animatesDirectionAwareProperty();
-        if (logicalPropertyDidChange || fontSizeChanged || inheritedPropertyChanged() || cssVariableChanged())
+        if (logicalPropertyDidChange || fontSizeChanged || inheritedPropertyChanged() || cssVariableChanged() || currentColorPropertyChanged())
             effect->propertyAffectingKeyframeResolutionDidChange(unanimatedStyle, resolutionContext);
 
         animation->resolve(targetStyle, resolutionContext);

Source/WebCore/style/StyleResolver.cpp

@@ -430,9 +430,10 @@ Vector<Ref<StyleRuleKeyframe>> Resolver::keyframeRulesForName(const AtomString&
     return deduplicatedKeyframes;
 }
 
-void Resolver::keyframeStylesForAnimation(const Element& element, const RenderStyle& elementStyle, const ResolutionContext& context, KeyframeList& list, bool& containsCSSVariableReferences, HashSet<CSSPropertyID>& inheritedProperties)
+void Resolver::keyframeStylesForAnimation(const Element& element, const RenderStyle& elementStyle, const ResolutionContext& context, KeyframeList& list, bool& containsCSSVariableReferences, HashSet<CSSPropertyID>& inheritedProperties, HashSet<CSSPropertyID>& currentColorProperties)
 {
     inheritedProperties.clear();
+    currentColorProperties.clear();
 
     list.clear();
 
@@ -458,8 +459,13 @@ void Resolver::keyframeStylesForAnimation(const Element& element, const RenderSt
             }
             for (auto property : keyframeRule->properties()) {
                 if (auto* cssValue = property.value()) {
-                    if (cssValue->isPrimitiveValue() && downcast<CSSPrimitiveValue>(cssValue)->valueID() == CSSValueInherit)
-                        inheritedProperties.add(property.id());
+                    if (cssValue->isPrimitiveValue()) {
+                        auto valueId = downcast<CSSPrimitiveValue>(*cssValue).valueID();
+                        if (valueId == CSSValueInherit)
+                            inheritedProperties.add(property.id());
+                        else if (valueId == CSSValueCurrentcolor)
+                            currentColorProperties.add(property.id());
+                    }
                 }
             }
             list.insert(WTFMove(keyframeValue));

Source/WebCore/style/StyleResolver.h

@@ -95,7 +95,7 @@ class Resolver : public RefCounted<Resolver> {
 
     ResolvedStyle styleForElement(const Element&, const ResolutionContext&, RuleMatchingBehavior = RuleMatchingBehavior::MatchAllRules);
 
-    void keyframeStylesForAnimation(const Element&, const RenderStyle& elementStyle, const ResolutionContext&, KeyframeList&, bool& containsCSSVariableReferences, HashSet<CSSPropertyID>& inheritedProperties);
+    void keyframeStylesForAnimation(const Element&, const RenderStyle& elementStyle, const ResolutionContext&, KeyframeList&, bool& containsCSSVariableReferences, HashSet<CSSPropertyID>& inheritedProperties, HashSet<CSSPropertyID>& currentColorProperties);
 
     WEBCORE_EXPORT std::optional<ResolvedStyle> styleForPseudoElement(const Element&, const PseudoElementRequest&, const ResolutionContext&);