Merge r228285 - Use invalidation rulesets for attribute selectors

https://bugs.webkit.org/show_bug.cgi?id=182569

Reviewed by Zalan Bujtas.

Attribute change style invalidation should use invalidation rulesets, similarly how class change invalidation already does.
We'll invalidate fewer unnecessary elements immediately and enable more significant future gains.

* css/DocumentRuleSets.cpp:
(WebCore::DocumentRuleSets::collectFeatures const):
(WebCore::DocumentRuleSets::classInvalidationRuleSets const):
(WebCore::DocumentRuleSets::attributeInvalidationRuleSets const):

Make and cache invalidation RuleSets for an attribute.

(WebCore::DocumentRuleSets::ancestorAttributeRulesForHTML const): Deleted.
* css/DocumentRuleSets.h:
* css/RuleFeature.cpp:
(WebCore::RuleFeatureSet::recursivelyCollectFeaturesFromSelector):

Collect attribute selectors along with match elements.

(WebCore::RuleFeatureSet::collectFeatures):
(WebCore::RuleFeatureSet::add):
(WebCore::RuleFeatureSet::registerContentAttribute):

Separate hash to deal with invalidation of content:attr(foo) special case.

(WebCore::RuleFeatureSet::clear):
(WebCore::RuleFeatureSet::shrinkToFit):
(WebCore::makeAttributeSelectorKey): Deleted.
* css/RuleFeature.h:
(WebCore::RuleFeature::RuleFeature):
* css/StyleBuilderCustom.h:
(WebCore::StyleBuilderCustom::applyValueContent):
(WebCore::StyleBuilderCustom::applyValueAlt):

Use registerContentAttribute()

* html/HTMLEmbedElement.cpp:
(WebCore::hasTypeOrSrc):
(WebCore::HTMLEmbedElement::parseAttribute):

    Invalidate style if both type and src attributes go missing as this changes result of rendererIsNeeded().
    This was previously relying on any attribute change invalidating style.

(WebCore::HTMLEmbedElement::rendererIsNeeded):
* style/AttributeChangeInvalidation.cpp:
(WebCore::Style::AttributeChangeInvalidation::invalidateStyle):

Collect the invalidation rulesets for this attribute change.
Also check if any attribute selector actually changes state, unlike with classes attribute changes may
often not lead to a selector becoming non-matching.

(WebCore::Style::AttributeChangeInvalidation::invalidateStyleWithRuleSets):
(WebCore::Style::AttributeChangeInvalidation::invalidateDescendants): Deleted.
* style/AttributeChangeInvalidation.h:
(WebCore::Style::AttributeChangeInvalidation::AttributeChangeInvalidation):
(WebCore::Style::AttributeChangeInvalidation::~AttributeChangeInvalidation):
* style/ClassChangeInvalidation.cpp:
(WebCore::Style::ClassChangeInvalidation::computeInvalidation):

Should not bail on shadow tree invalidation as we may also need to invalidate siblings.

Source/WebCore/ChangeLog

@@ -1,3 +1,69 @@
+2018-02-08  Antti Koivisto  <antti@apple.com>
+
+        Use invalidation rulesets for attribute selectors
+        https://bugs.webkit.org/show_bug.cgi?id=182569
+
+        Reviewed by Zalan Bujtas.
+
+        Attribute change style invalidation should use invalidation rulesets, similarly how class change invalidation already does.
+        We'll invalidate fewer unnecessary elements immediately and enable more significant future gains.
+
+        * css/DocumentRuleSets.cpp:
+        (WebCore::DocumentRuleSets::collectFeatures const):
+        (WebCore::DocumentRuleSets::classInvalidationRuleSets const):
+        (WebCore::DocumentRuleSets::attributeInvalidationRuleSets const):
+
+        Make and cache invalidation RuleSets for an attribute.
+
+        (WebCore::DocumentRuleSets::ancestorAttributeRulesForHTML const): Deleted.
+        * css/DocumentRuleSets.h:
+        * css/RuleFeature.cpp:
+        (WebCore::RuleFeatureSet::recursivelyCollectFeaturesFromSelector):
+
+        Collect attribute selectors along with match elements.
+
+        (WebCore::RuleFeatureSet::collectFeatures):
+        (WebCore::RuleFeatureSet::add):
+        (WebCore::RuleFeatureSet::registerContentAttribute):
+
+        Separate hash to deal with invalidation of content:attr(foo) special case.
+
+        (WebCore::RuleFeatureSet::clear):
+        (WebCore::RuleFeatureSet::shrinkToFit):
+        (WebCore::makeAttributeSelectorKey): Deleted.
+        * css/RuleFeature.h:
+        (WebCore::RuleFeature::RuleFeature):
+        * css/StyleBuilderCustom.h:
+        (WebCore::StyleBuilderCustom::applyValueContent):
+        (WebCore::StyleBuilderCustom::applyValueAlt):
+
+        Use registerContentAttribute()
+
+        * html/HTMLEmbedElement.cpp:
+        (WebCore::hasTypeOrSrc):
+        (WebCore::HTMLEmbedElement::parseAttribute):
+
+            Invalidate style if both type and src attributes go missing as this changes result of rendererIsNeeded().
+            This was previously relying on any attribute change invalidating style.
+
+        (WebCore::HTMLEmbedElement::rendererIsNeeded):
+        * style/AttributeChangeInvalidation.cpp:
+        (WebCore::Style::AttributeChangeInvalidation::invalidateStyle):
+
+        Collect the invalidation rulesets for this attribute change.
+        Also check if any attribute selector actually changes state, unlike with classes attribute changes may
+        often not lead to a selector becoming non-matching.
+
+        (WebCore::Style::AttributeChangeInvalidation::invalidateStyleWithRuleSets):
+        (WebCore::Style::AttributeChangeInvalidation::invalidateDescendants): Deleted.
+        * style/AttributeChangeInvalidation.h:
+        (WebCore::Style::AttributeChangeInvalidation::AttributeChangeInvalidation):
+        (WebCore::Style::AttributeChangeInvalidation::~AttributeChangeInvalidation):
+        * style/ClassChangeInvalidation.cpp:
+        (WebCore::Style::ClassChangeInvalidation::computeInvalidation):
+
+        Should not bail on shadow tree invalidation as we may also need to invalidate siblings.
+
 2018-02-08  Zalan Bujtas  <zalan@apple.com>
 
         [RenderTreeBuilder] Introduce RenderTreeBuilder to moveChild(ren)To() functions

Source/WebCore/css/DocumentRuleSets.cpp

@@ -168,47 +168,46 @@ void DocumentRuleSets::collectFeatures() const
     m_uncommonAttributeRuleSet = makeRuleSet(m_features.uncommonAttributeRules);
 
     m_classInvalidationRuleSets.clear();
-    m_ancestorAttributeRuleSetsForHTML.clear();
+    m_attributeInvalidationRuleSets.clear();
 
     m_features.shrinkToFit();
 }
 
-const Vector<InvalidationRuleSet>* DocumentRuleSets::classInvalidationRuleSets(const AtomicString& className) const
+static Vector<InvalidationRuleSet>* ensureInvalidationRuleSets(const AtomicString& key, HashMap<AtomicString, std::unique_ptr<Vector<InvalidationRuleSet>>>& ruleSetMap, const HashMap<AtomicString, std::unique_ptr<Vector<RuleFeature>>>& ruleFeatures)
 {
-    return m_classInvalidationRuleSets.ensure(className, [&] () -> std::unique_ptr<Vector<InvalidationRuleSet>> {
-        auto* features = m_features.classRules.get(className);
+    return ruleSetMap.ensure(key, [&] () -> std::unique_ptr<Vector<InvalidationRuleSet>> {
+        auto* features = ruleFeatures.get(key);
         if (!features)
             return nullptr;
+
         std::array<std::unique_ptr<RuleSet>, matchElementCount> matchElementArray;
+        std::array<Vector<const CSSSelector*>, matchElementCount> invalidationSelectorArray;
         for (auto& feature : *features) {
-            auto& ruleSet = matchElementArray[static_cast<unsigned>(*feature.matchElement)];
+            auto arrayIndex = static_cast<unsigned>(*feature.matchElement);
+            auto& ruleSet = matchElementArray[arrayIndex];
             if (!ruleSet)
                 ruleSet = std::make_unique<RuleSet>();
             ruleSet->addRule(feature.rule, feature.selectorIndex);
+            if (feature.invalidationSelector)
+                invalidationSelectorArray[arrayIndex].append(feature.invalidationSelector);
         }
         auto invalidationRuleSets = std::make_unique<Vector<InvalidationRuleSet>>();
         for (unsigned i = 0; i < matchElementArray.size(); ++i) {
             if (matchElementArray[i])
-                invalidationRuleSets->append({ static_cast<MatchElement>(i), WTFMove(matchElementArray[i]) });
+                invalidationRuleSets->append({ static_cast<MatchElement>(i), WTFMove(matchElementArray[i]), WTFMove(invalidationSelectorArray[i]) });
         }
         return invalidationRuleSets;
     }).iterator->value.get();
 }
 
-const DocumentRuleSets::AttributeRules* DocumentRuleSets::ancestorAttributeRulesForHTML(const AtomicString& attributeName) const
+const Vector<InvalidationRuleSet>* DocumentRuleSets::classInvalidationRuleSets(const AtomicString& className) const
 {
-    auto addResult = m_ancestorAttributeRuleSetsForHTML.add(attributeName, nullptr);
-    auto& value = addResult.iterator->value;
-    if (addResult.isNewEntry) {
-        if (auto* rules = m_features.ancestorAttributeRulesForHTML.get(attributeName)) {
-            value = std::make_unique<AttributeRules>();
-            value->attributeSelectors.reserveCapacity(rules->selectors.size());
-            for (auto* selector : rules->selectors.values())
-                value->attributeSelectors.uncheckedAppend(selector);
-            value->ruleSet = makeRuleSet(rules->features);
-        }
-    }
-    return value.get();
+    return ensureInvalidationRuleSets(className, m_classInvalidationRuleSets, m_features.classRules);
+}
+
+const Vector<InvalidationRuleSet>* DocumentRuleSets::attributeInvalidationRuleSets(const AtomicString& attributeName) const
+{
+    return ensureInvalidationRuleSets(attributeName, m_attributeInvalidationRuleSets, m_features.attributeRules);
 }
 
 } // namespace WebCore

Source/WebCore/css/DocumentRuleSets.h

@@ -41,6 +41,7 @@ class MediaQueryEvaluator;
 struct InvalidationRuleSet {
     MatchElement matchElement;
     std::unique_ptr<RuleSet> ruleSet;
+    Vector<const CSSSelector*> invalidationSelectors;
 
     WTF_MAKE_FAST_ALLOCATED;
 };
@@ -59,14 +60,7 @@ class DocumentRuleSets {
     RuleSet* uncommonAttribute() const { return m_uncommonAttributeRuleSet.get(); }
 
     const Vector<InvalidationRuleSet>* classInvalidationRuleSets(const AtomicString& className) const;
-
-    struct AttributeRules {
-        WTF_MAKE_FAST_ALLOCATED;
-    public:
-        Vector<const CSSSelector*> attributeSelectors;
-        std::unique_ptr<RuleSet> ruleSet;
-    };
-    const AttributeRules* ancestorAttributeRulesForHTML(const AtomicString&) const;
+    const Vector<InvalidationRuleSet>* attributeInvalidationRuleSets(const AtomicString& attributeName) const;
 
     void setIsForShadowScope() { m_isForShadowScope = true; }
 
@@ -99,7 +93,7 @@ class DocumentRuleSets {
     mutable std::unique_ptr<RuleSet> m_siblingRuleSet;
     mutable std::unique_ptr<RuleSet> m_uncommonAttributeRuleSet;
     mutable HashMap<AtomicString, std::unique_ptr<Vector<InvalidationRuleSet>>> m_classInvalidationRuleSets;
-    mutable HashMap<AtomicString, std::unique_ptr<AttributeRules>> m_ancestorAttributeRuleSetsForHTML;
+    mutable HashMap<AtomicString, std::unique_ptr<Vector<InvalidationRuleSet>>> m_attributeInvalidationRuleSets;
 };
 
 inline const RuleFeatureSet& DocumentRuleSets::features() const

Source/WebCore/css/RuleFeature.cpp

@@ -130,8 +130,7 @@ void RuleFeatureSet::recursivelyCollectFeaturesFromSelector(SelectorFeatures& se
             auto& localName = selector->attribute().localName();
             attributeCanonicalLocalNamesInRules.add(canonicalLocalName);
             attributeLocalNamesInRules.add(localName);
-            if (matchElement == MatchElement::Parent || matchElement == MatchElement::Ancestor)
-                selectorFeatures.attributeSelectorsMatchingAncestors.append(selector);
+            selectorFeatures.attributes.append(std::make_pair(selector, matchElement));
         } else if (selector->match() == CSSSelector::PseudoElement) {
             switch (selector->pseudoElementType()) {
             case CSSSelector::PseudoElementFirstLine:
@@ -164,13 +163,6 @@ void RuleFeatureSet::recursivelyCollectFeaturesFromSelector(SelectorFeatures& se
     } while (selector);
 }
 
-static RuleFeatureSet::AttributeRules::SelectorKey makeAttributeSelectorKey(const CSSSelector& selector)
-{
-    bool caseInsensitive = selector.attributeValueMatchingIsCaseInsensitive();
-    unsigned matchAndCase = static_cast<unsigned>(selector.match()) << 1 | (caseInsensitive ? 1 : 0);
-    return std::make_pair(selector.attributeCanonicalLocalName().impl(), std::make_pair(selector.value().impl(), matchAndCase));
-}
-
 void RuleFeatureSet::collectFeatures(const RuleData& ruleData)
 {
     SelectorFeatures selectorFeatures;
@@ -179,22 +171,23 @@ void RuleFeatureSet::collectFeatures(const RuleData& ruleData)
         siblingRules.append(RuleFeature(ruleData.rule(), ruleData.selectorIndex()));
     if (ruleData.containsUncommonAttributeSelector())
         uncommonAttributeRules.append(RuleFeature(ruleData.rule(), ruleData.selectorIndex()));
+
     for (auto& nameAndMatch : selectorFeatures.classes) {
         classRules.ensure(nameAndMatch.first, [] {
             return std::make_unique<Vector<RuleFeature>>();
         }).iterator->value->append(RuleFeature(ruleData.rule(), ruleData.selectorIndex(), nameAndMatch.second));
         if (nameAndMatch.second == MatchElement::Host)
             classesAffectingHost.add(nameAndMatch.first);
     }
-    for (auto* selector : selectorFeatures.attributeSelectorsMatchingAncestors) {
-        // Hashing by attributeCanonicalLocalName makes this HTML specific.
-        auto addResult = ancestorAttributeRulesForHTML.ensure(selector->attributeCanonicalLocalName(), [] {
-            return std::make_unique<AttributeRules>();
-        });
-        auto& rules = *addResult.iterator->value;
-        rules.features.append(RuleFeature(ruleData.rule(), ruleData.selectorIndex()));
-        // Deduplicate selectors.
-        rules.selectors.add(makeAttributeSelectorKey(*selector), selector);
+
+    for (auto& selectorAndMatch : selectorFeatures.attributes) {
+        auto* selector = selectorAndMatch.first;
+        auto matchElement = selectorAndMatch.second;
+        attributeRules.ensure(selector->attribute().localName().convertToASCIILowercase(), [] {
+            return std::make_unique<Vector<RuleFeature>>();
+        }).iterator->value->append(RuleFeature(ruleData.rule(), ruleData.selectorIndex(), matchElement, selector));
+        if (matchElement == MatchElement::Host)
+            attributesAffectingHost.add(selector->attribute().localName().convertToASCIILowercase());
     }
 }
 
@@ -204,6 +197,7 @@ void RuleFeatureSet::add(const RuleFeatureSet& other)
     idsMatchingAncestorsInRules.add(other.idsMatchingAncestorsInRules.begin(), other.idsMatchingAncestorsInRules.end());
     attributeCanonicalLocalNamesInRules.add(other.attributeCanonicalLocalNamesInRules.begin(), other.attributeCanonicalLocalNamesInRules.end());
     attributeLocalNamesInRules.add(other.attributeLocalNamesInRules.begin(), other.attributeLocalNamesInRules.end());
+    contentAttributeNamesInRules.add(other.contentAttributeNamesInRules.begin(), other.contentAttributeNamesInRules.end());
     siblingRules.appendVector(other.siblingRules);
     uncommonAttributeRules.appendVector(other.uncommonAttributeRules);
     for (auto& keyValuePair : other.classRules) {
@@ -213,30 +207,37 @@ void RuleFeatureSet::add(const RuleFeatureSet& other)
     }
     classesAffectingHost.add(other.classesAffectingHost.begin(), other.classesAffectingHost.end());
 
-    for (auto& keyValuePair : other.ancestorAttributeRulesForHTML) {
-        auto addResult = ancestorAttributeRulesForHTML.ensure(keyValuePair.key, [] {
-            return std::make_unique<AttributeRules>();
-        });
-        auto& rules = *addResult.iterator->value;
-        rules.features.appendVector(keyValuePair.value->features);
-        for (auto& selectorPair : keyValuePair.value->selectors)
-            rules.selectors.add(selectorPair.key, selectorPair.value);
+    for (auto& keyValuePair : other.attributeRules) {
+        attributeRules.ensure(keyValuePair.key, [] {
+            return std::make_unique<Vector<RuleFeature>>();
+        }).iterator->value->appendVector(*keyValuePair.value);
     }
+    attributesAffectingHost.add(other.attributesAffectingHost.begin(), other.attributesAffectingHost.end());
+
     usesFirstLineRules = usesFirstLineRules || other.usesFirstLineRules;
     usesFirstLetterRules = usesFirstLetterRules || other.usesFirstLetterRules;
 }
 
+void RuleFeatureSet::registerContentAttribute(const AtomicString& attributeName)
+{
+    contentAttributeNamesInRules.add(attributeName.convertToASCIILowercase());
+    attributeCanonicalLocalNamesInRules.add(attributeName);
+    attributeLocalNamesInRules.add(attributeName);
+}
+
 void RuleFeatureSet::clear()
 {
     idsInRules.clear();
     idsMatchingAncestorsInRules.clear();
     attributeCanonicalLocalNamesInRules.clear();
     attributeLocalNamesInRules.clear();
+    contentAttributeNamesInRules.clear();
     siblingRules.clear();
     uncommonAttributeRules.clear();
     classRules.clear();
     classesAffectingHost.clear();
-    ancestorAttributeRulesForHTML.clear();
+    attributeRules.clear();
+    attributesAffectingHost.clear();
     usesFirstLineRules = false;
     usesFirstLetterRules = false;
 }
@@ -247,8 +248,8 @@ void RuleFeatureSet::shrinkToFit()
     uncommonAttributeRules.shrinkToFit();
     for (auto& rules : classRules.values())
         rules->shrinkToFit();
-    for (auto& rules : ancestorAttributeRulesForHTML.values())
-        rules->features.shrinkToFit();
+    for (auto& rules : attributeRules.values())
+        rules->shrinkToFit();
 }
 
 } // namespace WebCore

Source/WebCore/css/RuleFeature.h

@@ -37,41 +37,39 @@ enum class MatchElement { Subject, Parent, Ancestor, DirectSibling, IndirectSibl
 constexpr unsigned matchElementCount = static_cast<unsigned>(MatchElement::Host) + 1;
 
 struct RuleFeature {
-    RuleFeature(StyleRule* rule, unsigned selectorIndex, std::optional<MatchElement> matchElement = std::nullopt)
+    RuleFeature(StyleRule* rule, unsigned selectorIndex, std::optional<MatchElement> matchElement = std::nullopt, const CSSSelector* invalidationSelector = nullptr)
         : rule(rule)
         , selectorIndex(selectorIndex)
         , matchElement(matchElement)
+        , invalidationSelector(invalidationSelector)
     {
     }
     StyleRule* rule;
     unsigned selectorIndex;
     std::optional<MatchElement> matchElement;
+    const CSSSelector* invalidationSelector;
 };
 
 struct RuleFeatureSet {
     void add(const RuleFeatureSet&);
     void clear();
     void shrinkToFit();
     void collectFeatures(const RuleData&);
+    void registerContentAttribute(const AtomicString&);
 
     HashSet<AtomicString> idsInRules;
     HashSet<AtomicString> idsMatchingAncestorsInRules;
     HashSet<AtomicString> attributeCanonicalLocalNamesInRules;
     HashSet<AtomicString> attributeLocalNamesInRules;
+    HashSet<AtomicString> contentAttributeNamesInRules;
     Vector<RuleFeature> siblingRules;
     Vector<RuleFeature> uncommonAttributeRules;
 
     HashMap<AtomicString, std::unique_ptr<Vector<RuleFeature>>> classRules;
+    HashMap<AtomicString, std::unique_ptr<Vector<RuleFeature>>> attributeRules;
     HashSet<AtomicString> classesAffectingHost;
+    HashSet<AtomicString> attributesAffectingHost;
 
-    struct AttributeRules {
-        WTF_MAKE_FAST_ALLOCATED;
-    public:
-        using SelectorKey = std::pair<AtomicStringImpl*, std::pair<AtomicStringImpl*, unsigned>>;
-        HashMap<SelectorKey, const CSSSelector*> selectors;
-        Vector<RuleFeature> features;
-    };
-    HashMap<AtomicString, std::unique_ptr<AttributeRules>> ancestorAttributeRulesForHTML;
     bool usesFirstLineRules { false };
     bool usesFirstLetterRules { false };
 
@@ -83,7 +81,7 @@ struct RuleFeatureSet {
         bool hasSiblingSelector { false };
 
         Vector<std::pair<AtomicString, MatchElement>, 32> classes;
-        Vector<const CSSSelector*> attributeSelectorsMatchingAncestors;
+        Vector<std::pair<const CSSSelector*, MatchElement>, 32> attributes;
     };
     void recursivelyCollectFeaturesFromSelector(SelectorFeatures&, const CSSSelector&, MatchElement = MatchElement::Subject);
 };

Source/WebCore/css/StyleBuilderCustom.h

@@ -1423,8 +1423,7 @@ inline void StyleBuilderCustom::applyValueContent(StyleResolver& styleResolver,
             styleResolver.style()->setContent(value.isNull() ? emptyAtom() : value.impl(), didSet);
             didSet = true;
             // Register the fact that the attribute value affects the style.
-            styleResolver.ruleSets().mutableFeatures().attributeCanonicalLocalNamesInRules.add(attr.localName().impl());
-            styleResolver.ruleSets().mutableFeatures().attributeLocalNamesInRules.add(attr.localName().impl());
+            styleResolver.ruleSets().mutableFeatures().registerContentAttribute(attr.localName());
         } else if (contentValue.isCounter()) {
             auto* counterValue = contentValue.counterValue();
             EListStyleType listStyleType = NoneListStyle;
@@ -1821,8 +1820,7 @@ void StyleBuilderCustom::applyValueAlt(StyleResolver& styleResolver, CSSValue& v
         styleResolver.style()->setContentAltText(value.isNull() ? emptyAtom() : value);
 
         // Register the fact that the attribute value affects the style.
-        styleResolver.ruleSets().mutableFeatures().attributeCanonicalLocalNamesInRules.add(attr.localName().impl());
-        styleResolver.ruleSets().mutableFeatures().attributeLocalNamesInRules.add(attr.localName().impl());
+        styleResolver.ruleSets().mutableFeatures().registerContentAttribute(attr.localName());
     } else
         styleResolver.style()->setContentAltText(emptyAtom());
 }