Move remaining border-image related properties into CSSPropertyParser

Move remaining border-image related properties into CSSPropertyParser

Move the remaining border-image related properties from
LegacyCSSPropertyParser into CSSPropertyParser:
border-image
border-mask-box-image
webkit-border-image
webkit-box-reflect

Remove the legacy code that dealt with border-image parsing.

BUG=499780
Committed: https://crrev.com/51b59e453e55491e5d7896e25e24020a93e248c6
Cr-Commit-Position: refs/heads/master@{#374281}

[rwlbuis, 2016-02-03 22:11:34]

Description was changed from

==========
border-image WIP

BUG=
==========

to

==========
Move border-image related shorthands into CSSPropertyParser

Move the following border-image related shorthands from
LegacyCSSPropertyParser into CSSPropertyParser:
border-image
webkit-border-image
webkit-reflect

BUG=499780
==========

[rwlbuis, 2016-02-03 22:12:04]

Description was changed from

==========
Move border-image related shorthands into CSSPropertyParser

Move the following border-image related shorthands from
LegacyCSSPropertyParser into CSSPropertyParser:
border-image
webkit-border-image
webkit-reflect

BUG=499780
==========

to

==========
Move border-image related shorthands into CSSPropertyParser

Move the following border-image related shorthands from
LegacyCSSPropertyParser into CSSPropertyParser:
border-image
webkit-border-image
webkit-box-reflect

BUG=499780
==========

[rwlbuis, 2016-02-03 22:12:51]

Description was changed from

==========
Move border-image related shorthands into CSSPropertyParser

Move the following border-image related shorthands from
LegacyCSSPropertyParser into CSSPropertyParser:
border-image
webkit-border-image
webkit-box-reflect

BUG=499780
==========

to

==========
Move border-image related shorthands into CSSPropertyParser

Move the following border-image related shorthands from
LegacyCSSPropertyParser into CSSPropertyParser:
border-image
border-mask-box-image
webkit-border-image
webkit-box-reflect

BUG=499780
==========

[rwlbuis, 2016-02-03 22:13:54]

Description was changed from

==========
Move border-image related shorthands into CSSPropertyParser

Move the following border-image related shorthands from
LegacyCSSPropertyParser into CSSPropertyParser:
border-image
border-mask-box-image
webkit-border-image
webkit-box-reflect

BUG=499780
==========

to

==========
Move remaining border-image related properties into CSSPropertyParser

Move the remaining border-image related properties from
LegacyCSSPropertyParser into CSSPropertyParser:
border-image
border-mask-box-image
webkit-border-image
webkit-box-reflect

Remove the legacy code that dealt with border-image parsing.

BUG=499780
==========

[rwlbuis, 2016-02-05 03:47:52]

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

[rwlbuis, 2016-02-05 03:47:52]

PTAL:)

[Timothy Loh, 2016-02-07 23:38:05]

https://codereview.chromium.org/1662733002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp (right):

https://codereview.chromium.org/1662733002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:3286: static
bool consumeBorderImageComponents(CSSPropertyID property, CSSParserTokenRange&
range, const CSSParserContext& context, RefPtrWillBeRawPtr<CSSValue> &source,
& always before space, I think?

https://codereview.chromium.org/1662733002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:3291: if (value)
{
IMO it's nicer (and a little more efficient) to check whether we need to consume
before trying to consume. Maybe something like:

if (!source && (source = consumeImage(range, context)))
    continue;
if (!repeat && (repeat = consumeBorderImageRepeat(range)))
    continue;
if (!slice && (slice = consumeBorderImageSlice(property, range,
context.mode()))) {
    ASSERT(!width && !outset);
    // etc.
}
return false;

[rwlbuis, 2016-02-08 02:22:18]

PTAL.

https://codereview.chromium.org/1662733002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp (right):

https://codereview.chromium.org/1662733002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:3286: static
bool consumeBorderImageComponents(CSSPropertyID property, CSSParserTokenRange&
range, const CSSParserContext& context, RefPtrWillBeRawPtr<CSSValue> &source,
On 2016/02/07 23:38:05, Timothy Loh wrote:
> & always before space, I think?

Ouch you are right! I also fixed other instances in CSSPropertyParser.cpp.

https://codereview.chromium.org/1662733002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:3291: if (value)
{
On 2016/02/07 23:38:05, Timothy Loh wrote:
> IMO it's nicer (and a little more efficient) to check whether we need to
consume
> before trying to consume. Maybe something like:
> 
> if (!source && (source = consumeImage(range, context)))
>     continue;
> if (!repeat && (repeat = consumeBorderImageRepeat(range)))
>     continue;
> if (!slice && (slice = consumeBorderImageSlice(property, range,
> context.mode()))) {
>     ASSERT(!width && !outset);
>     // etc.
> }
> return false;

Nice! Fixed.

[Timothy Loh, 2016-02-08 02:30:26]

https://codereview.chromium.org/1662733002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp (right):

https://codereview.chromium.org/1662733002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:3296: if
(range.peek().type() == DelimiterToken && range.peek().delimiter() == '/') {
I think this incorrectly allows the following cases. We should add a test for
these.
border-image: 1px / 1 2 3 4 5;
border-image: 1px / 1 /;

I'd probably write the logic as follows (a consumeSlash function might be nice):

if (..delimiter..) {
    range.consumeIncludingWhitespace();
    width = consumeBorderImageWidth(range);
    if (..delimiter..) {
        range.consumeIncludingWhitespace();
        outset = consumeBorderImageOutset(range);
        if (!outset)
            return false;
    } else if (!width)
        return false;
    }
}

[rwlbuis, 2016-02-08 20:12:34]

PTAL :)

https://codereview.chromium.org/1662733002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp (right):

https://codereview.chromium.org/1662733002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:3296: if
(range.peek().type() == DelimiterToken && range.peek().delimiter() == '/') {
On 2016/02/08 02:30:26, Timothy Loh wrote:
> I think this incorrectly allows the following cases. We should add a test for
> these.
> border-image: 1px / 1 2 3 4 5;
> border-image: 1px / 1 /;
> 
> I'd probably write the logic as follows (a consumeSlash function might be
nice):
> 
> if (..delimiter..) {
>     range.consumeIncludingWhitespace();
>     width = consumeBorderImageWidth(range);
>     if (..delimiter..) {
>         range.consumeIncludingWhitespace();
>         outset = consumeBorderImageOutset(range);
>         if (!outset)
>             return false;
>     } else if (!width)
>         return false;
>     }
> }

Done.

[Timothy Loh, 2016-02-08 23:24:44]

On 2016/02/08 20:12:34, rwlbuis wrote:
> PTAL :)
> 
>
https://codereview.chromium.org/1662733002/diff/120001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp (right):
> 
>
https://codereview.chromium.org/1662733002/diff/120001/third_party/WebKit/Sou...
> third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:3296: if
> (range.peek().type() == DelimiterToken && range.peek().delimiter() == '/') {
> On 2016/02/08 02:30:26, Timothy Loh wrote:
> > I think this incorrectly allows the following cases. We should add a test
for
> > these.
> > border-image: 1px / 1 2 3 4 5;
> > border-image: 1px / 1 /;
> > 
> > I'd probably write the logic as follows (a consumeSlash function might be
> nice):
> > 
> > if (..delimiter..) {
> >     range.consumeIncludingWhitespace();
> >     width = consumeBorderImageWidth(range);
> >     if (..delimiter..) {
> >         range.consumeIncludingWhitespace();
> >         outset = consumeBorderImageOutset(range);
> >         if (!outset)
> >             return false;
> >     } else if (!width)
> >         return false;
> >     }
> > }
> 
> Done.

lgtm

[rwlbuis, 2016-02-08 23:51:17]

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

[commit-bot: I haz the power, 2016-02-08 23:54:38]

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

[commit-bot: I haz the power, 2016-02-09 02:15:26]

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

[commit-bot: I haz the power, 2016-02-09 02:15:29]

Failed to request the patch to try. Please note that binary files are still
unsupported at the moment, this is being worked on.

Thanks for your patience.

Transient error: No JSON object could be decoded

[commit-bot: I haz the power, 2016-02-09 02:18:39]

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

[rwlbuis, 2016-02-09 02:27:10]

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

[commit-bot: I haz the power, 2016-02-09 02:35:08]

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

[commit-bot: I haz the power, 2016-02-09 02:47:56]

Description was changed from

==========
Move remaining border-image related properties into CSSPropertyParser

Move the remaining border-image related properties from
LegacyCSSPropertyParser into CSSPropertyParser:
border-image
border-mask-box-image
webkit-border-image
webkit-box-reflect

Remove the legacy code that dealt with border-image parsing.

BUG=499780
==========

to

==========
Move remaining border-image related properties into CSSPropertyParser

Move the remaining border-image related properties from
LegacyCSSPropertyParser into CSSPropertyParser:
border-image
border-mask-box-image
webkit-border-image
webkit-box-reflect

Remove the legacy code that dealt with border-image parsing.

BUG=499780
==========

[commit-bot: I haz the power, 2016-02-09 02:47:56]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2016-02-09 02:49:42]

Description was changed from

==========
Move remaining border-image related properties into CSSPropertyParser

Move the remaining border-image related properties from
LegacyCSSPropertyParser into CSSPropertyParser:
border-image
border-mask-box-image
webkit-border-image
webkit-box-reflect

Remove the legacy code that dealt with border-image parsing.

BUG=499780
==========

to

==========
Move remaining border-image related properties into CSSPropertyParser

Move the remaining border-image related properties from
LegacyCSSPropertyParser into CSSPropertyParser:
border-image
border-mask-box-image
webkit-border-image
webkit-box-reflect

Remove the legacy code that dealt with border-image parsing.

BUG=499780

Committed: https://crrev.com/51b59e453e55491e5d7896e25e24020a93e248c6
Cr-Commit-Position: refs/heads/master@{#374281}
==========

[commit-bot: I haz the power, 2016-02-09 02:49:44]

Patchset 9 (id:??) landed as
https://crrev.com/51b59e453e55491e5d7896e25e24020a93e248c6
Cr-Commit-Position: refs/heads/master@{#374281}