Unreviewed, rolling out r252944.
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 29 Nov 2019 22:17:00 +0000 (22:17 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 29 Nov 2019 22:17:00 +0000 (22:17 +0000)
https://bugs.webkit.org/show_bug.cgi?id=204709

Broke Windows Build (Requested by aakashja_ on #webkit).

Reverted changeset:

"[Web Animations] Forward-filling animations should not
schedule updates while filling"
https://bugs.webkit.org/show_bug.cgi?id=204697
https://trac.webkit.org/changeset/252944

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

LayoutTests/ChangeLog
LayoutTests/webanimations/no-scheduling-while-filling-accelerated-expected.txt [deleted file]
LayoutTests/webanimations/no-scheduling-while-filling-accelerated.html [deleted file]
LayoutTests/webanimations/no-scheduling-while-filling-non-accelerated-expected.txt [deleted file]
LayoutTests/webanimations/no-scheduling-while-filling-non-accelerated.html [deleted file]
Source/WebCore/ChangeLog
Source/WebCore/animation/DocumentTimeline.cpp
Source/WebCore/animation/KeyframeEffect.cpp
Source/WebCore/animation/WebAnimation.cpp
Source/WebCore/animation/WebAnimation.h

index 778f23d..ea7ca87 100644 (file)
@@ -1,3 +1,17 @@
+2019-11-29  Commit Queue  <commit-queue@webkit.org>
+
+        Unreviewed, rolling out r252944.
+        https://bugs.webkit.org/show_bug.cgi?id=204709
+
+        Broke Windows Build (Requested by aakashja_ on #webkit).
+
+        Reverted changeset:
+
+        "[Web Animations] Forward-filling animations should not
+        schedule updates while filling"
+        https://bugs.webkit.org/show_bug.cgi?id=204697
+        https://trac.webkit.org/changeset/252944
+
 2019-11-29  Antoine Quint  <graouts@apple.com>
 
         [Web Animations] Forward-filling animations should not schedule updates while filling
diff --git a/LayoutTests/webanimations/no-scheduling-while-filling-accelerated-expected.txt b/LayoutTests/webanimations/no-scheduling-while-filling-accelerated-expected.txt
deleted file mode 100644 (file)
index f395b60..0000000
+++ /dev/null
@@ -1,3 +0,0 @@
-
-PASS There should not be more than one update made to the timeline after a forward-filling accelerated animation completes. 
-
diff --git a/LayoutTests/webanimations/no-scheduling-while-filling-accelerated.html b/LayoutTests/webanimations/no-scheduling-while-filling-accelerated.html
deleted file mode 100644 (file)
index efee157..0000000
+++ /dev/null
@@ -1,45 +0,0 @@
-<!DOCTYPE html>
-<html>
-<head>
-<style>
-
-#target {
-    width: 100px;
-    height: 100px;
-    background-color: black;
-}
-
-</style>
-</head>
-<body>
-<script src="../resources/testharness.js"></script>
-<script src="../resources/testharnessreport.js"></script>
-<div id="target"></div>
-<script>
-
-function waitNFrames(numberOfFrames, continuation)
-{
-    let elapsedFrames = 0;
-    (function rAFCallback() {
-        if (elapsedFrames++ >= numberOfFrames)
-            continuation();
-        else
-            requestAnimationFrame(rAFCallback);
-    })();
-}
-
-async_test(t => {
-    const keyframes = { transform: ["translateX(50px)", "translateX(100px)"] };
-    const timing = { duration: 50, fill: "forwards" };
-    document.getElementById("target").animate(keyframes, timing).addEventListener("finish", event => {
-        const numberOfTimelineInvalidationsWhenFinished = internals.numberOfAnimationTimelineInvalidations();
-        waitNFrames(3, () => {
-            assert_equals(internals.numberOfAnimationTimelineInvalidations(), numberOfTimelineInvalidationsWhenFinished + 1);
-            t.done();
-        });
-    });
-}, "There should not be more than one update made to the timeline after a forward-filling accelerated animation completes.");
-
-</script>
-</body>
-</html>
diff --git a/LayoutTests/webanimations/no-scheduling-while-filling-non-accelerated-expected.txt b/LayoutTests/webanimations/no-scheduling-while-filling-non-accelerated-expected.txt
deleted file mode 100644 (file)
index aa9e105..0000000
+++ /dev/null
@@ -1,3 +0,0 @@
-
-PASS There should not be any updates made to the timeline after a forward-filling animation completes. 
-
diff --git a/LayoutTests/webanimations/no-scheduling-while-filling-non-accelerated.html b/LayoutTests/webanimations/no-scheduling-while-filling-non-accelerated.html
deleted file mode 100644 (file)
index 087b0ce..0000000
+++ /dev/null
@@ -1,45 +0,0 @@
-<!DOCTYPE html>
-<html>
-<head>
-<style>
-
-#target {
-    width: 100px;
-    height: 100px;
-    background-color: black;
-}
-
-</style>
-</head>
-<body>
-<script src="../resources/testharness.js"></script>
-<script src="../resources/testharnessreport.js"></script>
-<div id="target"></div>
-<script>
-
-function waitNFrames(numberOfFrames, continuation)
-{
-    let elapsedFrames = 0;
-    (function rAFCallback() {
-        if (elapsedFrames++ >= numberOfFrames)
-            continuation();
-        else
-            requestAnimationFrame(rAFCallback);
-    })();
-}
-
-async_test(t => {
-    const keyframes = { marginLeft: ["50px", "100px"] };
-    const timing = { duration: 50, fill: "forwards" };
-    document.getElementById("target").animate(keyframes, timing).addEventListener("finish", event => {
-        const numberOfTimelineInvalidationsWhenFinished = internals.numberOfAnimationTimelineInvalidations();
-        waitNFrames(2, () => {
-            assert_equals(internals.numberOfAnimationTimelineInvalidations(), numberOfTimelineInvalidationsWhenFinished);
-            t.done();
-        });
-    });
-}, "There should not be any updates made to the timeline after a forward-filling animation completes.");
-
-</script>
-</body>
-</html>
index 8856fce..0331a75 100644 (file)
@@ -1,3 +1,17 @@
+2019-11-29  Commit Queue  <commit-queue@webkit.org>
+
+        Unreviewed, rolling out r252944.
+        https://bugs.webkit.org/show_bug.cgi?id=204709
+
+        Broke Windows Build (Requested by aakashja_ on #webkit).
+
+        Reverted changeset:
+
+        "[Web Animations] Forward-filling animations should not
+        schedule updates while filling"
+        https://bugs.webkit.org/show_bug.cgi?id=204697
+        https://trac.webkit.org/changeset/252944
+
 2019-11-29  Charlie Turner  <cturner@igalia.com>
 
         [GStreamer] MediaPlayerPrivateGStreamer style cleanups
index 630c2fa..20a07ee 100644 (file)
@@ -350,15 +350,11 @@ void DocumentTimeline::updateAnimationsAndSendEvents(DOMHighResTimeStamp timesta
     if (m_isSuspended || !m_animationResolutionScheduled || !shouldRunUpdateAnimationsAndSendEventsIgnoringSuspensionState())
         return;
 
-    // Updating animations and sending events may invalidate the timing of some animations, so we must set the m_animationResolutionScheduled
-    // flag to false prior to running that procedure to allow animation with timing model updates to schedule updates.
-    m_animationResolutionScheduled = false;
-
     internalUpdateAnimationsAndSendEvents();
     applyPendingAcceleratedAnimations();
 
-    if (!m_animationResolutionScheduled)
-        scheduleNextTick();
+    m_animationResolutionScheduled = false;
+    scheduleNextTick();
 }
 
 void DocumentTimeline::internalUpdateAnimationsAndSendEvents()
@@ -533,6 +529,13 @@ void DocumentTimeline::scheduleNextTick()
     if (m_animations.isEmpty())
         return;
 
+    for (const auto& animation : m_animations) {
+        if (!animation->isRunningAccelerated()) {
+            scheduleAnimationResolution();
+            return;
+        }
+    }
+
     Seconds scheduleDelay = Seconds::infinity();
 
     for (const auto& animation : m_animations) {
index 6f5ed4a..af30ddc 100644 (file)
@@ -663,10 +663,7 @@ Vector<Strong<JSObject>> KeyframeEffect::getKeyframes(JSGlobalObject& lexicalGlo
 
 ExceptionOr<void> KeyframeEffect::setKeyframes(JSGlobalObject& lexicalGlobalObject, Strong<JSObject>&& keyframesInput)
 {
-    auto processKeyframesResult = processKeyframes(lexicalGlobalObject, WTFMove(keyframesInput));
-    if (!processKeyframesResult.hasException() && animation())
-        animation()->effectTimingDidChange();
-    return processKeyframesResult;
+    return processKeyframes(lexicalGlobalObject, WTFMove(keyframesInput));
 }
 
 ExceptionOr<void> KeyframeEffect::processKeyframes(JSGlobalObject& lexicalGlobalObject, Strong<JSObject>&& keyframesInput)
@@ -1345,9 +1342,6 @@ void KeyframeEffect::updateAcceleratedAnimationState()
 
 void KeyframeEffect::addPendingAcceleratedAction(AcceleratedAction action)
 {
-    if (action == m_lastRecordedAcceleratedAction)
-        return;
-
     if (action == AcceleratedAction::Stop)
         m_pendingAcceleratedActions.clear();
     m_pendingAcceleratedActions.append(action);
@@ -1387,13 +1381,8 @@ void KeyframeEffect::applyPendingAcceleratedActions()
         return;
 
     auto* renderer = this->renderer();
-    if (!renderer || !renderer->isComposited()) {
-        // The renderer may no longer be composited because the accelerated animation ended before we had a chance to update it,
-        // in which case if we asked for the animation to stop, we can discard the current set of accelerated actions.
-        if (m_lastRecordedAcceleratedAction == AcceleratedAction::Stop)
-            m_pendingAcceleratedActions.clear();
+    if (!renderer || !renderer->isComposited())
         return;
-    }
 
     auto pendingAcceleratedActions = m_pendingAcceleratedActions;
     m_pendingAcceleratedActions.clear();
index 732fed7..737da75 100644 (file)
@@ -717,7 +717,7 @@ ExceptionOr<void> WebAnimation::finish()
     return { };
 }
 
-void WebAnimation::timingDidChange(DidSeek didSeek, SynchronouslyNotify synchronouslyNotify, Silently silently)
+void WebAnimation::timingDidChange(DidSeek didSeek, SynchronouslyNotify synchronouslyNotify)
 {
     m_shouldSkipUpdatingFinishedStateWhenResolving = false;
     updateFinishedState(didSeek, synchronouslyNotify);
@@ -727,7 +727,7 @@ void WebAnimation::timingDidChange(DidSeek didSeek, SynchronouslyNotify synchron
         downcast<KeyframeEffect>(*m_effect).animationTimingDidChange();
     }
 
-    if (silently == Silently::No && m_timeline)
+    if (m_timeline)
         m_timeline->animationTimingDidChange(*this);
 };
 
@@ -979,7 +979,7 @@ void WebAnimation::runPendingPlayTask()
         m_readyPromise->resolve(*this);
 
     // 5. Run the procedure to update an animation's finished state for animation with the did seek flag set to false, and the synchronously notify flag set to false.
-    timingDidChange(DidSeek::No, SynchronouslyNotify::No, Silently::Yes);
+    timingDidChange(DidSeek::No, SynchronouslyNotify::No);
 
     invalidateEffect();
 }
@@ -1105,7 +1105,7 @@ void WebAnimation::runPendingPauseTask()
 
     // 6. Run the procedure to update an animation's finished state for animation with the did seek flag set to false, and the
     //    synchronously notify flag set to false.
-    timingDidChange(DidSeek::No, SynchronouslyNotify::No, Silently::Yes);
+    timingDidChange(DidSeek::No, SynchronouslyNotify::No);
 
     invalidateEffect();
 }
@@ -1289,39 +1289,40 @@ ExceptionOr<void> WebAnimation::commitStyles()
 
 Seconds WebAnimation::timeToNextTick() const
 {
-    // Any animation that is pending needs immediate resolution.
+    ASSERT(isRunningAccelerated());
+
     if (pending())
         return 0_s;
 
-    // If we're not running, or time is not advancing for this animation, there's no telling when we'll end.
-    auto playbackRate = effectivePlaybackRate();
-    if (playState() != PlayState::Running || !playbackRate)
+    // If we're not running, there's no telling when we'll end.
+    if (playState() != PlayState::Running)
         return Seconds::infinity();
 
-    auto& effect = *this->effect();
-    auto timing = effect.getBasicTiming();
-    switch (timing.phase) {
-    case AnimationEffectPhase::Before:
-        // The animation is in its "before" phase, in this case we can wait until it enters its "active" phase.
-        return (effect.delay() - timing.localTime.value()) / playbackRate;
-    case AnimationEffectPhase::Active:
-        // Non-accelerated animations in the "active" phase will need to update their animated value at the immediate next opportunity.
-        if (!isRunningAccelerated())
-            return 0_s;
-        // Accelerated CSS Animations need to trigger "animationiteration" events, in this case we can wait until the next iteration.
-        if (isCSSAnimation()) {
-            if (auto iterationProgress = effect.getComputedTiming().simpleIterationProgress)
-                return effect.iterationDuration() * (1 - *iterationProgress);
+    // CSS Animations dispatch events for each iteration, so compute the time until
+    // the end of this iteration. Any other animation only cares about remaning total time.
+    if (isCSSAnimation()) {
+        auto* animationEffect = effect();
+        auto timing = animationEffect->getComputedTiming();
+        // If we're actively running, we need the time until the next iteration.
+        if (auto iterationProgress = timing.simpleIterationProgress)
+            return animationEffect->iterationDuration() * (1 - *iterationProgress);
+
+        // Otherwise we're probably in the before phase waiting to reach our start time.
+        if (auto animationCurrentTime = currentTime()) {
+            // If our current time is negative, we need to be scheduled to be resolved at the inverse
+            // of our current time, unless we fill backwards, in which case we want to invalidate as
+            // soon as possible.
+            auto localTime = animationCurrentTime.value();
+            if (localTime < 0_s)
+                return -localTime;
+            if (localTime < animationEffect->delay())
+                return animationEffect->delay() - localTime;
         }
-        // Accelerated animations in the "active" phase can wait until they ended.
-        return (effect.endTime() - timing.localTime.value()) / playbackRate;
-    case AnimationEffectPhase::After:
-        // The animation is in its after phase, which means it will no longer update its value, so it doens't need a tick.
-        return Seconds::infinity();
-    case AnimationEffectPhase::Idle:
-        ASSERT_NOT_REACHED();
-        return Seconds::infinity();
-    }
+    } else if (auto animationCurrentTime = currentTime())
+        return effect()->endTime() - *animationCurrentTime;
+
+    ASSERT_NOT_REACHED();
+    return Seconds::infinity();
 }
 
 } // namespace WebCore
index da4d8ec..15fad61 100644 (file)
@@ -154,7 +154,7 @@ private:
     enum class AutoRewind : uint8_t { Yes, No };
     enum class TimeToRunPendingTask : uint8_t { NotScheduled, ASAP, WhenReady };
 
-    void timingDidChange(DidSeek, SynchronouslyNotify, Silently = Silently::No);
+    void timingDidChange(DidSeek, SynchronouslyNotify);
     void updateFinishedState(DidSeek, SynchronouslyNotify);
     Seconds effectEndTime() const;
     WebAnimation& readyPromiseResolve();