REGRESSION(r125294): A style rule with more than 8192 selectors can cause style corru...
authorakling@apple.com <akling@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 8 Jul 2013 15:53:54 +0000 (15:53 +0000)
committerakling@apple.com <akling@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 8 Jul 2013 15:53:54 +0000 (15:53 +0000)
<http://webkit.org/b/118369>
<rdar://problem/14291428>

Reviewed by Antti Koivisto.

Source/WebCore:

Add a semi-hard cap of 8192 selectors per CSS rule. Rules with longer selector lists will be split into
multiple rules (with 8192 selectors per rule.) The style properties are shared behind the scenes
so it's all pretty efficient.

This does generate a different CSSOM for gigantic selector lists, I've added a test to document this.
Note that setting CSSStyleRule.selectorText with over 8192 selectors will now fail, the splitting into
multiple rules only happens when parsing full style sheets.

Test: fast/css/rule-selector-overflow.html

* css/CSSSelectorList.cpp:
(WebCore::CSSSelectorList::selectorCount):

    Add a helper to count the number of selectors in a list (not the number of raw selector components.)

* css/RuleSet.h:
* css/CSSStyleRule.cpp:
(WebCore::CSSStyleRule::setSelectorText):

    Add RuleData::maximumSelectorCount constant and cap CSSStyleRule.selectorText to that many selectors.

* css/CSSSelectorList.h:
(WebCore::CSSSelectorList::adoptSelectorArray):
* css/StyleRule.cpp:
(WebCore::StyleRule::create):
(WebCore::StyleRule::splitIntoMultipleRulesWithMaximumSelectorCount):
* css/StyleRule.h:
(WebCore::StyleRule::parserAdoptSelectorArray):
* css/StyleSheetContents.cpp:
(WebCore::StyleSheetContents::parserAppendRule):

    The meat of this change. Split rules with >8K selectors into multiple rules with 8K selectors in each.

* css/StyleRule.h:
(WebCore::toStyleRule):

    Added a toStyleRule() helper for good measure.

LayoutTests:

Added a test to document the new cap of 8192 selectors per CSS rule.

* fast/css/rule-selector-overflow-expected.txt: Added.
* fast/css/rule-selector-overflow.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@152453 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/css/rule-selector-overflow-expected.txt [new file with mode: 0644]
LayoutTests/fast/css/rule-selector-overflow.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/css/CSSSelectorList.cpp
Source/WebCore/css/CSSSelectorList.h
Source/WebCore/css/CSSStyleRule.cpp
Source/WebCore/css/RuleSet.h
Source/WebCore/css/StyleRule.cpp
Source/WebCore/css/StyleRule.h
Source/WebCore/css/StyleSheetContents.cpp

index 0cbde02..7a9c0e4 100644 (file)
@@ -1,3 +1,16 @@
+2013-07-08  Andreas Kling  <akling@apple.com>
+
+        REGRESSION(r125294): A style rule with more than 8192 selectors can cause style corruption.
+        <http://webkit.org/b/118369>
+        <rdar://problem/14291428>
+
+        Reviewed by Antti Koivisto.
+
+        Added a test to document the new cap of 8192 selectors per CSS rule.
+
+        * fast/css/rule-selector-overflow-expected.txt: Added.
+        * fast/css/rule-selector-overflow.html: Added.
+
 2013-07-08  Simon Pena  <simon.pena@samsung.com>
 
         [GTK] Unreviewed gardening. Update TestExpectations
diff --git a/LayoutTests/fast/css/rule-selector-overflow-expected.txt b/LayoutTests/fast/css/rule-selector-overflow-expected.txt
new file mode 100644 (file)
index 0000000..4d0cd75
--- /dev/null
@@ -0,0 +1,27 @@
+This test tests and documents the behavior of CSS style rules with a massive number of selectors. Rules with >8192 selectors get split into multiple rules at the parsing stage. Setting a rule's selectorText via CSSOM will do nothing if there are more than 8192 selectors.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS rule().selectorText = selectorListWithLength(1); rule().selectorText is selectorListWithLength(1)
+PASS rule().selectorText = selectorListWithLength(8192); rule().selectorText is selectorListWithLength(8192)
+PASS rule().selectorText = '.reset'; rule().selectorText is '.reset'
+PASS rule().selectorText = selectorListWithLength(8193); rule().selectorText is '.reset'
+PASS rule().selectorText = '.reset'; rule().selectorText is '.reset'
+PASS rule().selectorText = selectorListWithLength(8193); sheet().rules.length is 1
+PASS rule().selectorText = selectorListWithLength(8192); rule().selectorText is selectorListWithLength(8192)
+PASS rule().selectorText = selectorListWithLength(8192); sheet().rules.length is 1
+PASS rule().selectorText = '.reset'; rule().selectorText is '.reset'
+PASS rule().selectorText = selectorListWithLength(8193); rule().selectorText is '.reset'
+PASS rule().selectorText = selectorListWithLength(8193); sheet().rules.length is 1
+PASS styleElement.innerText = styleSheetWithSelectorLength(1); rule().selectorText is selectorListWithLength(1)
+PASS styleElement.innerText = styleSheetWithSelectorLength(8192); rule().selectorText is selectorListWithLength(8192)
+PASS styleElement.innerText = styleSheetWithSelectorLength(8192); sheet().rules.length is 1
+PASS styleElement.innerText = styleSheetWithSelectorLength(8193); rule().selectorText is selectorListWithLength(8192)
+PASS styleElement.innerText = styleSheetWithSelectorLength(8193); sheet().rules.length is 2
+PASS styleElement.innerText = styleSheetWithSelectorLength(16384); sheet().rules.length is 2
+PASS styleElement.innerText = styleSheetWithSelectorLength(16385); sheet().rules.length is 3
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/css/rule-selector-overflow.html b/LayoutTests/fast/css/rule-selector-overflow.html
new file mode 100644 (file)
index 0000000..763ebbf
--- /dev/null
@@ -0,0 +1,65 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta charset="utf-8">
+<script src="../js/resources/js-test-pre.js"></script>
+<style id="stylebro" type="text/css">
+.peb {
+    color: red;
+}
+</style>
+</head>
+<body>
+<script>
+
+styleElement = document.getElementById("stylebro");
+
+function selectorListWithLength(length)
+{
+    var a = new Array();
+    for (i = 0; i < length; ++i)
+        a.push(".x");
+    return a.join(", ");
+}
+
+function styleSheetWithSelectorLength(length)
+{
+    return selectorListWithLength(length) + " { color: red; }";
+}
+
+function sheet()
+{
+    return styleElement.sheet;
+}
+
+function rule()
+{
+    return sheet().cssRules[0];
+}
+
+description("This test tests and documents the behavior of CSS style rules with a massive number of selectors. Rules with >8192 selectors get split into multiple rules at the parsing stage. Setting a rule's selectorText via CSSOM will do nothing if there are more than 8192 selectors.");
+
+shouldBe("rule().selectorText = selectorListWithLength(1); rule().selectorText", "selectorListWithLength(1)");
+shouldBe("rule().selectorText = selectorListWithLength(8192); rule().selectorText", "selectorListWithLength(8192)");
+shouldBe("rule().selectorText = '.reset'; rule().selectorText", "'.reset'");
+shouldBe("rule().selectorText = selectorListWithLength(8193); rule().selectorText", "'.reset'");
+shouldBe("rule().selectorText = '.reset'; rule().selectorText", "'.reset'");
+shouldBe("rule().selectorText = selectorListWithLength(8193); sheet().rules.length", "1");
+shouldBe("rule().selectorText = selectorListWithLength(8192); rule().selectorText", "selectorListWithLength(8192)");
+shouldBe("rule().selectorText = selectorListWithLength(8192); sheet().rules.length", "1");
+shouldBe("rule().selectorText = '.reset'; rule().selectorText", "'.reset'");
+shouldBe("rule().selectorText = selectorListWithLength(8193); rule().selectorText", "'.reset'");
+shouldBe("rule().selectorText = selectorListWithLength(8193); sheet().rules.length", "1");
+
+shouldBe("styleElement.innerText = styleSheetWithSelectorLength(1); rule().selectorText", "selectorListWithLength(1)");
+shouldBe("styleElement.innerText = styleSheetWithSelectorLength(8192); rule().selectorText", "selectorListWithLength(8192)");
+shouldBe("styleElement.innerText = styleSheetWithSelectorLength(8192); sheet().rules.length", "1");
+shouldBe("styleElement.innerText = styleSheetWithSelectorLength(8193); rule().selectorText", "selectorListWithLength(8192)");
+shouldBe("styleElement.innerText = styleSheetWithSelectorLength(8193); sheet().rules.length", "2");
+shouldBe("styleElement.innerText = styleSheetWithSelectorLength(16384); sheet().rules.length", "2");
+shouldBe("styleElement.innerText = styleSheetWithSelectorLength(16385); sheet().rules.length", "3");
+
+</script>
+<script src="../js/resources/js-test-post.js"></script>
+</body>
+</html>
index ce08379..a50763f 100644 (file)
@@ -1,3 +1,49 @@
+2013-07-08  Andreas Kling  <akling@apple.com>
+
+        REGRESSION(r125294): A style rule with more than 8192 selectors can cause style corruption.
+        <http://webkit.org/b/118369>
+        <rdar://problem/14291428>
+
+        Reviewed by Antti Koivisto.
+
+        Add a semi-hard cap of 8192 selectors per CSS rule. Rules with longer selector lists will be split into
+        multiple rules (with 8192 selectors per rule.) The style properties are shared behind the scenes
+        so it's all pretty efficient.
+
+        This does generate a different CSSOM for gigantic selector lists, I've added a test to document this.
+        Note that setting CSSStyleRule.selectorText with over 8192 selectors will now fail, the splitting into
+        multiple rules only happens when parsing full style sheets.
+
+        Test: fast/css/rule-selector-overflow.html
+
+        * css/CSSSelectorList.cpp:
+        (WebCore::CSSSelectorList::selectorCount):
+
+            Add a helper to count the number of selectors in a list (not the number of raw selector components.)
+
+        * css/RuleSet.h:
+        * css/CSSStyleRule.cpp:
+        (WebCore::CSSStyleRule::setSelectorText):
+
+            Add RuleData::maximumSelectorCount constant and cap CSSStyleRule.selectorText to that many selectors.
+
+        * css/CSSSelectorList.h:
+        (WebCore::CSSSelectorList::adoptSelectorArray):
+        * css/StyleRule.cpp:
+        (WebCore::StyleRule::create):
+        (WebCore::StyleRule::splitIntoMultipleRulesWithMaximumSelectorCount):
+        * css/StyleRule.h:
+        (WebCore::StyleRule::parserAdoptSelectorArray):
+        * css/StyleSheetContents.cpp:
+        (WebCore::StyleSheetContents::parserAppendRule):
+
+            The meat of this change. Split rules with >8K selectors into multiple rules with 8K selectors in each.
+
+        * css/StyleRule.h:
+        (WebCore::toStyleRule):
+
+            Added a toStyleRule() helper for good measure.
+
 2013-07-08  Christophe Dumez  <ch.dumez@sisa.samsung.com>
 
         Introduce DECLARE_FORWARDING_ATTRIBUTE_EVENT_LISTENER() macro
index f76b4a8..0cec28c 100644 (file)
@@ -85,6 +85,14 @@ void CSSSelectorList::adoptSelectorVector(Vector<OwnPtr<CSSParserSelector> >& se
     selectorVector.clear();
 }
 
+unsigned CSSSelectorList::selectorCount() const
+{
+    unsigned count = 0;
+    for (const CSSSelector* s = first(); s; s = next(s))
+        ++count;
+    return count;
+}
+
 unsigned CSSSelectorList::length() const
 {
     if (!m_selectorArray)
index bd85ed5..6d40bb4 100644 (file)
@@ -42,6 +42,7 @@ public:
 
     void adopt(CSSSelectorList& list);
     void adoptSelectorVector(Vector<OwnPtr<CSSParserSelector> >& selectorVector);
+    void adoptSelectorArray(CSSSelector* selectors) { ASSERT(!m_selectorArray); m_selectorArray = selectors; }
 
     bool isValid() const { return !!m_selectorArray; }
     const CSSSelector* first() const { return m_selectorArray; }
@@ -63,6 +64,8 @@ public:
 
     String selectorsText() const;
 
+    unsigned selectorCount() const;
+
 private:
     unsigned length() const;
     void deleteSelectors();
index 08ed4d4..d31917e 100644 (file)
@@ -27,6 +27,7 @@
 #include "CSSStyleSheet.h"
 #include "Document.h"
 #include "PropertySetCSSStyleDeclaration.h"
+#include "RuleSet.h"
 #include "StylePropertySet.h"
 #include "StyleRule.h"
 #include <wtf/text/StringBuilder.h>
@@ -98,6 +99,10 @@ void CSSStyleRule::setSelectorText(const String& selectorText)
     if (!selectorList.isValid())
         return;
 
+    // NOTE: The selector list has to fit into RuleData. <http://webkit.org/b/118369>
+    if (selectorList.selectorCount() > RuleData::maximumSelectorCount)
+        return;
+
     CSSStyleSheet::RuleMutationScope mutationScope(this);
 
     m_styleRule->wrapperAdoptSelectorList(selectorList);
index 37e7de6..b71a35b 100644 (file)
@@ -55,6 +55,8 @@ class StyleSheetContents;
 
 class RuleData {
 public:
+    static const unsigned maximumSelectorCount = 8192;
+
     RuleData(StyleRule*, unsigned selectorIndex, unsigned position, AddRuleFlags);
 
     unsigned position() const { return m_position; }
index d070875..fb2c237 100644 (file)
@@ -260,6 +260,44 @@ void StyleRule::setProperties(PassRefPtr<StylePropertySet> properties)
     m_properties = properties;
 }
 
+PassRefPtr<StyleRule> StyleRule::create(int sourceLine, const Vector<const CSSSelector*>& selectors, PassRefPtr<StylePropertySet> properties)
+{
+    CSSSelector* selectorListArray = reinterpret_cast<CSSSelector*>(fastMalloc(sizeof(CSSSelector) * selectors.size()));
+    for (unsigned i = 0; i < selectors.size(); ++i)
+        new (NotNull, &selectorListArray[i]) CSSSelector(*selectors.at(i));
+    selectorListArray[selectors.size() - 1].setLastInSelectorList();
+    RefPtr<StyleRule> rule = StyleRule::create(sourceLine);
+    rule->parserAdoptSelectorArray(selectorListArray);
+    rule->setProperties(properties);
+    return rule.release();
+}
+
+Vector<RefPtr<StyleRule> > StyleRule::splitIntoMultipleRulesWithMaximumSelectorCount(unsigned maximumSelectorCount) const
+{
+    ASSERT(selectorList().selectorCount() > maximumSelectorCount);
+
+    Vector<RefPtr<StyleRule> > rules;
+    Vector<const CSSSelector*> selectorsToCopy;
+
+    unsigned selectorCount = 0;
+
+    for (const CSSSelector* selector = selectorList().first(); selector; selector = CSSSelectorList::next(selector)) {
+        for (const CSSSelector* component = selector; component; component = component->tagHistory())
+            selectorsToCopy.append(component);
+
+        if (++selectorCount == maximumSelectorCount) {
+            rules.append(create(sourceLine(), selectorsToCopy, m_properties));
+            selectorsToCopy.clear();
+            selectorCount = 0;
+        }
+    }
+
+    if (selectorCount)
+        rules.append(create(sourceLine(), selectorsToCopy, m_properties));
+
+    return rules;
+}
+
 StyleRulePage::StyleRulePage()
     : StyleRuleBase(Page)
 {
index 9e9ca99..2725b57 100644 (file)
@@ -127,20 +127,31 @@ public:
     
     void parserAdoptSelectorVector(Vector<OwnPtr<CSSParserSelector> >& selectors) { m_selectorList.adoptSelectorVector(selectors); }
     void wrapperAdoptSelectorList(CSSSelectorList& selectors) { m_selectorList.adopt(selectors); }
+    void parserAdoptSelectorArray(CSSSelector* selectors) { m_selectorList.adoptSelectorArray(selectors); }
     void setProperties(PassRefPtr<StylePropertySet>);
 
     PassRefPtr<StyleRule> copy() const { return adoptRef(new StyleRule(*this)); }
 
+    Vector<RefPtr<StyleRule> > splitIntoMultipleRulesWithMaximumSelectorCount(unsigned maxSelectorCount) const;
+
     static unsigned averageSizeInBytes();
 
 private:
     StyleRule(int sourceLine);
     StyleRule(const StyleRule&);
 
+    static PassRefPtr<StyleRule> create(int sourceLine, const Vector<const CSSSelector*>&, PassRefPtr<StylePropertySet>);
+
     RefPtr<StylePropertySet> m_properties;
     CSSSelectorList m_selectorList;
 };
 
+inline const StyleRule* toStyleRule(const StyleRuleBase* rule)
+{
+    ASSERT_WITH_SECURITY_IMPLICATION(!rule || rule->isStyleRule());
+    return static_cast<const StyleRule*>(rule);
+}
+
 class StyleRuleFontFace : public StyleRuleBase {
 public:
     static PassRefPtr<StyleRuleFontFace> create() { return adoptRef(new StyleRuleFontFace); }
index a1e0dc8..eb50420 100644 (file)
@@ -1,6 +1,6 @@
 /*
  * (C) 1999-2003 Lars Knoll (knoll@kde.org)
- * Copyright (C) 2004, 2006, 2007, 2012 Apple Inc. All rights reserved.
+ * Copyright (C) 2004, 2006, 2007, 2012, 2013 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ -28,6 +28,7 @@
 #include "Document.h"
 #include "MediaList.h"
 #include "Node.h"
+#include "RuleSet.h"
 #include "SecurityOrigin.h"
 #include "StylePropertySet.h"
 #include "StyleRule.h"
@@ -140,6 +141,14 @@ void StyleSheetContents::parserAppendRule(PassRefPtr<StyleRuleBase> rule)
         reportMediaQueryWarningIfNeeded(singleOwnerDocument(), static_cast<StyleRuleMedia*>(rule.get())->mediaQueries());
 #endif
 
+    // NOTE: The selector list has to fit into RuleData. <http://webkit.org/b/118369>
+    // If we're adding a rule with a huge number of selectors, split it up into multiple rules
+    if (rule->isStyleRule() && toStyleRule(rule.get())->selectorList().selectorCount() > RuleData::maximumSelectorCount) {
+        Vector<RefPtr<StyleRule> > rules = toStyleRule(rule.get())->splitIntoMultipleRulesWithMaximumSelectorCount(RuleData::maximumSelectorCount);
+        m_childRules.appendVector(rules);
+        return;
+    }
+
     m_childRules.append(rule);
 }