Don't attempt to animate invalid CSS properties
authorbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 13 Dec 2018 20:30:01 +0000 (20:30 +0000)
committerbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 13 Dec 2018 20:30:01 +0000 (20:30 +0000)
https://bugs.webkit.org/show_bug.cgi?id=192630
<rdar://problem/46664433>

Reviewed by Antoine Quint.

Source/WebCore:

Inherited animation properties can cause child elements to think they need to animate CSS properties
that they do not support, leading to nullptr crashes.

Recognize that CSSPropertyInvalid is a potential requested animation property, and handle it
cleanly.

Tests: animations/invalid-property-animation.html

* page/animation/CompositeAnimation.cpp:
(WebCore::CompositeAnimation::updateTransitions):
* svg/SVGAnimateElementBase.cpp:
(WebCore::SVGAnimateElementBase::calculateAnimatedValue):

LayoutTests:

* animations/invalid-property-animation-expected.txt: Added.
* animations/invalid-property-animation.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@239174 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/animations/invalid-property-animation-expected.txt [new file with mode: 0644]
LayoutTests/animations/invalid-property-animation.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/page/animation/CompositeAnimation.cpp

index fc660cd..83074a5 100644 (file)
@@ -1,3 +1,14 @@
+2018-12-13  Brent Fulgham  <bfulgham@apple.com>
+
+        Don't attempt to animate invalid CSS properties
+        https://bugs.webkit.org/show_bug.cgi?id=192630
+        <rdar://problem/46664433>
+
+        Reviewed by Antoine Quint.
+
+        * animations/invalid-property-animation-expected.txt: Added.
+        * animations/invalid-property-animation.html: Added.
+
 2018-12-13  Eric Carlson  <eric.carlson@apple.com>
 
         [MediaStream] Calculate width or height when constraints contain only the other
diff --git a/LayoutTests/animations/invalid-property-animation-expected.txt b/LayoutTests/animations/invalid-property-animation-expected.txt
new file mode 100644 (file)
index 0000000..4fdde57
--- /dev/null
@@ -0,0 +1,3 @@
+The test passes if it does not crash.
+
+
diff --git a/LayoutTests/animations/invalid-property-animation.html b/LayoutTests/animations/invalid-property-animation.html
new file mode 100644 (file)
index 0000000..288955f
--- /dev/null
@@ -0,0 +1,19 @@
+<!DOCTYPE html> 
+<html>
+<head>
+<script>
+function runTest() {
+    if (window.testRunner) {
+        testRunner.dumpAsText(true);
+        internals.updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks();
+    }
+}
+</script>
+</head>
+<body onload="runTest()">
+    <p>The test passes if it does not crash.</p>
+    <button style="transition: width">
+        <embed style="transition-delay: inherit" src="x"></embed>
+    </button>
+</body>
+</html>
index 97464dc..80805ae 100644 (file)
@@ -1,3 +1,24 @@
+2018-12-13  Brent Fulgham  <bfulgham@apple.com>
+
+        Don't attempt to animate invalid CSS properties
+        https://bugs.webkit.org/show_bug.cgi?id=192630
+        <rdar://problem/46664433>
+
+        Reviewed by Antoine Quint.
+
+        Inherited animation properties can cause child elements to think they need to animate CSS properties
+        that they do not support, leading to nullptr crashes.
+
+        Recognize that CSSPropertyInvalid is a potential requested animation property, and handle it
+        cleanly.
+
+        Tests: animations/invalid-property-animation.html
+
+        * page/animation/CompositeAnimation.cpp:
+        (WebCore::CompositeAnimation::updateTransitions):
+        * svg/SVGAnimateElementBase.cpp:
+        (WebCore::SVGAnimateElementBase::calculateAnimatedValue):
+
 2018-12-13  Timothy Hatcher  <timothy@apple.com>
 
         REGRESSION (r230064): Focus rings on webpages are fainter than in native UI.
index b77a6c2..dcebf08 100644 (file)
@@ -115,6 +115,12 @@ void CompositeAnimation::updateTransitions(Element& element, const RenderStyle*
                         continue;
                 }
 
+                if (prop == CSSPropertyInvalid) {
+                    if (!all)
+                        break;
+                    continue;
+                }
+                
                 // ImplicitAnimations are always hashed by actual properties, never animateAll.
                 ASSERT(prop >= firstCSSProperty && prop < (firstCSSProperty + numCSSProperties));