Changeset 229372 in webkit

Timestamp:

Mar 7, 2018 12:29:17 PM (6 years ago)

Author:

Antti Koivisto

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.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• css/SelectorChecker.cpp (modified) (3 diffs)
• cssjit/SelectorCompiler.cpp (modified) (5 diffs)
• dom/Element.cpp (modified) (1 diff)
• dom/Element.h (modified) (2 diffs)
• dom/Node.h (modified) (1 diff)
• style/StyleRelations.cpp (modified) (2 diffs)
• style/StyleRelations.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/css/SelectorChecker.cpp

@@ -363 +363 @@
     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();
@@ -383 +384 @@
             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();
@@ -403 +405 @@
         };
         return MatchResult::fails(Match::SelectorFailsAllSiblings);
-
+    }
     case CSSSelector::Subselector:
         {

trunk/Source/WebCore/cssjit/SelectorCompiler.cpp

@@ -423 +423 @@
 }
 
-static inline bool fragmentMatchesTheRightmostElement(SelectorContext selectorContext, 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);
+static inline bool fragmentMatchesTheRightmostElement(const SelectorFragment& fragment)
+{
     return fragment.relationToRightFragment == FragmentRelation::Rightmost && fragment.positionInRootFragments == FragmentPositionInRootFragments::Rightmost;
 }
@@ -1735 +1732 @@
     // 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);
     }
@@ -1896 +1893 @@
             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);
     }
@@ -3732 +3733 @@
     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.");
 }
 
@@ -3742 +3743 @@
     // 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)));

trunk/Source/WebCore/dom/Element.cpp

@@ -1485 +1485 @@
     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;

trunk/Source/WebCore/dom/Element.h

@@ -328 +328 @@
     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(); }
@@ -342 +343 @@
     void setStyleAffectedByEmpty();
     void setStyleAffectedByFocusWithin();
+    void setDescendantsAffectedByPreviousSibling() const { return setFlag(DescendantsAffectedByPreviousSiblingFlag); }
     void setChildrenAffectedByHover() { setFlag(ChildrenAffectedByHoverRulesFlag); }
     void setStyleAffectedByActive();

trunk/Source/WebCore/dom/Node.h

@@ -570 +570 @@
         IsHTMLFlag = 1 << 4,
         IsSVGFlag = 1 << 5,
-        // One free bit left.
+        DescendantsAffectedByPreviousSiblingFlag = 1 << 6,
         ChildNeedsStyleRecalcFlag = 1 << 7,
         IsConnectedFlag = 1 << 8,

trunk/Source/WebCore/style/StyleRelations.cpp

@@ -76 +76 @@
         case Relation::AffectedByFocusWithin:
         case Relation::AffectedByPreviousSibling:
+        case Relation::DescendantsAffectedByPreviousSibling:
         case Relation::AffectsNextSibling:
         case Relation::ChildrenAffectedByForwardPositionalRules:
@@ -114 +115 @@
         case Relation::AffectedByPreviousSibling:
             element.setStyleIsAffectedByPreviousSibling();
+            break;
+        case Relation::DescendantsAffectedByPreviousSibling:
+            element.setDescendantsAffectedByPreviousSibling();
             break;
         case Relation::AffectsNextSibling: {

trunk/Source/WebCore/style/StyleRelations.h

@@ -45 +45 @@
         AffectedByHover,
         AffectedByPreviousSibling,
+        DescendantsAffectedByPreviousSibling,
         // For AffectsNextSibling 'value' tells how many element siblings to mark starting with 'element'.
         AffectsNextSibling,