Title: [173910] trunk
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; }
 
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to