Changeset 287707 in webkit


Ignore:
Timestamp:
Jan 6, 2022 11:57:38 AM (6 months ago)
Author:
graouts@webkit.org
Message:

[Web Animations] inserting a rule within a @keyframes rule should update animations
​https://bugs.webkit.org/show_bug.cgi?id=234895

Reviewed by Darin Adler.

LayoutTests/imported/w3c:

Mark WPT progressions.

  • web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative-expected.txt:
  • web-platform-tests/css/css-animations/KeyframeEffect-setKeyframes.tentative-expected.txt:

Source/WebCore:

Using the CSSOM, it is possible to insert or delete rules within an @keyframes rule.
In fact, there are two WPT that check this behavior with the getKeyframes() and
setKeyframes() methods.

This would not have any effect until now because when we consider whether to invalidate
animations in Styleable::updateCSSAnimations(), we look at the previous and current
AnimationList and don't see any difference because we look, as far as keyframes are
concerned, at the @keyframes name but not at the keyframes content.

Now, when a rule is added or deleted from an @keyframes rule using the CSSOM, we notify
the Document using the new keyframesRuleDidChange() method, which in turn checks all
CSSAnimation objects applied to elements in that document using that @keyframes rule
and notifies them of the change by calling keyframesRuleDidChange().

This clears the keyframes on the associated KeyframeEffect, invalidates the target
and sets a flag on the ElementAnimationRareData that this element is pending update
to its CSS Animations' keyframe such that during the next call
Styleable::updateCSSAnimations() we force the update even if the previous and current
AnimationList look identical.

During that next call to Styleable::updateCSSAnimations(), we will call into the new
CSSAnimation::updateKeyframesIfNeeded() which will re-compute the keyframes based on the
current set of rules within the @keyframes rule.

The final piece of work required is to track when setKeyframes() is called on the effect
that updates to the @keyframes rule should no longer apply since they keyframes were overriden
by the Web Animations API. This is done by setting an additional flag in
CSSAnimation::effectKeyframesWereSetUsingBindings().

  • animation/CSSAnimation.cpp:

(WebCore::CSSAnimation::effectKeyframesWereSetUsingBindings):
(WebCore::CSSAnimation::keyframesRuleDidChange):
(WebCore::CSSAnimation::updateKeyframesIfNeeded):

  • animation/CSSAnimation.h:
  • animation/ElementAnimationRareData.h:

(WebCore::ElementAnimationRareData::cssAnimationsDidUpdate):
(WebCore::ElementAnimationRareData::keyframesRuleDidChange):
(WebCore::ElementAnimationRareData::hasPendingKeyframesUpdate const):

  • animation/KeyframeEffect.cpp:

(WebCore::KeyframeEffect::keyframesRuleDidChange):

  • animation/KeyframeEffect.h:
  • css/CSSStyleSheet.cpp:

(WebCore::CSSStyleSheet::didMutateRules):
(WebCore::CSSStyleSheet::RuleMutationScope::RuleMutationScope):
(WebCore::CSSStyleSheet::RuleMutationScope::~RuleMutationScope):

  • css/CSSStyleSheet.h:
  • dom/Document.cpp:

(WebCore::Document::keyframesRuleDidChange):

  • dom/Document.h:
  • dom/Element.cpp:

(WebCore::Element::cssAnimationsDidUpdate):
(WebCore::Element::keyframesRuleDidChange):
(WebCore::Element::hasPendingKeyframesUpdate const):

  • dom/Element.h:
  • style/Styleable.cpp:

(WebCore::Styleable::updateCSSAnimations const):

  • style/Styleable.h:

(WebCore::Styleable::keyframesRuleDidChange const):

Location:
trunk
Files:
17 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r287688 r287707  
     12022-01-06  Antoine Quint  <graouts@webkit.org>
     2
     3        [Web Animations] inserting a rule within a @keyframes rule should update animations
     4        https://bugs.webkit.org/show_bug.cgi?id=234895
     5
     6        Reviewed by Darin Adler.
     7
     8        Mark WPT progressions.
     9
     10        * web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative-expected.txt:
     11        * web-platform-tests/css/css-animations/KeyframeEffect-setKeyframes.tentative-expected.txt:
     12
    1132022-01-06  Martin Robinson  <mrobinson@webkit.org>
    214

  • trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative-expected.txt

    r287524 r287707  
    2424PASS KeyframeEffect.getKeyframes() returns expected values for animations with a CSS variable which is overriden by the value in keyframe
    2525FAIL KeyframeEffect.getKeyframes() returns expected values for animations with only custom property in a keyframe assert_equals: value for 'transform' on Keyframe #0 should match expected "translate(100px)" but got "matrix(1, 0, 0, 1, 100, 0)"
    26 FAIL KeyframeEffect.getKeyframes() reflects changes to @keyframes rules assert_equals: value for 'left' on Keyframes reflects the updated @keyframes rule should match expected "200px" but got "100px"
     26PASS KeyframeEffect.getKeyframes() reflects changes to @keyframes rules
    2727

  • trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/KeyframeEffect-setKeyframes.tentative-expected.txt

    r267650 r287707  
    22PASS KeyframeEffect.setKeyframes() causes subsequent changes to @keyframes rules to be ignored
    33PASS KeyframeEffect.setKeyframes() causes subsequent changes to animation-timing-function to be ignored
    4 FAIL KeyframeEffect.setKeyframes() should NOT cause subsequent changes to @keyframes rules to be ignored if it threw assert_equals: value for 'left' on Keyframes reflect the value set via style should match expected "300px" but got "100px"
     4PASS KeyframeEffect.setKeyframes() should NOT cause subsequent changes to @keyframes rules to be ignored if it threw
    55

  • trunk/Source/WebCore/ChangeLog

    r287704 r287707  
     12022-01-06  Antoine Quint  <graouts@webkit.org>
     2
     3        [Web Animations] inserting a rule within a @keyframes rule should update animations
     4        https://bugs.webkit.org/show_bug.cgi?id=234895
     5
     6        Reviewed by Darin Adler.
     7
     8        Using the CSSOM, it is possible to insert or delete rules within an @keyframes rule.
     9        In fact, there are two WPT that check this behavior with the getKeyframes() and
     10        setKeyframes() methods.
     11
     12        This would not have any effect until now because when we consider whether to invalidate
     13        animations in Styleable::updateCSSAnimations(), we look at the previous and current
     14        AnimationList and don't see any difference because we look, as far as keyframes are
     15        concerned, at the @keyframes name but not at the keyframes content.
     16
     17        Now, when a rule is added or deleted from an @keyframes rule using the CSSOM, we notify
     18        the Document using the new keyframesRuleDidChange() method, which in turn checks all
     19        CSSAnimation objects applied to elements in that document using that @keyframes rule
     20        and notifies them of the change by calling keyframesRuleDidChange().
     21
     22        This clears the keyframes on the associated KeyframeEffect, invalidates the target
     23        and sets a flag on the ElementAnimationRareData that this element is pending update
     24        to its CSS Animations' keyframe such that during the next call
     25        Styleable::updateCSSAnimations() we force the update even if the previous and current
     26        AnimationList look identical.
     27
     28        During that next call to Styleable::updateCSSAnimations(), we will call into the new
     29        CSSAnimation::updateKeyframesIfNeeded() which will re-compute the keyframes based on the
     30        current set of rules within the @keyframes rule.
     31
     32        The final piece of work required is to track when setKeyframes() is called on the effect
     33        that updates to the @keyframes rule should no longer apply since they keyframes were overriden
     34        by the Web Animations API. This is done by setting an additional flag in
     35        CSSAnimation::effectKeyframesWereSetUsingBindings().
     36
     37        * animation/CSSAnimation.cpp:
     38        (WebCore::CSSAnimation::effectKeyframesWereSetUsingBindings):
     39        (WebCore::CSSAnimation::keyframesRuleDidChange):
     40        (WebCore::CSSAnimation::updateKeyframesIfNeeded):
     41        * animation/CSSAnimation.h:
     42        * animation/ElementAnimationRareData.h:
     43        (WebCore::ElementAnimationRareData::cssAnimationsDidUpdate):
     44        (WebCore::ElementAnimationRareData::keyframesRuleDidChange):
     45        (WebCore::ElementAnimationRareData::hasPendingKeyframesUpdate const):
     46        * animation/KeyframeEffect.cpp:
     47        (WebCore::KeyframeEffect::keyframesRuleDidChange):
     48        * animation/KeyframeEffect.h:
     49        * css/CSSStyleSheet.cpp:
     50        (WebCore::CSSStyleSheet::didMutateRules):
     51        (WebCore::CSSStyleSheet::RuleMutationScope::RuleMutationScope):
     52        (WebCore::CSSStyleSheet::RuleMutationScope::~RuleMutationScope):
     53        * css/CSSStyleSheet.h:
     54        * dom/Document.cpp:
     55        (WebCore::Document::keyframesRuleDidChange):
     56        * dom/Document.h:
     57        * dom/Element.cpp:
     58        (WebCore::Element::cssAnimationsDidUpdate):
     59        (WebCore::Element::keyframesRuleDidChange):
     60        (WebCore::Element::hasPendingKeyframesUpdate const):
     61        * dom/Element.h:
     62        * style/Styleable.cpp:
     63        (WebCore::Styleable::updateCSSAnimations const):
     64        * style/Styleable.h:
     65        (WebCore::Styleable::keyframesRuleDidChange const):
     66
    1672022-01-06  Andres Gonzalez  <andresg_22@apple.com>
    268

  • trunk/Source/WebCore/animation/CSSAnimation.cpp

    r284693 r287707  
    238238    // matching @keyframes rules or the resolved value of the animation-timing-function property for the target element will not
    239239    // be reflected in that animation.
     240    m_overriddenProperties.add(Property::Keyframes);
    240241    m_overriddenProperties.add(Property::TimingFunction);
    241242}
    242243
     244void CSSAnimation::keyframesRuleDidChange()
     245{
     246    if (m_overriddenProperties.contains(Property::Keyframes) || !is<KeyframeEffect>(effect()))
     247        return;
     248
     249    auto owningElement = this->owningElement();
     250    if (!owningElement)
     251        return;
     252
     253    downcast<KeyframeEffect>(*effect()).keyframesRuleDidChange();
     254    owningElement->keyframesRuleDidChange();
     255}
     256
     257void CSSAnimation::updateKeyframesIfNeeded(const RenderStyle* oldStyle, const RenderStyle& newStyle, const Style::ResolutionContext& resolutionContext)
     258{
     259    if (m_overriddenProperties.contains(Property::Keyframes) || !is<KeyframeEffect>(effect()))
     260        return;
     261
     262    auto& keyframeEffect = downcast<KeyframeEffect>(*effect());
     263    if (keyframeEffect.blendingKeyframes().isEmpty())
     264        keyframeEffect.computeDeclarativeAnimationBlendingKeyframes(oldStyle, newStyle, resolutionContext);
     265}
     266
    243267Ref<AnimationEventBase> CSSAnimation::createEvent(const AtomString& eventType, double elapsedTime, const String& pseudoId, std::optional<Seconds> timelineTime)
    244268{

  • trunk/Source/WebCore/animation/CSSAnimation.h

    r284693 r287707  
    4747    void effectTimingWasUpdatedUsingBindings(OptionalEffectTiming);
    4848    void effectKeyframesWereSetUsingBindings();
     49    void keyframesRuleDidChange();
     50    void updateKeyframesIfNeeded(const RenderStyle* oldStyle, const RenderStyle& newStyle, const Style::ResolutionContext&);
    4951
    5052private:
    … …  
    6062    ExceptionOr<void> bindingsReverse() final;
    6163
    62     enum class Property : uint8_t {
     64    enum class Property : uint16_t {
    6365        Name = 1 << 0,
    6466        Duration = 1 << 1,
    … …  
    6870        PlayState = 1 << 5,
    6971        Delay = 1 << 6,
    70         FillMode = 1 << 7
     72        FillMode = 1 << 7,
     73        Keyframes = 1 << 8
    7174    };
    7275

  • trunk/Source/WebCore/animation/ElementAnimationRareData.h

    r267571 r287707  
    5555    const RenderStyle* lastStyleChangeEventStyle() const { return m_lastStyleChangeEventStyle.get(); }
    5656    void setLastStyleChangeEventStyle(std::unique_ptr<const RenderStyle>&& style) { m_lastStyleChangeEventStyle = WTFMove(style); }
     57    void cssAnimationsDidUpdate() { m_hasPendingKeyframesUpdate = false; }
     58    void keyframesRuleDidChange() { m_hasPendingKeyframesUpdate = true; }
     59    bool hasPendingKeyframesUpdate() const { return m_hasPendingKeyframesUpdate; }
    5760
    5861private:
    … …  
    6568    PropertyToTransitionMap m_runningTransitionsByProperty;
    6669    PseudoId m_pseudoId;
     70    bool m_hasPendingKeyframesUpdate { false };
    6771};
    6872

  • trunk/Source/WebCore/animation/KeyframeEffect.cpp

    r287549 r287707  
    785785}
    786786
     787void KeyframeEffect::keyframesRuleDidChange()
     788{
     789    ASSERT(is<CSSAnimation>(animation()));
     790    clearBlendingKeyframes();
     791    invalidate();
     792}
     793
    787794ExceptionOr<void> KeyframeEffect::processKeyframes(JSGlobalObject& lexicalGlobalObject, Strong<JSObject>&& keyframesInput)
    788795{

  • trunk/Source/WebCore/animation/KeyframeEffect.h

    r286544 r287707  
    169169
    170170    void stopAcceleratingTransformRelatedProperties(UseAcceleratedAction);
     171
     172    void keyframesRuleDidChange();
    171173
    172174private:

  • trunk/Source/WebCore/css/CSSStyleSheet.cpp

    r283851 r287707  
    152152}
    153153
    154 void CSSStyleSheet::didMutateRules(RuleMutationType mutationType, WhetherContentsWereClonedForMutation contentsWereClonedForMutation, StyleRuleKeyframes* insertedKeyframesRule)
     154void CSSStyleSheet::didMutateRules(RuleMutationType mutationType, WhetherContentsWereClonedForMutation contentsWereClonedForMutation, StyleRuleKeyframes* insertedKeyframesRule, const String& modifiedKeyframesRuleName)
    155155{
    156156    ASSERT(m_contents->isMutable());
    … …  
    171171    }
    172172
     173    if (mutationType == RuleInsertion && !contentsWereClonedForMutation && !scope->activeStyleSheetsContains(this)) {
     174        if (insertedKeyframesRule) {
     175            if (auto* resolver = scope->resolverIfExists())
     176                resolver->addKeyframeStyle(*insertedKeyframesRule);
     177            return;
     178        }
     179        scope->didChangeActiveStyleSheetCandidates();
     180        return;
     181    }
     182
     183    if (mutationType == KeyframesRuleMutation) {
     184        if (auto* ownerDocument = this->ownerDocument())
     185            ownerDocument->keyframesRuleDidChange(modifiedKeyframesRuleName);
     186    }
     187
    173188    scope->didChangeStyleSheetContents();
    174189
    … …  
    402417CSSStyleSheet::RuleMutationScope::RuleMutationScope(CSSRule* rule)
    403418    : m_styleSheet(rule ? rule->parentStyleSheet() : nullptr)
    404     , m_mutationType(OtherMutation)
     419    , m_mutationType(is<CSSKeyframesRule>(rule) ? KeyframesRuleMutation : OtherMutation)
    405420    , m_contentsWereClonedForMutation(ContentsWereNotClonedForMutation)
    406421    , m_insertedKeyframesRule(nullptr)
     422    , m_modifiedKeyframesRuleName(is<CSSKeyframesRule>(rule) ? downcast<CSSKeyframesRule>(*rule).name() : emptyString())
    407423{
    408424    if (m_styleSheet)
    … …  
    413429{
    414430    if (m_styleSheet)
    415         m_styleSheet->didMutateRules(m_mutationType, m_contentsWereClonedForMutation, m_insertedKeyframesRule);
    416 }
    417 
    418 }
     431        m_styleSheet->didMutateRules(m_mutationType, m_contentsWereClonedForMutation, m_insertedKeyframesRule, m_modifiedKeyframesRuleName);
     432}
     433
     434}

  • trunk/Source/WebCore/css/CSSStyleSheet.h

    r279847 r287707  
    9696    void clearHadRulesMutation() { m_mutatedRules = false; }
    9797
    98     enum RuleMutationType { OtherMutation, RuleInsertion };
     98    enum RuleMutationType { OtherMutation, RuleInsertion, KeyframesRuleMutation };
    9999    enum WhetherContentsWereClonedForMutation { ContentsWereNotClonedForMutation = 0, ContentsWereClonedForMutation };
    100100
    … …  
    111111        WhetherContentsWereClonedForMutation m_contentsWereClonedForMutation;
    112112        StyleRuleKeyframes* m_insertedKeyframesRule;
     113        String m_modifiedKeyframesRuleName;
    113114    };
    114115
    115116    WhetherContentsWereClonedForMutation willMutateRules();
    116     void didMutateRules(RuleMutationType, WhetherContentsWereClonedForMutation, StyleRuleKeyframes* insertedKeyframesRule);
     117    void didMutateRules(RuleMutationType, WhetherContentsWereClonedForMutation, StyleRuleKeyframes* insertedKeyframesRule, const String& modifiedKeyframesRuleName);
    117118    void didMutateRuleFromCSSStyleDeclaration();
    118119    void didMutate();

  • trunk/Source/WebCore/dom/Document.cpp

    r287327 r287707  
    3434#include "BeforeUnloadEvent.h"
    3535#include "CDATASection.h"
     36#include "CSSAnimation.h"
    3637#include "CSSFontSelector.h"
    3738#include "CSSParser.h"
    … …  
    85708571}
    85718572
     8573void Document::keyframesRuleDidChange(const String& name)
     8574{
     8575    for (auto* animation : WebAnimation::instances()) {
     8576        if (!is<CSSAnimation>(animation) || !animation->isRelevant())
     8577            continue;
     8578
     8579        auto& cssAnimation = downcast<CSSAnimation>(*animation);
     8580        if (cssAnimation.animationName() != name)
     8581            continue;
     8582
     8583        auto owningElement = cssAnimation.owningElement();
     8584        if (!owningElement || !owningElement->element.isConnected() || &owningElement->element.document() != this)
     8585            continue;
     8586
     8587        cssAnimation.keyframesRuleDidChange();
     8588    }
     8589}
     8590
    85728591void Document::addTopLayerElement(Element& element)
    85738592{

  • trunk/Source/WebCore/dom/Document.h

    r287611 r287707  
    15421542    DocumentTimelinesController* timelinesController() const { return m_timelinesController.get(); }
    15431543    WEBCORE_EXPORT DocumentTimelinesController& ensureTimelinesController();
     1544    void keyframesRuleDidChange(const String& name);
    15441545
    15451546    void addTopLayerElement(Element&);

  • trunk/Source/WebCore/dom/Element.cpp

    r287563 r287707  
    40514051}
    40524052
     4053void Element::cssAnimationsDidUpdate(PseudoId pseudoId)
     4054{
     4055    ensureAnimationRareData(pseudoId).cssAnimationsDidUpdate();
     4056}
     4057
     4058void Element::keyframesRuleDidChange(PseudoId pseudoId)
     4059{
     4060    ensureAnimationRareData(pseudoId).keyframesRuleDidChange();
     4061}
     4062
     4063bool Element::hasPendingKeyframesUpdate(PseudoId pseudoId) const
     4064{
     4065    auto* data = animationRareData(pseudoId);
     4066    return data && data->hasPendingKeyframesUpdate();
     4067}
     4068
    40534069void Element::disconnectFromResizeObservers()
    40544070{

  • trunk/Source/WebCore/dom/Element.h

    r287563 r287707  
    533533    void setLastStyleChangeEventStyle(PseudoId, std::unique_ptr<const RenderStyle>&&);
    534534
     535    void cssAnimationsDidUpdate(PseudoId);
     536    void keyframesRuleDidChange(PseudoId);
     537    bool hasPendingKeyframesUpdate(PseudoId) const;
     538
    535539    bool isInTopLayer() const { return hasNodeFlag(NodeFlag::IsInTopLayer); }
    536540    void addToTopLayer();

  • trunk/Source/WebCore/style/Styleable.cpp

    r287548 r287707  
    231231    auto* currentAnimationList = newStyle.animations();
    232232    auto* previousAnimationList = keyframeEffectStack.cssAnimationList();
    233     if (previousAnimationList && !previousAnimationList->isEmpty() && newStyle.hasAnimations() && *(previousAnimationList) == *(newStyle.animations()))
     233    if (!element.hasPendingKeyframesUpdate(pseudoId) && previousAnimationList && !previousAnimationList->isEmpty() && newStyle.hasAnimations() && *(previousAnimationList) == *(newStyle.animations()))
    234234        return;
    235235
    … …  
    258258                    // the Animation found in the current style.
    259259                    previousAnimation->setBackingAnimation(currentAnimation);
     260                    // Keyframes may have been cleared if the @keyframes rules was changed since
     261                    // the last style update, so we must ensure keyframes are picked up.
     262                    previousAnimation->updateKeyframesIfNeeded(currentStyle, newStyle, resolutionContext);
    260263                    newAnimations.add(previousAnimation);
    261264                    // Remove the matched animation from the list of previous animations so we may not match it again.
    … …  
    282285
    283286    keyframeEffectStack.setCSSAnimationList(currentAnimationList);
     287
     288    element.cssAnimationsDidUpdate(pseudoId);
    284289}
    285290

  • trunk/Source/WebCore/style/Styleable.h

    r284693 r287707  
    146146    }
    147147
     148    void keyframesRuleDidChange() const
     149    {
     150        element.keyframesRuleDidChange(pseudoId);
     151    }
     152
    148153    void elementWasRemoved() const;
    149154
Note: See TracChangeset for help on using the changeset viewer.