Inspector crashes when trying to inspect a page with CSS region styling

https://bugs.webkit.org/show_bug.cgi?id=91503 Patch by Raul Hudea <rhudea@adobe.com> on 2012-07-31 Reviewed by Alexander Pavlov. Source/WebCore: Fix by making sure a CSSRuleSourceData is always created when parsing @-webkit-region rules. Test: inspector/styles/region-style-crash.html * css/CSSGrammar.y: Made ruleset called explicitly markRuleBodyStart instead of depending on updateLastSelectorLineAndPosition to call it. * css/CSSParser.cpp: (WebCore::CSSParser::updateLastSelectorLineAndPosition): markRuleBodyStart should be called via at_rule_body_start. * css/CSSPropertySourceData.h: LayoutTests: Test to make sure inspector is not crashing when having to parse @-webkit-region rules * inspector/styles/region-style-crash-expected.txt: Added. * inspector/styles/region-style-crash.html: Added. * platform/chromium/TestExpectations: Canonical link: https://commits.webkit.org/110561@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@124186 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Jul 31, 2012 · c26c472 · c26c472
1 parent 96de427
commit c26c472
Show file tree

Hide file tree

Showing 8 changed files with 189 additions and 7 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,16 @@
+2012-07-31  Raul Hudea  <rhudea@adobe.com>
+
+        Inspector crashes when trying to inspect a page with CSS region styling
+        https://bugs.webkit.org/show_bug.cgi?id=91503
+
+        Reviewed by Alexander Pavlov.
+
+        Test to make sure inspector is not crashing when having to parse @-webkit-region rules
+
+        * inspector/styles/region-style-crash-expected.txt: Added.
+        * inspector/styles/region-style-crash.html: Added.
+        * platform/chromium/TestExpectations:
+
 2012-07-30  Keishi Hattori  <keishi@webkit.org>
 
         Implement datalist UI for input type color for Chromium

diff --git a/LayoutTests/inspector/styles/region-style-crash-expected.txt b/LayoutTests/inspector/styles/region-style-crash-expected.txt
@@ -0,0 +1,101 @@
+Tests that webkit css region styling can be parsed correctly. Test passes if it doesn't crash.
+
+P color styled in region: #008000.
+
+[expanded] 
+color: red;
+    #p1 - #ff0000 region-style-crash.html:6
+display: block;
+    p - block user agent stylesheet
+
+[expanded] 
+element.style  { ()
+
+======== Matched CSS Rules ========
+[expanded] 
+#p1  { (region-style-crash.html:8)
+
+[expanded] 
+#p1  { (region-style-crash.html:6)
+color: red;
+
+[expanded] 
+p  { (user agent stylesheet)
+display: block;
+-webkit-margin-before: 1em;
+-webkit-margin-after: 1em;
+-webkit-margin-start: 0px;
+-webkit-margin-end: 0px;
+
+
+======== Pseudo ::first-line element ========
+[expanded] 
+#p1  { (region-style-crash.html:8)
+
+
+======== Pseudo ::first-letter element ========
+[expanded] 
+#p1  { (region-style-crash.html:8)
+
+
+======== Pseudo ::before element ========
+[expanded] 
+#p1  { (region-style-crash.html:8)
+
+
+======== Pseudo ::after element ========
+[expanded] 
+#p1  { (region-style-crash.html:8)
+
+
+======== Pseudo ::selection element ========
+[expanded] 
+#p1  { (region-style-crash.html:8)
+
+
+======== Pseudo element ========
+[expanded] 
+#p1  { (region-style-crash.html:8)
+
+
+======== Pseudo ::-webkit-scrollbar element ========
+[expanded] 
+#p1  { (region-style-crash.html:8)
+
+
+======== Pseudo ::-webkit-file-upload-button element ========
+[expanded] 
+#p1  { (region-style-crash.html:8)
+
+
+======== Pseudo ::-webkit-input-placeholder element ========
+[expanded] 
+#p1  { (region-style-crash.html:8)
+
+
+======== Pseudo ::-webkit-slider-thumb element ========
+[expanded] 
+#p1  { (region-style-crash.html:8)
+
+
+======== Pseudo ::-webkit-search-cancel-button element ========
+[expanded] 
+#p1  { (region-style-crash.html:8)
+
+
+======== Pseudo ::-webkit-search-decoration element ========
+[expanded] 
+#p1  { (region-style-crash.html:8)
+
+
+======== Pseudo ::-webkit-search-results-decoration element ========
+[expanded] 
+#p1  { (region-style-crash.html:8)
+
+
+======== Pseudo ::-webkit-search-results-button element ========
+[expanded] 
+#p1  { (region-style-crash.html:8)
+
+
+
diff --git a/LayoutTests/inspector/styles/region-style-crash.html b/LayoutTests/inspector/styles/region-style-crash.html
@@ -0,0 +1,42 @@
+<html>
+<head>
+<style>
+#article1 { -webkit-flow-into: flow1; }
+#region1 { -webkit-flow-from: flow1; position: absolute; top: 10px; width: 350px; height: 25px;}
+#p1 { color: #ff0000; }
+@-webkit-region #region1 {
+    #p1 { color: #008000; }
+}
+
+</style>
+<script src="../../http/tests/inspector/inspector-test.js"></script>
+<script src="../../http/tests/inspector/elements-test.js"></script>
+<script>
+
+function test()
+{
+    WebInspector.showPanel("elements");
+    InspectorTest.selectNodeAndWaitForStylesWithComputed("p1", dumpAllStyles);
+
+    function dumpAllStyles()
+    {
+        InspectorTest.dumpSelectedElementStyles();
+        InspectorTest.completeTest();
+    }
+}
+
+</script>
+</head>
+
+<body onload="runTest()">
+<p>
+Tests that webkit css region styling can be parsed correctly. Test passes if it doesn't crash.
+</p>
+
+<div id="article1">
+    <p id="p1">P color styled in region: #008000.</p>
+</div>
+<div id="region1" class="regionBox"></div>
+
+</body>
+</html>
diff --git a/LayoutTests/platform/chromium/TestExpectations b/LayoutTests/platform/chromium/TestExpectations
@@ -2624,6 +2624,8 @@ BUGWK49142 : fast/regions/style-scoped-in-flow-override-region-styling-multiple-
 BUGWK49142 : fast/regions/style-scoped-in-flow-override-region-styling.html = PASS IMAGE IMAGE+TEXT TEXT MISSING
 BUGWK49142 : fast/regions/style-scoped-in-flow.html = PASS IMAGE IMAGE+TEXT TEXT MISSING
 
+BUGWK92131 SKIP : inspector/styles/region-style-crash.html = PASS
+
 BUGCR85755 : fast/js/exception-properties.html = TEXT
 
 BUGWK62580 : fast/loader/inherit-charset-to-empty-frame.html = PASS TEXT

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,19 @@
+2012-07-31  Raul Hudea  <rhudea@adobe.com>
+
+        Inspector crashes when trying to inspect a page with CSS region styling
+        https://bugs.webkit.org/show_bug.cgi?id=91503
+
+        Reviewed by Alexander Pavlov.
+
+        Fix by making sure a CSSRuleSourceData is always created when parsing @-webkit-region rules.
+
+        Test: inspector/styles/region-style-crash.html
+
+        * css/CSSGrammar.y: Made ruleset called explicitly markRuleBodyStart instead of depending on updateLastSelectorLineAndPosition to call it.
+        * css/CSSParser.cpp:
+        (WebCore::CSSParser::updateLastSelectorLineAndPosition): markRuleBodyStart should be called via at_rule_body_start. 
+        * css/CSSPropertySourceData.h:
+
 2012-07-31  Sheriff Bot  <webkit.review.bot@gmail.com>
 
         Unreviewed, rolling out r124179.

diff --git a/Source/WebCore/css/CSSGrammar.y b/Source/WebCore/css/CSSGrammar.y
@@ -869,12 +869,20 @@ region_selector:
     }
 ;
 
+before_region_rule:
+    /* empty */ {
+        static_cast<CSSParser*>(parser)->markRuleHeaderStart(CSSRuleSourceData::REGION_RULE);
+    }
+    ;
+
 region:
-    WEBKIT_REGION_RULE_SYM WHITESPACE region_selector '{' maybe_space block_rule_list save_block {
-        if ($3)
-            $$ = static_cast<CSSParser*>(parser)->createRegionRule($3, $6);
-        else
+    before_region_rule WEBKIT_REGION_RULE_SYM WHITESPACE region_selector at_rule_header_end '{' at_rule_body_start maybe_space block_rule_list save_block {
+        if ($4)
+            $$ = static_cast<CSSParser*>(parser)->createRegionRule($4, $9);
+        else {
             $$ = 0;
+            static_cast<CSSParser*>(parser)->popRuleData();
+        }
     }
 ;
 
@@ -913,7 +921,7 @@ at_rule_header_end:
   ;
 
 ruleset:
-    before_selector_list selector_list at_rule_header_end '{' maybe_space_before_declaration declaration_list closing_brace {
+    before_selector_list selector_list at_rule_header_end '{' at_rule_body_start maybe_space_before_declaration declaration_list closing_brace {
         CSSParser* p = static_cast<CSSParser*>(parser);
         $$ = p->createStyleRule($2);
     }

diff --git a/Source/WebCore/css/CSSParser.cpp b/Source/WebCore/css/CSSParser.cpp
@@ -9747,7 +9747,6 @@ void CSSParser::invalidBlockHit()
 void CSSParser::updateLastSelectorLineAndPosition()
 {
     m_lastSelectorLineNumber = m_lineNumber;
-    markRuleBodyStart();
 }
 
 void CSSParser::updateLastMediaLine(MediaQuerySet* media)

diff --git a/Source/WebCore/css/CSSPropertySourceData.h b/Source/WebCore/css/CSSPropertySourceData.h
@@ -93,7 +93,8 @@ struct CSSRuleSourceData : public RefCounted<CSSRuleSourceData> {
         MEDIA_RULE,
         FONT_FACE_RULE,
         PAGE_RULE,
-        KEYFRAMES_RULE
+        KEYFRAMES_RULE,
+        REGION_RULE
     };
 
     static PassRefPtr<CSSRuleSourceData> create(Type type)