Use PathSegmentData+SVGPath{Consumer,Source} in PathSVGInterpolation

Use PathSegmentData+SVGPath{Consumer,Source} in PathSVGInterpolation

Drive the generation of SVG paths using SVGPathParser, and use
SVGPathSource and SVGPathByteStreams to avoid using the SVGPathSeg*
interfaces which will be going away RSN.

BUG=539385
Committed: https://crrev.com/5e72acc3d18c197ace1654253dc5d90aaa4530d8
Cr-Commit-Position: refs/heads/master@{#355759}

[fs, 2015-10-21 21:29:21]

fs@opera.com changed reviewers:
+ ericwilligers@chromium.org, pdr@chromium.org

[fs, 2015-10-21 21:29:21]

This is an intermediary step towards dropping SVGPathSegList - the only changes
needed in PathSVGInterpolation after this should be simple name changes and the
odd removal (or, well, so I hope...). Maybe a bit rough in some areas - I'm
primarily thinking of the traversal in maybeCreate(), which I left as
pre-allocating with the downside of having to walk one of the inputs twice et.c.
Since the length() method is about to disappear though it's better to not use it
(still a linear operation and so on.)

Eric: PTAL at animation/
Philip: svg/ tomfoolery - congratulations!

[pdr., 2015-10-22 04:53:24]

SVG looks great, just one question about the overall plan.

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

https://codereview.chromium.org/1408143004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/animation/PathSVGInterpolation.cpp:438:
InterpolablePathSource source(toInterpolableList(value), pathSegTypes);
(Checking my high-level understanding) Is the plan for this to only temporarily
be exposed during the transition, with SVGPathSegList (internal and external
usage) being removed in a followup?

https://codereview.chromium.org/1408143004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/svg/SVGPathSegList.cpp (right):

https://codereview.chromium.org/1408143004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/svg/SVGPathSegList.cpp:96:
ASSERT(Base::isEmpty());
We could invalidate the list here instead of exposing invalidateList. If this is
going away in the next patch, don't even worry about it.

[fs, 2015-10-22 07:44:32]

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

https://codereview.chromium.org/1408143004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/animation/PathSVGInterpolation.cpp:438:
InterpolablePathSource source(toInterpolableList(value), pathSegTypes);
On 2015/10/22 at 04:53:24, pdr wrote:
> (Checking my high-level understanding) Is the plan for this to only
temporarily be exposed during the transition, with SVGPathSegList (internal and
external usage) being removed in a followup?

Assuming you meant the invalidateList()-bit - yes that's temporary. (There may
be more cleanup opportunities here, w.r.t the creation of the new path data,
when all is said and done with the removal.)

https://codereview.chromium.org/1408143004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/svg/SVGPathSegList.cpp (right):

https://codereview.chromium.org/1408143004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/svg/SVGPathSegList.cpp:96:
ASSERT(Base::isEmpty());
On 2015/10/22 at 04:53:24, pdr wrote:
> We could invalidate the list here instead of exposing invalidateList. If this
is going away in the next patch, don't even worry about it.

Yes, this (and invalidateList) will be going away when the list does.

[pdr., 2015-10-22 18:44:16]

On 2015/10/22 at 07:44:32, fs wrote:
>
https://codereview.chromium.org/1408143004/diff/20001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/animation/PathSVGInterpolation.cpp
(right):
> 
>
https://codereview.chromium.org/1408143004/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/animation/PathSVGInterpolation.cpp:438:
InterpolablePathSource source(toInterpolableList(value), pathSegTypes);
> On 2015/10/22 at 04:53:24, pdr wrote:
> > (Checking my high-level understanding) Is the plan for this to only
temporarily be exposed during the transition, with SVGPathSegList (internal and
external usage) being removed in a followup?
> 
> Assuming you meant the invalidateList()-bit - yes that's temporary. (There may
be more cleanup opportunities here, w.r.t the creation of the new path data,
when all is said and done with the removal.)
> 
>
https://codereview.chromium.org/1408143004/diff/20001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/svg/SVGPathSegList.cpp (right):
> 
>
https://codereview.chromium.org/1408143004/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/svg/SVGPathSegList.cpp:96:
ASSERT(Base::isEmpty());
> On 2015/10/22 at 04:53:24, pdr wrote:
> > We could invalidate the list here instead of exposing invalidateList. If
this is going away in the next patch, don't even worry about it.
> 
> Yes, this (and invalidateList) will be going away when the list does.

LGTM here.

@ericwilligers, does the PathSVGInterpolation code look good to you?

[Eric Willigers, 2015-10-23 02:58:23]

> @ericwilligers, does the PathSVGInterpolation code look good to you?

'InterpolablePathSource' isn't an InterpolableValue so the name isn't perfect.

Perhaps InterpolatedPathSource is slightly better. Otherwise, lgtm.

[fs, 2015-10-23 07:44:50]

On 2015/10/23 at 02:58:23, ericwilligers wrote:
> > @ericwilligers, does the PathSVGInterpolation code look good to you?
> 
> 'InterpolablePathSource' isn't an InterpolableValue so the name isn't perfect.
> 
> Perhaps InterpolatedPathSource is slightly better. Otherwise, lgtm.

Renamed.

[fs, 2015-10-23 08:18:28]

The CQ bit was checked by fs@opera.com

[fs, 2015-10-23 08:18:28]

The patchset sent to the CQ was uploaded after l-g-t-m from pdr@chromium.org,
ericwilligers@chromium.org
Link to the patchset: https://codereview.chromium.org/1408143004/#ps40001
(title: "InterpolatedPathSource")

[commit-bot: I haz the power, 2015-10-23 08:18:55]

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

[commit-bot: I haz the power, 2015-10-23 09:43:02]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-10-23 09:44:27]

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