diff --git a/components/script/dom/bindings/trace.rs b/components/script/dom/bindings/trace.rs index 122823f6f0d..efa1a5646f2 100644 --- a/components/script/dom/bindings/trace.rs +++ b/components/script/dom/bindings/trace.rs @@ -502,7 +502,7 @@ unsafe impl JSTraceable for Mutex> { } } -unsafe impl JSTraceable for RwLock { +unsafe impl JSTraceable for StyleLocked { unsafe fn trace(&self, _trc: *mut JSTracer) { // Do nothing. } @@ -520,31 +520,31 @@ unsafe impl JSTraceable for RwLock { } } -unsafe impl JSTraceable for RwLock { +unsafe impl JSTraceable for StyleLocked { unsafe fn trace(&self, _trc: *mut JSTracer) { // Do nothing. } } -unsafe impl JSTraceable for RwLock { +unsafe impl JSTraceable for StyleLocked { unsafe fn trace(&self, _trc: *mut JSTracer) { // Do nothing. } } -unsafe impl JSTraceable for RwLock { +unsafe impl JSTraceable for StyleLocked { unsafe fn trace(&self, _trc: *mut JSTracer) { // Do nothing. } } -unsafe impl JSTraceable for RwLock { +unsafe impl JSTraceable for StyleLocked { unsafe fn trace(&self, _trc: *mut JSTracer) { // Do nothing. } } -unsafe impl JSTraceable for RwLock { +unsafe impl JSTraceable for StyleLocked { unsafe fn trace(&self, _trc: *mut JSTracer) { // Do nothing. } @@ -556,7 +556,7 @@ unsafe impl JSTraceable for RwLock { } } -unsafe impl JSTraceable for RwLock { +unsafe impl JSTraceable for StyleLocked { unsafe fn trace(&self, _trc: *mut JSTracer) { // Do nothing. } diff --git a/components/script/dom/cssfontfacerule.rs b/components/script/dom/cssfontfacerule.rs index 05b665a9975..184f6784afa 100644 --- a/components/script/dom/cssfontfacerule.rs +++ b/components/script/dom/cssfontfacerule.rs @@ -10,20 +10,19 @@ use dom::cssrule::{CSSRule, SpecificCSSRule}; use dom::cssstylesheet::CSSStyleSheet; use dom::window::Window; use dom_struct::dom_struct; -use parking_lot::RwLock; use std::sync::Arc; use style::font_face::FontFaceRule; -use style::shared_lock::ToCssWithGuard; +use style::shared_lock::{Locked, ToCssWithGuard}; #[dom_struct] pub struct CSSFontFaceRule { cssrule: CSSRule, #[ignore_heap_size_of = "Arc"] - fontfacerule: Arc>, + fontfacerule: Arc>, } impl CSSFontFaceRule { - fn new_inherited(parent_stylesheet: &CSSStyleSheet, fontfacerule: Arc>) + fn new_inherited(parent_stylesheet: &CSSStyleSheet, fontfacerule: Arc>) -> CSSFontFaceRule { CSSFontFaceRule { cssrule: CSSRule::new_inherited(parent_stylesheet), @@ -33,7 +32,7 @@ impl CSSFontFaceRule { #[allow(unrooted_must_root)] pub fn new(window: &Window, parent_stylesheet: &CSSStyleSheet, - fontfacerule: Arc>) -> Root { + fontfacerule: Arc>) -> Root { reflect_dom_object(box CSSFontFaceRule::new_inherited(parent_stylesheet, fontfacerule), window, CSSFontFaceRuleBinding::Wrap) @@ -48,6 +47,6 @@ impl SpecificCSSRule for CSSFontFaceRule { fn get_css(&self) -> DOMString { let guard = self.cssrule.shared_lock().read(); - self.fontfacerule.read().to_css_string(&guard).into() + self.fontfacerule.read_with(&guard).to_css_string(&guard).into() } } diff --git a/components/script/dom/cssimportrule.rs b/components/script/dom/cssimportrule.rs index bb01fdfcc84..3c0eb7eb4aa 100644 --- a/components/script/dom/cssimportrule.rs +++ b/components/script/dom/cssimportrule.rs @@ -10,21 +10,20 @@ use dom::cssrule::{CSSRule, SpecificCSSRule}; use dom::cssstylesheet::CSSStyleSheet; use dom::window::Window; use dom_struct::dom_struct; -use parking_lot::RwLock; use std::sync::Arc; -use style::shared_lock::ToCssWithGuard; +use style::shared_lock::{Locked, ToCssWithGuard}; use style::stylesheets::ImportRule; #[dom_struct] pub struct CSSImportRule { cssrule: CSSRule, #[ignore_heap_size_of = "Arc"] - import_rule: Arc>, + import_rule: Arc>, } impl CSSImportRule { fn new_inherited(parent_stylesheet: &CSSStyleSheet, - import_rule: Arc>) + import_rule: Arc>) -> Self { CSSImportRule { cssrule: CSSRule::new_inherited(parent_stylesheet), @@ -35,7 +34,7 @@ impl CSSImportRule { #[allow(unrooted_must_root)] pub fn new(window: &Window, parent_stylesheet: &CSSStyleSheet, - import_rule: Arc>) -> Root { + import_rule: Arc>) -> Root { reflect_dom_object(box Self::new_inherited(parent_stylesheet, import_rule), window, CSSImportRuleBinding::Wrap) @@ -50,6 +49,6 @@ impl SpecificCSSRule for CSSImportRule { fn get_css(&self) -> DOMString { let guard = self.cssrule.shared_lock().read(); - self.import_rule.read().to_css_string(&guard).into() + self.import_rule.read_with(&guard).to_css_string(&guard).into() } } diff --git a/components/script/dom/csskeyframesrule.rs b/components/script/dom/csskeyframesrule.rs index 330b1be3627..d17eeeb51b5 100644 --- a/components/script/dom/csskeyframesrule.rs +++ b/components/script/dom/csskeyframesrule.rs @@ -16,24 +16,23 @@ use dom::cssrulelist::{CSSRuleList, RulesSource}; use dom::cssstylesheet::CSSStyleSheet; use dom::window::Window; use dom_struct::dom_struct; -use parking_lot::RwLock; use servo_atoms::Atom; use std::sync::Arc; use style::keyframes::{Keyframe, KeyframeSelector}; use style::parser::ParserContextExtraData; -use style::shared_lock::ToCssWithGuard; +use style::shared_lock::{Locked, ToCssWithGuard}; use style::stylesheets::KeyframesRule; #[dom_struct] pub struct CSSKeyframesRule { cssrule: CSSRule, #[ignore_heap_size_of = "Arc"] - keyframesrule: Arc>, + keyframesrule: Arc>, rulelist: MutNullableJS, } impl CSSKeyframesRule { - fn new_inherited(parent_stylesheet: &CSSStyleSheet, keyframesrule: Arc>) + fn new_inherited(parent_stylesheet: &CSSStyleSheet, keyframesrule: Arc>) -> CSSKeyframesRule { CSSKeyframesRule { cssrule: CSSRule::new_inherited(parent_stylesheet), @@ -44,7 +43,7 @@ impl CSSKeyframesRule { #[allow(unrooted_must_root)] pub fn new(window: &Window, parent_stylesheet: &CSSStyleSheet, - keyframesrule: Arc>) -> Root { + keyframesrule: Arc>) -> Root { reflect_dom_object(box CSSKeyframesRule::new_inherited(parent_stylesheet, keyframesrule), window, CSSKeyframesRuleBinding::Wrap) @@ -63,9 +62,10 @@ impl CSSKeyframesRule { fn find_rule(&self, selector: &str) -> Option { let mut input = Parser::new(selector); if let Ok(sel) = KeyframeSelector::parse(&mut input) { + let guard = self.cssrule.shared_lock().read(); // This finds the *last* element matching a selector // because that's the rule that applies. Thus, rposition - self.keyframesrule.read() + self.keyframesrule.read_with(&guard) .keyframes.iter().rposition(|frame| { frame.read().selector == sel }) @@ -86,7 +86,8 @@ impl CSSKeyframesRuleMethods for CSSKeyframesRule { let rule = Keyframe::parse(&rule, self.cssrule.parent_stylesheet().style_stylesheet(), ParserContextExtraData::default()); if let Ok(rule) = rule { - self.keyframesrule.write().keyframes.push(rule); + let mut guard = self.cssrule.shared_lock().write(); + self.keyframesrule.write_with(&mut guard).keyframes.push(rule); self.rulelist().append_lazy_dom_rule(); } } @@ -107,7 +108,8 @@ impl CSSKeyframesRuleMethods for CSSKeyframesRule { // https://drafts.csswg.org/css-animations/#dom-csskeyframesrule-name fn Name(&self) -> DOMString { - DOMString::from(&*self.keyframesrule.read().name) + let guard = self.cssrule.shared_lock().read(); + DOMString::from(&*self.keyframesrule.read_with(&guard).name) } // https://drafts.csswg.org/css-animations/#dom-csskeyframesrule-name @@ -122,7 +124,8 @@ impl CSSKeyframesRuleMethods for CSSKeyframesRule { "none" => return Err(Error::Syntax), _ => () } - self.keyframesrule.write().name = Atom::from(value); + let mut guard = self.cssrule.shared_lock().write(); + self.keyframesrule.write_with(&mut guard).name = Atom::from(value); Ok(()) } } @@ -135,7 +138,7 @@ impl SpecificCSSRule for CSSKeyframesRule { fn get_css(&self) -> DOMString { let guard = self.cssrule.shared_lock().read(); - self.keyframesrule.read().to_css_string(&guard).into() + self.keyframesrule.read_with(&guard).to_css_string(&guard).into() } fn deparent_children(&self) { diff --git a/components/script/dom/cssmediarule.rs b/components/script/dom/cssmediarule.rs index 1d1f0846a51..2a07471679e 100644 --- a/components/script/dom/cssmediarule.rs +++ b/components/script/dom/cssmediarule.rs @@ -14,10 +14,9 @@ use dom::cssstylesheet::CSSStyleSheet; use dom::medialist::MediaList; use dom::window::Window; use dom_struct::dom_struct; -use parking_lot::RwLock; use std::sync::Arc; use style::media_queries::parse_media_query_list; -use style::shared_lock::ToCssWithGuard; +use style::shared_lock::{Locked, ToCssWithGuard}; use style::stylesheets::MediaRule; use style_traits::ToCss; @@ -25,14 +24,15 @@ use style_traits::ToCss; pub struct CSSMediaRule { cssconditionrule: CSSConditionRule, #[ignore_heap_size_of = "Arc"] - mediarule: Arc>, + mediarule: Arc>, medialist: MutNullableJS, } impl CSSMediaRule { - fn new_inherited(parent_stylesheet: &CSSStyleSheet, mediarule: Arc>) + fn new_inherited(parent_stylesheet: &CSSStyleSheet, mediarule: Arc>) -> CSSMediaRule { - let list = mediarule.read().rules.clone(); + let guard = parent_stylesheet.shared_lock().read(); + let list = mediarule.read_with(&guard).rules.clone(); CSSMediaRule { cssconditionrule: CSSConditionRule::new_inherited(parent_stylesheet, list), mediarule: mediarule, @@ -42,34 +42,43 @@ impl CSSMediaRule { #[allow(unrooted_must_root)] pub fn new(window: &Window, parent_stylesheet: &CSSStyleSheet, - mediarule: Arc>) -> Root { + mediarule: Arc>) -> Root { reflect_dom_object(box CSSMediaRule::new_inherited(parent_stylesheet, mediarule), window, CSSMediaRuleBinding::Wrap) } fn medialist(&self) -> Root { - self.medialist.or_init(|| MediaList::new(self.global().as_window(), - self.cssconditionrule.parent_stylesheet(), - self.mediarule.read().media_queries.clone())) + self.medialist.or_init(|| { + let guard = self.cssconditionrule.shared_lock().read(); + MediaList::new(self.global().as_window(), + self.cssconditionrule.parent_stylesheet(), + self.mediarule.read_with(&guard).media_queries.clone()) + }) } /// https://drafts.csswg.org/css-conditional-3/#the-cssmediarule-interface pub fn get_condition_text(&self) -> DOMString { let guard = self.cssconditionrule.shared_lock().read(); - let rule = self.mediarule.read(); + let rule = self.mediarule.read_with(&guard); let list = rule.media_queries.read_with(&guard); list.to_css_string().into() } /// https://drafts.csswg.org/css-conditional-3/#the-cssmediarule-interface pub fn set_condition_text(&self, text: DOMString) { - let mut guard = self.cssconditionrule.shared_lock().write(); let mut input = Parser::new(&text); let new_medialist = parse_media_query_list(&mut input); - let rule = self.mediarule.read(); - let mut list = rule.media_queries.write_with(&mut guard); - *list = new_medialist; + let mut guard = self.cssconditionrule.shared_lock().write(); + + // Clone an Arc because we can’t borrow `guard` twice at the same time. + + // FIXME(SimonSapin): allow access to multiple objects with one write guard? + // Would need a set of usize pointer addresses or something, + // the same object is not accessed more than once. + let mqs = Arc::clone(&self.mediarule.write_with(&mut guard).media_queries); + + *mqs.write_with(&mut guard) = new_medialist; } } @@ -81,7 +90,7 @@ impl SpecificCSSRule for CSSMediaRule { fn get_css(&self) -> DOMString { let guard = self.cssconditionrule.shared_lock().read(); - self.mediarule.read().to_css_string(&guard).into() + self.mediarule.read_with(&guard).to_css_string(&guard).into() } } diff --git a/components/script/dom/cssnamespacerule.rs b/components/script/dom/cssnamespacerule.rs index 46702c275ba..744a8020667 100644 --- a/components/script/dom/cssnamespacerule.rs +++ b/components/script/dom/cssnamespacerule.rs @@ -11,20 +11,19 @@ use dom::cssrule::{CSSRule, SpecificCSSRule}; use dom::cssstylesheet::CSSStyleSheet; use dom::window::Window; use dom_struct::dom_struct; -use parking_lot::RwLock; use std::sync::Arc; -use style::shared_lock::ToCssWithGuard; +use style::shared_lock::{Locked, ToCssWithGuard}; use style::stylesheets::NamespaceRule; #[dom_struct] pub struct CSSNamespaceRule { cssrule: CSSRule, #[ignore_heap_size_of = "Arc"] - namespacerule: Arc>, + namespacerule: Arc>, } impl CSSNamespaceRule { - fn new_inherited(parent_stylesheet: &CSSStyleSheet, namespacerule: Arc>) + fn new_inherited(parent_stylesheet: &CSSStyleSheet, namespacerule: Arc>) -> CSSNamespaceRule { CSSNamespaceRule { cssrule: CSSRule::new_inherited(parent_stylesheet), @@ -34,7 +33,7 @@ impl CSSNamespaceRule { #[allow(unrooted_must_root)] pub fn new(window: &Window, parent_stylesheet: &CSSStyleSheet, - namespacerule: Arc>) -> Root { + namespacerule: Arc>) -> Root { reflect_dom_object(box CSSNamespaceRule::new_inherited(parent_stylesheet, namespacerule), window, CSSNamespaceRuleBinding::Wrap) @@ -44,14 +43,16 @@ impl CSSNamespaceRule { impl CSSNamespaceRuleMethods for CSSNamespaceRule { // https://drafts.csswg.org/cssom/#dom-cssnamespacerule-prefix fn Prefix(&self) -> DOMString { - self.namespacerule.read().prefix + let guard = self.cssrule.shared_lock().read(); + self.namespacerule.read_with(&guard).prefix .as_ref().map(|s| s.to_string().into()) .unwrap_or(DOMString::new()) } // https://drafts.csswg.org/cssom/#dom-cssnamespacerule-namespaceuri fn NamespaceURI(&self) -> DOMString { - (*self.namespacerule.read().url).into() + let guard = self.cssrule.shared_lock().read(); + (*self.namespacerule.read_with(&guard).url).into() } } @@ -63,6 +64,6 @@ impl SpecificCSSRule for CSSNamespaceRule { fn get_css(&self) -> DOMString { let guard = self.cssrule.shared_lock().read(); - self.namespacerule.read().to_css_string(&guard).into() + self.namespacerule.read_with(&guard).to_css_string(&guard).into() } } diff --git a/components/script/dom/cssrulelist.rs b/components/script/dom/cssrulelist.rs index 6321296c785..bc77361b5ae 100644 --- a/components/script/dom/cssrulelist.rs +++ b/components/script/dom/cssrulelist.rs @@ -13,7 +13,6 @@ use dom::cssrule::CSSRule; use dom::cssstylesheet::CSSStyleSheet; use dom::window::Window; use dom_struct::dom_struct; -use parking_lot::RwLock; use std::sync::Arc; use style::shared_lock::Locked; use style::stylesheets::{CssRules, KeyframesRule, RulesMutateError}; @@ -45,19 +44,19 @@ pub struct CSSRuleList { pub enum RulesSource { Rules(Arc>), - Keyframes(Arc>), + Keyframes(Arc>), } impl CSSRuleList { #[allow(unrooted_must_root)] pub fn new_inherited(parent_stylesheet: &CSSStyleSheet, rules: RulesSource) -> CSSRuleList { + let guard = parent_stylesheet.shared_lock().read(); let dom_rules = match rules { RulesSource::Rules(ref rules) => { - let guard = parent_stylesheet.shared_lock().read(); rules.read_with(&guard).0.iter().map(|_| MutNullableJS::new(None)).collect() } RulesSource::Keyframes(ref rules) => { - rules.read().keyframes.iter().map(|_| MutNullableJS::new(None)).collect() + rules.read_with(&guard).keyframes.iter().map(|_| MutNullableJS::new(None)).collect() } }; @@ -104,10 +103,10 @@ impl CSSRuleList { // In case of a keyframe rule, index must be valid. pub fn remove_rule(&self, index: u32) -> ErrorResult { let index = index as usize; + let mut guard = self.parent_stylesheet.shared_lock().write(); match self.rules { RulesSource::Rules(ref css_rules) => { - let mut guard = self.parent_stylesheet.shared_lock().write(); css_rules.write_with(&mut guard).remove_rule(index)?; let mut dom_rules = self.dom_rules.borrow_mut(); dom_rules[index].get().map(|r| r.detach()); @@ -119,7 +118,7 @@ impl CSSRuleList { let mut dom_rules = self.dom_rules.borrow_mut(); dom_rules[index].get().map(|r| r.detach()); dom_rules.remove(index); - kf.write().keyframes.remove(index); + kf.write_with(&mut guard).keyframes.remove(index); Ok(()) } } @@ -136,9 +135,9 @@ impl CSSRuleList { self.dom_rules.borrow().get(idx as usize).map(|rule| { rule.or_init(|| { let parent_stylesheet = &self.parent_stylesheet; + let guard = parent_stylesheet.shared_lock().read(); match self.rules { RulesSource::Rules(ref rules) => { - let guard = parent_stylesheet.shared_lock().read(); CSSRule::new_specific(self.global().as_window(), parent_stylesheet, rules.read_with(&guard).0[idx as usize].clone()) @@ -146,7 +145,7 @@ impl CSSRuleList { RulesSource::Keyframes(ref rules) => { Root::upcast(CSSKeyframeRule::new(self.global().as_window(), parent_stylesheet, - rules.read() + rules.read_with(&guard) .keyframes[idx as usize] .clone())) } diff --git a/components/script/dom/csssupportsrule.rs b/components/script/dom/csssupportsrule.rs index d5e844e52aa..7ba3a24d038 100644 --- a/components/script/dom/csssupportsrule.rs +++ b/components/script/dom/csssupportsrule.rs @@ -13,10 +13,9 @@ use dom::cssrule::SpecificCSSRule; use dom::cssstylesheet::CSSStyleSheet; use dom::window::Window; use dom_struct::dom_struct; -use parking_lot::RwLock; use std::sync::Arc; use style::parser::ParserContext; -use style::shared_lock::ToCssWithGuard; +use style::shared_lock::{Locked, ToCssWithGuard}; use style::stylesheets::SupportsRule; use style::supports::SupportsCondition; use style_traits::ToCss; @@ -25,13 +24,14 @@ use style_traits::ToCss; pub struct CSSSupportsRule { cssconditionrule: CSSConditionRule, #[ignore_heap_size_of = "Arc"] - supportsrule: Arc>, + supportsrule: Arc>, } impl CSSSupportsRule { - fn new_inherited(parent_stylesheet: &CSSStyleSheet, supportsrule: Arc>) + fn new_inherited(parent_stylesheet: &CSSStyleSheet, supportsrule: Arc>) -> CSSSupportsRule { - let list = supportsrule.read().rules.clone(); + let guard = parent_stylesheet.shared_lock().read(); + let list = supportsrule.read_with(&guard).rules.clone(); CSSSupportsRule { cssconditionrule: CSSConditionRule::new_inherited(parent_stylesheet, list), supportsrule: supportsrule, @@ -40,7 +40,7 @@ impl CSSSupportsRule { #[allow(unrooted_must_root)] pub fn new(window: &Window, parent_stylesheet: &CSSStyleSheet, - supportsrule: Arc>) -> Root { + supportsrule: Arc>) -> Root { reflect_dom_object(box CSSSupportsRule::new_inherited(parent_stylesheet, supportsrule), window, CSSSupportsRuleBinding::Wrap) @@ -48,7 +48,8 @@ impl CSSSupportsRule { /// https://drafts.csswg.org/css-conditional-3/#the-csssupportsrule-interface pub fn get_condition_text(&self) -> DOMString { - let rule = self.supportsrule.read(); + let guard = self.cssconditionrule.shared_lock().read(); + let rule = self.supportsrule.read_with(&guard); rule.condition.to_css_string().into() } @@ -62,7 +63,8 @@ impl CSSSupportsRule { let url = win.Document().url(); let context = ParserContext::new_for_cssom(&url, win.css_error_reporter()); let enabled = cond.eval(&context); - let mut rule = self.supportsrule.write(); + let mut guard = self.cssconditionrule.shared_lock().write(); + let rule = self.supportsrule.write_with(&mut guard); rule.condition = cond; rule.enabled = enabled; } @@ -77,6 +79,6 @@ impl SpecificCSSRule for CSSSupportsRule { fn get_css(&self) -> DOMString { let guard = self.cssconditionrule.shared_lock().read(); - self.supportsrule.read().to_css_string(&guard).into() + self.supportsrule.read_with(&guard).to_css_string(&guard).into() } } diff --git a/components/script/dom/cssviewportrule.rs b/components/script/dom/cssviewportrule.rs index 855c8bdda57..38abf909ff0 100644 --- a/components/script/dom/cssviewportrule.rs +++ b/components/script/dom/cssviewportrule.rs @@ -10,20 +10,19 @@ use dom::cssrule::{CSSRule, SpecificCSSRule}; use dom::cssstylesheet::CSSStyleSheet; use dom::window::Window; use dom_struct::dom_struct; -use parking_lot::RwLock; use std::sync::Arc; -use style::shared_lock::ToCssWithGuard; +use style::shared_lock::{Locked, ToCssWithGuard}; use style::viewport::ViewportRule; #[dom_struct] pub struct CSSViewportRule { cssrule: CSSRule, #[ignore_heap_size_of = "Arc"] - viewportrule: Arc>, + viewportrule: Arc>, } impl CSSViewportRule { - fn new_inherited(parent_stylesheet: &CSSStyleSheet, viewportrule: Arc>) -> CSSViewportRule { + fn new_inherited(parent_stylesheet: &CSSStyleSheet, viewportrule: Arc>) -> CSSViewportRule { CSSViewportRule { cssrule: CSSRule::new_inherited(parent_stylesheet), viewportrule: viewportrule, @@ -32,7 +31,7 @@ impl CSSViewportRule { #[allow(unrooted_must_root)] pub fn new(window: &Window, parent_stylesheet: &CSSStyleSheet, - viewportrule: Arc>) -> Root { + viewportrule: Arc>) -> Root { reflect_dom_object(box CSSViewportRule::new_inherited(parent_stylesheet, viewportrule), window, CSSViewportRuleBinding::Wrap) @@ -47,6 +46,6 @@ impl SpecificCSSRule for CSSViewportRule { fn get_css(&self) -> DOMString { let guard = self.cssrule.shared_lock().read(); - self.viewportrule.read().to_css_string(&guard).into() + self.viewportrule.read_with(&guard).to_css_string(&guard).into() } } diff --git a/components/script/dom/htmlmetaelement.rs b/components/script/dom/htmlmetaelement.rs index a564c6b2661..bc6f37561d8 100644 --- a/components/script/dom/htmlmetaelement.rs +++ b/components/script/dom/htmlmetaelement.rs @@ -19,7 +19,6 @@ use dom::node::{Node, UnbindContext, document_from_node, window_from_node}; use dom::virtualmethods::VirtualMethods; use dom_struct::dom_struct; use html5ever_atoms::LocalName; -use parking_lot::RwLock; use servo_config::prefs::PREFS; use std::ascii::AsciiExt; use std::sync::Arc; @@ -101,7 +100,7 @@ impl HTMLMetaElement { if let Some(translated_rule) = ViewportRule::from_meta(&**content) { let document = self.upcast::().owner_doc(); let shared_lock = document.style_shared_lock(); - let rule = CssRule::Viewport(Arc::new(RwLock::new(translated_rule))); + let rule = CssRule::Viewport(Arc::new(shared_lock.wrap(translated_rule))); *self.stylesheet.borrow_mut() = Some(Arc::new(Stylesheet { rules: CssRules::new(vec![rule], shared_lock), origin: Origin::Author, diff --git a/components/script/stylesheet_loader.rs b/components/script/stylesheet_loader.rs index 79b90be7426..db21ee795fd 100644 --- a/components/script/stylesheet_loader.rs +++ b/components/script/stylesheet_loader.rs @@ -22,13 +22,13 @@ use ipc_channel::router::ROUTER; use net_traits::{FetchResponseListener, FetchMetadata, FilteredMetadata, Metadata, NetworkError, ReferrerPolicy}; use net_traits::request::{CorsSettings, CredentialsMode, Destination, RequestInit, RequestMode, Type as RequestType}; use network_listener::{NetworkListener, PreInvoke}; -use parking_lot::RwLock; use script_layout_interface::message::Msg; use servo_url::ServoUrl; use std::mem; use std::sync::{Arc, Mutex}; use style::media_queries::MediaList; use style::parser::ParserContextExtraData; +use style::shared_lock::Locked as StyleLocked; use style::stylesheets::{ImportRule, Stylesheet, Origin}; use style::stylesheets::StylesheetLoader as StyleStylesheetLoader; @@ -55,15 +55,16 @@ pub trait StylesheetOwner { pub enum StylesheetContextSource { // NB: `media` is just an option so we avoid cloning it. LinkElement { media: Option, url: ServoUrl }, - Import(Arc>), + Import(Arc>), } impl StylesheetContextSource { - fn url(&self) -> ServoUrl { + fn url(&self, document: &Document) -> ServoUrl { match *self { StylesheetContextSource::LinkElement { ref url, .. } => url.clone(), StylesheetContextSource::Import(ref import) => { - let import = import.read(); + let guard = document.style_shared_lock().read(); + let import = import.read_with(&guard); // Look at the parser in style::stylesheets, where we don't // trigger a load if the url is invalid. import.url.url() @@ -174,9 +175,16 @@ impl FetchResponseListener for StylesheetContext { } } StylesheetContextSource::Import(ref import) => { - let import = import.read(); let mut guard = document.style_shared_lock().write(); - Stylesheet::update_from_bytes(&import.stylesheet, + + // Clone an Arc because we can’t borrow `guard` twice at the same time. + + // FIXME(SimonSapin): allow access to multiple objects with one write guard? + // Would need a set of usize pointer addresses or something, + // the same object is not accessed more than once. + let stylesheet = Arc::clone(&import.write_with(&mut guard).stylesheet); + + Stylesheet::update_from_bytes(&stylesheet, &data, protocol_encoding_label, Some(environment_encoding), @@ -201,7 +209,7 @@ impl FetchResponseListener for StylesheetContext { document.decrement_script_blocking_stylesheet_count(); } - let url = self.source.url(); + let url = self.source.url(&document); document.finish_load(LoadType::Stylesheet(url)); if let Some(any_failed) = owner.load_finished(successful) { @@ -226,8 +234,8 @@ impl<'a> StylesheetLoader<'a> { impl<'a> StylesheetLoader<'a> { pub fn load(&self, source: StylesheetContextSource, cors_setting: Option, integrity_metadata: String) { - let url = source.url(); let document = document_from_node(self.elem); + let url = source.url(&document); let gen = self.elem.downcast::() .map(HTMLLinkElement::get_request_generation_id); let context = Arc::new(Mutex::new(StylesheetContext { @@ -289,7 +297,7 @@ impl<'a> StylesheetLoader<'a> { } impl<'a> StyleStylesheetLoader for StylesheetLoader<'a> { - fn request_stylesheet(&self, import: &Arc>) { + fn request_stylesheet(&self, import: &Arc>) { //TODO (mrnayak) : Whether we should use the original loader's CORS setting? //Fix this when spec has more details. self.load(StylesheetContextSource::Import(import.clone()), None, "".to_owned()) diff --git a/components/style/gecko/arc_types.rs b/components/style/gecko/arc_types.rs index 798af00906e..58ec43cf387 100644 --- a/components/style/gecko/arc_types.rs +++ b/components/style/gecko/arc_types.rs @@ -51,10 +51,21 @@ impl_arc_ffi!(ComputedValues => ServoComputedValues impl_arc_ffi!(RwLock => RawServoDeclarationBlock [Servo_DeclarationBlock_AddRef, Servo_DeclarationBlock_Release]); +/// FIXME: Remove once StyleRule is actually in Locked<_> +pub trait HackHackHack { + fn as_arc<'a>(ptr: &'a &RawServoStyleRule) -> &'a ::std::sync::Arc>; +} + +impl HackHackHack for Locked { + fn as_arc<'a>(ptr: &'a &RawServoStyleRule) -> &'a ::std::sync::Arc> { + RwLock::::as_arc(ptr) + } +} + impl_arc_ffi!(RwLock => RawServoStyleRule [Servo_StyleRule_AddRef, Servo_StyleRule_Release]); -impl_arc_ffi!(RwLock => RawServoImportRule +impl_arc_ffi!(Locked => RawServoImportRule [Servo_ImportRule_AddRef, Servo_ImportRule_Release]); impl_arc_ffi!(AnimationValue => RawServoAnimationValue @@ -66,8 +77,8 @@ impl_arc_ffi!(RwLock => RawServoAnimationValueMap impl_arc_ffi!(Locked => RawServoMediaList [Servo_MediaList_AddRef, Servo_MediaList_Release]); -impl_arc_ffi!(RwLock => RawServoMediaRule +impl_arc_ffi!(Locked => RawServoMediaRule [Servo_MediaRule_AddRef, Servo_MediaRule_Release]); -impl_arc_ffi!(RwLock => RawServoNamespaceRule +impl_arc_ffi!(Locked => RawServoNamespaceRule [Servo_NamespaceRule_AddRef, Servo_NamespaceRule_Release]); diff --git a/components/style/stylesheets.rs b/components/style/stylesheets.rs index 07dc2632b2d..a0f0baa3840 100644 --- a/components/style/stylesheets.rs +++ b/components/style/stylesheets.rs @@ -213,14 +213,14 @@ pub enum CssRule { // No Charset here, CSSCharsetRule has been removed from CSSOM // https://drafts.csswg.org/cssom/#changes-from-5-december-2013 - Namespace(Arc>), - Import(Arc>), + Namespace(Arc>), + Import(Arc>), Style(Arc>), - Media(Arc>), - FontFace(Arc>), - Viewport(Arc>), - Keyframes(Arc>), - Supports(Arc>), + Media(Arc>), + FontFace(Arc>), + Viewport(Arc>), + Keyframes(Arc>), + Supports(Arc>), } #[allow(missing_docs)] @@ -301,7 +301,7 @@ impl CssRule { where F: FnMut(&[CssRule], Option<&MediaList>) -> R { match *self { CssRule::Import(ref lock) => { - let rule = lock.read(); + let rule = lock.read_with(guard); let media = rule.stylesheet.media.read_with(guard); let rules = rule.stylesheet.rules.read_with(guard); // FIXME(emilio): Include the nested rules if the stylesheet is @@ -316,13 +316,13 @@ impl CssRule { f(&[], None) } CssRule::Media(ref lock) => { - let media_rule = lock.read(); + let media_rule = lock.read_with(guard); let mq = media_rule.media_queries.read_with(guard); let rules = &media_rule.rules.read_with(guard).0; f(rules, Some(&mq)) } CssRule::Supports(ref lock) => { - let supports_rule = lock.read(); + let supports_rule = lock.read_with(guard); let enabled = supports_rule.enabled; if enabled { let rules = &supports_rule.rules.read_with(guard).0; @@ -378,14 +378,14 @@ impl ToCssWithGuard for CssRule { fn to_css(&self, guard: &SharedRwLockReadGuard, dest: &mut W) -> fmt::Result where W: fmt::Write { match *self { - CssRule::Namespace(ref lock) => lock.read().to_css(guard, dest), - CssRule::Import(ref lock) => lock.read().to_css(guard, dest), + CssRule::Namespace(ref lock) => lock.read_with(guard).to_css(guard, dest), + CssRule::Import(ref lock) => lock.read_with(guard).to_css(guard, dest), CssRule::Style(ref lock) => lock.read().to_css(guard, dest), - CssRule::FontFace(ref lock) => lock.read().to_css(guard, dest), - CssRule::Viewport(ref lock) => lock.read().to_css(guard, dest), - CssRule::Keyframes(ref lock) => lock.read().to_css(guard, dest), - CssRule::Media(ref lock) => lock.read().to_css(guard, dest), - CssRule::Supports(ref lock) => lock.read().to_css(guard, dest), + CssRule::FontFace(ref lock) => lock.read_with(guard).to_css(guard, dest), + CssRule::Viewport(ref lock) => lock.read_with(guard).to_css(guard, dest), + CssRule::Keyframes(ref lock) => lock.read_with(guard).to_css(guard, dest), + CssRule::Media(ref lock) => lock.read_with(guard).to_css(guard, dest), + CssRule::Supports(ref lock) => lock.read_with(guard).to_css(guard, dest), } } } @@ -717,7 +717,7 @@ macro_rules! rule_filter { where F: FnMut(&$rule_type) { self.effective_rules(device, guard, |rule| { if let CssRule::$variant(ref lock) = *rule { - let rule = lock.read(); + let rule = lock.read_with(guard); f(&rule) } }) @@ -736,6 +736,18 @@ rule_filter! { effective_supports_rules(Supports => SupportsRule), } +/// FIXME: Remove once StyleRule is actually in Locked<_> +pub trait RwLockStyleRulePretendLockedStyleRule { + /// Pretend we’re Locked<_> + fn read_with(&self, guard: &SharedRwLockReadGuard) -> ::parking_lot::RwLockReadGuard; +} + +impl RwLockStyleRulePretendLockedStyleRule for RwLock { + fn read_with(&self, _: &SharedRwLockReadGuard) -> ::parking_lot::RwLockReadGuard { + self.read() + } +} + /// The stylesheet loader is the abstraction used to trigger network requests /// for `@import` rules. pub trait StylesheetLoader { @@ -743,7 +755,7 @@ pub trait StylesheetLoader { /// /// The called code is responsible to update the `stylesheet` rules field /// when the sheet is done loading. - fn request_stylesheet(&self, import: &Arc>); + fn request_stylesheet(&self, import: &Arc>); } struct TopLevelRuleParser<'a> { @@ -811,7 +823,7 @@ impl<'a> AtRuleParser for TopLevelRuleParser<'a> { let is_valid_url = url.url().is_some(); - let import_rule = Arc::new(RwLock::new( + let import_rule = Arc::new(self.shared_lock.wrap( ImportRule { url: url, stylesheet: Arc::new(Stylesheet { @@ -855,12 +867,12 @@ impl<'a> AtRuleParser for TopLevelRuleParser<'a> { None }; - return Ok(AtRuleType::WithoutBlock(CssRule::Namespace(Arc::new(RwLock::new( - NamespaceRule { + return Ok(AtRuleType::WithoutBlock(CssRule::Namespace(Arc::new( + self.shared_lock.wrap(NamespaceRule { prefix: opt_prefix, url: url, - } - ))))) + }) + )))) } else { self.state.set(State::Invalid); return Err(()) // "@namespace must be before any rule but @charset and @import" @@ -973,29 +985,29 @@ impl<'a, 'b> AtRuleParser for NestedRuleParser<'a, 'b> { fn parse_block(&mut self, prelude: AtRulePrelude, input: &mut Parser) -> Result { match prelude { AtRulePrelude::FontFace => { - Ok(CssRule::FontFace(Arc::new(RwLock::new( + Ok(CssRule::FontFace(Arc::new(self.shared_lock.wrap( try!(parse_font_face_block(self.context, input)))))) } AtRulePrelude::Media(media_queries) => { - Ok(CssRule::Media(Arc::new(RwLock::new(MediaRule { + Ok(CssRule::Media(Arc::new(self.shared_lock.wrap(MediaRule { media_queries: media_queries, rules: self.parse_nested_rules(input), })))) } AtRulePrelude::Supports(cond) => { let enabled = cond.eval(self.context); - Ok(CssRule::Supports(Arc::new(RwLock::new(SupportsRule { + Ok(CssRule::Supports(Arc::new(self.shared_lock.wrap(SupportsRule { condition: cond, rules: self.parse_nested_rules(input), enabled: enabled, })))) } AtRulePrelude::Viewport => { - Ok(CssRule::Viewport(Arc::new(RwLock::new( + Ok(CssRule::Viewport(Arc::new(self.shared_lock.wrap( try!(ViewportRule::parse(input, self.context)))))) } AtRulePrelude::Keyframes(name) => { - Ok(CssRule::Keyframes(Arc::new(RwLock::new(KeyframesRule { + Ok(CssRule::Keyframes(Arc::new(self.shared_lock.wrap(KeyframesRule { name: name, keyframes: parse_keyframe_list(&self.context, input), })))) diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 73684a4b8fd..4e6b6350beb 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -274,11 +274,11 @@ impl Stylist { } } CssRule::Import(ref import) => { - let import = import.read(); + let import = import.read_with(guard); self.add_stylesheet(&import.stylesheet, guard) } CssRule::Keyframes(ref keyframes_rule) => { - let keyframes_rule = keyframes_rule.read(); + let keyframes_rule = keyframes_rule.read_with(guard); debug!("Found valid keyframes rule: {:?}", *keyframes_rule); let animation = KeyframesAnimation::from_keyframes(&keyframes_rule.keyframes); debug!("Found valid keyframe animation: {:?}", animation); diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index e260f365fa5..24380439062 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -23,6 +23,7 @@ use style::context::{ThreadLocalStyleContext, ThreadLocalStyleContextCreationInf use style::data::{ElementData, ElementStyles, RestyleData}; use style::dom::{ShowSubtreeData, TElement, TNode}; use style::error_reporting::StdoutErrorReporter; +use style::gecko::arc_types::HackHackHack; use style::gecko::data::{PerDocumentStyleData, PerDocumentStyleDataImpl}; use style::gecko::restyle_damage::GeckoRestyleDamage; use style::gecko::selector_parser::{SelectorImpl, PseudoElement}; @@ -81,6 +82,7 @@ use style::string_cache::Atom; use style::stylesheets::{CssRule, CssRules, ImportRule, MediaRule, NamespaceRule}; use style::stylesheets::{Origin, Stylesheet, StyleRule}; use style::stylesheets::StylesheetLoader as StyleStylesheetLoader; +use style::stylesheets::RwLockStyleRulePretendLockedStyleRule; use style::supports::parse_condition_or_declaration; use style::thread_state; use style::timer::Timer; @@ -586,17 +588,19 @@ macro_rules! impl_basic_rule_funcs { #[no_mangle] pub extern "C" fn $debug(rule: &$raw_type, result: *mut nsACString) { - let rule = RwLock::<$rule_type>::as_arc(&rule); + let global_style_data = &*GLOBAL_STYLE_DATA; + let guard = global_style_data.shared_lock.read(); + let rule = Locked::<$rule_type>::as_arc(&rule); let result = unsafe { result.as_mut().unwrap() }; - write!(result, "{:?}", *rule.read()).unwrap(); + write!(result, "{:?}", *rule.read_with(&guard)).unwrap(); } #[no_mangle] pub extern "C" fn $to_css(rule: &$raw_type, result: *mut nsAString) { let global_style_data = &*GLOBAL_STYLE_DATA; let guard = global_style_data.shared_lock.read(); - let rule = RwLock::<$rule_type>::as_arc(&rule); - rule.read().to_css(&guard, unsafe { result.as_mut().unwrap() }).unwrap(); + let rule = Locked::<$rule_type>::as_arc(&rule); + rule.read_with(&guard).to_css(&guard, unsafe { result.as_mut().unwrap() }).unwrap(); } } } @@ -641,26 +645,34 @@ pub extern "C" fn Servo_StyleRule_GetSelectorText(rule: RawServoStyleRuleBorrowe #[no_mangle] pub extern "C" fn Servo_MediaRule_GetMedia(rule: RawServoMediaRuleBorrowed) -> RawServoMediaListStrong { - let rule = RwLock::::as_arc(&rule); - rule.read().media_queries.clone().into_strong() + let global_style_data = &*GLOBAL_STYLE_DATA; + let guard = global_style_data.shared_lock.read(); + let rule = Locked::::as_arc(&rule); + rule.read_with(&guard).media_queries.clone().into_strong() } #[no_mangle] pub extern "C" fn Servo_MediaRule_GetRules(rule: RawServoMediaRuleBorrowed) -> ServoCssRulesStrong { - let rule = RwLock::::as_arc(&rule); - rule.read().rules.clone().into_strong() + let global_style_data = &*GLOBAL_STYLE_DATA; + let guard = global_style_data.shared_lock.read(); + let rule = Locked::::as_arc(&rule); + rule.read_with(&guard).rules.clone().into_strong() } #[no_mangle] pub extern "C" fn Servo_NamespaceRule_GetPrefix(rule: RawServoNamespaceRuleBorrowed) -> *mut nsIAtom { - let rule = RwLock::::as_arc(&rule); - rule.read().prefix.as_ref().unwrap_or(&atom!("")).as_ptr() + let global_style_data = &*GLOBAL_STYLE_DATA; + let guard = global_style_data.shared_lock.read(); + let rule = Locked::::as_arc(&rule); + rule.read_with(&guard).prefix.as_ref().unwrap_or(&atom!("")).as_ptr() } #[no_mangle] pub extern "C" fn Servo_NamespaceRule_GetURI(rule: RawServoNamespaceRuleBorrowed) -> *mut nsIAtom { - let rule = RwLock::::as_arc(&rule); - rule.read().url.0.as_ptr() + let global_style_data = &*GLOBAL_STYLE_DATA; + let guard = global_style_data.shared_lock.read(); + let rule = Locked::::as_arc(&rule); + rule.read_with(&guard).url.0.as_ptr() } #[no_mangle] @@ -1404,8 +1416,10 @@ pub extern "C" fn Servo_NoteExplicitHints(element: RawGeckoElementBorrowed, pub extern "C" fn Servo_ImportRule_GetSheet(import_rule: RawServoImportRuleBorrowed) -> RawServoStyleSheetStrong { - let import_rule = RwLock::::as_arc(&import_rule); - import_rule.read().stylesheet.clone().into_strong() + let global_style_data = &*GLOBAL_STYLE_DATA; + let guard = global_style_data.shared_lock.read(); + let import_rule = Locked::::as_arc(&import_rule); + import_rule.read_with(&guard).stylesheet.clone().into_strong() } #[no_mangle] diff --git a/ports/geckolib/stylesheet_loader.rs b/ports/geckolib/stylesheet_loader.rs index bcd55cce2fd..2e81de1a57e 100644 --- a/ports/geckolib/stylesheet_loader.rs +++ b/ports/geckolib/stylesheet_loader.rs @@ -2,11 +2,11 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -use parking_lot::RwLock; use std::sync::Arc; use style::gecko_bindings::bindings::Gecko_LoadStyleSheet; use style::gecko_bindings::structs::{Loader, ServoStyleSheet}; use style::gecko_bindings::sugar::ownership::HasArcFFI; +use style::shared_lock::Locked; use style::stylesheets::{ImportRule, StylesheetLoader as StyleStylesheetLoader}; use style_traits::ToCss; use super::glue::GLOBAL_STYLE_DATA; @@ -20,10 +20,10 @@ impl StylesheetLoader { } impl StyleStylesheetLoader for StylesheetLoader { - fn request_stylesheet(&self, import_rule: &Arc>) { + fn request_stylesheet(&self, import_rule: &Arc>) { let global_style_data = &*GLOBAL_STYLE_DATA; let guard = global_style_data.shared_lock.read(); - let import = import_rule.read(); + let import = import_rule.read_with(&guard); let (spec_bytes, spec_len) = import.url.as_slice_components() .expect("Import only loads valid URLs"); diff --git a/tests/unit/style/stylesheets.rs b/tests/unit/style/stylesheets.rs index 00cefe23ce9..a1c67932b53 100644 --- a/tests/unit/style/stylesheets.rs +++ b/tests/unit/style/stylesheets.rs @@ -77,7 +77,7 @@ fn test_parse_stylesheet() { dirty_on_viewport_size_change: AtomicBool::new(false), disabled: AtomicBool::new(false), rules: CssRules::new(vec![ - CssRule::Namespace(Arc::new(RwLock::new(NamespaceRule { + CssRule::Namespace(Arc::new(stylesheet.shared_lock.wrap(NamespaceRule { prefix: None, url: NsAtom::from("http://www.w3.org/1999/xhtml") }))), @@ -235,7 +235,7 @@ fn test_parse_stylesheet() { Importance::Normal), ]))), }))), - CssRule::Keyframes(Arc::new(RwLock::new(KeyframesRule { + CssRule::Keyframes(Arc::new(stylesheet.shared_lock.wrap(KeyframesRule { name: "foo".into(), keyframes: vec![ Arc::new(RwLock::new(Keyframe {