Support keywords in LengthStyleInterpolation

Support keywords in LengthStyleInterpolation

letter-spacing normal corresponds to 0px
http://dev.w3.org/csswg/css-text-3/#letter-spacing-property

For column-rule-width, outline-width, border-bottom-width etc.:
width thin corresponds to 1px
width medium corresponds to 3px
width thick corresponds to 5px
https://developer.mozilla.org/en-US/docs/Web/CSS/outline-width

Interpolations to and from perspective 'none' now flip, consistent with gecko.

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

[Eric Willigers, 2015-02-25 00:27:55]

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

[Eric Willigers, 2015-03-03 22:40:11]

ericwilligers@chromium.org changed reviewers:
+ dstockwell@chromium.org

[alancutter (OOO until 2018), 2015-03-05 23:38:50]

https://codereview.chromium.org/955863002/diff/1/Source/core/animation/Length...
File Source/core/animation/LengthStyleInterpolation.cpp (left):

https://codereview.chromium.org/955863002/diff/1/Source/core/animation/Length...
Source/core/animation/LengthStyleInterpolation.cpp:32:
OwnPtrWillBeRawPtr<InterpolableList> listOfTypes =
InterpolableList::create(CSSPrimitiveValue::LengthUnitTypeCount);
Do these need to move? I think the original placement made it more obvious what
the shape of the IV is.

https://codereview.chromium.org/955863002/diff/1/Source/core/animation/Length...
File Source/core/animation/LengthStyleInterpolation.cpp (right):

https://codereview.chromium.org/955863002/diff/1/Source/core/animation/Length...
Source/core/animation/LengthStyleInterpolation.cpp:13: bool
LengthStyleInterpolation::canCreateFrom(const CSSValue& value, CSSPropertyID id)
Nit: I think we should name our CSSPropertyID variables "property" instead of
"id".

https://codereview.chromium.org/955863002/diff/1/Source/core/animation/Length...
Source/core/animation/LengthStyleInterpolation.cpp:20: if
(primitiveValue.isValueID()) {
You should only enter this branch if the property has been specified.

https://codereview.chromium.org/955863002/diff/1/Source/core/animation/Length...
Source/core/animation/LengthStyleInterpolation.cpp:28: || (id ==
CSSPropertyPerspective && valueID == CSSValueNone);
The properties should be called out in code, a comment is hard to test.
I'd pull this out into a single switch as bool pixelsForKeyword(CSSPropertyID,
CSSValueID, double& result).

https://codereview.chromium.org/955863002/diff/1/Source/core/animation/Length...
Source/core/animation/LengthStyleInterpolation.cpp:55: arrayOfValues[0] = 1;
Don't index by 0, use CSSPrimitiveValue::UnitTypePixels.

[Eric Willigers, 2015-03-07 23:55:37]

Patchset #2 (id:20001) has been deleted

[Eric Willigers, 2015-03-07 23:58:22]

https://codereview.chromium.org/955863002/diff/1/Source/core/animation/Length...
File Source/core/animation/LengthStyleInterpolation.cpp (left):

https://codereview.chromium.org/955863002/diff/1/Source/core/animation/Length...
Source/core/animation/LengthStyleInterpolation.cpp:32:
OwnPtrWillBeRawPtr<InterpolableList> listOfTypes =
InterpolableList::create(CSSPrimitiveValue::LengthUnitTypeCount);
On 2015/03/05 23:38:50, alancutter wrote:
> Do these need to move? I think the original placement made it more obvious
what
> the shape of the IV is.

Acknowledged.

https://codereview.chromium.org/955863002/diff/1/Source/core/animation/Length...
File Source/core/animation/LengthStyleInterpolation.cpp (right):

https://codereview.chromium.org/955863002/diff/1/Source/core/animation/Length...
Source/core/animation/LengthStyleInterpolation.cpp:13: bool
LengthStyleInterpolation::canCreateFrom(const CSSValue& value, CSSPropertyID id)
On 2015/03/05 23:38:49, alancutter wrote:
> Nit: I think we should name our CSSPropertyID variables "property" instead of
> "id".

Done.

https://codereview.chromium.org/955863002/diff/1/Source/core/animation/Length...
Source/core/animation/LengthStyleInterpolation.cpp:20: if
(primitiveValue.isValueID()) {
On 2015/03/05 23:38:49, alancutter wrote:
> You should only enter this branch if the property has been specified.

Acknowledged.

https://codereview.chromium.org/955863002/diff/1/Source/core/animation/Length...
Source/core/animation/LengthStyleInterpolation.cpp:28: || (id ==
CSSPropertyPerspective && valueID == CSSValueNone);
On 2015/03/05 23:38:50, alancutter wrote:
> The properties should be called out in code, a comment is hard to test.
> I'd pull this out into a single switch as bool pixelsForKeyword(CSSPropertyID,
> CSSValueID, double& result).

Done.

https://codereview.chromium.org/955863002/diff/1/Source/core/animation/Length...
Source/core/animation/LengthStyleInterpolation.cpp:55: arrayOfValues[0] = 1;
On 2015/03/05 23:38:50, alancutter wrote:
> Don't index by 0, use CSSPrimitiveValue::UnitTypePixels.

Done.

[alancutter (OOO until 2018), 2015-03-09 00:05:00]

lgtm.
I suspect we'll eventually want to avoid depending on the CSSPropertyID when
creating interpolations and instead have an aliasing pass e.g.
PassRefPtr<CSSValue> animationValueAlias(CSSPropertyID, CSSValue&);

[alancutter (OOO until 2018), 2015-03-09 00:34:15]

https://codereview.chromium.org/955863002/diff/40001/Source/core/animation/Le...
File Source/core/animation/LengthStyleInterpolation.cpp (right):

https://codereview.chromium.org/955863002/diff/40001/Source/core/animation/Le...
Source/core/animation/LengthStyleInterpolation.cpp:45: result = 0;
Actually we shouldn't be animating perspective: none, we should instead continue
to fall back to legacy and just remove the interpolation behaviour from AV (in a
separate patch).

[Eric Willigers, 2015-03-09 02:04:55]

https://codereview.chromium.org/955863002/diff/40001/Source/core/animation/Le...
File Source/core/animation/LengthStyleInterpolation.cpp (right):

https://codereview.chromium.org/955863002/diff/40001/Source/core/animation/Le...
Source/core/animation/LengthStyleInterpolation.cpp:45: result = 0;
On 2015/03/09 00:34:15, alancutter wrote:
> Actually we shouldn't be animating perspective: none, we should instead
continue
> to fall back to legacy and just remove the interpolation behaviour from AV (in
a
> separate patch).

Done.

[Eric Willigers, 2015-03-09 02:05:07]

The CQ bit was checked by ericwilligers@chromium.org

[Eric Willigers, 2015-03-09 02:05:08]

The patchset sent to the CQ was uploaded after l-g-t-m from
alancutter@chromium.org
Link to the patchset: https://codereview.chromium.org/955863002/#ps60001 (title:
"flip perspective none")

[commit-bot: I haz the power, 2015-03-09 02:06:16]

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

[commit-bot: I haz the power, 2015-03-09 08:53:51]

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

[commit-bot: I haz the power, 2015-03-09 08:53:52]

Try jobs failed on following builders:
  linux_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/5...)

[Eric Willigers, 2015-03-09 09:33:46]

On 2015/03/09 08:53:52, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   linux_blink_rel on tryserver.blink (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/5...)

Transitions test needed updating, as we no longer interpolate from default
perspective (none).

[Eric Willigers, 2015-03-09 09:33:57]

The CQ bit was checked by ericwilligers@chromium.org

[Eric Willigers, 2015-03-09 09:33:57]

The patchset sent to the CQ was uploaded after l-g-t-m from
alancutter@chromium.org
Link to the patchset: https://codereview.chromium.org/955863002/#ps80001 (title:
"transitions test with interpolated perspective")

[commit-bot: I haz the power, 2015-03-09 09:34:35]

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

[commit-bot: I haz the power, 2015-03-09 11:04:02]

Committed patchset #4 (id:80001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=191535