Title: [274165] trunk
- Revision
- 274165
- Author
- grao...@webkit.org
- Date
- 2021-03-09 12:09:49 -0800 (Tue, 09 Mar 2021)
Log Message
[Web Animations] setKeyframes does not preserve animation's current offset
https://bugs.webkit.org/show_bug.cgi?id=222939
<rdar://problem/75207793>
Reviewed by Dean Jackson.
Source/WebCore:
Test: webanimations/set-keyframes-after-animation-completion.html
When setKeyframes() is called on a KeyframeEffect, it clears the existing blending keyframes
created for the style system which will be re-populated on the next style update.
When an animation completes, it becomes eligible for being removed unless its effect is the
top-most effect in the target element's effect stack affecting one of the CSS properties
animated by effects in the stack. To determine what properties an effect animates, the method
KeyframeEffect::animatedProperties() is used.
Until now, KeyframeEffect::animatedProperties() simply returned the properties from the
blending keyframes. As such, if blending keyframes are empty, that method returns an empty
set of properties.
Since blending keyframes are cleared by a call to setKeyframes(), calling this method on
the effect of an animation that just finished will remove that animation and the effect
will no longer be applied when updating styles.
To fix this, we add a new instance variable on KeyframeEffect to store the list of properties
affected by the effect from the keyframes parsed by setKeyframes() until we generate the
blending keyframes during the next style update.
As soon as we generate the blending keyframes and setBlendingKeyframes() is called, we clear
that new property. This guarantees that no matter how keyframes are specified – CSS Transitions,
CSS Animations or Web Animations JS API – the animatedProperties() method returns the set
of affected properties.
* animation/KeyframeEffect.cpp:
(WebCore::KeyframeEffect::animatedProperties):
(WebCore::KeyframeEffect::setBlendingKeyframes):
* animation/KeyframeEffect.h:
(WebCore::KeyframeEffect::animatedProperties const): Deleted.
LayoutTests:
Add a new test that checks that updating keyframes after an animation has completed
correctly updates styles accounting for the new keyframes.
* webanimations/set-keyframes-after-animation-completion-expected.html: Added.
* webanimations/set-keyframes-after-animation-completion.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (274164 => 274165)
--- trunk/LayoutTests/ChangeLog 2021-03-09 20:03:27 UTC (rev 274164)
+++ trunk/LayoutTests/ChangeLog 2021-03-09 20:09:49 UTC (rev 274165)
@@ -1,3 +1,17 @@
+2021-03-09 Antoine Quint <grao...@webkit.org>
+
+ [Web Animations] setKeyframes does not preserve animation's current offset
+ https://bugs.webkit.org/show_bug.cgi?id=222939
+ <rdar://problem/75207793>
+
+ Reviewed by Dean Jackson.
+
+ Add a new test that checks that updating keyframes after an animation has completed
+ correctly updates styles accounting for the new keyframes.
+
+ * webanimations/set-keyframes-after-animation-completion-expected.html: Added.
+ * webanimations/set-keyframes-after-animation-completion.html: Added.
+
2021-03-09 Peng Liu <peng.l...@apple.com>
[GPUP] Test fast/images/animated-image-mp4.html times out when media in GPU Process is enabled
Added: trunk/LayoutTests/webanimations/set-keyframes-after-animation-completion-expected.html (0 => 274165)
--- trunk/LayoutTests/webanimations/set-keyframes-after-animation-completion-expected.html (rev 0)
+++ trunk/LayoutTests/webanimations/set-keyframes-after-animation-completion-expected.html 2021-03-09 20:09:49 UTC (rev 274165)
@@ -0,0 +1,11 @@
+<style>
+
+#target {
+ width: 100px;
+ height: 100px;
+ background-color: black;
+ margin-left: 150px;
+}
+
+</style>
+<div id="target"></div>
Added: trunk/LayoutTests/webanimations/set-keyframes-after-animation-completion.html (0 => 274165)
--- trunk/LayoutTests/webanimations/set-keyframes-after-animation-completion.html (rev 0)
+++ trunk/LayoutTests/webanimations/set-keyframes-after-animation-completion.html 2021-03-09 20:09:49 UTC (rev 274165)
@@ -0,0 +1,31 @@
+<style>
+
+#target {
+ width: 100px;
+ height: 100px;
+ background-color: black;
+}
+
+</style>
+<div id="target"></div>
+<script>
+
+(async () => {
+ if (window.testRunner)
+ testRunner.waitUntilDone();
+
+ const timing = { fill: "forwards", duration: 1 };
+ const originalKeyframes = { marginLeft: ["0px", "100px"] };
+ const updatedKeyframes = { marginLeft: ["50px", "150px"] };
+ const animation = document.getElementById("target").animate(originalKeyframes, timing);
+
+ await animation.finished;
+ await new Promise(resolve => setTimeout(resolve));
+
+ animation.effect.setKeyframes(updatedKeyframes);
+
+ if (window.testRunner)
+ testRunner.notifyDone();
+})();
+
+</script>
Modified: trunk/Source/WebCore/ChangeLog (274164 => 274165)
--- trunk/Source/WebCore/ChangeLog 2021-03-09 20:03:27 UTC (rev 274164)
+++ trunk/Source/WebCore/ChangeLog 2021-03-09 20:09:49 UTC (rev 274165)
@@ -1,3 +1,44 @@
+2021-03-09 Antoine Quint <grao...@webkit.org>
+
+ [Web Animations] setKeyframes does not preserve animation's current offset
+ https://bugs.webkit.org/show_bug.cgi?id=222939
+ <rdar://problem/75207793>
+
+ Reviewed by Dean Jackson.
+
+ Test: webanimations/set-keyframes-after-animation-completion.html
+
+ When setKeyframes() is called on a KeyframeEffect, it clears the existing blending keyframes
+ created for the style system which will be re-populated on the next style update.
+
+ When an animation completes, it becomes eligible for being removed unless its effect is the
+ top-most effect in the target element's effect stack affecting one of the CSS properties
+ animated by effects in the stack. To determine what properties an effect animates, the method
+ KeyframeEffect::animatedProperties() is used.
+
+ Until now, KeyframeEffect::animatedProperties() simply returned the properties from the
+ blending keyframes. As such, if blending keyframes are empty, that method returns an empty
+ set of properties.
+
+ Since blending keyframes are cleared by a call to setKeyframes(), calling this method on
+ the effect of an animation that just finished will remove that animation and the effect
+ will no longer be applied when updating styles.
+
+ To fix this, we add a new instance variable on KeyframeEffect to store the list of properties
+ affected by the effect from the keyframes parsed by setKeyframes() until we generate the
+ blending keyframes during the next style update.
+
+ As soon as we generate the blending keyframes and setBlendingKeyframes() is called, we clear
+ that new property. This guarantees that no matter how keyframes are specified – CSS Transitions,
+ CSS Animations or Web Animations JS API – the animatedProperties() method returns the set
+ of affected properties.
+
+ * animation/KeyframeEffect.cpp:
+ (WebCore::KeyframeEffect::animatedProperties):
+ (WebCore::KeyframeEffect::setBlendingKeyframes):
+ * animation/KeyframeEffect.h:
+ (WebCore::KeyframeEffect::animatedProperties const): Deleted.
+
2021-03-09 Said Abou-Hallawa <s...@apple.com>
[GPU Process] ImageBitmap should release its ImageBuffer in the main thread
Modified: trunk/Source/WebCore/animation/KeyframeEffect.cpp (274164 => 274165)
--- trunk/Source/WebCore/animation/KeyframeEffect.cpp 2021-03-09 20:03:27 UTC (rev 274164)
+++ trunk/Source/WebCore/animation/KeyframeEffect.cpp 2021-03-09 20:09:49 UTC (rev 274165)
@@ -834,6 +834,21 @@
setBlendingKeyframes(keyframeList);
}
+const HashSet<CSSPropertyID>& KeyframeEffect::animatedProperties()
+{
+ if (!m_blendingKeyframes.isEmpty())
+ return m_blendingKeyframes.properties();
+
+ if (m_animatedProperties.isEmpty()) {
+ for (auto& keyframe : m_parsedKeyframes) {
+ for (auto keyframeProperty : keyframe.unparsedStyle.keys())
+ m_animatedProperties.add(keyframeProperty);
+ }
+ }
+
+ return m_animatedProperties;
+}
+
bool KeyframeEffect::animatesProperty(CSSPropertyID property) const
{
if (!m_blendingKeyframes.isEmpty())
@@ -876,6 +891,7 @@
void KeyframeEffect::setBlendingKeyframes(KeyframeList& blendingKeyframes)
{
m_blendingKeyframes = WTFMove(blendingKeyframes);
+ m_animatedProperties.clear();
computedNeedsForcedLayout();
computeStackingContextImpact();
Modified: trunk/Source/WebCore/animation/KeyframeEffect.h (274164 => 274165)
--- trunk/Source/WebCore/animation/KeyframeEffect.h 2021-03-09 20:03:27 UTC (rev 274164)
+++ trunk/Source/WebCore/animation/KeyframeEffect.h 2021-03-09 20:09:49 UTC (rev 274165)
@@ -156,7 +156,7 @@
void computeDeclarativeAnimationBlendingKeyframes(const RenderStyle* oldStyle, const RenderStyle& newStyle, const RenderStyle* parentElementStyle);
const KeyframeList& blendingKeyframes() const { return m_blendingKeyframes; }
- const HashSet<CSSPropertyID>& animatedProperties() const { return m_blendingKeyframes.properties(); }
+ const HashSet<CSSPropertyID>& animatedProperties();
bool animatesProperty(CSSPropertyID) const;
bool computeExtentOfTransformAnimation(LayoutRect&) const;
@@ -215,6 +215,7 @@
#endif
KeyframeList m_blendingKeyframes { emptyString() };
+ HashSet<CSSPropertyID> m_animatedProperties;
Vector<ParsedKeyframe> m_parsedKeyframes;
Vector<AcceleratedAction> m_pendingAcceleratedActions;
RefPtr<Element> m_target;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
