Invalid block doesn't make declaration invalid.

Invalid block doesn't make declaration invalid.

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

[TabAtkins, 2013-04-29 17:44:42]

The tests look good to me.  Again, not familiar enough with our parser to
comment on the grammar changes.

[SeRya, 2013-04-29 18:01:26]

I explained in inline comments why I believe this change doesn't break any code
where recovery worked previously. Plus it recovers in some other cases.

https://codereview.chromium.org/14083017/diff/3001/Source/core/css/CSSGrammar...
File Source/core/css/CSSGrammar.y.in (left):

https://codereview.chromium.org/14083017/diff/3001/Source/core/css/CSSGrammar...
Source/core/css/CSSGrammar.y.in:1529: | declaration error_location
invalid_block_list maybe_space {
What's wrong with this rule?
1. It covers "body { color: red {} background-color: blue; }". It must be
invalid.
2. It covers "color: red {}{}{}{}" but not "color: red {}#".

How to make sure any CSS code covered by this rule still covered by new grammar?
Let's introduce an invariant (#inv1): each rule reducable to declaration has a
counterpart ending with declaration_recovery which able to cover any
invalid_block_list maybe_space.

https://codereview.chromium.org/14083017/diff/3001/Source/core/css/CSSGrammar...
Source/core/css/CSSGrammar.y.in:1534: | declaration error_location
invalid_block_list ';' maybe_space {
Same as above. Just add that #inv1 guarantees this rule is removed correctly.

https://codereview.chromium.org/14083017/diff/3001/Source/core/css/CSSGrammar...
Source/core/css/CSSGrammar.y.in:1586: property errors {
Why this change? To inforce #inv1.
"error" is covered by "declaration_recovery" so any text covered by old code
still covered by new one.

https://codereview.chromium.org/14083017/diff/3001/Source/core/css/CSSGrammar...
Source/core/css/CSSGrammar.y.in:1592: property ':' maybe_space errors expr prio
{
What's wrong with this rule?
1. It covers exactly one error. If next token followed by one generated error
generate error as well it would be covered by property ':' maybe_space errors.
2. In violates #inv1.

Why any case covered by this reule covered in new grammar?
"property ':' maybe_space declaration_recovery" just enough.

https://codereview.chromium.org/14083017/diff/3001/Source/core/css/CSSGrammar...
Source/core/css/CSSGrammar.y.in:1607: IMPORTANT_SYM maybe_space error_location {
What's wrong?
Violate #inv1.

"body { !important ; color: red; }" "body { !important }" as well as is covered
by changed variant.

https://codereview.chromium.org/14083017/diff/3001/Source/core/css/CSSGrammar...
Source/core/css/CSSGrammar.y.in:1614: property ':' maybe_space error_location {
What's wrong with this rule:
1. It's redundant. "property ':' maybe_space declaration_recovery" covers cases
like "body { color: }" or "body { color: ; }

https://codereview.chromium.org/14083017/diff/3001/Source/core/css/CSSGrammar...
Source/core/css/CSSGrammar.y.in:1614: property ':' maybe_space error_location {
What's wrong with this rule?
It's redundant. "div { font-family: }" is perfectly covered by "property ':'
maybe_space declaration_recovery".

https://codereview.chromium.org/14083017/diff/3001/Source/core/css/CSSGrammar...
Source/core/css/CSSGrammar.y.in:1621: property ':' maybe_space errors {
Tranformed to "property ':' maybe_space declaration_recovery".

https://codereview.chromium.org/14083017/diff/3001/Source/core/css/CSSGrammar...
Source/core/css/CSSGrammar.y.in:1621: property ':' maybe_space errors {
Transformed to "property ':' maybe_space declaration_recovery"

https://codereview.chromium.org/14083017/diff/3001/Source/core/css/CSSGrammar...
Source/core/css/CSSGrammar.y.in:1628: property error_location invalid_block {
It perfectly covered by "property declaration_recovery".

[rune, 2013-04-30 11:19:14]

It certainly looks better with fewer special cases. I have looked through your
grammar changes, and I couldn't spot any potential regressions, but I find it
hard to verify 100%.

I think that using a type of error recovery as I've introduced for media queries
here:

https://codereview.chromium.org/14065029/

will make error recovery simpler and more correct for recovering declarations
and other parts of the grammar, too.

[apavlov, 2013-04-30 17:00:04]

lgtm

The grammar changes look sane given the author's inline comments.

[commit-bot: I haz the power, 2013-04-30 17:00:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/serya@chromium.org/14083017/3001

[commit-bot: I haz the power, 2013-04-30 18:53:21]

Change committed as 149446