Diff
Modified: trunk/LayoutTests/imported/w3c/ChangeLog (287706 => 287707)
--- trunk/LayoutTests/imported/w3c/ChangeLog 2022-01-06 19:47:35 UTC (rev 287706)
+++ trunk/LayoutTests/imported/w3c/ChangeLog 2022-01-06 19:57:38 UTC (rev 287707)
@@ -1,3 +1,15 @@
+2022-01-06 Antoine Quint <grao...@webkit.org>
+
+ [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.
+
+ 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:
+
2022-01-06 Martin Robinson <mrobin...@webkit.org>
REGRESSION (r287323): [iOS] 9 imported/w3c/web-platform-tests/css/css-transforms tests consistently failing
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative-expected.txt (287706 => 287707)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative-expected.txt 2022-01-06 19:47:35 UTC (rev 287706)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative-expected.txt 2022-01-06 19:57:38 UTC (rev 287707)
@@ -23,5 +23,5 @@
PASS KeyframeEffect.getKeyframes() returns expected values for animations with CSS variables as keyframe values in a shorthand property
PASS KeyframeEffect.getKeyframes() returns expected values for animations with a CSS variable which is overriden by the value in keyframe
FAIL 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)"
-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"
+PASS KeyframeEffect.getKeyframes() reflects changes to @keyframes rules
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/KeyframeEffect-setKeyframes.tentative-expected.txt (287706 => 287707)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/KeyframeEffect-setKeyframes.tentative-expected.txt 2022-01-06 19:47:35 UTC (rev 287706)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/KeyframeEffect-setKeyframes.tentative-expected.txt 2022-01-06 19:57:38 UTC (rev 287707)
@@ -1,5 +1,5 @@
PASS KeyframeEffect.setKeyframes() causes subsequent changes to @keyframes rules to be ignored
PASS KeyframeEffect.setKeyframes() causes subsequent changes to animation-timing-function to be ignored
-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"
+PASS KeyframeEffect.setKeyframes() should NOT cause subsequent changes to @keyframes rules to be ignored if it threw
Modified: trunk/Source/WebCore/ChangeLog (287706 => 287707)
--- trunk/Source/WebCore/ChangeLog 2022-01-06 19:47:35 UTC (rev 287706)
+++ trunk/Source/WebCore/ChangeLog 2022-01-06 19:57:38 UTC (rev 287707)
@@ -1,3 +1,69 @@
+2022-01-06 Antoine Quint <grao...@webkit.org>
+
+ [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.
+
+ 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):
+
2022-01-06 Andres Gonzalez <andresg...@apple.com>
Replace WTFMove + clear with std::exchange for AXObjectCache notification queues.
Modified: trunk/Source/WebCore/animation/CSSAnimation.cpp (287706 => 287707)
--- trunk/Source/WebCore/animation/CSSAnimation.cpp 2022-01-06 19:47:35 UTC (rev 287706)
+++ trunk/Source/WebCore/animation/CSSAnimation.cpp 2022-01-06 19:57:38 UTC (rev 287707)
@@ -237,9 +237,33 @@
// After a successful call to setKeyframes() on the KeyframeEffect associated with a CSSAnimation, any subsequent change to
// matching @keyframes rules or the resolved value of the animation-timing-function property for the target element will not
// be reflected in that animation.
+ m_overriddenProperties.add(Property::Keyframes);
m_overriddenProperties.add(Property::TimingFunction);
}
+void CSSAnimation::keyframesRuleDidChange()
+{
+ if (m_overriddenProperties.contains(Property::Keyframes) || !is<KeyframeEffect>(effect()))
+ return;
+
+ auto owningElement = this->owningElement();
+ if (!owningElement)
+ return;
+
+ downcast<KeyframeEffect>(*effect()).keyframesRuleDidChange();
+ owningElement->keyframesRuleDidChange();
+}
+
+void CSSAnimation::updateKeyframesIfNeeded(const RenderStyle* oldStyle, const RenderStyle& newStyle, const Style::ResolutionContext& resolutionContext)
+{
+ if (m_overriddenProperties.contains(Property::Keyframes) || !is<KeyframeEffect>(effect()))
+ return;
+
+ auto& keyframeEffect = downcast<KeyframeEffect>(*effect());
+ if (keyframeEffect.blendingKeyframes().isEmpty())
+ keyframeEffect.computeDeclarativeAnimationBlendingKeyframes(oldStyle, newStyle, resolutionContext);
+}
+
Ref<AnimationEventBase> CSSAnimation::createEvent(const AtomString& eventType, double elapsedTime, const String& pseudoId, std::optional<Seconds> timelineTime)
{
return AnimationEvent::create(eventType, m_animationName, elapsedTime, pseudoId, timelineTime, this);
Modified: trunk/Source/WebCore/animation/CSSAnimation.h (287706 => 287707)
--- trunk/Source/WebCore/animation/CSSAnimation.h 2022-01-06 19:47:35 UTC (rev 287706)
+++ trunk/Source/WebCore/animation/CSSAnimation.h 2022-01-06 19:57:38 UTC (rev 287707)
@@ -46,6 +46,8 @@
void effectTimingWasUpdatedUsingBindings(OptionalEffectTiming);
void effectKeyframesWereSetUsingBindings();
+ void keyframesRuleDidChange();
+ void updateKeyframesIfNeeded(const RenderStyle* oldStyle, const RenderStyle& newStyle, const Style::ResolutionContext&);
private:
CSSAnimation(const Styleable&, const Animation&);
@@ -59,7 +61,7 @@
void setBindingsStartTime(std::optional<double>) final;
ExceptionOr<void> bindingsReverse() final;
- enum class Property : uint8_t {
+ enum class Property : uint16_t {
Name = 1 << 0,
Duration = 1 << 1,
TimingFunction = 1 << 2,
@@ -67,7 +69,8 @@
Direction = 1 << 4,
PlayState = 1 << 5,
Delay = 1 << 6,
- FillMode = 1 << 7
+ FillMode = 1 << 7,
+ Keyframes = 1 << 8
};
String m_animationName;
Modified: trunk/Source/WebCore/animation/ElementAnimationRareData.h (287706 => 287707)
--- trunk/Source/WebCore/animation/ElementAnimationRareData.h 2022-01-06 19:47:35 UTC (rev 287706)
+++ trunk/Source/WebCore/animation/ElementAnimationRareData.h 2022-01-06 19:57:38 UTC (rev 287707)
@@ -54,6 +54,9 @@
PropertyToTransitionMap& runningTransitionsByProperty() { return m_runningTransitionsByProperty; }
const RenderStyle* lastStyleChangeEventStyle() const { return m_lastStyleChangeEventStyle.get(); }
void setLastStyleChangeEventStyle(std::unique_ptr<const RenderStyle>&& style) { m_lastStyleChangeEventStyle = WTFMove(style); }
+ void cssAnimationsDidUpdate() { m_hasPendingKeyframesUpdate = false; }
+ void keyframesRuleDidChange() { m_hasPendingKeyframesUpdate = true; }
+ bool hasPendingKeyframesUpdate() const { return m_hasPendingKeyframesUpdate; }
private:
@@ -64,6 +67,7 @@
PropertyToTransitionMap m_completedTransitionsByProperty;
PropertyToTransitionMap m_runningTransitionsByProperty;
PseudoId m_pseudoId;
+ bool m_hasPendingKeyframesUpdate { false };
};
} // namespace WebCore
Modified: trunk/Source/WebCore/animation/KeyframeEffect.cpp (287706 => 287707)
--- trunk/Source/WebCore/animation/KeyframeEffect.cpp 2022-01-06 19:47:35 UTC (rev 287706)
+++ trunk/Source/WebCore/animation/KeyframeEffect.cpp 2022-01-06 19:57:38 UTC (rev 287707)
@@ -784,6 +784,13 @@
return processKeyframesResult;
}
+void KeyframeEffect::keyframesRuleDidChange()
+{
+ ASSERT(is<CSSAnimation>(animation()));
+ clearBlendingKeyframes();
+ invalidate();
+}
+
ExceptionOr<void> KeyframeEffect::processKeyframes(JSGlobalObject& lexicalGlobalObject, Strong<JSObject>&& keyframesInput)
{
// 1. If object is null, return an empty sequence of keyframes.
Modified: trunk/Source/WebCore/animation/KeyframeEffect.h (287706 => 287707)
--- trunk/Source/WebCore/animation/KeyframeEffect.h 2022-01-06 19:47:35 UTC (rev 287706)
+++ trunk/Source/WebCore/animation/KeyframeEffect.h 2022-01-06 19:57:38 UTC (rev 287707)
@@ -169,6 +169,8 @@
void stopAcceleratingTransformRelatedProperties(UseAcceleratedAction);
+ void keyframesRuleDidChange();
+
private:
KeyframeEffect(Element*, PseudoId);
Modified: trunk/Source/WebCore/css/CSSStyleSheet.cpp (287706 => 287707)
--- trunk/Source/WebCore/css/CSSStyleSheet.cpp 2022-01-06 19:47:35 UTC (rev 287706)
+++ trunk/Source/WebCore/css/CSSStyleSheet.cpp 2022-01-06 19:57:38 UTC (rev 287707)
@@ -151,7 +151,7 @@
didMutate();
}
-void CSSStyleSheet::didMutateRules(RuleMutationType mutationType, WhetherContentsWereClonedForMutation contentsWereClonedForMutation, StyleRuleKeyframes* insertedKeyframesRule)
+void CSSStyleSheet::didMutateRules(RuleMutationType mutationType, WhetherContentsWereClonedForMutation contentsWereClonedForMutation, StyleRuleKeyframes* insertedKeyframesRule, const String& modifiedKeyframesRuleName)
{
ASSERT(m_contents->isMutable());
ASSERT(m_contents->hasOneClient());
@@ -170,6 +170,21 @@
return;
}
+ if (mutationType == RuleInsertion && !contentsWereClonedForMutation && !scope->activeStyleSheetsContains(this)) {
+ if (insertedKeyframesRule) {
+ if (auto* resolver = scope->resolverIfExists())
+ resolver->addKeyframeStyle(*insertedKeyframesRule);
+ return;
+ }
+ scope->didChangeActiveStyleSheetCandidates();
+ return;
+ }
+
+ if (mutationType == KeyframesRuleMutation) {
+ if (auto* ownerDocument = this->ownerDocument())
+ ownerDocument->keyframesRuleDidChange(modifiedKeyframesRuleName);
+ }
+
scope->didChangeStyleSheetContents();
m_mutatedRules = true;
@@ -401,9 +416,10 @@
CSSStyleSheet::RuleMutationScope::RuleMutationScope(CSSRule* rule)
: m_styleSheet(rule ? rule->parentStyleSheet() : nullptr)
- , m_mutationType(OtherMutation)
+ , m_mutationType(is<CSSKeyframesRule>(rule) ? KeyframesRuleMutation : OtherMutation)
, m_contentsWereClonedForMutation(ContentsWereNotClonedForMutation)
, m_insertedKeyframesRule(nullptr)
+ , m_modifiedKeyframesRuleName(is<CSSKeyframesRule>(rule) ? downcast<CSSKeyframesRule>(*rule).name() : emptyString())
{
if (m_styleSheet)
m_contentsWereClonedForMutation = m_styleSheet->willMutateRules();
@@ -412,7 +428,7 @@
CSSStyleSheet::RuleMutationScope::~RuleMutationScope()
{
if (m_styleSheet)
- m_styleSheet->didMutateRules(m_mutationType, m_contentsWereClonedForMutation, m_insertedKeyframesRule);
+ m_styleSheet->didMutateRules(m_mutationType, m_contentsWereClonedForMutation, m_insertedKeyframesRule, m_modifiedKeyframesRuleName);
}
}
Modified: trunk/Source/WebCore/css/CSSStyleSheet.h (287706 => 287707)
--- trunk/Source/WebCore/css/CSSStyleSheet.h 2022-01-06 19:47:35 UTC (rev 287706)
+++ trunk/Source/WebCore/css/CSSStyleSheet.h 2022-01-06 19:57:38 UTC (rev 287707)
@@ -95,7 +95,7 @@
bool hadRulesMutation() const { return m_mutatedRules; }
void clearHadRulesMutation() { m_mutatedRules = false; }
- enum RuleMutationType { OtherMutation, RuleInsertion };
+ enum RuleMutationType { OtherMutation, RuleInsertion, KeyframesRuleMutation };
enum WhetherContentsWereClonedForMutation { ContentsWereNotClonedForMutation = 0, ContentsWereClonedForMutation };
class RuleMutationScope {
@@ -110,10 +110,11 @@
RuleMutationType m_mutationType;
WhetherContentsWereClonedForMutation m_contentsWereClonedForMutation;
StyleRuleKeyframes* m_insertedKeyframesRule;
+ String m_modifiedKeyframesRuleName;
};
WhetherContentsWereClonedForMutation willMutateRules();
- void didMutateRules(RuleMutationType, WhetherContentsWereClonedForMutation, StyleRuleKeyframes* insertedKeyframesRule);
+ void didMutateRules(RuleMutationType, WhetherContentsWereClonedForMutation, StyleRuleKeyframes* insertedKeyframesRule, const String& modifiedKeyframesRuleName);
void didMutateRuleFromCSSStyleDeclaration();
void didMutate();
Modified: trunk/Source/WebCore/dom/Document.cpp (287706 => 287707)
--- trunk/Source/WebCore/dom/Document.cpp 2022-01-06 19:47:35 UTC (rev 287706)
+++ trunk/Source/WebCore/dom/Document.cpp 2022-01-06 19:57:38 UTC (rev 287707)
@@ -33,6 +33,7 @@
#include "Attr.h"
#include "BeforeUnloadEvent.h"
#include "CDATASection.h"
+#include "CSSAnimation.h"
#include "CSSFontSelector.h"
#include "CSSParser.h"
#include "CSSStyleDeclaration.h"
@@ -8569,6 +8570,24 @@
return animations;
}
+void Document::keyframesRuleDidChange(const String& name)
+{
+ for (auto* animation : WebAnimation::instances()) {
+ if (!is<CSSAnimation>(animation) || !animation->isRelevant())
+ continue;
+
+ auto& cssAnimation = downcast<CSSAnimation>(*animation);
+ if (cssAnimation.animationName() != name)
+ continue;
+
+ auto owningElement = cssAnimation.owningElement();
+ if (!owningElement || !owningElement->element.isConnected() || &owningElement->element.document() != this)
+ continue;
+
+ cssAnimation.keyframesRuleDidChange();
+ }
+}
+
void Document::addTopLayerElement(Element& element)
{
RELEASE_ASSERT(&element.document() == this && element.isConnected() && !element.isInTopLayer());
Modified: trunk/Source/WebCore/dom/Document.h (287706 => 287707)
--- trunk/Source/WebCore/dom/Document.h 2022-01-06 19:47:35 UTC (rev 287706)
+++ trunk/Source/WebCore/dom/Document.h 2022-01-06 19:57:38 UTC (rev 287707)
@@ -1541,6 +1541,7 @@
Vector<RefPtr<WebAnimation>> matchingAnimations(const Function<bool(Element&)>&);
DocumentTimelinesController* timelinesController() const { return m_timelinesController.get(); }
WEBCORE_EXPORT DocumentTimelinesController& ensureTimelinesController();
+ void keyframesRuleDidChange(const String& name);
void addTopLayerElement(Element&);
void removeTopLayerElement(Element&);
Modified: trunk/Source/WebCore/dom/Element.cpp (287706 => 287707)
--- trunk/Source/WebCore/dom/Element.cpp 2022-01-06 19:47:35 UTC (rev 287706)
+++ trunk/Source/WebCore/dom/Element.cpp 2022-01-06 19:57:38 UTC (rev 287707)
@@ -4050,6 +4050,22 @@
ensureAnimationRareData(pseudoId).setLastStyleChangeEventStyle(WTFMove(style));
}
+void Element::cssAnimationsDidUpdate(PseudoId pseudoId)
+{
+ ensureAnimationRareData(pseudoId).cssAnimationsDidUpdate();
+}
+
+void Element::keyframesRuleDidChange(PseudoId pseudoId)
+{
+ ensureAnimationRareData(pseudoId).keyframesRuleDidChange();
+}
+
+bool Element::hasPendingKeyframesUpdate(PseudoId pseudoId) const
+{
+ auto* data = ""
+ return data && data->hasPendingKeyframesUpdate();
+}
+
void Element::disconnectFromResizeObservers()
{
auto* observerData = resizeObserverData();
Modified: trunk/Source/WebCore/dom/Element.h (287706 => 287707)
--- trunk/Source/WebCore/dom/Element.h 2022-01-06 19:47:35 UTC (rev 287706)
+++ trunk/Source/WebCore/dom/Element.h 2022-01-06 19:57:38 UTC (rev 287707)
@@ -532,6 +532,10 @@
const RenderStyle* lastStyleChangeEventStyle(PseudoId) const;
void setLastStyleChangeEventStyle(PseudoId, std::unique_ptr<const RenderStyle>&&);
+ void cssAnimationsDidUpdate(PseudoId);
+ void keyframesRuleDidChange(PseudoId);
+ bool hasPendingKeyframesUpdate(PseudoId) const;
+
bool isInTopLayer() const { return hasNodeFlag(NodeFlag::IsInTopLayer); }
void addToTopLayer();
void removeFromTopLayer();
Modified: trunk/Source/WebCore/style/Styleable.cpp (287706 => 287707)
--- trunk/Source/WebCore/style/Styleable.cpp 2022-01-06 19:47:35 UTC (rev 287706)
+++ trunk/Source/WebCore/style/Styleable.cpp 2022-01-06 19:57:38 UTC (rev 287707)
@@ -230,7 +230,7 @@
auto* currentAnimationList = newStyle.animations();
auto* previousAnimationList = keyframeEffectStack.cssAnimationList();
- if (previousAnimationList && !previousAnimationList->isEmpty() && newStyle.hasAnimations() && *(previousAnimationList) == *(newStyle.animations()))
+ if (!element.hasPendingKeyframesUpdate(pseudoId) && previousAnimationList && !previousAnimationList->isEmpty() && newStyle.hasAnimations() && *(previousAnimationList) == *(newStyle.animations()))
return;
CSSAnimationCollection newAnimations;
@@ -257,6 +257,9 @@
// Timing properties or play state may have changed so we need to update the backing animation with
// the Animation found in the current style.
previousAnimation->setBackingAnimation(currentAnimation);
+ // Keyframes may have been cleared if the @keyframes rules was changed since
+ // the last style update, so we must ensure keyframes are picked up.
+ previousAnimation->updateKeyframesIfNeeded(currentStyle, newStyle, resolutionContext);
newAnimations.add(previousAnimation);
// Remove the matched animation from the list of previous animations so we may not match it again.
previousAnimations.remove(previousAnimation);
@@ -281,6 +284,8 @@
setAnimationsCreatedByMarkup(WTFMove(newAnimations));
keyframeEffectStack.setCSSAnimationList(currentAnimationList);
+
+ element.cssAnimationsDidUpdate(pseudoId);
}
static KeyframeEffect* keyframeEffectForElementAndProperty(const Styleable& styleable, CSSPropertyID property)
Modified: trunk/Source/WebCore/style/Styleable.h (287706 => 287707)
--- trunk/Source/WebCore/style/Styleable.h 2022-01-06 19:47:35 UTC (rev 287706)
+++ trunk/Source/WebCore/style/Styleable.h 2022-01-06 19:57:38 UTC (rev 287707)
@@ -145,6 +145,11 @@
element.setLastStyleChangeEventStyle(pseudoId, WTFMove(style));
}
+ void keyframesRuleDidChange() const
+ {
+ element.keyframesRuleDidChange(pseudoId);
+ }
+
void elementWasRemoved() const;
void willChangeRenderer() const;
