Implement Shadow DOM support for mixins. Similar to how we have a separate pass to extract mixins from a TreeScope before creating the RuleSets, we now have a separate pass to extract mixins from _all_ relevant TreeScopes before creating any RuleSets. This allows us to implement mixin inheritance, where we pull in mixins from parent TreeScopes to the set of effective mixins, allowing a stylesheet in Shadow DOM to use a mixin from its parent. We still do not support invalidation of mixins, so if you modify a mixin in a parent TreeScope, we will not mark any child (shadow) TreeScope as dirty, and the RuleSets will not be properly updated. Change-Id: I940dbd982724cad6821013745ca1897d31311ae7 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6828630 Reviewed-by: Anders Hartvoll Ruud <andruud@chromium.org> Commit-Queue: Steinar H Gunderson <sesse@chromium.org> Cr-Commit-Position: refs/heads/main@{#1500720}
diff --git a/third_party/blink/renderer/core/css/resolver/style_cascade_test.cc b/third_party/blink/renderer/core/css/resolver/style_cascade_test.cc index 24e3ba7..92504b8 100644 --- a/third_party/blink/renderer/core/css/resolver/style_cascade_test.cc +++ b/third_party/blink/renderer/core/css/resolver/style_cascade_test.cc
@@ -360,11 +360,11 @@ tree_scope.EnsureScopedStyleResolver(); ActiveStyleSheetVector active_sheets{std::make_pair(sheet, nullptr)}; scoped_resolver.AppendActiveStyleSheets(0, active_sheets); - GetDocument() - .GetStyleEngine() - .GetDocumentStyleSheetCollection() - .ReplaceActiveStyleSheets(MediaQueryEvaluator(GetDocument().GetFrame()), - active_sheets); + StyleSheetCollection& collection = + GetDocument().GetStyleEngine().GetDocumentStyleSheetCollection(); + collection.AddPendingActiveStyleSheetForTest(sheet); + collection.FinishUpdateActiveStyleSheets( + MediaQueryEvaluator(GetDocument().GetFrame()), /*effective_mixins=*/{}); } Element* DocumentElement() const { return GetDocument().documentElement(); }
diff --git a/third_party/blink/renderer/core/css/style_engine.cc b/third_party/blink/renderer/core/css/style_engine.cc index 5bfc51a..05af6d804 100644 --- a/third_party/blink/renderer/core/css/style_engine.cc +++ b/third_party/blink/renderer/core/css/style_engine.cc
@@ -634,20 +634,14 @@ } } -void StyleEngine::UpdateActiveStyleSheetsInShadow( +void StyleEngine::PrepareUpdateActiveStyleSheetsInShadow( TreeScope* tree_scope, - UnorderedTreeScopeSet& tree_scopes_removed) { + UnorderedTreeScopeSet& tree_scopes_removed, + const MediaQueryEvaluator& medium) { DCHECK_NE(tree_scope, document_); auto* collection = StyleSheetCollectionFor(*tree_scope); DCHECK(collection); - collection->UpdateActiveStyleSheets(*this, EnsureMediaQueryEvaluator()); - if (!collection->HasStyleSheetCandidateNodes() && - !tree_scope->HasAdoptedStyleSheets()) { - tree_scopes_removed.insert(tree_scope); - // When removing TreeScope from ActiveTreeScopes, - // its resolver should be destroyed by invoking resetAuthorStyle. - DCHECK(!tree_scope->GetScopedStyleResolver()); - } + collection->PrepareUpdateActiveStyleSheets(medium); } void StyleEngine::UpdateActiveUserStyleSheets() { @@ -679,18 +673,45 @@ UpdateActiveUserStyleSheets(); } - if (ShouldUpdateDocumentStyleSheetCollection()) { - GetDocumentStyleSheetCollection().UpdateActiveStyleSheets( - *this, EnsureMediaQueryEvaluator()); - } + const MediaQueryEvaluator& medium = EnsureMediaQueryEvaluator(); + // Prepare the stylesheet collections for update. This collects all the + // stylesheets from the nodes in question and extracts mixins. + // + // Note that if mixins in a parent changes, we should invalidate all children + // (in addition to the parent itself), or at least all mixin-using children, + // but we do not do invalidation of mixins in general yet. + if (ShouldUpdateDocumentStyleSheetCollection()) { + document_style_sheet_collection_->PrepareUpdateActiveStyleSheets(medium); + } if (ShouldUpdateShadowTreeStyleSheetCollection()) { UnorderedTreeScopeSet tree_scopes_removed; for (TreeScope* tree_scope : dirty_tree_scopes_) { - UpdateActiveStyleSheetsInShadow(tree_scope, tree_scopes_removed); + PrepareUpdateActiveStyleSheetsInShadow(tree_scope, tree_scopes_removed, + medium); } - for (TreeScope* tree_scope : tree_scopes_removed) { - active_tree_scopes_.erase(tree_scope); + } + + // Now create the actual RuleSets. + if (ShouldUpdateDocumentStyleSheetCollection()) { + document_style_sheet_collection_->FinishUpdateActiveStyleSheets( + medium, document_style_sheet_collection_->Mixins()); + } + if (ShouldUpdateShadowTreeStyleSheetCollection()) { + for (TreeScope* tree_scope : dirty_tree_scopes_) { + StyleSheetCollection& collection = *StyleSheetCollectionFor(*tree_scope); + collection.FinishUpdateActiveStyleSheets( + EnsureMediaQueryEvaluator(), + EffectiveMixinsForTreeScope(*tree_scope)); + if (!collection.HasStyleSheetCandidateNodes() && + !tree_scope->HasAdoptedStyleSheets()) { + active_tree_scopes_.erase(tree_scope); + // When removing TreeScope from ActiveTreeScopes, + // its resolver should have been destroyed by + // invoking ResetAuthorStyle() (in particular, + // ShadowRootRemovedFromDocument() does this). + DCHECK(!tree_scope->GetScopedStyleResolver()); + } } } @@ -702,6 +723,34 @@ user_style_dirty_ = false; } +MixinMap StyleEngine::EffectiveMixinsForTreeScope(TreeScope& tree_scope) { + TreeScope* parent_scope = tree_scope.ParentTreeScope(); + + StyleSheetCollection* collection = StyleSheetCollectionFor(tree_scope); + if (!collection) { + // If there's no collection, there are also no style sheets. + if (parent_scope) { + return EffectiveMixinsForTreeScope(*parent_scope); + } else { + return {}; + } + } + + if (!parent_scope) { + return collection->Mixins(); + } + + MixinMap inherited_mixins = EffectiveMixinsForTreeScope(*parent_scope); + if (inherited_mixins.empty()) { + return collection->Mixins(); + } + + for (const auto& [name, value] : collection->Mixins()) { + inherited_mixins.insert(name, value); + } + return inherited_mixins; +} + void StyleEngine::UpdateCounterStyles() { if (!counter_styles_need_update_) { return;
diff --git a/third_party/blink/renderer/core/css/style_engine.h b/third_party/blink/renderer/core/css/style_engine.h index 5d5e531..b7eba8f 100644 --- a/third_party/blink/renderer/core/css/style_engine.h +++ b/third_party/blink/renderer/core/css/style_engine.h
@@ -830,9 +830,10 @@ return *document_style_sheet_collection_; } - void UpdateActiveStyleSheetsInShadow( + void PrepareUpdateActiveStyleSheetsInShadow( TreeScope*, - UnorderedTreeScopeSet& tree_scopes_removed); + UnorderedTreeScopeSet& tree_scopes_removed, + const MediaQueryEvaluator& medium); bool ShouldSkipInvalidationFor(const Element&) const; bool IsSubtreeAndSiblingsStyleDirty(const Element&) const; @@ -956,6 +957,8 @@ // See EvaluateFunctionalMediaQuery void InvalidateFunctionalMediaDependentStylesIfNeeded(); + MixinMap EffectiveMixinsForTreeScope(TreeScope& tree_scope); + Member<Document> document_; // Tree of style containment scopes. Is in charge of the document's quotes.
diff --git a/third_party/blink/renderer/core/css/style_sheet_collection.cc b/third_party/blink/renderer/core/css/style_sheet_collection.cc index 61619b1b..7c6928e6 100644 --- a/third_party/blink/renderer/core/css/style_sheet_collection.cc +++ b/third_party/blink/renderer/core/css/style_sheet_collection.cc
@@ -42,21 +42,23 @@ static void CreateRuleSets(const StyleEngine& engine, const MediaQueryEvaluator& medium, + const MixinMap& effective_mixins, ActiveStyleSheetVector& active_style_sheets, HeapVector<Member<RuleSetDiff>>& rule_set_diffs); -void StyleSheetCollection::ReplaceActiveStyleSheets( +void StyleSheetCollection::FinishUpdateActiveStyleSheets( const MediaQueryEvaluator& medium, - ActiveStyleSheetVector new_active_style_sheets) { + const MixinMap& effective_mixins) { HeapVector<Member<RuleSetDiff>> rule_set_diffs; - CreateRuleSets(GetDocument().GetStyleEngine(), medium, - new_active_style_sheets, rule_set_diffs); + CreateRuleSets(GetDocument().GetStyleEngine(), medium, effective_mixins, + pending_active_style_sheets_, rule_set_diffs); GetDocument().GetStyleEngine().ApplyRuleSetChanges( - *tree_scope_, active_style_sheets_, new_active_style_sheets, + *tree_scope_, active_style_sheets_, pending_active_style_sheets_, rule_set_diffs); - active_style_sheets_ = std::move(new_active_style_sheets); + active_style_sheets_ = std::move(pending_active_style_sheets_); + pending_active_style_sheets_.clear(); } // FIXME(sesse): Store this somewhere (including the two-level Eval() form), @@ -98,13 +100,9 @@ // Can only be called once. static void CreateRuleSets(const StyleEngine& engine, const MediaQueryEvaluator& medium, + const MixinMap& effective_mixins, ActiveStyleSheetVector& active_style_sheets, HeapVector<Member<RuleSetDiff>>& rule_set_diffs) { - MixinMap mixins; - for (auto& [css_sheet, rule_set] : active_style_sheets) { - ExtractMixinsFromRules(css_sheet->Contents()->ChildRules(), medium, mixins); - } - // Keep track of ensured RuleSets with @layer rules to detect // StyleSheetContents sharing; RuleSets should not be shared // between two equal sheets with @layer rules, since anonymous @@ -113,7 +111,7 @@ for (auto& [css_sheet, rule_set] : active_style_sheets) { CHECK_EQ(rule_set, nullptr); - rule_set = engine.RuleSetForSheet(*css_sheet, mixins); + rule_set = engine.RuleSetForSheet(*css_sheet, effective_mixins); // NOTE: If the user has specified the same CSSStyleSheet object multiple // times (which is only possible for constructible stylesheets, in @@ -138,7 +136,7 @@ // // TODO(sesse): Can we detect this before creating the RuleSet? css_sheet->WillMutateRules(); - rule_set = engine.RuleSetForSheet(*css_sheet, mixins); + rule_set = engine.RuleSetForSheet(*css_sheet, effective_mixins); } if (css_sheet->Contents()->GetRuleSetDiff()) { @@ -150,9 +148,11 @@ void StyleSheetCollection::Trace(Visitor* visitor) const { visitor->Trace(active_style_sheets_); + visitor->Trace(pending_active_style_sheets_); visitor->Trace(style_sheets_for_style_sheet_list_); visitor->Trace(tree_scope_); visitor->Trace(style_sheet_candidate_nodes_); + visitor->Trace(mixins_); } StyleSheetCollection::StyleSheetCollection(TreeScope& tree_scope) @@ -191,8 +191,7 @@ sheet_list_dirty_ = false; } -void StyleSheetCollection::UpdateActiveStyleSheets( - const StyleEngine& engine, +void StyleSheetCollection::PrepareUpdateActiveStyleSheets( const MediaQueryEvaluator& medium) { ActiveStyleSheetVector new_active_style_sheets; const String& preferred_name = @@ -238,7 +237,14 @@ } } - ReplaceActiveStyleSheets(medium, std::move(new_active_style_sheets)); + mixins_.clear(); + for (auto& [css_sheet, rule_set] : new_active_style_sheets) { + ExtractMixinsFromRules(css_sheet->Contents()->ChildRules(), medium, + mixins_); + } + + DCHECK(pending_active_style_sheets_.empty()); + pending_active_style_sheets_ = std::move(new_active_style_sheets); } } // namespace blink
diff --git a/third_party/blink/renderer/core/css/style_sheet_collection.h b/third_party/blink/renderer/core/css/style_sheet_collection.h index 3c5b061..1a36dec 100644 --- a/third_party/blink/renderer/core/css/style_sheet_collection.h +++ b/third_party/blink/renderer/core/css/style_sheet_collection.h
@@ -45,7 +45,6 @@ class MediaQueryEvaluator; class Node; class StyleSheet; -class StyleEngine; // StyleSheetCollection is responsible for keeping track of which style sheets // are relevant for a given tree scope. Style sheets may be relevant for either @@ -104,16 +103,21 @@ bool IsShadowTreeStyleSheetCollection() const { return is_shadow_tree_; } void UpdateStyleSheetList(); - void UpdateActiveStyleSheets(const StyleEngine&, const MediaQueryEvaluator&); + + const MixinMap& Mixins() const { return mixins_; } + + // Updates mixins_ but not active_style_sheets_. + void PrepareUpdateActiveStyleSheets(const MediaQueryEvaluator&); + + // Must be called once, after all PrepareUpdateActiveStyleSheets(). + void FinishUpdateActiveStyleSheets(const MediaQueryEvaluator&, + const MixinMap& effective_mixins); private: - friend class StyleCascadeTest; // For ReplaceActiveStyleSheets(). - - // Called after collecting a new set of active style sheets. - // Creates RuleSets, notifies the StyleEngine and moves the new values into - // place. - void ReplaceActiveStyleSheets(const MediaQueryEvaluator& medium, - ActiveStyleSheetVector new_active_style_sheets); + friend class StyleCascadeTest; + void AddPendingActiveStyleSheetForTest(CSSStyleSheet* sheet) { + pending_active_style_sheets_.push_back(std::pair(sheet, nullptr)); + } Document& GetDocument() const { return tree_scope_->GetDocument(); } @@ -121,6 +125,8 @@ HeapVector<Member<StyleSheet>> style_sheets_for_style_sheet_list_; TreeOrderedList<Node> style_sheet_candidate_nodes_; ActiveStyleSheetVector active_style_sheets_; + ActiveStyleSheetVector pending_active_style_sheets_; + MixinMap mixins_; bool sheet_list_dirty_ = true; const bool is_shadow_tree_; };
diff --git a/third_party/blink/web_tests/external/wpt/css/css-mixins/shadow-dom-expected.txt b/third_party/blink/web_tests/external/wpt/css/css-mixins/shadow-dom-expected.txt deleted file mode 100644 index 996922f..0000000 --- a/third_party/blink/web_tests/external/wpt/css/css-mixins/shadow-dom-expected.txt +++ /dev/null
@@ -1,5 +0,0 @@ -This is a testharness.js-based test. -[FAIL] Style in shadow DOM should have access to outside non-adopted mixins - assert_equals: expected "rgb(0, 128, 0)" but got "rgb(255, 0, 0)" -Harness: the test ran to completion. -