Implement frames() timing function

Implement frames() timing function

This patch adds support for the frames timing function defined in
https://www.w3.org/TR/css-timing-1/#frames-timing-functions, based on
the implementation for the steps timing function.

BUG=646265
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2775143002
Cr-Commit-Position: refs/heads/master@{#465916}
Committed: https://chromium.googlesource.com/chromium/src/+/1b0bb2fe0fb6094a235a10b4e7d921c9b04e6d2e

[suzyh_UTC10 (ex-contributor), 2017-03-27 04:01:48]

Description was changed from

==========
Implement frames() timing function

This patch adds support for the frames timing function defined in
https://www.w3.org/TR/css-timing-1/#frames-timing-functions, based on
the implementation for the steps timing function.

BUG=646265
==========

to

==========
Implement frames() timing function

This patch adds support for the frames timing function defined in
https://www.w3.org/TR/css-timing-1/#frames-timing-functions, based on
the implementation for the steps timing function.

BUG=646265
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[suzyh_UTC10 (ex-contributor), 2017-03-27 04:02:43]

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

[commit-bot: I haz the power, 2017-03-27 04:02:51]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[suzyh_UTC10 (ex-contributor), 2017-03-27 04:02:57]

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

[commit-bot: I haz the power, 2017-03-27 06:00:31]

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

[commit-bot: I haz the power, 2017-03-27 06:00:32]

Dry run: This issue passed the CQ dry run.

[alancutter (OOO until 2018), 2017-03-29 04:58:31]

This needs LayoutTests that checks the interpolation state of CSS Animations,
CSS Transitions and Web Animations that use the frames() timing function.

https://codereview.chromium.org/2775143002/diff/1/cc/animation/keyframed_anim...
File cc/animation/keyframed_animation_curve_unittest.cc (right):

https://codereview.chromium.org/2775143002/diff/1/cc/animation/keyframed_anim...
cc/animation/keyframed_animation_curve_unittest.cc:560: EXPECT_FLOAT_EQ(1.f,
curve->GetValue(base::TimeDelta::FromSecondsD(1.0)));
These line wraps make the test a bit harder to read and verify, I'd consider
having a helper function to reduce the line length.

https://codereview.chromium.org/2775143002/diff/1/cc/animation/keyframed_anim...
cc/animation/keyframed_animation_curve_unittest.cc:864: // range [0,1]
Bah these comments, they're just repeating the test name. No need to alter given
it's consistent.

https://codereview.chromium.org/2775143002/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/transitions/transitions-parsing.html
(right):

https://codereview.chromium.org/2775143002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/transitions/transitions-parsing.html:487:
shouldBe("computedStyle.webkitTransitionTimingFunction", "'ease'");
I wish there were a helper function like shouldNotParse('pants') or something to
make the intent more clear and reduce repetition but it's okay to leave as is
since it's consistent with the existing code.

https://codereview.chromium.org/2775143002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/transitions/transitions-parsing.html:488: 
Let's also test frames() for parse failure.

[suzyh_UTC10 (ex-contributor), 2017-04-04 01:02:04]

> This needs LayoutTests that checks the interpolation state of CSS Animations,
CSS Transitions
> and Web Animations that use the frames() timing function.

Mozilla have some tests that are waiting to be upstreamed to the Web Platform
Tests:
https://hg.mozilla.org/mozilla-central/file/6d38ad302429/testing/web-platform...
Once these go up to WPT, they should be imported in via our usual automated
process. The move from csswg-test to wpt/css is not yet finished
(crbug.com/706118), so there's still going to be a delay in having these tests
be available, but they're coming.

https://codereview.chromium.org/2775143002/diff/1/cc/animation/keyframed_anim...
File cc/animation/keyframed_animation_curve_unittest.cc (right):

https://codereview.chromium.org/2775143002/diff/1/cc/animation/keyframed_anim...
cc/animation/keyframed_animation_curve_unittest.cc:560: EXPECT_FLOAT_EQ(1.f,
curve->GetValue(base::TimeDelta::FromSecondsD(1.0)));
On 2017/03/29 at 04:58:30, alancutter wrote:
> These line wraps make the test a bit harder to read and verify, I'd consider
having a helper function to reduce the line length.

Rearranged. How's this?

git cl format has Opinions about how the expectations data should be formatted.
Let me know if this formatting is fine or if I should add incantations to force
it into a different format.

https://codereview.chromium.org/2775143002/diff/1/cc/animation/keyframed_anim...
cc/animation/keyframed_animation_curve_unittest.cc:864: // range [0,1]
On 2017/03/29 at 04:58:30, alancutter wrote:
> Bah these comments, they're just repeating the test name. No need to alter
given it's consistent.

Ack.

https://codereview.chromium.org/2775143002/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/transitions/transitions-parsing.html
(right):

https://codereview.chromium.org/2775143002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/transitions/transitions-parsing.html:487:
shouldBe("computedStyle.webkitTransitionTimingFunction", "'ease'");
On 2017/03/29 at 04:58:31, alancutter wrote:
> I wish there were a helper function like shouldNotParse('pants') or something
to make the intent more clear and reduce repetition but it's okay to leave as is
since it's consistent with the existing code.

Ack.

https://codereview.chromium.org/2775143002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/transitions/transitions-parsing.html:488: 
On 2017/03/29 at 04:58:30, alancutter wrote:
> Let's also test frames() for parse failure.

Done.

[suzyh_UTC10 (ex-contributor), 2017-04-04 04:19:34]

I'm holding off on landing this until after the branch point, so no rush on this
review.


I found a place where my copy-pasted code did not update a variable name ("stf"
becomes "ftf"), so made that last tweak.


On 2017/04/04 at 01:02:04, suzyh wrote:
> > This needs LayoutTests that checks the interpolation state of CSS
Animations, CSS Transitions
> > and Web Animations that use the frames() timing function.
> 
> Mozilla have some tests that are waiting to be upstreamed to the Web Platform
Tests:
>
https://hg.mozilla.org/mozilla-central/file/6d38ad302429/testing/web-platform...
> Once these go up to WPT, they should be imported in via our usual automated
process. The move from csswg-test to wpt/css is not yet finished
(crbug.com/706118), so there's still going to be a delay in having these tests
be available, but they're coming.

I've run these tests locally and discovered some failures in the frames test:

PASS For an input progress of 0.0, the output of a frames timing function is the
first frame 
PASS At a frame boundary, the output of a frames timing function is the next
frame 
PASS For an input progress of 1.0, the output of a frames timing function is the
final frame 
PASS The number of frames is correctly reflected in the frames timing function
output 
PASS The number of frames is correctly reflected in the frames timing function
output on CSS Transitions 
FAIL frames easing with input progress greater than 1 assert_equals: expected
"200px" but got "100px"
FAIL frames easing with input progress greater than 1.5 assert_equals: expected
"300px" but got "100px"
FAIL frames easing with input progress less than 0 assert_equals: expected
"-100px" but got "0px"

We're getting similar failures for some of the step timing function tests:

FAIL step-start easing with input progress greater than 1 assert_equals:
expected "200px" but got "100px"
PASS step-end easing with input progress greater than 1 
FAIL step-end easing with input progress greater than 2 assert_equals: expected
"200px" but got "100px"
PASS step-start easing with input progress less than 0 
FAIL step-start easing with input progress less than -1 assert_equals: expected
"-100px" but got "0px"
FAIL step-end easing with input progress less than 0 assert_equals: expected
"-100px" but got "0px"

I think there's something here that we'll need to fix but that is not a new
problem.

(We're also getting failures for the cubic-bezier easing test which we can worry
about later.)

[alancutter (OOO until 2018), 2017-04-04 04:24:39]

On 2017/04/04 at 04:19:34, suzyh wrote:
> FAIL step-start easing with input progress greater than 1 assert_equals:
expected "200px" but got "100px"
> PASS step-end easing with input progress greater than 1 
> FAIL step-end easing with input progress greater than 2 assert_equals:
expected "200px" but got "100px"
> PASS step-start easing with input progress less than 0 
> FAIL step-start easing with input progress less than -1 assert_equals:
expected "-100px" but got "0px"
> FAIL step-end easing with input progress less than 0 assert_equals: expected
"-100px" but got "0px"
> 
> I think there's something here that we'll need to fix but that is not a new
problem.

Probably just need to remove the MathUtil::ClampToRange() call.

[suzyh_UTC10 (ex-contributor), 2017-04-04 04:37:59]

On 2017/04/04 at 04:24:39, alancutter wrote:
> On 2017/04/04 at 04:19:34, suzyh wrote:
> > FAIL step-start easing with input progress greater than 1 assert_equals:
expected "200px" but got "100px"
> > PASS step-end easing with input progress greater than 1 
> > FAIL step-end easing with input progress greater than 2 assert_equals:
expected "200px" but got "100px"
> > PASS step-start easing with input progress less than 0 
> > FAIL step-start easing with input progress less than -1 assert_equals:
expected "-100px" but got "0px"
> > FAIL step-end easing with input progress less than 0 assert_equals: expected
"-100px" but got "0px"
> > 
> > I think there's something here that we'll need to fix but that is not a new
problem.
> 
> Probably just need to remove the MathUtil::ClampToRange() call.

Nope, that gives a different pattern; some PASS that previously FAILed, and vice
versa.

[suzyh_UTC10 (ex-contributor), 2017-04-10 05:34:31]

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

[commit-bot: I haz the power, 2017-04-10 05:34:39]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[suzyh_UTC10 (ex-contributor), 2017-04-10 06:03:26]

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

[commit-bot: I haz the power, 2017-04-10 06:03:41]

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-04-10 07:24:56]

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

[commit-bot: I haz the power, 2017-04-10 07:24:57]

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_...)

[suzyh_UTC10 (ex-contributor), 2017-04-10 07:58:35]

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

[commit-bot: I haz the power, 2017-04-10 07:58: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-04-10 09:39:23]

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

[commit-bot: I haz the power, 2017-04-10 09:39:24]

Dry run: This issue passed the CQ dry run.

[suzyh_UTC10 (ex-contributor), 2017-04-19 08:20:19]

alancutter PTAL

Branch point has passed, so we can proceed with getting this landed once you're
OK with it.

Since last time I've changed the implementation of the timing function to match
spec for inputs outside the range [0,1], which enables us to pass the
css-timing-1 frames tests. (The equivalent change to fix the steps tests is in a
separate patch which this now depends on.)

[suzyh_UTC10 (ex-contributor), 2017-04-19 08:20:28]

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

[commit-bot: I haz the power, 2017-04-19 08:20:48]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[alancutter (OOO until 2018), 2017-04-19 08:33:08]

lgtm

[suzyh_UTC10 (ex-contributor), 2017-04-19 08:41:07]

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

[suzyh_UTC10 (ex-contributor), 2017-04-19 08:41:19]

+ajuma for cc OWNERS

[commit-bot: I haz the power, 2017-04-19 10:44:06]

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

[commit-bot: I haz the power, 2017-04-19 10:44:07]

Dry run: This issue passed the CQ dry run.

[ajuma, 2017-04-19 14:40:35]

cc lgtm

[suzyh_UTC10 (ex-contributor), 2017-04-20 00:55:51]

The CQ bit was checked by suzyh@chromium.org

[commit-bot: I haz the power, 2017-04-20 00:56:36]

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

[suzyh_UTC10 (ex-contributor), 2017-04-20 01:02:58]

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

[suzyh_UTC10 (ex-contributor), 2017-04-20 01:03:19]

+caseq for Source/devtools/front_end OWNERS

[commit-bot: I haz the power, 2017-04-20 01:08:28]

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

[commit-bot: I haz the power, 2017-04-20 01:08:30]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[suzyh_UTC10 (ex-contributor), 2017-04-20 01:14:12]

suzyh@chromium.org changed reviewers:
+ dgozman@chromium.org
- caseq@chromium.org

[suzyh_UTC10 (ex-contributor), 2017-04-20 01:14:49]

-caseq (OOO)
+dgozman
PTAL for Source/devtools OWNERS

[dgozman, 2017-04-20 06:16:04]

devtools lgtm

[suzyh_UTC10 (ex-contributor), 2017-04-20 06:19:07]

The CQ bit was checked by suzyh@chromium.org

[commit-bot: I haz the power, 2017-04-20 06:19:33]

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-04-20 06:24:26]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1492669147767210,
"parent_rev": "9d28c22de85528d1f87bfbf3f88dc531027b24bd", "commit_rev":
"1b0bb2fe0fb6094a235a10b4e7d921c9b04e6d2e"}

[commit-bot: I haz the power, 2017-04-20 06:24:33]

Description was changed from

==========
Implement frames() timing function

This patch adds support for the frames timing function defined in
https://www.w3.org/TR/css-timing-1/#frames-timing-functions, based on
the implementation for the steps timing function.

BUG=646265
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
Implement frames() timing function

This patch adds support for the frames timing function defined in
https://www.w3.org/TR/css-timing-1/#frames-timing-functions, based on
the implementation for the steps timing function.

BUG=646265
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2775143002
Cr-Commit-Position: refs/heads/master@{#465916}
Committed:
https://chromium.googlesource.com/chromium/src/+/1b0bb2fe0fb6094a235a10b4e7d9...
==========

[commit-bot: I haz the power, 2017-04-20 06:24:34]

Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/1b0bb2fe0fb6094a235a10b4e7d9...