Title: [292858] trunk
- Revision
- 292858
- Author
- grao...@webkit.org
- Date
- 2022-04-13 23:11:25 -0700 (Wed, 13 Apr 2022)
Log Message
[web-animations] REGRESSION(r291527): assertion hit during teardown of document with CSS Animations
https://bugs.webkit.org/show_bug.cgi?id=239291
rdar://90699078
Reviewed by Dean Jackson.
Source/WebCore:
When a CSS Animation is not considered to be relevant anymore, it is removed from both AnimationTimeline::m_animations
and Styleable::animations(). However, if that animation becomes relevant again, it will be added back to the associated
effect stack as well as AnimationTimeline::m_animations but not to Styleable::animations().
This causes a problem because when eventually that CSS Animation's target is removed from the tree, such as during
document teardown, Styleable::cancelDeclarativeAnimations() will be called an iterate over Styleable::animations()
to find declarative animations to cancel. Since the CSS animation was not added to Styleable::animations(), it will
not be canceled and the associated effect will not be removed from the effect stack.
Later in Styleable::cancelDeclarativeAnimations(), the list of associated CSS Animation names is cleared.
If during that teardown an animation resolution is performed, such as within a "beforeunload" event listener as
shown in the new test, we will get into a state where there are effects left in the effect stack of the element
being torn down but no associated CSS Animation names and we will hit the RELEASE_ASSERT_NOT_REACHED() at the
end of compareCSSAnimations().
To fix this, we simply ensure that we add animations back to Styleable::animations() within
AnimationTimeline::animationTimingDidChange() the same way we add the animations back to
AnimationTimeline::m_animations.
Test: webanimations/css-animation-resolution-during-teardown.html
* animation/AnimationTimeline.cpp:
(WebCore::AnimationTimeline::animationTimingDidChange):
LayoutTests:
Add a new test that would have asserted prior to the source change.
* webanimations/css-animation-resolution-during-teardown-expected.txt: Added.
* webanimations/css-animation-resolution-during-teardown.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (292857 => 292858)
--- trunk/LayoutTests/ChangeLog 2022-04-14 04:50:36 UTC (rev 292857)
+++ trunk/LayoutTests/ChangeLog 2022-04-14 06:11:25 UTC (rev 292858)
@@ -1,3 +1,16 @@
+2022-04-13 Antoine Quint <grao...@apple.com>
+
+ [web-animations] REGRESSION(r291527): assertion hit during teardown of document with CSS Animations
+ https://bugs.webkit.org/show_bug.cgi?id=239291
+ rdar://90699078
+
+ Reviewed by Dean Jackson.
+
+ Add a new test that would have asserted prior to the source change.
+
+ * webanimations/css-animation-resolution-during-teardown-expected.txt: Added.
+ * webanimations/css-animation-resolution-during-teardown.html: Added.
+
2022-04-13 Alan Bujtas <za...@apple.com>
REGRESSION (r292043): [ Mac ] fast/block/positioning/fixed-container-with-relative-parent.html is a flaky image failure
Added: trunk/LayoutTests/webanimations/css-animation-resolution-during-teardown-expected.txt (0 => 292858)
--- trunk/LayoutTests/webanimations/css-animation-resolution-during-teardown-expected.txt (rev 0)
+++ trunk/LayoutTests/webanimations/css-animation-resolution-during-teardown-expected.txt 2022-04-14 06:11:25 UTC (rev 292858)
@@ -0,0 +1 @@
+PASS
Added: trunk/LayoutTests/webanimations/css-animation-resolution-during-teardown.html (0 => 292858)
--- trunk/LayoutTests/webanimations/css-animation-resolution-during-teardown.html (rev 0)
+++ trunk/LayoutTests/webanimations/css-animation-resolution-during-teardown.html 2022-04-14 06:11:25 UTC (rev 292858)
@@ -0,0 +1,41 @@
+<style>
+ @keyframes a0 {
+ }
+ div, span {
+ animation-name: a0;
+ }
+ slot {
+ animation-name: a0, a0;
+ }
+</style>
+<script>
+ window.testRunner?.dumpAsText();
+ window.testRunner?.waitUntilDone();
+
+ _onload_ = () => {
+ document.body.append(document.createElement('input'));
+ document.body.append(document.createElement('slot'));
+ let br0 = document.createElement('br');
+ document.body.append(br0);
+ let div0 = document.createElement('div');
+ br0.append(div0);
+ document.body.offsetTop;
+ document.body.append(document.createElement('input'));
+ div0.append(document.createElement('span'));
+ document.execCommand('SelectAll');
+ _onbeforeunload_ = () => {
+ document.styleSheets[0].insertRule(`slot { animation-delay: 25ms; }`);
+ document.designMode = 'on';
+ document.execCommand('Bold');
+ };
+
+ requestAnimationFrame(() => {
+ if (!window.location.search)
+ window.location.href = "" + "?foo=1";
+ else {
+ document.body.textContent = "PASS";
+ window.testRunner?.notifyDone();
+ }
+ });
+ };
+</script>
Modified: trunk/Source/WebCore/ChangeLog (292857 => 292858)
--- trunk/Source/WebCore/ChangeLog 2022-04-14 04:50:36 UTC (rev 292857)
+++ trunk/Source/WebCore/ChangeLog 2022-04-14 06:11:25 UTC (rev 292858)
@@ -1,3 +1,36 @@
+2022-04-13 Antoine Quint <grao...@apple.com>
+
+ [web-animations] REGRESSION(r291527): assertion hit during teardown of document with CSS Animations
+ https://bugs.webkit.org/show_bug.cgi?id=239291
+ rdar://90699078
+
+ Reviewed by Dean Jackson.
+
+ When a CSS Animation is not considered to be relevant anymore, it is removed from both AnimationTimeline::m_animations
+ and Styleable::animations(). However, if that animation becomes relevant again, it will be added back to the associated
+ effect stack as well as AnimationTimeline::m_animations but not to Styleable::animations().
+
+ This causes a problem because when eventually that CSS Animation's target is removed from the tree, such as during
+ document teardown, Styleable::cancelDeclarativeAnimations() will be called an iterate over Styleable::animations()
+ to find declarative animations to cancel. Since the CSS animation was not added to Styleable::animations(), it will
+ not be canceled and the associated effect will not be removed from the effect stack.
+
+ Later in Styleable::cancelDeclarativeAnimations(), the list of associated CSS Animation names is cleared.
+
+ If during that teardown an animation resolution is performed, such as within a "beforeunload" event listener as
+ shown in the new test, we will get into a state where there are effects left in the effect stack of the element
+ being torn down but no associated CSS Animation names and we will hit the RELEASE_ASSERT_NOT_REACHED() at the
+ end of compareCSSAnimations().
+
+ To fix this, we simply ensure that we add animations back to Styleable::animations() within
+ AnimationTimeline::animationTimingDidChange() the same way we add the animations back to
+ AnimationTimeline::m_animations.
+
+ Test: webanimations/css-animation-resolution-during-teardown.html
+
+ * animation/AnimationTimeline.cpp:
+ (WebCore::AnimationTimeline::animationTimingDidChange):
+
2022-04-13 Chris Dumez <cdu...@apple.com>
Replace calls to substring(0, x) with the more concise left(x)
Modified: trunk/Source/WebCore/animation/AnimationTimeline.cpp (292857 => 292858)
--- trunk/Source/WebCore/animation/AnimationTimeline.cpp 2022-04-14 04:50:36 UTC (rev 292857)
+++ trunk/Source/WebCore/animation/AnimationTimeline.cpp 2022-04-14 06:11:25 UTC (rev 292858)
@@ -52,6 +52,10 @@
auto* timeline = animation.timeline();
if (timeline && timeline != this)
timeline->removeAnimation(animation);
+ else if (timeline == this && is<KeyframeEffect>(animation.effect())) {
+ if (auto styleable = downcast<KeyframeEffect>(animation.effect())->targetStyleable())
+ styleable->animationWasAdded(animation);
+ }
}
}
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
