commit ad632bac51fdb78e32501a06b578eb504a797201
parent 397011f0aa9b0ea55391f3cf41ab1c80e7cb5db1
Author: Emilio Cobos Álvarez <emilio@crisal.io>
Date: Mon, 20 Oct 2025 13:16:02 +0000
Bug 1995268 - Make StylesheetContents more obviously immutable. r=nicoburns,Oriol
I want to update the UrlExtraData of the stylesheet contents when
cloning for uniqueness.
This removes an API that servo uses but that's easily replaceable, see:
https://github.com/servo/servo/pull/40024
Differential Revision: https://phabricator.services.mozilla.com/D269200
Diffstat:
5 files changed, 29 insertions(+), 70 deletions(-)
diff --git a/servo/components/style/gecko/data.rs b/servo/components/style/gecko/data.rs
@@ -45,7 +45,7 @@ impl fmt::Debug for GeckoStyleSheet {
formatter
.debug_struct("GeckoStyleSheet")
.field("origin", &contents.origin)
- .field("url_data", &*contents.url_data.read())
+ .field("url_data", &contents.url_data)
.finish()
}
}
diff --git a/servo/components/style/stylesheets/keyframes_rule.rs b/servo/components/style/stylesheets/keyframes_rule.rs
@@ -204,8 +204,8 @@ impl Keyframe {
parent_stylesheet_contents: &StylesheetContents,
lock: &SharedRwLock,
) -> Result<Arc<Locked<Self>>, ParseError<'i>> {
- let url_data = parent_stylesheet_contents.url_data.read();
- let namespaces = parent_stylesheet_contents.namespaces.read();
+ let url_data = &parent_stylesheet_contents.url_data;
+ let namespaces = &parent_stylesheet_contents.namespaces;
let mut context = ParserContext::new(
parent_stylesheet_contents.origin,
&url_data,
diff --git a/servo/components/style/stylesheets/mod.rs b/servo/components/style/stylesheets/mod.rs
@@ -569,8 +569,8 @@ impl CssRule {
loader: Option<&dyn StylesheetLoader>,
allow_import_rules: AllowImportRules,
) -> Result<Self, RulesMutateError> {
- let url_data = parent_stylesheet_contents.url_data.read();
- let namespaces = parent_stylesheet_contents.namespaces.read();
+ let url_data = &parent_stylesheet_contents.url_data;
+ let namespaces = &parent_stylesheet_contents.namespaces;
let mut context = ParserContext::new(
parent_stylesheet_contents.origin,
&url_data,
diff --git a/servo/components/style/stylesheets/stylesheet.rs b/servo/components/style/stylesheets/stylesheet.rs
@@ -18,7 +18,6 @@ use crate::{Namespace, Prefix};
use cssparser::{Parser, ParserInput, StyleSheetParser};
#[cfg(feature = "gecko")]
use malloc_size_of::{MallocSizeOfOps, MallocUnconditionalShallowSizeOf};
-use parking_lot::RwLock;
use rustc_hash::FxHashMap;
use servo_arc::Arc;
use std::sync::atomic::{AtomicBool, Ordering};
@@ -56,15 +55,15 @@ pub struct StylesheetContents {
/// The origin of this stylesheet.
pub origin: Origin,
/// The url data this stylesheet should use.
- pub url_data: RwLock<UrlExtraData>,
+ pub url_data: UrlExtraData,
/// The namespaces that apply to this stylesheet.
- pub namespaces: RwLock<Namespaces>,
+ pub namespaces: Namespaces,
/// The quirks mode of this stylesheet.
pub quirks_mode: QuirksMode,
/// This stylesheet's source map URL.
- pub source_map_url: RwLock<Option<String>>,
+ pub source_map_url: Option<String>,
/// This stylesheet's source URL.
- pub source_url: RwLock<Option<String>>,
+ pub source_url: Option<String>,
/// The use counters of the original stylesheet.
pub use_counters: UseCounters,
@@ -104,11 +103,11 @@ impl StylesheetContents {
Arc::new(Self {
rules: CssRules::new(rules, &shared_lock),
origin,
- url_data: RwLock::new(url_data),
- namespaces: RwLock::new(namespaces),
+ url_data,
+ namespaces,
quirks_mode,
- source_map_url: RwLock::new(source_map_url),
- source_url: RwLock::new(source_url),
+ source_map_url,
+ source_url,
use_counters,
_forbid_construction: (),
})
@@ -135,11 +134,11 @@ impl StylesheetContents {
Arc::new(Self {
rules,
origin,
- url_data: RwLock::new(url_data),
- namespaces: RwLock::new(Namespaces::default()),
+ url_data,
+ namespaces: Namespaces::default(),
quirks_mode,
- source_map_url: RwLock::new(None),
- source_url: RwLock::new(None),
+ source_map_url: None,
+ source_url: None,
use_counters: UseCounters::default(),
_forbid_construction: (),
})
@@ -175,10 +174,10 @@ impl DeepCloneWithLock for StylesheetContents {
rules: Arc::new(lock.wrap(rules)),
quirks_mode: self.quirks_mode,
origin: self.origin,
- url_data: RwLock::new((*self.url_data.read()).clone()),
- namespaces: RwLock::new((*self.namespaces.read()).clone()),
- source_map_url: RwLock::new((*self.source_map_url.read()).clone()),
- source_url: RwLock::new((*self.source_url.read()).clone()),
+ url_data: self.url_data.clone(),
+ namespaces: self.namespaces.clone(),
+ source_map_url: self.source_map_url.clone(),
+ source_url: self.source_url.clone(),
use_counters: self.use_counters.clone(),
_forbid_construction: (),
}
@@ -397,40 +396,6 @@ impl SanitizationData {
}
impl Stylesheet {
- /// Updates an empty stylesheet from a given string of text.
- pub fn update_from_str(
- existing: &Stylesheet,
- css: &str,
- url_data: UrlExtraData,
- stylesheet_loader: Option<&dyn StylesheetLoader>,
- error_reporter: Option<&dyn ParseErrorReporter>,
- allow_import_rules: AllowImportRules,
- ) {
- let use_counters = UseCounters::default();
- let (namespaces, rules, source_map_url, source_url) = Self::parse_rules(
- css,
- &url_data,
- existing.contents.origin,
- &existing.shared_lock,
- stylesheet_loader,
- error_reporter,
- existing.contents.quirks_mode,
- Some(&use_counters),
- allow_import_rules,
- /* sanitization_data = */ None,
- );
-
- *existing.contents.url_data.write() = url_data;
- *existing.contents.namespaces.write() = namespaces;
-
- // Acquire the lock *after* parsing, to minimize the exclusive section.
- let mut guard = existing.shared_lock.write();
- *existing.contents.rules.write_with(&mut guard) = CssRules(rules);
- *existing.contents.source_map_url.write() = source_map_url;
- *existing.contents.source_url.write() = source_url;
- existing.contents.use_counters.merge(&use_counters);
- }
-
fn parse_rules(
css: &str,
url_data: &UrlExtraData,
@@ -508,11 +473,7 @@ impl Stylesheet {
)
}
- /// Creates an empty stylesheet and parses it with a given base url, origin
- /// and media.
- ///
- /// Effectively creates a new stylesheet and forwards the hard work to
- /// `Stylesheet::update_from_str`.
+ /// Creates an empty stylesheet and parses it with a given base url, origin and media.
pub fn from_str(
css: &str,
url_data: UrlExtraData,
diff --git a/servo/ports/geckolib/glue.rs b/servo/ports/geckolib/glue.rs
@@ -2099,8 +2099,7 @@ pub extern "C" fn Servo_StyleSheet_GetSourceMapURL(
contents: &StylesheetContents,
result: &mut nsACString,
) {
- let url_opt = contents.source_map_url.read();
- if let Some(ref url) = *url_opt {
+ if let Some(ref url) = contents.source_map_url {
result.assign(url);
}
}
@@ -2110,8 +2109,7 @@ pub extern "C" fn Servo_StyleSheet_GetSourceURL(
contents: &StylesheetContents,
result: &mut nsACString,
) {
- let url_opt = contents.source_url.read();
- if let Some(ref url) = *url_opt {
+ if let Some(ref url) = contents.source_url {
result.assign(url);
}
}
@@ -2904,8 +2902,8 @@ pub extern "C" fn Servo_StyleRule_SetSelectorText(
use selectors::parser::ParseRelative;
use style::selector_parser::SelectorParser;
- let namespaces = contents.namespaces.read();
- let url_data = contents.url_data.read();
+ let namespaces = &contents.namespaces;
+ let url_data = &contents.url_data;
let parser = SelectorParser {
stylesheet_origin: contents.origin,
namespaces: &namespaces,
@@ -3324,7 +3322,7 @@ pub extern "C" fn Servo_PageRule_SetSelectorText(
return true;
}
- let url_data = contents.url_data.read();
+ let url_data = &contents.url_data;
let context = ParserContext::new(
Origin::Author,
&url_data,
@@ -9452,8 +9450,8 @@ pub unsafe extern "C" fn Servo_SharedMemoryBuilder_AddStylesheet(
// Assert some things we assume when we create a style sheet from shared
// memory.
debug_assert_eq!(contents.quirks_mode, QuirksMode::NoQuirks);
- debug_assert!(contents.source_map_url.read().is_none());
- debug_assert!(contents.source_url.read().is_none());
+ debug_assert!(contents.source_map_url.is_none());
+ debug_assert!(contents.source_url.is_none());
match builder.write(&contents.rules) {
Ok(rules_ptr) => &**rules_ptr,
