Move size property into CSSPropertyParser

Move size property into CSSPropertyParser

Move size property handling from
LegacyCSSPropertyParser into CSSPropertyParser.

BUG=499780
Committed: https://crrev.com/68142d4441ac101cb60f52c06bd9b2ba514e852d
Cr-Commit-Position: refs/heads/master@{#353644}

[rwlbuis, 2015-10-08 22:16:38]

rob.buis@samsung.com changed reviewers:
+ alancutter@chromium.org, timloh@chromium.org

[rwlbuis, 2015-10-08 22:16:39]

PTAL (assuming it shows up green).

[Timothy Loh, 2015-10-08 23:36:38]

https://codereview.chromium.org/1397553003/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp (right):

https://codereview.chromium.org/1397553003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:699: // First
parameter.
This can be lots simpler I think. Sorry for code dump, I wasn't sure how to word
this in text.

if (range.peek().id() == CSSValueAuto) {
    result->append(consumeIdent(range));
    return result.release();
}

if (RefPtrWillBeRawPtr<CSSValue> width = consumeLength(range, cssParserMode,
ValueRangeNonNegative)) {
    RefPtrWillBeRawPtr<CSSValue> height = consumeLength(range, cssParserMode,
ValueRangeNonNegative);
    if (!height)
        height = width;
    result->append(width.release());
    result->append(height.release());
}

RefPtrWillBeRawPtr<CSSValue> pageSize = consumePageSize(range);
RefPtrWillBeRawPtr<CSSValue> orientation = nullptr;
if (range.peek().id() == CSSValuePortrait || range.peek().id() ==
CSSValueLandscape)
    orientation = consumeIdent(range);
if (!pageSize)
    pageSize = consumePageSize(range);

if (!orientation && !pageSize)
    return nullptr;
if (pageSize)
    result->append(pageSize.release());
if (orientation)
    result->append(orientation.release());
return result.release();


(I'm thinking of making a templated consumeIdent which will make a whole bunch
of code a little nicer, e.g. consumeIdent<CSSValuePortrait, CSSValueLandscape>)

[rwlbuis, 2015-10-12 17:13:19]

Changes done, PTAL.

https://codereview.chromium.org/1397553003/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp (right):

https://codereview.chromium.org/1397553003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:699: // First
parameter.
On 2015/10/08 23:36:38, Timothy Loh wrote:
> This can be lots simpler I think. Sorry for code dump, I wasn't sure how to
word
> this in text.

I like it! But the part where height is set to width in case only one length is
given leads to a test difference. Also I think if we'd do that, then we could
remove some code in applyValueCSSPropertySize, so it sounds like a bigger
change...

[Timothy Loh, 2015-10-12 22:58:29]

On 2015/10/12 17:13:19, rwlbuis wrote:
> Changes done, PTAL.
> 
>
https://codereview.chromium.org/1397553003/diff/60001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp (right):
> 
>
https://codereview.chromium.org/1397553003/diff/60001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:699: // First
> parameter.
> On 2015/10/08 23:36:38, Timothy Loh wrote:
> > This can be lots simpler I think. Sorry for code dump, I wasn't sure how to
> word
> > this in text.
> 
> I like it! But the part where height is set to width in case only one length
is
> given leads to a test difference. Also I think if we'd do that, then we could
> remove some code in applyValueCSSPropertySize, so it sounds like a bigger
> change...

lgtm

[rwlbuis, 2015-10-12 23:02:11]

The CQ bit was checked by rob.buis@samsung.com

[commit-bot: I haz the power, 2015-10-12 23:04:55]

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

[commit-bot: I haz the power, 2015-10-13 00:41:24]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-10-13 00:42:23]

Patchset 5 (id:??) landed as
https://crrev.com/68142d4441ac101cb60f52c06bd9b2ba514e852d
Cr-Commit-Position: refs/heads/master@{#353644}