Separate method for adding features to an invalidation set.

Source/core/css/RuleFeature.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 2004-2005 Allan Sandfeld Jensen (kde@carewolf.com)
  * Copyright (C) 2006, 2007 Nicholas Shanks (webkit@nickshanks.com)
  * Copyright (C) 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012 Apple Inc. All rights reserved.
  * Copyright (C) 2007 Alexey Proskuryakov <ap@webkit.org>
  * Copyright (C) 2007, 2008 Eric Seidel <eric@webkit.org>
  * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
  * Copyright (c) 2011, Code Aurora Forum. All rights reserved.
  * Copyright (C) Research In Motion Limited 2011. All rights reserved.
@@ skipping 266 matching lines @@
 //
 // Style invalidation is currently supported for descendants only, not for sibling subtrees.
 // We use wholeSubtree invalidation for features found left of adjacent combinators as
 // SubtreeStyleChange will force sibling subtree recalc in
 // ContainerNode::checkForChildrenAdjacentRuleChanges.
 //
 // As we encounter a descendant type of combinator, the features only need to be checked
 // against descendants in the same subtree only. features.adjacent is set to false, and
 // we start adding features instead of calling setWholeSubtreeInvalid.
 
+void RuleFeatureSet::addFeaturesToInvalidationSet(DescendantInvalidationSet& invalidationSet, const InvalidationSetFeatures& features)
+{
+    if (features.treeBoundaryCrossing)
+        invalidationSet.setTreeBoundaryCrossing();
+    if (features.adjacent) {
+        invalidationSet.setWholeSubtreeInvalid();
+        return;
+    }
+    if (!features.id.isEmpty())
+        invalidationSet.addId(features.id);
+    if (!features.tagName.isEmpty())
+        invalidationSet.addTagName(features.tagName);
+    for (const auto& className : features.classes)
+        invalidationSet.addClass(className);
+    for (const auto& attribute : features.attributes)
+        invalidationSet.addAttribute(attribute);
+    if (features.customPseudoElement)
+        invalidationSet.setCustomPseudoInvalid();
+}
+
 void RuleFeatureSet::addFeaturesToInvalidationSets(const CSSSelector& selector, InvalidationSetFeatures& features)
 {
     for (const CSSSelector* current = &selector; current; current = current->tagHistory()) {
         if (DescendantInvalidationSet* invalidationSet = invalidationSetForSelector(*current)) {
-            if (features.treeBoundaryCrossing)
-                invalidationSet->setTreeBoundaryCrossing();
-            if (features.adjacent) {
-                invalidationSet->setWholeSubtreeInvalid();
-            } else {
-                if (!features.id.isEmpty())
-                    invalidationSet->addId(features.id);
-                if (!features.tagName.isEmpty())
-                    invalidationSet->addTagName(features.tagName);
-                for (const auto& className : features.classes)
-                    invalidationSet->addClass(className);
-                for (const auto& attribute : features.attributes)
-                    invalidationSet->addAttribute(attribute);
-                if (features.customPseudoElement)
-                    invalidationSet->setCustomPseudoInvalid();
-            }
+            addFeaturesToInvalidationSet(*invalidationSet, features);
         } else {
             if (current->pseudoType() == CSSSelector::PseudoHost)
                 features.treeBoundaryCrossing = true;
             if (const CSSSelectorList* selectorList = current->selectorList()) {
                 ASSERT(current->pseudoType() == CSSSelector::PseudoHost || current->pseudoType() == CSSSelector::PseudoAny || current->pseudoType() == CSSSelector::PseudoNot);
                 for (const CSSSelector* selector = selectorList->first(); selector; selector = CSSSelectorList::next(*selector))
                     addFeaturesToInvalidationSets(*selector, features);
             }
         }
 
+        if (current->relation() == CSSSelector::SubSelector)

[chrishtr, 2014/10/13 15:44:31]

Why is this continue needed here? Is it because we need to avoid setting
adjacent to false after setting it to true earlier?

[rune, 2014/10/13 21:42:57]

Yes, without this "continue", we'd have to check for != SubSelector before
setting adjacent. We can keep the old code. I thought this looked clearer.

[chrishtr, 2014/10/13 22:00:25]

Let me put it in a different way: what would be the appropriate comment to put
before 322 to explain to readers why we short-circuit for subselector? That
would be a nice comment to add. (I actually don't have it in my head right now
either, so want to know.)

[rune, 2014/10/13 22:08:09]

Something like:

// No combinator to update flags for.

adjacent can be set unconditionally, since it's flipping true/false for all
combinators, but not for subselectors.

+            continue;
+
         if (current->isShadowSelector())

[chrishtr, 2014/10/13 15:44:31]

Is isShadowSelector() checked in?

[rune, 2014/10/13 21:42:57]

I don't understand.

[chrishtr, 2014/10/13 22:00:25]

Nevermind. I tried codesearching for this method earlier to understand it and
failed to find it. Can find it now.

             features.treeBoundaryCrossing = true;
 
-        if (current->relation() != CSSSelector::SubSelector)
-            features.adjacent = current->isAdjacentSelector();
+        features.adjacent = current->isAdjacentSelector();
     }
 }
 
 void RuleFeatureSet::addContentAttr(const AtomicString& attributeName)
 {
     ensureAttributeInvalidationSet(attributeName);
 }
 
 void RuleFeatureSet::collectFeaturesFromRuleData(const RuleData& ruleData)
 {
@@ skipping 212 matching lines @@
     visitor->trace(uncommonAttributeRules);
     visitor->trace(m_classInvalidationSets);
     visitor->trace(m_attributeInvalidationSets);
     visitor->trace(m_idInvalidationSets);
     visitor->trace(m_pseudoInvalidationSets);
     visitor->trace(m_styleInvalidator);
 #endif
 }
 
 } // namespace blink