Commit c97c740d authored by Anders Hartvoll Ruud's avatar Anders Hartvoll Ruud Committed by Commit Bot

[:is/:where] Handle type rule invalidation

We currently have a bug where :is/:where/:-webkit-any(.a, div)
doesn't cause 'div' to enter the type rule invalidation set.
This CL fixes that bug by tracking the emitted tag names separately.
The idea is that you can look at the features returned from
UpdateInvalidationSetsForComplex, and if has_features_for_rule_-
set_invalidation is false, then the (regular) tag names are the ones
that need to be added to the type rule invalidation set, and the
emitted tag names can be ignored.

It was necessary to change how has_features_for_rule_set_invalidation
is set during feature extraction. We currently set this to true if
we have _any_ id/class/attribute feature after list extraction, but
this means we consider :is(.a, div) to have features for type rule
purposes, so that doesn't work. This CL changes that such that all
selectors in the nested list need to have features for the flag to
be set for the outer selector.

Bug: 568705
Change-Id: Ibbfa8166a1bbb27d7e90f48f44bfe7bc9c6e6851
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2433928
Commit-Queue: Anders Hartvoll Ruud <andruud@chromium.org>
Reviewed-by: default avatarRune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#812628}
parent a08fd07d
...@@ -511,6 +511,9 @@ void RuleFeatureSet::UpdateFeaturesFromCombinator( ...@@ -511,6 +511,9 @@ void RuleFeatureSet::UpdateFeaturesFromCombinator(
void RuleFeatureSet::ExtractInvalidationSetFeaturesFromSimpleSelector( void RuleFeatureSet::ExtractInvalidationSetFeaturesFromSimpleSelector(
const CSSSelector& selector, const CSSSelector& selector,
InvalidationSetFeatures& features) { InvalidationSetFeatures& features) {
features.has_features_for_rule_set_invalidation |=
selector.IsIdClassOrAttributeSelector();
if (selector.Match() == CSSSelector::kTag && if (selector.Match() == CSSSelector::kTag &&
selector.TagQName().LocalName() != CSSSelector::UniversalSelectorAtom()) { selector.TagQName().LocalName() != CSSSelector::UniversalSelectorAtom()) {
features.NarrowToTag(selector.TagQName().LocalName()); features.NarrowToTag(selector.TagQName().LocalName());
...@@ -729,6 +732,7 @@ RuleFeatureSet::ExtractInvalidationSetFeaturesFromSelectorList( ...@@ -729,6 +732,7 @@ RuleFeatureSet::ExtractInvalidationSetFeaturesFromSelectorList(
const CSSSelector* sub_selector = selector_list->First(); const CSSSelector* sub_selector = selector_list->First();
bool all_sub_selectors_have_features = true; bool all_sub_selectors_have_features = true;
bool all_sub_selectors_have_features_for_ruleset_invalidation = true;
InvalidationSetFeatures any_features; InvalidationSetFeatures any_features;
for (; sub_selector; sub_selector = CSSSelectorList::Next(*sub_selector)) { for (; sub_selector; sub_selector = CSSSelectorList::Next(*sub_selector)) {
...@@ -739,6 +743,8 @@ RuleFeatureSet::ExtractInvalidationSetFeaturesFromSelectorList( ...@@ -739,6 +743,8 @@ RuleFeatureSet::ExtractInvalidationSetFeaturesFromSelectorList(
features.invalidation_flags.SetWholeSubtreeInvalid(true); features.invalidation_flags.SetWholeSubtreeInvalid(true);
return kRequiresSubtreeInvalidation; return kRequiresSubtreeInvalidation;
} }
all_sub_selectors_have_features_for_ruleset_invalidation &=
complex_features.has_features_for_rule_set_invalidation;
if (complex_features.has_nth_pseudo) if (complex_features.has_nth_pseudo)
features.has_nth_pseudo = true; features.has_nth_pseudo = true;
if (!all_sub_selectors_have_features) if (!all_sub_selectors_have_features)
...@@ -752,6 +758,8 @@ RuleFeatureSet::ExtractInvalidationSetFeaturesFromSelectorList( ...@@ -752,6 +758,8 @@ RuleFeatureSet::ExtractInvalidationSetFeaturesFromSelectorList(
// any invalidation set features. E.g. :-webkit-any(*, span). // any invalidation set features. E.g. :-webkit-any(*, span).
if (all_sub_selectors_have_features) if (all_sub_selectors_have_features)
features.NarrowToFeatures(any_features); features.NarrowToFeatures(any_features);
features.has_features_for_rule_set_invalidation |=
all_sub_selectors_have_features_for_ruleset_invalidation;
return kNormalInvalidation; return kNormalInvalidation;
} }
...@@ -810,8 +818,6 @@ const CSSSelector* RuleFeatureSet::ExtractInvalidationSetFeaturesFromCompound( ...@@ -810,8 +818,6 @@ const CSSSelector* RuleFeatureSet::ExtractInvalidationSetFeaturesFromCompound(
if (!simple_selector->TagHistory() || if (!simple_selector->TagHistory() ||
simple_selector->Relation() != CSSSelector::kSubSelector) { simple_selector->Relation() != CSSSelector::kSubSelector) {
features.has_features_for_rule_set_invalidation =
features.HasIdClassOrAttribute();
return simple_selector; return simple_selector;
} }
} }
...@@ -849,6 +855,8 @@ void RuleFeatureSet::AddFeaturesToInvalidationSet( ...@@ -849,6 +855,8 @@ void RuleFeatureSet::AddFeaturesToInvalidationSet(
invalidation_set.AddId(id); invalidation_set.AddId(id);
for (const auto& tag_name : features.tag_names) for (const auto& tag_name : features.tag_names)
invalidation_set.AddTagName(tag_name); invalidation_set.AddTagName(tag_name);
for (const auto& emitted_tag_name : features.emitted_tag_names)
invalidation_set.AddTagName(emitted_tag_name);
for (const auto& class_name : features.classes) for (const auto& class_name : features.classes)
invalidation_set.AddClass(class_name); invalidation_set.AddClass(class_name);
for (const auto& attribute : features.attributes) for (const auto& attribute : features.attributes)
...@@ -1384,7 +1392,22 @@ void RuleFeatureSet::InvalidationSetFeatures::Add( ...@@ -1384,7 +1392,22 @@ void RuleFeatureSet::InvalidationSetFeatures::Add(
classes.AppendVector(other.classes); classes.AppendVector(other.classes);
attributes.AppendVector(other.attributes); attributes.AppendVector(other.attributes);
ids.AppendVector(other.ids); ids.AppendVector(other.ids);
tag_names.AppendVector(other.tag_names); // Tag names that have been added to an invalidation set for an ID, a class,
// or an attribute are called "emitted" tag names. Emitted tag names need to
// go in a separate vector in order to correctly track which tag names to
// add to the type rule invalidation set.
//
// Example: :is(.a, div) :is(span, .b, ol, .c li)
//
// For the above selector, we need span and ol in the type invalidation set,
// but not li, since that tag name was added to the invalidation set for .c.
// Hence, when processing the rightmost :is(), we end up with li in the
// emitted_tag_names vector, and span and ol in the regular tag_names vector.
if (other.has_features_for_rule_set_invalidation)
emitted_tag_names.AppendVector(other.tag_names);
else
tag_names.AppendVector(other.tag_names);
emitted_tag_names.AppendVector(other.emitted_tag_names);
max_direct_adjacent_selectors = std::max(max_direct_adjacent_selectors, max_direct_adjacent_selectors = std::max(max_direct_adjacent_selectors,
other.max_direct_adjacent_selectors); other.max_direct_adjacent_selectors);
invalidation_flags.Merge(other.invalidation_flags); invalidation_flags.Merge(other.invalidation_flags);
...@@ -1404,7 +1427,8 @@ void RuleFeatureSet::InvalidationSetFeatures::NarrowToFeatures( ...@@ -1404,7 +1427,8 @@ void RuleFeatureSet::InvalidationSetFeatures::NarrowToFeatures(
bool RuleFeatureSet::InvalidationSetFeatures::HasFeatures() const { bool RuleFeatureSet::InvalidationSetFeatures::HasFeatures() const {
return !classes.IsEmpty() || !attributes.IsEmpty() || !ids.IsEmpty() || return !classes.IsEmpty() || !attributes.IsEmpty() || !ids.IsEmpty() ||
!tag_names.IsEmpty() || invalidation_flags.InvalidateCustomPseudo() || !tag_names.IsEmpty() || !emitted_tag_names.IsEmpty() ||
invalidation_flags.InvalidateCustomPseudo() ||
invalidation_flags.InvalidatesParts(); invalidation_flags.InvalidatesParts();
} }
......
...@@ -273,15 +273,18 @@ class CORE_EXPORT RuleFeatureSet { ...@@ -273,15 +273,18 @@ class CORE_EXPORT RuleFeatureSet {
attributes.clear(); attributes.clear();
ids.clear(); ids.clear();
tag_names.clear(); tag_names.clear();
emitted_tag_names.clear();
} }
unsigned Size() const { unsigned Size() const {
return classes.size() + attributes.size() + ids.size() + tag_names.size(); return classes.size() + attributes.size() + ids.size() +
tag_names.size() + emitted_tag_names.size();
} }
Vector<AtomicString> classes; Vector<AtomicString> classes;
Vector<AtomicString> attributes; Vector<AtomicString> attributes;
Vector<AtomicString> ids; Vector<AtomicString> ids;
Vector<AtomicString> tag_names; Vector<AtomicString> tag_names;
Vector<AtomicString> emitted_tag_names;
unsigned max_direct_adjacent_selectors = 0; unsigned max_direct_adjacent_selectors = 0;
InvalidationFlags invalidation_flags; InvalidationFlags invalidation_flags;
bool content_pseudo_crossing = false; bool content_pseudo_crossing = false;
......
...@@ -1512,6 +1512,13 @@ RefTestData ref_equal_test_data[] = { ...@@ -1512,6 +1512,13 @@ RefTestData ref_equal_test_data[] = {
{":is(.a + .b, .c + .d) + :is(.e + .f, .g + .h)", {":is(.a + .b, .c + .d) + :is(.e + .f, .g + .h)",
".a + .b + .f, .a + .b + .h, .c + .d + .f, .c + .d + .h," ".a + .b + .f, .a + .b + .h, .c + .d + .f, .c + .d + .h,"
".e + .f, .g + .h"}, ".e + .f, .g + .h"},
{":is(div)", "div"},
{":is(div, span)", "div, span"},
{":is(.a, div)", ".a, div"},
{":is(.a, :is(div, span))", ".a, div, span"},
{":is(.a, span) :is(div, .b)", ".a div, .a .b, span div, span .b"},
{":is(.a, span) + :is(div, .b)",
".a + div, .a + .b, span + div, span + .b"},
// clang-format on // clang-format on
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment