[Web Animations] Ensure setting the hold time invalidates the timing …

…model

https://bugs.webkit.org/show_bug.cgi?id=183136

Reviewed by Dean Jackson.

LayoutTests/imported/w3c:

Update test expectations with progressions.

* web-platform-tests/css/css-multicol/multicol-gap-animation-001-expected.txt:
* web-platform-tests/web-animations/animation-model/keyframe-effects/effect-value-iteration-composite-operation-expected.txt:
* web-platform-tests/web-animations/interfaces/Animation/cancel-expected.txt:
* web-platform-tests/web-animations/interfaces/Animation/finish-expected.txt:
* web-platform-tests/web-animations/interfaces/KeyframeEffect/iterationComposite-expected.txt:

Source/WebCore:

We used to always set the m_holdTime member variable directly, but the computation of the currentTime
depends on the value of m_holdTime, so setting the hold time should invalidate the timing model as well
as setting the m_holdTime member variable. In this patch we add a new setHoldTime() private method that
sets the member variable and invalidates the timing model.

* animation/WebAnimation.cpp:
(WebCore::WebAnimation::setTimeline):
(WebCore::WebAnimation::setHoldTime):
(WebCore::WebAnimation::silentlySetCurrentTime):
(WebCore::WebAnimation::setCurrentTime):
(WebCore::WebAnimation::cancel):
(WebCore::WebAnimation::finish):
(WebCore::WebAnimation::updateFinishedState):
(WebCore::WebAnimation::play):
(WebCore::WebAnimation::runPendingPlayTask):
(WebCore::WebAnimation::pause):
(WebCore::WebAnimation::runPendingPauseTask):
* animation/WebAnimation.h:

Canonical link: https://commits.webkit.org/198850@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@229030 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/imported/w3c/ChangeLog

@@ -1,3 +1,18 @@
+2018-02-26  Antoine Quint  <graouts@apple.com>
+
+        [Web Animations] Ensure setting the hold time invalidates the timing model
+        https://bugs.webkit.org/show_bug.cgi?id=183136
+
+        Reviewed by Dean Jackson.
+
+        Update test expectations with progressions.
+
+        * web-platform-tests/css/css-multicol/multicol-gap-animation-001-expected.txt:
+        * web-platform-tests/web-animations/animation-model/keyframe-effects/effect-value-iteration-composite-operation-expected.txt:
+        * web-platform-tests/web-animations/interfaces/Animation/cancel-expected.txt:
+        * web-platform-tests/web-animations/interfaces/Animation/finish-expected.txt:
+        * web-platform-tests/web-animations/interfaces/KeyframeEffect/iterationComposite-expected.txt:
+
 2018-02-26  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r226745.

LayoutTests/imported/w3c/web-platform-tests/css/css-multicol/multicol-gap-animation-001-expected.txt

@@ -1,3 +1,3 @@
 
-FAIL column-gap property is animatable assert_equals: expected "150px" but got "100px"
+PASS column-gap property is animatable 
 

LayoutTests/imported/w3c/web-platform-tests/web-animations/animation-model/keyframe-effects/effect-value-iteration-composite-operation-expected.txt

@@ -1,15 +1,15 @@
 
 FAIL iteration composition of discrete type animation (align-content) assert_equals: Animated align-content style at 50s of the first iteration expected "flex-end" but got "normal"
-FAIL iteration composition of <length> type animation assert_equals: Animated margin-left style at 50s of the first iteration expected "5px" but got "0px"
-FAIL iteration composition of <percentage> type animation assert_equals: Animated width style at 50s of the first iteration expected "25px" but got "0px"
-FAIL iteration composition of <color> type animation assert_equals: Animated color style at 50s of the first iteration expected "rgb(60, 60, 60)" but got "rgb(0, 0, 0)"
-FAIL iteration composition of <color> type animation that green component is decreasing assert_equals: Animated color style at 50s of the first iteration expected "rgb(30, 90, 30)" but got "rgb(0, 120, 0)"
+FAIL iteration composition of <length> type animation assert_equals: Animated margin-left style at 0s of the third iteration expected "20px" but got "0px"
+FAIL iteration composition of <percentage> type animation assert_equals: Animated width style at 0s of the third iteration expected "100px" but got "0px"
+FAIL iteration composition of <color> type animation assert_equals: Animated color style at 0s of the third iteration expected "rgb(240, 240, 240)" but got "rgb(0, 0, 0)"
+FAIL iteration composition of <color> type animation that green component is decreasing assert_equals: Animated color style at 0s of the third iteration expected "rgb(120, 240, 120)" but got "rgb(0, 120, 0)"
 FAIL iteration composition of <number> type animation assert_equals: Animated flex-grow style at 50s of the first iteration expected "5" but got "0"
 FAIL iteration composition of <shape> type animation assert_equals: Animated clip style at 50s of the first iteration expected "rect(5px, 5px, 5px, 5px)" but got "auto"
-FAIL iteration composition of <calc()> value animation assert_equals: Animated calc width style at 50s of the first iteration expected "5px" but got "0px"
-FAIL iteration composition of <calc()> value animation that the values can'tbe reduced assert_equals: Animated calc width style at 50s of the first iteration expected "10px" but got "0px"
+FAIL iteration composition of <calc()> value animation assert_equals: Animated calc width style at 0s of the third iteration expected "20px" but got "0px"
+FAIL iteration composition of <calc()> value animation that the values can'tbe reduced assert_equals: Animated calc width style at 0s of the third iteration expected "40px" but got "0px"
 FAIL iteration composition of opacity animation assert_equals: Animated opacity style at 50s of the first iteration expected "0.2" but got "0.20000000298023224"
-FAIL iteration composition of box-shadow animation assert_equals: Animated box-shadow style at 50s of the first iteration expected "rgb(60, 60, 60) 5px 5px 5px 0px" but got "rgb(0, 0, 0) 0px 0px 0px 0px"
+FAIL iteration composition of box-shadow animation assert_equals: Animated box-shadow style at 0s of the third iteration expected "rgb(240, 240, 240) 20px 20px 20px 0px" but got "rgb(0, 0, 0) 0px 0px 0px 0px"
 FAIL iteration composition of filter blur animation assert_equals: Animated filter blur style at 50s of the first iteration expected "blur(5px)" but got "blur(10px)"
 FAIL iteration composition of filter brightness for different unit animation assert_equals: Animated filter brightness style at 50s of the first iteration expected "brightness(1.4)" but got "brightness(1.8)"
 FAIL iteration composition of filter brightness animation assert_equals: Animated filter brightness style at 50s of the first iteration expected "brightness(0.5)" but got "brightness(1)"
@@ -28,7 +28,7 @@ FAIL iteration composition of transform list animation whose order is mismatched
 FAIL iteration composition of transform from none to translate assert_regexp_match: Actual value is not a matrix expected object "/^matrix(?:3d)*\((.+)\)/" but got "none"
 FAIL iteration composition of transform of matrix3d function assert_regexp_match: Actual value is not a matrix expected object "/^matrix(?:3d)*\((.+)\)/" but got "none"
 FAIL iteration composition of transform of rotate3d function assert_regexp_match: Actual value is not a matrix expected object "/^matrix(?:3d)*\((.+)\)/" but got "none"
-FAIL iteration composition starts with non-zero value animation assert_equals: Animated margin-left style at 50s of the first iteration expected "15px" but got "10px"
-FAIL iteration composition with negative final value animation assert_equals: Animated margin-left style at 50s of the first iteration expected "0px" but got "10px"
-FAIL duration changes with an iteration composition operation of accumulate assert_equals: Animated style at 50s of the third iteration expected "25px" but got "0px"
+FAIL iteration composition starts with non-zero value animation assert_equals: Animated margin-left style at 0s of the third iteration expected "50px" but got "10px"
+FAIL iteration composition with negative final value animation assert_equals: Animated margin-left style at 0s of the third iteration expected "-10px" but got "10px"
+FAIL duration changes with an iteration composition operation of accumulate assert_equals: Animated style at 50s of the third iteration expected "25px" but got "5px"
 

LayoutTests/imported/w3c/web-platform-tests/web-animations/interfaces/Animation/cancel-expected.txt

@@ -1,5 +1,5 @@
 
 FAIL Animated style is cleared after calling Animation.cancel() assert_not_equals: transform style is animated before cancelling got disallowed value "none"
-FAIL After cancelling an animation, it can still be seeked assert_equals: margin-left style is updated when cancelled animation is seeked expected "150px" but got "0px"
+PASS After cancelling an animation, it can still be seeked 
 PASS After cancelling an animation, it can still be re-used 
 

LayoutTests/imported/w3c/web-platform-tests/web-animations/interfaces/Animation/finish-expected.txt

@@ -5,15 +5,15 @@ PASS Test exceptions when finishing non-running animation
 PASS Test exceptions when finishing infinite animation 
 PASS Test finishing of animation 
 PASS Test finishing of animation with a current time past the effect end 
-TIMEOUT Test finishing of reversed animation Test timed out
-NOTRUN Test finishing of reversed animation with a current time less than zero 
-NOTRUN Test finish() while paused 
+FAIL Test finishing of reversed animation assert_equals: After finishing a reversed animation the currentTime should be set to zero expected 0 but got -0
+PASS Test finishing of reversed animation with a current time less than zero 
+PASS Test finish() while paused 
 PASS Test finish() while pause-pending with positive playbackRate 
 PASS Test finish() while pause-pending with negative playbackRate 
 PASS Test finish() while play-pending 
-NOTRUN Test finish() during aborted pause 
-NOTRUN Test resetting of computed style 
-NOTRUN Test finish() resolves finished promise synchronously 
-NOTRUN Test finish() resolves finished promise synchronously with an animation without a target 
+PASS Test finish() during aborted pause 
+PASS Test resetting of computed style 
+PASS Test finish() resolves finished promise synchronously 
+TIMEOUT Test finish() resolves finished promise synchronously with an animation without a target Test timed out
 NOTRUN Test normally finished animation resolves finished promise synchronously with an animation without a target 
 

LayoutTests/imported/w3c/web-platform-tests/web-animations/interfaces/KeyframeEffect/iterationComposite-expected.txt

@@ -1,3 +1,3 @@
 
-FAIL iterationComposite can be updated while an animation is in progress assert_equals: Animated style at 50s of the third iteration expected "25px" but got "0px"
+FAIL iterationComposite can be updated while an animation is in progress assert_equals: Animated style at 50s of the third iteration expected "25px" but got "5px"
 

Source/WebCore/ChangeLog

@@ -1,3 +1,29 @@
+2018-02-26  Antoine Quint  <graouts@apple.com>
+
+        [Web Animations] Ensure setting the hold time invalidates the timing model
+        https://bugs.webkit.org/show_bug.cgi?id=183136
+
+        Reviewed by Dean Jackson.
+
+        We used to always set the m_holdTime member variable directly, but the computation of the currentTime
+        depends on the value of m_holdTime, so setting the hold time should invalidate the timing model as well
+        as setting the m_holdTime member variable. In this patch we add a new setHoldTime() private method that
+        sets the member variable and invalidates the timing model.
+
+        * animation/WebAnimation.cpp:
+        (WebCore::WebAnimation::setTimeline):
+        (WebCore::WebAnimation::setHoldTime):
+        (WebCore::WebAnimation::silentlySetCurrentTime):
+        (WebCore::WebAnimation::setCurrentTime):
+        (WebCore::WebAnimation::cancel):
+        (WebCore::WebAnimation::finish):
+        (WebCore::WebAnimation::updateFinishedState):
+        (WebCore::WebAnimation::play):
+        (WebCore::WebAnimation::runPendingPlayTask):
+        (WebCore::WebAnimation::pause):
+        (WebCore::WebAnimation::runPendingPauseTask):
+        * animation/WebAnimation.h:
+
 2018-02-26  Youenn Fablet  <youenn@apple.com>
 
         MessagePort is not always destroyed in the right thread

Source/WebCore/animation/WebAnimation.cpp

@@ -134,7 +134,7 @@ void WebAnimation::setTimeline(RefPtr<AnimationTimeline>&& timeline)
 
     // 4. If the animation start time of animation is resolved, make animation’s hold time unresolved.
     if (startTime())
-        m_holdTime = std::nullopt;
+        setHoldTime(std::nullopt);
 
     if (m_timeline)
         m_timeline->removeAnimation(*this);
@@ -174,6 +174,15 @@ void WebAnimation::effectTargetDidChange(Element* previousTarget, Element* newTa
         m_timeline->animationWasAddedToElement(*this, *newTarget);
 }
 
+void WebAnimation::setHoldTime(std::optional<Seconds> holdTime)
+{
+    if (m_holdTime == holdTime)
+        return;
+
+    m_holdTime = holdTime;
+    timingModelDidChange();
+}
+
 std::optional<double> WebAnimation::bindingsStartTime() const
 {
     if (!m_startTime)
@@ -270,7 +279,7 @@ ExceptionOr<void> WebAnimation::silentlySetCurrentTime(std::optional<Seconds> se
     // Otherwise, set animation's start time to the result of evaluating timeline time - (seek time / playback rate)
     // where timeline time is the current time value of timeline associated with animation.
     if (m_holdTime || !startTime() || !m_timeline || !m_timeline->currentTime() || !m_playbackRate)
-        m_holdTime = seekTime;
+        setHoldTime(seekTime);
     else
         setStartTime(m_timeline->currentTime().value() - (seekTime.value() / m_playbackRate));
 
@@ -297,7 +306,7 @@ ExceptionOr<void> WebAnimation::setCurrentTime(std::optional<Seconds> seekTime)
     // 2. If animation has a pending pause task, synchronously complete the pause operation by performing the following steps:
     if (hasPendingPauseTask()) {
         // 1. Set animation's hold time to seek time.
-        m_holdTime = seekTime;
+        setHoldTime(seekTime);
         // 2. Make animation's start time unresolved.
         setStartTime(std::nullopt);
         // 3. Cancel the pending pause task.
@@ -411,7 +420,7 @@ void WebAnimation::cancel()
     }
 
     // 2. Make animation's hold time unresolved.
-    m_holdTime = std::nullopt;
+    setHoldTime(std::nullopt);
 
     // 3. Make animation's start time unresolved.
     setStartTime(std::nullopt);
@@ -489,7 +498,7 @@ ExceptionOr<void> WebAnimation::finish()
     // 5. If there is a pending pause task and start time is resolved,
     if (hasPendingPauseTask() && startTime()) {
         // 1. Let the hold time be unresolved.
-        m_holdTime = std::nullopt;
+        setHoldTime(std::nullopt);
         // 2. Cancel the pending pause task.
         setTimeToRunPendingPauseTask(TimeToRunPendingTask::NotScheduled);
         // 3. Resolve the current ready promise of animation with animation.
@@ -528,22 +537,22 @@ void WebAnimation::updateFinishedState(DidSeek didSeek, SynchronouslyNotify sync
             // If animation playback rate > 0 and unconstrained current time is greater than or equal to target effect end,
             // If did seek is true, let the hold time be the value of unconstrained current time.
             if (didSeek == DidSeek::Yes)
-                m_holdTime = unconstrainedCurrentTime;
+                setHoldTime(unconstrainedCurrentTime);
             // If did seek is false, let the hold time be the maximum value of previous current time and target effect end. If the previous current time is unresolved, let the hold time be target effect end.
             else if (!m_previousCurrentTime)
-                m_holdTime = endTime;
+                setHoldTime(endTime);
             else
-                m_holdTime = std::max(m_previousCurrentTime.value(), endTime);
+                setHoldTime(std::max(m_previousCurrentTime.value(), endTime));
         } else if (m_playbackRate < 0 && unconstrainedCurrentTime <= 0_s) {
             // If animation playback rate < 0 and unconstrained current time is less than or equal to 0,
             // If did seek is true, let the hold time be the value of unconstrained current time.
             if (didSeek == DidSeek::Yes)
-                m_holdTime = unconstrainedCurrentTime;
+                setHoldTime(unconstrainedCurrentTime);
             // If did seek is false, let the hold time be the minimum value of previous current time and zero. If the previous current time is unresolved, let the hold time be zero.
             else if (!m_previousCurrentTime)
-                m_holdTime = 0_s;
+                setHoldTime(0_s);
             else
-                m_holdTime = std::min(m_previousCurrentTime.value(), 0_s);
+                setHoldTime(std::min(m_previousCurrentTime.value(), 0_s));
         } else if (m_playbackRate && m_timeline && m_timeline->currentTime()) {
             // If animation playback rate ≠ 0, and animation is associated with an active timeline,
             // Perform the following steps:
@@ -552,7 +561,7 @@ void WebAnimation::updateFinishedState(DidSeek didSeek, SynchronouslyNotify sync
             if (didSeek == DidSeek::Yes && m_holdTime)
                 setStartTime(m_timeline->currentTime().value() - (m_holdTime.value() / m_playbackRate));
             // 2. Let the hold time be unresolved.
-            m_holdTime = std::nullopt;
+            setHoldTime(std::nullopt);
         }
     }
 
@@ -653,7 +662,7 @@ ExceptionOr<void> WebAnimation::play(AutoRewind autoRewind)
         //     - current time < zero, or
         //     - current time ≥ target effect end,
         // Set animation's hold time to zero.
-        m_holdTime = 0_s;
+        setHoldTime(0_s);
     } else if (m_playbackRate < 0 && autoRewind == AutoRewind::Yes && (!localTime || localTime.value() <= 0_s || localTime.value() > endTime)) {
         // If animation playback rate < 0, the auto-rewind flag is true and either animation's:
         //     - current time is unresolved, or
@@ -662,11 +671,11 @@ ExceptionOr<void> WebAnimation::play(AutoRewind autoRewind)
         // If target effect end is positive infinity, throw an InvalidStateError and abort these steps.
         if (endTime == Seconds::infinity())
             return Exception { InvalidStateError };
-        m_holdTime = endTime;
+        setHoldTime(endTime);
     } else if (!m_playbackRate && !localTime) {
         // If animation playback rate = 0 and animation's current time is unresolved,
         // Set animation's hold time to zero.
-        m_holdTime = 0_s;
+        setHoldTime(0_s);
     }
 
     // 4. If animation has a pending play task or a pending pause task,
@@ -723,7 +732,7 @@ void WebAnimation::runPendingPlayTask()
             newStartTime -= m_holdTime.value() / m_playbackRate;
         // 2. If animation's playback rate is not 0, make animation's hold time unresolved.
         if (m_playbackRate)
-            m_holdTime = std::nullopt;
+            setHoldTime(std::nullopt);
         // 3. Set the animation start time of animation to new start time.
         setStartTime(newStartTime);
     }
@@ -754,13 +763,13 @@ ExceptionOr<void> WebAnimation::pause()
     if (!localTime) {
         if (m_playbackRate >= 0) {
             // If animation's playback rate is ≥ 0, let animation's hold time be zero.
-            m_holdTime = 0_s;
+            setHoldTime(0_s);
         } else if (effectEndTime() == Seconds::infinity()) {
             // Otherwise, if target effect end for animation is positive infinity, throw an InvalidStateError and abort these steps.
             return Exception { InvalidStateError };
         } else {
             // Otherwise, let animation's hold time be target effect end.
-            m_holdTime = effectEndTime();
+            setHoldTime(effectEndTime());
         }
     }
 
@@ -842,7 +851,7 @@ void WebAnimation::runPendingPauseTask()
     //    Note: The hold time might be already set if the animation is finished, or if the animation is pending, waiting to begin
     //    playback. In either case we want to preserve the hold time as we enter the paused state.
     if (animationStartTime && !m_holdTime)
-        m_holdTime = (readyTime.value() - animationStartTime.value()) * m_playbackRate;
+        setHoldTime((readyTime.value() - animationStartTime.value()) * m_playbackRate);
 
     // 3. Make animation's start time unresolved.
     setStartTime(std::nullopt);

Source/WebCore/animation/WebAnimation.h

@@ -118,6 +118,7 @@ class WebAnimation final : public RefCounted<WebAnimation>, public EventTargetWi
     Seconds effectEndTime() const;
     WebAnimation& readyPromiseResolve();
     WebAnimation& finishedPromiseResolve();
+    void setHoldTime(std::optional<Seconds>);
     std::optional<Seconds> currentTime(RespectHoldTime) const;
     ExceptionOr<void> silentlySetCurrentTime(std::optional<Seconds>);
     void finishNotificationSteps();