diff options
author | rune <rune@opera.com> | 2016-02-15 05:39:54 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2016-02-15 13:42:32 +0000 |
commit | 87e6a478af25c7976849f5bc8cf88d16cdcd635c (patch) | |
tree | de8a9ca8c87c7a80da7d53377ae08b9525d409c7 | |
parent | 2dc964d1727a9a45c94f411d959b59bcb45a33ec (diff) | |
download | chromium_src-87e6a478af25c7976849f5bc8cf88d16cdcd635c.zip chromium_src-87e6a478af25c7976849f5bc8cf88d16cdcd635c.tar.gz chromium_src-87e6a478af25c7976849f5bc8cf88d16cdcd635c.tar.bz2 |
Don't add siblingRules with combinator left of ::content.
RuleFeatureSet::siblingRules are collected because we need to skip
style sharing between elements which do not match the same set of
sibling selectors. Style sharing only happens if two elements' parent
chain of computed styles are common. That means that descendants of
their common ancestor also need to share style.
Due to this check in SharedStyleFinder::canShareStyleWithElement:
if (!sharingCandidateDistributedToSameInsertionPoint(candidate))
return false;
Elements with distributed nodes in their ancestor chain need to be
distributed to the same insertion points in order to share style. That
is, two elements which may share style match the same insertion points
for all selectors containing the ::content pseudo. Which also means
that any sibling selectors left of any ::content pseudo will match the
same sibling combinations for all elements which may share style.
Hence, we don't need to reject style sharing based on selectors like:
.a + .b ::content .c
because the sharing would already be rejected because of insertion
point mismatch in the ancestor chain.
This CL skips adding rules to RuleFeatureSet::siblingRules if we have
seen a ::content combinator before we see any sibling selectors.
We currently rejecting style sharing for slots checking
isChildOfV1ShadowHost() in Element::supportsStyleSharing(), so we could
have skipped adding any of the ::slotted rules to siblingRules. I'm not
doing that here as that might change since ::slotted is work in
progress.
If continue to skip style sharing past different slots, this means we
will only need to consider siblingRules from the same TreeScope when
for Shadow DOM V1, which means we don't need a global set of
siblingRules.
R=kochi@chromium.org,hayato@chromium.org,esprehn@chromium.org
BUG=401359
Review URL: https://codereview.chromium.org/1683923003
Cr-Commit-Position: refs/heads/master@{#375454}
-rw-r--r-- | third_party/WebKit/Source/core/css/RuleFeature.cpp | 15 | ||||
-rw-r--r-- | third_party/WebKit/Source/core/css/RuleFeature.h | 15 | ||||
-rw-r--r-- | third_party/WebKit/Source/core/css/RuleFeatureSetTest.cpp | 68 |
3 files changed, 81 insertions, 17 deletions
diff --git a/third_party/WebKit/Source/core/css/RuleFeature.cpp b/third_party/WebKit/Source/core/css/RuleFeature.cpp index d732d58..f2da0c2 100644 --- a/third_party/WebKit/Source/core/css/RuleFeature.cpp +++ b/third_party/WebKit/Source/core/css/RuleFeature.cpp @@ -587,15 +587,15 @@ void RuleFeatureSet::collectFeaturesFromSelector(const CSSSelector& selector, Ru metadata.maxDirectAdjacentSelectors = maxDirectAdjacentSelectors; maxDirectAdjacentSelectors = 0; } - if (current->isSiblingSelector()) + if (!metadata.foundInsertionPointCrossing && current->isSiblingSelector()) metadata.foundSiblingSelector = true; - const CSSSelectorList* selectorList = current->selectorList(); - if (!selectorList) - continue; - - for (const CSSSelector* subSelector = selectorList->first(); subSelector; subSelector = CSSSelectorList::next(*subSelector)) - collectFeaturesFromSelector(*subSelector, metadata); + if (const CSSSelectorList* selectorList = current->selectorList()) { + for (const CSSSelector* subSelector = selectorList->first(); subSelector; subSelector = CSSSelectorList::next(*subSelector)) + collectFeaturesFromSelector(*subSelector, metadata); + } + if (current->relationIsAffectedByPseudoContent()) + metadata.foundInsertionPointCrossing = true; } ASSERT(!maxDirectAdjacentSelectors); @@ -613,6 +613,7 @@ void RuleFeatureSet::FeatureMetadata::clear() usesFirstLineRules = false; usesWindowInactiveSelector = false; foundSiblingSelector = false; + foundInsertionPointCrossing = false; maxDirectAdjacentSelectors = 0; } diff --git a/third_party/WebKit/Source/core/css/RuleFeature.h b/third_party/WebKit/Source/core/css/RuleFeature.h index 60a023f..609466e 100644 --- a/third_party/WebKit/Source/core/css/RuleFeature.h +++ b/third_party/WebKit/Source/core/css/RuleFeature.h @@ -108,19 +108,14 @@ private: struct FeatureMetadata { DISALLOW_NEW(); - FeatureMetadata() - : usesFirstLineRules(false) - , usesWindowInactiveSelector(false) - , foundSiblingSelector(false) - , maxDirectAdjacentSelectors(0) - { } void add(const FeatureMetadata& other); void clear(); - bool usesFirstLineRules; - bool usesWindowInactiveSelector; - bool foundSiblingSelector; - unsigned maxDirectAdjacentSelectors; + bool usesFirstLineRules = false; + bool usesWindowInactiveSelector = false; + bool foundSiblingSelector = false; + bool foundInsertionPointCrossing = false; + unsigned maxDirectAdjacentSelectors = 0; }; void collectFeaturesFromSelector(const CSSSelector&, FeatureMetadata&); diff --git a/third_party/WebKit/Source/core/css/RuleFeatureSetTest.cpp b/third_party/WebKit/Source/core/css/RuleFeatureSetTest.cpp index 95e7c0a..64168dc 100644 --- a/third_party/WebKit/Source/core/css/RuleFeatureSetTest.cpp +++ b/third_party/WebKit/Source/core/css/RuleFeatureSetTest.cpp @@ -44,6 +44,15 @@ public: m_ruleFeatureSet.updateInvalidationSets(ruleData); } + void collectFeatures(const String& selectorText) + { + CSSSelectorList selectorList = CSSParser::parseSelector(strictCSSParserContext(), nullptr, selectorText); + + RefPtrWillBeRawPtr<StyleRule> styleRule = StyleRule::create(std::move(selectorList), MutableStylePropertySet::create(HTMLStandardMode)); + RuleData ruleData(styleRule.get(), 0, 0, RuleHasNoSpecialState); + m_ruleFeatureSet.collectFeaturesFromRuleData(ruleData); + } + void collectInvalidationSetsForClass(InvalidationLists& invalidationLists, const AtomicString& className) const { Element* element = Traversal<HTMLElement>::firstChild(*Traversal<HTMLElement>::firstChild(*m_document->body())); @@ -170,6 +179,11 @@ public: EXPECT_TRUE(attributes.contains(attribute)); } + void expectSiblingRuleCount(unsigned count) + { + EXPECT_EQ(count, m_ruleFeatureSet.siblingRules.size()); + } + DEFINE_INLINE_TRACE() { #if ENABLE(OILPAN) @@ -337,4 +351,58 @@ TEST_F(RuleFeatureSetTest, contentPseudo) expectClassesInvalidation("b", "c", invalidationLists.descendants); } +TEST_F(RuleFeatureSetTest, siblingRulesBeforeContentPseudo) +{ + collectFeatures("a + b ::content .c"); + expectSiblingRuleCount(0); +} + +TEST_F(RuleFeatureSetTest, siblingRulesAfterContentPseudo) +{ + collectFeatures(".a ::content .b + .c"); + expectSiblingRuleCount(1); +} + +TEST_F(RuleFeatureSetTest, siblingRulesNthBeforeContentPseudo) +{ + collectFeatures(":nth-child(2) ::content .a"); + expectSiblingRuleCount(0); +} + +TEST_F(RuleFeatureSetTest, siblingRulesNthAfterContentPseudo) +{ + collectFeatures(".a ::content :nth-child(2)"); + expectSiblingRuleCount(1); +} + +TEST_F(RuleFeatureSetTest, siblingRulesBeforeDeep) +{ + collectFeatures("a + b /deep/ .c"); + expectSiblingRuleCount(1); +} + +TEST_F(RuleFeatureSetTest, siblingRulesAfterDeep) +{ + collectFeatures(".a /deep/ .b + .c"); + expectSiblingRuleCount(1); +} + +TEST_F(RuleFeatureSetTest, siblingRulesBeforeShadow) +{ + collectFeatures(".a + .b::shadow .c"); + expectSiblingRuleCount(1); +} + +TEST_F(RuleFeatureSetTest, siblingRulesAfterShadow) +{ + collectFeatures(".a ::shadow .b + .c"); + expectSiblingRuleCount(1); +} + +TEST_F(RuleFeatureSetTest, siblingRulesBeforeSlotted) +{ + collectFeatures(".a + ::slotted(.b)"); + expectSiblingRuleCount(1); +} + } // namespace blink |