Web Animations CSS: Fix clamping to non-negative values for BorderImageLength

Web Animations CSS: Fix clamping to non-negative values for BorderImageLength

Also beef up a number of tests that use this code

BUG=257591
R=alancutter@chromium.org, alancutter

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

[Steve Block, 2013-11-01 02:25:38]

Note that this depends on https://codereview.chromium.org/54293006

[alancutter (OOO until 2018), 2013-11-02 00:58:01]

Adding Relative to AnimatableLength may not be the right way to go about this.
As far as I can tell a CSS number maps onto two different internal
representations in render styles; numbers and Relative Lengths. The purpose of a
Relative Length is to represent CSS numbers in a length box.
Our AnimatableLengthBox (which should really be called AnimatableBox) can handle
numbers without the need to extend AnimatableLength and have two ways of
representing the same thing.

I think we should stick to the CSS way of representing values as that is what is
in store for us in the future with the Web Animations API and use
AnimatableNumbers that convert into Relative Lengths instead of extending
AnimatableLength.

[Timothy Loh, 2013-11-02 01:28:35]

We don't actually need to worry about this, davve is removing the Relative enum
from Length in a series of patches (crrev.com/55783002 crrev.com/55813002
crrev.com/55833002 crrev.com/55843002) and fixing our handling of this as well.
Still worthwhile to add tests probably, but adding Relative to AnimatableLength
here is probably wrong (we don't want them to interpolate with lengths/percents
afaict) and just going to conflict with davve's work.

[Steve Block, 2013-11-03 08:53:45]

> We don't actually need to worry about this, davve is removing the Relative
enum
> from Length in a series of patches
Yes, just seen this.

> Still worthwhile to add tests probably, but adding Relative to
AnimatableLength
> here is probably wrong (we don't want them to interpolate with
lengths/percents
> afaict)
It looks like jchaffraix thinks we should interpolate between numbers and
lengths, presumably with a calc.

I any case, I'll wait for davve's patches to land before deciding where this
should go.

[Steve Block, 2013-11-03 23:53:38]

> It looks like jchaffraix thinks we should interpolate between numbers and
> lengths, presumably with a calc.
See
https://codereview.chromium.org/55813002/diff/1/Source/core/frame/animation/C...

[Steve Block, 2013-11-13 05:23:11]

PTAL

https://codereview.chromium.org/54123007/diff/80001/LayoutTests/virtual/web-a...
File
LayoutTests/virtual/web-animations-css/animations/interpolation/border-image-outset-interpolation-expected.txt
(right):

https://codereview.chromium.org/54123007/diff/80001/LayoutTests/virtual/web-a...
LayoutTests/virtual/web-animations-css/animations/interpolation/border-image-outset-interpolation-expected.txt:17:
FAIL: border-image-outset from [0] to [5] was [0] at 0.1, expected [0.5]
This looks like a bug to me. Note that the legacy implementation does not suffer
from this problem, for this property at least. Will fix in a separate patch.

https://codereview.chromium.org/54123007/diff/80001/LayoutTests/virtual/web-a...
File
LayoutTests/virtual/web-animations-css/animations/interpolation/border-image-width-interpolation-expected.txt
(right):

https://codereview.chromium.org/54123007/diff/80001/LayoutTests/virtual/web-a...
LayoutTests/virtual/web-animations-css/animations/interpolation/border-image-width-interpolation-expected.txt:33:
FAIL: border-image-width from [10%] to [20px] was [calc(13% + -6px)] at -0.3,
expected [calc(13% + -6px)] (parsed as [7px])
I think this is a bug too. The spec for border-image-width says that the
computed value should make all lengths absolute. Will fix with a later patch

[alancutter (OOO until 2018), 2013-11-13 07:48:27]

https://codereview.chromium.org/54123007/diff/80001/LayoutTests/virtual/web-a...
File
LayoutTests/virtual/web-animations-css/animations/interpolation/border-image-outset-interpolation-expected.txt
(right):

https://codereview.chromium.org/54123007/diff/80001/LayoutTests/virtual/web-a...
LayoutTests/virtual/web-animations-css/animations/interpolation/border-image-outset-interpolation-expected.txt:17:
FAIL: border-image-outset from [0] to [5] was [0] at 0.1, expected [0.5]
On 2013/11/13 05:23:12, Steve Block wrote:
> This looks like a bug to me. Note that the legacy implementation does not
suffer
> from this problem, for this property at least. Will fix in a separate patch.

I wonder if this is due to the clampTo call added in AnimatedStyleBuilder.cpp.

https://codereview.chromium.org/54123007/diff/80001/LayoutTests/virtual/web-a...
File
LayoutTests/virtual/web-animations-css/animations/interpolation/border-image-width-interpolation-expected.txt
(right):

https://codereview.chromium.org/54123007/diff/80001/LayoutTests/virtual/web-a...
LayoutTests/virtual/web-animations-css/animations/interpolation/border-image-width-interpolation-expected.txt:33:
FAIL: border-image-width from [10%] to [20px] was [calc(13% + -6px)] at -0.3,
expected [calc(13% + -6px)] (parsed as [7px])
On 2013/11/13 05:23:12, Steve Block wrote:
> I think this is a bug too. The spec for border-image-width says that the
> computed value should make all lengths absolute. Will fix with a later patch

Looking at the code for applying border-image-width styles it seems like
percentages in calcs are treated as px. Most definitely a bug. ):

[Steve Block, 2013-11-13 08:50:44]

https://codereview.chromium.org/54123007/diff/80001/LayoutTests/virtual/web-a...
File
LayoutTests/virtual/web-animations-css/animations/interpolation/border-image-outset-interpolation-expected.txt
(right):

https://codereview.chromium.org/54123007/diff/80001/LayoutTests/virtual/web-a...
LayoutTests/virtual/web-animations-css/animations/interpolation/border-image-outset-interpolation-expected.txt:17:
FAIL: border-image-outset from [0] to [5] was [0] at 0.1, expected [0.5]
Good catch, you're right - it was picking up the wrong template instantiation

https://codereview.chromium.org/54123007/diff/80001/LayoutTests/virtual/web-a...
File
LayoutTests/virtual/web-animations-css/animations/interpolation/border-image-width-interpolation-expected.txt
(right):

https://codereview.chromium.org/54123007/diff/80001/LayoutTests/virtual/web-a...
LayoutTests/virtual/web-animations-css/animations/interpolation/border-image-width-interpolation-expected.txt:33:
FAIL: border-image-width from [10%] to [20px] was [calc(13% + -6px)] at -0.3,
expected [calc(13% + -6px)] (parsed as [7px])
> Looking at the code for applying border-image-width styles it seems like
> percentages in calcs are treated as px. Most definitely a bug. ):
I'll add some comments to the test to make clear what's going on

Isn't it also a bug that getComputedStyle() returns percentages in a calc at
all? Shouldn't all percentages be resolved to px in the used value?

[alancutter (OOO until 2018), 2013-11-13 09:04:59]

lgtm

https://codereview.chromium.org/54123007/diff/80001/LayoutTests/virtual/web-a...
File
LayoutTests/virtual/web-animations-css/animations/interpolation/border-image-width-interpolation-expected.txt
(right):

https://codereview.chromium.org/54123007/diff/80001/LayoutTests/virtual/web-a...
LayoutTests/virtual/web-animations-css/animations/interpolation/border-image-width-interpolation-expected.txt:33:
FAIL: border-image-width from [10%] to [20px] was [calc(13% + -6px)] at -0.3,
expected [calc(13% + -6px)] (parsed as [7px])
On 2013/11/13 08:50:44, Steve Block wrote:
> > Looking at the code for applying border-image-width styles it seems like
> > percentages in calcs are treated as px. Most definitely a bug. ):
> I'll add some comments to the test to make clear what's going on
> 
> Isn't it also a bug that getComputedStyle() returns percentages in a calc at
> all? Shouldn't all percentages be resolved to px in the used value?

getComputedStyle() only return used values for a few properties, the remaining
have their computed values returned.
http://dev.w3.org/csswg/cssom/#resolved-values

[Steve Block, 2013-11-13 23:29:45]

> getComputedStyle() only return used values for a few properties, the remaining
> have their computed values returned.
Right, was forgetting that new properties use the computed values

[commit-bot: I haz the power, 2013-11-13 23:37:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/steveblock@chromium.org/54123007/170001

[commit-bot: I haz the power, 2013-11-14 03:15:53]

Retried try job too often on linux_blink_rel for step(s) webkit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_blin...

[Steve Block, 2013-11-14 03:17:26]

Bot failure is existing flakiness, landing manually.

[Steve Block, 2013-11-14 03:18:53]

Committed patchset #3 manually as r161971 (presubmit successful).