Fix assert on unknown rule type in the BisonCSSParser::endRuleHeader.

Fix assert on unknown rule type in the BisonCSSParser::endRuleHeader.

Allow the @media rules' media query to be more tolerant on
malformed data. Move the at_rule_header_end after '{' token parsing to
avoid double match on the rule when the Bison's error token seeks for
a recovery point (thus avoiding the double call on the endRuleHeader).

BUG=377488
TEST=webkit_unit_tests --gtest_filter=BisonCSSParserTest.MediaQuery

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

[eseidel, 2014-05-30 17:52:46]

I'm not sure if the test you're using affects this parser or not?  Yoav would
know.

[Yoav Weiss, 2014-05-30 18:33:52]

On 2014/05/30 17:52:46, eseidel wrote:
> I'm not sure if the test you're using affects this parser or not?  Yoav would
> know.

I'm afraid it doesn't. That test is testing media query parsing (of both the
Bison parser and the MQParser) when the input is a media query string.
It does not test the code paths that Media Queries (and other CSS rules) go
through when they arrive from CSS.

[Julien - ping for review, 2014-06-06 20:13:07]

https://codereview.chromium.org/308733005/diff/1/Source/core/css/parser/Bison...
File Source/core/css/parser/BisonCSSParser-in.cpp (right):

https://codereview.chromium.org/308733005/diff/1/Source/core/css/parser/Bison...
Source/core/css/parser/BisonCSSParser-in.cpp:2013: if (m_ruleHeaderType ==
CSSRuleSourceData::UNKNOWN_RULE)
I don't think it's the right fix: we shouldn't be calling endRuleHeader without
a matching startRuleHeader and this ASSERT is preventing us from doing that.

The issue is in CSSGrammar.y where we are missing an invalid rule for @media
that would call endInvalidRuleHeader.

[elecro, 2014-07-17 16:19:45]

Thanks for the info and sorry for the big delay. 

After additional debugging I've found out that the problem originated from the
'media_query' rule in the CSSGrammar.y where the error token is used. Bison's
error mechanizm was looking for a point where it can recover and continue it's
matching. During this it called the 'at_rule_header_end' rule twice as it has an
empty match which resulted in the assert. 

I've uploaded an updated patch and updated the description accordingly.

[Julien - ping for review, 2014-07-18 21:14:31]

lgtm

https://codereview.chromium.org/308733005/diff/20001/Source/core/css/parser/B...
File Source/core/css/parser/BisonCSSParserTest.cpp (right):

https://codereview.chromium.org/308733005/diff/20001/Source/core/css/parser/B...
Source/core/css/parser/BisonCSSParserTest.cpp:104: for (unsigned i = 0;
testCases[i].input; ++i) {
You can actually do without the terminator as |testCases| is an inline array:

for (unsigned i = 0; i < sizeof(testCases)/sizeof(testCases[0]); ++i)

[alancutter (OOO until 2018), 2014-07-21 05:58:10]

https://codereview.chromium.org/308733005/diff/20001/Source/core/css/parser/B...
File Source/core/css/parser/BisonCSSParserTest.cpp (right):

https://codereview.chromium.org/308733005/diff/20001/Source/core/css/parser/B...
Source/core/css/parser/BisonCSSParserTest.cpp:104: for (unsigned i = 0;
testCases[i].input; ++i) {
On 2014/07/18 21:14:31, Julien Chaffraix - PST wrote:
> You can actually do without the terminator as |testCases| is an inline array:
> 
> for (unsigned i = 0; i < sizeof(testCases)/sizeof(testCases[0]); ++i)

We can go one step further and use the macro ARRAY_SIZE(testCases).

[elecro, 2014-07-22 08:30:51]

Updated the CL, now using the ARRAY_SIZE macro.

[elecro, 2014-07-22 12:33:01]

The CQ bit was checked by pgal.u-szeged@partner.samsung.com

[commit-bot: I haz the power, 2014-07-22 12:34:50]

CQ is trying da patch. Follow status at

https://chromium-status.appspot.com/cq/pgal.u-szeged@partner.samsung.com/3087...

[commit-bot: I haz the power, 2014-07-22 14:33:30]

Change committed as 178659