Title: [288423] trunk
- Revision
- 288423
- Author
- grao...@webkit.org
- Date
- 2022-01-23 13:21:20 -0800 (Sun, 23 Jan 2022)
Log Message
m_lastStyleChangeEventStyle null ptr deref for accelerated CSS Animation with no duration and an implicit keyframe
https://bugs.webkit.org/show_bug.cgi?id=235394
<rdar://problem/87701738>
Reviewed by Antti Koivisto.
Source/WebCore:
Test: webanimations/accelerated-animation-without-duration-crash.html
In r287827, the fix for bug 235014, we stopped filling implicit keyframes for CSS Animations at creation
time such that the output of getKeyframes() would correctly account for the missing keyframes. This meant
that we have to fill in those implicit keyframes when running an accelerated animation before we pass it
on to GraphicsLayer.
We would always use the value stored by lastStyleChangeEventStyle() with an assert that this value was
never null. However, in the case of an animation that is not relevant, such as a CSS Animation with no
duration, we've never had a chance to set that style since Style::TreeResolver::createAnimatedElementUpdate()
would not see any "relevant" (a term defined by the Web Animations specification to specify an animation
that has an effect on its target) animations.
We now use the renderer's style as a fallback, which is guaranteed to be defined at this stage.
* animation/KeyframeEffect.cpp:
(WebCore::KeyframeEffect::applyPendingAcceleratedActions):
LayoutTests:
New test, created by Gabriel Nava Marino, that creates an accelerated animation with no
duration and with implicit keyframes that would crash prior to this patch.
* webanimations/accelerated-animation-without-duration-crash-expected.txt: Added.
* webanimations/accelerated-animation-without-duration-crash.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (288422 => 288423)
--- trunk/LayoutTests/ChangeLog 2022-01-23 19:27:53 UTC (rev 288422)
+++ trunk/LayoutTests/ChangeLog 2022-01-23 21:21:20 UTC (rev 288423)
@@ -1,3 +1,17 @@
+2022-01-23 Antoine Quint <grao...@webkit.org>
+
+ m_lastStyleChangeEventStyle null ptr deref for accelerated CSS Animation with no duration and an implicit keyframe
+ https://bugs.webkit.org/show_bug.cgi?id=235394
+ <rdar://problem/87701738>
+
+ Reviewed by Antti Koivisto.
+
+ New test, created by Gabriel Nava Marino, that creates an accelerated animation with no
+ duration and with implicit keyframes that would crash prior to this patch.
+
+ * webanimations/accelerated-animation-without-duration-crash-expected.txt: Added.
+ * webanimations/accelerated-animation-without-duration-crash.html: Added.
+
2022-01-23 Alan Bujtas <za...@apple.com>
[LFC][IFC] Enable unicode-bidi: plaintext for IFC
Added: trunk/LayoutTests/webanimations/accelerated-animation-without-duration-crash-expected.txt (0 => 288423)
--- trunk/LayoutTests/webanimations/accelerated-animation-without-duration-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/webanimations/accelerated-animation-without-duration-crash-expected.txt 2022-01-23 21:21:20 UTC (rev 288423)
@@ -0,0 +1 @@
+PASS if this doesn't crash
Added: trunk/LayoutTests/webanimations/accelerated-animation-without-duration-crash.html (0 => 288423)
--- trunk/LayoutTests/webanimations/accelerated-animation-without-duration-crash.html (rev 0)
+++ trunk/LayoutTests/webanimations/accelerated-animation-without-duration-crash.html 2022-01-23 21:21:20 UTC (rev 288423)
@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<style>
+
+html {
+ animation-name: anim;
+ rotate: 0 0 0 0deg;
+}
+
+@keyframes anim {
+ from { filter: invert() }
+}
+
+</style>
+
+<div>PASS if this doesn't crash</div>
+
+<script>
+
+window.testRunner?.dumpAsText();
+
+</script>
Modified: trunk/Source/WebCore/ChangeLog (288422 => 288423)
--- trunk/Source/WebCore/ChangeLog 2022-01-23 19:27:53 UTC (rev 288422)
+++ trunk/Source/WebCore/ChangeLog 2022-01-23 21:21:20 UTC (rev 288423)
@@ -1,3 +1,29 @@
+2022-01-23 Antoine Quint <grao...@webkit.org>
+
+ m_lastStyleChangeEventStyle null ptr deref for accelerated CSS Animation with no duration and an implicit keyframe
+ https://bugs.webkit.org/show_bug.cgi?id=235394
+ <rdar://problem/87701738>
+
+ Reviewed by Antti Koivisto.
+
+ Test: webanimations/accelerated-animation-without-duration-crash.html
+
+ In r287827, the fix for bug 235014, we stopped filling implicit keyframes for CSS Animations at creation
+ time such that the output of getKeyframes() would correctly account for the missing keyframes. This meant
+ that we have to fill in those implicit keyframes when running an accelerated animation before we pass it
+ on to GraphicsLayer.
+
+ We would always use the value stored by lastStyleChangeEventStyle() with an assert that this value was
+ never null. However, in the case of an animation that is not relevant, such as a CSS Animation with no
+ duration, we've never had a chance to set that style since Style::TreeResolver::createAnimatedElementUpdate()
+ would not see any "relevant" (a term defined by the Web Animations specification to specify an animation
+ that has an effect on its target) animations.
+
+ We now use the renderer's style as a fallback, which is guaranteed to be defined at this stage.
+
+ * animation/KeyframeEffect.cpp:
+ (WebCore::KeyframeEffect::applyPendingAcceleratedActions):
+
2022-01-23 Philippe Normand <pnorm...@igalia.com>
[GStreamer] C++20 warnings
Modified: trunk/Source/WebCore/animation/KeyframeEffect.cpp (288422 => 288423)
--- trunk/Source/WebCore/animation/KeyframeEffect.cpp 2022-01-23 19:27:53 UTC (rev 288422)
+++ trunk/Source/WebCore/animation/KeyframeEffect.cpp 2022-01-23 21:21:20 UTC (rev 288423)
@@ -1858,15 +1858,17 @@
ASSERT(m_target);
- auto* lastStyleChangeEventStyle = m_target->lastStyleChangeEventStyle(m_pseudoId);
- ASSERT(lastStyleChangeEventStyle);
-
// We need to resolve all animations up to this point to ensure any forward-filling
// effect is accounted for when computing the "from" value for the accelerated animation.
- auto underlyingStyle = RenderStyle::clonePtr(*lastStyleChangeEventStyle);
auto* effectStack = m_target->keyframeEffectStack(m_pseudoId);
ASSERT(effectStack);
+ auto underlyingStyle = [&]() {
+ if (auto* lastStyleChangeEventStyle = m_target->lastStyleChangeEventStyle(m_pseudoId))
+ return RenderStyle::clonePtr(*lastStyleChangeEventStyle);
+ return RenderStyle::clonePtr(renderer->style());
+ }();
+
for (const auto& effect : effectStack->sortedEffects()) {
if (this == effect.get())
break;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
