Invalid block doesn't make declaration invalid.

Source/core/css/CSSGrammar.y.in

Index: Source/core/css/CSSGrammar.y.in
diff --git a/Source/core/css/CSSGrammar.y.in b/Source/core/css/CSSGrammar.y.in
index a400d8c0c24d2c8532a52416570a6cf70db53410..32fd8654702746b761bae5b9167c6ede428a8d37 100644
--- a/Source/core/css/CSSGrammar.y.in
+++ b/Source/core/css/CSSGrammar.y.in
@@ -87,7 +87,7 @@ static inline bool isCSSTokenAString(int yytype)
 
 %}
 
-%expect 38
+%expect 35
 
 %nonassoc LOWEST_PREC
 
@@ -314,7 +314,6 @@ static inline bool isCSSTokenAString(int yytype)
 %type <string> element_name
 %type <string> attr_name
 
-%type <location> errors
 %type <location> error_location
 
 %%
@@ -1526,16 +1525,6 @@ decl_list:
         parser->startProperty();
         $$ = $1;
     }
-    | declaration error_location invalid_block_list maybe_space {

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

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.

-        parser->syntaxError($2);
-        parser->startProperty();
-        $$ = false;
-    }
-    | declaration error_location invalid_block_list ';' maybe_space {

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

Same as above. Just add that #inv1 guarantees this rule is removed correctly.

-        parser->syntaxError($2);
-        parser->startProperty();
-        $$ = false;
-    }
     | decl_list_recovery ';' maybe_space {
         parser->startProperty();
         $$ = false;
@@ -1583,54 +1572,36 @@ declaration:
         parser->endProperty($5, isPropertyParsed);
     }
     |
-    property errors {

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

Why this change? To inforce #inv1.
"error" is covered by "declaration_recovery" so any text covered by old code
still covered by new one.

-        parser->syntaxError($2);
+    property declaration_recovery {
         parser->endProperty(false, false, CSSParser::GeneralSyntaxError);
         $$ = false;
     }
     |
-    property ':' maybe_space errors expr prio {

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

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.

-        /* The default movable type template has letter-spacing: .none;  Handle this by looking for
-        error tokens at the start of an expr, recover the expr and then treat as an error, cleaning
-        up and deleting the shifted expr.  */
-        parser->syntaxError($4);
-        parser->endProperty(false, false);
-        $$ = false;
-    }
-    |
-    property ':' maybe_space expr prio error {
+    property ':' maybe_space expr prio declaration_recovery {
         /* When we encounter something like p {color: red !important fail;} we should drop the declaration */
         parser->endProperty(false, false);
         $$ = false;
     }
     |
-    IMPORTANT_SYM maybe_space error_location {

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

What's wrong?
Violate #inv1.

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

+    IMPORTANT_SYM maybe_space declaration_recovery {
         /* Handle this case: div { text-align: center; !important } Just reduce away the stray !important. */
-        parser->syntaxError($3);
         parser->endProperty(false, false, CSSParser::GeneralSyntaxError);
         $$ = false;
     }
     |
-    property ':' maybe_space error_location {

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

What's wrong with this rule:
1. It's redundant. "property ':' maybe_space declaration_recovery" covers cases
like "body { color: }" or "body { color: ; }

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

What's wrong with this rule?
It's redundant. "div { font-family: }" is perfectly covered by "property ':'
maybe_space declaration_recovery".

-        /* div { font-family: } Just reduce away this property with no value. */
-        parser->syntaxError($4);
-        parser->endProperty(false, false);
-        $$ = false;
-    }
-    |
-    property ':' maybe_space errors {

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

Tranformed to "property ':' maybe_space declaration_recovery".

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

Transformed to "property ':' maybe_space declaration_recovery"

+    property ':' maybe_space declaration_recovery {
         /* if we come across rules with invalid values like this case: p { weight: *; }, just discard the rule */
-        parser->syntaxError($4);
         parser->endProperty(false, false);
         $$ = false;
     }
-    |
-    property error_location invalid_block {

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

It perfectly covered by "property declaration_recovery".

-        /* if we come across: div { color{;color:maroon} }, ignore everything within curly brackets */
+  ;
+
+declaration_recovery:
+    error error_location {
         parser->syntaxError($2);
-        parser->endProperty(false, false, CSSParser::GeneralSyntaxError);
-        $$ = false;
     }
+    | declaration_recovery invalid_block
+    | declaration_recovery error
   ;
 
 property:
@@ -2004,12 +1975,6 @@ invalid_block_list:
   | invalid_block_list error invalid_block
 ;
 
-errors:
-    error error_location {
-        $$ = $2;
-    }
-    ;
-
 error_location: {
         $$ = parser->currentLocation();
     }