Changeset 117809 in webkit

Timestamp:

May 21, 2012 12:36:39 PM (12 years ago)

Author:

kling@webkit.org

Message:

CSS: Move duplicate property elimination to parser.
<​http://webkit.org/b/86948>

Reviewed by Antti Koivisto.

Source/WebCore:

Remove the StylePropertySet constructor that handled elimination of duplicate
properties and move that to a new parser method, CSSParser::filteredProperties().
Call sites are converted to using StylePropertySet(const Vector<CSSProperty>&).

Instead of building a hashmap of seen properties, use the new WTF::BitArray class
to track whether a given property ID has been seen, and whether we have an
!important entry for a given ID.

• css/CSSParser.cpp:

(WebCore::CSSParser::parseValue):
(WebCore::CSSParser::parseDeclaration):
(WebCore::CSSParser::filteredProperties):
(WebCore::CSSParser::createStyleRule):
(WebCore::CSSParser::createFontFaceRule):
(WebCore::CSSParser::createPageRule):
(WebCore::CSSParser::createKeyframe):

• css/CSSParser.h:
• css/StylePropertySet.cpp:

(WebCore::StylePropertySet::StylePropertySet):
(WebCore::StylePropertySet::addParsedProperties):

• css/StylePropertySet.h:

(WebCore::StylePropertySet::create):
(StylePropertySet):

• svg/SVGFontFaceElement.cpp:

(WebCore::SVGFontFaceElement::rebuildFontFace):

Source/WTF:

Add WTF::BitArray, a simple, malloc free, fixed-size bit array class.

• GNUmakefile.list.am:
• WTF.gypi:
• WTF.pro:
• WTF.vcproj/WTF.vcproj:
• WTF.xcodeproj/project.pbxproj:
• wtf/BitArray.h: Added.

(WTF):
(BitArray):
(WTF::BitArray::BitArray):
(WTF::BitArray::set):
(WTF::BitArray::get):

• wtf/CMakeLists.txt:

Location:

trunk/Source

Files:

• WTF/ChangeLog (modified) (1 diff)
• WTF/GNUmakefile.list.am (modified) (1 diff)
• WTF/WTF.gypi (modified) (1 diff)
• WTF/WTF.pro (modified) (1 diff)
• WTF/WTF.vcproj/WTF.vcproj (modified) (1 diff)
• WTF/WTF.xcodeproj/project.pbxproj (modified) (4 diffs)
• WTF/wtf/BitArray.h (added)
• WTF/wtf/CMakeLists.txt (modified) (1 diff)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/css/CSSParser.cpp (modified) (8 diffs)
• WebCore/css/CSSParser.h (modified) (1 diff)
• WebCore/css/StylePropertySet.cpp (modified) (2 diffs)
• WebCore/css/StylePropertySet.h (modified) (3 diffs)
• WebCore/svg/SVGFontFaceElement.cpp (modified) (1 diff)

trunk/Source/WTF/ChangeLog

@@ +1 @@
+2012-05-21  Andreas Kling  <kling@webkit.org>
+
+        CSS: Move duplicate property elimination to parser.
+        <http://webkit.org/b/86948>
+
+        Reviewed by Antti Koivisto.
+
+        Add WTF::BitArray, a simple, malloc free, fixed-size bit array class.
+
+        * GNUmakefile.list.am:
+        * WTF.gypi:
+        * WTF.pro:
+        * WTF.vcproj/WTF.vcproj:
+        * WTF.xcodeproj/project.pbxproj:
+        * wtf/BitArray.h: Added.
+        (WTF):
+        (BitArray):
+        (WTF::BitArray::BitArray):
+        (WTF::BitArray::set):
+        (WTF::BitArray::get):
+        * wtf/CMakeLists.txt:
+
 2012-05-21  Allan Sandfeld Jensen  <allan.jensen@nokia.com>
 

trunk/Source/WTF/GNUmakefile.list.am

@@ -13 +13 @@
     Source/WTF/wtf/Assertions.h \
     Source/WTF/wtf/Atomics.h \
+    Source/WTF/wtf/BitArray.h \
     Source/WTF/wtf/BitVector.cpp \
     Source/WTF/wtf/BitVector.h \

trunk/Source/WTF/WTF.gypi

@@ -9 +9 @@
             'wtf/Assertions.h',
             'wtf/Atomics.h',
+            'wtf/BitArray.h',
             'wtf/BitVector.h',
             'wtf/Bitmap.h',

trunk/Source/WTF/WTF.pro

@@ -23 +23 @@
     AVLTree.h \
     Bitmap.h \
+    BitArray.h \
     BitVector.h \
     BloomFilter.h \

trunk/Source/WTF/WTF.vcproj/WTF.vcproj

@@ -606 +606 @@
                 </File>
                 <File
+                        RelativePath="..\wtf\BitArray.h"
+                        >
+                </File>
+                <File
                         RelativePath="..\wtf\BitVector.cpp"
                         >

trunk/Source/WTF/WTF.xcodeproj/project.pbxproj

@@ -8 +8 @@
 
 /* Begin PBXBuildFile section */
+                4F0321BC156AA8D1006EBAF6 /* BitArray.h in Headers */ = {isa = PBXBuildFile; fileRef = 4F0321BB156AA8D1006EBAF6 /* BitArray.h */; };
                 A876DBD8151816E500DADB95 /* Platform.h in Headers */ = {isa = PBXBuildFile; fileRef = A876DBD7151816E500DADB95 /* Platform.h */; };
                 A8A4737F151A825B004123FF /* Alignment.h in Headers */ = {isa = PBXBuildFile; fileRef = A8A47254151A825A004123FF /* Alignment.h */; };
@@ -241 +242 @@
 
 /* Begin PBXFileReference section */
+                4F0321BB156AA8D1006EBAF6 /* BitArray.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = BitArray.h; sourceTree = "<group>"; };
                 5D247B6214689B8600E78B76 /* libWTF.a */ = {isa = PBXFileReference; explicitFileType = archive.ar; includeInIndex = 0; path = libWTF.a; sourceTree = BUILT_PRODUCTS_DIR; };
                 5D247B6E14689C4700E78B76 /* Base.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; path = Base.xcconfig; sourceTree = "<group>"; };
@@ -547 +549 @@
                                 A8A4725E151A825A004123FF /* AVLTree.h */,
                                 A8A4725F151A825A004123FF /* Bitmap.h */,
+                                4F0321BB156AA8D1006EBAF6 /* BitArray.h */,
                                 A8A47260151A825A004123FF /* BitVector.cpp */,
                                 A8A47261151A825A004123FF /* BitVector.h */,
@@ -861 +864 @@
                                 A8A47389151A825B004123FF /* AVLTree.h in Headers */,
                                 A8A4738A151A825B004123FF /* Bitmap.h in Headers */,
+                                4F0321BC156AA8D1006EBAF6 /* BitArray.h in Headers */,
                                 A8A4738C151A825B004123FF /* BitVector.h in Headers */,
                                 A8A4738E151A825B004123FF /* BlockStack.h in Headers */,

trunk/Source/WTF/wtf/CMakeLists.txt

@@ -6 +6 @@
     Assertions.h
     Atomics.h
+    BitArray.h
     BitVector.h
     Bitmap.h

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-05-21  Andreas Kling  <kling@webkit.org>
+
+        CSS: Move duplicate property elimination to parser.
+        <http://webkit.org/b/86948>
+
+        Reviewed by Antti Koivisto.
+
+        Remove the StylePropertySet constructor that handled elimination of duplicate
+        properties and move that to a new parser method, CSSParser::filteredProperties().
+        Call sites are converted to using StylePropertySet(const Vector<CSSProperty>&).
+
+        Instead of building a hashmap of seen properties, use the new WTF::BitArray class
+        to track whether a given property ID has been seen, and whether we have an
+        !important entry for a given ID.
+
+        * css/CSSParser.cpp:
+        (WebCore::CSSParser::parseValue):
+        (WebCore::CSSParser::parseDeclaration):
+        (WebCore::CSSParser::filteredProperties):
+        (WebCore::CSSParser::createStyleRule):
+        (WebCore::CSSParser::createFontFaceRule):
+        (WebCore::CSSParser::createPageRule):
+        (WebCore::CSSParser::createKeyframe):
+        * css/CSSParser.h:
+        * css/StylePropertySet.cpp:
+        (WebCore::StylePropertySet::StylePropertySet):
+        (WebCore::StylePropertySet::addParsedProperties):
+        * css/StylePropertySet.h:
+        (WebCore::StylePropertySet::create):
+        (StylePropertySet):
+        * svg/SVGFontFaceElement.cpp:
+        (WebCore::SVGFontFaceElement::rebuildFontFace):
+
 2012-05-21  Joshua Bell  <jsbell@chromium.org>
 

trunk/Source/WebCore/css/CSSParser.cpp

@@ -90 +90 @@
 #endif
 #include <limits.h>
+#include <wtf/BitArray.h>
 #include <wtf/HexNumber.h>
 #include <wtf/dtoa.h>
@@ -978 +979 @@
     if (!m_parsedProperties.isEmpty()) {
         ok = true;
-        declaration->addParsedProperties(m_parsedProperties.data(), m_parsedProperties.size());
+        declaration->addParsedProperties(m_parsedProperties);
         clearProperties();
     }
@@ -1074 +1075 @@
     if (!m_parsedProperties.isEmpty()) {
         ok = true;
-        declaration->addParsedProperties(m_parsedProperties.data(), m_parsedProperties.size());
+        declaration->addParsedProperties(m_parsedProperties);
         clearProperties();
     }
@@ -1108 +1109 @@
 
     return m_mediaQuery.release();
+}
+
+Vector<CSSProperty> CSSParser::filteredProperties() const
+{
+    BitArray<numCSSProperties> seenProperties;
+    BitArray<numCSSProperties> seenImportantProperties;
+
+    Vector<CSSProperty> results;
+    results.reserveInitialCapacity(m_parsedProperties.size());
+
+    for (unsigned i = 0; i < m_parsedProperties.size(); ++i) {
+        const CSSProperty& property = m_parsedProperties[i];
+        const unsigned propertyIDIndex = property.id() - firstCSSProperty;
+
+        // Ignore non-important properties if we already have an important property with the same ID.
+        if (!property.isImportant() && seenImportantProperties.get(propertyIDIndex))
+            continue;
+
+        // If we already had this property, this new one takes precedence, so wipe out the old one.
+        if (seenProperties.get(propertyIDIndex)) {
+            for (unsigned i = 0; i < results.size(); ++i) {
+                if (results[i].id() == property.id()) {
+                    results.remove(i);
+                    break;
+                }
+            }
+        }
+
+        if (property.isImportant())
+            seenImportantProperties.set(propertyIDIndex);
+        seenProperties.set(propertyIDIndex);
+
+        results.append(property);
+    }
+
+    return results;
 }
 
@@ -9042 +9079 @@
         if (m_hasFontFaceOnlyValues)
             deleteFontFaceOnlyValues();
-        rule->setProperties(StylePropertySet::create(m_parsedProperties.data(), m_parsedProperties.size(), m_context.mode));
+        rule->setProperties(StylePropertySet::create(filteredProperties(), m_context.mode));
         result = rule.get();
         m_parsedRules.append(rule.release());
@@ -9078 +9115 @@
     }
     RefPtr<StyleRuleFontFace> rule = StyleRuleFontFace::create();
-    rule->setProperties(StylePropertySet::create(m_parsedProperties.data(), m_parsedProperties.size(), m_context.mode));
+    rule->setProperties(StylePropertySet::create(filteredProperties(), m_context.mode));
     clearProperties();
     StyleRuleFontFace* result = rule.get();
@@ -9151 +9188 @@
         selectorVector.append(pageSelector);
         rule->parserAdoptSelectorVector(selectorVector);
-        rule->setProperties(StylePropertySet::create(m_parsedProperties.data(), m_parsedProperties.size(), m_context.mode));
+        rule->setProperties(StylePropertySet::create(filteredProperties(), m_context.mode));
         pageRule = rule.get();
         m_parsedRules.append(rule.release());
@@ -9229 +9266 @@
     RefPtr<StyleKeyframe> keyframe = StyleKeyframe::create();
     keyframe->setKeyText(keyString);
-    keyframe->setProperties(StylePropertySet::create(m_parsedProperties.data(), m_parsedProperties.size(), m_context.mode));
+    keyframe->setProperties(StylePropertySet::create(filteredProperties(), m_context.mode));
 
     clearProperties();

trunk/Source/WebCore/css/CSSParser.h

@@ -283 +283 @@
     void clearProperties();
 
+    Vector<CSSProperty> filteredProperties() const;
+
     CSSParserContext m_context;
 

trunk/Source/WebCore/css/StylePropertySet.cpp

@@ -57 +57 @@
 }
 
-StylePropertySet::StylePropertySet(const Vector<CSSProperty>& properties)
+StylePropertySet::StylePropertySet(const Vector<CSSProperty>& properties, CSSParserMode cssParserMode)
     : m_properties(properties)
-    , m_cssParserMode(CSSStrictMode)
+    , m_cssParserMode(cssParserMode)
     , m_ownsCSSOMWrapper(false)
 {
     m_properties.shrinkToFit();
-}
-
-StylePropertySet::StylePropertySet(const CSSProperty* properties, int numProperties, CSSParserMode cssParserMode)
-    : m_cssParserMode(cssParserMode)
-    , m_ownsCSSOMWrapper(false)
-{
-    // FIXME: This logic belongs in CSSParser.
-
-    m_properties.reserveInitialCapacity(numProperties);
-    HashMap<int, bool> candidates;
-    for (int i = 0; i < numProperties; ++i) {
-        const CSSProperty& property = properties[i];
-        bool important = property.isImportant();
-
-        HashMap<int, bool>::iterator it = candidates.find(property.id());
-        if (it != candidates.end()) {
-            if (!important && it->second)
-                continue;
-            removeProperty(property.id());
-        }
-
-        m_properties.append(property);
-        candidates.set(property.id(), important);
-    }
 }
 
@@ -593 +569 @@
 }
 
-void StylePropertySet::addParsedProperties(const CSSProperty* properties, int numProperties)
-{
-    m_properties.reserveCapacity(numProperties);
-    for (int i = 0; i < numProperties; ++i)
+void StylePropertySet::addParsedProperties(const Vector<CSSProperty>& properties)
+{
+    m_properties.reserveCapacity(m_properties.size() + properties.size());
+    for (unsigned i = 0; i < properties.size(); ++i)
         addParsedProperty(properties[i]);
 }

trunk/Source/WebCore/css/StylePropertySet.h

@@ -48 +48 @@
         return adoptRef(new StylePropertySet(cssParserMode));
     }
-    static PassRefPtr<StylePropertySet> create(const CSSProperty* properties, int numProperties, CSSParserMode cssParserMode)
+    static PassRefPtr<StylePropertySet> create(const Vector<CSSProperty>& properties, CSSParserMode cssParserMode = CSSStrictMode)
     {
-        return adoptRef(new StylePropertySet(properties, numProperties, cssParserMode));
-    }
-    static PassRefPtr<StylePropertySet> create(const Vector<CSSProperty>& properties)
-    {
-        return adoptRef(new StylePropertySet(properties));
+        return adoptRef(new StylePropertySet(properties, cssParserMode));
     }
 
@@ -81 +77 @@
     void parseDeclaration(const String& styleDeclaration, StyleSheetInternal* contextStyleSheet);
 
-    void addParsedProperties(const CSSProperty*, int numProperties);
+    void addParsedProperties(const Vector<CSSProperty>&);
     void addParsedProperty(const CSSProperty&);
 
@@ -122 +118 @@
 private:
     StylePropertySet(CSSParserMode);
-    StylePropertySet(const Vector<CSSProperty>&);
+    StylePropertySet(const Vector<CSSProperty>&, CSSParserMode);
     StylePropertySet(const StylePropertySet&);
-    StylePropertySet(const CSSProperty*, int numProperties, CSSParserMode);
 
     void setNeedsStyleRecalc();

trunk/Source/WebCore/svg/SVGFontFaceElement.cpp

@@ -298 +298 @@
 
     // Parse in-memory CSS rules
-    CSSProperty srcProperty(CSSPropertySrc, list);
-    m_fontFaceRule->properties()->addParsedProperties(&srcProperty, 1);
+    m_fontFaceRule->properties()->addParsedProperty(CSSProperty(CSSPropertySrc, list));
 
     if (describesParentFont) {