Title: [229372] trunk/Source/WebCore
Revision
229372
Author
an...@apple.com
Date
2018-03-07 12:29:17 -0800 (Wed, 07 Mar 2018)

Log Message

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.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (229371 => 229372)


--- trunk/Source/WebCore/ChangeLog	2018-03-07 18:53:21 UTC (rev 229371)
+++ trunk/Source/WebCore/ChangeLog	2018-03-07 20:29:17 UTC (rev 229372)
@@ -1,5 +1,48 @@
 2018-03-07  Antti Koivisto  <an...@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  <an...@apple.com>
+
         checkForSiblingStyleChanges should use internal versions of the invalidation functions
         https://bugs.webkit.org/show_bug.cgi?id=183405
         <rdar://problem/38218310>

Modified: trunk/Source/WebCore/css/SelectorChecker.cpp (229371 => 229372)


--- trunk/Source/WebCore/css/SelectorChecker.cpp	2018-03-07 18:53:21 UTC (rev 229371)
+++ trunk/Source/WebCore/css/SelectorChecker.cpp	2018-03-07 20:29:17 UTC (rev 229372)
@@ -362,7 +362,8 @@
 
     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 @@
 
             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 @@
                 return MatchResult::updateWithMatchType(result, matchType);
         };
         return MatchResult::fails(Match::SelectorFailsAllSiblings);
-
+    }
     case CSSSelector::Subselector:
         {
             // a selector is invalid if something follows a pseudo-element

Modified: trunk/Source/WebCore/cssjit/SelectorCompiler.cpp (229371 => 229372)


--- trunk/Source/WebCore/cssjit/SelectorCompiler.cpp	2018-03-07 18:53:21 UTC (rev 229371)
+++ trunk/Source/WebCore/cssjit/SelectorCompiler.cpp	2018-03-07 20:29:17 UTC (rev 229372)
@@ -422,11 +422,8 @@
     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 @@
     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 @@
             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 @@
 {
     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 @@
     // 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 {

Modified: trunk/Source/WebCore/dom/Element.cpp (229371 => 229372)


--- trunk/Source/WebCore/dom/Element.cpp	2018-03-07 18:53:21 UTC (rev 229371)
+++ trunk/Source/WebCore/dom/Element.cpp	2018-03-07 20:29:17 UTC (rev 229372)
@@ -1484,7 +1484,11 @@
 {
     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;
     }

Modified: trunk/Source/WebCore/dom/Element.h (229371 => 229372)


--- trunk/Source/WebCore/dom/Element.h	2018-03-07 18:53:21 UTC (rev 229371)
+++ trunk/Source/WebCore/dom/Element.h	2018-03-07 20:29:17 UTC (rev 229372)
@@ -327,6 +327,7 @@
     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 @@
 
     void setStyleAffectedByEmpty();
     void setStyleAffectedByFocusWithin();
+    void setDescendantsAffectedByPreviousSibling() const { return setFlag(DescendantsAffectedByPreviousSiblingFlag); }
     void setChildrenAffectedByHover() { setFlag(ChildrenAffectedByHoverRulesFlag); }
     void setStyleAffectedByActive();
     void setChildrenAffectedByDrag();

Modified: trunk/Source/WebCore/dom/Node.h (229371 => 229372)


--- trunk/Source/WebCore/dom/Node.h	2018-03-07 18:53:21 UTC (rev 229371)
+++ trunk/Source/WebCore/dom/Node.h	2018-03-07 20:29:17 UTC (rev 229372)
@@ -569,7 +569,7 @@
         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,

Modified: trunk/Source/WebCore/style/StyleRelations.cpp (229371 => 229372)


--- trunk/Source/WebCore/style/StyleRelations.cpp	2018-03-07 18:53:21 UTC (rev 229371)
+++ trunk/Source/WebCore/style/StyleRelations.cpp	2018-03-07 20:29:17 UTC (rev 229372)
@@ -75,6 +75,7 @@
             break;
         case Relation::AffectedByFocusWithin:
         case Relation::AffectedByPreviousSibling:
+        case Relation::DescendantsAffectedByPreviousSibling:
         case Relation::AffectsNextSibling:
         case Relation::ChildrenAffectedByForwardPositionalRules:
         case Relation::ChildrenAffectedByBackwardPositionalRules:
@@ -114,6 +115,9 @@
         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())

Modified: trunk/Source/WebCore/style/StyleRelations.h (229371 => 229372)


--- trunk/Source/WebCore/style/StyleRelations.h	2018-03-07 18:53:21 UTC (rev 229371)
+++ trunk/Source/WebCore/style/StyleRelations.h	2018-03-07 20:29:17 UTC (rev 229372)
@@ -44,6 +44,7 @@
         AffectedByFocusWithin,
         AffectedByHover,
         AffectedByPreviousSibling,
+        DescendantsAffectedByPreviousSibling,
         // For AffectsNextSibling 'value' tells how many element siblings to mark starting with 'element'.
         AffectsNextSibling,
         ChildrenAffectedByForwardPositionalRules, 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to