Use Length for the stroke-dashoffset property in SVGLayoutStyle

Use Length for the stroke-dashoffset property in SVGLayoutStyle

This replaces the SVGLength used for strokeDashOffset in
SVGLayoutStyle with a Length.

BUG=461375
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=191191

[fs, 2015-03-02 16:33:55]

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

[fs, 2015-03-02 16:35:06]

@ericwilligers: I think the animation changes in this one are pretty
straight-forward, but feel free to have a look. the next one will be more
involved ;-)

[pdr., 2015-03-02 19:16:06]

https://codereview.chromium.org/962763003/diff/1/LayoutTests/animations/inter...
File
LayoutTests/animations/interpolation/svg-stroke-dashoffset-interpolation-expected.txt
(left):

https://codereview.chromium.org/962763003/diff/1/LayoutTests/animations/inter...
LayoutTests/animations/interpolation/svg-stroke-dashoffset-interpolation-expected.txt:9:
PASS: stroke-dashoffset from [-20px] to [30em] was [-20px] at -0.25
This change looks great except for these expectation changes. I wouldn't have
expected pixel changes from this change.

[fs, 2015-03-02 22:21:52]

https://codereview.chromium.org/962763003/diff/1/LayoutTests/animations/inter...
File
LayoutTests/animations/interpolation/svg-stroke-dashoffset-interpolation-expected.txt
(left):

https://codereview.chromium.org/962763003/diff/1/LayoutTests/animations/inter...
LayoutTests/animations/interpolation/svg-stroke-dashoffset-interpolation-expected.txt:9:
PASS: stroke-dashoffset from [-20px] to [30em] was [-20px] at -0.25
On 2015/03/02 19:16:06, pdr wrote:
> This change looks great except for these expectation changes. I wouldn't have
> expected pixel changes from this change.

The reason for this is that it is now (stored/handled as) a computed value
(properly, IMHO), while previously it was (...) a specified value - much to the
dismay of the animation people I'm sure =).
I don't have a Gecko at hand ATM, but I'm pretty sure this is what it'll look
like there too. (The animation engine will have this as a sort of calc()
expression internally, but the test framework used here uses getComputedStyle
and some smarts to compare expected and actual values - i.e. the "30em" @ 1 will
actually also yield a "px" result - but ends up outputting the expected value
because the computed(expected) == computed(actual) - IIUC... Eric can probably
confirm.)

(SVG 1.1 wasn't very clear on this I think, and SVG2 looks like it might need
some love in that department...)

[pdr., 2015-03-02 22:42:41]

On 2015/03/02 at 22:21:52, fs wrote:
>
https://codereview.chromium.org/962763003/diff/1/LayoutTests/animations/inter...
> File
LayoutTests/animations/interpolation/svg-stroke-dashoffset-interpolation-expected.txt
(left):
> 
>
https://codereview.chromium.org/962763003/diff/1/LayoutTests/animations/inter...
>
LayoutTests/animations/interpolation/svg-stroke-dashoffset-interpolation-expected.txt:9:
PASS: stroke-dashoffset from [-20px] to [30em] was [-20px] at -0.25
> On 2015/03/02 19:16:06, pdr wrote:
> > This change looks great except for these expectation changes. I wouldn't
have
> > expected pixel changes from this change.
> 
> The reason for this is that it is now (stored/handled as) a computed value
(properly, IMHO), while previously it was (...) a specified value - much to the
dismay of the animation people I'm sure =).
> I don't have a Gecko at hand ATM, but I'm pretty sure this is what it'll look
like there too. (The animation engine will have this as a sort of calc()
expression internally, but the test framework used here uses getComputedStyle
and some smarts to compare expected and actual values - i.e. the "30em" @ 1 will
actually also yield a "px" result - but ends up outputting the expected value
because the computed(expected) == computed(actual) - IIUC... Eric can probably
confirm.)
> 
> (SVG 1.1 wasn't very clear on this I think, and SVG2 looks like it might need
some love in that department...)

lgtm if we match gecko now

[Eric Willigers, 2015-03-03 00:30:46]

lgtm 

For clarity, the .target style should contain an explicit
font-size: 16px;

[fs, 2015-03-03 11:42:19]

On 2015/03/03 00:30:46, Eric Willigers wrote:
> lgtm 
> 
> For clarity, the .target style should contain an explicit
> font-size: 16px;

Added, thanks.

[fs, 2015-03-03 11:43:57]

On 2015/03/02 22:42:41, pdr wrote:
> On 2015/03/02 at 22:21:52, fs wrote:
> >
>
https://codereview.chromium.org/962763003/diff/1/LayoutTests/animations/inter...
> > File
>
LayoutTests/animations/interpolation/svg-stroke-dashoffset-interpolation-expected.txt
> (left):
> > 
> >
>
https://codereview.chromium.org/962763003/diff/1/LayoutTests/animations/inter...
> >
>
LayoutTests/animations/interpolation/svg-stroke-dashoffset-interpolation-expected.txt:9:
> PASS: stroke-dashoffset from [-20px] to [30em] was [-20px] at -0.25
> > On 2015/03/02 19:16:06, pdr wrote:
> > > This change looks great except for these expectation changes. I wouldn't
> have
> > > expected pixel changes from this change.
> > 
> > The reason for this is that it is now (stored/handled as) a computed value
> (properly, IMHO), while previously it was (...) a specified value - much to
the
> dismay of the animation people I'm sure =).
> > I don't have a Gecko at hand ATM, but I'm pretty sure this is what it'll
look
> like there too. (The animation engine will have this as a sort of calc()
> expression internally, but the test framework used here uses getComputedStyle
> and some smarts to compare expected and actual values - i.e. the "30em" @ 1
will
> actually also yield a "px" result - but ends up outputting the expected value
> because the computed(expected) == computed(actual) - IIUC... Eric can probably
> confirm.)
> > 
> > (SVG 1.1 wasn't very clear on this I think, and SVG2 looks like it might
need
> some love in that department...)
> 
> lgtm if we match gecko now

Checked in Gecko: They produce the same values - only without units (quirky
number I suppose - something we could consider too I suppose).

[fs, 2015-03-03 14:12:56]

The CQ bit was checked by fs@opera.com

[fs, 2015-03-03 14:12:56]

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/962763003/#ps20001 (title:
"Make font-size explicit.")

[commit-bot: I haz the power, 2015-03-03 14:13:31]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/962763003/20001

[commit-bot: I haz the power, 2015-03-03 14:16:55]

Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=191191