Bug 178988 - [Web Animations] Expose the currentTime property on Animation
Summary: [Web Animations] Expose the currentTime property on Animation
Alias: None
Product: WebKit
Classification: Unclassified
Component: Animations (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Antoine Quint
Keywords: InRadar
Depends on:
Reported: 2017-10-29 11:36 PDT by Antoine Quint
Modified: 2017-10-31 06:42 PDT (History)
6 users (show)

See Also:

Patch (9.37 KB, patch)
2017-10-29 11:40 PDT, Antoine Quint
dino: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antoine Quint 2017-10-29 11:36:07 PDT
[Web Animations] Expose the currentTime property on Animation
Comment 1 Antoine Quint 2017-10-29 11:40:38 PDT
Created attachment 325287 [details]
Comment 2 Dean Jackson 2017-10-29 12:20:18 PDT
Comment on attachment 325287 [details]

View in context: https://bugs.webkit.org/attachment.cgi?id=325287&action=review

> Source/WebCore/animation/WebAnimation.cpp:94
> +    if (time == std::nullopt)
> +        return std::nullopt;
> +    return time->value();

If you want to test for the optional having a value you can just do

if (!time)
  return std::nullopt;

But in this case there is a helper to make it even easier for you:

return time.value_or(std::nullopt);

> Source/WebCore/animation/WebAnimation.cpp:100
> +    if (currentTime == std::nullopt)
> +        return Exception { TypeError };

if (!currentTime)

> Source/WebCore/animation/WebAnimation.cpp:110
> +    if (!m_timeline || m_startTime == std::nullopt)
> +        return std::nullopt;

if (!m_timeline || !m_startTime)

> Source/WebCore/animation/WebAnimation.cpp:113
> +    if (timelineTime == std::nullopt)

if (!timelineTime)

> Source/WebCore/animation/WebAnimation.cpp:129
> +    std::optional<Seconds> timelineTime = m_timeline->currentTime();

auto timelineTime = m_timeline->currentTime();

> Source/WebCore/animation/WebAnimation.cpp:130
> +    if (timelineTime == std::nullopt) {

if (!timelineTime)

> LayoutTests/ChangeLog:11
> +        Add a new test that checks that the currentTime property is set
> +        correctly based on the startTime value and the document timeline
> +        currentTime, and that setting the property may raise an exception
> +        and otherwise update the animation startTime.

If this is tested by the existing WPT tests, then you'd be better off importing them with expected results that show a lot of failures. Then, as you implement, you'll slowly change those failures into passes.

Then again, since you need to go through Internals, maybe there is no way to test this using WPT.
Comment 3 Antoine Quint 2017-10-29 13:22:43 PDT
Committed r224163: <https://trac.webkit.org/changeset/224163>
Comment 4 Antoine Quint 2017-10-29 14:08:57 PDT
Committed r224164: <https://trac.webkit.org/changeset/224164>
Comment 5 Radar WebKit Bug Importer 2017-10-31 06:41:13 PDT