diff options
author | Simon Sapin <simon.sapin@exyr.org> | 2017-03-14 16:09:41 +0100 |
---|---|---|
committer | Simon Sapin <simon.sapin@exyr.org> | 2017-03-19 22:30:31 +0100 |
commit | c5a7294e05a3b370981170ea273acb60b6116bc0 (patch) | |
tree | dbac59e9b0f90ecc0dd907d430660ef6f6f8abce | |
parent | 8feb9e80477376a529b8c5ce9951e78f6048f054 (diff) | |
download | servo-c5a7294e05a3b370981170ea273acb60b6116bc0.tar.gz servo-c5a7294e05a3b370981170ea273acb60b6116bc0.zip |
Replace RwLock<MediaList> with shared_lock::Locked<MediaList>
-rw-r--r-- | components/layout_thread/lib.rs | 17 | ||||
-rw-r--r-- | components/script/dom/bindings/trace.rs | 2 | ||||
-rw-r--r-- | components/script/dom/document.rs | 16 | ||||
-rw-r--r-- | components/script/dom/htmlmetaelement.rs | 5 | ||||
-rw-r--r-- | components/script/dom/htmlstyleelement.rs | 3 | ||||
-rw-r--r-- | components/script/layout_wrapper.rs | 5 | ||||
-rw-r--r-- | components/script/stylesheet_loader.rs | 2 | ||||
-rw-r--r-- | components/style/encoding_support.rs | 3 | ||||
-rw-r--r-- | components/style/stylesheets.rs | 37 | ||||
-rw-r--r-- | components/style/stylist.rs | 15 | ||||
-rw-r--r-- | ports/geckolib/glue.rs | 17 | ||||
-rw-r--r-- | ports/geckolib/stylesheet_loader.rs | 5 | ||||
-rw-r--r-- | tests/unit/style/media_queries.rs | 14 | ||||
-rw-r--r-- | tests/unit/style/rule_tree/bench.rs | 2 | ||||
-rw-r--r-- | tests/unit/style/stylesheets.rs | 8 | ||||
-rw-r--r-- | tests/unit/style/viewport.rs | 2 | ||||
-rw-r--r-- | tests/unit/stylo/lib.rs | 3 | ||||
-rw-r--r-- | tests/unit/stylo/servo_function_signatures.rs | 3 |
18 files changed, 117 insertions, 42 deletions
diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs index 566d6e437cd..ee374d18fc8 100644 --- a/components/layout_thread/lib.rs +++ b/components/layout_thread/lib.rs @@ -115,6 +115,7 @@ use style::logical_geometry::LogicalPoint; use style::media_queries::{Device, MediaType}; use style::parser::ParserContextExtraData; use style::servo::restyle_damage::{REFLOW, REFLOW_OUT_OF_FLOW, REPAINT, REPOSITION, STORE_OVERFLOW}; +use style::shared_lock::SharedRwLock; use style::stylesheets::{Origin, Stylesheet, UserAgentStylesheets}; use style::stylist::Stylist; use style::thread_state; @@ -934,6 +935,7 @@ impl LayoutThread { possibly_locked_rw_data: &mut RwData<'a, 'b>) { let document = unsafe { ServoLayoutNode::new(&data.document) }; let document = document.as_document().unwrap(); + let style_guard = document.style_shared_lock().read(); self.quirks_mode = Some(document.quirks_mode()); // FIXME(pcwalton): Combine `ReflowGoal` and `ReflowQueryType`. Then remove this assert. @@ -1010,7 +1012,8 @@ impl LayoutThread { // Calculate the actual viewport as per DEVICE-ADAPT § 6 let device = Device::new(MediaType::Screen, initial_viewport); - Arc::get_mut(&mut rw_data.stylist).unwrap().set_device(device, &data.document_stylesheets); + Arc::get_mut(&mut rw_data.stylist).unwrap() + .set_device(device, &style_guard, &data.document_stylesheets); self.viewport_size = rw_data.stylist.viewport_constraints().map_or(current_screen_size, |constraints| { @@ -1528,7 +1531,8 @@ fn get_root_flow_background_color(flow: &mut Flow) -> webrender_traits::ColorF { } fn get_ua_stylesheets() -> Result<UserAgentStylesheets, &'static str> { - fn parse_ua_stylesheet(filename: &'static str) -> Result<Stylesheet, &'static str> { + fn parse_ua_stylesheet(shared_lock: &SharedRwLock, filename: &'static str) + -> Result<Stylesheet, &'static str> { let res = try!(read_resource_file(filename).map_err(|_| filename)); Ok(Stylesheet::from_bytes( &res, @@ -1537,26 +1541,29 @@ fn get_ua_stylesheets() -> Result<UserAgentStylesheets, &'static str> { None, Origin::UserAgent, Default::default(), + shared_lock.clone(), None, &StdoutErrorReporter, ParserContextExtraData::default())) } + let shared_lock = SharedRwLock::new(); let mut user_or_user_agent_stylesheets = vec!(); // FIXME: presentational-hints.css should be at author origin with zero specificity. // (Does it make a difference?) for &filename in &["user-agent.css", "servo.css", "presentational-hints.css"] { - user_or_user_agent_stylesheets.push(try!(parse_ua_stylesheet(filename))); + user_or_user_agent_stylesheets.push(try!(parse_ua_stylesheet(&shared_lock, filename))); } for &(ref contents, ref url) in &opts::get().user_stylesheets { user_or_user_agent_stylesheets.push(Stylesheet::from_bytes( &contents, url.clone(), None, None, Origin::User, Default::default(), - None, &StdoutErrorReporter, ParserContextExtraData::default())); + shared_lock.clone(), None, &StdoutErrorReporter, ParserContextExtraData::default())); } - let quirks_mode_stylesheet = try!(parse_ua_stylesheet("quirks-mode.css")); + let quirks_mode_stylesheet = try!(parse_ua_stylesheet(&shared_lock, "quirks-mode.css")); Ok(UserAgentStylesheets { + shared_lock: shared_lock, user_or_user_agent_stylesheets: user_or_user_agent_stylesheets, quirks_mode_stylesheet: quirks_mode_stylesheet, }) diff --git a/components/script/dom/bindings/trace.rs b/components/script/dom/bindings/trace.rs index 4d38b39d333..b0828ab6d56 100644 --- a/components/script/dom/bindings/trace.rs +++ b/components/script/dom/bindings/trace.rs @@ -99,6 +99,7 @@ use style::keyframes::Keyframe; use style::media_queries::MediaList; use style::properties::PropertyDeclarationBlock; use style::selector_parser::{PseudoElement, Snapshot}; +use style::shared_lock::SharedRwLock as StyleSharedRwLock; use style::stylesheets::{CssRules, KeyframesRule, MediaRule, NamespaceRule, StyleRule, ImportRule}; use style::stylesheets::SupportsRule; use style::values::specified::Length; @@ -360,6 +361,7 @@ unsafe_no_jsmanaged_fields!(HttpsState); unsafe_no_jsmanaged_fields!(Request); unsafe_no_jsmanaged_fields!(RequestInit); unsafe_no_jsmanaged_fields!(SharedRt); +unsafe_no_jsmanaged_fields!(StyleSharedRwLock); unsafe_no_jsmanaged_fields!(TouchpadPressurePhase); unsafe_no_jsmanaged_fields!(USVString); unsafe_no_jsmanaged_fields!(ReferrerPolicy); diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index b85366a27d2..7a50fe5cf6c 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -134,6 +134,7 @@ use style::attr::AttrValue; use style::context::{QuirksMode, ReflowGoal}; use style::restyle_hints::{RestyleHint, RESTYLE_STYLE_ATTRIBUTE}; use style::selector_parser::{RestyleDamage, Snapshot}; +use style::shared_lock::SharedRwLock as StyleSharedRwLock; use style::str::{HTML_SPACE_CHARACTERS, split_html_space_chars, str_join}; use style::stylesheets::Stylesheet; use task_source::TaskSource; @@ -220,6 +221,9 @@ pub struct Document { scripts: MutNullableJS<HTMLCollection>, anchors: MutNullableJS<HTMLCollection>, applets: MutNullableJS<HTMLCollection>, + /// Lock use for style attributes and author-origin stylesheet objects in this document. + /// Can be acquired once for accessing many objects. + style_shared_lock: StyleSharedRwLock, /// List of stylesheets associated with nodes in this document. |None| if the list needs to be refreshed. stylesheets: DOMRefCell<Option<Vec<StylesheetInDocument>>>, /// Whether the list of stylesheets has changed since the last reflow was triggered. @@ -1964,6 +1968,7 @@ pub trait LayoutDocumentHelpers { unsafe fn needs_paint_from_layout(&self); unsafe fn will_paint(&self); unsafe fn quirks_mode(&self) -> QuirksMode; + unsafe fn style_shared_lock(&self) -> &StyleSharedRwLock; } #[allow(unsafe_code)] @@ -2000,6 +2005,11 @@ impl LayoutDocumentHelpers for LayoutJS<Document> { unsafe fn quirks_mode(&self) -> QuirksMode { (*self.unsafe_get()).quirks_mode() } + + #[inline] + unsafe fn style_shared_lock(&self) -> &StyleSharedRwLock { + (*self.unsafe_get()).style_shared_lock() + } } // https://html.spec.whatwg.org/multipage/#is-a-registrable-domain-suffix-of-or-is-equal-to @@ -2121,6 +2131,7 @@ impl Document { scripts: Default::default(), anchors: Default::default(), applets: Default::default(), + style_shared_lock: StyleSharedRwLock::new(), stylesheets: DOMRefCell::new(None), stylesheets_changed_since_reflow: Cell::new(false), stylesheet_list: MutNullableJS::new(None), @@ -2250,6 +2261,11 @@ impl Document { }; } + /// Return a reference to the per-document shared lock used in stylesheets. + pub fn style_shared_lock(&self) -> &StyleSharedRwLock { + &self.style_shared_lock + } + /// Returns the list of stylesheets associated with nodes in the document. pub fn stylesheets(&self) -> Vec<Arc<Stylesheet>> { self.ensure_stylesheets(); diff --git a/components/script/dom/htmlmetaelement.rs b/components/script/dom/htmlmetaelement.rs index a751486de7d..2f78e74d3b4 100644 --- a/components/script/dom/htmlmetaelement.rs +++ b/components/script/dom/htmlmetaelement.rs @@ -99,12 +99,15 @@ impl HTMLMetaElement { let content = content.value(); if !content.is_empty() { if let Some(translated_rule) = ViewportRule::from_meta(&**content) { + let document = self.upcast::<Node>().owner_doc(); + let shared_lock = document.style_shared_lock(); *self.stylesheet.borrow_mut() = Some(Arc::new(Stylesheet { rules: CssRules::new(vec![CssRule::Viewport(Arc::new(RwLock::new(translated_rule)))]), origin: Origin::Author, + shared_lock: shared_lock.clone(), base_url: window_from_node(self).get_url(), namespaces: Default::default(), - media: Default::default(), + media: Arc::new(shared_lock.wrap(Default::default())), // Viewport constraints are always recomputed on resize; they don't need to // force all styles to be recomputed. dirty_on_viewport_size_change: AtomicBool::new(false), diff --git a/components/script/dom/htmlstyleelement.rs b/components/script/dom/htmlstyleelement.rs index f9c6412f854..13721feba58 100644 --- a/components/script/dom/htmlstyleelement.rs +++ b/components/script/dom/htmlstyleelement.rs @@ -84,9 +84,10 @@ impl HTMLStyleElement { let data = node.GetTextContent().expect("Element.textContent must be a string"); let mq = parse_media_query_list(&mut CssParser::new(&mq_str)); + let shared_lock = node.owner_doc().style_shared_lock().clone(); let loader = StylesheetLoader::for_element(self.upcast()); let sheet = Stylesheet::from_str(&data, url, Origin::Author, mq, - Some(&loader), + shared_lock, Some(&loader), win.css_error_reporter(), ParserContextExtraData::default()); diff --git a/components/script/layout_wrapper.rs b/components/script/layout_wrapper.rs index 98ce86745f2..5a9a834667b 100644 --- a/components/script/layout_wrapper.rs +++ b/components/script/layout_wrapper.rs @@ -69,6 +69,7 @@ use style::dom::UnsafeNode; use style::element_state::*; use style::properties::{ComputedValues, PropertyDeclarationBlock}; use style::selector_parser::{NonTSPseudoClass, PseudoElement, SelectorImpl}; +use style::shared_lock::SharedRwLock as StyleSharedRwLock; use style::sink::Push; use style::str::is_whitespace; use style::stylist::ApplicableDeclarationBlock; @@ -330,6 +331,10 @@ impl<'ld> ServoLayoutDocument<'ld> { unsafe { self.document.quirks_mode() } } + pub fn style_shared_lock(&self) -> &StyleSharedRwLock { + unsafe { self.document.style_shared_lock() } + } + pub fn from_layout_js(doc: LayoutJS<Document>) -> ServoLayoutDocument<'ld> { ServoLayoutDocument { document: doc, diff --git a/components/script/stylesheet_loader.rs b/components/script/stylesheet_loader.rs index cdc93c9808b..26dffa5fe56 100644 --- a/components/script/stylesheet_loader.rs +++ b/components/script/stylesheet_loader.rs @@ -152,12 +152,14 @@ impl FetchResponseListener for StylesheetContext { let is_stylesheet_load_applicable = self.request_generation_id.map_or(true, |gen| gen == link.get_request_generation_id()); if is_stylesheet_load_applicable { + let shared_lock = document.style_shared_lock().clone(); let sheet = Arc::new(Stylesheet::from_bytes(&data, final_url, protocol_encoding_label, Some(environment_encoding), Origin::Author, media.take().unwrap(), + shared_lock, Some(&loader), win.css_error_reporter(), ParserContextExtraData::default())); diff --git a/components/style/encoding_support.rs b/components/style/encoding_support.rs index 62323ec2314..141291c896e 100644 --- a/components/style/encoding_support.rs +++ b/components/style/encoding_support.rs @@ -12,6 +12,7 @@ use media_queries::MediaList; use parser::ParserContextExtraData; use self::encoding::{EncodingRef, DecoderTrap}; use servo_url::ServoUrl; +use shared_lock::SharedRwLock; use std::str; use stylesheets::{Stylesheet, StylesheetLoader, Origin}; @@ -54,6 +55,7 @@ impl Stylesheet { environment_encoding: Option<EncodingRef>, origin: Origin, media: MediaList, + shared_lock: SharedRwLock, stylesheet_loader: Option<&StylesheetLoader>, error_reporter: &ParseErrorReporter, extra_data: ParserContextExtraData) @@ -64,6 +66,7 @@ impl Stylesheet { base_url, origin, media, + shared_lock, stylesheet_loader, error_reporter, extra_data) diff --git a/components/style/stylesheets.rs b/components/style/stylesheets.rs index 5b5c47e152f..3d487ec8db1 100644 --- a/components/style/stylesheets.rs +++ b/components/style/stylesheets.rs @@ -21,6 +21,7 @@ use selector_parser::{SelectorImpl, SelectorParser}; use selectors::parser::SelectorList; use servo_config::prefs::PREFS; use servo_url::ServoUrl; +use shared_lock::{SharedRwLock, Locked, SharedRwLockReadGuard}; use std::cell::Cell; use std::fmt; use std::sync::Arc; @@ -175,11 +176,13 @@ pub struct Stylesheet { /// cascading order) pub rules: Arc<RwLock<CssRules>>, /// List of media associated with the Stylesheet. - pub media: Arc<RwLock<MediaList>>, + pub media: Arc<Locked<MediaList>>, /// The origin of this stylesheet. pub origin: Origin, /// The base url this stylesheet should use. pub base_url: ServoUrl, + /// The lock used for objects inside this stylesheet + pub shared_lock: SharedRwLock, /// The namespaces that apply to this stylesheet. pub namespaces: RwLock<Namespaces>, /// Whether this stylesheet would be dirty when the viewport size changes. @@ -191,6 +194,8 @@ pub struct Stylesheet { /// This structure holds the user-agent and user stylesheets. pub struct UserAgentStylesheets { + /// The lock used for user-agent stylesheets. + pub shared_lock: SharedRwLock, /// The user or user agent stylesheets. pub user_or_user_agent_stylesheets: Vec<Stylesheet>, /// The quirks mode stylesheet. @@ -291,12 +296,12 @@ impl CssRule { /// used for others. /// /// This will not recurse down unsupported @supports rules - pub fn with_nested_rules_and_mq<F, R>(&self, mut f: F) -> R + pub fn with_nested_rules_and_mq<F, R>(&self, guard: &SharedRwLockReadGuard, mut f: F) -> R where F: FnMut(&[CssRule], Option<&MediaList>) -> R { match *self { CssRule::Import(ref lock) => { let rule = lock.read(); - let media = rule.stylesheet.media.read(); + let media = rule.stylesheet.media.read_with(guard); let rules = rule.stylesheet.rules.read(); // FIXME(emilio): Include the nested rules if the stylesheet is // loaded. @@ -349,6 +354,7 @@ impl CssRule { let mut rule_parser = TopLevelRuleParser { stylesheet_origin: parent_stylesheet.origin, context: context, + shared_lock: &parent_stylesheet.shared_lock, loader: None, state: Cell::new(state), namespaces: &mut namespaces, @@ -424,7 +430,8 @@ impl ToCss for ImportRule { fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { try!(dest.write_str("@import ")); try!(self.url.to_css(dest)); - let media = self.stylesheet.media.read(); + let guard = self.stylesheet.shared_lock.read(); // FIXME: have the caller pass this? + let media = self.stylesheet.media.read_with(&guard); if !media.is_empty() { try!(dest.write_str(" ")); try!(media.to_css(dest)); @@ -554,6 +561,7 @@ impl Stylesheet { let rule_parser = TopLevelRuleParser { stylesheet_origin: existing.origin, namespaces: &mut namespaces, + shared_lock: &existing.shared_lock, loader: stylesheet_loader, context: ParserContext::new_with_extra_data(existing.origin, &existing.base_url, @@ -591,6 +599,7 @@ impl Stylesheet { base_url: ServoUrl, origin: Origin, media: MediaList, + shared_lock: SharedRwLock, stylesheet_loader: Option<&StylesheetLoader>, error_reporter: &ParseErrorReporter, extra_data: ParserContextExtraData) -> Stylesheet { @@ -599,7 +608,8 @@ impl Stylesheet { base_url: base_url, namespaces: RwLock::new(Namespaces::default()), rules: CssRules::new(vec![]), - media: Arc::new(RwLock::new(media)), + media: Arc::new(shared_lock.wrap(media)), + shared_lock: shared_lock.clone(), dirty_on_viewport_size_change: AtomicBool::new(false), disabled: AtomicBool::new(false), }; @@ -638,7 +648,8 @@ impl Stylesheet { /// /// Always true if no associated MediaList exists. pub fn is_effective_for_device(&self, device: &Device) -> bool { - self.media.read().evaluate(device) + let guard = self.shared_lock.read(); // FIXME: have the caller pass this? + self.media.read_with(&guard).evaluate(device) } /// Return an iterator over the effective rules within the style-sheet, as @@ -649,7 +660,8 @@ impl Stylesheet { /// examined. #[inline] pub fn effective_rules<F>(&self, device: &Device, mut f: F) where F: FnMut(&CssRule) { - effective_rules(&self.rules.read().0, device, &mut f); + let guard = self.shared_lock.read(); // FIXME: have the caller pass this? + effective_rules(&self.rules.read().0, device, &guard, &mut f); } /// Returns whether the stylesheet has been explicitly disabled through the @@ -670,16 +682,17 @@ impl Stylesheet { } } -fn effective_rules<F>(rules: &[CssRule], device: &Device, f: &mut F) where F: FnMut(&CssRule) { +fn effective_rules<F>(rules: &[CssRule], device: &Device, guard: &SharedRwLockReadGuard, f: &mut F) +where F: FnMut(&CssRule) { for rule in rules { f(rule); - rule.with_nested_rules_and_mq(|rules, mq| { + rule.with_nested_rules_and_mq(guard, |rules, mq| { if let Some(media_queries) = mq { if !media_queries.evaluate(device) { return } } - effective_rules(rules, device, f) + effective_rules(rules, device, guard, f) }) } } @@ -724,6 +737,7 @@ pub trait StylesheetLoader { struct TopLevelRuleParser<'a> { stylesheet_origin: Origin, namespaces: &'a mut Namespaces, + shared_lock: &'a SharedRwLock, loader: Option<&'a StylesheetLoader>, context: ParserContext<'a>, state: Cell<State>, @@ -780,7 +794,7 @@ impl<'a> AtRuleParser for TopLevelRuleParser<'a> { &self.context)); let media = - Arc::new(RwLock::new(parse_media_query_list(input))); + Arc::new(self.shared_lock.wrap(parse_media_query_list(input))); let is_valid_url = url.url().is_some(); @@ -790,6 +804,7 @@ impl<'a> AtRuleParser for TopLevelRuleParser<'a> { stylesheet: Arc::new(Stylesheet { rules: Arc::new(RwLock::new(CssRules(vec![]))), media: media, + shared_lock: self.shared_lock.clone(), origin: self.context.stylesheet_origin, base_url: self.context.base_url.clone(), namespaces: RwLock::new(Namespaces::default()), diff --git a/components/style/stylist.rs b/components/style/stylist.rs index f7dd149e7d3..e28bb641cd0 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -28,6 +28,7 @@ use selectors::matching::{AFFECTED_BY_STYLE_ATTRIBUTE, AFFECTED_BY_PRESENTATIONA use selectors::matching::{ElementSelectorFlags, StyleRelations, matches_complex_selector}; use selectors::parser::{Selector, SimpleSelector, LocalName as LocalNameSelector, ComplexSelector}; use selectors::parser::SelectorMethods; +#[cfg(feature = "servo")] use shared_lock::SharedRwLockReadGuard; use sink::Push; use smallvec::VecLike; use std::borrow::Borrow; @@ -461,7 +462,8 @@ impl Stylist { /// FIXME(emilio): The semantics of the device for Servo and Gecko are /// different enough we may want to unify them. #[cfg(feature = "servo")] - pub fn set_device(&mut self, mut device: Device, stylesheets: &[Arc<Stylesheet>]) { + pub fn set_device(&mut self, mut device: Device, guard: &SharedRwLockReadGuard, + stylesheets: &[Arc<Stylesheet>]) { let cascaded_rule = ViewportRule { declarations: viewport::Cascade::from_stylesheets(stylesheets, &device).finish(), }; @@ -473,15 +475,16 @@ impl Stylist { device.account_for_viewport_rule(constraints); } - fn mq_eval_changed(rules: &[CssRule], before: &Device, after: &Device) -> bool { + fn mq_eval_changed(guard: &SharedRwLockReadGuard, rules: &[CssRule], + before: &Device, after: &Device) -> bool { for rule in rules { - let changed = rule.with_nested_rules_and_mq(|rules, mq| { + let changed = rule.with_nested_rules_and_mq(guard, |rules, mq| { if let Some(mq) = mq { if mq.evaluate(before) != mq.evaluate(after) { return true } } - mq_eval_changed(rules, before, after) + mq_eval_changed(guard, rules, before, after) }); if changed { return true @@ -490,12 +493,12 @@ impl Stylist { false } self.is_device_dirty |= stylesheets.iter().any(|stylesheet| { - let mq = stylesheet.media.read(); + let mq = stylesheet.media.read_with(guard); if mq.evaluate(&self.device) != mq.evaluate(&device) { return true } - mq_eval_changed(&stylesheet.rules.read().0, &self.device, &device) + mq_eval_changed(guard, &stylesheet.rules.read().0, &self.device, &device) }); self.device = Arc::new(device); diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index 16d8b0c9f36..7330d8f63de 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -76,6 +76,7 @@ use style::properties::parse_one_declaration; use style::restyle_hints::{self, RestyleHint}; use style::selector_parser::PseudoElementCascadeType; use style::sequential; +use style::shared_lock::SharedRwLock; use style::string_cache::Atom; use style::stylesheets::{CssRule, CssRules, ImportRule, MediaRule, NamespaceRule}; use style::stylesheets::{Origin, Stylesheet, StyleRule}; @@ -85,7 +86,7 @@ use style::thread_state; use style::timer::Timer; use style::traversal::{resolve_style, DomTraversal, TraversalDriver}; use style_traits::ToCss; -use stylesheet_loader::StylesheetLoader; +use super::stylesheet_loader::StylesheetLoader; /* * For Gecko->Servo function calls, we need to redeclare the same signature that was declared in @@ -95,12 +96,15 @@ use stylesheet_loader::StylesheetLoader; * depend on but good enough for our purposes. */ -struct GlobalStyleData { +pub struct GlobalStyleData { // How many threads parallel styling can use. pub num_threads: usize, // The parallel styling thread pool. pub style_thread_pool: Option<rayon::ThreadPool>, + + // Shared RWLock for CSSOM objects + pub shared_lock: SharedRwLock, } impl GlobalStyleData { @@ -124,12 +128,13 @@ impl GlobalStyleData { GlobalStyleData { num_threads: num_threads, style_thread_pool: pool, + shared_lock: SharedRwLock::new(), } } } lazy_static! { - static ref GLOBAL_STYLE_DATA: GlobalStyleData = { + pub static ref GLOBAL_STYLE_DATA: GlobalStyleData = { GlobalStyleData::new() }; } @@ -335,8 +340,9 @@ pub extern "C" fn Servo_StyleSheet_Empty(mode: SheetParsingMode) -> RawServoStyl SheetParsingMode::eUserSheetFeatures => Origin::User, SheetParsingMode::eAgentSheetFeatures => Origin::UserAgent, }; + let shared_lock = GLOBAL_STYLE_DATA.shared_lock.clone(); Arc::new(Stylesheet::from_str( - "", url, origin, Default::default(), None, + "", url, origin, Default::default(), shared_lock, None, &StdoutErrorReporter, extra_data) ).into_strong() } @@ -378,8 +384,9 @@ pub extern "C" fn Servo_StyleSheet_FromUTF8Bytes(loader: *mut Loader, Some(ref s) => Some(s), }; + let shared_lock = GLOBAL_STYLE_DATA.shared_lock.clone(); Arc::new(Stylesheet::from_str( - input, url, origin, Default::default(), loader, + input, url, origin, Default::default(), shared_lock, loader, &StdoutErrorReporter, extra_data) ).into_strong() } diff --git a/ports/geckolib/stylesheet_loader.rs b/ports/geckolib/stylesheet_loader.rs index 3e6fb421c86..bcd55cce2fd 100644 --- a/ports/geckolib/stylesheet_loader.rs +++ b/ports/geckolib/stylesheet_loader.rs @@ -9,6 +9,7 @@ use style::gecko_bindings::structs::{Loader, ServoStyleSheet}; use style::gecko_bindings::sugar::ownership::HasArcFFI; use style::stylesheets::{ImportRule, StylesheetLoader as StyleStylesheetLoader}; use style_traits::ToCss; +use super::glue::GLOBAL_STYLE_DATA; pub struct StylesheetLoader(*mut Loader, *mut ServoStyleSheet); @@ -20,6 +21,8 @@ impl StylesheetLoader { impl StyleStylesheetLoader for StylesheetLoader { fn request_stylesheet(&self, import_rule: &Arc<RwLock<ImportRule>>) { + let global_style_data = &*GLOBAL_STYLE_DATA; + let guard = global_style_data.shared_lock.read(); let import = import_rule.read(); let (spec_bytes, spec_len) = import.url.as_slice_components() .expect("Import only loads valid URLs"); @@ -32,7 +35,7 @@ impl StyleStylesheetLoader for StylesheetLoader { // evaluate them on the main thread. // // Meanwhile, this works. - let media = import.stylesheet.media.read().to_css_string(); + let media = import.stylesheet.media.read_with(&guard).to_css_string(); unsafe { Gecko_LoadStyleSheet(self.0, diff --git a/tests/unit/style/media_queries.rs b/tests/unit/style/media_queries.rs index 95182676e3c..485ccd1d486 100644 --- a/tests/unit/style/media_queries.rs +++ b/tests/unit/style/media_queries.rs @@ -11,6 +11,7 @@ use style::error_reporting::ParseErrorReporter; use style::media_queries::*; use style::parser::ParserContextExtraData; use style::servo::media_queries::*; +use style::shared_lock::{SharedRwLock, SharedRwLockReadGuard}; use style::stylesheets::{Stylesheet, Origin, CssRule}; use style::values::specified; use style_traits::ToCss; @@ -29,26 +30,27 @@ fn test_media_rule<F>(css: &str, callback: F) let url = ServoUrl::parse("http://localhost").unwrap(); let css_str = css.to_owned(); let stylesheet = Stylesheet::from_str( - css, url, Origin::Author, Default::default(), + css, url, Origin::Author, Default::default(), SharedRwLock::new(), None, &CSSErrorReporterTest, ParserContextExtraData::default()); let mut rule_count = 0; - media_queries(&stylesheet.rules.read().0, &mut |mq| { + let guard = stylesheet.shared_lock.read(); + media_queries(&guard, &stylesheet.rules.read().0, &mut |mq| { rule_count += 1; callback(mq, css); }); assert!(rule_count > 0, css_str); } -fn media_queries<F>(rules: &[CssRule], f: &mut F) +fn media_queries<F>(guard: &SharedRwLockReadGuard, rules: &[CssRule], f: &mut F) where F: FnMut(&MediaList), { for rule in rules { - rule.with_nested_rules_and_mq(|rules, mq| { + rule.with_nested_rules_and_mq(guard, |rules, mq| { if let Some(mq) = mq { f(mq) } - media_queries(rules, f) + media_queries(guard, rules, f) }) } } @@ -56,7 +58,7 @@ fn media_queries<F>(rules: &[CssRule], f: &mut F) fn media_query_test(device: &Device, css: &str, expected_rule_count: usize) { let url = ServoUrl::parse("http://localhost").unwrap(); let ss = Stylesheet::from_str( - css, url, Origin::Author, Default::default(), + css, url, Origin::Author, Default::default(), SharedRwLock::new(), None, &CSSErrorReporterTest, ParserContextExtraData::default()); let mut rule_count = 0; diff --git a/tests/unit/style/rule_tree/bench.rs b/tests/unit/style/rule_tree/bench.rs index 37bef590a64..623a534159a 100644 --- a/tests/unit/style/rule_tree/bench.rs +++ b/tests/unit/style/rule_tree/bench.rs @@ -12,6 +12,7 @@ use style::media_queries::MediaList; use style::parser::ParserContextExtraData; use style::properties::{longhands, Importance, PropertyDeclaration, PropertyDeclarationBlock}; use style::rule_tree::{CascadeLevel, RuleTree, StrongRuleNode, StyleSource}; +use style::shared_lock::SharedRwLock; use style::stylesheets::{Origin, Stylesheet, CssRule}; use test::{self, Bencher}; @@ -44,6 +45,7 @@ fn parse_rules(css: &str) -> Vec<(StyleSource, CascadeLevel)> { MediaList { media_queries: vec![], }, + SharedRwLock::new(), None, &ErrorringErrorReporter, ParserContextExtraData {}); diff --git a/tests/unit/style/stylesheets.rs b/tests/unit/style/stylesheets.rs index a1d649c099c..590e93185f4 100644 --- a/tests/unit/style/stylesheets.rs +++ b/tests/unit/style/stylesheets.rs @@ -20,6 +20,7 @@ use style::properties::Importance; use style::properties::{CSSWideKeyword, DeclaredValueOwned, PropertyDeclaration, PropertyDeclarationBlock}; use style::properties::longhands; use style::properties::longhands::animation_play_state; +use style::shared_lock::SharedRwLock; use style::stylesheets::{Origin, Namespaces}; use style::stylesheets::{Stylesheet, NamespaceRule, CssRule, CssRules, StyleRule, KeyframesRule}; use style::values::specified::{LengthOrPercentageOrAuto, Percentage}; @@ -62,14 +63,15 @@ fn test_parse_stylesheet() { }"; let url = ServoUrl::parse("about::test").unwrap(); let stylesheet = Stylesheet::from_str(css, url.clone(), Origin::UserAgent, Default::default(), - None, + SharedRwLock::new(), None, &CSSErrorReporterTest, ParserContextExtraData::default()); let mut namespaces = Namespaces::default(); namespaces.default = Some(ns!(html)); let expected = Stylesheet { origin: Origin::UserAgent, - media: Default::default(), + media: Arc::new(stylesheet.shared_lock.wrap(Default::default())), + shared_lock: stylesheet.shared_lock.clone(), namespaces: RwLock::new(namespaces), base_url: url, dirty_on_viewport_size_change: AtomicBool::new(false), @@ -324,7 +326,7 @@ fn test_report_error_stylesheet() { let errors = error_reporter.errors.clone(); Stylesheet::from_str(css, url.clone(), Origin::UserAgent, Default::default(), - None, + SharedRwLock::new(), None, &error_reporter, ParserContextExtraData::default()); diff --git a/tests/unit/style/viewport.rs b/tests/unit/style/viewport.rs index 166094ebdc1..9fe53ebb40c 100644 --- a/tests/unit/style/viewport.rs +++ b/tests/unit/style/viewport.rs @@ -9,6 +9,7 @@ use servo_config::prefs::{PREFS, PrefValue}; use servo_url::ServoUrl; use style::media_queries::{Device, MediaType}; use style::parser::{ParserContext, ParserContextExtraData}; +use style::shared_lock::SharedRwLock; use style::stylesheets::{Stylesheet, Origin}; use style::values::specified::LengthOrPercentageOrAuto::{self, Auto}; use style::values::specified::NoCalcLength::{self, ViewportPercentage}; @@ -24,6 +25,7 @@ macro_rules! stylesheet { ServoUrl::parse("http://localhost").unwrap(), Origin::$origin, Default::default(), + SharedRwLock::new(), None, &$error_reporter, ParserContextExtraData::default() diff --git a/tests/unit/stylo/lib.rs b/tests/unit/stylo/lib.rs index c89687918ef..c0d04ffcbec 100644 --- a/tests/unit/stylo/lib.rs +++ b/tests/unit/stylo/lib.rs @@ -21,8 +21,5 @@ extern crate style_traits; mod sanity_checks; mod size_of; -#[path = "../../../ports/geckolib/stylesheet_loader.rs"] -mod stylesheet_loader; - mod servo_function_signatures; diff --git a/tests/unit/stylo/servo_function_signatures.rs b/tests/unit/stylo/servo_function_signatures.rs index 6bdeb6d5af7..30d6d02439b 100644 --- a/tests/unit/stylo/servo_function_signatures.rs +++ b/tests/unit/stylo/servo_function_signatures.rs @@ -10,6 +10,9 @@ use style::gecko_properties::*; include!(concat!(env!("OUT_DIR"), "/check_bindings.rs")); +#[path = "../../../ports/geckolib/stylesheet_loader.rs"] +mod stylesheet_loader; + #[allow(non_snake_case, unused_unsafe, private_no_mangle_fns)] mod glue { // this module pretends to be glue.rs, with the safe functions swapped for unsafe ones. This is |