Move shared lock acquires up the call stack. (Or is it down?)

This commit is contained in:
Simon Sapin 2017-03-16 23:08:10 +01:00
parent c5a7294e05
commit d18b1280f2
5 changed files with 46 additions and 27 deletions

View file

@ -731,7 +731,8 @@ impl LayoutThread {
// GWTODO: Need to handle unloading web fonts. // GWTODO: Need to handle unloading web fonts.
let rw_data = possibly_locked_rw_data.lock(); let rw_data = possibly_locked_rw_data.lock();
if stylesheet.is_effective_for_device(&rw_data.stylist.device) { let guard = stylesheet.shared_lock.read();
if stylesheet.is_effective_for_device(&rw_data.stylist.device, &guard) {
add_font_face_rules(&*stylesheet, add_font_face_rules(&*stylesheet,
&rw_data.stylist.device, &rw_data.stylist.device,
&self.font_cache_thread, &self.font_cache_thread,
@ -1057,7 +1058,9 @@ impl LayoutThread {
} }
// If the entire flow tree is invalid, then it will be reflowed anyhow. // If the entire flow tree is invalid, then it will be reflowed anyhow.
let needs_dirtying = Arc::get_mut(&mut rw_data.stylist).unwrap().update(&data.document_stylesheets, let needs_dirtying = Arc::get_mut(&mut rw_data.stylist).unwrap().update(
&data.document_stylesheets,
&style_guard,
Some(&*UA_STYLESHEETS), Some(&*UA_STYLESHEETS),
data.stylesheets_changed); data.stylesheets_changed);
let needs_reflow = viewport_size_changed && !needs_dirtying; let needs_reflow = viewport_size_changed && !needs_dirtying;

View file

@ -13,6 +13,7 @@ use gecko_bindings::sugar::ownership::{HasBoxFFI, HasFFI, HasSimpleFFI};
use media_queries::Device; use media_queries::Device;
use parking_lot::RwLock; use parking_lot::RwLock;
use properties::ComputedValues; use properties::ComputedValues;
use shared_lock::SharedRwLockReadGuard;
use std::collections::HashMap; use std::collections::HashMap;
use std::sync::Arc; use std::sync::Arc;
use std::sync::mpsc::{Receiver, Sender, channel}; use std::sync::mpsc::{Receiver, Sender, channel};
@ -83,20 +84,20 @@ impl PerDocumentStyleDataImpl {
/// Reset the device state because it may have changed. /// Reset the device state because it may have changed.
/// ///
/// Implies also a stylesheet flush. /// Implies also a stylesheet flush.
pub fn reset_device(&mut self) { pub fn reset_device(&mut self, guard: &SharedRwLockReadGuard) {
{ {
let mut stylist = Arc::get_mut(&mut self.stylist).unwrap(); let mut stylist = Arc::get_mut(&mut self.stylist).unwrap();
Arc::get_mut(&mut stylist.device).unwrap().reset(); Arc::get_mut(&mut stylist.device).unwrap().reset();
} }
self.stylesheets_changed = true; self.stylesheets_changed = true;
self.flush_stylesheets(); self.flush_stylesheets(guard);
} }
/// Recreate the style data if the stylesheets have changed. /// Recreate the style data if the stylesheets have changed.
pub fn flush_stylesheets(&mut self) { pub fn flush_stylesheets(&mut self, guard: &SharedRwLockReadGuard) {
if self.stylesheets_changed { if self.stylesheets_changed {
let mut stylist = Arc::get_mut(&mut self.stylist).unwrap(); let mut stylist = Arc::get_mut(&mut self.stylist).unwrap();
stylist.update(&self.stylesheets, None, true); stylist.update(&self.stylesheets, guard, None, true);
self.stylesheets_changed = false; self.stylesheets_changed = false;
} }
} }

View file

@ -647,9 +647,8 @@ impl Stylesheet {
/// on the associated MediaList. /// on the associated MediaList.
/// ///
/// Always true if no associated MediaList exists. /// Always true if no associated MediaList exists.
pub fn is_effective_for_device(&self, device: &Device) -> bool { pub fn is_effective_for_device(&self, device: &Device, guard: &SharedRwLockReadGuard) -> bool {
let guard = self.shared_lock.read(); // FIXME: have the caller pass this? self.media.read_with(guard).evaluate(device)
self.media.read_with(&guard).evaluate(device)
} }
/// Return an iterator over the effective rules within the style-sheet, as /// Return an iterator over the effective rules within the style-sheet, as

View file

@ -28,7 +28,7 @@ use selectors::matching::{AFFECTED_BY_STYLE_ATTRIBUTE, AFFECTED_BY_PRESENTATIONA
use selectors::matching::{ElementSelectorFlags, StyleRelations, matches_complex_selector}; use selectors::matching::{ElementSelectorFlags, StyleRelations, matches_complex_selector};
use selectors::parser::{Selector, SimpleSelector, LocalName as LocalNameSelector, ComplexSelector}; use selectors::parser::{Selector, SimpleSelector, LocalName as LocalNameSelector, ComplexSelector};
use selectors::parser::SelectorMethods; use selectors::parser::SelectorMethods;
#[cfg(feature = "servo")] use shared_lock::SharedRwLockReadGuard; use shared_lock::SharedRwLockReadGuard;
use sink::Push; use sink::Push;
use smallvec::VecLike; use smallvec::VecLike;
use std::borrow::Borrow; use std::borrow::Borrow;
@ -159,6 +159,7 @@ impl Stylist {
/// device is dirty, which means we need to re-evaluate media queries. /// device is dirty, which means we need to re-evaluate media queries.
pub fn update(&mut self, pub fn update(&mut self,
doc_stylesheets: &[Arc<Stylesheet>], doc_stylesheets: &[Arc<Stylesheet>],
doc_guard: &SharedRwLockReadGuard,
ua_stylesheets: Option<&UserAgentStylesheets>, ua_stylesheets: Option<&UserAgentStylesheets>,
stylesheets_changed: bool) -> bool { stylesheets_changed: bool) -> bool {
if !(self.is_device_dirty || stylesheets_changed) { if !(self.is_device_dirty || stylesheets_changed) {
@ -193,17 +194,18 @@ impl Stylist {
self.non_common_style_affecting_attributes_selectors.clear(); self.non_common_style_affecting_attributes_selectors.clear();
if let Some(ua_stylesheets) = ua_stylesheets { if let Some(ua_stylesheets) = ua_stylesheets {
let ua_guard = ua_stylesheets.shared_lock.read();
for stylesheet in &ua_stylesheets.user_or_user_agent_stylesheets { for stylesheet in &ua_stylesheets.user_or_user_agent_stylesheets {
self.add_stylesheet(&stylesheet); self.add_stylesheet(&stylesheet, &ua_guard);
} }
if self.quirks_mode { if self.quirks_mode {
self.add_stylesheet(&ua_stylesheets.quirks_mode_stylesheet); self.add_stylesheet(&ua_stylesheets.quirks_mode_stylesheet, &ua_guard);
} }
} }
for ref stylesheet in doc_stylesheets.iter() { for ref stylesheet in doc_stylesheets.iter() {
self.add_stylesheet(stylesheet); self.add_stylesheet(stylesheet, doc_guard);
} }
debug!("Stylist stats:"); debug!("Stylist stats:");
@ -227,8 +229,8 @@ impl Stylist {
true true
} }
fn add_stylesheet(&mut self, stylesheet: &Stylesheet) { fn add_stylesheet(&mut self, stylesheet: &Stylesheet, guard: &SharedRwLockReadGuard) {
if stylesheet.disabled() || !stylesheet.is_effective_for_device(&self.device) { if stylesheet.disabled() || !stylesheet.is_effective_for_device(&self.device, guard) {
return; return;
} }
@ -271,7 +273,7 @@ impl Stylist {
} }
CssRule::Import(ref import) => { CssRule::Import(ref import) => {
let import = import.read(); let import = import.read();
self.add_stylesheet(&import.stylesheet) self.add_stylesheet(&import.stylesheet, guard)
} }
CssRule::Keyframes(ref keyframes_rule) => { CssRule::Keyframes(ref keyframes_rule) => {
let keyframes_rule = keyframes_rule.read(); let keyframes_rule = keyframes_rule.read();

View file

@ -204,7 +204,7 @@ fn traverse_subtree(element: GeckoElement, raw_data: RawServoStyleSetBorrowed,
debug!("{:?}", ShowSubtreeData(element.as_node())); debug!("{:?}", ShowSubtreeData(element.as_node()));
let shared_style_context = create_shared_context(&per_doc_data); let shared_style_context = create_shared_context(&per_doc_data);
let ref global_style_data = *GLOBAL_STYLE_DATA; let global_style_data = &*GLOBAL_STYLE_DATA;
let traversal_driver = if global_style_data.style_thread_pool.is_none() { let traversal_driver = if global_style_data.style_thread_pool.is_none() {
TraversalDriver::Sequential TraversalDriver::Sequential
@ -333,6 +333,7 @@ pub extern "C" fn Servo_Element_ClearData(element: RawGeckoElementBorrowed) {
#[no_mangle] #[no_mangle]
pub extern "C" fn Servo_StyleSheet_Empty(mode: SheetParsingMode) -> RawServoStyleSheetStrong { pub extern "C" fn Servo_StyleSheet_Empty(mode: SheetParsingMode) -> RawServoStyleSheetStrong {
let global_style_data = &*GLOBAL_STYLE_DATA;
let url = ServoUrl::parse("about:blank").unwrap(); let url = ServoUrl::parse("about:blank").unwrap();
let extra_data = ParserContextExtraData::default(); let extra_data = ParserContextExtraData::default();
let origin = match mode { let origin = match mode {
@ -340,7 +341,7 @@ pub extern "C" fn Servo_StyleSheet_Empty(mode: SheetParsingMode) -> RawServoStyl
SheetParsingMode::eUserSheetFeatures => Origin::User, SheetParsingMode::eUserSheetFeatures => Origin::User,
SheetParsingMode::eAgentSheetFeatures => Origin::UserAgent, SheetParsingMode::eAgentSheetFeatures => Origin::UserAgent,
}; };
let shared_lock = GLOBAL_STYLE_DATA.shared_lock.clone(); let shared_lock = global_style_data.shared_lock.clone();
Arc::new(Stylesheet::from_str( Arc::new(Stylesheet::from_str(
"", url, origin, Default::default(), shared_lock, None, "", url, origin, Default::default(), shared_lock, None,
&StdoutErrorReporter, extra_data) &StdoutErrorReporter, extra_data)
@ -357,6 +358,7 @@ pub extern "C" fn Servo_StyleSheet_FromUTF8Bytes(loader: *mut Loader,
referrer: *mut ThreadSafeURIHolder, referrer: *mut ThreadSafeURIHolder,
principal: *mut ThreadSafePrincipalHolder) principal: *mut ThreadSafePrincipalHolder)
-> RawServoStyleSheetStrong { -> RawServoStyleSheetStrong {
let global_style_data = &*GLOBAL_STYLE_DATA;
let input = unsafe { data.as_ref().unwrap().as_str_unchecked() }; let input = unsafe { data.as_ref().unwrap().as_str_unchecked() };
let origin = match mode { let origin = match mode {
@ -384,7 +386,7 @@ pub extern "C" fn Servo_StyleSheet_FromUTF8Bytes(loader: *mut Loader,
Some(ref s) => Some(s), Some(ref s) => Some(s),
}; };
let shared_lock = GLOBAL_STYLE_DATA.shared_lock.clone(); let shared_lock = global_style_data.shared_lock.clone();
Arc::new(Stylesheet::from_str( Arc::new(Stylesheet::from_str(
input, url, origin, Default::default(), shared_lock, loader, input, url, origin, Default::default(), shared_lock, loader,
&StdoutErrorReporter, extra_data) &StdoutErrorReporter, extra_data)
@ -430,13 +432,15 @@ pub extern "C" fn Servo_StyleSheet_ClearAndUpdate(stylesheet: RawServoStyleSheet
pub extern "C" fn Servo_StyleSet_AppendStyleSheet(raw_data: RawServoStyleSetBorrowed, pub extern "C" fn Servo_StyleSet_AppendStyleSheet(raw_data: RawServoStyleSetBorrowed,
raw_sheet: RawServoStyleSheetBorrowed, raw_sheet: RawServoStyleSheetBorrowed,
flush: bool) { flush: bool) {
let global_style_data = &*GLOBAL_STYLE_DATA;
let guard = global_style_data.shared_lock.read();
let mut data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut(); let mut data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut();
let sheet = HasArcFFI::as_arc(&raw_sheet); let sheet = HasArcFFI::as_arc(&raw_sheet);
data.stylesheets.retain(|x| !arc_ptr_eq(x, sheet)); data.stylesheets.retain(|x| !arc_ptr_eq(x, sheet));
data.stylesheets.push(sheet.clone()); data.stylesheets.push(sheet.clone());
data.stylesheets_changed = true; data.stylesheets_changed = true;
if flush { if flush {
data.flush_stylesheets(); data.flush_stylesheets(&guard);
} }
} }
@ -444,13 +448,15 @@ pub extern "C" fn Servo_StyleSet_AppendStyleSheet(raw_data: RawServoStyleSetBorr
pub extern "C" fn Servo_StyleSet_PrependStyleSheet(raw_data: RawServoStyleSetBorrowed, pub extern "C" fn Servo_StyleSet_PrependStyleSheet(raw_data: RawServoStyleSetBorrowed,
raw_sheet: RawServoStyleSheetBorrowed, raw_sheet: RawServoStyleSheetBorrowed,
flush: bool) { flush: bool) {
let global_style_data = &*GLOBAL_STYLE_DATA;
let guard = global_style_data.shared_lock.read();
let mut data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut(); let mut data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut();
let sheet = HasArcFFI::as_arc(&raw_sheet); let sheet = HasArcFFI::as_arc(&raw_sheet);
data.stylesheets.retain(|x| !arc_ptr_eq(x, sheet)); data.stylesheets.retain(|x| !arc_ptr_eq(x, sheet));
data.stylesheets.insert(0, sheet.clone()); data.stylesheets.insert(0, sheet.clone());
data.stylesheets_changed = true; data.stylesheets_changed = true;
if flush { if flush {
data.flush_stylesheets(); data.flush_stylesheets(&guard);
} }
} }
@ -459,6 +465,8 @@ pub extern "C" fn Servo_StyleSet_InsertStyleSheetBefore(raw_data: RawServoStyleS
raw_sheet: RawServoStyleSheetBorrowed, raw_sheet: RawServoStyleSheetBorrowed,
raw_reference: RawServoStyleSheetBorrowed, raw_reference: RawServoStyleSheetBorrowed,
flush: bool) { flush: bool) {
let global_style_data = &*GLOBAL_STYLE_DATA;
let guard = global_style_data.shared_lock.read();
let mut data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut(); let mut data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut();
let sheet = HasArcFFI::as_arc(&raw_sheet); let sheet = HasArcFFI::as_arc(&raw_sheet);
let reference = HasArcFFI::as_arc(&raw_reference); let reference = HasArcFFI::as_arc(&raw_reference);
@ -467,7 +475,7 @@ pub extern "C" fn Servo_StyleSet_InsertStyleSheetBefore(raw_data: RawServoStyleS
data.stylesheets.insert(index, sheet.clone()); data.stylesheets.insert(index, sheet.clone());
data.stylesheets_changed = true; data.stylesheets_changed = true;
if flush { if flush {
data.flush_stylesheets(); data.flush_stylesheets(&guard);
} }
} }
@ -475,19 +483,23 @@ pub extern "C" fn Servo_StyleSet_InsertStyleSheetBefore(raw_data: RawServoStyleS
pub extern "C" fn Servo_StyleSet_RemoveStyleSheet(raw_data: RawServoStyleSetBorrowed, pub extern "C" fn Servo_StyleSet_RemoveStyleSheet(raw_data: RawServoStyleSetBorrowed,
raw_sheet: RawServoStyleSheetBorrowed, raw_sheet: RawServoStyleSheetBorrowed,
flush: bool) { flush: bool) {
let global_style_data = &*GLOBAL_STYLE_DATA;
let guard = global_style_data.shared_lock.read();
let mut data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut(); let mut data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut();
let sheet = HasArcFFI::as_arc(&raw_sheet); let sheet = HasArcFFI::as_arc(&raw_sheet);
data.stylesheets.retain(|x| !arc_ptr_eq(x, sheet)); data.stylesheets.retain(|x| !arc_ptr_eq(x, sheet));
data.stylesheets_changed = true; data.stylesheets_changed = true;
if flush { if flush {
data.flush_stylesheets(); data.flush_stylesheets(&guard);
} }
} }
#[no_mangle] #[no_mangle]
pub extern "C" fn Servo_StyleSet_FlushStyleSheets(raw_data: RawServoStyleSetBorrowed) { pub extern "C" fn Servo_StyleSet_FlushStyleSheets(raw_data: RawServoStyleSetBorrowed) {
let global_style_data = &*GLOBAL_STYLE_DATA;
let guard = global_style_data.shared_lock.read();
let mut data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut(); let mut data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut();
data.flush_stylesheets(); data.flush_stylesheets(&guard);
} }
#[no_mangle] #[no_mangle]
@ -732,8 +744,10 @@ pub extern "C" fn Servo_StyleSet_Init(pres_context: RawGeckoPresContextOwned)
#[no_mangle] #[no_mangle]
pub extern "C" fn Servo_StyleSet_RebuildData(raw_data: RawServoStyleSetBorrowed) { pub extern "C" fn Servo_StyleSet_RebuildData(raw_data: RawServoStyleSetBorrowed) {
let global_style_data = &*GLOBAL_STYLE_DATA;
let guard = global_style_data.shared_lock.read();
let mut data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut(); let mut data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut();
data.reset_device(); data.reset_device(&guard);
} }
#[no_mangle] #[no_mangle]