Title: [291281] trunk
Revision
291281
Author
gnavamar...@apple.com
Date
2022-03-15 04:01:41 -0700 (Tue, 15 Mar 2022)

Log Message

Crash in KeyframeList.cpp:183 in WebCore::KeyframeList::fillImplicitKeyframes
https://bugs.webkit.org/show_bug.cgi?id=237858

Reviewed by Antoine Quint.

Source/WebCore:

When filling implicit key frames, we iterate through the current keyframes (m_keyframes),
and cache the address of the implicitZeroKeyframe and implicitOneKeyframe.

However, if we're not provided with an existing implicit zero keyframe, we will create a new one
and insert it to the list of current keyframes.

This mutates m_keyframes and the old address for the implicitOneKeyframe would no longer be valid.
Thus we should iterate through the current keyframes, after the insertion, to get the latest address.

Test: animations/fill-implicit-keyframes-crash.html

* rendering/style/KeyframeList.cpp:
(WebCore::KeyframeList::fillImplicitKeyframes):

LayoutTests:

* animations/fill-implicit-keyframes-crash-expected.txt: Added.
* animations/fill-implicit-keyframes-crash.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (291280 => 291281)


--- trunk/LayoutTests/ChangeLog	2022-03-15 10:50:45 UTC (rev 291280)
+++ trunk/LayoutTests/ChangeLog	2022-03-15 11:01:41 UTC (rev 291281)
@@ -1,3 +1,13 @@
+2022-03-15  Gabriel Nava Marino  <gnavamar...@apple.com>
+
+        Crash in KeyframeList.cpp:183 in WebCore::KeyframeList::fillImplicitKeyframes
+        https://bugs.webkit.org/show_bug.cgi?id=237858
+
+        Reviewed by Antoine Quint.
+
+        * animations/fill-implicit-keyframes-crash-expected.txt: Added.
+        * animations/fill-implicit-keyframes-crash.html: Added.
+
 2022-03-15  Youenn Fablet  <you...@apple.com>
 
         Make sure to end any pending AudioSession interruption when activating it

Added: trunk/LayoutTests/animations/fill-implicit-keyframes-crash-expected.txt (0 => 291281)


--- trunk/LayoutTests/animations/fill-implicit-keyframes-crash-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/animations/fill-implicit-keyframes-crash-expected.txt	2022-03-15 11:01:41 UTC (rev 291281)
@@ -0,0 +1,2 @@
+CONSOLE MESSAGE: This test passes if it does not crash.
+

Added: trunk/LayoutTests/animations/fill-implicit-keyframes-crash.html (0 => 291281)


--- trunk/LayoutTests/animations/fill-implicit-keyframes-crash.html	                        (rev 0)
+++ trunk/LayoutTests/animations/fill-implicit-keyframes-crash.html	2022-03-15 11:01:41 UTC (rev 291281)
@@ -0,0 +1,23 @@
+<!DOCTYPE html>
+<style>
+  body {
+    animation-name: a0;
+    animation-duration: 100ms
+  }
+  @keyframes a0 {
+    10% {
+      scale: 1
+    }
+    11%, 12%, 13%, 14%, 15%, 16%, 17%, 18%, 19%, 20%, 21%, 22%, 23%, 24%, 100% {
+      background: blue
+    }
+
+  }
+</style>
+<script>
+  _onload_ = () => {
+    if (window.testRunner)
+      testRunner.dumpAsText();
+    console.log("This test passes if it does not crash.");
+  }
+</script>

Modified: trunk/Source/WebCore/ChangeLog (291280 => 291281)


--- trunk/Source/WebCore/ChangeLog	2022-03-15 10:50:45 UTC (rev 291280)
+++ trunk/Source/WebCore/ChangeLog	2022-03-15 11:01:41 UTC (rev 291281)
@@ -1,3 +1,24 @@
+2022-03-15  Gabriel Nava Marino  <gnavamar...@apple.com>
+
+        Crash in KeyframeList.cpp:183 in WebCore::KeyframeList::fillImplicitKeyframes
+        https://bugs.webkit.org/show_bug.cgi?id=237858
+
+        Reviewed by Antoine Quint.
+
+        When filling implicit key frames, we iterate through the current keyframes (m_keyframes),
+        and cache the address of the implicitZeroKeyframe and implicitOneKeyframe.
+
+        However, if we're not provided with an existing implicit zero keyframe, we will create a new one
+        and insert it to the list of current keyframes.
+
+        This mutates m_keyframes and the old address for the implicitOneKeyframe would no longer be valid.
+        Thus we should iterate through the current keyframes, after the insertion, to get the latest address.
+
+        Test: animations/fill-implicit-keyframes-crash.html
+
+        * rendering/style/KeyframeList.cpp:
+        (WebCore::KeyframeList::fillImplicitKeyframes):
+
 2022-03-15  Zan Dobersek  <zdober...@igalia.com>
 
         [GTK][WPE] Provide DMABuf-based composition layers, DMABufVideoSink integration

Modified: trunk/Source/WebCore/rendering/style/KeyframeList.cpp (291280 => 291281)


--- trunk/Source/WebCore/rendering/style/KeyframeList.cpp	2022-03-15 10:50:45 UTC (rev 291280)
+++ trunk/Source/WebCore/rendering/style/KeyframeList.cpp	2022-03-15 11:01:41 UTC (rev 291281)
@@ -168,11 +168,6 @@
                 zeroKeyframeImplicitProperties.remove(cssPropertyId);
             if (!implicitZeroKeyframe && isSuitableKeyframeForImplicitValues(keyframe))
                 implicitZeroKeyframe = &keyframe;
-        } else if (keyframe.key() == 1) {
-            for (auto cssPropertyId : keyframe.properties())
-                oneKeyframeImplicitProperties.remove(cssPropertyId);
-            if (!implicitOneKeyframe && isSuitableKeyframeForImplicitValues(keyframe))
-                implicitOneKeyframe = &keyframe;
         }
     }
 
@@ -199,6 +194,16 @@
 
     if (!zeroKeyframeImplicitProperties.isEmpty())
         addImplicitKeyframe(0, zeroKeyframeImplicitProperties, zeroPercentKeyframe(), implicitZeroKeyframe);
+
+    for (auto& keyframe : m_keyframes) {
+        if (keyframe.key() == 1) {
+            for (auto cssPropertyId : keyframe.properties())
+                oneKeyframeImplicitProperties.remove(cssPropertyId);
+            if (!implicitOneKeyframe && isSuitableKeyframeForImplicitValues(keyframe))
+                implicitOneKeyframe = &keyframe;
+        }
+    }
+
     if (!oneKeyframeImplicitProperties.isEmpty())
         addImplicitKeyframe(1, oneKeyframeImplicitProperties, hundredPercentKeyframe(), implicitOneKeyframe);
 }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to