- Revision
- 173910
- Author
- [email protected]
- Date
- 2014-09-24 00:56:52 -0700 (Wed, 24 Sep 2014)
Log Message
Remove the style marking from :nth-child()
https://bugs.webkit.org/show_bug.cgi?id=137055
Patch by Benjamin Poulain <[email protected]> on 2014-09-24
Reviewed by Andreas Kling.
Source/WebCore:
Previously, :nth-child() had to mark the RenderStyle as unique in order
to prevent it from being used for style sharing.
After r173229, :nth-child() use the more generic element marking
"StyleIsAffectedByPreviousSibling".
In StyleResolver::canShareStyleWithElement(), StyleIsAffectedByPreviousSibling
is already used to prevent style sharing of those elements, making the "Unique"
flag redundant.
Since it is now useless, remove the style marking from SelectorChecker and the CSS JIT.
Tests: fast/css/nth-child-style-sharing-even.html
fast/css/nth-child-style-sharing-fixed-integer.html
fast/css/nth-child-style-sharing-odd.html
* css/SelectorChecker.cpp:
(WebCore::SelectorChecker::checkOne):
* cssjit/SelectorCompiler.cpp:
(WebCore::SelectorCompiler::SelectorCodeGenerator::generateElementIsNthChild):
(WebCore::SelectorCompiler::setElementChildIndexAndUpdateStyle): Deleted.
* rendering/style/RenderStyle.h:
LayoutTests:
Add basic tests for style sharing with :nth-child().
* fast/css/nth-child-style-sharing-even-expected.html: Added.
* fast/css/nth-child-style-sharing-even.html: Added.
* fast/css/nth-child-style-sharing-fixed-integer-expected.html: Added.
* fast/css/nth-child-style-sharing-fixed-integer.html: Added.
* fast/css/nth-child-style-sharing-odd-expected.html: Added.
* fast/css/nth-child-style-sharing-odd.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (173909 => 173910)
--- trunk/LayoutTests/ChangeLog 2014-09-24 05:10:18 UTC (rev 173909)
+++ trunk/LayoutTests/ChangeLog 2014-09-24 07:56:52 UTC (rev 173910)
@@ -1,3 +1,19 @@
+2014-09-24 Benjamin Poulain <[email protected]>
+
+ Remove the style marking from :nth-child()
+ https://bugs.webkit.org/show_bug.cgi?id=137055
+
+ Reviewed by Andreas Kling.
+
+ Add basic tests for style sharing with :nth-child().
+
+ * fast/css/nth-child-style-sharing-even-expected.html: Added.
+ * fast/css/nth-child-style-sharing-even.html: Added.
+ * fast/css/nth-child-style-sharing-fixed-integer-expected.html: Added.
+ * fast/css/nth-child-style-sharing-fixed-integer.html: Added.
+ * fast/css/nth-child-style-sharing-odd-expected.html: Added.
+ * fast/css/nth-child-style-sharing-odd.html: Added.
+
2014-09-23 Benjamin Poulain <[email protected]>
The style resolution cache applies properties incorrectly whenever direction != ltr
Added: trunk/LayoutTests/fast/css/nth-child-style-sharing-even-expected.html (0 => 173910)
--- trunk/LayoutTests/fast/css/nth-child-style-sharing-even-expected.html (rev 0)
+++ trunk/LayoutTests/fast/css/nth-child-style-sharing-even-expected.html 2014-09-24 07:56:52 UTC (rev 173910)
@@ -0,0 +1,44 @@
+<!doctype html>
+<html>
+<head>
+<style>
+div {
+ background-color: white;
+}
+</style>
+</head>
+<body>
+ <section>
+ <p>This tests basic style sharing with :nth-child(even). If the test pass, lines with the text "green" should have a green background.</p>
+ <div style="background-color: lime">Green</div>
+ <div>White</div>
+ <div style="background-color: lime">Green</div>
+ <div>
+ <div>White</div>
+ <div style="background-color: lime">Green</div>
+ <div>White</div>
+ <div style="background-color: lime">Green</div>
+ </div>
+ <div style="background-color: lime">
+ <div>White</div>
+ <div style="background-color: lime">Green</div>
+ <div>White</div>
+ <div style="background-color: lime">Green</div>
+ </div>
+ </section>
+
+ <section>
+ <div>White</div>
+ <div style="background-color: lime">Green</div>
+ <div>White</div>
+ <div style="background-color: lime">Green</div>
+ <div>
+ <div>White</div>
+ <div style="background-color: lime">Green</div>
+ <div>White</div>
+ <div style="background-color: lime">Green</div>
+ <div>White</div>
+ </div>
+ </section>
+</body>
+</html>
Added: trunk/LayoutTests/fast/css/nth-child-style-sharing-even.html (0 => 173910)
--- trunk/LayoutTests/fast/css/nth-child-style-sharing-even.html (rev 0)
+++ trunk/LayoutTests/fast/css/nth-child-style-sharing-even.html 2014-09-24 07:56:52 UTC (rev 173910)
@@ -0,0 +1,47 @@
+<!doctype html>
+<html>
+<head>
+<style>
+div {
+ background-color: white;
+}
+div:nth-child(even) {
+ background-color: lime;
+}
+</style>
+</head>
+<body>
+ <section>
+ <p>This tests basic style sharing with :nth-child(even). If the test pass, lines with the text "green" should have a green background.</p>
+ <div>Green</div>
+ <div>White</div>
+ <div>Green</div>
+ <div>
+ <div>White</div>
+ <div>Green</div>
+ <div>White</div>
+ <div>Green</div>
+ </div>
+ <div>
+ <div>White</div>
+ <div>Green</div>
+ <div>White</div>
+ <div>Green</div>
+ </div>
+ </section>
+
+ <section>
+ <div>White</div>
+ <div>Green</div>
+ <div>White</div>
+ <div>Green</div>
+ <div>
+ <div>White</div>
+ <div>Green</div>
+ <div>White</div>
+ <div>Green</div>
+ <div>White</div>
+ </div>
+ </section>
+</body>
+</html>
Added: trunk/LayoutTests/fast/css/nth-child-style-sharing-fixed-integer-expected.html (0 => 173910)
--- trunk/LayoutTests/fast/css/nth-child-style-sharing-fixed-integer-expected.html (rev 0)
+++ trunk/LayoutTests/fast/css/nth-child-style-sharing-fixed-integer-expected.html 2014-09-24 07:56:52 UTC (rev 173910)
@@ -0,0 +1,51 @@
+<!doctype html>
+<html>
+<head>
+<style>
+div {
+ background-color: white;
+}
+</style>
+</head>
+<body>
+ <section>
+ <p>This tests basic style sharing with :nth-child(number). If the test pass, lines with the text "green" should have a green background.</p>
+ <div style="background-color:lime">Green</div>
+ <div>White</div>
+ <div>
+ <div>White</div>
+ <div style="background-color:lime">Green</div>
+ <div>White</div>
+ <div>White</div>
+ <div style="background-color:lime">Green</div>
+ </div>
+ <div style="background-color:lime">
+ <div>White</div>
+ <div style="background-color:lime">Green</div>
+ <div>White</div>
+ <div>White</div>
+ <div style="background-color:lime">Green</div>
+ </div>
+ </section>
+
+ <section>
+ <div>White</div>
+ <div style="background-color:lime">Green</div>
+ <div>White</div>
+ <div>
+ <div>White</div>
+ <div style="background-color:lime">Green</div>
+ <div>White</div>
+ <div>White</div>
+ <div style="background-color:lime">Green</div>
+ </div>
+ <div style="background-color:lime">
+ <div>White</div>
+ <div style="background-color:lime">Green</div>
+ <div>White</div>
+ <div>White</div>
+ <div style="background-color:lime">Green</div>
+ </div>
+ </section>
+</body>
+</html>
Added: trunk/LayoutTests/fast/css/nth-child-style-sharing-fixed-integer.html (0 => 173910)
--- trunk/LayoutTests/fast/css/nth-child-style-sharing-fixed-integer.html (rev 0)
+++ trunk/LayoutTests/fast/css/nth-child-style-sharing-fixed-integer.html 2014-09-24 07:56:52 UTC (rev 173910)
@@ -0,0 +1,54 @@
+<!doctype html>
+<html>
+<head>
+<style>
+div {
+ background-color: white;
+}
+div:nth-child(2), div:nth-child(5) {
+ background-color: lime;
+}
+</style>
+</head>
+<body>
+ <section>
+ <p>This tests basic style sharing with :nth-child(number). If the test pass, lines with the text "green" should have a green background.</p>
+ <div>Green</div>
+ <div>White</div>
+ <div>
+ <div>White</div>
+ <div>Green</div>
+ <div>White</div>
+ <div>White</div>
+ <div>Green</div>
+ </div>
+ <div>
+ <div>White</div>
+ <div>Green</div>
+ <div>White</div>
+ <div>White</div>
+ <div>Green</div>
+ </div>
+ </section>
+
+ <section>
+ <div>White</div>
+ <div>Green</div>
+ <div>White</div>
+ <div>
+ <div>White</div>
+ <div>Green</div>
+ <div>White</div>
+ <div>White</div>
+ <div>Green</div>
+ </div>
+ <div>
+ <div>White</div>
+ <div>Green</div>
+ <div>White</div>
+ <div>White</div>
+ <div>Green</div>
+ </div>
+ </section>
+</body>
+</html>
Added: trunk/LayoutTests/fast/css/nth-child-style-sharing-odd-expected.html (0 => 173910)
--- trunk/LayoutTests/fast/css/nth-child-style-sharing-odd-expected.html (rev 0)
+++ trunk/LayoutTests/fast/css/nth-child-style-sharing-odd-expected.html 2014-09-24 07:56:52 UTC (rev 173910)
@@ -0,0 +1,47 @@
+<!doctype html>
+<html>
+<head>
+<style>
+div {
+ background-color: white;
+}
+</style>
+</head>
+<body>
+ <section>
+ <p><!-- To avoid styling the description. --></p>
+ <p>This tests basic style sharing with :nth-child(odd). If the test pass, lines with the text "green" should have a green background.</p>
+ <div style="background-color: lime">Green</div>
+ <div>White</div>
+ <div style="background-color: lime">Green</div>
+ <div>White</div>
+ <div style="background-color: lime">Green</div>
+ <div>
+ <div style="background-color: lime">Green</div>
+ <div>White</div>
+ <div style="background-color: lime">Green</div>
+ </div>
+ <div style="background-color: lime">
+ <div style="background-color: lime">Green</div>
+ <div>White</div>
+ <div style="background-color: lime">Green</div>
+ <div>White</div>
+ <div style="background-color: lime">Green</div>
+ </div>
+ </section>
+
+ <section>
+ <div style="background-color: lime">Green</div>
+ <div>White</div>
+ <div style="background-color: lime">Green</div>
+ <div>
+ <div style="background-color: lime">Green</div>
+ <div>White</div>
+ <div style="background-color: lime">Green</div>
+ <div>White</div>
+ <div style="background-color: lime">Green</div>
+ <div>White</div>
+ </div>
+ </section>
+</body>
+</html>
Added: trunk/LayoutTests/fast/css/nth-child-style-sharing-odd.html (0 => 173910)
--- trunk/LayoutTests/fast/css/nth-child-style-sharing-odd.html (rev 0)
+++ trunk/LayoutTests/fast/css/nth-child-style-sharing-odd.html 2014-09-24 07:56:52 UTC (rev 173910)
@@ -0,0 +1,50 @@
+<!doctype html>
+<html>
+<head>
+<style>
+div {
+ background-color: white;
+}
+div:nth-child(odd) {
+ background-color: lime;
+}
+</style>
+</head>
+<body>
+ <section>
+ <p><!-- To avoid styling the description. --></p>
+ <p>This tests basic style sharing with :nth-child(odd). If the test pass, lines with the text "green" should have a green background.</p>
+ <div>Green</div>
+ <div>White</div>
+ <div>Green</div>
+ <div>White</div>
+ <div>Green</div>
+ <div>
+ <div>Green</div>
+ <div>White</div>
+ <div>Green</div>
+ </div>
+ <div>
+ <div>Green</div>
+ <div>White</div>
+ <div>Green</div>
+ <div>White</div>
+ <div>Green</div>
+ </div>
+ </section>
+
+ <section>
+ <div>Green</div>
+ <div>White</div>
+ <div>Green</div>
+ <div>
+ <div>Green</div>
+ <div>White</div>
+ <div>Green</div>
+ <div>White</div>
+ <div>Green</div>
+ <div>White</div>
+ </div>
+ </section>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (173909 => 173910)
--- trunk/Source/WebCore/ChangeLog 2014-09-24 05:10:18 UTC (rev 173909)
+++ trunk/Source/WebCore/ChangeLog 2014-09-24 07:56:52 UTC (rev 173910)
@@ -1,3 +1,33 @@
+2014-09-24 Benjamin Poulain <[email protected]>
+
+ Remove the style marking from :nth-child()
+ https://bugs.webkit.org/show_bug.cgi?id=137055
+
+ Reviewed by Andreas Kling.
+
+ Previously, :nth-child() had to mark the RenderStyle as unique in order
+ to prevent it from being used for style sharing.
+
+ After r173229, :nth-child() use the more generic element marking
+ "StyleIsAffectedByPreviousSibling".
+
+ In StyleResolver::canShareStyleWithElement(), StyleIsAffectedByPreviousSibling
+ is already used to prevent style sharing of those elements, making the "Unique"
+ flag redundant.
+
+ Since it is now useless, remove the style marking from SelectorChecker and the CSS JIT.
+
+ Tests: fast/css/nth-child-style-sharing-even.html
+ fast/css/nth-child-style-sharing-fixed-integer.html
+ fast/css/nth-child-style-sharing-odd.html
+
+ * css/SelectorChecker.cpp:
+ (WebCore::SelectorChecker::checkOne):
+ * cssjit/SelectorCompiler.cpp:
+ (WebCore::SelectorCompiler::SelectorCodeGenerator::generateElementIsNthChild):
+ (WebCore::SelectorCompiler::setElementChildIndexAndUpdateStyle): Deleted.
+ * rendering/style/RenderStyle.h:
+
2014-09-23 Chris Dumez <[email protected]>
Add support for is<HTML*Element>() for type checking
Modified: trunk/Source/WebCore/css/SelectorChecker.cpp (173909 => 173910)
--- trunk/Source/WebCore/css/SelectorChecker.cpp 2014-09-24 05:10:18 UTC (rev 173909)
+++ trunk/Source/WebCore/css/SelectorChecker.cpp 2014-09-24 07:56:52 UTC (rev 173910)
@@ -695,11 +695,6 @@
element->setChildIndex(count);
}
- if (context.resolvingMode == Mode::ResolvingStyle) {
- if (RenderStyle* childStyle = context.elementStyle ? context.elementStyle : element->renderStyle())
- childStyle->setUnique();
- }
-
if (selector->matchNth(count))
return true;
}
Modified: trunk/Source/WebCore/cssjit/SelectorCompiler.cpp (173909 => 173910)
--- trunk/Source/WebCore/cssjit/SelectorCompiler.cpp 2014-09-24 05:10:18 UTC (rev 173909)
+++ trunk/Source/WebCore/cssjit/SelectorCompiler.cpp 2014-09-24 07:56:52 UTC (rev 173910)
@@ -2910,13 +2910,6 @@
element->setChildIndex(index);
}
-static void setElementChildIndexAndUpdateStyle(Element* element, int index)
-{
- element->setChildIndex(index);
- if (RenderStyle* childStyle = element->renderStyle())
- childStyle->setUnique();
-}
-
void SelectorCodeGenerator::generateElementIsNthChild(Assembler::JumpList& failureCases, const SelectorFragment& fragment)
{
{
@@ -2984,22 +2977,12 @@
LocalRegister checkingContext(m_registerAllocator);
Assembler::Jump notResolvingStyle = jumpIfNotResolvingStyle(checkingContext);
- if (fragmentMatchesTheRightmostElement(m_selectorContext, fragment)) {
- addFlagsToElementStyleFromContext(checkingContext, RenderStyle::NonInheritedFlags::flagIsUnique());
+ Assembler::RegisterID elementAddress = elementAddressRegister;
+ FunctionCall functionCall(m_assembler, m_registerAllocator, m_stackAllocator, m_functionCalls);
+ functionCall.setFunctionAddress(setElementChildIndex);
+ functionCall.setTwoArguments(elementAddress, elementCounter);
+ functionCall.call();
- Assembler::RegisterID elementAddress = elementAddressRegister;
- FunctionCall functionCall(m_assembler, m_registerAllocator, m_stackAllocator, m_functionCalls);
- functionCall.setFunctionAddress(setElementChildIndex);
- functionCall.setTwoArguments(elementAddress, elementCounter);
- functionCall.call();
- } else {
- Assembler::RegisterID elementAddress = elementAddressRegister;
- FunctionCall functionCall(m_assembler, m_registerAllocator, m_stackAllocator, m_functionCalls);
- functionCall.setFunctionAddress(setElementChildIndexAndUpdateStyle);
- functionCall.setTwoArguments(elementAddress, elementCounter);
- functionCall.call();
- }
-
notResolvingStyle.link(&m_assembler);
}
Modified: trunk/Source/WebCore/rendering/style/RenderStyle.h (173909 => 173910)
--- trunk/Source/WebCore/rendering/style/RenderStyle.h 2014-09-24 05:10:18 UTC (rev 173909)
+++ trunk/Source/WebCore/rendering/style/RenderStyle.h 2014-09-24 07:56:52 UTC (rev 173910)
@@ -277,7 +277,6 @@
static ETableLayout initialTableLayout() { return TAUTO; }
static ptrdiff_t flagsMemoryOffset() { return OBJECT_OFFSETOF(NonInheritedFlags, m_flags); }
- static uint64_t flagIsUnique() { return oneBitMask << isUniqueOffset; }
static uint64_t flagIsaffectedByActive() { return oneBitMask << affectedByActiveOffset; }
static uint64_t flagIsaffectedByHover() { return oneBitMask << affectedByHoverOffset; }
static uint64_t flagPseudoStyle(PseudoId pseudo) { return oneBitMask << (pseudoBitsOffset - 1 + pseudo); }
@@ -309,6 +308,7 @@
return static_cast<unsigned>((m_flags >> offset) & positionIndependentMask);
}
+ static uint64_t flagIsUnique() { return oneBitMask << isUniqueOffset; }
static uint64_t flagFirstChildState() { return oneBitMask << firstChildStateOffset; }
static uint64_t flagLastChildState() { return oneBitMask << lastChildStateOffset; }