Implemented applyPropertyAll for applying all shorthand property.

Implemented applyPropertyAll for applying all shorthand property.

applyPropertyAll expands all shorthand property to longhand properties,
and apply each longhand property according to all's value (e.g. if
all's value is initial, initial).

Spec: http://dev.w3.org/csswg/css-cascade/#all-shorthand

BUG=172051
TEST=fast/css/all-shorthand.html

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

[tasak, 2014-06-17 11:07:32]

Would you review this CL?

This is a patch for applying all shorthand property.
I added comment to applyAllProperty(), firstCSSPropertyId<>(), and
lastCSSPropertyId<>().

[dglazkov, 2014-06-17 15:13:14]

lgtm

https://codereview.chromium.org/339163002/diff/40001/Source/core/css/StylePro...
File Source/core/css/StylePropertyShorthandCustom.cpp (right):

https://codereview.chromium.org/339163002/diff/40001/Source/core/css/StylePro...
Source/core/css/StylePropertyShorthandCustom.cpp:121: // "font: initial;
font-family: initial;...". To avoid this, we need to
This seems like a bug in our code? I remember someone working on this, but can't
recall who that was.

https://codereview.chromium.org/339163002/diff/40001/Source/core/css/resolver...
File Source/core/css/resolver/StyleResolver.cpp (right):

https://codereview.chromium.org/339163002/diff/40001/Source/core/css/resolver...
Source/core/css/resolver/StyleResolver.cpp:1239: template<> CSSPropertyID
StyleResolver::firstCSSPropertyId<StyleResolver::AnimationProperties>()
Food for thought (won't block your work on this): The first/last/range property
thing seems like a useful separate unit of code that could its own class.

https://codereview.chromium.org/339163002/diff/40001/Source/core/css/resolver...
Source/core/css/resolver/StyleResolver.cpp:1249: return
CSSPropertyTransitionTimingFunction;
That handy COMPILE_ASSERT in the firstCSSPropertyId was neat. Can we have one
here, too?

[tasak, 2014-06-18 05:16:31]

Thank you for reviewing.

https://codereview.chromium.org/339163002/diff/40001/Source/core/css/StylePro...
File Source/core/css/StylePropertyShorthandCustom.cpp (right):

https://codereview.chromium.org/339163002/diff/40001/Source/core/css/StylePro...
Source/core/css/StylePropertyShorthandCustom.cpp:121: // "font: initial;
font-family: initial;...". To avoid this, we need to
On 2014/06/17 15:13:14, dglazkov wrote:
> This seems like a bug in our code? I remember someone working on this, but
can't
> recall who that was.

Yes. Next I will create StylePropertySerializer patch to support "all" property.
The patch needs this FIXME comment.

Firstly I used isExpandedShorthand to expand all shorthand to longhands, however
I found that serialized text had "font: initial; font-family: initial; ..." (I
tried, "all: initial; color: green".)

Looking at the isExpandedShorthand's comment, I thought that it is (probably)
not good to change isExpandedShorthand logic. So I added
isExpandedShorthandForAll with the FIXME comment.

https://codereview.chromium.org/339163002/diff/40001/Source/core/css/resolver...
File Source/core/css/resolver/StyleResolver.cpp (right):

https://codereview.chromium.org/339163002/diff/40001/Source/core/css/resolver...
Source/core/css/resolver/StyleResolver.cpp:1239: template<> CSSPropertyID
StyleResolver::firstCSSPropertyId<StyleResolver::AnimationProperties>()
On 2014/06/17 15:13:14, dglazkov wrote:
> Food for thought (won't block your work on this): The first/last/range
property
> thing seems like a useful separate unit of code that could its own class.

I see. I added FIXME. After finishing "all", I would like to consider the
refactoring.

https://codereview.chromium.org/339163002/diff/40001/Source/core/css/resolver...
Source/core/css/resolver/StyleResolver.cpp:1249: return
CSSPropertyTransitionTimingFunction;
On 2014/06/17 15:13:14, dglazkov wrote:
> That handy COMPILE_ASSERT in the firstCSSPropertyId was neat. Can we have one
> here, too?

Done.

[tasak, 2014-06-18 05:16:38]

The CQ bit was checked by tasak@google.com

[commit-bot: I haz the power, 2014-06-18 05:17:36]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tasak@google.com/339163002/100001

[commit-bot: I haz the power, 2014-06-18 06:25:59]

Change committed as 176396