Changeset 229368 in webkit

Timestamp:

Mar 7, 2018 10:27:55 AM (6 years ago)

Author:

Antti Koivisto

Message:

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):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• css/SelectorChecker.cpp (modified) (3 diffs)
• dom/Element.cpp (modified) (7 diffs)

trunk/Source/WebCore/ChangeLog

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

trunk/Source/WebCore/css/SelectorChecker.cpp

@@ -104 +104 @@
 }
 
-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;
@@ -746 +744 @@
         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;
@@ -786 +784 @@
             // 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;

trunk/Source/WebCore/dom/Element.cpp

@@ -1481 +1481 @@
 }
 
-static void invalidateSiblingsIfNeeded(Element& element)
-{
-    if (!element.affectsNextSiblingElementStyle())
-        return;
-    auto* parent = element.parentElement();
-    if (parent && parent->styleValidity() >= Style::Validity::SubtreeInvalid)
-        return;
-
-    for (auto* sibling = element.nextElementSibling(); sibling; sibling = sibling->nextElementSibling()) {
+static void invalidateForSiblingCombinators(Element* sibling)
+{
+    for (; sibling; sibling = sibling->nextElementSibling()) {
         if (sibling->styleIsAffectedByPreviousSibling())
             sibling->invalidateStyleForSubtreeInternal();
@@ -1495 +1489 @@
             return;
     }
+}
+
+static void invalidateSiblingsIfNeeded(Element& element)
+{
+    if (!element.affectsNextSiblingElementStyle())
+        return;
+    auto* parent = element.parentElement();
+    if (parent && parent->styleValidity() >= Style::Validity::SubtreeInvalid)
+        return;
+
+    invalidateForSiblingCombinators(element.nextElementSibling());
 }
 
@@ -2028 +2033 @@
             auto* style = elementAfterChange->renderStyle();
             if (!style || style->firstChildState())
-                elementAfterChange->invalidateStyleForSubtree();
+                elementAfterChange->invalidateStyleForSubtreeInternal();
         }
 
@@ -2035 +2040 @@
             auto* style = newFirstElement->renderStyle();
             if (!style || !style->firstChildState())
-                newFirstElement->invalidateStyleForSubtree();
+                newFirstElement->invalidateStyleForSubtreeInternal();
         }
     }
@@ -2048 +2053 @@
             auto* style = elementBeforeChange->renderStyle();
             if (!style || style->lastChildState())
-                elementBeforeChange->invalidateStyleForSubtree();
+                elementBeforeChange->invalidateStyleForSubtreeInternal();
         }
 
@@ -2056 +2061 @@
             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
@@ -2078 +2071 @@
     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();
     }
 }