Title: [225928] trunk
- Revision
- 225928
- Author
- [email protected]
- Date
- 2017-12-14 14:13:45 -0800 (Thu, 14 Dec 2017)
Log Message
[Web Animations] Bring timeline and currentTime setters closer to compliance
https://bugs.webkit.org/show_bug.cgi?id=180834
Reviewed by Dean Jackson.
Source/WebCore:
Now that we've added support for the concept of a hold time, pending tasks
and updating the finished state, adopt those in places we had already implemented
but weren't fully compliant.
Web Platform Tests cover these behaviors, but we're currently failing those tests
due to lacking an implementation for Element.animate().
* animation/WebAnimation.cpp:
(WebCore::WebAnimation::setTimeline): Add some spec comments to clarify the code behavior
and implement step 4 of the "setting the timeline" procedure where we reset the hold time
to an unresolved value if the start time is resolved, as well as step 5 where we update the
finished state. Finally, we also ensure we update the pending tasks as the ready state is
dependent on a timeline being set.
(WebCore::WebAnimation::bindingsStartTime const): Invert the way we test for an unresolved
value to match prior review comments by Dean Jackson.
(WebCore::WebAnimation::setBindingsStartTime): Use a boolean check rather than checking
equality with std::nullopt.
(WebCore::WebAnimation::setBindingsCurrentTime): Do not raise an exception when setting
an unresolved time.
LayoutTests:
Remove a test clause which tested a behavior that is not part of the spec.
* http/wpt/wk-web-animations/timing-model/animation-current-time.html:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (225927 => 225928)
--- trunk/LayoutTests/ChangeLog 2017-12-14 22:12:37 UTC (rev 225927)
+++ trunk/LayoutTests/ChangeLog 2017-12-14 22:13:45 UTC (rev 225928)
@@ -1,5 +1,16 @@
2017-12-14 Antoine Quint <[email protected]>
+ [Web Animations] Bring timeline and currentTime setters closer to compliance
+ https://bugs.webkit.org/show_bug.cgi?id=180834
+
+ Reviewed by Dean Jackson.
+
+ Remove a test clause which tested a behavior that is not part of the spec.
+
+ * http/wpt/wk-web-animations/timing-model/animation-current-time.html:
+
+2017-12-14 Antoine Quint <[email protected]>
+
[Web Animations] Implement the cancel() method on Animation
https://bugs.webkit.org/show_bug.cgi?id=180830
<rdar://problem/36055816>
Modified: trunk/LayoutTests/http/wpt/wk-web-animations/timing-model/animation-current-time-expected.txt (225927 => 225928)
--- trunk/LayoutTests/http/wpt/wk-web-animations/timing-model/animation-current-time-expected.txt 2017-12-14 22:12:37 UTC (rev 225927)
+++ trunk/LayoutTests/http/wpt/wk-web-animations/timing-model/animation-current-time-expected.txt 2017-12-14 22:13:45 UTC (rev 225928)
@@ -2,5 +2,4 @@
PASS An animation's currentTime is null when the timeline has no currentTime set.
PASS An animation accounts for its start time and document current time when computing its current time.
PASS Setting the current time should adjust the start time.
-PASS Setting the current time to null should throw a TypeError.
Modified: trunk/LayoutTests/http/wpt/wk-web-animations/timing-model/animation-current-time.html (225927 => 225928)
--- trunk/LayoutTests/http/wpt/wk-web-animations/timing-model/animation-current-time.html 2017-12-14 22:12:37 UTC (rev 225927)
+++ trunk/LayoutTests/http/wpt/wk-web-animations/timing-model/animation-current-time.html 2017-12-14 22:13:45 UTC (rev 225928)
@@ -36,10 +36,5 @@
assert_equals(animation.startTime, 2000);
}, "Setting the current time should adjust the start time.");
-test(t => {
- const animation = new Animation;
- assert_throws(new TypeError, () => animation.currentTime = null);
-}, "Setting the current time to null should throw a TypeError.");
-
</script>
</body>
Modified: trunk/Source/WebCore/ChangeLog (225927 => 225928)
--- trunk/Source/WebCore/ChangeLog 2017-12-14 22:12:37 UTC (rev 225927)
+++ trunk/Source/WebCore/ChangeLog 2017-12-14 22:13:45 UTC (rev 225928)
@@ -1,5 +1,32 @@
2017-12-14 Antoine Quint <[email protected]>
+ [Web Animations] Bring timeline and currentTime setters closer to compliance
+ https://bugs.webkit.org/show_bug.cgi?id=180834
+
+ Reviewed by Dean Jackson.
+
+ Now that we've added support for the concept of a hold time, pending tasks
+ and updating the finished state, adopt those in places we had already implemented
+ but weren't fully compliant.
+
+ Web Platform Tests cover these behaviors, but we're currently failing those tests
+ due to lacking an implementation for Element.animate().
+
+ * animation/WebAnimation.cpp:
+ (WebCore::WebAnimation::setTimeline): Add some spec comments to clarify the code behavior
+ and implement step 4 of the "setting the timeline" procedure where we reset the hold time
+ to an unresolved value if the start time is resolved, as well as step 5 where we update the
+ finished state. Finally, we also ensure we update the pending tasks as the ready state is
+ dependent on a timeline being set.
+ (WebCore::WebAnimation::bindingsStartTime const): Invert the way we test for an unresolved
+ value to match prior review comments by Dean Jackson.
+ (WebCore::WebAnimation::setBindingsStartTime): Use a boolean check rather than checking
+ equality with std::nullopt.
+ (WebCore::WebAnimation::setBindingsCurrentTime): Do not raise an exception when setting
+ an unresolved time.
+
+2017-12-14 Antoine Quint <[email protected]>
+
[Web Animations] Implement the cancel() method on Animation
https://bugs.webkit.org/show_bug.cgi?id=180830
<rdar://problem/36055816>
Modified: trunk/Source/WebCore/animation/WebAnimation.cpp (225927 => 225928)
--- trunk/Source/WebCore/animation/WebAnimation.cpp 2017-12-14 22:12:37 UTC (rev 225927)
+++ trunk/Source/WebCore/animation/WebAnimation.cpp 2017-12-14 22:13:45 UTC (rev 225928)
@@ -110,11 +110,16 @@
void WebAnimation::setTimeline(RefPtr<AnimationTimeline>&& timeline)
{
+ // 3.4.1. Setting the timeline of an animation
+ // https://drafts.csswg.org/web-animations-1/#setting-the-timeline
+
+ // 2. If new timeline is the same object as old timeline, abort this procedure.
if (timeline == m_timeline)
return;
- // FIXME: If the animation start time of animation is resolved, make animation’s
- // hold time unresolved (webkit.org/b/178932).
+ // 4. If the animation start time of animation is resolved, make animation’s hold time unresolved.
+ if (startTime())
+ m_holdTime = std::nullopt;
if (m_timeline)
m_timeline->removeAnimation(*this);
@@ -134,18 +139,24 @@
}
m_timeline = WTFMove(timeline);
+
+ updatePendingTasks();
+
+ // 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.
+ updateFinishedState(DidSeek::No, SynchronouslyNotify::No);
}
std::optional<double> WebAnimation::bindingsStartTime() const
{
- if (m_startTime)
- return m_startTime->milliseconds();
- return std::nullopt;
+ if (!m_startTime)
+ return std::nullopt;
+ return m_startTime->milliseconds();
}
void WebAnimation::setBindingsStartTime(std::optional<double> startTime)
{
- if (startTime == std::nullopt)
+ if (!startTime)
setStartTime(std::nullopt);
else
setStartTime(Seconds::fromMilliseconds(startTime.value()));
@@ -178,9 +189,8 @@
ExceptionOr<void> WebAnimation::setBindingsCurrentTime(std::optional<double> currentTime)
{
if (!currentTime)
- return Exception { TypeError };
- setCurrentTime(Seconds::fromMilliseconds(currentTime.value()));
- return { };
+ return setCurrentTime(std::nullopt);
+ return setCurrentTime(Seconds::fromMilliseconds(currentTime.value()));
}
std::optional<Seconds> WebAnimation::currentTime() const
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
