Refactor RuleFeatureSet to simplify iteration over CSS Selectors when collecting data.

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 104 matching lines @@
             if (!tagName.isEmpty())
                 invalidationSet.addTagName(tagName);
             for (HashSet<AtomicString>::const_iterator it = classes.begin(); it != classes.end(); ++it) {
                 invalidationSet.addClass(*it);
             }
         }
     }
     return true;
 }
 
+void RuleFeatureSet::addAttributeInASelector(const AtomicString& attributeName)
+{
+    m_metadata.attrsInRules.add(attributeName);
+}
+
+
 void RuleFeatureSet::collectFeaturesFromRuleData(const RuleData& ruleData)
 {
-    bool foundSiblingSelector = false;
-    unsigned maxDirectAdjacentSelectors = 0;
-    for (const CSSSelector* selector = ruleData.selector(); selector; selector = selector->tagHistory()) {
-        collectFeaturesFromSelector(selector);
 
-        if (const CSSSelectorList* selectorList = selector->selectorList()) {
-            for (const CSSSelector* subSelector = selectorList->first(); subSelector; subSelector = CSSSelectorList::next(subSelector)) {
-                // FIXME: Shouldn't this be checking subSelector->isSiblingSelector()?
-                if (!foundSiblingSelector && selector->isSiblingSelector())
-                    foundSiblingSelector = true;
-                if (subSelector->isDirectAdjacentSelector())
-                    maxDirectAdjacentSelectors++;
-                collectFeaturesFromSelector(subSelector);
-            }
-        } else {
-            if (!foundSiblingSelector && selector->isSiblingSelector())
-                foundSiblingSelector = true;
-            if (selector->isDirectAdjacentSelector())
-                maxDirectAdjacentSelectors++;
-        }
-    }
+    FeatureMetadata metadata;
+    collectFeaturesFromSelector(ruleData.selector(), metadata);
+    m_metadata.add(metadata);

[esprehn, 2014/01/23 01:13:29]

Doesn't this mean we need to do more hash table iterations and copying?

[chrishtr, 2014/01/23 01:20:56]

Yes. It'll have to do twice as many operations as before. However, I will soon
be able to remove the redundant ones, since a descendant invalidation set will
have the same information about affected classe, ids and attrs, so we can get
rid of these hash sets.

     if (RuntimeEnabledFeatures::targetedStyleRecalcEnabled()) {
         bool selectorUsesClassInvalidationSet = updateClassInvalidationSets(ruleData.selector());
         if (!selectorUsesClassInvalidationSet) {
-            for (HashSet<AtomicString>::const_iterator it = classesInRules.begin(); it != classesInRules.end(); ++it) {
+            for (HashSet<AtomicString>::const_iterator it = metadata.classesInRules.begin(); it != metadata.classesInRules.end(); ++it) {
                 DescendantInvalidationSet& invalidationSet = ensureClassInvalidationSet(*it);
                 invalidationSet.setWholeSubtreeInvalid();
             }
         }
     }
-    setMaxDirectAdjacentSelectors(maxDirectAdjacentSelectors);
-    if (foundSiblingSelector)
+    if (metadata.foundSiblingSelector)
         siblingRules.append(RuleFeature(ruleData.rule(), ruleData.selectorIndex(), ruleData.hasDocumentSecurityOrigin()));
     if (ruleData.containsUncommonAttributeSelector())
         uncommonAttributeRules.append(RuleFeature(ruleData.rule(), ruleData.selectorIndex(), ruleData.hasDocumentSecurityOrigin()));
 }
 
 DescendantInvalidationSet& RuleFeatureSet::ensureClassInvalidationSet(const AtomicString& className)
 {
     InvalidationSetMap::AddResult addResult = m_classInvalidationSets.add(className, 0);
     if (addResult.isNewEntry)
         addResult.iterator->value = DescendantInvalidationSet::create();
     return *addResult.iterator->value;
 }
 
 void RuleFeatureSet::collectFeaturesFromSelector(const CSSSelector* selector)
 {
-    if (selector->m_match == CSSSelector::Id)
-        idsInRules.add(selector->value());
-    else if (selector->m_match == CSSSelector::Class)
-        classesInRules.add(selector->value());
-    else if (selector->isAttributeSelector())
-        attrsInRules.add(selector->attribute().localName());
-    switch (selector->pseudoType()) {
-    case CSSSelector::PseudoFirstLine:
-        m_usesFirstLineRules = true;
-        break;
-        break;
-    case CSSSelector::PseudoHost:
-        collectFeaturesFromSelectorList(selector->selectorList());
-        break;
-    default:
-        break;
+    collectFeaturesFromSelector(selector, m_metadata);
+}
+
+void RuleFeatureSet::collectFeaturesFromSelector(const CSSSelector* selector, RuleFeatureSet::FeatureMetadata& metadata)
+{
+    for (; selector; selector = selector->tagHistory()) {
+        if (selector->m_match == CSSSelector::Id)
+            metadata.idsInRules.add(selector->value());
+        else if (selector->m_match == CSSSelector::Class)
+            metadata.classesInRules.add(selector->value());
+        else if (selector->isAttributeSelector())
+            metadata.attrsInRules.add(selector->attribute().localName());
+
+        if (selector->pseudoType() == CSSSelector::PseudoFirstLine)
+            metadata.usesFirstLineRules = true;
+        if (selector->isDirectAdjacentSelector())
+            metadata.maxDirectAdjacentSelectors++;
+        if (selector->isSiblingSelector())
+            metadata.foundSiblingSelector = true;
+
+        collectFeaturesFromSelectorList(selector->selectorList(), metadata);
     }
 }
 
-void RuleFeatureSet::collectFeaturesFromSelectorList(const CSSSelectorList* selectorList)
+void RuleFeatureSet::collectFeaturesFromSelectorList(const CSSSelectorList* selectorList, RuleFeatureSet::FeatureMetadata& metadata)
 {
     if (!selectorList)
         return;
 
     for (const CSSSelector* selector = selectorList->first(); selector; selector = CSSSelectorList::next(selector)) {
         for (const CSSSelector* subSelector = selector; subSelector; subSelector = subSelector->tagHistory())
-            collectFeaturesFromSelector(subSelector);
+            collectFeaturesFromSelector(subSelector, metadata);
     }
 }
 
-void RuleFeatureSet::add(const RuleFeatureSet& other)
+void RuleFeatureSet::FeatureMetadata::add(const FeatureMetadata& other)
 {
-    for (InvalidationSetMap::const_iterator it = other.m_classInvalidationSets.begin(); it != other.m_classInvalidationSets.end(); ++it) {
-        ensureClassInvalidationSet(it->key).combine(*it->value);
-    }
+    usesFirstLineRules = usesFirstLineRules || other.usesFirstLineRules;
+    maxDirectAdjacentSelectors = std::max(maxDirectAdjacentSelectors, other.maxDirectAdjacentSelectors);
 
     HashSet<AtomicString>::const_iterator end = other.idsInRules.end();
     for (HashSet<AtomicString>::const_iterator it = other.idsInRules.begin(); it != end; ++it)
         idsInRules.add(*it);
     end = other.classesInRules.end();
     for (HashSet<AtomicString>::const_iterator it = other.classesInRules.begin(); it != end; ++it)
         classesInRules.add(*it);
     end = other.attrsInRules.end();
     for (HashSet<AtomicString>::const_iterator it = other.attrsInRules.begin(); it != end; ++it)
         attrsInRules.add(*it);
+}
+
+void RuleFeatureSet::FeatureMetadata::clear()
+{
+
+    idsInRules.clear();
+    classesInRules.clear();
+    attrsInRules.clear();
+    usesFirstLineRules = false;
+    foundSiblingSelector = false;
+    maxDirectAdjacentSelectors = 0;
+}
+
+void RuleFeatureSet::add(const RuleFeatureSet& other)
+{
+    for (InvalidationSetMap::const_iterator it = other.m_classInvalidationSets.begin(); it != other.m_classInvalidationSets.end(); ++it) {
+        ensureClassInvalidationSet(it->key).combine(*it->value);
+    }
+
+    m_metadata.add(other.m_metadata);
+
     siblingRules.append(other.siblingRules);
     uncommonAttributeRules.append(other.uncommonAttributeRules);
-    m_usesFirstLineRules = m_usesFirstLineRules || other.m_usesFirstLineRules;
-    m_maxDirectAdjacentSelectors = std::max(m_maxDirectAdjacentSelectors, other.maxDirectAdjacentSelectors());
 }
 
 void RuleFeatureSet::clear()
 {
-    idsInRules.clear();
-    classesInRules.clear();
-    attrsInRules.clear();
+    m_metadata.clear();
     siblingRules.clear();
     uncommonAttributeRules.clear();
-    m_usesFirstLineRules = false;
-    m_maxDirectAdjacentSelectors = 0;
 }
 
 } // namespace WebCore