[Web Animations] Crash accessing CSSAnimation::bindingsCurrentTime when effect has...
authorgraouts@webkit.org <graouts@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 24 Jul 2018 19:28:41 +0000 (19:28 +0000)
committergraouts@webkit.org <graouts@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 24 Jul 2018 19:28:41 +0000 (19:28 +0000)
https://bugs.webkit.org/show_bug.cgi?id=187950
<rdar://problem/42515747>

Reviewed by Dean Jackson.

Source/WebCore:

Test: webanimations/accessing-current-time-after-clearing-css-animation-effect.html

While a CSSAnimation has an effect created for it by the implementation, the developer may yet manipulate
its effect via the Web Animations API and set it to null. As such, we must not assume it's always non-null.

* animation/CSSAnimation.cpp:
(WebCore::CSSAnimation::bindingsCurrentTime const):

LayoutTests:

Add a new test where we check that the current time of a CSSAnimation can be accessed after setting its effect to null.

* webanimations/accessing-current-time-after-clearing-css-animation-effect-expected.txt: Added.
* webanimations/accessing-current-time-after-clearing-css-animation-effect.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/webanimations/accessing-current-time-after-clearing-css-animation-effect-expected.txt [new file with mode: 0644]
LayoutTests/webanimations/accessing-current-time-after-clearing-css-animation-effect.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/animation/CSSAnimation.cpp

index 8f3c4cd..8c812a6 100644 (file)
@@ -1,3 +1,16 @@
+2018-07-24  Antoine Quint  <graouts@apple.com>
+
+        [Web Animations] Crash accessing CSSAnimation::bindingsCurrentTime when effect has been set to null
+        https://bugs.webkit.org/show_bug.cgi?id=187950
+        <rdar://problem/42515747>
+
+        Reviewed by Dean Jackson.
+
+        Add a new test where we check that the current time of a CSSAnimation can be accessed after setting its effect to null.
+
+        * webanimations/accessing-current-time-after-clearing-css-animation-effect-expected.txt: Added.
+        * webanimations/accessing-current-time-after-clearing-css-animation-effect.html: Added.
+
 2018-07-24  Daniel Bates  <dabates@apple.com>
 
         Cannot view PDF's on my.gov.au: "Refused to load https://my.gov.au/attachment/viewAttachment because it
diff --git a/LayoutTests/webanimations/accessing-current-time-after-clearing-css-animation-effect-expected.txt b/LayoutTests/webanimations/accessing-current-time-after-clearing-css-animation-effect-expected.txt
new file mode 100644 (file)
index 0000000..a44fdaf
--- /dev/null
@@ -0,0 +1,3 @@
+
+PASS Current time is 0 after removing its effect. 
+
diff --git a/LayoutTests/webanimations/accessing-current-time-after-clearing-css-animation-effect.html b/LayoutTests/webanimations/accessing-current-time-after-clearing-css-animation-effect.html
new file mode 100644 (file)
index 0000000..2434255
--- /dev/null
@@ -0,0 +1,35 @@
+<!DOCTYPE html><!-- webkit-test-runner [ enableWebAnimationsCSSIntegration=true ] -->
+<meta charset=utf-8>
+<title>Crash accessing a CSSAnimation's current time after setting its effect to null</title>
+<style>
+    @keyframes animation {
+        from {
+            margin-left: 0px;
+        }
+        to {
+            margin-left: 100px;
+        }
+    }
+</style>
+<body>
+<script src="../resources/testharness.js"></script>
+<script src="../resources/testharnessreport.js"></script>
+<script>
+
+'use strict';
+
+test(t => {
+    const target = document.body.appendChild(document.createElement("div"));
+    target.style.animation = "animation 1s";
+
+    const animations = target.getAnimations();
+    assert_equals(animations.length, 1, "The target element has one animation.");
+
+    const animation = animations[0];
+    animation.effect = null;
+
+    assert_equals(animation.currentTime, 0, "The animation's current time is 0 after removing its effect.");
+}, "Current time is 0 after removing its effect.");
+
+</script>
+</body>
\ No newline at end of file
index 5bcdb20..2f31df2 100644 (file)
@@ -1,3 +1,19 @@
+2018-07-24  Antoine Quint  <graouts@apple.com>
+
+        [Web Animations] Crash accessing CSSAnimation::bindingsCurrentTime when effect has been set to null
+        https://bugs.webkit.org/show_bug.cgi?id=187950
+        <rdar://problem/42515747>
+
+        Reviewed by Dean Jackson.
+
+        Test: webanimations/accessing-current-time-after-clearing-css-animation-effect.html
+
+        While a CSSAnimation has an effect created for it by the implementation, the developer may yet manipulate
+        its effect via the Web Animations API and set it to null. As such, we must not assume it's always non-null.
+
+        * animation/CSSAnimation.cpp:
+        (WebCore::CSSAnimation::bindingsCurrentTime const):
+
 2018-07-24  Zalan Bujtas  <zalan@apple.com>
 
         [LFC][IFC] BlockContainer::establishesInlineFormattingContext should only check the first inflow child.
index 931b3bf..90572e2 100644 (file)
@@ -116,8 +116,10 @@ std::optional<double> CSSAnimation::bindingsCurrentTime() const
 {
     flushPendingStyleChanges();
     auto currentTime = DeclarativeAnimation::bindingsCurrentTime();
-    if (currentTime)
-        return std::max(0.0, std::min(currentTime.value(), effect()->timing()->activeDuration().milliseconds()));
+    if (currentTime) {
+        if (auto* animationEffect = effect())
+            return std::max(0.0, std::min(currentTime.value(), animationEffect->timing()->activeDuration().milliseconds()));
+    }
     return currentTime;
 }