Unreviewed, rolling out r252526.
authorphiln@webkit.org <philn@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 16 Nov 2019 17:11:44 +0000 (17:11 +0000)
committerphiln@webkit.org <philn@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 16 Nov 2019 17:11:44 +0000 (17:11 +0000)
broke iOS and mac builds

Reverted changeset:

"Unreviewed, rolling out r252455."
https://bugs.webkit.org/show_bug.cgi?id=204272
https://trac.webkit.org/changeset/252526

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@252527 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 [new file with mode: 0644]
LayoutTests/webanimations/css-transition-in-flight-reversal-accelerated-expected.txt [new file with mode: 0644]
LayoutTests/webanimations/css-transition-in-flight-reversal-accelerated.html [new file with mode: 0644]
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 08fd6ec..5dd28d1 100644 (file)
@@ -1,3 +1,15 @@
+2019-11-16  Philippe Normand  <pnormand@igalia.com>
+
+        Unreviewed, rolling out r252526.
+
+        broke iOS and mac builds
+
+        Reverted changeset:
+
+        "Unreviewed, rolling out r252455."
+        https://bugs.webkit.org/show_bug.cgi?id=204272
+        https://trac.webkit.org/changeset/252526
+
 2019-11-16  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r252455.
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
new file mode 100644 (file)
index 0000000..67c33e0
--- /dev/null
@@ -0,0 +1,13 @@
+
+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
new file mode 100644 (file)
index 0000000..cb4f160
--- /dev/null
@@ -0,0 +1,4 @@
+
+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
new file mode 100644 (file)
index 0000000..6da5bfa
--- /dev/null
@@ -0,0 +1,72 @@
+<!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 d76e7ee..6b24d68 100644 (file)
@@ -1,3 +1,15 @@
+2019-11-16  Philippe Normand  <pnormand@igalia.com>
+
+        Unreviewed, rolling out r252526.
+
+        broke iOS and mac builds
+
+        Reverted changeset:
+
+        "Unreviewed, rolling out r252455."
+        https://bugs.webkit.org/show_bug.cgi?id=204272
+        https://trac.webkit.org/changeset/252526
+
 2019-11-16  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r252455.
index e1eced4..8ebb44c 100644 (file)
@@ -61,6 +61,7 @@ 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 c4dea3c..3a92a93 100644 (file)
@@ -399,10 +399,29 @@ 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.
-    auto existingAnimation = cssAnimationForElementAndProperty(element, property);
-    const auto& beforeChangeStyle = existingAnimation ? downcast<CSSAnimation>(existingAnimation.get())->unanimatedStyle() : currentStyle;
+    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;
+                }
+            }
+        }
 
-    if (!runningTransitionsByProperty.contains(property)
+        if (auto existingAnimation = cssAnimationForElementAndProperty(element, property))
+            return downcast<CSSAnimation>(existingAnimation.get())->unanimatedStyle();
+
+        return currentStyle;
+    }();
+
+    if (!hasRunningTransition
         && !CSSPropertyAnimation::propertiesEqual(property, &beforeChangeStyle, &afterChangeStyle)
         && CSSPropertyAnimation::canPropertyBeInterpolated(property, &beforeChangeStyle, &afterChangeStyle)
         && !propertyInStyleMatchesValueForTransitionInMap(property, afterChangeStyle, completedTransitionsByProperty)
@@ -435,7 +454,7 @@ void AnimationTimeline::updateCSSTransitionsForElementAndProperty(Element& eleme
         completedTransitionsByProperty.remove(property);
     }
 
-    bool hasRunningTransition = runningTransitionsByProperty.contains(property);
+    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 1881c80..d373a57 100644 (file)
@@ -307,13 +307,13 @@ void DocumentTimeline::removeAnimation(WebAnimation& animation)
 {
     AnimationTimeline::removeAnimation(animation);
 
-    if (m_animations.isEmpty())
+    if (!shouldRunUpdateAnimationsAndSendEventsIgnoringSuspensionState())
         unscheduleAnimationResolution();
 }
 
 void DocumentTimeline::scheduleAnimationResolution()
 {
-    if (m_isSuspended || m_animations.isEmpty() || m_animationResolutionScheduled)
+    if (m_isSuspended || m_animationResolutionScheduled || !shouldRunUpdateAnimationsAndSendEventsIgnoringSuspensionState())
         return;
 
     if (!m_document || !m_document->page())
@@ -329,6 +329,11 @@ 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.
@@ -337,7 +342,7 @@ void DocumentTimeline::updateAnimationsAndSendEvents(DOMHighResTimeStamp timesta
     if (!m_isSuspended)
         cacheCurrentTime(timestamp);
 
-    if (m_isSuspended || m_animations.isEmpty() || !m_animationResolutionScheduled)
+    if (m_isSuspended || !m_animationResolutionScheduled || !shouldRunUpdateAnimationsAndSendEventsIgnoringSuspensionState())
         return;
 
     internalUpdateAnimationsAndSendEvents();
index 7c481f2..2c35d24 100644 (file)
@@ -99,6 +99,7 @@ private:
     void scheduleNextTick();
     void removeReplacedAnimations();
     bool animationCanBeRemoved(WebAnimation&);
+    bool shouldRunUpdateAnimationsAndSendEventsIgnoringSuspensionState() const;
 
     Timer m_tickScheduleTimer;
     GenericTaskQueue<Timer> m_currentTimeClearingTaskQueue;
index 5ebe116..86590d1 100644 (file)
@@ -1334,6 +1334,12 @@ 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 3cd4879..444d23f 100644 (file)
@@ -113,6 +113,7 @@ 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 b91184e..b3f746d 100644 (file)
@@ -614,6 +614,9 @@ 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)