Skip to content
Permalink
Browse files
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.


Canonical link: https://commits.webkit.org/198419@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@228285 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
anttijk committed Feb 8, 2018
1 parent e90e6d9 commit 25623ea20ee39f3dd6c9d72d88c82317c3c0393c
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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);
};
@@ -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());
}

0 comments on commit 25623ea

Please sign in to comment.