commit 1d41dfae06684b0cfaffd0bd77ce492b5c0cfb0f
parent 47ae5f5f7b18ceeae9cd1b1405ddb1e20c65a9c7
Author: Emilio Cobos Álvarez <emilio@crisal.io>
Date: Tue, 21 Oct 2025 13:45:08 +0000
Bug 1995525 - Tweak stylo APIs to make it easier for servo to replace the StylesheetContents without needing a whole new stylesheet. r=firefox-style-system-reviewers,dshin
Put the Arc<StylesheetContents> under a Locked<T> so that Servo can
replace it.
Differential Revision: https://phabricator.services.mozilla.com/D269382
Diffstat:
9 files changed, 139 insertions(+), 165 deletions(-)
diff --git a/servo/components/style/gecko/data.rs b/servo/components/style/gecko/data.rs
@@ -41,7 +41,7 @@ unsafe impl Sync for GeckoStyleSheet {}
impl fmt::Debug for GeckoStyleSheet {
fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
- let contents = self.contents();
+ let contents = self.raw_contents();
formatter
.debug_struct("GeckoStyleSheet")
.field("origin", &contents.origin)
@@ -89,6 +89,11 @@ impl GeckoStyleSheet {
fn inner(&self) -> &StyleSheetInfo {
unsafe { &*(self.raw().mInner as *const StyleSheetInfo) }
}
+
+ fn raw_contents(&self) -> &StylesheetContents {
+ debug_assert!(!self.inner().mContents.mRawPtr.is_null());
+ unsafe { &*self.inner().mContents.mRawPtr }
+ }
}
impl Drop for GeckoStyleSheet {
@@ -125,9 +130,8 @@ impl StylesheetInDocument for GeckoStyleSheet {
}
#[inline]
- fn contents(&self) -> &StylesheetContents {
- debug_assert!(!self.inner().mContents.mRawPtr.is_null());
- unsafe { &*self.inner().mContents.mRawPtr }
+ fn contents<'a>(&'a self, _: &'a SharedRwLockReadGuard) -> &'a StylesheetContents {
+ self.raw_contents()
}
fn implicit_scope_root(&self) -> Option<ImplicitScopeRoot> {
diff --git a/servo/components/style/invalidation/stylesheets.rs b/servo/components/style/invalidation/stylesheets.rs
@@ -146,7 +146,7 @@ impl StylesheetInvalidationSet {
}
let quirks_mode = device.quirks_mode();
- for rule in stylesheet.effective_rules(device, guard) {
+ for rule in stylesheet.contents(guard).effective_rules(device, guard) {
self.collect_invalidations_for_rule(
rule,
guard,
diff --git a/servo/components/style/stylesheet_set.rs b/servo/components/style/stylesheet_set.rs
@@ -361,7 +361,7 @@ macro_rules! sheet_set_methods {
) {
debug!(concat!($set_name, "::append_stylesheet"));
self.collect_invalidations_for(device, &sheet, guard);
- let collection = self.collection_for(&sheet);
+ let collection = self.collection_for(&sheet, guard);
collection.append(sheet);
}
@@ -376,7 +376,7 @@ macro_rules! sheet_set_methods {
debug!(concat!($set_name, "::insert_stylesheet_before"));
self.collect_invalidations_for(device, &sheet, guard);
- let collection = self.collection_for(&sheet);
+ let collection = self.collection_for(&sheet, guard);
collection.insert_before(sheet, &before_sheet);
}
@@ -390,7 +390,7 @@ macro_rules! sheet_set_methods {
debug!(concat!($set_name, "::remove_stylesheet"));
self.collect_invalidations_for(device, &sheet, guard);
- let collection = self.collection_for(&sheet);
+ let collection = self.collection_for(&sheet, guard);
collection.remove(&sheet)
}
@@ -440,7 +440,7 @@ macro_rules! sheet_set_methods {
RuleChangeKind::StyleRuleDeclarations => DataValidity::FullyInvalid,
};
- let collection = self.collection_for(&sheet);
+ let collection = self.collection_for(&sheet, guard);
collection.set_data_validity_at_least(validity);
}
};
@@ -458,8 +458,12 @@ where
}
}
- fn collection_for(&mut self, sheet: &S) -> &mut SheetCollection<S> {
- let origin = sheet.contents().origin;
+ fn collection_for(
+ &mut self,
+ sheet: &S,
+ guard: &SharedRwLockReadGuard,
+ ) -> &mut SheetCollection<S> {
+ let origin = sheet.contents(guard).origin;
self.collections.borrow_mut_for_origin(&origin)
}
@@ -613,7 +617,7 @@ where
self.collection.len()
}
- fn collection_for(&mut self, _sheet: &S) -> &mut SheetCollection<S> {
+ fn collection_for(&mut self, _: &S, _: &SharedRwLockReadGuard) -> &mut SheetCollection<S> {
&mut self.collection
}
diff --git a/servo/components/style/stylesheets/import_rule.rs b/servo/components/style/stylesheets/import_rule.rs
@@ -79,7 +79,7 @@ impl ImportSheet {
/// Returns the rule list for this import rule.
pub fn rules<'a>(&'a self, guard: &'a SharedRwLockReadGuard) -> &'a [CssRule] {
match self.as_sheet() {
- Some(s) => s.rules(guard),
+ Some(s) => s.contents(guard).rules(guard),
None => &[],
}
}
diff --git a/servo/components/style/stylesheets/mod.rs b/servo/components/style/stylesheets/mod.rs
@@ -68,7 +68,7 @@ pub use self::origin::{Origin, OriginSet, OriginSetIterator, PerOrigin, PerOrigi
pub use self::page_rule::{PagePseudoClassFlags, PageRule, PageSelector, PageSelectors};
pub use self::position_try_rule::PositionTryRule;
pub use self::property_rule::PropertyRule;
-pub use self::rule_list::{CssRules, CssRulesHelpers};
+pub use self::rule_list::CssRules;
pub use self::rule_parser::{InsertRuleContext, State, TopLevelRuleParser};
pub use self::rules_iterator::{AllRules, EffectiveRules};
pub use self::rules_iterator::{
diff --git a/servo/components/style/stylesheets/rule_list.rs b/servo/components/style/stylesheets/rule_list.rs
@@ -120,11 +120,9 @@ impl CssRules {
}
dest.write_str("\n}")
}
-}
-/// A trait to implement helpers for `Arc<Locked<CssRules>>`.
-pub trait CssRulesHelpers {
- /// <https://drafts.csswg.org/cssom/#insert-a-css-rule>
+ /// Parses a rule for <https://drafts.csswg.org/cssom/#insert-a-css-rule>. Caller is
+ /// responsible for calling insert() afterwards.
///
/// Written in this funky way because parsing an @import rule may cause us
/// to clone a stylesheet from the same document due to caching in the CSS
@@ -132,21 +130,7 @@ pub trait CssRulesHelpers {
///
/// TODO(emilio): We could also pass the write guard down into the loader
/// instead, but that seems overkill.
- fn insert_rule(
- &self,
- lock: &SharedRwLock,
- rule: &str,
- parent_stylesheet_contents: &StylesheetContents,
- index: usize,
- nested: CssRuleTypes,
- parse_relative_rule_type: Option<CssRuleType>,
- loader: Option<&dyn StylesheetLoader>,
- allow_import_rules: AllowImportRules,
- ) -> Result<CssRule, RulesMutateError>;
-}
-
-impl CssRulesHelpers for Locked<CssRules> {
- fn insert_rule(
+ pub fn parse_rule_for_insert(
&self,
lock: &SharedRwLock,
rule: &str,
@@ -157,39 +141,26 @@ impl CssRulesHelpers for Locked<CssRules> {
loader: Option<&dyn StylesheetLoader>,
allow_import_rules: AllowImportRules,
) -> Result<CssRule, RulesMutateError> {
- let new_rule = {
- let read_guard = lock.read();
- let rules = self.read_with(&read_guard);
-
- // Step 1, 2
- if index > rules.0.len() {
- return Err(RulesMutateError::IndexSize);
- }
+ // Step 1, 2
+ if index > self.0.len() {
+ return Err(RulesMutateError::IndexSize);
+ }
- let insert_rule_context = InsertRuleContext {
- rule_list: &rules.0,
- index,
- containing_rule_types,
- parse_relative_rule_type,
- };
-
- // Steps 3, 4, 5, 6
- CssRule::parse(
- &rule,
- insert_rule_context,
- parent_stylesheet_contents,
- lock,
- loader,
- allow_import_rules,
- )?
+ let insert_rule_context = InsertRuleContext {
+ rule_list: &self.0,
+ index,
+ containing_rule_types,
+ parse_relative_rule_type,
};
- {
- let mut write_guard = lock.write();
- let rules = self.write_with(&mut write_guard);
- rules.0.insert(index, new_rule.clone());
- }
-
- Ok(new_rule)
+ // Steps 3, 4, 5, 6
+ CssRule::parse(
+ &rule,
+ insert_rule_context,
+ parent_stylesheet_contents,
+ lock,
+ loader,
+ allow_import_rules,
+ )
}
}
diff --git a/servo/components/style/stylesheets/stylesheet.rs b/servo/components/style/stylesheets/stylesheet.rs
@@ -160,6 +160,30 @@ impl StylesheetContents {
self.rules.unconditional_shallow_size_of(ops)
+ self.rules.read_with(guard).size_of(guard, ops)
}
+
+ /// Return an iterator using the condition `C`.
+ #[inline]
+ pub fn iter_rules<'a, 'b, C>(
+ &'a self,
+ device: &'a Device,
+ guard: &'a SharedRwLockReadGuard<'b>,
+ ) -> RulesIterator<'a, 'b, C>
+ where
+ C: NestedRuleIterationCondition,
+ {
+ RulesIterator::new(device, self.quirks_mode, guard, self.rules(guard).iter())
+ }
+
+ /// Return an iterator over the effective rules within the style-sheet, as
+ /// according to the supplied `Device`.
+ #[inline]
+ pub fn effective_rules<'a, 'b>(
+ &'a self,
+ device: &'a Device,
+ guard: &'a SharedRwLockReadGuard<'b>,
+ ) -> EffectiveRulesIterator<'a, 'b> {
+ self.iter_rules::<EffectiveRules>(device, guard)
+ }
}
impl DeepCloneWithLock for StylesheetContents {
@@ -188,7 +212,7 @@ impl DeepCloneWithLock for StylesheetContents {
#[derive(Debug)]
pub struct Stylesheet {
/// The contents of this stylesheet.
- pub contents: Arc<StylesheetContents>,
+ pub contents: Locked<Arc<StylesheetContents>>,
/// The lock used for objects inside this stylesheet
pub shared_lock: SharedRwLock,
/// List of media associated with the Stylesheet.
@@ -205,52 +229,17 @@ pub trait StylesheetInDocument: ::std::fmt::Debug {
/// Get the media associated with this stylesheet.
fn media<'a>(&'a self, guard: &'a SharedRwLockReadGuard) -> Option<&'a MediaList>;
- /// Returns a reference to the list of rules in this stylesheet.
- fn rules<'a, 'b: 'a>(&'a self, guard: &'b SharedRwLockReadGuard) -> &'a [CssRule] {
- self.contents().rules(guard)
- }
-
/// Returns a reference to the contents of the stylesheet.
- fn contents(&self) -> &StylesheetContents;
-
- /// Return an iterator using the condition `C`.
- #[inline]
- fn iter_rules<'a, 'b, C>(
- &'a self,
- device: &'a Device,
- guard: &'a SharedRwLockReadGuard<'b>,
- ) -> RulesIterator<'a, 'b, C>
- where
- C: NestedRuleIterationCondition,
- {
- let contents = self.contents();
- RulesIterator::new(
- device,
- contents.quirks_mode,
- guard,
- contents.rules(guard).iter(),
- )
- }
+ fn contents<'a>(&'a self, guard: &'a SharedRwLockReadGuard) -> &'a StylesheetContents;
/// Returns whether the style-sheet applies for the current device.
fn is_effective_for_device(&self, device: &Device, guard: &SharedRwLockReadGuard) -> bool {
match self.media(guard) {
- Some(medialist) => medialist.evaluate(device, self.contents().quirks_mode),
+ Some(medialist) => medialist.evaluate(device, self.contents(guard).quirks_mode),
None => true,
}
}
- /// Return an iterator over the effective rules within the style-sheet, as
- /// according to the supplied `Device`.
- #[inline]
- fn effective_rules<'a, 'b>(
- &'a self,
- device: &'a Device,
- guard: &'a SharedRwLockReadGuard<'b>,
- ) -> EffectiveRulesIterator<'a, 'b> {
- self.iter_rules::<EffectiveRules>(device, guard)
- }
-
/// Return the implicit scope root for this stylesheet, if one exists.
fn implicit_scope_root(&self) -> Option<ImplicitScopeRoot>;
}
@@ -265,8 +254,8 @@ impl StylesheetInDocument for Stylesheet {
}
#[inline]
- fn contents(&self) -> &StylesheetContents {
- &self.contents
+ fn contents<'a>(&'a self, guard: &'a SharedRwLockReadGuard) -> &'a StylesheetContents {
+ self.contents.read_with(guard)
}
fn implicit_scope_root(&self) -> Option<ImplicitScopeRoot> {
@@ -298,8 +287,8 @@ impl StylesheetInDocument for DocumentStyleSheet {
}
#[inline]
- fn contents(&self) -> &StylesheetContents {
- self.0.contents()
+ fn contents<'a>(&'a self, guard: &'a SharedRwLockReadGuard) -> &'a StylesheetContents {
+ self.0.contents(guard)
}
fn implicit_scope_root(&self) -> Option<ImplicitScopeRoot> {
@@ -499,7 +488,7 @@ impl Stylesheet {
);
Stylesheet {
- contents,
+ contents: shared_lock.wrap(contents),
shared_lock,
media,
disabled: AtomicBool::new(false),
@@ -534,7 +523,11 @@ impl Clone for Stylesheet {
// Make a deep clone of the media, using the new lock.
let media = self.media.read_with(&guard).clone();
let media = Arc::new(lock.wrap(media));
- let contents = Arc::new(self.contents.deep_clone_with_lock(&lock, &guard));
+ let contents = lock.wrap(Arc::new(
+ self.contents
+ .read_with(&guard)
+ .deep_clone_with_lock(&lock, &guard),
+ ));
Stylesheet {
contents,
diff --git a/servo/components/style/stylist.rs b/servo/components/style/stylist.rs
@@ -1228,7 +1228,9 @@ impl Stylist {
} else {
None
};
- let fallback_block = fallback_rule.as_ref().map(|r| &r.read_with(guards.author).block);
+ let fallback_block = fallback_rule
+ .as_ref()
+ .map(|r| &r.read_with(guards.author).block);
let pseudo = style
.pseudo()
.or_else(|| element.implemented_pseudo_element());
@@ -1257,20 +1259,23 @@ impl Stylist {
}
inputs
};
- crate::style_resolver::with_default_parent_styles(element, |parent_style, layout_parent_style| {
- Some(self.cascade_style_and_visited(
- Some(element),
- pseudo.as_ref(),
- inputs,
- guards,
- parent_style,
- layout_parent_style,
- FirstLineReparenting::No,
- name_and_try_tactic.try_tactic,
- /* rule_cache = */ None,
- &mut RuleCacheConditions::default(),
- ))
- })
+ crate::style_resolver::with_default_parent_styles(
+ element,
+ |parent_style, layout_parent_style| {
+ Some(self.cascade_style_and_visited(
+ Some(element),
+ pseudo.as_ref(),
+ inputs,
+ guards,
+ parent_style,
+ layout_parent_style,
+ FirstLineReparenting::No,
+ name_and_try_tactic.try_tactic,
+ /* rule_cache = */ None,
+ &mut RuleCacheConditions::default(),
+ ))
+ },
+ )
}
/// Computes a style using the given CascadeInputs. This can be used to
@@ -3453,15 +3458,15 @@ impl CascadeData {
}
debug!(" + {:?}", stylesheet);
- let contents = stylesheet.contents();
+ let contents = stylesheet.contents(guard);
results.push(contents.to_media_list_key());
// Safety: StyleSheetContents are reference-counted with Arc.
contents_list.push(StylesheetContentsPtr(unsafe {
- Arc::from_raw_addrefed(contents)
+ Arc::from_raw_addrefed(&*contents)
}));
- for rule in stylesheet.effective_rules(device, guard) {
+ for rule in stylesheet.contents(guard).effective_rules(device, guard) {
match *rule {
CssRule::Import(ref lock) => {
let import_rule = lock.read_with(guard);
@@ -4140,10 +4145,9 @@ impl CascadeData {
return Ok(());
}
- let contents = stylesheet.contents();
-
+ let contents = stylesheet.contents(guard);
if rebuild_kind.should_rebuild_invalidation() {
- self.effective_media_query_results.saw_effective(contents);
+ self.effective_media_query_results.saw_effective(&*contents);
}
let mut state = ContainingRuleState::default();
@@ -4178,9 +4182,8 @@ impl CascadeData {
let effective_now = stylesheet.is_effective_for_device(device, guard);
- let effective_then = self
- .effective_media_query_results
- .was_effective(stylesheet.contents());
+ let contents = stylesheet.contents(guard);
+ let effective_then = self.effective_media_query_results.was_effective(contents);
if effective_now != effective_then {
debug!(
@@ -4196,8 +4199,7 @@ impl CascadeData {
return true;
}
- let mut iter = stylesheet.iter_rules::<PotentiallyEffectiveMediaRules>(device, guard);
-
+ let mut iter = contents.iter_rules::<PotentiallyEffectiveMediaRules>(device, guard);
while let Some(rule) = iter.next() {
match *rule {
CssRule::Style(..)
diff --git a/servo/ports/geckolib/glue.rs b/servo/ports/geckolib/glue.rs
@@ -136,12 +136,12 @@ use style::stylesheets::scope_rule::{ImplicitScopeRoot, ScopeRootCandidate, Scop
use style::stylesheets::supports_rule::parse_condition_or_declaration;
use style::stylesheets::{
AllowImportRules, ContainerRule, CounterStyleRule, CssRule, CssRuleType, CssRuleTypes,
- CssRules, CssRulesHelpers, DocumentRule, FontFaceRule, FontFeatureValuesRule,
- FontPaletteValuesRule, ImportRule, KeyframesRule, LayerBlockRule, LayerStatementRule,
- MarginRule, MediaRule, NamespaceRule, NestedDeclarationsRule, Origin, OriginSet,
- PagePseudoClassFlags, PageRule, PositionTryRule, PropertyRule, SanitizationData,
- SanitizationKind, ScopeRule, StartingStyleRule, StyleRule, StylesheetContents,
- StylesheetInDocument, StylesheetLoader as StyleStylesheetLoader, SupportsRule, UrlExtraData,
+ CssRules, DocumentRule, FontFaceRule, FontFeatureValuesRule, FontPaletteValuesRule, ImportRule,
+ KeyframesRule, LayerBlockRule, LayerStatementRule, MarginRule, MediaRule, NamespaceRule,
+ NestedDeclarationsRule, Origin, OriginSet, PagePseudoClassFlags, PageRule, PositionTryRule,
+ PropertyRule, SanitizationData, SanitizationKind, ScopeRule, StartingStyleRule, StyleRule,
+ StylesheetContents, StylesheetInDocument, StylesheetLoader as StyleStylesheetLoader,
+ SupportsRule, UrlExtraData,
};
use style::stylist::{
add_size_of_ua_cache, replace_parent_selector_with_implicit_scope, scope_root_candidates,
@@ -170,9 +170,9 @@ use style::values::generics::easing::BeforeFlag;
use style::values::generics::length::GenericAnchorSizeFunction;
use style::values::resolved;
use style::values::specified::intersection_observer::IntersectionObserverMargin;
+use style::values::specified::position::DashedIdentAndOrTryTactic;
use style::values::specified::source_size_list::SourceSizeList;
use style::values::specified::svg_path::PathCommand;
-use style::values::specified::position::DashedIdentAndOrTryTactic;
use style::values::specified::{AbsoluteLength, NoCalcLength};
use style::values::{specified, AtomIdent, CustomIdent, KeyframesName};
use style_traits::{CssWriter, ParseError, ParsingMode, ToCss, TypedValue};
@@ -2229,24 +2229,28 @@ pub extern "C" fn Servo_CssRules_InsertRule(
let rule = unsafe { rule.as_str_unchecked() };
let global_style_data = &*GLOBAL_STYLE_DATA;
- let result = rules.insert_rule(
- &global_style_data.shared_lock,
- rule,
- contents,
- index as usize,
- CssRuleTypes::from_bits(containing_rule_types),
- parse_relative_rule_type.cloned(),
- loader,
- allow_import_rules,
- );
+ let index = index as usize;
+ let new_rule = {
+ let guard = global_style_data.shared_lock.read();
+ match rules.read_with(&guard).parse_rule_for_insert(
+ &global_style_data.shared_lock,
+ rule,
+ contents,
+ index,
+ CssRuleTypes::from_bits(containing_rule_types),
+ parse_relative_rule_type.cloned(),
+ loader,
+ allow_import_rules,
+ ) {
+ Ok(r) => r,
+ Err(e) => return e.into(),
+ }
+ };
- match result {
- Ok(new_rule) => {
- *rule_type = new_rule.rule_type();
- nsresult::NS_OK
- },
- Err(err) => err.into(),
- }
+ let mut write_guard = global_style_data.shared_lock.write();
+ *rule_type = new_rule.rule_type();
+ rules.write_with(&mut write_guard).0.insert(index, new_rule);
+ nsresult::NS_OK
}
#[no_mangle]
@@ -4393,13 +4397,9 @@ pub unsafe extern "C" fn Servo_ComputedValues_GetForPositionTry(
let guards = StylesheetGuards::same(&guard);
let element = GeckoElement(element);
let data = raw_data.borrow();
- data.stylist.resolve_position_try(
- style,
- &guards,
- element,
- name_and_try_tactic
- )
- .into()
+ data.stylist
+ .resolve_position_try(style, &guards, element, name_and_try_tactic)
+ .into()
}
#[no_mangle]
