commit b0833eed7d0c7691e30d6693dde945969a0a4af1 parent d5db0cc63b58fa9a476f12cbe0244eb4bcb22bf9 Author: Emilio Cobos Álvarez <emilio@crisal.io> Date: Tue, 6 Jan 2026 15:09:37 +0000 Bug 2008650 - Notify and invalidate on @position-try declaration changes. r=firefox-style-system-reviewers,layout-anchor-positioning-reviewers,layout-reviewers,dshin This builds on top of bug 1910616. I decided to bubble up the invalidation sets and keep the cascade data difference in StylesheetInvalidationSet because it felt clearer (it always was a bit of a wart that we had to pass the root and snapshot map down to flush). Now the caller is responsible to flush of course, but I think it's nicer, specially now that we also merged the shadow dom invalidation there. Eventually, it might be worth moving most of the existing sets into the `CascadeDataDifference`, to be able to track stuff more accurately... Maybe we can just diff the ElementAndPseudoRules maps? Though given some pages have hundreds of thousands of rules that might be too much, not sure. Anyways, one step at a time. Differential Revision: https://phabricator.services.mozilla.com/D277951 Diffstat:
14 files changed, 186 insertions(+), 242 deletions(-)
diff --git a/layout/inspector/ServoStyleRuleMap.cpp b/layout/inspector/ServoStyleRuleMap.cpp @@ -13,6 +13,7 @@ #include "mozilla/css/GroupRule.h" #include "mozilla/dom/CSSImportRule.h" #include "mozilla/dom/CSSNestedDeclarations.h" +#include "mozilla/dom/CSSPositionTryRule.h" #include "mozilla/dom/CSSRuleBinding.h" #include "mozilla/dom/CSSStyleRule.h" #include "mozilla/dom/Document.h" @@ -140,6 +141,11 @@ void ServoStyleRuleMap::FillTableFromRule(css::Rule& aRule) { mTable.InsertOrUpdate(rule.RawStyle(), &rule); break; } + case StyleCssRuleType::PositionTry: { + auto& rule = static_cast<CSSPositionTryRule&>(aRule); + mTable.InsertOrUpdate(rule.RawStyle(), &rule); + break; + } case StyleCssRuleType::Style: { auto& rule = static_cast<CSSStyleRule&>(aRule); mTable.InsertOrUpdate(rule.RawStyle(), &rule); @@ -175,7 +181,6 @@ void ServoStyleRuleMap::FillTableFromRule(css::Rule& aRule) { case StyleCssRuleType::CounterStyle: case StyleCssRuleType::FontFeatureValues: case StyleCssRuleType::FontPaletteValues: - case StyleCssRuleType::PositionTry: break; } } diff --git a/layout/style/CSSNestedDeclarations.cpp b/layout/style/CSSNestedDeclarations.cpp @@ -64,14 +64,14 @@ nsresult CSSNestedDeclarationsDeclaration::SetCSSDeclaration( DeclarationBlock* aDecl, MutationClosureData* aClosureData) { CSSNestedDeclarations* rule = Rule(); RefPtr<DeclarationBlock> oldDecls; + if (aDecl != mDecls) { + oldDecls = std::move(mDecls); + oldDecls->SetOwningRule(nullptr); + Servo_NestedDeclarationsRule_SetStyle(rule->Raw(), aDecl->Raw()); + mDecls = aDecl; + mDecls->SetOwningRule(rule); + } if (StyleSheet* sheet = rule->GetStyleSheet()) { - if (aDecl != mDecls) { - oldDecls = std::move(mDecls); - oldDecls->SetOwningRule(nullptr); - Servo_NestedDeclarationsRule_SetStyle(rule->Raw(), aDecl->Raw()); - mDecls = aDecl; - mDecls->SetOwningRule(rule); - } sheet->RuleChanged(rule, {StyleRuleChangeKind::StyleRuleDeclarations, oldDecls ? oldDecls.get() : aDecl, aDecl}); } diff --git a/layout/style/CSSPositionTryRule.cpp b/layout/style/CSSPositionTryRule.cpp @@ -82,7 +82,7 @@ nsresult CSSPositionTryRuleDeclaration::SetCSSDeclaration( DeclarationBlock* aDecl, MutationClosureData* aClosureData) { MOZ_ASSERT(aDecl, "must be non-null"); CSSPositionTryRule* rule = Rule(); - + RefPtr<DeclarationBlock> oldDecls; if (aDecl != mDecls) { mDecls->SetOwningRule(nullptr); Servo_PositionTryRule_SetStyle(rule->Raw(), aDecl->Raw()); @@ -90,6 +90,11 @@ nsresult CSSPositionTryRuleDeclaration::SetCSSDeclaration( mDecls->SetOwningRule(rule); } + if (StyleSheet* sheet = rule->GetStyleSheet()) { + sheet->RuleChanged(rule, {StyleRuleChangeKind::PositionTryDeclarations, + oldDecls ? oldDecls.get() : aDecl, aDecl}); + } + return NS_OK; } @@ -186,4 +191,8 @@ JSObject* CSSPositionTryRule::WrapObject(JSContext* aCx, return CSSPositionTryRule_Binding::Wrap(aCx, this, aGivenProto); } +const StyleLockedDeclarationBlock* CSSPositionTryRule::RawStyle() const { + return mDecls.mDecls->Raw(); +} + } // namespace mozilla::dom diff --git a/layout/style/CSSPositionTryRule.h b/layout/style/CSSPositionTryRule.h @@ -84,6 +84,8 @@ class CSSPositionTryRule final : public css::Rule { JSObject* WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto) final; + const StyleLockedDeclarationBlock* RawStyle() const; + private: ~CSSPositionTryRule() = default; diff --git a/layout/style/CSSStyleRule.cpp b/layout/style/CSSStyleRule.cpp @@ -89,14 +89,14 @@ nsresult CSSStyleRuleDeclaration::SetCSSDeclaration( DeclarationBlock* aDecl, MutationClosureData* aClosureData) { CSSStyleRule* rule = Rule(); RefPtr<DeclarationBlock> oldDecls; + if (aDecl != mDecls) { + oldDecls = std::move(mDecls); + oldDecls->SetOwningRule(nullptr); + Servo_StyleRule_SetStyle(rule->Raw(), aDecl->Raw()); + mDecls = aDecl; + mDecls->SetOwningRule(rule); + } if (StyleSheet* sheet = rule->GetStyleSheet()) { - if (aDecl != mDecls) { - oldDecls = std::move(mDecls); - oldDecls->SetOwningRule(nullptr); - Servo_StyleRule_SetStyle(rule->Raw(), aDecl->Raw()); - mDecls = aDecl; - mDecls->SetOwningRule(rule); - } sheet->RuleChanged(rule, {StyleRuleChangeKind::StyleRuleDeclarations, oldDecls ? oldDecls.get() : aDecl, aDecl}); } diff --git a/layout/style/StyleSheet.h b/layout/style/StyleSheet.h @@ -46,7 +46,7 @@ enum class StyleNonLocalUriDependency : uint8_t; struct StyleRuleChange { StyleRuleChange() = delete; MOZ_IMPLICIT StyleRuleChange(StyleRuleChangeKind aKind) : mKind(aKind) {} - // Only relevant for Kind::StyleRuleDeclarations. + // Only relevant for Kind::*Declarations. StyleRuleChange(StyleRuleChangeKind aKind, const DeclarationBlock* aOldBlock, const DeclarationBlock* aNewBlock) : mKind(aKind), mOldBlock(aOldBlock), mNewBlock(aNewBlock) {} diff --git a/servo/components/style/author_styles.rs b/servo/components/style/author_styles.rs @@ -6,12 +6,11 @@ //! ones used for ShadowRoot. use crate::derives::*; -use crate::dom::TElement; +use crate::invalidation::stylesheets::StylesheetInvalidationSet; use crate::shared_lock::SharedRwLockReadGuard; use crate::stylesheet_set::AuthorStylesheetSet; use crate::stylesheets::StylesheetInDocument; use crate::stylist::CascadeData; -use crate::stylist::CascadeDataDifference; use crate::stylist::Stylist; use servo_arc::Arc; use std::sync::LazyLock; @@ -50,28 +49,22 @@ where } /// Flush the pending sheet changes, updating `data` as appropriate. - /// - /// TODO(emilio): Need a host element and a snapshot map to do invalidation - /// properly. #[inline] - pub fn flush<E>( + pub fn flush( &mut self, stylist: &mut Stylist, guard: &SharedRwLockReadGuard, - ) -> CascadeDataDifference - where - E: TElement, - { - let flusher = self - .stylesheets - .flush::<E>(/* host = */ None, /* snapshot_map = */ None); - - let mut difference = CascadeDataDifference::default(); - let result = - stylist.rebuild_author_data(&self.data, flusher.sheets, guard, &mut difference); + ) -> StylesheetInvalidationSet { + let (flusher, mut invalidations) = self.stylesheets.flush(); + let result = stylist.rebuild_author_data( + &self.data, + flusher.sheets, + guard, + &mut invalidations.cascade_data_difference, + ); if let Ok(Some(new_data)) = result { self.data = new_data; } - difference + invalidations } } diff --git a/servo/components/style/gecko/data.rs b/servo/components/style/gecko/data.rs @@ -5,18 +5,17 @@ //! Data needed to style a Gecko document. use crate::derives::*; -use crate::dom::TElement; use crate::gecko_bindings::bindings; use crate::gecko_bindings::structs::{ self, ServoStyleSetSizes, StyleSheet as DomStyleSheet, StyleSheetInfo, }; +use crate::invalidation::stylesheets::StylesheetInvalidationSet; use crate::media_queries::{Device, MediaList}; use crate::properties::ComputedValues; -use crate::selector_parser::SnapshotMap; use crate::shared_lock::{SharedRwLockReadGuard, StylesheetGuards}; use crate::stylesheets::scope_rule::ImplicitScopeRoot; use crate::stylesheets::{StylesheetContents, StylesheetInDocument}; -use crate::stylist::{DocumentFlushResult, Stylist}; +use crate::stylist::Stylist; use atomic_refcell::{AtomicRef, AtomicRefCell, AtomicRefMut}; use malloc_size_of::MallocSizeOfOps; use selectors::Element; @@ -198,17 +197,11 @@ impl PerDocumentStyleData { impl PerDocumentStyleDataImpl { /// Recreate the style data if the stylesheets have changed. - pub fn flush_stylesheets<E>( + pub fn flush_stylesheets( &mut self, guard: &SharedRwLockReadGuard, - document_element: Option<E>, - snapshots: Option<&SnapshotMap>, - ) -> DocumentFlushResult - where - E: TElement, - { - self.stylist - .flush(&StylesheetGuards::same(guard), document_element, snapshots) + ) -> StylesheetInvalidationSet { + self.stylist.flush(&StylesheetGuards::same(guard)) } /// Get the default computed values for this document. diff --git a/servo/components/style/invalidation/stylesheets.rs b/servo/components/style/invalidation/stylesheets.rs @@ -18,8 +18,11 @@ use crate::selector_map::PrecomputedHashSet; use crate::selector_parser::{SelectorImpl, Snapshot, SnapshotMap}; use crate::shared_lock::SharedRwLockReadGuard; use crate::simple_buckets_map::SimpleBucketsMap; -use crate::stylesheets::{CssRule, CssRuleRef, CustomMediaMap, StylesheetInDocument}; -use crate::stylesheets::{EffectiveRules, EffectiveRulesIterator}; +use crate::stylesheets::{ + CssRule, CssRuleRef, CustomMediaMap, EffectiveRules, EffectiveRulesIterator, + StylesheetInDocument, +}; +use crate::stylist::CascadeDataDifference; use crate::values::specified::position::PositionTryFallbacksItem; use crate::values::AtomIdent; use crate::Atom; @@ -39,6 +42,8 @@ pub enum RuleChangeKind { Removal, /// A change in the declarations of a style rule. StyleRuleDeclarations, + /// A change in the declarations of an @position-try rule. + PositionTryDeclarations, } /// A style sheet invalidation represents a kind of element or subtree that may @@ -98,14 +103,17 @@ impl InvalidationKind { } } -/// A set of invalidations due to stylesheet additions. +/// A set of invalidations due to stylesheet changes. /// -/// TODO(emilio): We might be able to do the same analysis for media query -/// changes too (or even selector changes?). +/// TODO(emilio): We might be able to do the same analysis for media query changes too (or even +/// selector changes?) specially now that we take the cascade data difference into account. #[derive(Debug, Default, MallocSizeOf)] pub struct StylesheetInvalidationSet { buckets: SimpleBucketsMap<InvalidationKind>, - fully_invalid: bool, + style_fully_invalid: bool, + /// The difference between the old and new cascade data, incrementally collected until flush() + /// returns it. + pub cascade_data_difference: CascadeDataDifference, } impl StylesheetInvalidationSet { @@ -117,20 +125,12 @@ impl StylesheetInvalidationSet { /// Mark the DOM tree styles' as fully invalid. pub fn invalidate_fully(&mut self) { debug!("StylesheetInvalidationSet::invalidate_fully"); - self.clear(); - self.fully_invalid = true; - } - - fn shrink_if_needed(&mut self) { - if self.fully_invalid { - return; - } - self.buckets.shrink_if_needed(); + self.buckets.clear(); + self.style_fully_invalid = true; } - /// Analyze the given stylesheet, and collect invalidations from their - /// rules, in order to avoid doing a full restyle when we style the document - /// next time. + /// Analyze the given stylesheet, and collect invalidations from their rules, in order to avoid + /// doing a full restyle when we style the document next time. pub fn collect_invalidations_for<S>( &mut self, device: &Device, @@ -141,7 +141,7 @@ impl StylesheetInvalidationSet { S: StylesheetInDocument, { debug!("StylesheetInvalidationSet::collect_invalidations_for"); - if self.fully_invalid { + if self.style_fully_invalid { debug!(" > Fully invalid already"); return; } @@ -167,13 +167,11 @@ impl StylesheetInvalidationSet { // traverses down, but it shouldn't make a difference. &[], ); - if self.fully_invalid { + if self.style_fully_invalid { break; } } - self.shrink_if_needed(); - debug!( " > resulting class invalidations: {:?}", self.buckets.classes @@ -183,33 +181,14 @@ impl StylesheetInvalidationSet { " > resulting local name invalidations: {:?}", self.buckets.local_names ); - debug!(" > fully_invalid: {}", self.fully_invalid); - } - - /// Clears the invalidation set, invalidating elements as needed if - /// `document_element` is provided. - /// - /// Returns true if any invalidations ocurred. - pub fn flush<E>(&mut self, document_element: Option<E>, snapshots: Option<&SnapshotMap>) -> bool - where - E: TElement, - { - debug!( - "Stylist::flush({:?}, snapshots: {})", - document_element, - snapshots.is_some() - ); - let have_invalidations = match document_element { - Some(e) => self.process_invalidations(e, snapshots), - None => false, - }; - self.clear(); - have_invalidations + debug!(" > style_fully_invalid: {}", self.style_fully_invalid); } /// Returns whether there's no invalidation to process. pub fn is_empty(&self) -> bool { - !self.fully_invalid && self.buckets.is_empty() + !self.style_fully_invalid + && self.buckets.is_empty() + && self.cascade_data_difference.is_empty() } fn invalidation_kind_for<E>( @@ -221,7 +200,7 @@ impl StylesheetInvalidationSet { where E: TElement, { - debug_assert!(!self.fully_invalid); + debug_assert!(!self.style_fully_invalid); let mut kind = InvalidationKind::None; @@ -268,39 +247,37 @@ impl StylesheetInvalidationSet { kind } - /// Clears the invalidation set without processing. - pub fn clear(&mut self) { - self.buckets.clear(); - self.fully_invalid = false; - debug_assert!(self.is_empty()); - } - - fn process_invalidations<E>(&self, element: E, snapshots: Option<&SnapshotMap>) -> bool + /// Processes the style invalidation set, invalidating elements as needed. + /// Returns true if any style invalidations occurred. + pub fn process_style<E>(&self, root: E, snapshots: Option<&SnapshotMap>) -> bool where E: TElement, { - debug!("Stylist::process_invalidations({:?}, {:?})", element, self); + debug!( + "StylesheetInvalidationSet::process_style({root:?}, snapshots: {})", + snapshots.is_some() + ); { - let mut data = match element.mutate_data() { + let mut data = match root.mutate_data() { Some(data) => data, None => return false, }; - if self.fully_invalid { - debug!("process_invalidations: fully_invalid({:?})", element); + if self.style_fully_invalid { + debug!("process_invalidations: fully_invalid({:?})", root); data.hint.insert(RestyleHint::restyle_subtree()); return true; } } - if self.is_empty() { + if self.buckets.is_empty() { debug!("process_invalidations: empty invalidation set"); return false; } - let quirks_mode = element.as_node().owner_doc().quirks_mode(); - self.process_invalidations_in_subtree(element, snapshots, quirks_mode) + let quirks_mode = root.as_node().owner_doc().quirks_mode(); + self.process_invalidations_in_subtree(root, snapshots, quirks_mode) } /// Process style invalidations in a given subtree. This traverses the @@ -539,10 +516,6 @@ impl StylesheetInvalidationSet { S: StylesheetInDocument, { debug!("StylesheetInvalidationSet::rule_changed"); - if self.fully_invalid { - return; - } - if !stylesheet.enabled() || !stylesheet.is_effective_for_device(device, custom_media, guard) { debug!(" > Stylesheet was not effective"); @@ -557,6 +530,24 @@ impl StylesheetInvalidationSet { return; } + if change_kind == RuleChangeKind::PositionTryDeclarations { + // @position-try declaration changes need to be dealt explicitly, since the + // declarations are mutable and we can't otherwise detect changes to them. + match *rule { + CssRule::PositionTry(ref pt) => { + self.cascade_data_difference + .changed_position_try_names + .insert(pt.read_with(guard).name.0.clone()); + }, + _ => debug_assert!(false, "how did position-try decls change on anything else?"), + } + return; + } + + if self.style_fully_invalid { + return; + } + // If the change is generic, we don't have the old rule information to know e.g., the old // media condition, or the old selector text, so we might need to invalidate more // aggressively. That only applies to the changed rules, for other rules we can just @@ -570,7 +561,7 @@ impl StylesheetInvalidationSet { is_generic_change, ancestors, ); - if self.fully_invalid { + if self.style_fully_invalid { return; } @@ -597,7 +588,7 @@ impl StylesheetInvalidationSet { // Note(dshin): Technically, the iterator should provide the ancestor chain as it traverses down, which sould be appended to `ancestors`, but it shouldn't matter. &[], ); - if self.fully_invalid { + if self.style_fully_invalid { break; } } @@ -615,7 +606,7 @@ impl StylesheetInvalidationSet { ) { use crate::stylesheets::CssRule::*; debug!("StylesheetInvalidationSet::collect_invalidations_for_rule"); - debug_assert!(!self.fully_invalid, "Not worth being here!"); + debug_assert!(!self.style_fully_invalid, "Not worth being here!"); match *rule { Style(ref lock) => { @@ -631,7 +622,7 @@ impl StylesheetInvalidationSet { let style_rule = lock.read_with(guard); for selector in style_rule.selectors.slice() { self.collect_invalidations(selector, quirks_mode); - if self.fully_invalid { + if self.style_fully_invalid { return; } } diff --git a/servo/components/style/stylesheet_set.rs b/servo/components/style/stylesheet_set.rs @@ -5,10 +5,8 @@ //! A centralized set of stylesheets for a document. use crate::derives::*; -use crate::dom::TElement; use crate::invalidation::stylesheets::{RuleChangeKind, StylesheetInvalidationSet}; use crate::media_queries::Device; -use crate::selector_parser::SnapshotMap; use crate::shared_lock::SharedRwLockReadGuard; use crate::stylesheets::{ CssRule, CssRuleRef, CustomMediaMap, Origin, OriginSet, PerOrigin, StylesheetInDocument, @@ -67,7 +65,6 @@ where S: StylesheetInDocument + PartialEq + 'static, { collections: &'a mut PerOrigin<SheetCollection<S>>, - had_invalidations: bool, } /// The type of rebuild that we need to do for a given stylesheet. @@ -101,13 +98,6 @@ where pub fn origin_sheets(&self, origin: Origin) -> impl Iterator<Item = &S> { self.collections.borrow_for_origin(&origin).iter() } - - /// Returns whether any DOM invalidations were processed as a result of the - /// stylesheet flush. - #[inline] - pub fn had_invalidations(&self) -> bool { - self.had_invalidations - } } /// A flusher struct for a given collection, that takes care of returning the @@ -448,7 +438,9 @@ macro_rules! sheet_set_methods { // Maybe we could record whether we saw a clone in this flush, // and if so do the conservative thing, otherwise just // early-return. - RuleChangeKind::StyleRuleDeclarations => DataValidity::FullyInvalid, + RuleChangeKind::PositionTryDeclarations | RuleChangeKind::StyleRuleDeclarations => { + DataValidity::FullyInvalid + }, }; let collection = self.collection_for(&sheet, guard); @@ -508,24 +500,16 @@ where .any(|(collection, _)| collection.dirty) } - /// Flush the current set, unmarking it as dirty, and returns a - /// `DocumentStylesheetFlusher` in order to rebuild the stylist. - pub fn flush<E>( - &mut self, - document_element: Option<E>, - snapshots: Option<&SnapshotMap>, - ) -> DocumentStylesheetFlusher<'_, S> - where - E: TElement, - { + /// Flush the current set, unmarking it as dirty, and returns a `DocumentStylesheetFlusher` in + /// order to rebuild the stylist and the invalidation set. + pub fn flush(&mut self) -> (DocumentStylesheetFlusher<'_, S>, StylesheetInvalidationSet) { debug!("DocumentStylesheetSet::flush"); - - let had_invalidations = self.invalidations.flush(document_element, snapshots); - - DocumentStylesheetFlusher { - collections: &mut self.collections, - had_invalidations, - } + ( + DocumentStylesheetFlusher { + collections: &mut self.collections, + }, + std::mem::take(&mut self.invalidations), + ) } /// Flush stylesheets, but without running any of the invalidation passes. @@ -591,8 +575,6 @@ where { /// The actual flusher for the collection. pub sheets: SheetCollectionFlusher<'a, S>, - /// Whether any sheet invalidation matched. - pub had_invalidations: bool, } impl<S> AuthorStylesheetSet<S> @@ -652,21 +634,12 @@ where } /// Flush the stylesheets for this author set. - /// - /// `host` is the root of the affected subtree, like the shadow host, for - /// example. - pub fn flush<E>( - &mut self, - host: Option<E>, - snapshots: Option<&SnapshotMap>, - ) -> AuthorStylesheetFlusher<'_, S> - where - E: TElement, - { - let had_invalidations = self.invalidations.flush(host, snapshots); - AuthorStylesheetFlusher { - sheets: self.collection.flush(), - had_invalidations, - } + pub fn flush(&mut self) -> (AuthorStylesheetFlusher<'_, S>, StylesheetInvalidationSet) { + ( + AuthorStylesheetFlusher { + sheets: self.collection.flush(), + }, + std::mem::take(&mut self.invalidations), + ) } } diff --git a/servo/components/style/stylist.rs b/servo/components/style/stylist.rs @@ -21,7 +21,7 @@ use crate::invalidation::element::invalidation_map::{ use crate::invalidation::media_queries::{ EffectiveMediaQueryResults, MediaListKey, ToMediaListKey, }; -use crate::invalidation::stylesheets::RuleChangeKind; +use crate::invalidation::stylesheets::{RuleChangeKind, StylesheetInvalidationSet}; use crate::media_queries::Device; use crate::properties::{ self, AnimationDeclarations, CascadeMode, ComputedValues, FirstLineReparenting, @@ -34,9 +34,7 @@ use crate::rule_cache::{RuleCache, RuleCacheConditions}; use crate::rule_collector::RuleCollector; use crate::rule_tree::{CascadeLevel, RuleTree, StrongRuleNode, StyleSource}; use crate::selector_map::{PrecomputedHashMap, PrecomputedHashSet, SelectorMap, SelectorMapEntry}; -use crate::selector_parser::{ - NonTSPseudoClass, PerPseudoElementMap, PseudoElement, SelectorImpl, SnapshotMap, -}; +use crate::selector_parser::{NonTSPseudoClass, PerPseudoElementMap, PseudoElement, SelectorImpl}; use crate::shared_lock::{Locked, SharedRwLockReadGuard, StylesheetGuards}; use crate::sharing::{RevalidationResult, ScopeRevalidationResult}; use crate::stylesheet_set::{DataValidity, DocumentStylesheetSet, SheetRebuildKind}; @@ -49,14 +47,10 @@ use crate::stylesheets::scope_rule::{ collect_scope_roots, element_is_outside_of_scope, scope_selector_list_is_trivial, ImplicitScopeRoot, ScopeRootCandidate, ScopeSubjectMap, ScopeTarget, }; -#[cfg(feature = "gecko")] -use crate::stylesheets::{ - CounterStyleRule, FontFaceRule, FontFeatureValuesRule, FontPaletteValuesRule, - PagePseudoClassFlags, PositionTryRule, -}; use crate::stylesheets::{ - CssRule, CssRuleRef, EffectiveRulesIterator, Origin, OriginSet, PageRule, PerOrigin, - PerOriginIter, StylesheetContents, StylesheetInDocument, + CounterStyleRule, CssRule, CssRuleRef, EffectiveRulesIterator, FontFaceRule, + FontFeatureValuesRule, FontPaletteValuesRule, Origin, OriginSet, PagePseudoClassFlags, + PageRule, PerOrigin, PerOriginIter, PositionTryRule, StylesheetContents, StylesheetInDocument, }; use crate::stylesheets::{CustomMediaEvaluator, CustomMediaMap}; use crate::values::specified::position::PositionTryFallbacksItem; @@ -117,17 +111,8 @@ impl Hash for StylesheetContentsPtr { type StyleSheetContentList = Vec<StylesheetContentsPtr>; -/// The result of a flush of document stylesheets. -#[derive(Default)] -pub struct DocumentFlushResult { - /// The CascadeDataDifference for this flush. - pub difference: CascadeDataDifference, - /// Whether we had any style invalidations as a result of the flush. - pub had_invalidations: bool, -} - /// The @position-try rules that have changed. -#[derive(Default)] +#[derive(Default, Debug, MallocSizeOf)] pub struct CascadeDataDifference { /// The set of changed @position-try rule names. pub changed_position_try_names: PrecomputedHashSet<Atom>, @@ -140,6 +125,11 @@ impl CascadeDataDifference { .extend(other.changed_position_try_names.into_iter()) } + /// Returns whether we're empty. + pub fn is_empty(&self) -> bool { + self.changed_position_try_names.is_empty() + } + fn update(&mut self, old_data: &PositionTryMap, new_data: &PositionTryMap) { let mut any_different_key = false; let different_len = old_data.len() != new_data.len(); @@ -500,12 +490,12 @@ impl DocumentCascadeData { quirks_mode: QuirksMode, mut flusher: DocumentStylesheetFlusher<'a, S>, guards: &StylesheetGuards, - ) -> Result<CascadeDataDifference, AllocErr> + difference: &mut CascadeDataDifference, + ) -> Result<(), AllocErr> where S: StylesheetInDocument + PartialEq + 'static, { // First do UA sheets. - let mut difference = CascadeDataDifference::default(); { let origin_flusher = flusher.flush_origin(Origin::UserAgent); // Dirty check is just a minor optimization (no need to grab the @@ -518,7 +508,7 @@ impl DocumentCascadeData { origin_flusher, guards.ua_or_user, &self.user_agent, - &mut difference, + difference, )?; if let Some(new_data) = new_data { self.user_agent = new_data; @@ -535,7 +525,7 @@ impl DocumentCascadeData { quirks_mode, flusher.flush_origin(Origin::User), guards.ua_or_user, - &mut difference, + difference, )?; // And now the author sheets. @@ -544,10 +534,10 @@ impl DocumentCascadeData { quirks_mode, flusher.flush_origin(Origin::Author), guards.author, - &mut difference, + difference, )?; - Ok(difference) + Ok(()) } /// Measures heap usage. @@ -1040,39 +1030,29 @@ impl Stylist { /// Flush the list of stylesheets if they changed, ensuring the stylist is /// up-to-date. - pub fn flush<E>( - &mut self, - guards: &StylesheetGuards, - document_element: Option<E>, - snapshots: Option<&SnapshotMap>, - ) -> DocumentFlushResult - where - E: TElement, - { + pub fn flush(&mut self, guards: &StylesheetGuards) -> StylesheetInvalidationSet { if !self.stylesheets.has_changed() { - return DocumentFlushResult::default(); + return Default::default(); } self.num_rebuilds += 1; - let flusher = self.stylesheets.flush(document_element, snapshots); - - let had_invalidations = flusher.had_invalidations(); + let (flusher, mut invalidations) = self.stylesheets.flush(); - let difference = self - .cascade_data - .rebuild(&self.device, self.quirks_mode, flusher, guards) + self.cascade_data + .rebuild( + &self.device, + self.quirks_mode, + flusher, + guards, + &mut invalidations.cascade_data_difference, + ) .unwrap_or_else(|_| { warn!("OOM in Stylist::flush"); - CascadeDataDifference::default() }); self.rebuild_initial_values_for_custom_properties(); - - DocumentFlushResult { - difference, - had_invalidations, - } + invalidations } /// Marks a given stylesheet origin as dirty, due to, for example, changes diff --git a/servo/ports/geckolib/glue.rs b/servo/ports/geckolib/glue.rs @@ -1985,39 +1985,43 @@ pub unsafe extern "C" fn Servo_StyleSet_FlushStyleSheets( let mut data = raw_data.borrow_mut(); let doc_element = doc_element.map(GeckoElement); - let mut doc_flush_result = data.flush_stylesheets(&guard, doc_element, snapshots.as_ref()); + let mut invalidations = data.flush_stylesheets(&guard); if !non_document_styles.is_empty() { for author_styles in non_document_styles { - // TODO(emilio): This is going to need an element or something to do proper - // invalidation in ShadowRoots. - let difference = author_styles.flush::<GeckoElement>(&mut data.stylist, &guard); - // TODO(emilio): Consider doing scoped invalidation, specially once we have tree-scoped - // names. - doc_flush_result.difference.merge_with(difference); + let shadow_invalidations = author_styles.flush(&mut data.stylist, &guard); + // TODO(emilio): For now we drop the style invalidations on the floor, relying on + // explicit invalidation from C++. + // TODO(emilio): Consider doing scoped cascade data invalidation, specially once we + // have tree-scoped names. + invalidations + .cascade_data_difference + .merge_with(shadow_invalidations.cascade_data_difference); } data.stylist.remove_unique_author_data_cache_entries(); } // TODO(emilio): consider merging the existing stylesheet invalidation machinery into the // `CascadeDataDifference`. - if let Some(doc_element) = doc_element { - let changed_position_try_names = &doc_flush_result.difference.changed_position_try_names; - if !changed_position_try_names.is_empty() { - style::invalidation::stylesheets::invalidate_position_try( - doc_element, - &changed_position_try_names, - &mut |e, _data| unsafe { - bindings::Gecko_InvalidatePositionTry(e.0); - }, - &mut |_| {}, - ); - } - - if doc_flush_result.had_invalidations { - // The invalidation machinery propagates the bits up, but we still need to tell the Gecko - // restyle root machinery about it. - bindings::Gecko_NoteDirtySubtreeForInvalidation(doc_element.0); - } + let Some(doc_element) = doc_element else { + return; + }; + if invalidations.process_style(doc_element, snapshots.as_ref()) { + // The style invalidation machinery propagates the bits up, but we still need to tell the + // Gecko restyle root machinery about it. + bindings::Gecko_NoteDirtySubtreeForInvalidation(doc_element.0); + } + let changed_position_try_names = &invalidations + .cascade_data_difference + .changed_position_try_names; + if !changed_position_try_names.is_empty() { + style::invalidation::stylesheets::invalidate_position_try( + doc_element, + &changed_position_try_names, + &mut |e, _data| unsafe { + bindings::Gecko_InvalidatePositionTry(e.0); + }, + &mut |_| {}, + ); } } diff --git a/testing/web-platform/meta/css/css-anchor-position/at-position-try-cssom.html.ini b/testing/web-platform/meta/css/css-anchor-position/at-position-try-cssom.html.ini @@ -1,3 +0,0 @@ -[at-position-try-cssom.html] - [CSSPositionTryRule.style.setProperty setting allowed and disallowed properties] - expected: FAIL diff --git a/testing/web-platform/meta/css/css-anchor-position/last-successful-change-try-rule.html.ini b/testing/web-platform/meta/css/css-anchor-position/last-successful-change-try-rule.html.ini @@ -1,3 +0,0 @@ -[last-successful-change-try-rule.html] - [No successful position, last successful invalidated by @position-try change] - expected: FAIL