Changeset 121874 in webkit

Timestamp:

Jul 4, 2012 5:15:54 PM (12 years ago)

Author:

macpherson@chromium.org

Message:

Inspector crashes when trying to inspect a page with CSS variables
​https://bugs.webkit.org/show_bug.cgi?id=89818

Reviewed by Antti Koivisto.

Patch works by fixing treating handling of CSSPropertyID == CSSPropertyVariable as a special case,
and looking up the author-defined property name from the CSSValue.

Added test inspector/styles/variables/css-variables.html that inspects an element using CSS variables.
Test is skipped when variables are compiled out.

• css/CSSProperty.cpp:

(WebCore::CSSProperty::cssName):
(WebCore):
(WebCore::CSSProperty::cssText):

• css/CSSProperty.h:

(CSSProperty):

• css/PropertySetCSSStyleDeclaration.cpp:

(WebCore::PropertySetCSSStyleDeclaration::item):

• css/StylePropertySet.cpp:

(WebCore::StylePropertySet::asText):

• inspector/InspectorStyleSheet.cpp:

(WebCore::InspectorStyle::populateAllProperties):

Location:

trunk

Files:

• LayoutTests/inspector/styles/variables (added)
• LayoutTests/inspector/styles/variables/css-variables-expected.txt (added)
• LayoutTests/inspector/styles/variables/css-variables.html (added)
• LayoutTests/platform/chromium/TestExpectations (modified) (1 diff)
• LayoutTests/platform/gtk/TestExpectations (modified) (1 diff)
• LayoutTests/platform/mac/TestExpectations (modified) (1 diff)
• LayoutTests/platform/qt/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/css/CSSGrammar.y (modified) (1 diff)
• Source/WebCore/css/CSSParser.cpp (modified) (4 diffs)
• Source/WebCore/css/CSSProperty.cpp (modified) (2 diffs)
• Source/WebCore/css/CSSProperty.h (modified) (1 diff)
• Source/WebCore/css/PropertySetCSSStyleDeclaration.cpp (modified) (1 diff)
• Source/WebCore/css/StylePropertySet.cpp (modified) (1 diff)
• Source/WebCore/inspector/InspectorStyleSheet.cpp (modified) (1 diff)
• Tools/Scripts/webkitpy/layout_tests/port/webkit.py (modified) (1 diff)
• Tools/Scripts/webkitpy/layout_tests/port/webkit_unittest.py (modified) (1 diff)

trunk/LayoutTests/platform/chromium/TestExpectations

@@ -159 +159 @@
 // CSS Variables are not yet enabled.
 BUGWK85580 SKIP : fast/css/variables = PASS
+BUGWK85580 SKIP : inspector/styles/variables = PASS
 
 // CSS image-resolution is not yet enabled.

trunk/LayoutTests/platform/gtk/TestExpectations

@@ -319 +319 @@
 // CSS Variables are not yet enabled.
 BUGWK85580 SKIP : fast/css/variables = PASS TEXT
+BUGWK85580 SKIP : inspector/styles/variables = PASS
 
 // CSS image-resolution is not yet enabled.

trunk/LayoutTests/platform/mac/TestExpectations

@@ -12 +12 @@
 // CSS Variables are not yet enabled.
 BUGWK85580 SKIP : fast/css/variables = PASS
+BUGWK85580 SKIP : inspector/styles/variables = PASS
 
 // UndoManager is not yet enabled.

trunk/LayoutTests/platform/qt/TestExpectations

@@ -74 +74 @@
 // CSS Variables are not yet enabled.
 BUGWK85580 SKIP : fast/css/variables = PASS
+BUGWK85580 SKIP : inspector/styles/variables = PASS
 
 // UndoManager is not yet enabled.

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-07-04  Luke Macpherson  <macpherson@chromium.org>
+
+        Inspector crashes when trying to inspect a page with CSS variables
+        https://bugs.webkit.org/show_bug.cgi?id=89818
+
+        Reviewed by Antti Koivisto.
+
+        Patch works by fixing treating handling of CSSPropertyID == CSSPropertyVariable as a special case,
+        and looking up the author-defined property name from the CSSValue.
+
+        Added test inspector/styles/variables/css-variables.html that inspects an element using CSS variables.
+        Test is skipped when variables are compiled out.
+
+        * css/CSSProperty.cpp:
+        (WebCore::CSSProperty::cssName):
+        (WebCore):
+        (WebCore::CSSProperty::cssText):
+        * css/CSSProperty.h:
+        (CSSProperty):
+        * css/PropertySetCSSStyleDeclaration.cpp:
+        (WebCore::PropertySetCSSStyleDeclaration::item):
+        * css/StylePropertySet.cpp:
+        (WebCore::StylePropertySet::asText):
+        * inspector/InspectorStyleSheet.cpp:
+        (WebCore::InspectorStyle::populateAllProperties):
+
 2012-07-04  Anthony Scian  <ascian@rim.com>
 

trunk/Source/WebCore/css/CSSGrammar.y

@@ -1361 +1361 @@
         p->storeVariableDeclaration($1, p->sinkFloatingValueList($4), $5);
         $$ = true;
-        p->markPropertyEnd($5, $$);
+        p->markPropertyEnd($5, true);
 #else
         $$ = false;

trunk/Source/WebCore/css/CSSParser.cpp

@@ -3013 +3013 @@
 void CSSParser::storeVariableDeclaration(const CSSParserString& name, PassOwnPtr<CSSParserValueList> value, bool important)
 {
+    ASSERT(name.length > 12);
+    AtomicString variableName = String(name.characters + 12, name.length - 12);
+
     StringBuilder builder;
     for (unsigned i = 0, size = value->size(); i < size; i++) {
@@ -3019 +3022 @@
         builder.append(value->valueAt(i)->createCSSValue()->cssText());
     }
-    addProperty(CSSPropertyVariable, CSSVariableValue::create(name, builder.toString()), important, false);
+    addProperty(CSSPropertyVariable, CSSVariableValue::create(variableName, builder.toString()), important, false);
 }
 #endif
@@ -8982 +8985 @@
 
     case CharacterDash:
-#if ENABLE(CSS_VARIABLES)
-        if (cssVariablesEnabled() && isEqualToCSSIdentifier(m_currentCharacter, "webkit-var") && m_currentCharacter[10] == '-' && isIdentifierStartAfterDash(m_currentCharacter + 11)) {
-            // handle variable declarations
-            m_currentCharacter += 11;
-            parseIdentifier(result, hasEscape);
-            m_token = VAR_DEFINITION;
-            yylval->string.characters = m_tokenStart;
-            yylval->string.length = result - m_tokenStart;
-        } else
-#endif
         if (isIdentifierStartAfterDash(m_currentCharacter)) {
             --m_currentCharacter;
@@ -8997 +8990 @@
             m_token = IDENT;
 
+#if ENABLE(CSS_VARIABLES)
+            if (cssVariablesEnabled() && isEqualToCSSIdentifier(m_tokenStart + 1, "webkit-var") && m_tokenStart[11] == '-' && isIdentifierStartAfterDash(m_tokenStart + 12))
+                m_token = VAR_DEFINITION;
+            else
+#endif
             if (*m_currentCharacter == '(') {
                 m_token = FUNCTION;

trunk/Source/WebCore/css/CSSProperty.cpp

@@ -27 +27 @@
 #include "StylePropertyShorthand.h"
 
+#if ENABLE(CSS_VARIABLES)
+#include "CSSVariableValue.h"
+#endif
+
 namespace WebCore {
 
@@ -36 +40 @@
 COMPILE_ASSERT(sizeof(CSSProperty) == sizeof(SameSizeAsCSSProperty), CSSProperty_should_stay_small);
 
+String CSSProperty::cssName() const
+{
+#if ENABLE(CSS_VARIABLES)
+    if (id() == CSSPropertyVariable) {
+        ASSERT(value()->isVariableValue());
+        return "-webkit-var-" + static_cast<CSSVariableValue*>(value())->name();
+    }
+#endif
+    return String(getPropertyName(id()));
+}
+
 String CSSProperty::cssText() const
 {
-    return String(getPropertyName(id())) + ": " + m_value->cssText() + (isImportant() ? " !important" : "") + "; ";
+    return cssName() + ": " + m_value->cssText() + (isImportant() ? " !important" : "") + "; ";
 }
 

trunk/Source/WebCore/css/CSSProperty.h

@@ -52 +52 @@
     CSSValue* value() const { return m_value.get(); }
 
+    String cssName() const;
     String cssText() const;
 

trunk/Source/WebCore/css/PropertySetCSSStyleDeclaration.cpp

@@ -140 +140 @@
     if (i >= m_propertySet->propertyCount())
         return "";
-    return getPropertyName(m_propertySet->propertyAt(i).id());
+    return m_propertySet->propertyAt(i).cssName();
 }
 

trunk/Source/WebCore/css/StylePropertySet.cpp

@@ -663 +663 @@
 
         switch (propertyID) {
+#if ENABLE(CSS_VARIABLES)
+        case CSSPropertyVariable:
+            result.append(prop.cssText());
+            continue;
+#endif
         case CSSPropertyBackgroundPositionX:
             positionXProp = ∝

trunk/Source/WebCore/inspector/InspectorStyleSheet.cpp

@@ -1117 +1117 @@
 
     RefPtr<StyleSheetContents> newStyleSheet = StyleSheetContents::create();
-    CSSParser p(CSSStrictMode);
+    CSSParser p(m_pageStyleSheet->ownerDocument());
     OwnPtr<RuleSourceDataList> ruleSourceDataResult = adoptPtr(new RuleSourceDataList());
     p.parseSheet(newStyleSheet.get(), m_parsedStyleSheet->text(), 0, ruleSourceDataResult.get());

trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit.py

@@ -299 +299 @@
             "WebGLShader": ["fast/canvas/webgl", "compositing/webgl", "http/tests/canvas/webgl"],
             "MHTMLArchive": ["mhtml"],
-            "CSSVariableValue": ["fast/css/variables"],
+            "CSSVariableValue": ["fast/css/variables", "inspector/styles/variables"],
         }
 

trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit_unittest.py

@@ -101 +101 @@
             "mhtml",  # Requires MHTMLArchive
             "fast/css/variables",  # Requires CSS Variables
+            "inspector/styles/variables",  # Requires CSS Variables
         ])