Don't invalidate descendants for sibling combinators unless needed
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 7 Mar 2018 20:29:17 +0000 (20:29 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 7 Mar 2018 20:29:17 +0000 (20:29 +0000)
https://bugs.webkit.org/show_bug.cgi?id=183410
<rdar://problem/38227297>

Reviewed by Zalan Bujtas.

If we know the matched sibling combinator doesn't affect descendants we shouldn't invalidate them.

* css/SelectorChecker.cpp:
(WebCore::SelectorChecker::matchRecursively const):

    Use different bit for the descendant case.

* cssjit/SelectorCompiler.cpp:
(WebCore::SelectorCompiler::fragmentMatchesTheRightmostElement):

    Remove unneeded context assert.

(WebCore::SelectorCompiler::SelectorCodeGenerator::generateSelectorChecker):

    Use different bit for the descendant case.

(WebCore::SelectorCompiler::SelectorCodeGenerator::generateSelectorCheckerExcludingPseudoElements):
(WebCore::SelectorCompiler::SelectorCodeGenerator::generateElementHasPseudoElement):
(WebCore::SelectorCompiler::SelectorCodeGenerator::generateRequestedPseudoElementEqualsToSelectorPseudoElement):
* dom/Element.cpp:
(WebCore::invalidateForSiblingCombinators):

    Invalidate the target sibling or all descendants based on the bits.

* dom/Element.h:
(WebCore::Element::descendantsAffectedByPreviousSibling const):
(WebCore::Element::setDescendantsAffectedByPreviousSibling const):
* dom/Node.h:
* style/StyleRelations.cpp:
(WebCore::Style::commitRelationsToRenderStyle):
(WebCore::Style::commitRelations):
* style/StyleRelations.h:

    Add DescendantsAffectedByPreviousSibling bit. AffectedByPreviousSibling is now just about the target element.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@229372 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/css/SelectorChecker.cpp
Source/WebCore/cssjit/SelectorCompiler.cpp
Source/WebCore/dom/Element.cpp
Source/WebCore/dom/Element.h
Source/WebCore/dom/Node.h
Source/WebCore/style/StyleRelations.cpp
Source/WebCore/style/StyleRelations.h

index 6844096..5a1d8cf 100644 (file)
@@ -1,5 +1,48 @@
 2018-03-07  Antti Koivisto  <antti@apple.com>
 
+        Don't invalidate descendants for sibling combinators unless needed
+        https://bugs.webkit.org/show_bug.cgi?id=183410
+        <rdar://problem/38227297>
+
+        Reviewed by Zalan Bujtas.
+
+        If we know the matched sibling combinator doesn't affect descendants we shouldn't invalidate them.
+
+        * css/SelectorChecker.cpp:
+        (WebCore::SelectorChecker::matchRecursively const):
+
+            Use different bit for the descendant case.
+
+        * cssjit/SelectorCompiler.cpp:
+        (WebCore::SelectorCompiler::fragmentMatchesTheRightmostElement):
+
+            Remove unneeded context assert.
+
+        (WebCore::SelectorCompiler::SelectorCodeGenerator::generateSelectorChecker):
+
+            Use different bit for the descendant case.
+
+        (WebCore::SelectorCompiler::SelectorCodeGenerator::generateSelectorCheckerExcludingPseudoElements):
+        (WebCore::SelectorCompiler::SelectorCodeGenerator::generateElementHasPseudoElement):
+        (WebCore::SelectorCompiler::SelectorCodeGenerator::generateRequestedPseudoElementEqualsToSelectorPseudoElement):
+        * dom/Element.cpp:
+        (WebCore::invalidateForSiblingCombinators):
+
+            Invalidate the target sibling or all descendants based on the bits.
+
+        * dom/Element.h:
+        (WebCore::Element::descendantsAffectedByPreviousSibling const):
+        (WebCore::Element::setDescendantsAffectedByPreviousSibling const):
+        * dom/Node.h:
+        * style/StyleRelations.cpp:
+        (WebCore::Style::commitRelationsToRenderStyle):
+        (WebCore::Style::commitRelations):
+        * style/StyleRelations.h:
+
+            Add DescendantsAffectedByPreviousSibling bit. AffectedByPreviousSibling is now just about the target element.
+
+2018-03-07  Antti Koivisto  <antti@apple.com>
+
         checkForSiblingStyleChanges should use internal versions of the invalidation functions
         https://bugs.webkit.org/show_bug.cgi?id=183405
         <rdar://problem/38218310>
index cd00744..ccf9be7 100644 (file)
@@ -362,7 +362,8 @@ SelectorChecker::MatchResult SelectorChecker::matchRecursively(CheckingContext&
 
     case CSSSelector::DirectAdjacent:
         {
-            addStyleRelation(checkingContext, *context.element, Style::Relation::AffectedByPreviousSibling);
+            auto relation = context.isMatchElement ? Style::Relation::AffectedByPreviousSibling : Style::Relation::DescendantsAffectedByPreviousSibling;
+            addStyleRelation(checkingContext, *context.element, relation);
 
             Element* previousElement = context.element->previousElementSibling();
             if (!previousElement)
@@ -382,8 +383,9 @@ SelectorChecker::MatchResult SelectorChecker::matchRecursively(CheckingContext&
 
             return MatchResult::updateWithMatchType(result, matchType);
         }
-    case CSSSelector::IndirectAdjacent:
-        addStyleRelation(checkingContext, *context.element, Style::Relation::AffectedByPreviousSibling);
+    case CSSSelector::IndirectAdjacent: {
+        auto relation = context.isMatchElement ? Style::Relation::AffectedByPreviousSibling : Style::Relation::DescendantsAffectedByPreviousSibling;
+        addStyleRelation(checkingContext, *context.element, relation);
 
         nextContext.element = context.element->previousElementSibling();
         nextContext.firstSelectorOfTheFragment = nextContext.selector;
@@ -402,7 +404,7 @@ SelectorChecker::MatchResult SelectorChecker::matchRecursively(CheckingContext&
                 return MatchResult::updateWithMatchType(result, matchType);
         };
         return MatchResult::fails(Match::SelectorFailsAllSiblings);
-
+    }
     case CSSSelector::Subselector:
         {
             // a selector is invalid if something follows a pseudo-element
index 1f3bc33..a79b267 100644 (file)
@@ -422,11 +422,8 @@ static inline FunctionType mostRestrictiveFunctionType(FunctionType a, FunctionT
     return std::max(a, b);
 }
 
-static inline bool fragmentMatchesTheRightmostElement(SelectorContext selectorContext, const SelectorFragment& fragment)
+static inline bool fragmentMatchesTheRightmostElement(const SelectorFragment& fragment)
 {
-    // Return true if the position of this fragment is Rightmost in the root fragments.
-    // In this case, we should use the RenderStyle stored in the CheckingContext.
-    ASSERT_UNUSED(selectorContext, selectorContext != SelectorContext::QuerySelector);
     return fragment.relationToRightFragment == FragmentRelation::Rightmost && fragment.positionInRootFragments == FragmentPositionInRootFragments::Rightmost;
 }
 
@@ -1734,7 +1731,7 @@ void SelectorCodeGenerator::generateSelectorChecker()
     Assembler::JumpList failureOnFunctionEntry;
     // Test selector's pseudo element equals to requested PseudoId.
     if (m_selectorContext != SelectorContext::QuerySelector && m_functionType == FunctionType::SelectorCheckerWithCheckingContext) {
-        ASSERT_WITH_MESSAGE(fragmentMatchesTheRightmostElement(m_selectorContext, m_selectorFragments.first()), "Matching pseudo elements only make sense for the rightmost fragment.");
+        ASSERT_WITH_MESSAGE(fragmentMatchesTheRightmostElement(m_selectorFragments.first()), "Matching pseudo elements only make sense for the rightmost fragment.");
         generateRequestedPseudoElementEqualsToSelectorPseudoElement(failureOnFunctionEntry, m_selectorFragments.first(), checkingContextRegister);
     }
 
@@ -1895,8 +1892,12 @@ void SelectorCodeGenerator::generateSelectorCheckerExcludingPseudoElements(Assem
             generateIndirectAdjacentTreeWalker(failureCases, fragment);
             break;
         }
-        if (shouldMarkStyleIsAffectedByPreviousSibling(fragment))
-            generateAddStyleRelationIfResolvingStyle(elementAddressRegister, Style::Relation::AffectedByPreviousSibling);
+        if (shouldMarkStyleIsAffectedByPreviousSibling(fragment)) {
+            if (fragmentMatchesTheRightmostElement(fragment))
+                generateAddStyleRelationIfResolvingStyle(elementAddressRegister, Style::Relation::AffectedByPreviousSibling);
+            else
+                generateAddStyleRelationIfResolvingStyle(elementAddressRegister, Style::Relation::DescendantsAffectedByPreviousSibling);
+        }
         generateBacktrackingTailsIfNeeded(failureCases, fragment);
     }
 
@@ -3731,7 +3732,7 @@ void SelectorCodeGenerator::generateElementHasPseudoElement(Assembler::JumpList&
 {
     ASSERT_UNUSED(fragment, fragment.pseudoElementSelector);
     ASSERT_WITH_MESSAGE(m_selectorContext != SelectorContext::QuerySelector, "When the fragment has pseudo element, the selector becomes CannotMatchAnything for QuerySelector and this test function is not called.");
-    ASSERT_WITH_MESSAGE_UNUSED(fragment, fragmentMatchesTheRightmostElement(m_selectorContext, fragment), "Virtual pseudo elements handling is only effective in the rightmost fragment. If the current fragment is not rightmost fragment, CSS JIT compiler makes it CannotMatchAnything in fragment construction phase, so never reach here.");
+    ASSERT_WITH_MESSAGE_UNUSED(fragment, fragmentMatchesTheRightmostElement(fragment), "Virtual pseudo elements handling is only effective in the rightmost fragment. If the current fragment is not rightmost fragment, CSS JIT compiler makes it CannotMatchAnything in fragment construction phase, so never reach here.");
 }
 
 void SelectorCodeGenerator::generateRequestedPseudoElementEqualsToSelectorPseudoElement(Assembler::JumpList& failureCases, const SelectorFragment& fragment, Assembler::RegisterID checkingContext)
@@ -3741,7 +3742,7 @@ void SelectorCodeGenerator::generateRequestedPseudoElementEqualsToSelectorPseudo
     // Make sure that the requested pseudoId equals to the pseudo element of the rightmost fragment.
     // If the rightmost fragment doesn't have a pseudo element, the requested pseudoId need to be NOPSEUDO to succeed the matching.
     // Otherwise, if the requested pseudoId is not NOPSEUDO, the requested pseudoId need to equal to the pseudo element of the rightmost fragment.
-    if (fragmentMatchesTheRightmostElement(m_selectorContext, fragment)) {
+    if (fragmentMatchesTheRightmostElement(fragment)) {
         if (!fragment.pseudoElementSelector)
             failureCases.append(m_assembler.branch8(Assembler::NotEqual, Assembler::Address(checkingContext, OBJECT_OFFSETOF(SelectorChecker::CheckingContext, pseudoId)), Assembler::TrustedImm32(NOPSEUDO)));
         else {
index 7100bda..5e34aae 100644 (file)
@@ -1484,7 +1484,11 @@ static void invalidateForSiblingCombinators(Element* sibling)
 {
     for (; sibling; sibling = sibling->nextElementSibling()) {
         if (sibling->styleIsAffectedByPreviousSibling())
-            sibling->invalidateStyleForSubtreeInternal();
+            sibling->invalidateStyleInternal();
+        if (sibling->descendantsAffectedByPreviousSibling()) {
+            for (auto* siblingChild = sibling->firstElementChild(); siblingChild; siblingChild = siblingChild->nextElementSibling())
+                siblingChild->invalidateStyleForSubtreeInternal();
+        }
         if (!sibling->affectsNextSiblingElementStyle())
             return;
     }
index 4513192..38f2487 100644 (file)
@@ -327,6 +327,7 @@ public:
     bool styleAffectedByActive() const { return hasRareData() && rareDataStyleAffectedByActive(); }
     bool styleAffectedByEmpty() const { return hasRareData() && rareDataStyleAffectedByEmpty(); }
     bool styleAffectedByFocusWithin() const { return hasRareData() && rareDataStyleAffectedByFocusWithin(); }
+    bool descendantsAffectedByPreviousSibling() const { return getFlag(DescendantsAffectedByPreviousSiblingFlag); }
     bool childrenAffectedByHover() const { return getFlag(ChildrenAffectedByHoverRulesFlag); }
     bool childrenAffectedByDrag() const { return hasRareData() && rareDataChildrenAffectedByDrag(); }
     bool childrenAffectedByFirstChildRules() const { return getFlag(ChildrenAffectedByFirstChildRulesFlag); }
@@ -341,6 +342,7 @@ public:
 
     void setStyleAffectedByEmpty();
     void setStyleAffectedByFocusWithin();
+    void setDescendantsAffectedByPreviousSibling() const { return setFlag(DescendantsAffectedByPreviousSiblingFlag); }
     void setChildrenAffectedByHover() { setFlag(ChildrenAffectedByHoverRulesFlag); }
     void setStyleAffectedByActive();
     void setChildrenAffectedByDrag();
index eb857fe..acee486 100644 (file)
@@ -569,7 +569,7 @@ protected:
         IsStyledElementFlag = 1 << 3,
         IsHTMLFlag = 1 << 4,
         IsSVGFlag = 1 << 5,
-        // One free bit left.
+        DescendantsAffectedByPreviousSiblingFlag = 1 << 6,
         ChildNeedsStyleRecalcFlag = 1 << 7,
         IsConnectedFlag = 1 << 8,
         IsLinkFlag = 1 << 9,
index 185b36f..5e9fa76 100644 (file)
@@ -75,6 +75,7 @@ std::unique_ptr<Relations> commitRelationsToRenderStyle(RenderStyle& style, cons
             break;
         case Relation::AffectedByFocusWithin:
         case Relation::AffectedByPreviousSibling:
+        case Relation::DescendantsAffectedByPreviousSibling:
         case Relation::AffectsNextSibling:
         case Relation::ChildrenAffectedByForwardPositionalRules:
         case Relation::ChildrenAffectedByBackwardPositionalRules:
@@ -114,6 +115,9 @@ void commitRelations(std::unique_ptr<Relations> relations, Update& update)
         case Relation::AffectedByPreviousSibling:
             element.setStyleIsAffectedByPreviousSibling();
             break;
+        case Relation::DescendantsAffectedByPreviousSibling:
+            element.setDescendantsAffectedByPreviousSibling();
+            break;
         case Relation::AffectsNextSibling: {
             auto* sibling = &element;
             for (unsigned i = 0; i < relation.value && sibling; ++i, sibling = sibling->nextElementSibling())
index 9587e27..fb1b5dc 100644 (file)
@@ -44,6 +44,7 @@ struct Relation {
         AffectedByFocusWithin,
         AffectedByHover,
         AffectedByPreviousSibling,
+        DescendantsAffectedByPreviousSibling,
         // For AffectsNextSibling 'value' tells how many element siblings to mark starting with 'element'.
         AffectsNextSibling,
         ChildrenAffectedByForwardPositionalRules,