Unreviewed, rolling out r252455.
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 16 Nov 2019 14:28:55 +0000 (14:28 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 16 Nov 2019 14:28:55 +0000 (14:28 +0000)
https://bugs.webkit.org/show_bug.cgi?id=204272

Broke a layout-test on iOS (Requested by aakashja_ on
#webkit).

Reverted changeset:

"[Web Animations] Retargeted transitions targeting accelerated
properties do not stop the original transition"
https://bugs.webkit.org/show_bug.cgi?id=204116
https://trac.webkit.org/changeset/252455

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

12 files changed:
LayoutTests/ChangeLog
LayoutTests/platform/mac-wk1/imported/w3c/web-platform-tests/dom/events/Event-dispatch-on-disabled-elements-expected.txt [deleted file]
LayoutTests/webanimations/css-transition-in-flight-reversal-accelerated-expected.txt [deleted file]
LayoutTests/webanimations/css-transition-in-flight-reversal-accelerated.html [deleted file]
Source/WebCore/ChangeLog
Source/WebCore/animation/AnimationEffect.h
Source/WebCore/animation/AnimationTimeline.cpp
Source/WebCore/animation/DocumentTimeline.cpp
Source/WebCore/animation/DocumentTimeline.h
Source/WebCore/animation/KeyframeEffect.cpp
Source/WebCore/animation/KeyframeEffect.h
Source/WebCore/animation/WebAnimation.cpp

index af03aca..08fd6ec 100644 (file)
@@ -1,3 +1,18 @@
+2019-11-16  Commit Queue  <commit-queue@webkit.org>
+
+        Unreviewed, rolling out r252455.
+        https://bugs.webkit.org/show_bug.cgi?id=204272
+
+        Broke a layout-test on iOS (Requested by aakashja_ on
+        #webkit).
+
+        Reverted changeset:
+
+        "[Web Animations] Retargeted transitions targeting accelerated
+        properties do not stop the original transition"
+        https://bugs.webkit.org/show_bug.cgi?id=204116
+        https://trac.webkit.org/changeset/252455
+
 2019-11-15  Per Arne Vollan  <pvollan@apple.com>
 
         Layout test animations/no-style-recalc-during-accelerated-animation.html is failing
diff --git a/LayoutTests/platform/mac-wk1/imported/w3c/web-platform-tests/dom/events/Event-dispatch-on-disabled-elements-expected.txt b/LayoutTests/platform/mac-wk1/imported/w3c/web-platform-tests/dom/events/Event-dispatch-on-disabled-elements-expected.txt
deleted file mode 100644 (file)
index 67c33e0..0000000
+++ /dev/null
@@ -1,13 +0,0 @@
-
-Harness Error (TIMEOUT), message = null
-
-PASS Can dispatch untrusted 'click' Events at disabled HTML elements. 
-PASS Can dispatch untrusted Events at disabled HTML elements. 
-PASS Can dispatch CustomEvents at disabled HTML elements. 
-PASS Calling click() on disabled elements must not dispatch events. 
-PASS CSS Transitions transitionrun, transitionstart, transitionend events fire on disabled form elements 
-TIMEOUT CSS Transitions transitioncancel event fires on disabled form elements Test timed out
-NOTRUN CSS Animation animationstart, animationiteration, animationend fire on disabled form elements 
-NOTRUN CSS Animation's animationcancel event fires on disabled form elements 
-NOTRUN Real clicks on disabled elements must not dispatch events. 
-
diff --git a/LayoutTests/webanimations/css-transition-in-flight-reversal-accelerated-expected.txt b/LayoutTests/webanimations/css-transition-in-flight-reversal-accelerated-expected.txt
deleted file mode 100644 (file)
index cb4f160..0000000
+++ /dev/null
@@ -1,4 +0,0 @@
-
-PASS A CSS transition targeting opacity can be reversed in-flight. 
-PASS A CSS transition targeting transform can be reversed in-flight. 
-
diff --git a/LayoutTests/webanimations/css-transition-in-flight-reversal-accelerated.html b/LayoutTests/webanimations/css-transition-in-flight-reversal-accelerated.html
deleted file mode 100644 (file)
index 6da5bfa..0000000
+++ /dev/null
@@ -1,72 +0,0 @@
-<!DOCTYPE html><!-- webkit-test-runner [ experimental:WebAnimationsCSSIntegrationEnabled=true ] -->
-<meta charset="utf-8">
-<title>Reversal of accelerated in-flight CSS Transitions</title>
-<style type="text/css" media="screen">
-
-.target.opacity {
-    transition: opacity 2s linear;
-}
-
-.target.opacity.in-flight {
-    opacity: 0;
-}
-
-.target.transform {
-    transition: transform 2s linear;
-}
-
-.target.transform.in-flight {
-    transform: scale(0);
-}
-
-</style>
-<body>
-<script src="../resources/testharness.js"></script>
-<script src="../resources/testharnessreport.js"></script>
-<script>
-
-'use strict';
-
-function targetTest(propertyName)
-{
-    async_test(test => {
-        const target = document.body.appendChild(document.createElement("div"));
-        target.classList.add("target");
-        target.classList.add(propertyName);
-
-        let initialTransition;
-        let reversedTransition;
-
-        // Start the initial transition.
-        requestAnimationFrame(() => {
-            target.classList.add("in-flight");
-            const animations = target.getAnimations();
-            assert_equals(animations.length, 1, "There is one animation applied to the target after starting the initial transition.");
-
-            initialTransition = animations[0];
-            assert_true(initialTransition instanceof CSSTransition, "There is one animation applied to the target after starting the initial transition.");
-
-            // Wait for the initial transition to start and then another frame before reversing the transition.
-            target.addEventListener("transitionstart", event => {
-                requestAnimationFrame(() => {
-                    target.classList.remove("in-flight");
-                    const animations = target.getAnimations();
-                    assert_equals(animations.length, 1, "There is one animation applied to the target after reversing the initial transition.");
-
-                    reversedTransition = animations[0];
-                    assert_true(reversedTransition instanceof CSSTransition, "There is one animation applied to the target after reversing the initial transition.");
-                    assert_not_equals(initialTransition, reversedTransition, "The animation applied to the target after reversing the initial transition is different than the original transition.");
-
-                    target.remove();
-                    test.done();
-                });
-            });
-        });
-    }, `A CSS transition targeting ${propertyName} can be reversed in-flight.`);
-}
-
-targetTest("opacity");
-targetTest("transform");
-
-</script>
-</body>
\ No newline at end of file
index 7432d6e..d76e7ee 100644 (file)
@@ -1,3 +1,18 @@
+2019-11-16  Commit Queue  <commit-queue@webkit.org>
+
+        Unreviewed, rolling out r252455.
+        https://bugs.webkit.org/show_bug.cgi?id=204272
+
+        Broke a layout-test on iOS (Requested by aakashja_ on
+        #webkit).
+
+        Reverted changeset:
+
+        "[Web Animations] Retargeted transitions targeting accelerated
+        properties do not stop the original transition"
+        https://bugs.webkit.org/show_bug.cgi?id=204116
+        https://trac.webkit.org/changeset/252455
+
 2019-11-15  Chris Dumez  <cdumez@apple.com>
 
         REGRESSION (r252497): Multiple mediacapturefromelement tests are crashing on mac debug
index 8ebb44c..e1eced4 100644 (file)
@@ -61,7 +61,6 @@ public:
     virtual void apply(RenderStyle&) = 0;
     virtual void invalidate() = 0;
     virtual void animationDidSeek() = 0;
-    virtual void animationWasCanceled() = 0;
     virtual void animationSuspensionStateDidChange(bool) = 0;
     virtual void animationTimelineDidChange(AnimationTimeline*) = 0;
 
index 3a92a93..c4dea3c 100644 (file)
@@ -399,29 +399,10 @@ void AnimationTimeline::updateCSSTransitionsForElementAndProperty(Element& eleme
     // https://drafts.csswg.org/css-transitions-1/#before-change-style
     // Define the before-change style as the computed values of all properties on the element as of the previous style change event, except with
     // any styles derived from declarative animations such as CSS Transitions, CSS Animations, and SMIL Animations updated to the current time.
-    bool hasRunningTransition = runningTransitionsByProperty.contains(property);
-    auto& beforeChangeStyle = [&]() -> const RenderStyle& {
-        if (hasRunningTransition && CSSPropertyAnimation::animationOfPropertyIsAccelerated(property)) {
-            // In case we have an accelerated transition running for this element, we need to get its computed style as the before-change style
-            // since otherwise the animated value for that property won't be visible.
-            auto* runningTransition = runningTransitionsByProperty.get(property);
-            if (is<KeyframeEffect>(runningTransition->effect())) {
-                auto& keyframeEffect = *downcast<KeyframeEffect>(runningTransition->effect());
-                if (keyframeEffect.isAccelerated()) {
-                    auto animatedStyle = RenderStyle::clonePtr(currentStyle);
-                    runningTransition->resolve(*animatedStyle);
-                    return *animatedStyle;
-                }
-            }
-        }
+    auto existingAnimation = cssAnimationForElementAndProperty(element, property);
+    const auto& beforeChangeStyle = existingAnimation ? downcast<CSSAnimation>(existingAnimation.get())->unanimatedStyle() : currentStyle;
 
-        if (auto existingAnimation = cssAnimationForElementAndProperty(element, property))
-            return downcast<CSSAnimation>(existingAnimation.get())->unanimatedStyle();
-
-        return currentStyle;
-    }();
-
-    if (!hasRunningTransition
+    if (!runningTransitionsByProperty.contains(property)
         && !CSSPropertyAnimation::propertiesEqual(property, &beforeChangeStyle, &afterChangeStyle)
         && CSSPropertyAnimation::canPropertyBeInterpolated(property, &beforeChangeStyle, &afterChangeStyle)
         && !propertyInStyleMatchesValueForTransitionInMap(property, afterChangeStyle, completedTransitionsByProperty)
@@ -454,7 +435,7 @@ void AnimationTimeline::updateCSSTransitionsForElementAndProperty(Element& eleme
         completedTransitionsByProperty.remove(property);
     }
 
-    hasRunningTransition = runningTransitionsByProperty.contains(property);
+    bool hasRunningTransition = runningTransitionsByProperty.contains(property);
     if ((hasRunningTransition || completedTransitionsByProperty.contains(property)) && !matchingBackingAnimation) {
         // 3. If the element has a running transition or completed transition for the property, and there is not a matching transition-property
         //    value, then implementations must cancel the running transition or remove the completed transition from the set of completed transitions.
index d373a57..1881c80 100644 (file)
@@ -307,13 +307,13 @@ void DocumentTimeline::removeAnimation(WebAnimation& animation)
 {
     AnimationTimeline::removeAnimation(animation);
 
-    if (!shouldRunUpdateAnimationsAndSendEventsIgnoringSuspensionState())
+    if (m_animations.isEmpty())
         unscheduleAnimationResolution();
 }
 
 void DocumentTimeline::scheduleAnimationResolution()
 {
-    if (m_isSuspended || m_animationResolutionScheduled || !shouldRunUpdateAnimationsAndSendEventsIgnoringSuspensionState())
+    if (m_isSuspended || m_animations.isEmpty() || m_animationResolutionScheduled)
         return;
 
     if (!m_document || !m_document->page())
@@ -329,11 +329,6 @@ void DocumentTimeline::unscheduleAnimationResolution()
     m_animationResolutionScheduled = false;
 }
 
-bool DocumentTimeline::shouldRunUpdateAnimationsAndSendEventsIgnoringSuspensionState() const
-{
-    return !m_animations.isEmpty() || !m_acceleratedAnimationsPendingRunningStateChange.isEmpty();
-}
-
 void DocumentTimeline::updateAnimationsAndSendEvents(DOMHighResTimeStamp timestamp)
 {
     // We need to freeze the current time even if no animation is running.
@@ -342,7 +337,7 @@ void DocumentTimeline::updateAnimationsAndSendEvents(DOMHighResTimeStamp timesta
     if (!m_isSuspended)
         cacheCurrentTime(timestamp);
 
-    if (m_isSuspended || !m_animationResolutionScheduled || !shouldRunUpdateAnimationsAndSendEventsIgnoringSuspensionState())
+    if (m_isSuspended || m_animations.isEmpty() || !m_animationResolutionScheduled)
         return;
 
     internalUpdateAnimationsAndSendEvents();
index 2c35d24..7c481f2 100644 (file)
@@ -99,7 +99,6 @@ private:
     void scheduleNextTick();
     void removeReplacedAnimations();
     bool animationCanBeRemoved(WebAnimation&);
-    bool shouldRunUpdateAnimationsAndSendEventsIgnoringSuspensionState() const;
 
     Timer m_tickScheduleTimer;
     GenericTaskQueue<Timer> m_currentTimeClearingTaskQueue;
index 86590d1..5ebe116 100644 (file)
@@ -1334,12 +1334,6 @@ void KeyframeEffect::animationDidSeek()
         addPendingAcceleratedAction(AcceleratedAction::Seek);
 }
 
-void KeyframeEffect::animationWasCanceled()
-{
-    if (m_shouldRunAccelerated && isRunningAccelerated())
-        addPendingAcceleratedAction(AcceleratedAction::Stop);
-}
-
 void KeyframeEffect::animationSuspensionStateDidChange(bool animationIsSuspended)
 {
     if (m_shouldRunAccelerated)
index 444d23f..3cd4879 100644 (file)
@@ -113,7 +113,6 @@ public:
     void apply(RenderStyle&) override;
     void invalidate() override;
     void animationDidSeek() final;
-    void animationWasCanceled() final;
     void animationSuspensionStateDidChange(bool) final;
     void animationTimelineDidChange(AnimationTimeline*) final;
     void applyPendingAcceleratedActions();
index b3f746d..b91184e 100644 (file)
@@ -614,9 +614,6 @@ void WebAnimation::cancel(Silently silently)
     timingDidChange(DidSeek::No, SynchronouslyNotify::No);
 
     invalidateEffect();
-
-    if (m_effect)
-        m_effect->animationWasCanceled();
 }
 
 void WebAnimation::enqueueAnimationPlaybackEvent(const AtomString& type, Optional<Seconds> currentTime, Optional<Seconds> timelineTime)