Title: [152788] trunk
Revision
152788
Author
[email protected]
Date
2013-07-17 10:21:51 -0700 (Wed, 17 Jul 2013)

Log Message

CSS selector list splitting should be by component, not by selector.
<http://webkit.org/b/118761>
<rdar://problem/14421609>

Reviewed by Antti Koivisto.

Source/WebCore:

Test (amended): fast/css/rule-selector-overflow.html

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

    Renamed CSSSelectorList::length() to componentCount() and made it public.

* css/RuleSet.h:

    maximumSelectorCount => maximumSelectorComponentCount

* css/StyleRule.cpp:
(WebCore::StyleRule::splitIntoMultipleRulesWithMaximumSelectorComponentCount):

    Make the splits after accumulating 'maximumSelectorComponentCount' components.

* css/StyleRule.h:
* css/StyleSheetContents.cpp:
(WebCore::StyleSheetContents::parserAppendRule):

    splitIntoMultipleRulesWithMaximumSelectorCount => splitIntoMultipleRulesWithMaximumSelectorComponentCount

LayoutTests:

Added more cases to the already existing selector list splitting test.

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

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (152787 => 152788)


--- trunk/LayoutTests/ChangeLog	2013-07-17 17:10:21 UTC (rev 152787)
+++ trunk/LayoutTests/ChangeLog	2013-07-17 17:21:51 UTC (rev 152788)
@@ -1,3 +1,16 @@
+2013-07-17  Andreas Kling  <[email protected]>
+
+        CSS selector list splitting should be by component, not by selector.
+        <http://webkit.org/b/118761>
+        <rdar://problem/14421609>
+
+        Reviewed by Antti Koivisto.
+
+        Added more cases to the already existing selector list splitting test.
+
+        * fast/css/rule-selector-overflow-expected.txt:
+        * fast/css/rule-selector-overflow.html:
+
 2013-07-17  Rob Buis  <[email protected]>
 
         [Mac] REGRESSION(r152685): svg/custom/xlink-prefix-in-attributes.html failed unexpectedly

Modified: trunk/LayoutTests/fast/css/rule-selector-overflow-expected.txt (152787 => 152788)


--- trunk/LayoutTests/fast/css/rule-selector-overflow-expected.txt	2013-07-17 17:10:21 UTC (rev 152787)
+++ trunk/LayoutTests/fast/css/rule-selector-overflow-expected.txt	2013-07-17 17:21:51 UTC (rev 152788)
@@ -1,4 +1,4 @@
-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.
+This test tests and documents the behavior of CSS style rules with a massive number of selectors. Rules with >8192 selector components 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 components.
 
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
@@ -14,6 +14,11 @@
 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 rule().selectorText = fatSelectorListWithLength(1); sheet().rules.length is 1
+PASS rule().selectorText = fatSelectorListWithLength(1); rule().selectorText is fatSelectorListWithLength(1)
+PASS rule().selectorText = fatSelectorListWithLength(2048); rule().selectorText is fatSelectorListWithLength(2048)
+PASS rule().selectorText = '.reset'; rule().selectorText is '.reset'
+PASS rule().selectorText = fatSelectorListWithLength(2049); rule().selectorText is '.reset'
 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
@@ -21,6 +26,14 @@
 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 styleElement.innerText = fatStyleSheetWithSelectorLength(1); sheet().rules.length is 1
+PASS styleElement.innerText = fatStyleSheetWithSelectorLength(2048); sheet().rules.length is 1
+PASS styleElement.innerText = fatStyleSheetWithSelectorLength(2049); sheet().rules.length is 2
+PASS styleElement.innerText = fatStyleSheetWithSelectorLength(4096); sheet().rules.length is 2
+PASS styleElement.innerText = fatStyleSheetWithSelectorLength(4097); sheet().rules.length is 3
+PASS styleElement.innerText = fatStyleSheetWithSelectorLength(16385); sheet().rules.length is 9
+PASS styleElement.innerText = fatStyleSheetWithSelectorLength(16384); sheet().rules.length is 8
+PASS styleElement.innerText = fatStyleSheetWithSelectorLength(16385); sheet().rules.length is 9
 PASS successfullyParsed is true
 
 TEST COMPLETE

Modified: trunk/LayoutTests/fast/css/rule-selector-overflow.html (152787 => 152788)


--- trunk/LayoutTests/fast/css/rule-selector-overflow.html	2013-07-17 17:10:21 UTC (rev 152787)
+++ trunk/LayoutTests/fast/css/rule-selector-overflow.html	2013-07-17 17:21:51 UTC (rev 152788)
@@ -14,19 +14,34 @@
 
 styleElement = document.getElementById("stylebro");
 
-function selectorListWithLength(length)
+function repeatAndJoin(s, count)
 {
     var a = new Array();
-    for (i = 0; i < length; ++i)
-        a.push(".x");
+    for (i = 0; i < count; ++i)
+        a.push(s);
     return a.join(", ");
 }
 
+function selectorListWithLength(length)
+{
+    return repeatAndJoin(".x", length);
+}
+
+function fatSelectorListWithLength(length)
+{
+    return repeatAndJoin(".x .y .z .q", length);
+}
+
 function styleSheetWithSelectorLength(length)
 {
     return selectorListWithLength(length) + " { color: red; }";
 }
 
+function fatStyleSheetWithSelectorLength(length)
+{
+    return fatSelectorListWithLength(length) + " { color: red; }";
+}
+
 function sheet()
 {
     return styleElement.sheet;
@@ -37,7 +52,7 @@
     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.");
+description("This test tests and documents the behavior of CSS style rules with a massive number of selectors. Rules with >8192 selector components 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 components.");
 
 shouldBe("rule().selectorText = selectorListWithLength(1); rule().selectorText", "selectorListWithLength(1)");
 shouldBe("rule().selectorText = selectorListWithLength(8192); rule().selectorText", "selectorListWithLength(8192)");
@@ -51,6 +66,12 @@
 shouldBe("rule().selectorText = selectorListWithLength(8193); rule().selectorText", "'.reset'");
 shouldBe("rule().selectorText = selectorListWithLength(8193); sheet().rules.length", "1");
 
+shouldBe("rule().selectorText = fatSelectorListWithLength(1); sheet().rules.length", "1");
+shouldBe("rule().selectorText = fatSelectorListWithLength(1); rule().selectorText", "fatSelectorListWithLength(1)");
+shouldBe("rule().selectorText = fatSelectorListWithLength(2048); rule().selectorText", "fatSelectorListWithLength(2048)");
+shouldBe("rule().selectorText = '.reset'; rule().selectorText", "'.reset'");
+shouldBe("rule().selectorText = fatSelectorListWithLength(2049); rule().selectorText", "'.reset'");
+
 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");
@@ -59,6 +80,15 @@
 shouldBe("styleElement.innerText = styleSheetWithSelectorLength(16384); sheet().rules.length", "2");
 shouldBe("styleElement.innerText = styleSheetWithSelectorLength(16385); sheet().rules.length", "3");
 
+shouldBe("styleElement.innerText = fatStyleSheetWithSelectorLength(1); sheet().rules.length", "1");
+shouldBe("styleElement.innerText = fatStyleSheetWithSelectorLength(2048); sheet().rules.length", "1");
+shouldBe("styleElement.innerText = fatStyleSheetWithSelectorLength(2049); sheet().rules.length", "2");
+shouldBe("styleElement.innerText = fatStyleSheetWithSelectorLength(4096); sheet().rules.length", "2");
+shouldBe("styleElement.innerText = fatStyleSheetWithSelectorLength(4097); sheet().rules.length", "3");
+shouldBe("styleElement.innerText = fatStyleSheetWithSelectorLength(16385); sheet().rules.length", "9");
+shouldBe("styleElement.innerText = fatStyleSheetWithSelectorLength(16384); sheet().rules.length", "8");
+shouldBe("styleElement.innerText = fatStyleSheetWithSelectorLength(16385); sheet().rules.length", "9");
+
 </script>
 <script src=""
 </body>

Modified: trunk/Source/WebCore/ChangeLog (152787 => 152788)


--- trunk/Source/WebCore/ChangeLog	2013-07-17 17:10:21 UTC (rev 152787)
+++ trunk/Source/WebCore/ChangeLog	2013-07-17 17:21:51 UTC (rev 152788)
@@ -1,3 +1,37 @@
+2013-07-17  Andreas Kling  <[email protected]>
+
+        CSS selector list splitting should be by component, not by selector.
+        <http://webkit.org/b/118761>
+        <rdar://problem/14421609>
+
+        Reviewed by Antti Koivisto.
+
+        Test (amended): fast/css/rule-selector-overflow.html
+
+        * css/CSSSelectorList.h:
+        * css/CSSSelectorList.cpp:
+        (WebCore::CSSSelectorList::CSSSelectorList):
+        (WebCore::CSSSelectorList::componentCount):
+        * css/CSSStyleRule.cpp:
+        (WebCore::CSSStyleRule::setSelectorText):
+
+            Renamed CSSSelectorList::length() to componentCount() and made it public.
+
+        * css/RuleSet.h:
+
+            maximumSelectorCount => maximumSelectorComponentCount
+
+        * css/StyleRule.cpp:
+        (WebCore::StyleRule::splitIntoMultipleRulesWithMaximumSelectorComponentCount):
+
+            Make the splits after accumulating 'maximumSelectorComponentCount' components.
+
+        * css/StyleRule.h:
+        * css/StyleSheetContents.cpp:
+        (WebCore::StyleSheetContents::parserAppendRule):
+
+            splitIntoMultipleRulesWithMaximumSelectorCount => splitIntoMultipleRulesWithMaximumSelectorComponentCount
+
 2013-07-17  Rob Buis  <[email protected]>
 
         [Mac] REGRESSION(r152685): svg/custom/xlink-prefix-in-attributes.html failed unexpectedly

Modified: trunk/Source/WebCore/css/CSSSelectorList.cpp (152787 => 152788)


--- trunk/Source/WebCore/css/CSSSelectorList.cpp	2013-07-17 17:10:21 UTC (rev 152787)
+++ trunk/Source/WebCore/css/CSSSelectorList.cpp	2013-07-17 17:21:51 UTC (rev 152788)
@@ -39,9 +39,9 @@
 
 CSSSelectorList::CSSSelectorList(const CSSSelectorList& other)
 {
-    unsigned otherLength = other.length();
-    m_selectorArray = reinterpret_cast<CSSSelector*>(fastMalloc(sizeof(CSSSelector) * otherLength));
-    for (unsigned i = 0; i < otherLength; ++i)
+    unsigned otherComponentCount = other.componentCount();
+    m_selectorArray = reinterpret_cast<CSSSelector*>(fastMalloc(sizeof(CSSSelector) * otherComponentCount));
+    for (unsigned i = 0; i < otherComponentCount; ++i)
         new (NotNull, &m_selectorArray[i]) CSSSelector(other.m_selectorArray[i]);
 }
 
@@ -85,16 +85,8 @@
     selectorVector.clear();
 }
 
-unsigned CSSSelectorList::selectorCount() const
+unsigned CSSSelectorList::componentCount() const
 {
-    unsigned count = 0;
-    for (const CSSSelector* s = first(); s; s = next(s))
-        ++count;
-    return count;
-}
-
-unsigned CSSSelectorList::length() const
-{
     if (!m_selectorArray)
         return 0;
     CSSSelector* current = m_selectorArray;

Modified: trunk/Source/WebCore/css/CSSSelectorList.h (152787 => 152788)


--- trunk/Source/WebCore/css/CSSSelectorList.h	2013-07-17 17:10:21 UTC (rev 152787)
+++ trunk/Source/WebCore/css/CSSSelectorList.h	2013-07-17 17:21:51 UTC (rev 152788)
@@ -64,10 +64,9 @@
 
     String selectorsText() const;
 
-    unsigned selectorCount() const;
+    unsigned componentCount() const;
 
 private:
-    unsigned length() const;
     void deleteSelectors();
 
     // End of a multipart selector is indicated by m_isLastInTagHistory bit in the last item.

Modified: trunk/Source/WebCore/css/CSSStyleRule.cpp (152787 => 152788)


--- trunk/Source/WebCore/css/CSSStyleRule.cpp	2013-07-17 17:10:21 UTC (rev 152787)
+++ trunk/Source/WebCore/css/CSSStyleRule.cpp	2013-07-17 17:21:51 UTC (rev 152788)
@@ -100,7 +100,7 @@
         return;
 
     // NOTE: The selector list has to fit into RuleData. <http://webkit.org/b/118369>
-    if (selectorList.selectorCount() > RuleData::maximumSelectorCount)
+    if (selectorList.componentCount() > RuleData::maximumSelectorComponentCount)
         return;
 
     CSSStyleSheet::RuleMutationScope mutationScope(this);

Modified: trunk/Source/WebCore/css/RuleSet.h (152787 => 152788)


--- trunk/Source/WebCore/css/RuleSet.h	2013-07-17 17:10:21 UTC (rev 152787)
+++ trunk/Source/WebCore/css/RuleSet.h	2013-07-17 17:21:51 UTC (rev 152788)
@@ -55,7 +55,7 @@
 
 class RuleData {
 public:
-    static const unsigned maximumSelectorCount = 8192;
+    static const unsigned maximumSelectorComponentCount = 8192;
 
     RuleData(StyleRule*, unsigned selectorIndex, unsigned position, AddRuleFlags);
 

Modified: trunk/Source/WebCore/css/StyleRule.cpp (152787 => 152788)


--- trunk/Source/WebCore/css/StyleRule.cpp	2013-07-17 17:10:21 UTC (rev 152787)
+++ trunk/Source/WebCore/css/StyleRule.cpp	2013-07-17 17:21:51 UTC (rev 152788)
@@ -272,28 +272,28 @@
     return rule.release();
 }
 
-Vector<RefPtr<StyleRule> > StyleRule::splitIntoMultipleRulesWithMaximumSelectorCount(unsigned maximumSelectorCount) const
+Vector<RefPtr<StyleRule> > StyleRule::splitIntoMultipleRulesWithMaximumSelectorComponentCount(unsigned maxCount) const
 {
-    ASSERT(selectorList().selectorCount() > maximumSelectorCount);
+    ASSERT(selectorList().componentCount() > maxCount);
 
     Vector<RefPtr<StyleRule> > rules;
-    Vector<const CSSSelector*> selectorsToCopy;
+    Vector<const CSSSelector*> componentsSinceLastSplit;
 
-    unsigned selectorCount = 0;
-
     for (const CSSSelector* selector = selectorList().first(); selector; selector = CSSSelectorList::next(selector)) {
+        Vector<const CSSSelector*, 8> componentsInThisSelector;
         for (const CSSSelector* component = selector; component; component = component->tagHistory())
-            selectorsToCopy.append(component);
+            componentsInThisSelector.append(component);
 
-        if (++selectorCount == maximumSelectorCount) {
-            rules.append(create(sourceLine(), selectorsToCopy, m_properties));
-            selectorsToCopy.clear();
-            selectorCount = 0;
+        if (componentsInThisSelector.size() + componentsSinceLastSplit.size() > maxCount) {
+            rules.append(create(sourceLine(), componentsSinceLastSplit, m_properties));
+            componentsSinceLastSplit.clear();
         }
+
+        componentsSinceLastSplit.appendVector(componentsInThisSelector);
     }
 
-    if (selectorCount)
-        rules.append(create(sourceLine(), selectorsToCopy, m_properties));
+    if (!componentsSinceLastSplit.isEmpty())
+        rules.append(create(sourceLine(), componentsSinceLastSplit, m_properties));
 
     return rules;
 }

Modified: trunk/Source/WebCore/css/StyleRule.h (152787 => 152788)


--- trunk/Source/WebCore/css/StyleRule.h	2013-07-17 17:10:21 UTC (rev 152787)
+++ trunk/Source/WebCore/css/StyleRule.h	2013-07-17 17:21:51 UTC (rev 152788)
@@ -132,7 +132,7 @@
 
     PassRefPtr<StyleRule> copy() const { return adoptRef(new StyleRule(*this)); }
 
-    Vector<RefPtr<StyleRule> > splitIntoMultipleRulesWithMaximumSelectorCount(unsigned maxSelectorCount) const;
+    Vector<RefPtr<StyleRule> > splitIntoMultipleRulesWithMaximumSelectorComponentCount(unsigned) const;
 
     static unsigned averageSizeInBytes();
 

Modified: trunk/Source/WebCore/css/StyleSheetContents.cpp (152787 => 152788)


--- trunk/Source/WebCore/css/StyleSheetContents.cpp	2013-07-17 17:10:21 UTC (rev 152787)
+++ trunk/Source/WebCore/css/StyleSheetContents.cpp	2013-07-17 17:21:51 UTC (rev 152788)
@@ -143,8 +143,8 @@
 
     // 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);
+    if (rule->isStyleRule() && toStyleRule(rule.get())->selectorList().componentCount() > RuleData::maximumSelectorComponentCount) {
+        Vector<RefPtr<StyleRule> > rules = toStyleRule(rule.get())->splitIntoMultipleRulesWithMaximumSelectorComponentCount(RuleData::maximumSelectorComponentCount);
         m_childRules.appendVector(rules);
         return;
     }
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to