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;
}