Implement 'scroll-behavior' parsing

Implement 'scroll-behavior' parsing

This implements parsing for the 'scroll-behavior' CSS
property, part of the CSSOM View smooth scroll API.

This property is protected by a runtime flag.

Intent-to-implement:
https://groups.google.com/a/chromium.org/forum/#!msg/blink-dev/YLWs9N46XjQ/mK7rCJ26BqAJ

Spec: http://www.w3.org/TR/cssom-view/


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

[ajuma, 2014-01-21 18:41:27]

https://codereview.chromium.org/140253004/diff/1/Source/build/scripts/name_ut...
File Source/build/scripts/name_utilities.py (right):

https://codereview.chromium.org/140253004/diff/1/Source/build/scripts/name_ut...
Source/build/scripts/name_utilities.py:34: 'CSSOM',
This change allows the runtime flag to be "CSSOMSmoothScroll".

[Julien - ping for review, 2014-01-22 16:24:34]

lgtm

https://codereview.chromium.org/140253004/diff/1/LayoutTests/fast/scroll-beha...
File LayoutTests/fast/scroll-behavior/parse-scroll-behavior.html (right):

https://codereview.chromium.org/140253004/diff/1/LayoutTests/fast/scroll-beha...
LayoutTests/fast/scroll-behavior/parse-scroll-behavior.html:62: </body>
I would throw an inheritance test for good measure, this would just double check
that we don't inherit the property by mistake.

https://codereview.chromium.org/140253004/diff/1/Source/core/css/CSSPropertie...
File Source/core/css/CSSProperties.in (right):

https://codereview.chromium.org/140253004/diff/1/Source/core/css/CSSPropertie...
Source/core/css/CSSProperties.in:108: scroll-behavior type_name=ScrollBehavior,
initial=initialScrollBehavior
Do we really need initial part? (I thought it would be inferred automatically
from the property name)

https://codereview.chromium.org/140253004/diff/1/Source/core/css/parser/Bison...
File Source/core/css/parser/BisonCSSParser-in.cpp (right):

https://codereview.chromium.org/140253004/diff/1/Source/core/css/parser/Bison...
Source/core/css/parser/BisonCSSParser-in.cpp:579: if
(RuntimeEnabledFeatures::cssomSmoothScrollEnabled() && (valueID ==
CSSValueInstant || valueID == CSSValueSmooth))
I much prefer to put the following as it underlines the logic better:

if (valueID == CSSValueInstant || valueID == CSSValueSmooth)
    return RuntimeEnabledFeatures::cssomSmoothScrollEnabled()

https://codereview.chromium.org/140253004/diff/1/Source/core/rendering/style/...
File Source/core/rendering/style/StyleRareNonInheritedData.h (right):

https://codereview.chromium.org/140253004/diff/1/Source/core/rendering/style/...
Source/core/rendering/style/StyleRareNonInheritedData.h:200: unsigned
m_scrollBehavior: 2; // ScrollBehavior
It's bad that we are using an extra bit for no good reason: the accepted values
are "instant" or "smooth" which is really a binary choice. I don't think we
should modify the code though but maybe worth mentioning it in a comment?

[ajuma, 2014-01-22 19:50:19]

Thanks for the review!

https://codereview.chromium.org/140253004/diff/1/LayoutTests/fast/scroll-beha...
File LayoutTests/fast/scroll-behavior/parse-scroll-behavior.html (right):

https://codereview.chromium.org/140253004/diff/1/LayoutTests/fast/scroll-beha...
LayoutTests/fast/scroll-behavior/parse-scroll-behavior.html:62: </body>
On 2014/01/22 16:24:35, Julien Chaffraix - PST wrote:
> I would throw an inheritance test for good measure, this would just double
check
> that we don't inherit the property by mistake.

Done.

https://codereview.chromium.org/140253004/diff/1/Source/core/css/CSSPropertie...
File Source/core/css/CSSProperties.in (right):

https://codereview.chromium.org/140253004/diff/1/Source/core/css/CSSPropertie...
Source/core/css/CSSProperties.in:108: scroll-behavior type_name=ScrollBehavior,
initial=initialScrollBehavior
On 2014/01/22 16:24:35, Julien Chaffraix - PST wrote:
> Do we really need initial part? (I thought it would be inferred automatically
> from the property name)

You're right, this part wasn't needed. Removed.

https://codereview.chromium.org/140253004/diff/1/Source/core/css/parser/Bison...
File Source/core/css/parser/BisonCSSParser-in.cpp (right):

https://codereview.chromium.org/140253004/diff/1/Source/core/css/parser/Bison...
Source/core/css/parser/BisonCSSParser-in.cpp:579: if
(RuntimeEnabledFeatures::cssomSmoothScrollEnabled() && (valueID ==
CSSValueInstant || valueID == CSSValueSmooth))
On 2014/01/22 16:24:35, Julien Chaffraix - PST wrote:
> I much prefer to put the following as it underlines the logic better:
> 
> if (valueID == CSSValueInstant || valueID == CSSValueSmooth)
>     return RuntimeEnabledFeatures::cssomSmoothScrollEnabled()

Done.

https://codereview.chromium.org/140253004/diff/1/Source/core/rendering/style/...
File Source/core/rendering/style/StyleRareNonInheritedData.h (right):

https://codereview.chromium.org/140253004/diff/1/Source/core/rendering/style/...
Source/core/rendering/style/StyleRareNonInheritedData.h:200: unsigned
m_scrollBehavior: 2; // ScrollBehavior
On 2014/01/22 16:24:35, Julien Chaffraix - PST wrote:
> It's bad that we are using an extra bit for no good reason: the accepted
values
> are "instant" or "smooth" which is really a binary choice. I don't think we
> should modify the code though but maybe worth mentioning it in a comment?

Added a comment.

[commit-bot: I haz the power, 2014-01-22 21:01:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ajuma@chromium.org/140253004/230001

[commit-bot: I haz the power, 2014-01-22 23:35:29]

Change committed as 165568