Invalidation set features recognized left of adjacent.

Source/core/css/RuleFeature.cpp

Index: Source/core/css/RuleFeature.cpp
diff --git a/Source/core/css/RuleFeature.cpp b/Source/core/css/RuleFeature.cpp
index 51f4966c9db8b0f4b8f1f377f9c0748ee269d8db..307c819e701a1a7033c5d8fa3e23c6e86d2dd94c 100644
--- a/Source/core/css/RuleFeature.cpp
+++ b/Source/core/css/RuleFeature.cpp
@@ -119,44 +119,30 @@ void RuleFeature::trace(Visitor* visitor)
 // This method is somewhat conservative in what it accepts.
 RuleFeatureSet::InvalidationSetMode RuleFeatureSet::invalidationSetModeForSelector(const CSSSelector& selector)
 {
-    bool foundDescendantRelation = false;
+    bool foundCombinator = false;
     bool foundIdent = false;
     for (const CSSSelector* component = &selector; component; component = component->tagHistory()) {
 
         if (component->match() == CSSSelector::Class || component->match() == CSSSelector::Id
             || (component->match() == CSSSelector::Tag && component->tagQName().localName() != starAtom)
             || component->isAttributeSelector() || component->isCustomPseudoElement()) {
-            if (!foundDescendantRelation)
+            if (!foundCombinator)
                 foundIdent = true;
         } else if (component->pseudoType() == CSSSelector::PseudoHost || component->pseudoType() == CSSSelector::PseudoAny) {
             if (const CSSSelectorList* selectorList = component->selectorList()) {
                 for (const CSSSelector* selector = selectorList->first(); selector; selector = CSSSelectorList::next(*selector)) {
                     InvalidationSetMode hostMode = invalidationSetModeForSelector(*selector);
                     if (hostMode == UseSubtreeStyleChange)
-                        return foundDescendantRelation ? UseLocalStyleChange : UseSubtreeStyleChange;
-                    if (!foundDescendantRelation && hostMode == AddFeatures)
+                        return foundCombinator ? UseLocalStyleChange : UseSubtreeStyleChange;
+                    if (!foundCombinator && hostMode == AddFeatures)
                         foundIdent = true;
                 }
             }
         } else if (!isSkippableComponentForInvalidation(*component)) {
-            return foundDescendantRelation ? UseLocalStyleChange : UseSubtreeStyleChange;
-        }
-        switch (component->relation()) {
-        case CSSSelector::Descendant:
-        case CSSSelector::Child:
-        case CSSSelector::ShadowPseudo:
-        case CSSSelector::ShadowDeep:
-            foundDescendantRelation = true;
-            // Fall through!
-        case CSSSelector::SubSelector:
-        case CSSSelector::DirectAdjacent:
-        case CSSSelector::IndirectAdjacent:
-            continue;
-        default:
-            // All combinators should be handled above.
-            ASSERT_NOT_REACHED();
-            return UseLocalStyleChange;
+            return foundCombinator ? UseLocalStyleChange : UseSubtreeStyleChange;
         }
+        if (component->relation() != CSSSelector::SubSelector)
+            foundCombinator = true;
     }
     return foundIdent ? AddFeatures : UseLocalStyleChange;
 }
@@ -234,6 +220,9 @@ const CSSSelector* RuleFeatureSet::extractInvalidationSetFeatures(const CSSSelec
             return current->tagHistory();
         case CSSSelector::DirectAdjacent:
         case CSSSelector::IndirectAdjacent:
+            // Style invalidation is currently supported for descendants only, not for sibling subtrees.
+            // Use wholeSubtree invalidation for adjacent combinators as SubtreeStyleChange will force
+            // sibling subtree recalc in ContainerNode::checkForChildrenAdjacentRuleChanges.
             features.wholeSubtree = true;
             return current->tagHistory();
         case CSSSelector::Descendant:
@@ -278,14 +267,21 @@ void RuleFeatureSet::addFeaturesToInvalidationSets(const CSSSelector& selector,
         case CSSSelector::ShadowPseudo:
         case CSSSelector::ShadowDeep:
             features.treeBoundaryCrossing = true;
+            // If we saw an adjacent combinator, wholeSubtree would have been set to true. A descendant
+            // type combinator cancels out the need for that.
             features.wholeSubtree = false;
             break;
         case CSSSelector::Descendant:
         case CSSSelector::Child:
+            // If we saw an adjacent combinator, wholeSubtree would have been set to true. A descendant
+            // type combinator cancels out the need for that.
             features.wholeSubtree = false;
             break;
         case CSSSelector::DirectAdjacent:
         case CSSSelector::IndirectAdjacent:
+            // Style invalidation is currently supported for descendants only, not for sibling subtrees.
+            // Use wholeSubtree invalidation for adjacent combinators as SubtreeStyleChange will force
+            // sibling subtree recalc in ContainerNode::checkForChildrenAdjacentRuleChanges.

[esprehn, 2014/08/16 07:58:34]

Not a fan of the copy pasta comments. Can you just put a comment above the
functions and explain what they do instead?

[rune, 2014/08/18 21:13:06]

Done.

             features.wholeSubtree = true;
             break;
         }