Basic ScrollTimeline implementation for Animation Worklet

Basic ScrollTimeline implementation for Animation Worklet

This is based on the WICG spec[0], but simplified for the Animation Worklet
origin trial. Specifically, the following things were changed from the spec:

  * ScrollDirection only has two values: "block" and "inline".
  * Dropped startScrollOffset.
  * Dropped endScrollOffset.
  * timeRange is not allowed to be "auto".
  * Dropped fill.

[0] https://wicg.github.io/scroll-animations/#scrolltimeline-interface

BUG=719517
Review-Url: https://codereview.chromium.org/2873493002
Cr-Commit-Position: refs/heads/master@{#484117}
Committed: https://chromium.googlesource.com/chromium/src/+/a5a47f2f0ae0b0ec8e114f0023d1bae7290475cd

[smcgruer, 2017-05-09 16:06:47]

smcgruer@chromium.org changed reviewers:
+ flackr@chromium.org, majidvp@chromium.org

[smcgruer, 2017-05-09 16:06:47]

majidvp, flackr - PTAL. I consider this to be a starting point for a discussion
with the animation team.

Once you two are happy with the approach and amount of API implemented, I will
add some people from the animation team and we can discuss integration issues -
e.g. the CurrentTimeInternal issues with AnimationTimeline.

[majidvp, 2017-05-15 14:38:52]

https://codereview.chromium.org/2873493002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/animation/ScrollTimeline.cpp (right):

https://codereview.chromium.org/2873493002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/animation/ScrollTimeline.cpp:35: //
TODO(smcgruer): The spec allows for a null scrollSource.
Hmmm, I presume that if the scrollSource is not specified then the timeline can
be considered "inactive" and return an unresolved time value. wdyt?

https://w3c.github.io/web-animations/#inactive-timeline

https://codereview.chromium.org/2873493002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/animation/ScrollTimeline.cpp:72: if (!layout_box
|| !layout_box->GetScrollableArea()) {
I don't think layout_box->GetScrollableArea() maps correctly to existence of
scroll container. At the moment we create a scrollable area for any layout box
[1]. 

Also note that an element with overflow:auto is considered an scroll container
[2] even if it is not currently overflowing.

[1]
https://codesearch.chromium.org/chromium/src/third_party/WebKit/Source/core/p...
[2] https://www.w3.org/TR/css-overflow-3/#scroll-container

https://codereview.chromium.org/2873493002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/animation/ScrollTimeline.cpp:94:
scroll_source_->scrollHeight() - scroll_source_->clientHeight();
Shouldn't we use the logical coordinates here as opposed to the physical one. I
thought a vertical writing mode flips height and width dimensions which can make
this incorrect.

[smcgruer, 2017-05-15 15:22:54]

https://codereview.chromium.org/2873493002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/animation/ScrollTimeline.cpp (right):

https://codereview.chromium.org/2873493002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/animation/ScrollTimeline.cpp:35: //
TODO(smcgruer): The spec allows for a null scrollSource.
On 2017/05/15 14:38:52, majidvp wrote:
> Hmmm, I presume that if the scrollSource is not specified then the timeline
can
> be considered "inactive" and return an unresolved time value. wdyt?
> 
> https://w3c.github.io/web-animations/#inactive-timeline

Nope, spec defines it as follows
(https://wicg.github.io/scroll-animations/#dom-scrolltimeline-scrollsource):

"If [the scrollSource] is not specified, the document element of the active
document for the current browsing context, as determined at the time the
ScrollTimeline is constructed, is used."

https://codereview.chromium.org/2873493002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/animation/ScrollTimeline.cpp:72: if (!layout_box
|| !layout_box->GetScrollableArea()) {
On 2017/05/15 14:38:52, majidvp wrote:
> I don't think layout_box->GetScrollableArea() maps correctly to existence of
> scroll container. At the moment we create a scrollable area for any layout box
> [1]. 
> 
> Also note that an element with overflow:auto is considered an scroll container
> [2] even if it is not currently overflowing.
> 
> [1]
>
https://codesearch.chromium.org/chromium/src/third_party/WebKit/Source/core/p...
> [2] https://www.w3.org/TR/css-overflow-3/#scroll-container

Ah, so looks like by the spec we just want to check the value of overflow for
the element? I'm sure there must be a function somewhere to do that...

https://codereview.chromium.org/2873493002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/animation/ScrollTimeline.cpp:94:
scroll_source_->scrollHeight() - scroll_source_->clientHeight();
On 2017/05/15 14:38:52, majidvp wrote:
> Shouldn't we use the logical coordinates here as opposed to the physical one.
I
> thought a vertical writing mode flips height and width dimensions which can
make
> this incorrect.

Possibly. I just wrote this based on a comment in
https://developer.mozilla.org/en-US/docs/Web/API/Element/scrollHeight :

"The following equivalence returns true if an element is at the end of its
scroll, false if it isn't.

element.scrollHeight - element.scrollTop === element.clientHeight"


I will check out vertical writing modes.

[flackr, 2017-05-15 17:29:06]

https://codereview.chromium.org/2873493002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/animation/ScrollTimeline.cpp (right):

https://codereview.chromium.org/2873493002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/animation/ScrollTimeline.cpp:35: //
TODO(smcgruer): The spec allows for a null scrollSource.
On 2017/05/15 15:22:54, smcgruer wrote:
> On 2017/05/15 14:38:52, majidvp wrote:
> > Hmmm, I presume that if the scrollSource is not specified then the timeline
> can
> > be considered "inactive" and return an unresolved time value. wdyt?
> > 
> > https://w3c.github.io/web-animations/#inactive-timeline
> 
> Nope, spec defines it as follows
> (https://wicg.github.io/scroll-animations/#dom-scrolltimeline-scrollsource):
> 
> "If [the scrollSource] is not specified, the document element of the active
> document for the current browsing context, as determined at the time the
> ScrollTimeline is constructed, is used."

Note, probably should be the document.scrollingElement of the active document.

https://codereview.chromium.org/2873493002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/animation/ScrollTimeline.cpp:72: if (!layout_box
|| !layout_box->GetScrollableArea()) {
On 2017/05/15 15:22:54, smcgruer wrote:
> On 2017/05/15 14:38:52, majidvp wrote:
> > I don't think layout_box->GetScrollableArea() maps correctly to existence of
> > scroll container. At the moment we create a scrollable area for any layout
box
> > [1]. 
> > 
> > Also note that an element with overflow:auto is considered an scroll
container
> > [2] even if it is not currently overflowing.
> > 
> > [1]
> >
>
https://codesearch.chromium.org/chromium/src/third_party/WebKit/Source/core/p...
> > [2] https://www.w3.org/TR/css-overflow-3/#scroll-container
> 
> Ah, so looks like by the spec we just want to check the value of overflow for
> the element? I'm sure there must be a function somewhere to do that...

LayoutObject::HasOverflowClip

https://codereview.chromium.org/2873493002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/animation/ScrollTimeline.cpp:85:
layout_box->HasScrollableOverflowY());
I don't think this check is correct. We could have a scroll source that does not
overflow in either direction. This would be expected to produce the end value if
fill forwards is specified or unresolved if not.
https://wicg.github.io/scroll-animations/#current-time-algorithm

https://codereview.chromium.org/2873493002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/animation/ScrollTimeline.cpp:86:
local_orientation = layout_box->HasScrollableOverflowY() ? Block : Inline;
Isn't this assuming LTR? I think you'd have to check the current writing mode to
know which direction is block and which is inline.

[smcgruer, 2017-05-16 19:24:48]

A few things fixed, added some tests. Writing mode stuff still TBD.

https://codereview.chromium.org/2873493002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/animation/ScrollTimeline.cpp (right):

https://codereview.chromium.org/2873493002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/animation/ScrollTimeline.cpp:35: //
TODO(smcgruer): The spec allows for a null scrollSource.
On 2017/05/15 17:29:06, flackr wrote:
> On 2017/05/15 15:22:54, smcgruer wrote:
> > On 2017/05/15 14:38:52, majidvp wrote:
> > > Hmmm, I presume that if the scrollSource is not specified then the
timeline
> > can
> > > be considered "inactive" and return an unresolved time value. wdyt?
> > > 
> > > https://w3c.github.io/web-animations/#inactive-timeline
> > 
> > Nope, spec defines it as follows
> > (https://wicg.github.io/scroll-animations/#dom-scrolltimeline-scrollsource):
> > 
> > "If [the scrollSource] is not specified, the document element of the active
> > document for the current browsing context, as determined at the time the
> > ScrollTimeline is constructed, is used."
> 
> Note, probably should be the document.scrollingElement of the active document.

Filed https://github.com/WICG/scroll-animations/issues/14 for the spec issue,
made code use the document scrolling element.

https://codereview.chromium.org/2873493002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/animation/ScrollTimeline.cpp:72: if (!layout_box
|| !layout_box->GetScrollableArea()) {
On 2017/05/15 17:29:06, flackr wrote:
> On 2017/05/15 15:22:54, smcgruer wrote:
> > On 2017/05/15 14:38:52, majidvp wrote:
> > > I don't think layout_box->GetScrollableArea() maps correctly to existence
of
> > > scroll container. At the moment we create a scrollable area for any layout
> box
> > > [1]. 
> > > 
> > > Also note that an element with overflow:auto is considered an scroll
> container
> > > [2] even if it is not currently overflowing.
> > > 
> > > [1]
> > >
> >
>
https://codesearch.chromium.org/chromium/src/third_party/WebKit/Source/core/p...
> > > [2] https://www.w3.org/TR/css-overflow-3/#scroll-container
> > 
> > Ah, so looks like by the spec we just want to check the value of overflow
for
> > the element? I'm sure there must be a function somewhere to do that...
> 
> LayoutObject::HasOverflowClip

Done.

[smcgruer, 2017-05-17 15:52:03]

https://codereview.chromium.org/2873493002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/animation/ScrollTimeline.cpp (right):

https://codereview.chromium.org/2873493002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/animation/ScrollTimeline.cpp:85:
layout_box->HasScrollableOverflowY());
On 2017/05/15 17:29:06, flackr wrote:
> I don't think this check is correct. We could have a scroll source that does
not
> overflow in either direction. This would be expected to produce the end value
if
> fill forwards is specified or unresolved if not.
> https://wicg.github.io/scroll-animations/#current-time-algorithm

Done.

https://codereview.chromium.org/2873493002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/animation/ScrollTimeline.cpp:86:
local_orientation = layout_box->HasScrollableOverflowY() ? Block : Inline;
On 2017/05/15 17:29:06, flackr wrote:
> Isn't this assuming LTR? I think you'd have to check the current writing mode
to
> know which direction is block and which is inline.

Done.

https://codereview.chromium.org/2873493002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/animation/ScrollTimeline.cpp:94:
scroll_source_->scrollHeight() - scroll_source_->clientHeight();
On 2017/05/15 15:22:54, smcgruer wrote:
> On 2017/05/15 14:38:52, majidvp wrote:
> > Shouldn't we use the logical coordinates here as opposed to the physical
one.
> I
> > thought a vertical writing mode flips height and width dimensions which can
> make
> > this incorrect.
> 
> Possibly. I just wrote this based on a comment in
> https://developer.mozilla.org/en-US/docs/Web/API/Element/scrollHeight :
> 
> "The following equivalence returns true if an element is at the end of its
> scroll, false if it isn't.
> 
> element.scrollHeight - element.scrollTop === element.clientHeight"
> 
> 
> I will check out vertical writing modes.

Done.

[majidvp, 2017-05-17 20:39:45]

https://codereview.chromium.org/2873493002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/animation/ScrollTimeline.cpp (right):

https://codereview.chromium.org/2873493002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/animation/ScrollTimeline.cpp:38: :
const_cast<Document&>(document).scrollingElement();
I believe this should instead use |ScrollingElementNoLayout()| as indicated in
the respective comments.

https://codereview.chromium.org/2873493002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/animation/ScrollTimeline.cpp:38: :
const_cast<Document&>(document).scrollingElement();
It is odd that this method is not |const| requiring you to cast! Can we make it
const?

https://codereview.chromium.org/2873493002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/animation/ScrollTimeline.cpp:96: }
Should this decision be made every time we query currentTime 
or only at timeline construction time. e.g., if I change the writing mode or my
overflow direction during the
animation. All of sudden we are linked to a different scroll orientation.

Spec [1] indicates to use the direction specified by the orientation during the
current time algorithm but not clear it mean resolving "auto" every time.

[1] https://wicg.github.io/scroll-animations/#current-time-algorithm

https://codereview.chromium.org/2873493002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/animation/ScrollTimeline.cpp:106: ScrollOffset
max_abs_scroll_offset = scrollable_area->MaximumScrollOffset() -
nit: s/max_abs_scroll_offset/scroll_dimensions/  (or scroll_size?)

https://codereview.chromium.org/2873493002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/animation/ScrollTimeline.cpp:138: return
(current_offset / max_offset) * time_range_;
may be cleaner if we just move std::abs operation down here?

https://codereview.chromium.org/2873493002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/animation/ScrollTimeline.cpp:162: return "auto";
If we decide to resolve "auto" value on construction then it may make
sense to return the resolved value here. It will be more useful.

https://codereview.chromium.org/2873493002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/animation/ScrollTimelineTest.cpp (right):

https://codereview.chromium.org/2873493002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/animation/ScrollTimelineTest.cpp:331: //
difference results in a neglible difference in the output value.
nit: s/neglible/negligible/

[smcgruer, 2017-05-18 13:58:56]

https://codereview.chromium.org/2873493002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/animation/ScrollTimeline.cpp (right):

https://codereview.chromium.org/2873493002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/animation/ScrollTimeline.cpp:38: :
const_cast<Document&>(document).scrollingElement();
On 2017/05/17 20:39:45, majidvp wrote:
> I believe this should instead use |ScrollingElementNoLayout()| as indicated in
> the respective comments.

My only concern here is that |scrollingElement()| will do layout, whilst
|ScrollingElementNoLayout()| has a pre-requisite that you have already done
layout or it will DCHECK. I don't think that assumption is valid since the user
can create this class from script?

> It is odd that this method is not |const| requiring you to cast! Can we make
it
> const?

Feasible to do for |ScrollingElementNoLayout()| I think, but not for
|scrollingElement()|. The latter calls |UpdateStyleAndLayoutTree()| which is
sensibly non-const. So depends on resolution of above comment.

https://codereview.chromium.org/2873493002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/animation/ScrollTimeline.cpp:96: }
On 2017/05/17 20:39:45, majidvp wrote:
> Should this decision be made every time we query currentTime 
> or only at timeline construction time. e.g., if I change the writing mode or
my
> overflow direction during the
> animation. All of sudden we are linked to a different scroll orientation.
> 
> Spec [1] indicates to use the direction specified by the orientation during
the
> current time algorithm but not clear it mean resolving "auto" every time.
> 
> [1] https://wicg.github.io/scroll-animations/#current-time-algorithm
> 

Filed https://github.com/WICG/scroll-animations/issues/15 to ask. My
interpretation is that it should be resolved each time, but best to just get a
clarification :)

https://codereview.chromium.org/2873493002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/animation/ScrollTimeline.cpp:106: ScrollOffset
max_abs_scroll_offset = scrollable_area->MaximumScrollOffset() -
On 2017/05/17 20:39:45, majidvp wrote:
> nit: s/max_abs_scroll_offset/scroll_dimensions/  (or scroll_size?)

Done.

https://codereview.chromium.org/2873493002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/animation/ScrollTimeline.cpp:138: return
(current_offset / max_offset) * time_range_;
On 2017/05/17 20:39:44, majidvp wrote:
> may be cleaner if we just move std::abs operation down here?

Done.

https://codereview.chromium.org/2873493002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/animation/ScrollTimelineTest.cpp (right):

https://codereview.chromium.org/2873493002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/animation/ScrollTimelineTest.cpp:331: //
difference results in a neglible difference in the output value.
On 2017/05/17 20:39:45, majidvp wrote:
> nit: s/neglible/negligible/

Done.

[smcgruer, 2017-05-18 14:17:21]

https://codereview.chromium.org/2873493002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/animation/ScrollTimeline.cpp (right):

https://codereview.chromium.org/2873493002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/animation/ScrollTimeline.cpp:38: :
const_cast<Document&>(document).scrollingElement();
> > It is odd that this method is not |const| requiring you to cast! Can we make
> it
> > const?
> 
> Feasible to do for |ScrollingElementNoLayout()| I think, but not for
> |scrollingElement()|. The latter calls |UpdateStyleAndLayoutTree()| which is
> sensibly non-const. So depends on resolution of above comment.

Turns out that the Document& doesnt have to be const when called from JS, so
fixed the const issue.

[flackr, 2017-05-18 15:11:50]

LGTM. When animations support construction with a timeline we should have a test
of driving an animation from a scroll timeline. I'm guessing you can't test the
currentTime from JS otherwise?

https://codereview.chromium.org/2873493002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/animation/ScrollTimeline.cpp (right):

https://codereview.chromium.org/2873493002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/animation/ScrollTimeline.cpp:99: // -
min_offset).
There is always the risk that the max or min may not be based around 0 and so
the absolute magnitude loses information, I don't think this ever happens
though. Probably worth a comment.

[smcgruer, 2017-05-18 15:42:56]

smcgruer@chromium.org changed reviewers:
+ alancutter@chromium.org, szager@chromium.org

[smcgruer, 2017-05-18 15:42:57]

Ok, since flackr and majidvp are generally happy, let's add +alancutter for
thoughts on the general implementation and +szager for sanity check of what I'm
doing with scroll offsets in ScrolLTimeline::currentTime(bool&).

> I'm guessing you can't test the currentTime from JS otherwise?

I will be adding LayoutTests later today. You should be able to construct and
query a ScrollTimeline from JS using this CL, although it'll obviously be
main-only.

https://codereview.chromium.org/2873493002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/animation/ScrollTimeline.cpp (right):

https://codereview.chromium.org/2873493002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/animation/ScrollTimeline.cpp:99: // -
min_offset).
On 2017/05/18 15:11:50, flackr wrote:
> There is always the risk that the max or min may not be based around 0 and so
> the absolute magnitude loses information, I don't think this ever happens
> though. Probably worth a comment.

Added a DCHECK, since this is a pre-requisite of the code operating correctly.

[smcgruer, 2017-05-23 14:47:27]

https://codereview.chromium.org/2873493002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/animation/ScrollTimeline.cpp (right):

https://codereview.chromium.org/2873493002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/animation/ScrollTimeline.cpp:96: }
On 2017/05/18 13:58:56, smcgruer wrote:
> On 2017/05/17 20:39:45, majidvp wrote:
> > Should this decision be made every time we query currentTime 
> > or only at timeline construction time. e.g., if I change the writing mode or
> my
> > overflow direction during the
> > animation. All of sudden we are linked to a different scroll orientation.
> > 
> > Spec [1] indicates to use the direction specified by the orientation during
> the
> > current time algorithm but not clear it mean resolving "auto" every time.
> > 
> > [1] https://wicg.github.io/scroll-animations/#current-time-algorithm
> > 
> 
> Filed https://github.com/WICG/scroll-animations/issues/15 to ask. My
> interpretation is that it should be resolved each time, but best to just get a
> clarification :)

https://github.com/WICG/scroll-animations/issues/15 specified that it should be
at construction time, but then I discovered
https://github.com/WICG/scroll-animations/issues/16 which makes it difficult to
implement orientation == 'auto' at this time. Therefore, I decided to just
disallow orientation == 'auto' for now.

[smcgruer, 2017-05-23 16:01:34]

The CQ bit was checked by smcgruer@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-23 16:03:05]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-23 16:07:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-23 16:07:38]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[smcgruer, 2017-05-23 16:16:06]

The CQ bit was checked by smcgruer@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-23 16:16:49]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-23 16:51:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-23 16:51:45]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[smcgruer, 2017-05-23 17:05:26]

The CQ bit was checked by smcgruer@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-23 17:05:52]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-23 19:44:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-23 19:44:49]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[smcgruer, 2017-05-23 21:14:10]

The CQ bit was checked by smcgruer@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-23 21:15:15]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-23 23:00:16]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-23 23:00:17]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[smcgruer, 2017-05-23 23:09:58]

The CQ bit was checked by smcgruer@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-23 23:10:45]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-24 02:38:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-24 02:38:49]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[smcgruer, 2017-05-24 13:43:11]

The CQ bit was checked by smcgruer@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-24 13:43:22]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-24 15:16:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-24 15:16:54]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[smcgruer, 2017-05-24 17:37:50]

The CQ bit was checked by smcgruer@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-24 17:38:36]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-24 21:43:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-24 21:43:16]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[smcgruer, 2017-05-25 13:41:12]

Description was changed from

==========
Basic ScrollTimeline implementation for Animation Worklet

This is based on the WICG spec[0], but simplified for the Animation Worklet
origin trial. Specifically, the following things were changed from the spec:

  * scrollSource is not allowed to be null.
  * ScrollDirection only has three values: "auto", "block", "inline".
  * Dropped startScrollOffset.
  * Dropped endScrollOffset.
  * timeRange is not allowed to be "auto".
  * Dropped fill.

[0] https://wicg.github.io/scroll-animations/#scrolltimeline-interface

BUG=719517
==========

to

==========
Basic ScrollTimeline implementation for Animation Worklet

This is based on the WICG spec[0], but simplified for the Animation Worklet
origin trial. Specifically, the following things were changed from the spec:

  * ScrollDirection only has two values: "block" and "inline".
  * Dropped startScrollOffset.
  * Dropped endScrollOffset.
  * timeRange is not allowed to be "auto".
  * Dropped fill.

[0] https://wicg.github.io/scroll-animations/#scrolltimeline-interface

BUG=719517
==========

[smcgruer, 2017-05-30 20:56:46]

Ping; Alan I would appreciate your thoughts on this partial implementation of
ScrollTimeline, thanks.

[smcgruer, 2017-06-05 14:00:44]

Second ping for comments by alancutter@, thanks.

[alancutter (OOO until 2018), 2017-06-08 05:00:32]

Sorry about the horrendously late review.

https://codereview.chromium.org/2873493002/diff/340001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/animation/ScrollTimeline.cpp (right):

https://codereview.chromium.org/2873493002/diff/340001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/animation/ScrollTimeline.cpp:60:
time_range_(time_range) {}
We should assert that the RuntimeEnabledFeature flag is enabled here.

https://codereview.chromium.org/2873493002/diff/340001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/animation/ScrollTimeline.h (right):

https://codereview.chromium.org/2873493002/diff/340001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/animation/ScrollTimeline.h:33: // only have to
override |currentTime|.
I agree with this for all the code in AnimationTimeline. It was written before
the idea of a separate DocumentTimeline interface existed, as a result the code
is really DocumentTimeline code and probably doesn't make sense to share with
ScrollTimeline. I'm not comfortable with that code getting inherited by
ScrollTimeline without necessity or test coverage, please reduce
AnimationTimeline to the bare minimum that ScrollTimeline requires before
inheriting from it.

Please let me know if this change is non-trivial.

https://codereview.chromium.org/2873493002/diff/340001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/animation/ScrollTimeline.idl (right):

https://codereview.chromium.org/2873493002/diff/340001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/animation/ScrollTimeline.idl:11:
RuntimeEnabled=CompositorWorker,
I think you want "Exposed(Window CompositorWorker)" so that the interface isn't
exposed to stable.
We should not be seeing a change to
LayoutTests/webexposed/global-interface-listing-expected.txt.

https://codereview.chromium.org/2873493002/diff/340001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/animation/ScrollTimelineTest.cpp (right):

https://codereview.chromium.org/2873493002/diff/340001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/animation/ScrollTimelineTest.cpp:77:
EXPECT_EQ(GetDocument().scrollingElement(), scroll_timeline->scrollSource());
These tests should be testharness.js tests where possible so they may contribute
to the WPT test suite to help ensure cross browser compatibility of the new
feature.

[smcgruer, 2017-06-08 14:43:47]

Thanks for the review Alan. I will look into refactoring AnimationTimeline as
per your recommendation - that one may take longer :)

https://codereview.chromium.org/2873493002/diff/340001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/animation/ScrollTimeline.cpp (right):

https://codereview.chromium.org/2873493002/diff/340001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/animation/ScrollTimeline.cpp:60:
time_range_(time_range) {}
On 2017/06/08 05:00:31, alancutter wrote:
> We should assert that the RuntimeEnabledFeature flag is enabled here.

Done.

https://codereview.chromium.org/2873493002/diff/340001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/animation/ScrollTimeline.idl (right):

https://codereview.chromium.org/2873493002/diff/340001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/animation/ScrollTimeline.idl:11:
RuntimeEnabled=CompositorWorker,
On 2017/06/08 05:00:32, alancutter wrote:
> I think you want "Exposed(Window CompositorWorker)" so that the interface
isn't
> exposed to stable.
> We should not be seeing a change to
> LayoutTests/webexposed/global-interface-listing-expected.txt.

Done.

https://codereview.chromium.org/2873493002/diff/340001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/animation/ScrollTimelineTest.cpp (right):

https://codereview.chromium.org/2873493002/diff/340001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/animation/ScrollTimelineTest.cpp:77:
EXPECT_EQ(GetDocument().scrollingElement(), scroll_timeline->scrollSource());
On 2017/06/08 05:00:32, alancutter wrote:
> These tests should be testharness.js tests where possible so they may
contribute
> to the WPT test suite to help ensure cross browser compatibility of the new
> feature.

I believe all are covered by WPT-compatible layout tests as well. (See
scrolltimeline-creation.html, scrolltimeline-currenttime.html,
scrolltimeline-currenttime-nan.html). Do you wish the unittests removed?

[alancutter (OOO until 2018), 2017-06-09 04:06:19]

https://codereview.chromium.org/2873493002/diff/340001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/animation/ScrollTimelineTest.cpp (right):

https://codereview.chromium.org/2873493002/diff/340001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/animation/ScrollTimelineTest.cpp:77:
EXPECT_EQ(GetDocument().scrollingElement(), scroll_timeline->scrollSource());
On 2017/06/08 at 14:43:47, smcgruer wrote:
> On 2017/06/08 05:00:32, alancutter wrote:
> > These tests should be testharness.js tests where possible so they may
contribute
> > to the WPT test suite to help ensure cross browser compatibility of the new
> > feature.
> 
> I believe all are covered by WPT-compatible layout tests as well. (See
scrolltimeline-creation.html, scrolltimeline-currenttime.html,
scrolltimeline-currenttime-nan.html). Do you wish the unittests removed?

I find unit tests to be more maintenance heavy than LayoutTests, let's avoid
having them if they're redundant.

[alancutter (OOO until 2018), 2017-06-22 01:39:30]

On 2017/06/08 at 14:43:47, smcgruer wrote:
> Thanks for the review Alan. I will look into refactoring AnimationTimeline as
per your recommendation - that one may take longer :)

This refactor is currently being done by suzyh:
https://codereview.chromium.org/2944423003
https://codereview.chromium.org/2948193002
https://codereview.chromium.org/2952093002

[suzyh_UTC10 (ex-contributor), 2017-06-22 03:29:41]

On 2017/06/22 at 01:39:30, alancutter wrote:
> On 2017/06/08 at 14:43:47, smcgruer wrote:
> > Thanks for the review Alan. I will look into refactoring AnimationTimeline
as per your recommendation - that one may take longer :)
> 
> This refactor is currently being done by suzyh:
> https://codereview.chromium.org/2944423003
> https://codereview.chromium.org/2948193002
> https://codereview.chromium.org/2952093002

I'm hoping to land these in the next couple of days. Just waiting for final
approvals for patches they depend on.

[smcgruer, 2017-06-22 14:34:25]

> I'm hoping to land these in the next couple of days. Just 
> waiting for final
> approvals for patches they depend on.

Awesome, I should go on vacation more often! :D. Looking forward to seeing those
landed.

https://codereview.chromium.org/2873493002/diff/340001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/animation/ScrollTimelineTest.cpp (right):

https://codereview.chromium.org/2873493002/diff/340001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/animation/ScrollTimelineTest.cpp:77:
EXPECT_EQ(GetDocument().scrollingElement(), scroll_timeline->scrollSource());
On 2017/06/09 04:06:19, alancutter wrote:
> On 2017/06/08 at 14:43:47, smcgruer wrote:
> > On 2017/06/08 05:00:32, alancutter wrote:
> > > These tests should be testharness.js tests where possible so they may
> contribute
> > > to the WPT test suite to help ensure cross browser compatibility of the
new
> > > feature.
> > 
> > I believe all are covered by WPT-compatible layout tests as well. (See
> scrolltimeline-creation.html, scrolltimeline-currenttime.html,
> scrolltimeline-currenttime-nan.html). Do you wish the unittests removed?
> 
> I find unit tests to be more maintenance heavy than LayoutTests, let's avoid
> having them if they're redundant.

Done.

[suzyh_UTC10 (ex-contributor), 2017-06-28 03:47:14]

On 2017/06/22 at 14:34:25, smcgruer wrote:
> > I'm hoping to land these in the next couple of days. Just 
> > waiting for final
> > approvals for patches they depend on.
> 
> Awesome, I should go on vacation more often! :D. Looking forward to seeing
those landed.

These patches are in now.

[smcgruer, 2017-06-28 13:47:32]

The CQ bit was checked by smcgruer@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-28 13:47:45]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[smcgruer, 2017-06-28 15:01:51]

Awesome, thanks Suzy. I've rebased on the refactors, so everyone PTAL and
hopefully we can head towards a submit :).

all reviewers: Currently this is guarded behind the CompositorWorker blink
feature. Does anyone have a preference on this feature vs WebAnimationsAPI ?

alancutter: I just ran the layout tests on the latest patchset, and it looks
like an interface for ScrollTimeline is being added for
webexposed/global-interface-listing.html despite using 'Exposed(Window
CompositorWorker)' as you suggested previously. Did I make a mistake in the IDL,
or is that expected and I should update the global listing?

[commit-bot: I haz the power, 2017-06-28 15:40:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-28 15:40:58]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[majidvp, 2017-06-28 15:52:38]

lgtm

> all reviewers: Currently this is guarded behind the CompositorWorker blink
feature. Does anyone have a preference on this feature vs WebAnimationsAPI ?

Given that ScrollTimeline is not yet part of the web-animation I think it is
reasonable to keep it under CompositorWorker feature flag. I don't have a 
strong opinion here though so fine if someone feels strongly about it
one way or the other.

https://codereview.chromium.org/2873493002/diff/480001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/fast/animation/scroll-animations/scrolltimeline-creation.html
(right):

https://codereview.chromium.org/2873493002/diff/480001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/animation/scroll-animations/scrolltimeline-creation.html:6:
overflow: auto;
nit: In this case this it always overflows so it is better to just state that as
opposed to auto.

https://codereview.chromium.org/2873493002/diff/480001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/fast/animation/scroll-animations/scrolltimeline-currenttime.html
(right):

https://codereview.chromium.org/2873493002/diff/480001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/animation/scroll-animations/scrolltimeline-currenttime.html:6:
overflow: auto;
ditto

[suzyh_UTC10 (ex-contributor), 2017-06-28 23:31:55]

> alancutter: I just ran the layout tests on the latest patchset, and it looks
like an interface for ScrollTimeline is being added for
webexposed/global-interface-listing.html despite using 'Exposed(Window
CompositorWorker)' as you suggested previously. Did I make a mistake in the IDL,
or is that expected and I should update the global listing?

Drive-by comment: I used "Exposed(Window WebAnimationsAPI)" in
https://codereview.chromium.org/2806623003 because Animation had already been
shipped and I wanted to implement just the constructor behind the flag. Do you
just want "RuntimeEnabled=CompositorWorker" instead, to keep the whole
ScrollTimeline interface behind a flag?

[smcgruer, 2017-06-29 03:36:44]

On 2017/06/28 23:31:55, suzyh_UTC10 wrote:
> > alancutter: I just ran the layout tests on the latest patchset, and it looks
> like an interface for ScrollTimeline is being added for
> webexposed/global-interface-listing.html despite using 'Exposed(Window
> CompositorWorker)' as you suggested previously. Did I make a mistake in the
IDL,
> or is that expected and I should update the global listing?
> 
> Drive-by comment: I used "Exposed(Window WebAnimationsAPI)" in
> https://codereview.chromium.org/2806623003 because Animation had already been
> shipped and I wanted to implement just the constructor behind the flag. Do you
> just want "RuntimeEnabled=CompositorWorker" instead, to keep the whole
> ScrollTimeline interface behind a flag?

See patchsets 18-20, where alancutter@ asked me to switch from
RuntimeEnabled=CompositorWorker to Exposed(Window CompositorWorker). Hopefully
Alan can clarify the matter. :)

[alancutter (OOO until 2018), 2017-06-29 03:38:05]

On 2017/06/28 at 23:31:55, suzyh wrote:
> > alancutter: I just ran the layout tests on the latest patchset, and it looks
like an interface for ScrollTimeline is being added for
webexposed/global-interface-listing.html despite using 'Exposed(Window
CompositorWorker)' as you suggested previously. Did I make a mistake in the IDL,
or is that expected and I should update the global listing?
> 
> Drive-by comment: I used "Exposed(Window WebAnimationsAPI)" in
https://codereview.chromium.org/2806623003 because Animation had already been
shipped and I wanted to implement just the constructor behind the flag. Do you
just want "RuntimeEnabled=CompositorWorker" instead, to keep the whole
ScrollTimeline interface behind a flag?

This is correct; my original comment was incorrect, sorry. I mistakenly thought
that the change to webexposed/global-interface-listing-expected.txt meant it was
being shipped to stable. It's changes to
virtual/stable/webexposed/global-interface-listing-expected.txt that indicate
shipping to stable.

[smcgruer, 2017-06-29 13:13:35]

The CQ bit was checked by smcgruer@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-29 13:13:46]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-29 15:16:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-29 15:16:07]

Dry run: This issue passed the CQ dry run.

[smcgruer, 2017-06-29 15:23:22]

Switched back to RuntimeEnabled, and fixed the nits referred to by majidvp@

https://codereview.chromium.org/2873493002/diff/480001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/fast/animation/scroll-animations/scrolltimeline-creation.html
(right):

https://codereview.chromium.org/2873493002/diff/480001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/animation/scroll-animations/scrolltimeline-creation.html:6:
overflow: auto;
On 2017/06/28 15:52:38, majidvp wrote:
> nit: In this case this it always overflows so it is better to just state that
as
> opposed to auto.

Done.

https://codereview.chromium.org/2873493002/diff/480001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/fast/animation/scroll-animations/scrolltimeline-currenttime.html
(right):

https://codereview.chromium.org/2873493002/diff/480001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/animation/scroll-animations/scrolltimeline-currenttime.html:6:
overflow: auto;
On 2017/06/28 15:52:38, majidvp wrote:
> ditto

Done.

[smcgruer, 2017-07-03 23:43:52]

smcgruer@chromium.org changed reviewers:
+ suzyh@chromium.org

[smcgruer, 2017-07-03 23:43:53]

Ping for core/animation OWNERS

[alancutter (OOO until 2018), 2017-07-04 04:13:41]

lgtm with nits. The tests are great.

https://codereview.chromium.org/2873493002/diff/500001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/animation/ScrollTimeline.cpp (right):

https://codereview.chromium.org/2873493002/diff/500001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/animation/ScrollTimeline.cpp:100: current_offset
=
Assert that orientation_ is Inline here to make the code a bit clearer.

https://codereview.chromium.org/2873493002/diff/500001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/animation/ScrollTimeline.h (right):

https://codereview.chromium.org/2873493002/diff/500001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/animation/ScrollTimeline.h:17: // Implements the
ScrollTimeline concept from the Scroll-linked Animations spec.
It's worth adding the spec link here since it's directly mentioned.

https://codereview.chromium.org/2873493002/diff/500001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/animation/ScrollTimeline.h:38: double
currentTime(bool& is_null) override;
Let's use the final keyword here and save override to indicate that there's
other subclasses that override it further.

[smcgruer, 2017-07-04 13:43:08]

smcgruer@chromium.org changed reviewers:
+ jbroman@chromium.org

[smcgruer, 2017-07-04 13:43:09]

+jbroman for bindings/core OWNERS

[Yuki, 2017-07-04 13:45:20]

yukishiino@chromium.org changed reviewers:
+ yukishiino@chromium.org

[Yuki, 2017-07-04 13:45:21]

bindings LGTM.

[smcgruer, 2017-07-04 13:48:53]

Thanks Yuki :)

https://codereview.chromium.org/2873493002/diff/500001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/animation/ScrollTimeline.cpp (right):

https://codereview.chromium.org/2873493002/diff/500001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/animation/ScrollTimeline.cpp:100: current_offset
=
On 2017/07/04 04:13:41, alancutter wrote:
> Assert that orientation_ is Inline here to make the code a bit clearer.

Done.

https://codereview.chromium.org/2873493002/diff/500001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/animation/ScrollTimeline.h (right):

https://codereview.chromium.org/2873493002/diff/500001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/animation/ScrollTimeline.h:17: // Implements the
ScrollTimeline concept from the Scroll-linked Animations spec.
On 2017/07/04 04:13:41, alancutter wrote:
> It's worth adding the spec link here since it's directly mentioned.

Done.

https://codereview.chromium.org/2873493002/diff/500001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/animation/ScrollTimeline.h:38: double
currentTime(bool& is_null) override;
On 2017/07/04 04:13:41, alancutter wrote:
> Let's use the final keyword here and save override to indicate that there's
> other subclasses that override it further.

Done.

[smcgruer, 2017-07-04 13:49:08]

The CQ bit was checked by smcgruer@chromium.org

[smcgruer, 2017-07-04 13:49:09]

The patchset sent to the CQ was uploaded after l-g-t-m from flackr@chromium.org,
majidvp@chromium.org, alancutter@chromium.org, yukishiino@chromium.org
Link to the patchset: https://codereview.chromium.org/2873493002/#ps540001
(title: "address reviewer nits")

[commit-bot: I haz the power, 2017-07-04 13:49:15]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-07-04 15:48:44]

CQ is committing da patch.

Bot data: {"patchset_id": 540001, "attempt_start_ts": 1499176148234470,
"parent_rev": "38d8bb9328a2d0a1b7d8373ddd70971c1ce4eeb6", "commit_rev":
"a5a47f2f0ae0b0ec8e114f0023d1bae7290475cd"}

[commit-bot: I haz the power, 2017-07-04 15:48:56]

Description was changed from

==========
Basic ScrollTimeline implementation for Animation Worklet

This is based on the WICG spec[0], but simplified for the Animation Worklet
origin trial. Specifically, the following things were changed from the spec:

  * ScrollDirection only has two values: "block" and "inline".
  * Dropped startScrollOffset.
  * Dropped endScrollOffset.
  * timeRange is not allowed to be "auto".
  * Dropped fill.

[0] https://wicg.github.io/scroll-animations/#scrolltimeline-interface

BUG=719517
==========

to

==========
Basic ScrollTimeline implementation for Animation Worklet

This is based on the WICG spec[0], but simplified for the Animation Worklet
origin trial. Specifically, the following things were changed from the spec:

  * ScrollDirection only has two values: "block" and "inline".
  * Dropped startScrollOffset.
  * Dropped endScrollOffset.
  * timeRange is not allowed to be "auto".
  * Dropped fill.

[0] https://wicg.github.io/scroll-animations/#scrolltimeline-interface

BUG=719517

Review-Url: https://codereview.chromium.org/2873493002
Cr-Commit-Position: refs/heads/master@{#484117}
Committed:
https://chromium.googlesource.com/chromium/src/+/a5a47f2f0ae0b0ec8e114f0023d1...
==========

[commit-bot: I haz the power, 2017-07-04 15:48:58]

Committed patchset #28 (id:540001) as
https://chromium.googlesource.com/chromium/src/+/a5a47f2f0ae0b0ec8e114f0023d1...