Changeset 167585 in webkit

Timestamp:

Apr 20, 2014, 11:26:20 PM (12 years ago)

Author:

benjamin@webkit.org

Message:

Compile the :root pseudo class and fix a related issue with :nth-child()
​https://bugs.webkit.org/show_bug.cgi?id=131926

Reviewed by Andreas Kling.

Source/WebCore:
Add the :root pseudo class. This is another trivial selector, we just need to compare
the element pointer with the documentElement.

I discovered some issues with :nth-child(n) through the layout tests for ":root".
When the pseudo class nth-child could match anything, no code was generated. That decision
was taken when generating the fragments.

The specification of :nth-child() has two tests: the parent test and the counter test.
Since some fragments would not generate any code for :nth-child(n), they would succeed on the root,
which is incorrect since the root should fail the parent test.

This was fixed by moving the filtering of non-counting :nth-child() after we generate the parent
check.
We still don't generate any counter test unless required.

Test: fast/selectors/nth-child-on-root.html

• cssjit/SelectorCompiler.cpp:

(WebCore::SelectorCompiler::addPseudoClassType):
(WebCore::SelectorCompiler::SelectorCodeGenerator::generateElementMatching):
(WebCore::SelectorCompiler::SelectorCodeGenerator::generateElementIsNthChild):
(WebCore::SelectorCompiler::SelectorCodeGenerator::generateElementIsRoot):

• dom/Document.h:

(WebCore::Document::documentElementMemoryOffset):

LayoutTests:
Add more test coverage that would have caught the bug with :nth-child(n).

• fast/selectors/nth-child-on-root-expected.txt: Added.
• fast/selectors/nth-child-on-root.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/selectors/nth-child-on-root-expected.txt (added)
• LayoutTests/fast/selectors/nth-child-on-root.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/cssjit/SelectorCompiler.cpp (modified) (7 diffs)
• Source/WebCore/dom/Document.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2014-04-20  Benjamin Poulain  <benjamin@webkit.org>
+
+        Compile the :root pseudo class and fix a related issue with :nth-child()
+        https://bugs.webkit.org/show_bug.cgi?id=131926
+
+        Reviewed by Andreas Kling.
+
+        Add more test coverage that would have caught the bug with :nth-child(n).
+
+        * fast/selectors/nth-child-on-root-expected.txt: Added.
+        * fast/selectors/nth-child-on-root.html: Added.
+
 2014-04-20  Benjamin Poulain  <benjamin@webkit.org>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2014-04-20  Benjamin Poulain  <benjamin@webkit.org>
+
+        Compile the :root pseudo class and fix a related issue with :nth-child()
+        https://bugs.webkit.org/show_bug.cgi?id=131926
+
+        Reviewed by Andreas Kling.
+
+        Add the :root pseudo class. This is another trivial selector, we just need to compare
+        the element pointer with the documentElement.
+
+        I discovered some issues with :nth-child(n) through the layout tests for ":root".
+        When the pseudo class nth-child could match anything, no code was generated. That decision
+        was taken when generating the fragments.
+
+        The specification of :nth-child() has two tests: the parent test and the counter test.
+        Since some fragments would not generate any code for :nth-child(n), they would succeed on the root,
+        which is incorrect since the root should fail the parent test.
+
+        This was fixed by moving the filtering of non-counting :nth-child() after we generate the parent
+        check.
+        We still don't generate any counter test unless required.
+
+        Test: fast/selectors/nth-child-on-root.html
+
+        * cssjit/SelectorCompiler.cpp:
+        (WebCore::SelectorCompiler::addPseudoClassType):
+        (WebCore::SelectorCompiler::SelectorCodeGenerator::generateElementMatching):
+        (WebCore::SelectorCompiler::SelectorCodeGenerator::generateElementIsNthChild):
+        (WebCore::SelectorCompiler::SelectorCodeGenerator::generateElementIsRoot):
+        * dom/Document.h:
+        (WebCore::Document::documentElementMemoryOffset):
+
 2014-04-20  Benjamin Poulain  <benjamin@webkit.org>
 

trunk/Source/WebCore/cssjit/SelectorCompiler.cpp

@@ -191 +191 @@
     void generateElementIsLink(Assembler::JumpList& failureCases);
     void generateElementIsNthChild(Assembler::JumpList& failureCases, const SelectorFragment&);
+    void generateElementIsRoot(Assembler::JumpList& failureCases);
     void generateElementIsTarget(Assembler::JumpList& failureCases);
 
@@ -320 +321 @@
 
     case CSSSelector::PseudoClassLink:
+    case CSSSelector::PseudoClassRoot:
     case CSSSelector::PseudoClassTarget:
         fragment.pseudoClasses.add(type);
@@ -343 +345 @@
             if (a <= 0 && b < 1)
                 return FunctionType::CannotMatchAnything;
-
-            // Anything modulo 1 is zero. Unless b restrict the range, this does not filter anything out.
-            if (a == 1 && (!b || (b == 1)))
-                return FunctionType::SimpleSelectorChecker;
 
             fragment.nthChildfilters.append(std::pair<int, int>(a, b));
@@ -1169 +1167 @@
         generateElementIsLink(failureCases);
 
+    if (fragment.pseudoClasses.contains(CSSSelector::PseudoClassRoot))
+        generateElementIsRoot(failureCases);
+
     if (fragment.pseudoClasses.contains(CSSSelector::PseudoClassTarget))
         generateElementIsTarget(failureCases);
@@ -1943 +1944 @@
     generateWalkToParentElement(failureCases, parentElement);
 
+    Vector<std::pair<int, int>> validSubsetFilters;
+    validSubsetFilters.reserveInitialCapacity(fragment.nthChildfilters.size());
+    for (const auto& slot : fragment.nthChildfilters) {
+        int a = slot.first;
+        int b = slot.second;
+
+        // Anything modulo 1 is zero. Unless b restricts the range, this does not filter anything out.
+        if (a == 1 && (!b || (b == 1)))
+            continue;
+        validSubsetFilters.uncheckedAppend(slot);
+    }
+    if (validSubsetFilters.isEmpty())
+        return;
+
     // Setup the counter at 1.
     LocalRegister elementCounter(m_registerAllocator);
@@ -2019 +2034 @@
 
     // Test every the nth-child filter.
-    for (const auto& slot : fragment.nthChildfilters) {
+    for (const auto& slot : validSubsetFilters) {
         int a = slot.first;
         int b = slot.second;
@@ -2044 +2059 @@
 }
 
+void SelectorCodeGenerator::generateElementIsRoot(Assembler::JumpList& failureCases)
+{
+    LocalRegister document(m_registerAllocator);
+    getDocument(m_assembler, elementAddressRegister, document);
+    failureCases.append(m_assembler.branchPtr(Assembler::NotEqual, Assembler::Address(document, Document::documentElementMemoryOffset()), elementAddressRegister));
+}
+
 void SelectorCodeGenerator::generateElementIsTarget(Assembler::JumpList& failureCases)
 {

trunk/Source/WebCore/dom/Document.h

@@ -407 +407 @@
         return m_documentElement.get();
     }
+    static ptrdiff_t documentElementMemoryOffset() { return OBJECT_OFFSETOF(Document, m_documentElement); }
 
     Element* activeElement();