Commit 1cec715d authored by Anders Hartvoll Ruud's avatar Anders Hartvoll Ruud Committed by Commit Bot

[:is/:where] Don't treat :is() as a universal sibling

When adding features to a simple selector, we are currently calling
IsIdClassOrAttributeSelector() to determine if we should add features
to the universal sibling set or not. This means selectors like
':is(.a) + .b' will result in a universal sibling set containing
'.b', since ':is(.a)' is not an ID/class/attribute.

This CL instead detects whether an ID/class/attributes exist inside
the nested selector list, thus avoiding the addition to the universal
sibling set.

Note that this affects -webkit-any() as well.

Bug: 568705
Change-Id: I7bba857a3f24e9789f5517617e4201079d7350ef
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2435089
Commit-Queue: Anders Hartvoll Ruud <andruud@chromium.org>
Reviewed-by: default avatarRune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#812593}
parent 93fc62b4
...@@ -895,6 +895,9 @@ void RuleFeatureSet::AddFeaturesToInvalidationSetsForSimpleSelector( ...@@ -895,6 +895,9 @@ void RuleFeatureSet::AddFeaturesToInvalidationSetsForSimpleSelector(
const CSSSelector& simple_selector, const CSSSelector& simple_selector,
InvalidationSetFeatures* sibling_features, InvalidationSetFeatures* sibling_features,
InvalidationSetFeatures& descendant_features) { InvalidationSetFeatures& descendant_features) {
if (simple_selector.IsIdClassOrAttributeSelector())
descendant_features.has_features_for_rule_set_invalidation = true;
if (InvalidationSet* invalidation_set = InvalidationSetForSimpleSelector( if (InvalidationSet* invalidation_set = InvalidationSetForSimpleSelector(
simple_selector, simple_selector,
sibling_features ? InvalidationType::kInvalidateSiblings sibling_features ? InvalidationType::kInvalidateSiblings
...@@ -947,20 +950,22 @@ RuleFeatureSet::AddFeaturesToInvalidationSetsForCompoundSelector( ...@@ -947,20 +950,22 @@ RuleFeatureSet::AddFeaturesToInvalidationSetsForCompoundSelector(
const CSSSelector& compound, const CSSSelector& compound,
InvalidationSetFeatures* sibling_features, InvalidationSetFeatures* sibling_features,
InvalidationSetFeatures& descendant_features) { InvalidationSetFeatures& descendant_features) {
bool compound_has_id_class_or_attribute = false; bool compound_has_features_for_rule_set_invalidation = false;
const CSSSelector* simple_selector = &compound; const CSSSelector* simple_selector = &compound;
for (; simple_selector; simple_selector = simple_selector->TagHistory()) { for (; simple_selector; simple_selector = simple_selector->TagHistory()) {
base::AutoReset<bool> reset_has_features(
&descendant_features.has_features_for_rule_set_invalidation, false);
AddFeaturesToInvalidationSetsForSimpleSelector( AddFeaturesToInvalidationSetsForSimpleSelector(
*simple_selector, sibling_features, descendant_features); *simple_selector, sibling_features, descendant_features);
if (simple_selector->IsIdClassOrAttributeSelector()) if (descendant_features.has_features_for_rule_set_invalidation)
compound_has_id_class_or_attribute = true; compound_has_features_for_rule_set_invalidation = true;
if (simple_selector->Relation() != CSSSelector::kSubSelector) if (simple_selector->Relation() != CSSSelector::kSubSelector)
break; break;
if (!simple_selector->TagHistory()) if (!simple_selector->TagHistory())
break; break;
} }
if (compound_has_id_class_or_attribute) { if (compound_has_features_for_rule_set_invalidation) {
descendant_features.has_features_for_rule_set_invalidation = true; descendant_features.has_features_for_rule_set_invalidation = true;
} else if (sibling_features) { } else if (sibling_features) {
AddFeaturesToUniversalSiblingInvalidationSet(*sibling_features, AddFeaturesToUniversalSiblingInvalidationSet(*sibling_features,
......
...@@ -736,28 +736,6 @@ TEST_F(RuleFeatureSetTest, nonUniversalSiblingInvalidationNot) { ...@@ -736,28 +736,6 @@ TEST_F(RuleFeatureSetTest, nonUniversalSiblingInvalidationNot) {
ExpectNoInvalidation(invalidation_lists.siblings); ExpectNoInvalidation(invalidation_lists.siblings);
} }
TEST_F(RuleFeatureSetTest, universalSiblingInvalidationAny) {
EXPECT_EQ(RuleFeatureSet::kSelectorMayMatch,
CollectFeatures(":-webkit-any(.a) + .b"));
InvalidationLists invalidation_lists;
CollectUniversalSiblingInvalidationSet(invalidation_lists);
ExpectSiblingClassInvalidation(1, "b", invalidation_lists.siblings);
ExpectSelfInvalidation(invalidation_lists.siblings);
}
TEST_F(RuleFeatureSetTest, universalSiblingIdInvalidationAny) {
EXPECT_EQ(RuleFeatureSet::kSelectorMayMatch,
CollectFeatures(":-webkit-any(.a) + #b"));
InvalidationLists invalidation_lists;
CollectUniversalSiblingInvalidationSet(invalidation_lists);
ExpectSiblingIdInvalidation(1, "b", invalidation_lists.siblings);
ExpectSelfInvalidation(invalidation_lists.siblings);
}
TEST_F(RuleFeatureSetTest, nonUniversalSiblingInvalidationAny) { TEST_F(RuleFeatureSetTest, nonUniversalSiblingInvalidationAny) {
EXPECT_EQ(RuleFeatureSet::kSelectorMayMatch, EXPECT_EQ(RuleFeatureSet::kSelectorMayMatch,
CollectFeatures("#x:-webkit-any(.a) + .b")); CollectFeatures("#x:-webkit-any(.a) + .b"));
...@@ -1528,15 +1506,12 @@ RefTestData ref_equal_test_data[] = { ...@@ -1528,15 +1506,12 @@ RefTestData ref_equal_test_data[] = {
{":is(.a + .b, .c + .d) .e", ".a + .b .e, .c + .d .e"}, {":is(.a + .b, .c + .d) .e", ".a + .b .e, .c + .d .e"},
{":is(.a ~ .b, .c + .e + .f) :is(.c .d, .e)", {":is(.a ~ .b, .c + .e + .f) :is(.c .d, .e)",
".a ~ .b .d, .a ~ .b .e, .c + .e + .f .d, .c + .e + .f .e, .c .d"}, ".a ~ .b .d, .a ~ .b .e, .c + .e + .f .d, .c + .e + .f .e, .c .d"},
// TODO(andruud): Don't treat :is() as a universal sibling. {":is(.a) + .b", ".a + .b"},
// The following tests add "div + <foo>" selectors in the refs {":is(.a, .b) + .c", ".a + .c, .b + .c"},
// to populate the universal sibling set with the expected data. {":is(.a + .b, .c + .d) + .e", ".a + .b + .e, .c + .d + .e"},
{":is(.a) + .b", ".a + .b, div + .b"},
{":is(.a, .b) + .c", ".a + .c, .b + .c, div + .c"},
{":is(.a + .b, .c + .d) + .e", ".a + .b + .e, .c + .d + .e, div + .e"},
{":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, div + .f, div + .h"}, ".e + .f, .g + .h"},
// 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