Title: [275228] trunk
- Revision
- 275228
- Author
- [email protected]
- Date
- 2021-03-30 12:47:31 -0700 (Tue, 30 Mar 2021)
Log Message
Release assert in compareAnimationsByCompositeOrder
https://bugs.webkit.org/show_bug.cgi?id=223368
Patch by Frédéric Wang <[email protected]> on 2021-03-30
Reviewed by Antoine Quint.
Source/WebCore:
AnimationTimeline::cancelDeclarativeAnimationsForStyleable currently only calls cancel() on
the declarative animation, which means the owning element is still associated to the
animation and that one continues to be accessible via Element's getAnimations(). When members
like pause() are called on that animation, one can obtain inconsistent state for
KeyframeEffectStack, leading to a debug ASSERT(!cssAnimationList->isEmpty()); and a
RELEASE_ASSERT_NOT_REACHED() in compareCSSAnimations(). This patch forces the owning element
to be clear during cancelDeclarativeAnimationsForStyleable in order to address these issues.
More code refactoring and cleanup should be done in follow-up patches, though.
Test: animations/animation-remove-element-crash.html
* animation/AnimationTimeline.cpp:
(WebCore::AnimationTimeline::cancelDeclarativeAnimationsForStyleable): Use cancelFromStyle()
so that the owning element is also cleared.
LayoutTests:
Add regression test.
* animations/animation-remove-element-crash-expected.txt: Added.
* animations/animation-remove-element-crash.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (275227 => 275228)
--- trunk/LayoutTests/ChangeLog 2021-03-30 19:42:45 UTC (rev 275227)
+++ trunk/LayoutTests/ChangeLog 2021-03-30 19:47:31 UTC (rev 275228)
@@ -1,3 +1,15 @@
+2021-03-30 Frédéric Wang <[email protected]>
+
+ Release assert in compareAnimationsByCompositeOrder
+ https://bugs.webkit.org/show_bug.cgi?id=223368
+
+ Reviewed by Antoine Quint.
+
+ Add regression test.
+
+ * animations/animation-remove-element-crash-expected.txt: Added.
+ * animations/animation-remove-element-crash.html: Added.
+
2021-03-30 Antoine Quint <[email protected]>
[CSS Backgrounds] WPT test css/css-backgrounds/animations/border-image-width-interpolation.html asserts
Added: trunk/LayoutTests/animations/animation-remove-element-crash-expected.txt (0 => 275228)
--- trunk/LayoutTests/animations/animation-remove-element-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/animations/animation-remove-element-crash-expected.txt 2021-03-30 19:47:31 UTC (rev 275228)
@@ -0,0 +1,2 @@
+CONSOLE MESSAGE: This test passes if it does not crash.
+
Added: trunk/LayoutTests/animations/animation-remove-element-crash.html (0 => 275228)
--- trunk/LayoutTests/animations/animation-remove-element-crash.html (rev 0)
+++ trunk/LayoutTests/animations/animation-remove-element-crash.html 2021-03-30 19:47:31 UTC (rev 275228)
@@ -0,0 +1,26 @@
+<!doctype>
+<style>
+ p:only-of-type {
+ -webkit-animation: reverse,alternate;
+ -webkit-animation-name: keyframes1;
+ -webkit-animation-duration: 0.5s;
+ }
+ @keyframes keyframes1 {}
+</style>
+<script>
+ function main() {
+ if (window.testRunner)
+ testRunner.dumpAsText();
+ console.log('This test passes if it does not crash.');
+ var animations = A.getAnimations();
+ document.body.appendChild(A);
+ animations[1].pause();
+ animations[0].startTime = 0;
+ }
+</script>
+<body _onload_="main()">
+ <p></p>
+ <div>
+ <p id="A"></p>
+ </div>
+</body>
Modified: trunk/Source/WebCore/ChangeLog (275227 => 275228)
--- trunk/Source/WebCore/ChangeLog 2021-03-30 19:42:45 UTC (rev 275227)
+++ trunk/Source/WebCore/ChangeLog 2021-03-30 19:47:31 UTC (rev 275228)
@@ -1,3 +1,25 @@
+2021-03-30 Frédéric Wang <[email protected]>
+
+ Release assert in compareAnimationsByCompositeOrder
+ https://bugs.webkit.org/show_bug.cgi?id=223368
+
+ Reviewed by Antoine Quint.
+
+ AnimationTimeline::cancelDeclarativeAnimationsForStyleable currently only calls cancel() on
+ the declarative animation, which means the owning element is still associated to the
+ animation and that one continues to be accessible via Element's getAnimations(). When members
+ like pause() are called on that animation, one can obtain inconsistent state for
+ KeyframeEffectStack, leading to a debug ASSERT(!cssAnimationList->isEmpty()); and a
+ RELEASE_ASSERT_NOT_REACHED() in compareCSSAnimations(). This patch forces the owning element
+ to be clear during cancelDeclarativeAnimationsForStyleable in order to address these issues.
+ More code refactoring and cleanup should be done in follow-up patches, though.
+
+ Test: animations/animation-remove-element-crash.html
+
+ * animation/AnimationTimeline.cpp:
+ (WebCore::AnimationTimeline::cancelDeclarativeAnimationsForStyleable): Use cancelFromStyle()
+ so that the owning element is also cleared.
+
2021-03-30 Antoine Quint <[email protected]>
CSS properties backed by StyleImage should not interpolate when one of the values is "none"
Modified: trunk/Source/WebCore/animation/AnimationTimeline.cpp (275227 => 275228)
--- trunk/Source/WebCore/animation/AnimationTimeline.cpp 2021-03-30 19:42:45 UTC (rev 275227)
+++ trunk/Source/WebCore/animation/AnimationTimeline.cpp 2021-03-30 19:47:31 UTC (rev 275228)
@@ -170,7 +170,7 @@
}
}
-void AnimationTimeline::cancelDeclarativeAnimationsForStyleable(const Styleable& styleable, WebAnimation::Silently silently)
+void AnimationTimeline::cancelDeclarativeAnimationsForStyleable(const Styleable& styleable, WebAnimation::Silently)
{
if (auto* animations = styleable.animations()) {
for (auto& animation : *animations) {
@@ -177,7 +177,7 @@
if (is<DeclarativeAnimation>(animation)) {
if (is<CSSAnimation>(animation))
removeCSSAnimationCreatedByMarkup(styleable, downcast<CSSAnimation>(*animation));
- animation->cancel(silently);
+ downcast<DeclarativeAnimation>(*animation).cancelFromStyle();
}
}
}
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
