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

components/layout_thread/lib.rs

@@ -731,7 +731,8 @@ impl LayoutThread {
         // GWTODO: Need to handle unloading web fonts.
 
         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,
                                 &rw_data.stylist.device,
                                 &self.font_cache_thread,
@@ -1057,9 +1058,11 @@ impl LayoutThread {
         }
 
         // 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,
-                                                                                 Some(&*UA_STYLESHEETS),
-                                                                                 data.stylesheets_changed);
+        let needs_dirtying = Arc::get_mut(&mut rw_data.stylist).unwrap().update(
+            &data.document_stylesheets,
+            &style_guard,
+            Some(&*UA_STYLESHEETS),
+            data.stylesheets_changed);
         let needs_reflow = viewport_size_changed && !needs_dirtying;
         if needs_dirtying {
             if let Some(mut d) = element.mutate_data() {

components/style/gecko/data.rs

@@ -13,6 +13,7 @@ use gecko_bindings::sugar::ownership::{HasBoxFFI, HasFFI, HasSimpleFFI};
 use media_queries::Device;
 use parking_lot::RwLock;
 use properties::ComputedValues;
+use shared_lock::SharedRwLockReadGuard;
 use std::collections::HashMap;
 use std::sync::Arc;
 use std::sync::mpsc::{Receiver, Sender, channel};
@@ -83,20 +84,20 @@ impl PerDocumentStyleDataImpl {
     /// Reset the device state because it may have changed.
     ///
     /// 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();
             Arc::get_mut(&mut stylist.device).unwrap().reset();
         }
         self.stylesheets_changed = true;
-        self.flush_stylesheets();
+        self.flush_stylesheets(guard);
     }
 
     /// 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 {
             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;
         }
     }

components/style/stylesheets.rs

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

components/style/stylist.rs

@@ -28,7 +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 shared_lock::SharedRwLockReadGuard;
 use sink::Push;
 use smallvec::VecLike;
 use std::borrow::Borrow;
@@ -159,6 +159,7 @@ impl Stylist {
     /// device is dirty, which means we need to re-evaluate media queries.
     pub fn update(&mut self,
                   doc_stylesheets: &[Arc<Stylesheet>],
+                  doc_guard: &SharedRwLockReadGuard,
                   ua_stylesheets: Option<&UserAgentStylesheets>,
                   stylesheets_changed: bool) -> bool {
         if !(self.is_device_dirty || stylesheets_changed) {
@@ -193,17 +194,18 @@ impl Stylist {
         self.non_common_style_affecting_attributes_selectors.clear();
 
         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 {
-                self.add_stylesheet(&stylesheet);
+                self.add_stylesheet(&stylesheet, &ua_guard);
             }
 
             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() {
-            self.add_stylesheet(stylesheet);
+            self.add_stylesheet(stylesheet, doc_guard);
         }
 
         debug!("Stylist stats:");
@@ -227,8 +229,8 @@ impl Stylist {
         true
     }
 
-    fn add_stylesheet(&mut self, stylesheet: &Stylesheet) {
-        if stylesheet.disabled() || !stylesheet.is_effective_for_device(&self.device) {
+    fn add_stylesheet(&mut self, stylesheet: &Stylesheet, guard: &SharedRwLockReadGuard) {
+        if stylesheet.disabled() || !stylesheet.is_effective_for_device(&self.device, guard) {
             return;
         }
 
@@ -271,7 +273,7 @@ impl Stylist {
                 }
                 CssRule::Import(ref import) => {
                     let import = import.read();
-                    self.add_stylesheet(&import.stylesheet)
+                    self.add_stylesheet(&import.stylesheet, guard)
                 }
                 CssRule::Keyframes(ref keyframes_rule) => {
                     let keyframes_rule = keyframes_rule.read();

ports/geckolib/glue.rs

@@ -204,7 +204,7 @@ fn traverse_subtree(element: GeckoElement, raw_data: RawServoStyleSetBorrowed,
     debug!("{:?}", ShowSubtreeData(element.as_node()));
 
     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() {
         TraversalDriver::Sequential
@@ -333,14 +333,15 @@ pub extern "C" fn Servo_Element_ClearData(element: RawGeckoElementBorrowed) {
 
 #[no_mangle]
 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 extra_data = ParserContextExtraData::default();
     let origin = match mode {
         SheetParsingMode::eAuthorSheetFeatures => Origin::Author,
         SheetParsingMode::eUserSheetFeatures => Origin::User,
         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(
         "", url, origin, Default::default(), shared_lock, None,
         &StdoutErrorReporter, extra_data)
@@ -357,6 +358,7 @@ pub extern "C" fn Servo_StyleSheet_FromUTF8Bytes(loader: *mut Loader,
                                                  referrer: *mut ThreadSafeURIHolder,
                                                  principal: *mut ThreadSafePrincipalHolder)
                                                  -> RawServoStyleSheetStrong {
+    let global_style_data = &*GLOBAL_STYLE_DATA;
     let input = unsafe { data.as_ref().unwrap().as_str_unchecked() };
 
     let origin = match mode {
@@ -384,7 +386,7 @@ pub extern "C" fn Servo_StyleSheet_FromUTF8Bytes(loader: *mut Loader,
         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(
         input, url, origin, Default::default(), shared_lock, loader,
         &StdoutErrorReporter, extra_data)
@@ -430,27 +432,31 @@ pub extern "C" fn Servo_StyleSheet_ClearAndUpdate(stylesheet: RawServoStyleSheet
 pub extern "C" fn Servo_StyleSet_AppendStyleSheet(raw_data: RawServoStyleSetBorrowed,
                                                   raw_sheet: RawServoStyleSheetBorrowed,
                                                   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 sheet = HasArcFFI::as_arc(&raw_sheet);
     data.stylesheets.retain(|x| !arc_ptr_eq(x, sheet));
     data.stylesheets.push(sheet.clone());
     data.stylesheets_changed = true;
     if flush {
-        data.flush_stylesheets();
+        data.flush_stylesheets(&guard);
     }
 }
 
 #[no_mangle]
 pub extern "C" fn Servo_StyleSet_PrependStyleSheet(raw_data: RawServoStyleSetBorrowed,
                                                    raw_sheet: RawServoStyleSheetBorrowed,
                                                    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 sheet = HasArcFFI::as_arc(&raw_sheet);
     data.stylesheets.retain(|x| !arc_ptr_eq(x, sheet));
     data.stylesheets.insert(0, sheet.clone());
     data.stylesheets_changed = true;
     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_reference: RawServoStyleSheetBorrowed,
                                                         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 sheet = HasArcFFI::as_arc(&raw_sheet);
     let reference = HasArcFFI::as_arc(&raw_reference);
@@ -467,27 +475,31 @@ pub extern "C" fn Servo_StyleSet_InsertStyleSheetBefore(raw_data: RawServoStyleS
     data.stylesheets.insert(index, sheet.clone());
     data.stylesheets_changed = true;
     if flush {
-        data.flush_stylesheets();
+        data.flush_stylesheets(&guard);
     }
 }
 
 #[no_mangle]
 pub extern "C" fn Servo_StyleSet_RemoveStyleSheet(raw_data: RawServoStyleSetBorrowed,
                                                   raw_sheet: RawServoStyleSheetBorrowed,
                                                   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 sheet = HasArcFFI::as_arc(&raw_sheet);
     data.stylesheets.retain(|x| !arc_ptr_eq(x, sheet));
     data.stylesheets_changed = true;
     if flush {
-        data.flush_stylesheets();
+        data.flush_stylesheets(&guard);
     }
 }
 
 #[no_mangle]
 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();
-    data.flush_stylesheets();
+    data.flush_stylesheets(&guard);
 }
 
 #[no_mangle]
@@ -732,8 +744,10 @@ pub extern "C" fn Servo_StyleSet_Init(pres_context: RawGeckoPresContextOwned)
 
 #[no_mangle]
 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();
-    data.reset_device();
+    data.reset_device(&guard);
 }
 
 #[no_mangle]