New issue
Advanced search Search tips

Issue 840383 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner: ----
Closed: Aug 17
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 430155



Sign in to add a comment

Animation Worklet - worklet animation constructor timeline and options should be optional

Project Member Reported by majidvp@chromium.org, May 7 2018

Issue description

According to the latest spec the last 3 parameters should be optional.

I think the correct behavior should be: 

1) If timeline is not provided  we should default to default document timeline.
2) If options is not provided, assume there is not options. We currently don't
even support "options" so it should definitely be optional. (Perhaps even we
should throw when it is actually specified?)

Note that Effect may also be allowed empty by we don't currently support
unprovided effects. Unlike timeline and options, I don't see why someone may
even try to no provide an effect so it is not as important to fix that one yet.


``` [Constructor (DOMString animatorName,
              optional (AnimationEffectReadOnly or sequence)? effects = null,
              optional AnimationTimeline? timeline, optional any options)]
interface WorkletAnimation : Animation {
        readonly attribute DOMString animatorName;
};

```

[1] https://wicg.github.io/animation-worklet/#creating-worklet-animation
 

Comment 1 by surma@chromium.org, May 7 2018

Definitely reasonable to warn/throw on empty effect, imo.
Project Member

Comment 2 by bugdroid1@chromium.org, May 16 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4344b812321a1c1c021723e5f6fce77920688c0e

commit 4344b812321a1c1c021723e5f6fce77920688c0e
Author: Majid Valipour <majidvp@chromium.org>
Date: Wed May 16 19:52:07 2018

[animation-worklet] Make timeline & options optional in WorkletAnimation constructor

The last two values should be optional according to the specification [1].
Added two new factory methods to allow this. To do this cleanly
switched WorkletAnimation to have Member<AnimationTimeline> instead of
holding onto a DocumentTimelineOrScrollTimeline.

[1] https://wicg.github.io/animation-worklet/#creating-worklet-animation

TEST: virtual/threaded/fast/animationworklet/worklet-animation-creation.html
Bug:  840383 
Change-Id: Ida5aa077fbf19b4918652c02e92f40435db4fbb4
Reviewed-on: https://chromium-review.googlesource.com/1047691
Commit-Queue: Majid Valipour <majidvp@chromium.org>
Reviewed-by: Majid Valipour <majidvp@chromium.org>
Reviewed-by: Stephen McGruer <smcgruer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559240}
[modify] https://crrev.com/4344b812321a1c1c021723e5f6fce77920688c0e/third_party/WebKit/LayoutTests/http/tests/origin_trials/webexposed/animationworklet-origin-trial-interfaces.html
[modify] https://crrev.com/4344b812321a1c1c021723e5f6fce77920688c0e/third_party/WebKit/LayoutTests/virtual/threaded/fast/animationworklet/worklet-animation-creation.html
[modify] https://crrev.com/4344b812321a1c1c021723e5f6fce77920688c0e/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/4344b812321a1c1c021723e5f6fce77920688c0e/third_party/blink/renderer/core/animation/animation_timeline.h
[modify] https://crrev.com/4344b812321a1c1c021723e5f6fce77920688c0e/third_party/blink/renderer/core/animation/scroll_timeline.h
[modify] https://crrev.com/4344b812321a1c1c021723e5f6fce77920688c0e/third_party/blink/renderer/modules/animationworklet/worklet_animation.cc
[modify] https://crrev.com/4344b812321a1c1c021723e5f6fce77920688c0e/third_party/blink/renderer/modules/animationworklet/worklet_animation.h
[modify] https://crrev.com/4344b812321a1c1c021723e5f6fce77920688c0e/third_party/blink/renderer/modules/animationworklet/worklet_animation.idl

Status: Fixed (was: Available)

Sign in to add a comment