Move rolling back badly parsed property to CSSPropertyParser::parseValue

Move rolling back badly parsed property to CSSPropertyParser::parseValue

When CSSPropertyParser::parseValue fails, we sometimes have already
yielded property/value pairs (e.g. when parsing shorthands). In this
case we roll back any yielded values. This patch moves the logic for this
to CSSPropertyParser since this is a better fit for it and makes the
parseValue interface more sensible.

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

[Timothy Loh, 2014-10-17 03:50:05]

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

[Timothy Loh, 2014-10-17 03:50:05]

timloh@chromium.org changed required reviewers:
+ alancutter@chromium.org

[rune, 2014-10-17 08:52:27]

rune@opera.com changed reviewers:
+ rune@opera.com

[rune, 2014-10-17 08:52:27]

lgtm

Did you consider getting rid of the rollback in CSSGrammar for @supports too? I
guess you could have a parseValueForSupports() in CSSProperyParser to make the
rollback method internal (private) to the parser class.

[Timothy Loh, 2014-10-17 10:44:58]

On 2014/10/17 08:52:27, rune wrote:
> lgtm
> 
> Did you consider getting rid of the rollback in CSSGrammar for @supports too?
I
> guess you could have a parseValueForSupports() in CSSProperyParser to make the
> rollback method internal (private) to the parser class.

That case might actually work with just clearing the parsedProperties vector,
although I'm rewriting the parser so it doesn't matter much. What I'll probably
do in the future is have the new parser clear the parsedProperties vector for
@supports/CSS.supports, get rid of the rollback call in parsing marker (when
parseValue is split off to something like "PassRefPtr<CSSValue>
parseSingleValue(CSSPropertyId)" for non-shorthands), and the function will be
inlined in parseValue.

[Timothy Loh, 2014-10-17 10:45:03]

The CQ bit was checked by timloh@chromium.org

[commit-bot: I haz the power, 2014-10-17 10:45:28]

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

[commit-bot: I haz the power, 2014-10-17 10:45:30]

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

[commit-bot: I haz the power, 2014-10-17 10:45:31]

All required reviewers (with asterisk prefixes) have not yet approved this CL.

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer or
a provisional committer, _not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[Timothy Loh, 2014-10-17 10:49:55]

On 2014/10/17 10:45:31, I haz the power (commit-bot) wrote:
> All required reviewers (with asterisk prefixes) have not yet approved this CL.
> 
> No LGTM from a valid reviewer yet. Only full committers are accepted.
> Even if an LGTM may have been provided, it was from a non-committer or
> a provisional committer, _not_ a full super star committer.
> See http://www.chromium.org/getting-involved/become-a-committer
> Note that this has nothing to do with OWNERS files.

..and now I know what the asterisk does :)

[Timothy Loh, 2014-10-17 10:50:18]

timloh@chromium.org changed required reviewers:
- alancutter@chromium.org

[Timothy Loh, 2014-10-17 10:50:26]

The CQ bit was checked by timloh@chromium.org

[commit-bot: I haz the power, 2014-10-17 10:51:36]

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

[commit-bot: I haz the power, 2014-10-17 10:55:39]

Committed patchset #1 (id:1) as 183879

[rune, 2014-10-17 11:45:59]

On 2014/10/17 10:49:55, Timothy Loh wrote:
> On 2014/10/17 10:45:31, I haz the power (commit-bot) wrote:
> > All required reviewers (with asterisk prefixes) have not yet approved this
CL.
> > 
> > No LGTM from a valid reviewer yet. Only full committers are accepted.
> > Even if an LGTM may have been provided, it was from a non-committer or
> > a provisional committer, _not_ a full super star committer.
> > See http://www.chromium.org/getting-involved/become-a-committer
> > Note that this has nothing to do with OWNERS files.
> 
> ..and now I know what the asterisk does :)

Woot? I'm not a full super star committer? :-)