checkForSiblingStyleChanges should use internal versions of the invalidation functions
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 7 Mar 2018 18:27:55 +0000 (18:27 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 7 Mar 2018 18:27:55 +0000 (18:27 +0000)
https://bugs.webkit.org/show_bug.cgi?id=183405
<rdar://problem/38218310>

Reviewed by Zalan Bujtas.

Non-internal invalidateStyleForElement/Subtree() implement sibling combinator invalidation. Checking this
is only needed if the element in question changed somehow. In checkForSiblingStyleChanges we know that
another element changed and we really just want to invalidate.

* css/SelectorChecker.cpp:
(WebCore::isFirstOfType):
(WebCore::SelectorChecker::checkOne const):

Also make :first-of-type use ChildrenAffectedByForwardPositionalRules for invalidation similar to :last-of-type
for more correct invalidation.

* dom/Element.cpp:
(WebCore::checkForSiblingStyleChanges):

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

Source/WebCore/ChangeLog
Source/WebCore/css/SelectorChecker.cpp
Source/WebCore/dom/Element.cpp

index b756c4d..6844096 100644 (file)
@@ -1,3 +1,25 @@
+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>
+
+        Reviewed by Zalan Bujtas.
+
+        Non-internal invalidateStyleForElement/Subtree() implement sibling combinator invalidation. Checking this
+        is only needed if the element in question changed somehow. In checkForSiblingStyleChanges we know that
+        another element changed and we really just want to invalidate.
+
+        * css/SelectorChecker.cpp:
+        (WebCore::isFirstOfType):
+        (WebCore::SelectorChecker::checkOne const):
+
+        Also make :first-of-type use ChildrenAffectedByForwardPositionalRules for invalidation similar to :last-of-type
+        for more correct invalidation.
+
+        * dom/Element.cpp:
+        (WebCore::checkForSiblingStyleChanges):
+
 2018-03-07  Yusuke Suzuki  <utatane.tea@gmail.com>
 
         HTML `pattern` attribute should set `u` flag for regular expressions
index 44b328b..cd00744 100644 (file)
@@ -103,11 +103,9 @@ static inline bool isLastChildElement(const Element& element)
     return !ElementTraversal::nextSibling(element);
 }
 
-static inline bool isFirstOfType(SelectorChecker::CheckingContext& checkingContext, const Element& element, const QualifiedName& type)
+static inline bool isFirstOfType(const Element& element, const QualifiedName& type)
 {
     for (const Element* sibling = ElementTraversal::previousSibling(element); sibling; sibling = ElementTraversal::previousSibling(*sibling)) {
-        addStyleRelation(checkingContext, *sibling, Style::Relation::AffectsNextSibling);
-
         if (sibling->hasTagName(type))
             return false;
     }
@@ -745,9 +743,9 @@ bool SelectorChecker::checkOne(CheckingContext& checkingContext, const LocalCont
             break;
         case CSSSelector::PseudoClassFirstOfType:
             // first-of-type matches the first element of its type
-            if (element.parentElement()) {
-                addStyleRelation(checkingContext, element, Style::Relation::AffectedByPreviousSibling);
-                return isFirstOfType(checkingContext, element, element.tagQName());
+            if (auto* parentElement = element.parentElement()) {
+                addStyleRelation(checkingContext, *parentElement, Style::Relation::ChildrenAffectedByForwardPositionalRules);
+                return isFirstOfType(element, element.tagQName());
             }
             break;
         case CSSSelector::PseudoClassLastChild:
@@ -785,11 +783,11 @@ bool SelectorChecker::checkOne(CheckingContext& checkingContext, const LocalCont
         case CSSSelector::PseudoClassOnlyOfType:
             // FIXME: This selector is very slow.
             if (Element* parentElement = element.parentElement()) {
-                addStyleRelation(checkingContext, element, Style::Relation::AffectedByPreviousSibling);
+                addStyleRelation(checkingContext, *parentElement, Style::Relation::ChildrenAffectedByForwardPositionalRules);
                 addStyleRelation(checkingContext, *parentElement, Style::Relation::ChildrenAffectedByBackwardPositionalRules);
                 if (!parentElement->isFinishedParsingChildren())
                     return false;
-                return isFirstOfType(checkingContext, element, element.tagQName()) && isLastOfType(element, element.tagQName());
+                return isFirstOfType(element, element.tagQName()) && isLastOfType(element, element.tagQName());
             }
             break;
         case CSSSelector::PseudoClassMatches:
index ef34040..7100bda 100644 (file)
@@ -1480,6 +1480,16 @@ ElementStyle Element::resolveStyle(const RenderStyle* parentStyle)
     return styleResolver().styleForElement(*this, parentStyle);
 }
 
+static void invalidateForSiblingCombinators(Element* sibling)
+{
+    for (; sibling; sibling = sibling->nextElementSibling()) {
+        if (sibling->styleIsAffectedByPreviousSibling())
+            sibling->invalidateStyleForSubtreeInternal();
+        if (!sibling->affectsNextSiblingElementStyle())
+            return;
+    }
+}
+
 static void invalidateSiblingsIfNeeded(Element& element)
 {
     if (!element.affectsNextSiblingElementStyle())
@@ -1488,12 +1498,7 @@ static void invalidateSiblingsIfNeeded(Element& element)
     if (parent && parent->styleValidity() >= Style::Validity::SubtreeInvalid)
         return;
 
-    for (auto* sibling = element.nextElementSibling(); sibling; sibling = sibling->nextElementSibling()) {
-        if (sibling->styleIsAffectedByPreviousSibling())
-            sibling->invalidateStyleForSubtreeInternal();
-        if (!sibling->affectsNextSiblingElementStyle())
-            return;
-    }
+    invalidateForSiblingCombinators(element.nextElementSibling());
 }
 
 void Element::invalidateStyle()
@@ -2027,14 +2032,14 @@ static void checkForSiblingStyleChanges(Element& parent, SiblingCheckType checkT
         if (newFirstElement != elementAfterChange) {
             auto* style = elementAfterChange->renderStyle();
             if (!style || style->firstChildState())
-                elementAfterChange->invalidateStyleForSubtree();
+                elementAfterChange->invalidateStyleForSubtreeInternal();
         }
 
         // We also have to handle node removal.
         if (checkType == SiblingElementRemoved && newFirstElement == elementAfterChange && newFirstElement) {
             auto* style = newFirstElement->renderStyle();
             if (!style || !style->firstChildState())
-                newFirstElement->invalidateStyleForSubtree();
+                newFirstElement->invalidateStyleForSubtreeInternal();
         }
     }
 
@@ -2047,7 +2052,7 @@ static void checkForSiblingStyleChanges(Element& parent, SiblingCheckType checkT
         if (newLastElement != elementBeforeChange) {
             auto* style = elementBeforeChange->renderStyle();
             if (!style || style->lastChildState())
-                elementBeforeChange->invalidateStyleForSubtree();
+                elementBeforeChange->invalidateStyleForSubtreeInternal();
         }
 
         // We also have to handle node removal.  The parser callback case is similar to node removal as well in that we need to change the last child
@@ -2055,34 +2060,22 @@ static void checkForSiblingStyleChanges(Element& parent, SiblingCheckType checkT
         if ((checkType == SiblingElementRemoved || checkType == FinishedParsingChildren) && newLastElement == elementBeforeChange && newLastElement) {
             auto* style = newLastElement->renderStyle();
             if (!style || !style->lastChildState())
-                newLastElement->invalidateStyleForSubtree();
+                newLastElement->invalidateStyleForSubtreeInternal();
         }
     }
 
-    if (elementAfterChange) {
-        if (elementAfterChange->styleIsAffectedByPreviousSibling())
-            elementAfterChange->invalidateStyleForSubtree();
-        else if (elementAfterChange->affectsNextSiblingElementStyle()) {
-            Element* elementToInvalidate = elementAfterChange;
-            do {
-                elementToInvalidate = elementToInvalidate->nextElementSibling();
-            } while (elementToInvalidate && !elementToInvalidate->styleIsAffectedByPreviousSibling());
-
-            if (elementToInvalidate)
-                elementToInvalidate->invalidateStyleForSubtree();
-        }
-    }
+    invalidateForSiblingCombinators(elementAfterChange);
 
     // We have to invalidate everything following the insertion point in the forward case, and everything before the insertion point in the
     // backward case.
     if (parent.childrenAffectedByForwardPositionalRules()) {
         for (auto* next = elementAfterChange; next; next = next->nextElementSibling())
-            next->invalidateStyleForSubtree();
+            next->invalidateStyleForSubtreeInternal();
     }
     // Backward positional selectors include nth-last-child, nth-last-of-type, last-of-type and only-of-type.
     if (parent.childrenAffectedByBackwardPositionalRules()) {
         for (auto* previous = elementBeforeChange; previous; previous = previous->previousElementSibling())
-            previous->invalidateStyleForSubtree();
+            previous->invalidateStyleForSubtreeInternal();
     }
 }