Add composition support for SVG Web Animation of x, y, dx, dy attributes

Add composition support for SVG Web Animation of x, y, dx, dy attributes

This change adds SVGLengthListInterpolationType to enable additive
and neutral Web Animation keyframes for SVG attributes: x, y, dx, dy

This is a continuation of: https://codereview.chromium.org/1535523002

LayoutTests have been updated to test lengths, length lists and numbers
separately on these attributes.

BUG=530436
Committed: https://crrev.com/3c5fdeeaf0e31f0982e1f457b54067b8dc2348fa
Cr-Commit-Position: refs/heads/master@{#369353}

[alancutter (OOO until 2018), 2016-01-13 07:28:49]

Description was changed from

==========
Add composition support for SVG Web Animation of x, y, dx, dy attributes

This change adds SVGLengthListInterpolationType to enable additive
and neutral Web Animation keyframes for SVG attributes: x, y, dx, dy

BUG=530436
==========

to

==========
Add composition support for SVG Web Animation of x, y, dx, dy attributes

This change adds SVGLengthListInterpolationType to enable additive
and neutral Web Animation keyframes for SVG attributes: x, y, dx, dy

This is a continuation of: https://codereview.chromium.org/1535523002

BUG=530436
==========

[alancutter (OOO until 2018), 2016-01-13 07:32:46]

Description was changed from

==========
Add composition support for SVG Web Animation of x, y, dx, dy attributes

This change adds SVGLengthListInterpolationType to enable additive
and neutral Web Animation keyframes for SVG attributes: x, y, dx, dy

This is a continuation of: https://codereview.chromium.org/1535523002

BUG=530436
==========

to

==========
Add composition support for SVG Web Animation of x, y, dx, dy attributes

This change adds SVGLengthListInterpolationType to enable additive
and neutral Web Animation keyframes for SVG attributes: x, y, dx, dy

This is a continuation of: https://codereview.chromium.org/1535523002

LayoutTests have been updated to test lengths, length lists and numbers
separately on these attributes.

BUG=530436
==========

[alancutter (OOO until 2018), 2016-01-13 07:32:46]

alancutter@chromium.org changed reviewers:
+ dstockwell@chromium.org, fs@opera.com

[fs, 2016-01-13 12:20:24]

LGTM w/ ultra nits

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

https://codereview.chromium.org/1587533002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/animation/SVGLengthInterpolationType.cpp:102:
PassRefPtrWillBeRawPtr<SVGLength>
SVGLengthInterpolationType::resolveInterpolableSVGLength(const
InterpolableValue& interpolableValue, const SVGLengthContext& lengthContext,
const SVGLengthMode& unitMode, bool negativeValuesForbidden)
uNit: SVGLengthMode is an enum, so passing as a reference is a bit overkill.

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

https://codereview.chromium.org/1587533002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/animation/SVGLengthListInterpolationType.cpp:71:
SVGElement& element = environment.svgElement();
uNit: const?

[suzyh_UTC10 (ex-contributor), 2016-01-13 23:33:58]

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

[suzyh_UTC10 (ex-contributor), 2016-01-13 23:34:00]

Thanks for taking over this patch for me, Alan! You did more than I expected ^.^

I have two minor comments to add.

https://codereview.chromium.org/1587533002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/animations/svg-attribute-composition/svg-dx-dy-length-list-composition.html
(right):

https://codereview.chromium.org/1587533002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/animations/svg-attribute-composition/svg-dx-dy-length-list-composition.html:14:
underlying: '2',
Is this file supposed to only have lists, or is this testing a list of length 1?

https://codereview.chromium.org/1587533002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/animation/LengthSVGInterpolation.h (left):

https://codereview.chromium.org/1587533002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/animation/LengthSVGInterpolation.h:1: //
Copyright 2015 The Chromium Authors. All rights reserved.
You can also delete the LengthSVGInterpolation.cpp file.

[suzyh_UTC10 (ex-contributor), 2016-01-13 23:35:53]

suzyh@chromium.org changed reviewers:
- suzyh@chromium.org

[alancutter (OOO until 2018), 2016-01-14 00:44:01]

Thanks for the reviews.


> LGTM w/ ultra nits

These are reasonable nits to me!

https://codereview.chromium.org/1587533002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/animations/svg-attribute-composition/svg-dx-dy-length-list-composition.html
(right):

https://codereview.chromium.org/1587533002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/animations/svg-attribute-composition/svg-dx-dy-length-list-composition.html:14:
underlying: '2',
On 2016/01/13 at 23:34:00, suzyh wrote:
> Is this file supposed to only have lists, or is this testing a list of length
1?

The latter, it is a length list by virtue of being the dx attribute on a <text>
element.

https://codereview.chromium.org/1587533002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/animation/LengthSVGInterpolation.h (left):

https://codereview.chromium.org/1587533002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/animation/LengthSVGInterpolation.h:1: //
Copyright 2015 The Chromium Authors. All rights reserved.
On 2016/01/13 at 23:34:00, suzyh wrote:
> You can also delete the LengthSVGInterpolation.cpp file.

Done.

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

https://codereview.chromium.org/1587533002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/animation/SVGLengthInterpolationType.cpp:102:
PassRefPtrWillBeRawPtr<SVGLength>
SVGLengthInterpolationType::resolveInterpolableSVGLength(const
InterpolableValue& interpolableValue, const SVGLengthContext& lengthContext,
const SVGLengthMode& unitMode, bool negativeValuesForbidden)
On 2016/01/13 at 12:20:24, fs wrote:
> uNit: SVGLengthMode is an enum, so passing as a reference is a bit overkill.

Done.

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

https://codereview.chromium.org/1587533002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/animation/SVGLengthListInterpolationType.cpp:71:
SVGElement& element = environment.svgElement();
On 2016/01/13 at 12:20:24, fs wrote:
> uNit: const?

element gets mutated at the end of this function.

[alancutter (OOO until 2018), 2016-01-14 00:44:06]

The CQ bit was checked by alancutter@chromium.org

[alancutter (OOO until 2018), 2016-01-14 00:44:06]

The patchset sent to the CQ was uploaded after l-g-t-m from fs@opera.com
Link to the patchset: https://codereview.chromium.org/1587533002/#ps40001
(title: "Review changes")

[commit-bot: I haz the power, 2016-01-14 00:47:33]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1587533002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1587533002/40001

[commit-bot: I haz the power, 2016-01-14 02:53:26]

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

[commit-bot: I haz the power, 2016-01-14 02:53:26]

Try jobs failed on following builders:
  android_arm64_dbg_recipe on tryserver.chromium.android (JOB_TIMED_OUT, no
build URL)
  android_chromium_gn_compile_dbg on tryserver.chromium.android (JOB_TIMED_OUT,
no build URL)
  android_chromium_gn_compile_rel on tryserver.chromium.android (JOB_TIMED_OUT,
no build URL)
  android_clang_dbg_recipe on tryserver.chromium.android (JOB_TIMED_OUT, no
build URL)
  android_compile_dbg on tryserver.chromium.android (JOB_TIMED_OUT, no build
URL)
  cast_shell_android on tryserver.chromium.android (JOB_TIMED_OUT, no build URL)
  linux_android_rel_ng on tryserver.chromium.android (JOB_TIMED_OUT, no build
URL)

[dstockwell, 2016-01-14 05:47:17]

lgtm

[alancutter (OOO until 2018), 2016-01-14 06:38:10]

The CQ bit was checked by alancutter@chromium.org

[commit-bot: I haz the power, 2016-01-14 06:38:35]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1587533002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1587533002/40001

[commit-bot: I haz the power, 2016-01-14 06:45:15]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-01-14 06:46:19]

Description was changed from

==========
Add composition support for SVG Web Animation of x, y, dx, dy attributes

This change adds SVGLengthListInterpolationType to enable additive
and neutral Web Animation keyframes for SVG attributes: x, y, dx, dy

This is a continuation of: https://codereview.chromium.org/1535523002

LayoutTests have been updated to test lengths, length lists and numbers
separately on these attributes.

BUG=530436
==========

to

==========
Add composition support for SVG Web Animation of x, y, dx, dy attributes

This change adds SVGLengthListInterpolationType to enable additive
and neutral Web Animation keyframes for SVG attributes: x, y, dx, dy

This is a continuation of: https://codereview.chromium.org/1535523002

LayoutTests have been updated to test lengths, length lists and numbers
separately on these attributes.

BUG=530436

Committed: https://crrev.com/3c5fdeeaf0e31f0982e1f457b54067b8dc2348fa
Cr-Commit-Position: refs/heads/master@{#369353}
==========

[commit-bot: I haz the power, 2016-01-14 06:46:20]

Patchset 3 (id:??) landed as
https://crrev.com/3c5fdeeaf0e31f0982e1f457b54067b8dc2348fa
Cr-Commit-Position: refs/heads/master@{#369353}