git://git.webkit.org / WebKit-https.git / commitdiff
? search:
summary | shortlog | log | commit | commitdiff | tree
raw | patch | inline | side by side (parent: 46e8d74)
[Web Animations] Crash when setting "animation: none" after clearing an animation...
authorgraouts@webkit.org <graouts@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 24 Jul 2018 19:30:36 +0000 (19:30 +0000)
committergraouts@webkit.org <graouts@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 24 Jul 2018 19:30:36 +0000 (19:30 +0000)
https://bugs.webkit.org/show_bug.cgi?id=187952

Reviewed by Dean Jackson.

Source/WebCore:

Test: webanimations/setting-css-animation-none-after-clearing-effect.html

We need to ensure that the animation we're trying to remove has not had its effect cleared via the
Web Animations API since its creation before trying to check its phase.

* animation/AnimationTimeline.cpp:
(WebCore::AnimationTimeline::cancelOrRemoveDeclarativeAnimation):

LayoutTests:

Add a new test that checks that setting "animation: none" on an element that previously had a valid
CSS animation and for which the effect was set to null does not crash.

* webanimations/setting-css-animation-none-after-clearing-effect-expected.txt: Added.
* webanimations/setting-css-animation-none-after-clearing-effect.html: Added.

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

LayoutTests/ChangeLog patch | blob | history
LayoutTests/webanimations/setting-css-animation-none-after-clearing-effect-expected.txt [new file with mode: 0644] patch | blob
LayoutTests/webanimations/setting-css-animation-none-after-clearing-effect.html [new file with mode: 0644] patch | blob
Source/WebCore/ChangeLog patch | blob | history
Source/WebCore/animation/AnimationTimeline.cpp patch | blob | history

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 8c812a6..5e63f6c 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,5 +1,18 @@
 2018-07-24  Antoine Quint  <graouts@apple.com>
 
+        [Web Animations] Crash when setting "animation: none" after clearing an animation's effect
+        https://bugs.webkit.org/show_bug.cgi?id=187952
+
+        Reviewed by Dean Jackson.
+
+        Add a new test that checks that setting "animation: none" on an element that previously had a valid
+        CSS animation and for which the effect was set to null does not crash.
+
+        * webanimations/setting-css-animation-none-after-clearing-effect-expected.txt: Added.
+        * webanimations/setting-css-animation-none-after-clearing-effect.html: Added.
+
+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>
diff --git a/LayoutTests/webanimations/setting-css-animation-none-after-clearing-effect-expected.txt b/LayoutTests/webanimations/setting-css-animation-none-after-clearing-effect-expected.txt
new file mode 100644 (file)
index 0000000..4550a94
--- /dev/null
+++ b/LayoutTests/webanimations/setting-css-animation-none-after-clearing-effect-expected.txt
@@ -0,0 +1,3 @@
+
+PASS Setting a CSS Animation to 'none' after setting its effect to null does not crash. 
+
diff --git a/LayoutTests/webanimations/setting-css-animation-none-after-clearing-effect.html b/LayoutTests/webanimations/setting-css-animation-none-after-clearing-effect.html
new file mode 100644 (file)
index 0000000..2e918cb
--- /dev/null
+++ b/LayoutTests/webanimations/setting-css-animation-none-after-clearing-effect.html
@@ -0,0 +1,34 @@
+<!DOCTYPE html><!-- webkit-test-runner [ enableWebAnimationsCSSIntegration=true ] -->
+<meta charset=utf-8>
+<title>Crash setting a CSS Animation to "none" 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.");
+
+    animations[0].effect = null;
+
+    target.style.animation = "none";
+}, "Setting a CSS Animation to 'none' after setting its effect to null does not crash.");
+
+</script>
+</body>
\ No newline at end of file
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 2f31df2..f56621c 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,5 +1,20 @@
 2018-07-24  Antoine Quint  <graouts@apple.com>
 
+        [Web Animations] Crash when setting "animation: none" after clearing an animation's effect
+        https://bugs.webkit.org/show_bug.cgi?id=187952
+
+        Reviewed by Dean Jackson.
+
+        Test: webanimations/setting-css-animation-none-after-clearing-effect.html
+
+        We need to ensure that the animation we're trying to remove has not had its effect cleared via the
+        Web Animations API since its creation before trying to check its phase.
+
+        * animation/AnimationTimeline.cpp:
+        (WebCore::AnimationTimeline::cancelOrRemoveDeclarativeAnimation):
+
+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>
diff --git a/Source/WebCore/animation/AnimationTimeline.cpp b/Source/WebCore/animation/AnimationTimeline.cpp
index 7348521..88791a2 100644 (file)
--- a/Source/WebCore/animation/AnimationTimeline.cpp
+++ b/Source/WebCore/animation/AnimationTimeline.cpp
@@ -474,11 +474,15 @@ void AnimationTimeline::updateCSSTransitionsForElement(Element& element, const R
 
 void AnimationTimeline::cancelOrRemoveDeclarativeAnimation(RefPtr<DeclarativeAnimation> animation)
 {
-    auto phase = animation->effect()->phase();
-    if (phase != AnimationEffectReadOnly::Phase::Idle && phase != AnimationEffectReadOnly::Phase::After)
-        animation->cancel();
-    else
-        removeAnimation(animation.releaseNonNull());
+    if (auto* effect = animation->effect()) {
+        auto phase = effect->phase();
+        if (phase != AnimationEffectReadOnly::Phase::Idle && phase != AnimationEffectReadOnly::Phase::After) {
+            animation->cancel();
+            return;
+        }
+    }
+
+    removeAnimation(animation.releaseNonNull());
 }
 
 String AnimationTimeline::description()
Mirror of the WebKit open source project from svn.webkit.org.