diff --git a/Cargo.lock b/Cargo.lock index d8bf220c6480..076924c397c7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2757,7 +2757,6 @@ dependencies = [ "num-integer 0.1.33 (registry+https://github.com/rust-lang/crates.io-index)", "num-traits 0.1.37 (registry+https://github.com/rust-lang/crates.io-index)", "ordered-float 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", - "owning_ref 0.2.4 (registry+https://github.com/rust-lang/crates.io-index)", "parking_lot 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)", "pdqsort 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", "rayon 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -2783,7 +2782,6 @@ dependencies = [ "cssparser 0.12.1 (registry+https://github.com/rust-lang/crates.io-index)", "euclid 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)", "html5ever-atoms 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)", - "owning_ref 0.2.4 (registry+https://github.com/rust-lang/crates.io-index)", "parking_lot 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)", "rayon 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)", "rustc-serialize 0.3.23 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/components/layout/block.rs b/components/layout/block.rs index 4bfef78cb3df..31f44f0ec0eb 100644 --- a/components/layout/block.rs +++ b/components/layout/block.rs @@ -446,7 +446,7 @@ fn translate_including_floats(cur_b: &mut Au, delta: Au, floats: &mut Floats) { /// /// Note that flows with position 'fixed' just form a flat list as they all /// have the Root flow as their CB. -pub struct AbsoluteAssignBSizesTraversal<'a>(pub &'a SharedStyleContext); +pub struct AbsoluteAssignBSizesTraversal<'a>(pub &'a SharedStyleContext<'a>); impl<'a> PreorderFlowTraversal for AbsoluteAssignBSizesTraversal<'a> { #[inline] diff --git a/components/layout/construct.rs b/components/layout/construct.rs index a6571d0e0937..00cd46a079a7 100644 --- a/components/layout/construct.rs +++ b/components/layout/construct.rs @@ -311,7 +311,7 @@ impl InlineFragmentsAccumulator { /// An object that knows how to create flows. pub struct FlowConstructor<'a, N: ThreadSafeLayoutNode> { /// The layout context. - pub layout_context: &'a LayoutContext, + pub layout_context: &'a LayoutContext<'a>, /// Satisfy the compiler about the unused parameters, which we use to improve the ergonomics of /// the ensuing impl {} by removing the need to parameterize all the methods individually. phantom2: PhantomData, @@ -320,7 +320,7 @@ pub struct FlowConstructor<'a, N: ThreadSafeLayoutNode> { impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode> FlowConstructor<'a, ConcreteThreadSafeLayoutNode> { /// Creates a new flow constructor. - pub fn new(layout_context: &'a LayoutContext) -> Self { + pub fn new(layout_context: &'a LayoutContext<'a>) -> Self { FlowConstructor { layout_context: layout_context, phantom2: PhantomData, @@ -660,10 +660,9 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode> let mut style = node.style(self.style_context()); if node_is_input_or_text_area { - style = self.style_context() - .stylist - .style_for_anonymous_box(&PseudoElement::ServoInputText, - &style) + let context = self.style_context(); + style = context.stylist.style_for_anonymous_box( + &context.guards, &PseudoElement::ServoInputText, &style) } self.create_fragments_for_node_text_content(&mut initial_fragments, node, &style) @@ -1096,11 +1095,14 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode> -> ConstructionResult { let mut legalizer = Legalizer::new(); - let table_style = node.style(self.style_context()); - let wrapper_style = self.style_context() - .stylist - .style_for_anonymous_box(&PseudoElement::ServoTableWrapper, - &table_style); + let table_style; + let wrapper_style; + { + let context = self.style_context(); + table_style = node.style(context); + wrapper_style = context.stylist.style_for_anonymous_box( + &context.guards, &PseudoElement::ServoTableWrapper, &table_style); + } let wrapper_fragment = Fragment::from_opaque_node_and_style(node.opaque(), PseudoElementType::Normal, @@ -2080,8 +2082,7 @@ impl Legalizer { let reference_block = reference.as_block(); let mut new_style = reference_block.fragment.style.clone(); for pseudo in pseudos { - new_style = context.stylist.style_for_anonymous_box(pseudo, - &new_style) + new_style = context.stylist.style_for_anonymous_box(&context.guards, pseudo, &new_style) } let fragment = reference_block.fragment .create_similar_anonymous_fragment(new_style, diff --git a/components/layout/context.rs b/components/layout/context.rs index 1dff9f6ea5eb..e2ffeb1cbd93 100644 --- a/components/layout/context.rs +++ b/components/layout/context.rs @@ -75,9 +75,9 @@ pub fn heap_size_of_persistent_local_context() -> usize { } /// Layout information shared among all workers. This must be thread-safe. -pub struct LayoutContext { +pub struct LayoutContext<'a> { /// Bits shared by the layout and style system. - pub style_context: SharedStyleContext, + pub style_context: SharedStyleContext<'a>, /// The shared image cache thread. pub image_cache_thread: Mutex, @@ -95,7 +95,7 @@ pub struct LayoutContext { pub pending_images: Option>> } -impl Drop for LayoutContext { +impl<'a> Drop for LayoutContext<'a> { fn drop(&mut self) { if !thread::panicking() { if let Some(ref pending_images) = self.pending_images { @@ -105,7 +105,7 @@ impl Drop for LayoutContext { } } -impl LayoutContext { +impl<'a> LayoutContext<'a> { #[inline(always)] pub fn shared_context(&self) -> &SharedStyleContext { &self.style_context diff --git a/components/layout/display_list_builder.rs b/components/layout/display_list_builder.rs index ab3636fdbc50..2ee9985cf0eb 100644 --- a/components/layout/display_list_builder.rs +++ b/components/layout/display_list_builder.rs @@ -121,7 +121,7 @@ fn get_cyclic(arr: &[T], index: usize) -> &T { } pub struct DisplayListBuildState<'a> { - pub layout_context: &'a LayoutContext, + pub layout_context: &'a LayoutContext<'a>, pub root_stacking_context: StackingContext, pub items: HashMap>, pub stacking_context_children: HashMap>, diff --git a/components/layout/generated_content.rs b/components/layout/generated_content.rs index ef2e270c0641..887562e89fbd 100644 --- a/components/layout/generated_content.rs +++ b/components/layout/generated_content.rs @@ -97,7 +97,7 @@ static KATAKANA_IROHA: [char; 47] = [ /// The generated content resolution traversal. pub struct ResolveGeneratedContent<'a> { /// The layout context. - layout_context: &'a LayoutContext, + layout_context: &'a LayoutContext<'a>, /// The counter representing an ordered list item. list_item: Counter, /// Named CSS counters. diff --git a/components/layout/traversal.rs b/components/layout/traversal.rs index 0088ef208f32..3c0c1763d0c3 100644 --- a/components/layout/traversal.rs +++ b/components/layout/traversal.rs @@ -22,20 +22,20 @@ use style::traversal::PerLevelTraversalData; use wrapper::{GetRawData, LayoutNodeHelpers, LayoutNodeLayoutData}; use wrapper::ThreadSafeLayoutNodeHelpers; -pub struct RecalcStyleAndConstructFlows { - context: LayoutContext, +pub struct RecalcStyleAndConstructFlows<'a> { + context: LayoutContext<'a>, driver: TraversalDriver, } -impl RecalcStyleAndConstructFlows { - pub fn layout_context(&self) -> &LayoutContext { +impl<'a> RecalcStyleAndConstructFlows<'a> { + pub fn layout_context(&self) -> &LayoutContext<'a> { &self.context } } -impl RecalcStyleAndConstructFlows { +impl<'a> RecalcStyleAndConstructFlows<'a> { /// Creates a traversal context, taking ownership of the shared layout context. - pub fn new(context: LayoutContext, driver: TraversalDriver) -> Self { + pub fn new(context: LayoutContext<'a>, driver: TraversalDriver) -> Self { RecalcStyleAndConstructFlows { context: context, driver: driver, @@ -44,13 +44,13 @@ impl RecalcStyleAndConstructFlows { /// Consumes this traversal context, returning ownership of the shared layout /// context to the caller. - pub fn destroy(self) -> LayoutContext { + pub fn destroy(self) -> LayoutContext<'a> { self.context } } #[allow(unsafe_code)] -impl DomTraversal for RecalcStyleAndConstructFlows +impl<'a, E> DomTraversal for RecalcStyleAndConstructFlows<'a> where E: TElement, E::ConcreteNode: LayoutNode, { @@ -152,7 +152,7 @@ fn construct_flows_at(context: &LayoutContext, /// The bubble-inline-sizes traversal, the first part of layout computation. This computes /// preferred and intrinsic inline-sizes and bubbles them up the tree. pub struct BubbleISizes<'a> { - pub layout_context: &'a LayoutContext, + pub layout_context: &'a LayoutContext<'a>, } impl<'a> PostorderFlowTraversal for BubbleISizes<'a> { @@ -171,7 +171,7 @@ impl<'a> PostorderFlowTraversal for BubbleISizes<'a> { /// The assign-inline-sizes traversal. In Gecko this corresponds to `Reflow`. #[derive(Copy, Clone)] pub struct AssignISizes<'a> { - pub layout_context: &'a LayoutContext, + pub layout_context: &'a LayoutContext<'a>, } impl<'a> PreorderFlowTraversal for AssignISizes<'a> { @@ -191,7 +191,7 @@ impl<'a> PreorderFlowTraversal for AssignISizes<'a> { /// positions. In Gecko this corresponds to `Reflow`. #[derive(Copy, Clone)] pub struct AssignBSizes<'a> { - pub layout_context: &'a LayoutContext, + pub layout_context: &'a LayoutContext<'a>, } impl<'a> PostorderFlowTraversal for AssignBSizes<'a> { @@ -220,7 +220,7 @@ impl<'a> PostorderFlowTraversal for AssignBSizes<'a> { #[derive(Copy, Clone)] pub struct ComputeAbsolutePositions<'a> { - pub layout_context: &'a LayoutContext, + pub layout_context: &'a LayoutContext<'a>, } impl<'a> PreorderFlowTraversal for ComputeAbsolutePositions<'a> { diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs index 566d6c7ca33a..327089114843 100644 --- a/components/layout_thread/lib.rs +++ b/components/layout_thread/lib.rs @@ -114,8 +114,9 @@ use style::error_reporting::StdoutErrorReporter; use style::logical_geometry::LogicalPoint; use style::media_queries::{Device, MediaType}; use style::parser::ParserContextExtraData; +use style::servo::AUTHOR_SHARED_LOCK; use style::servo::restyle_damage::{REFLOW, REFLOW_OUT_OF_FLOW, REPAINT, REPOSITION, STORE_OVERFLOW}; -use style::shared_lock::{SharedRwLock, SharedRwLockReadGuard}; +use style::shared_lock::{SharedRwLock, SharedRwLockReadGuard, ReadGuards}; use style::stylesheets::{Origin, Stylesheet, UserAgentStylesheets}; use style::stylist::Stylist; use style::thread_state; @@ -215,7 +216,7 @@ pub struct LayoutThread { WebRenderImageInfo, BuildHasherDefault>>>, - // Webrender interface. + /// Webrender interface. webrender_api: webrender_traits::RenderApi, /// The timer object to control the timing of the animations. This should @@ -498,16 +499,18 @@ impl LayoutThread { } // Create a layout context for use in building display lists, hit testing, &c. - fn build_layout_context(&self, - rw_data: &LayoutThreadData, - request_images: bool) - -> LayoutContext { + fn build_layout_context<'a>(&self, + guards: ReadGuards<'a>, + rw_data: &LayoutThreadData, + request_images: bool) + -> LayoutContext<'a> { let thread_local_style_context_creation_data = ThreadLocalStyleContextCreationInfo::new(self.new_animations_sender.clone()); LayoutContext { style_context: SharedStyleContext { stylist: rw_data.stylist.clone(), + guards: guards, running_animations: self.running_animations.clone(), expired_animations: self.expired_animations.clone(), error_reporter: Box::new(self.error_reporter.clone()), @@ -941,7 +944,6 @@ 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. @@ -1017,9 +1019,11 @@ impl LayoutThread { Au::from_f32_px(initial_viewport.height)); // Calculate the actual viewport as per DEVICE-ADAPT § 6 + + let author_guard = document.style_shared_lock().read(); let device = Device::new(MediaType::Screen, initial_viewport); Arc::get_mut(&mut rw_data.stylist).unwrap() - .set_device(device, &style_guard, &data.document_stylesheets); + .set_device(device, &author_guard, &data.document_stylesheets); self.viewport_size = rw_data.stylist.viewport_constraints().map_or(current_screen_size, |constraints| { @@ -1063,10 +1067,16 @@ impl LayoutThread { } // If the entire flow tree is invalid, then it will be reflowed anyhow. + let ua_stylesheets = &*UA_STYLESHEETS; + let ua_or_user_guard = ua_stylesheets.shared_lock.read(); + let guards = ReadGuards { + author: &author_guard, + ua_or_user: &ua_or_user_guard, + }; let needs_dirtying = Arc::get_mut(&mut rw_data.stylist).unwrap().update( &data.document_stylesheets, - &style_guard, - Some(&*UA_STYLESHEETS), + &guards, + Some(ua_stylesheets), data.stylesheets_changed); let needs_reflow = viewport_size_changed && !needs_dirtying; if needs_dirtying { @@ -1113,7 +1123,7 @@ impl LayoutThread { } // Create a layout context for use throughout the following passes. - let mut layout_context = self.build_layout_context(&*rw_data, true); + let mut layout_context = self.build_layout_context(guards.clone(), &*rw_data, true); // NB: Type inference falls apart here for some reason, so we need to be very verbose. :-( let traversal_driver = if self.parallel_flag && self.parallel_traversal.is_some() { @@ -1172,7 +1182,7 @@ impl LayoutThread { } if opts::get().dump_rule_tree { - layout_context.style_context.stylist.rule_tree.dump_stdout(); + layout_context.style_context.stylist.rule_tree.dump_stdout(&guards); } // GC the rule tree if some heuristics are met. @@ -1341,7 +1351,13 @@ impl LayoutThread { page_clip_rect: max_rect(), }; - let mut layout_context = self.build_layout_context(&*rw_data, false); + let author_guard = AUTHOR_SHARED_LOCK.read(); + let ua_or_user_guard = UA_STYLESHEETS.shared_lock.read(); + let guards = ReadGuards { + author: &author_guard, + ua_or_user: &ua_or_user_guard, + }; + let mut layout_context = self.build_layout_context(guards, &*rw_data, false); if let Some(mut root_flow) = self.root_flow.clone() { // Perform an abbreviated style recalc that operates without access to the DOM. diff --git a/components/script/dom/bindings/trace.rs b/components/script/dom/bindings/trace.rs index 387709d73328..7cad9f985236 100644 --- a/components/script/dom/bindings/trace.rs +++ b/components/script/dom/bindings/trace.rs @@ -550,7 +550,7 @@ unsafe impl JSTraceable for StyleLocked { } } -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/cssstylerule.rs b/components/script/dom/cssstylerule.rs index 2423861e4464..fed2b947f90c 100644 --- a/components/script/dom/cssstylerule.rs +++ b/components/script/dom/cssstylerule.rs @@ -12,21 +12,20 @@ use dom::cssstyledeclaration::{CSSModificationAccess, CSSStyleDeclaration, CSSSt 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::StyleRule; #[dom_struct] pub struct CSSStyleRule { cssrule: CSSRule, #[ignore_heap_size_of = "Arc"] - stylerule: Arc>, + stylerule: Arc>, style_decl: MutNullableJS, } impl CSSStyleRule { - fn new_inherited(parent_stylesheet: &CSSStyleSheet, stylerule: Arc>) + fn new_inherited(parent_stylesheet: &CSSStyleSheet, stylerule: Arc>) -> CSSStyleRule { CSSStyleRule { cssrule: CSSRule::new_inherited(parent_stylesheet), @@ -37,7 +36,7 @@ impl CSSStyleRule { #[allow(unrooted_must_root)] pub fn new(window: &Window, parent_stylesheet: &CSSStyleSheet, - stylerule: Arc>) -> Root { + stylerule: Arc>) -> Root { reflect_dom_object(box CSSStyleRule::new_inherited(parent_stylesheet, stylerule), window, CSSStyleRuleBinding::Wrap) @@ -52,7 +51,7 @@ impl SpecificCSSRule for CSSStyleRule { fn get_css(&self) -> DOMString { let guard = self.cssrule.shared_lock().read(); - self.stylerule.read().to_css_string(&guard).into() + self.stylerule.read_with(&guard).to_css_string(&guard).into() } } @@ -60,11 +59,16 @@ impl CSSStyleRuleMethods for CSSStyleRule { // https://drafts.csswg.org/cssom/#dom-cssstylerule-style fn Style(&self) -> Root { self.style_decl.or_init(|| { - CSSStyleDeclaration::new(self.global().as_window(), - CSSStyleOwner::CSSRule(JS::from_ref(self.upcast()), - self.stylerule.read().block.clone()), - None, - CSSModificationAccess::ReadWrite) + let guard = self.cssrule.shared_lock().read(); + CSSStyleDeclaration::new( + self.global().as_window(), + CSSStyleOwner::CSSRule( + JS::from_ref(self.upcast()), + self.stylerule.read_with(&guard).block.clone() + ), + None, + CSSModificationAccess::ReadWrite + ) }) } } diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index 7a50fe5cf6c0..b875c75d8371 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::servo::AUTHOR_SHARED_LOCK; use style::shared_lock::SharedRwLock as StyleSharedRwLock; use style::str::{HTML_SPACE_CHARACTERS, split_html_space_chars, str_join}; use style::stylesheets::Stylesheet; @@ -2131,7 +2132,7 @@ impl Document { scripts: Default::default(), anchors: Default::default(), applets: Default::default(), - style_shared_lock: StyleSharedRwLock::new(), + style_shared_lock: AUTHOR_SHARED_LOCK.clone(), stylesheets: DOMRefCell::new(None), stylesheets_changed_since_reflow: Cell::new(false), stylesheet_list: MutNullableJS::new(None), diff --git a/components/script_layout_interface/wrapper_traits.rs b/components/script_layout_interface/wrapper_traits.rs index 0e7cb1603343..86e293d4a05c 100644 --- a/components/script_layout_interface/wrapper_traits.rs +++ b/components/script_layout_interface/wrapper_traits.rs @@ -405,6 +405,7 @@ pub trait ThreadSafeLayoutElement: Clone + Copy + Sized + Debug + let mut data = self.get_style_data().unwrap().borrow_mut(); let new_style = context.stylist.precomputed_values_for_pseudo( + &context.guards, &style_pseudo, Some(data.styles().primary.values()), CascadeFlags::empty()); @@ -421,6 +422,7 @@ pub trait ThreadSafeLayoutElement: Clone + Copy + Sized + Debug + let new_style = context.stylist .lazily_compute_pseudo_element_style( + &context.guards, unsafe { &self.unsafe_get() }, &style_pseudo, data.styles().primary.values()); diff --git a/components/style/Cargo.toml b/components/style/Cargo.toml index a4a6e5ac8235..c1c234c994ac 100644 --- a/components/style/Cargo.toml +++ b/components/style/Cargo.toml @@ -40,7 +40,6 @@ nsstring_vendor = {path = "gecko_bindings/nsstring_vendor", optional = true} num-integer = "0.1.32" num-traits = "0.1.32" ordered-float = "0.4" -owning_ref = "0.2.2" parking_lot = "0.3.3" pdqsort = "0.1.0" rayon = "0.6" diff --git a/components/style/context.rs b/components/style/context.rs index 44485915c2ca..93dfde0932b5 100644 --- a/components/style/context.rs +++ b/components/style/context.rs @@ -17,6 +17,7 @@ use parking_lot::RwLock; use selector_parser::PseudoElement; use selectors::matching::ElementSelectorFlags; use servo_config::opts; +use shared_lock::ReadGuards; use std::collections::HashMap; use std::env; use std::fmt; @@ -61,10 +62,13 @@ pub enum QuirksMode { /// /// There's exactly one of these during a given restyle traversal, and it's /// shared among the worker threads. -pub struct SharedStyleContext { +pub struct SharedStyleContext<'a> { /// The CSS selector stylist. pub stylist: Arc, + /// Guards for pre-acquired locks + pub guards: ReadGuards<'a>, + /// The animations that are currently running. pub running_animations: Arc>>>, @@ -85,7 +89,7 @@ pub struct SharedStyleContext { pub quirks_mode: QuirksMode, } -impl SharedStyleContext { +impl<'a> SharedStyleContext<'a> { /// Return a suitable viewport size in order to be used for viewport units. pub fn viewport_size(&self) -> Size2D { self.stylist.device.au_viewport_size() @@ -306,7 +310,7 @@ impl Drop for ThreadLocalStyleContext { /// shared style context, and a mutable reference to a local one. pub struct StyleContext<'a, E: TElement + 'a> { /// The shared style context reference. - pub shared: &'a SharedStyleContext, + pub shared: &'a SharedStyleContext<'a>, /// The thread-local style context (mutable) reference. pub thread_local: &'a mut ThreadLocalStyleContext, } diff --git a/components/style/gecko/arc_types.rs b/components/style/gecko/arc_types.rs index 58ec43cf3873..d400c01c5686 100644 --- a/components/style/gecko/arc_types.rs +++ b/components/style/gecko/arc_types.rs @@ -51,18 +51,7 @@ 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 +impl_arc_ffi!(Locked => RawServoStyleRule [Servo_StyleRule_AddRef, Servo_StyleRule_Release]); impl_arc_ffi!(Locked => RawServoImportRule diff --git a/components/style/gecko/data.rs b/components/style/gecko/data.rs index e0f7df023e2f..ad6a1d1629c0 100644 --- a/components/style/gecko/data.rs +++ b/components/style/gecko/data.rs @@ -13,7 +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 shared_lock::{ReadGuards, SharedRwLockReadGuard}; use std::collections::HashMap; use std::sync::Arc; use std::sync::mpsc::{Receiver, Sender, channel}; @@ -97,7 +97,7 @@ impl PerDocumentStyleDataImpl { 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, guard, None, true); + stylist.update(&self.stylesheets, &ReadGuards::same(guard), None, true); self.stylesheets_changed = false; } } diff --git a/components/style/gecko/traversal.rs b/components/style/gecko/traversal.rs index 3cb7f30cb1b0..7a68f66c75f1 100644 --- a/components/style/gecko/traversal.rs +++ b/components/style/gecko/traversal.rs @@ -13,14 +13,14 @@ use traversal::{DomTraversal, PerLevelTraversalData, TraversalDriver, recalc_sty /// This is the simple struct that Gecko uses to encapsulate a DOM traversal for /// styling. -pub struct RecalcStyleOnly { - shared: SharedStyleContext, +pub struct RecalcStyleOnly<'a> { + shared: SharedStyleContext<'a>, driver: TraversalDriver, } -impl RecalcStyleOnly { +impl<'a> RecalcStyleOnly<'a> { /// Create a `RecalcStyleOnly` traversal from a `SharedStyleContext`. - pub fn new(shared: SharedStyleContext, driver: TraversalDriver) -> Self { + pub fn new(shared: SharedStyleContext<'a>, driver: TraversalDriver) -> Self { RecalcStyleOnly { shared: shared, driver: driver, @@ -28,10 +28,11 @@ impl RecalcStyleOnly { } } -impl<'le> DomTraversal> for RecalcStyleOnly { +impl<'recalc, 'le> DomTraversal> for RecalcStyleOnly<'recalc> { type ThreadLocalContext = ThreadLocalStyleContext>; - fn process_preorder(&self, traversal_data: &mut PerLevelTraversalData, + fn process_preorder(&self, + traversal_data: &mut PerLevelTraversalData, thread_local: &mut Self::ThreadLocalContext, node: GeckoNode<'le>) { diff --git a/components/style/lib.rs b/components/style/lib.rs index 58dcafe9ceb2..eabb8d20c51f 100644 --- a/components/style/lib.rs +++ b/components/style/lib.rs @@ -60,7 +60,6 @@ extern crate matches; extern crate num_integer; extern crate num_traits; extern crate ordered_float; -extern crate owning_ref; extern crate parking_lot; extern crate pdqsort; extern crate rayon; @@ -99,7 +98,6 @@ pub mod keyframes; pub mod logical_geometry; pub mod matching; pub mod media_queries; -pub mod owning_handle; pub mod parallel; pub mod parser; pub mod restyle_hints; diff --git a/components/style/matching.rs b/components/style/matching.rs index 296862cfe20c..f58d429beb05 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -541,6 +541,7 @@ trait PrivateMatchMethods: TElement { let values = Arc::new(cascade(&shared_context.stylist.device, rule_node, + &shared_context.guards, inherited_values, layout_parent_style, Some(&mut cascade_info), @@ -784,6 +785,7 @@ pub trait MatchMethods : TElement { style_attribute, animation_rules, None, + &context.shared.guards, &mut applicable_declarations, &mut flags); let primary_rule_node = compute_rule_node(context, &mut applicable_declarations); @@ -809,6 +811,7 @@ pub trait MatchMethods : TElement { Some(context.thread_local.bloom_filter.filter()), None, pseudo_animation_rules, Some(&pseudo), + &context.shared.guards, &mut applicable_declarations, &mut flags); diff --git a/components/style/owning_handle.rs b/components/style/owning_handle.rs deleted file mode 100644 index 1294bf7e59fe..000000000000 --- a/components/style/owning_handle.rs +++ /dev/null @@ -1,89 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * 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/. */ - -#![allow(unsafe_code)] -#![deny(missing_docs)] - -//! A handle that encapsulate a reference to a given data along with its owner. - -use owning_ref::StableAddress; -use std::ops::{Deref, DerefMut}; - -/// `OwningHandle` is a complement to `OwningRef`. Where `OwningRef` allows -/// consumers to pass around an owned object and a dependent reference, -/// `OwningHandle` contains an owned object and a dependent _object_. -/// -/// `OwningHandle` can encapsulate a `RefMut` along with its associated -/// `RefCell`, or an `RwLockReadGuard` along with its associated `RwLock`. -/// However, the API is completely generic and there are no restrictions on -/// what types of owning and dependent objects may be used. -/// -/// `OwningHandle` is created by passing an owner object (which dereferences -/// to a stable address) along with a callback which receives a pointer to -/// that stable location. The callback may then dereference the pointer and -/// mint a dependent object, with the guarantee that the returned object will -/// not outlive the referent of the pointer. -/// -/// This does foist some unsafety onto the callback, which needs an `unsafe` -/// block to dereference the pointer. It would be almost good enough for -/// OwningHandle to pass a transmuted &'static reference to the callback -/// since the lifetime is infinite as far as the minted handle is concerned. -/// However, even an `Fn` callback can still allow the reference to escape -/// via a `StaticMutex` or similar, which technically violates the safety -/// contract. Some sort of language support in the lifetime system could -/// make this API a bit nicer. -pub struct OwningHandle - where O: StableAddress, - H: Deref, -{ - handle: H, - _owner: O, -} - -impl Deref for OwningHandle - where O: StableAddress, - H: Deref, -{ - type Target = H::Target; - fn deref(&self) -> &H::Target { - self.handle.deref() - } -} - -unsafe impl StableAddress for OwningHandle - where O: StableAddress, - H: StableAddress, -{} - -impl DerefMut for OwningHandle - where O: StableAddress, - H: DerefMut, -{ - fn deref_mut(&mut self) -> &mut H::Target { - self.handle.deref_mut() - } -} - -impl OwningHandle - where O: StableAddress, - H: Deref, -{ - /// Create a new OwningHandle. The provided callback will be invoked with - /// a pointer to the object owned by `o`, and the returned value is stored - /// as the object to which this `OwningHandle` will forward `Deref` and - /// `DerefMut`. - pub fn new(o: O, f: F) -> Self - where F: Fn(*const O::Target) -> H, - { - let h: H; - { - h = f(o.deref() as *const O::Target); - } - - OwningHandle { - handle: h, - _owner: o, - } - } -} diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 7b81b9ae5942..1ae30358efa8 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -31,6 +31,7 @@ use parser::{Parse, ParserContext, ParserContextExtraData}; use properties::animated_properties::TransitionProperty; #[cfg(feature = "servo")] use servo_config::prefs::PREFS; use servo_url::ServoUrl; +use shared_lock::ReadGuards; use style_traits::ToCss; use stylesheets::Origin; #[cfg(feature = "servo")] use values::Either; @@ -1860,6 +1861,7 @@ bitflags! { /// pub fn cascade(device: &Device, rule_node: &StrongRuleNode, + guards: &ReadGuards, parent_style: Option<<&ComputedValues>, layout_parent_style: Option<<&ComputedValues>, cascade_info: Option<<&mut CascadeInfo>, @@ -1882,11 +1884,12 @@ pub fn cascade(device: &Device, // Hold locks until after the apply_declarations() call returns. // Use filter_map because the root node has no style source. - let lock_guards = rule_node.self_and_ancestors().filter_map(|node| { - node.style_source().map(|source| (source.read(), node.importance())) + let declaration_blocks = rule_node.self_and_ancestors().filter_map(|node| { + let guard = node.cascade_level().guard(guards); + node.style_source().map(|source| (source.read(guard), node.importance())) }).collect::>(); let iter_declarations = || { - lock_guards.iter().flat_map(|&(ref source, source_importance)| { + declaration_blocks.iter().flat_map(|&(ref source, source_importance)| { source.declarations().iter() // Yield declarations later in source order (with more precedence) first. .rev() diff --git a/components/style/rule_tree/mod.rs b/components/style/rule_tree/mod.rs index 9a725a19286f..c1d4b27edf5c 100644 --- a/components/style/rule_tree/mod.rs +++ b/components/style/rule_tree/mod.rs @@ -10,9 +10,9 @@ use arc_ptr_eq; #[cfg(feature = "servo")] use heapsize::HeapSizeOf; -use owning_handle::OwningHandle; use parking_lot::{RwLock, RwLockReadGuard}; use properties::{Importance, PropertyDeclarationBlock}; +use shared_lock::{Locked, ReadGuards, SharedRwLockReadGuard}; use std::io::{self, Write}; use std::ptr; use std::sync::Arc; @@ -52,35 +52,11 @@ pub struct RuleTree { #[derive(Debug, Clone)] pub enum StyleSource { /// A style rule stable pointer. - Style(Arc>), + Style(Arc>), /// A declaration block stable pointer. Declarations(Arc>), } -type StyleSourceGuardHandle<'a> = - OwningHandle< - RwLockReadGuard<'a, StyleRule>, - RwLockReadGuard<'a, PropertyDeclarationBlock>>; - -/// A guard for a given style source. -pub enum StyleSourceGuard<'a> { - /// A guard for a style rule. - Style(StyleSourceGuardHandle<'a>), - /// A guard for a declaration block. - Declarations(RwLockReadGuard<'a, PropertyDeclarationBlock>), -} - -impl<'a> ::std::ops::Deref for StyleSourceGuard<'a> { - type Target = PropertyDeclarationBlock; - - fn deref(&self) -> &Self::Target { - match *self { - StyleSourceGuard::Declarations(ref block) => &*block, - StyleSourceGuard::Style(ref handle) => &*handle, - } - } -} - impl StyleSource { #[inline] fn ptr_equals(&self, other: &Self) -> bool { @@ -92,28 +68,27 @@ impl StyleSource { } } - fn dump(&self, writer: &mut W) { + fn dump(&self, guard: &SharedRwLockReadGuard, writer: &mut W) { use self::StyleSource::*; if let Style(ref rule) = *self { - let _ = write!(writer, "{:?}", rule.read().selectors); + let rule = rule.read_with(guard); + let _ = write!(writer, "{:?}", rule.selectors); } - let _ = write!(writer, " -> {:?}", self.read().declarations()); + let _ = write!(writer, " -> {:?}", self.read(guard).declarations()); } /// Read the style source guard, and obtain thus read access to the /// underlying property declaration block. #[inline] - pub fn read<'a>(&'a self) -> StyleSourceGuard<'a> { - use self::StyleSource::*; - match *self { - Style(ref rule) => { - let owning_ref = OwningHandle::new(rule.read(), |r| unsafe { &*r }.block.read()); - StyleSourceGuard::Style(owning_ref) - } - Declarations(ref block) => StyleSourceGuard::Declarations(block.read()), - } + pub fn read<'a>(&'a self, guard: &'a SharedRwLockReadGuard) + -> RwLockReadGuard<'a, PropertyDeclarationBlock> { + let block = match *self { + StyleSource::Style(ref rule) => &rule.read_with(guard).block, + StyleSource::Declarations(ref block) => block, + }; + block.read() } } @@ -137,15 +112,15 @@ impl RuleTree { self.root.clone() } - fn dump(&self, writer: &mut W) { + fn dump(&self, guards: &ReadGuards, writer: &mut W) { let _ = writeln!(writer, " + RuleTree"); - self.root.get().dump(writer, 0); + self.root.get().dump(guards, writer, 0); } /// Dump the rule tree to stdout. - pub fn dump_stdout(&self) { + pub fn dump_stdout(&self, guards: &ReadGuards) { let mut stdout = io::stdout(); - self.dump(&mut stdout); + self.dump(guards, &mut stdout); } /// Insert the given rules, that must be in proper order by specifity, and @@ -307,6 +282,17 @@ pub enum CascadeLevel { } impl CascadeLevel { + /// Select a lock guard for this level + pub fn guard<'a>(&self, guards: &'a ReadGuards<'a>) -> &'a SharedRwLockReadGuard<'a> { + match *self { + CascadeLevel::UANormal | + CascadeLevel::UserNormal | + CascadeLevel::UserImportant | + CascadeLevel::UAImportant => guards.ua_or_user, + _ => guards.author, + } + } + /// Returns whether this cascade level is unique per element, in which case /// we can replace the path in the cascade without fear. pub fn is_unique_per_element(&self) -> bool { @@ -450,7 +436,7 @@ impl RuleNode { } } - fn dump(&self, writer: &mut W, indent: usize) { + fn dump(&self, guards: &ReadGuards, writer: &mut W, indent: usize) { const INDENT_INCREMENT: usize = 4; for _ in 0..indent { @@ -467,7 +453,7 @@ impl RuleNode { match self.source { Some(ref source) => { - source.dump(writer); + source.dump(self.level.guard(guards), writer); } None => { if indent != 0 { @@ -479,7 +465,7 @@ impl RuleNode { let _ = write!(writer, "\n"); for child in self.iter_children() { - child.get().dump(writer, indent + INDENT_INCREMENT); + child.get().dump(guards, writer, indent + INDENT_INCREMENT); } } @@ -627,6 +613,11 @@ impl StrongRuleNode { self.get().source.as_ref() } + /// The cascade level for this node + pub fn cascade_level(&self) -> CascadeLevel { + self.get().level + } + /// Get the importance that this rule node represents. pub fn importance(&self) -> Importance { self.get().level.importance() diff --git a/components/style/servo/mod.rs b/components/style/servo/mod.rs index ad741616eeb7..ff6890658d38 100644 --- a/components/style/servo/mod.rs +++ b/components/style/servo/mod.rs @@ -9,3 +9,13 @@ pub mod media_queries; pub mod restyle_damage; pub mod selector_parser; + +use shared_lock::SharedRwLock; + +lazy_static! { + /// Per-process shared lock for author-origin stylesheets + /// + /// FIXME: make it per-document or per-pipeline instead: + /// https://github.com/servo/servo/issues/16027 + pub static ref AUTHOR_SHARED_LOCK: SharedRwLock = SharedRwLock::new(); +} diff --git a/components/style/shared_lock.rs b/components/style/shared_lock.rs index d4d66b8fc5d8..c92ab9efccf2 100644 --- a/components/style/shared_lock.rs +++ b/components/style/shared_lock.rs @@ -177,3 +177,23 @@ pub trait ToCssWithGuard { s } } + +/// Guards for a document +#[derive(Clone)] +pub struct ReadGuards<'a> { + /// For author-origin stylesheets + pub author: &'a SharedRwLockReadGuard<'a>, + + /// For user-agent-origin and user-origin stylesheets + pub ua_or_user: &'a SharedRwLockReadGuard<'a>, +} + +impl<'a> ReadGuards<'a> { + /// Same guard for all origins + pub fn same(guard: &'a SharedRwLockReadGuard<'a>) -> Self { + ReadGuards { + author: guard, + ua_or_user: guard, + } + } +} diff --git a/components/style/stylesheets.rs b/components/style/stylesheets.rs index d132c64bbb92..96c553c4a499 100644 --- a/components/style/stylesheets.rs +++ b/components/style/stylesheets.rs @@ -215,7 +215,7 @@ pub enum CssRule { Namespace(Arc>), Import(Arc>), - Style(Arc>), + Style(Arc>), Media(Arc>), FontFace(Arc>), Viewport(Arc>), @@ -380,7 +380,7 @@ impl ToCssWithGuard for CssRule { match *self { 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::Style(ref lock) => lock.read_with(guard).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), @@ -736,18 +736,6 @@ 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 { @@ -1030,7 +1018,7 @@ impl<'a, 'b> QualifiedRuleParser for NestedRuleParser<'a, 'b> { fn parse_block(&mut self, prelude: SelectorList, input: &mut Parser) -> Result { - Ok(CssRule::Style(Arc::new(RwLock::new(StyleRule { + Ok(CssRule::Style(Arc::new(self.shared_lock.wrap(StyleRule { selectors: prelude, block: Arc::new(RwLock::new(parse_property_declaration_list(self.context, input))) })))) diff --git a/components/style/stylist.rs b/components/style/stylist.rs index a110e4f7abba..2166e81946a7 100644 --- a/components/style/stylist.rs +++ b/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; -use shared_lock::SharedRwLockReadGuard; +use shared_lock::{Locked, SharedRwLockReadGuard, ReadGuards}; use sink::Push; use smallvec::VecLike; use std::borrow::Borrow; @@ -159,7 +159,7 @@ impl Stylist { /// device is dirty, which means we need to re-evaluate media queries. pub fn update(&mut self, doc_stylesheets: &[Arc], - doc_guard: &SharedRwLockReadGuard, + guards: &ReadGuards, ua_stylesheets: Option<&UserAgentStylesheets>, stylesheets_changed: bool) -> bool { if !(self.is_device_dirty || stylesheets_changed) { @@ -168,7 +168,7 @@ impl Stylist { let cascaded_rule = ViewportRule { declarations: viewport::Cascade::from_stylesheets( - doc_stylesheets, doc_guard, &self.device + doc_stylesheets, guards.author, &self.device ).finish(), }; @@ -196,18 +196,17 @@ 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, &ua_guard); + self.add_stylesheet(&stylesheet, guards.ua_or_user); } if self.quirks_mode { - self.add_stylesheet(&ua_stylesheets.quirks_mode_stylesheet, &ua_guard); + self.add_stylesheet(&ua_stylesheets.quirks_mode_stylesheet, guards.ua_or_user); } } for ref stylesheet in doc_stylesheets.iter() { - self.add_stylesheet(stylesheet, doc_guard); + self.add_stylesheet(stylesheet, guards.author); } debug!("Stylist stats:"); @@ -221,8 +220,9 @@ impl Stylist { SelectorImpl::each_precomputed_pseudo_element(|pseudo| { if let Some(map) = self.pseudos_map.remove(&pseudo) { let declarations = - map.user_agent.get_universal_rules(CascadeLevel::UANormal, - CascadeLevel::UAImportant); + map.user_agent.get_universal_rules( + guards.ua_or_user, CascadeLevel::UANormal, CascadeLevel::UAImportant + ); self.precomputed_pseudo_element_decls.insert(pseudo, declarations); } }); @@ -241,9 +241,9 @@ impl Stylist { stylesheet.effective_rules(&device, guard, |rule| { match *rule { - CssRule::Style(ref style_rule) => { - let guard = style_rule.read(); - for selector in &guard.selectors.0 { + CssRule::Style(ref locked) => { + let style_rule = locked.read_with(&guard); + for selector in &style_rule.selectors.0 { let map = if let Some(ref pseudo) = selector.pseudo_element { self.pseudos_map .entry(pseudo.clone()) @@ -255,14 +255,14 @@ impl Stylist { map.insert(Rule { selector: selector.complex_selector.clone(), - style_rule: style_rule.clone(), + style_rule: locked.clone(), specificity: selector.specificity, source_order: self.rules_source_order, }); } self.rules_source_order += 1; - for selector in &guard.selectors.0 { + for selector in &style_rule.selectors.0 { self.state_deps.note_selector(&selector.complex_selector); if selector.affects_siblings() { self.sibling_affecting_selectors.push(selector.clone()); @@ -300,6 +300,7 @@ impl Stylist { /// values. The flow constructor uses this flag when constructing anonymous /// flows. pub fn precomputed_values_for_pseudo(&self, + guards: &ReadGuards, pseudo: &PseudoElement, parent: Option<&Arc>, cascade_flags: CascadeFlags) @@ -333,6 +334,7 @@ impl Stylist { let computed = properties::cascade(&self.device, &rule_node, + guards, parent.map(|p| &**p), parent.map(|p| &**p), None, @@ -344,6 +346,7 @@ impl Stylist { /// Returns the style for an anonymous box of the given type. #[cfg(feature = "servo")] pub fn style_for_anonymous_box(&self, + guards: &ReadGuards, pseudo: &PseudoElement, parent_style: &Arc) -> Arc { @@ -368,7 +371,7 @@ impl Stylist { if inherit_all { cascade_flags.insert(INHERIT_ALL); } - self.precomputed_values_for_pseudo(&pseudo, Some(parent_style), cascade_flags) + self.precomputed_values_for_pseudo(guards, &pseudo, Some(parent_style), cascade_flags) .values.unwrap() } @@ -380,6 +383,7 @@ impl Stylist { /// Check the documentation on lazy pseudo-elements in /// docs/components/style.md pub fn lazily_compute_pseudo_element_style(&self, + guards: &ReadGuards, element: &E, pseudo: &PseudoElement, parent: &Arc) @@ -401,6 +405,7 @@ impl Stylist { None, AnimationRules(None, None), Some(pseudo), + guards, &mut declarations, &mut flags); @@ -415,6 +420,7 @@ impl Stylist { let computed = properties::cascade(&self.device, &rule_node, + guards, Some(&**parent), Some(&**parent), None, @@ -539,6 +545,7 @@ impl Stylist { style_attribute: Option<&Arc>>, animation_rules: AnimationRules, pseudo_element: Option<&PseudoElement>, + guards: &ReadGuards, applicable_declarations: &mut V, flags: &mut ElementSelectorFlags) -> StyleRelations where E: TElement + @@ -564,6 +571,7 @@ impl Stylist { // Step 1: Normal user-agent rules. map.user_agent.get_all_matching_rules(element, parent_bf, + guards.ua_or_user, applicable_declarations, &mut relations, flags, @@ -588,6 +596,7 @@ impl Stylist { // Step 3: User and author normal rules. map.user.get_all_matching_rules(element, parent_bf, + guards.ua_or_user, applicable_declarations, &mut relations, flags, @@ -595,6 +604,7 @@ impl Stylist { debug!("user normal: {:?}", relations); map.author.get_all_matching_rules(element, parent_bf, + guards.author, applicable_declarations, &mut relations, flags, @@ -629,6 +639,7 @@ impl Stylist { // Step 6: Author-supplied `!important` rules. map.author.get_all_matching_rules(element, parent_bf, + guards.author, applicable_declarations, &mut relations, flags, @@ -652,6 +663,7 @@ impl Stylist { // Step 8: User `!important` rules. map.user.get_all_matching_rules(element, parent_bf, + guards.ua_or_user, applicable_declarations, &mut relations, flags, @@ -665,6 +677,7 @@ impl Stylist { // Step 9: UA `!important` rules. map.user_agent.get_all_matching_rules(element, parent_bf, + guards.ua_or_user, applicable_declarations, &mut relations, flags, @@ -903,6 +916,7 @@ impl SelectorMap { pub fn get_all_matching_rules(&self, element: &E, parent_bf: Option<&BloomFilter>, + guard: &SharedRwLockReadGuard, matching_rules_list: &mut V, relations: &mut StyleRelations, flags: &mut ElementSelectorFlags, @@ -921,6 +935,7 @@ impl SelectorMap { parent_bf, &self.id_hash, &id, + guard, matching_rules_list, relations, flags, @@ -932,6 +947,7 @@ impl SelectorMap { parent_bf, &self.class_hash, class, + guard, matching_rules_list, relations, flags, @@ -947,6 +963,7 @@ impl SelectorMap { parent_bf, local_name_hash, element.get_local_name(), + guard, matching_rules_list, relations, flags, @@ -955,6 +972,7 @@ impl SelectorMap { SelectorMap::get_matching_rules(element, parent_bf, &self.other_rules, + guard, matching_rules_list, relations, flags, @@ -968,6 +986,7 @@ impl SelectorMap { /// Append to `rule_list` all universal Rules (rules with selector `*|*`) in /// `self` sorted by specificity and source order. pub fn get_universal_rules(&self, + guard: &SharedRwLockReadGuard, cascade_level: CascadeLevel, important_cascade_level: CascadeLevel) -> Vec { @@ -985,8 +1004,8 @@ impl SelectorMap { for rule in self.other_rules.iter() { if rule.selector.compound_selector.is_empty() && rule.selector.next.is_none() { - let guard = rule.style_rule.read(); - let block = guard.block.read(); + let style_rule = rule.style_rule.read_with(guard); + let block = style_rule.block.read(); if block.any_normal() { matching_rules_list.push( rule.to_applicable_declaration_block(cascade_level)); @@ -1014,6 +1033,7 @@ impl SelectorMap { parent_bf: Option<&BloomFilter>, hash: &FnvHashMap>, key: &BorrowedStr, + guard: &SharedRwLockReadGuard, matching_rules: &mut Vector, relations: &mut StyleRelations, flags: &mut ElementSelectorFlags, @@ -1027,6 +1047,7 @@ impl SelectorMap { SelectorMap::get_matching_rules(element, parent_bf, rules, + guard, matching_rules, relations, flags, @@ -1038,6 +1059,7 @@ impl SelectorMap { fn get_matching_rules(element: &E, parent_bf: Option<&BloomFilter>, rules: &[Rule], + guard: &SharedRwLockReadGuard, matching_rules: &mut V, relations: &mut StyleRelations, flags: &mut ElementSelectorFlags, @@ -1046,8 +1068,8 @@ impl SelectorMap { V: VecLike { for rule in rules.iter() { - let guard = rule.style_rule.read(); - let block = guard.block.read(); + let style_rule = rule.style_rule.read_with(guard); + let block = style_rule.block.read(); let any_declaration_for_importance = if cascade_level.is_important() { block.any_important() } else { @@ -1145,7 +1167,7 @@ pub struct Rule { pub selector: Arc>, /// The actual style rule. #[cfg_attr(feature = "servo", ignore_heap_size_of = "Arc")] - pub style_rule: Arc>, + pub style_rule: Arc>, /// The source order this style rule appears in. pub source_order: usize, /// The specificity of the rule this selector represents. diff --git a/components/style/traversal.rs b/components/style/traversal.rs index 5c413b9c6bb5..1b7c8042baed 100644 --- a/components/style/traversal.rs +++ b/components/style/traversal.rs @@ -312,7 +312,8 @@ pub trait DomTraversal : Sync { } /// Helper for the function below. -fn resolve_style_internal(context: &mut StyleContext, element: E, ensure_data: &F) +fn resolve_style_internal(context: &mut StyleContext, + element: E, ensure_data: &F) -> Option where E: TElement, F: Fn(E), diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index 243804390624..bbe8bc6907d3 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -23,7 +23,6 @@ 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}; @@ -77,12 +76,11 @@ 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, ToCssWithGuard, Locked}; +use style::shared_lock::{SharedRwLock, SharedRwLockReadGuard, ReadGuards, ToCssWithGuard, Locked}; 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; @@ -167,12 +165,14 @@ pub extern "C" fn Servo_Shutdown() { gecko_properties::shutdown(); } -fn create_shared_context(per_doc_data: &PerDocumentStyleDataImpl) -> SharedStyleContext { +fn create_shared_context<'a>(guard: &'a SharedRwLockReadGuard, + per_doc_data: &PerDocumentStyleDataImpl) -> SharedStyleContext<'a> { let local_context_data = ThreadLocalStyleContextCreationInfo::new(per_doc_data.new_animations_sender.clone()); SharedStyleContext { stylist: per_doc_data.stylist.clone(), + guards: ReadGuards::same(guard), running_animations: per_doc_data.running_animations.clone(), expired_animations: per_doc_data.expired_animations.clone(), // FIXME(emilio): Stop boxing here. @@ -205,8 +205,9 @@ fn traverse_subtree(element: GeckoElement, raw_data: RawServoStyleSetBorrowed, debug!("Traversing subtree:"); debug!("{:?}", ShowSubtreeData(element.as_node())); - let shared_style_context = create_shared_context(&per_doc_data); let global_style_data = &*GLOBAL_STYLE_DATA; + let guard = global_style_data.shared_lock.read(); + let shared_style_context = create_shared_context(&guard, &per_doc_data); let traversal_driver = if global_style_data.style_thread_pool.is_none() { TraversalDriver::Sequential @@ -625,22 +626,28 @@ impl_basic_rule_funcs! { (Namespace, NamespaceRule, RawServoNamespaceRule), #[no_mangle] pub extern "C" fn Servo_StyleRule_GetStyle(rule: RawServoStyleRuleBorrowed) -> RawServoDeclarationBlockStrong { - let rule = RwLock::::as_arc(&rule); - rule.read().block.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).block.clone().into_strong() } #[no_mangle] pub extern "C" fn Servo_StyleRule_SetStyle(rule: RawServoStyleRuleBorrowed, declarations: RawServoDeclarationBlockBorrowed) { - let rule = RwLock::::as_arc(&rule); + let global_style_data = &*GLOBAL_STYLE_DATA; + let mut guard = global_style_data.shared_lock.write(); + let rule = Locked::::as_arc(&rule); let declarations = RwLock::::as_arc(&declarations); - rule.write().block = declarations.clone(); + rule.write_with(&mut guard).block = declarations.clone(); } #[no_mangle] pub extern "C" fn Servo_StyleRule_GetSelectorText(rule: RawServoStyleRuleBorrowed, result: *mut nsAString) { - let rule = RwLock::::as_arc(&rule); - rule.read().selectors.to_css(unsafe { result.as_mut().unwrap() }).unwrap(); + 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).selectors.to_css(unsafe { result.as_mut().unwrap() }).unwrap(); } #[no_mangle] @@ -681,6 +688,9 @@ pub extern "C" fn Servo_ComputedValues_GetForAnonymousBox(parent_style_or_null: skip_display_fixup: bool, raw_data: RawServoStyleSetBorrowed) -> ServoComputedValuesStrong { + let global_style_data = &*GLOBAL_STYLE_DATA; + let guard = global_style_data.shared_lock.read(); + let guards = ReadGuards::same(&guard); let data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut(); let atom = Atom::from(pseudo_tag); let pseudo = PseudoElement::from_atom_unchecked(atom, /* anon_box = */ true); @@ -691,7 +701,7 @@ pub extern "C" fn Servo_ComputedValues_GetForAnonymousBox(parent_style_or_null: if skip_display_fixup { cascade_flags.insert(SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP); } - data.stylist.precomputed_values_for_pseudo(&pseudo, maybe_parent, + data.stylist.precomputed_values_for_pseudo(&guards, &pseudo, maybe_parent, cascade_flags) .values.unwrap() .into_strong() @@ -717,14 +727,16 @@ pub extern "C" fn Servo_ResolvePseudoStyle(element: RawGeckoElementBorrowed, }; } - match get_pseudo_style(element, pseudo_tag, data.styles(), doc_data) { + let global_style_data = &*GLOBAL_STYLE_DATA; + let guard = global_style_data.shared_lock.read(); + match get_pseudo_style(&guard, element, pseudo_tag, data.styles(), doc_data) { Some(values) => values.into_strong(), None if !is_probe => data.styles().primary.values().clone().into_strong(), None => Strong::null(), } } -fn get_pseudo_style(element: GeckoElement, pseudo_tag: *mut nsIAtom, +fn get_pseudo_style(guard: &SharedRwLockReadGuard, element: GeckoElement, pseudo_tag: *mut nsIAtom, styles: &ElementStyles, doc_data: &PerDocumentStyleData) -> Option> { @@ -735,7 +747,9 @@ fn get_pseudo_style(element: GeckoElement, pseudo_tag: *mut nsIAtom, PseudoElementCascadeType::Lazy => { let d = doc_data.borrow_mut(); let base = styles.primary.values(); - d.stylist.lazily_compute_pseudo_element_style(&element, + let guards = ReadGuards::same(guard); + d.stylist.lazily_compute_pseudo_element_style(&guards, + &element, &pseudo, base) .map(|s| s.values().clone()) @@ -1463,11 +1477,13 @@ pub extern "C" fn Servo_ResolveStyleLazily(element: RawGeckoElementBorrowed, raw_data: RawServoStyleSetBorrowed) -> ServoComputedValuesStrong { + let global_style_data = &*GLOBAL_STYLE_DATA; + let guard = global_style_data.shared_lock.read(); let element = GeckoElement(element); let doc_data = PerDocumentStyleData::from_ffi(raw_data); let finish = |styles: &ElementStyles| -> Arc { let maybe_pseudo = if !pseudo_tag.is_null() { - get_pseudo_style(element, pseudo_tag, styles, doc_data) + get_pseudo_style(&guard, element, pseudo_tag, styles, doc_data) } else { None }; @@ -1483,7 +1499,7 @@ pub extern "C" fn Servo_ResolveStyleLazily(element: RawGeckoElementBorrowed, } // We don't have the style ready. Go ahead and compute it as necessary. - let shared = create_shared_context(&mut doc_data.borrow_mut()); + let shared = create_shared_context(&guard, &mut doc_data.borrow_mut()); let mut tlc = ThreadLocalStyleContext::new(&shared); let mut context = StyleContext { shared: &shared, diff --git a/tests/unit/style/Cargo.toml b/tests/unit/style/Cargo.toml index 988386769379..7939e6173422 100644 --- a/tests/unit/style/Cargo.toml +++ b/tests/unit/style/Cargo.toml @@ -17,7 +17,6 @@ app_units = "0.4" cssparser = "0.12" euclid = "0.11" html5ever-atoms = "0.2" -owning_ref = "0.2.2" parking_lot = "0.3" rayon = "0.6" rustc-serialize = "0.3" diff --git a/tests/unit/style/lib.rs b/tests/unit/style/lib.rs index 9d7cdcae651b..887de470ffac 100644 --- a/tests/unit/style/lib.rs +++ b/tests/unit/style/lib.rs @@ -9,7 +9,6 @@ extern crate app_units; extern crate cssparser; extern crate euclid; #[macro_use] extern crate html5ever_atoms; -extern crate owning_ref; extern crate parking_lot; extern crate rayon; extern crate rustc_serialize; @@ -26,7 +25,6 @@ mod attr; mod keyframes; mod logical_geometry; mod media_queries; -mod owning_handle; mod parsing; mod properties; mod rule_tree; diff --git a/tests/unit/style/owning_handle.rs b/tests/unit/style/owning_handle.rs deleted file mode 100644 index cf792ef96051..000000000000 --- a/tests/unit/style/owning_handle.rs +++ /dev/null @@ -1,34 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * 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 owning_ref::RcRef; -use std::cell::RefCell; -use std::rc::Rc; -use std::sync::{Arc, RwLock}; -use style::owning_handle::OwningHandle; - -#[test] -fn owning_handle() { - use std::cell::RefCell; - let cell = Rc::new(RefCell::new(2)); - let cell_ref = RcRef::new(cell); - let mut handle = OwningHandle::new(cell_ref, |x| unsafe { x.as_ref() }.unwrap().borrow_mut()); - assert_eq!(*handle, 2); - *handle = 3; - assert_eq!(*handle, 3); -} - -#[test] -fn nested() { - let result = { - let complex = Rc::new(RefCell::new(Arc::new(RwLock::new("someString")))); - let curr = RcRef::new(complex); - let curr = OwningHandle::new(curr, |x| unsafe { x.as_ref() }.unwrap().borrow_mut()); - let mut curr = OwningHandle::new(curr, |x| unsafe { x.as_ref() }.unwrap().try_write().unwrap()); - assert_eq!(*curr, "someString"); - *curr = "someOtherString"; - curr - }; - assert_eq!(*result, "someOtherString"); -} diff --git a/tests/unit/style/stylesheets.rs b/tests/unit/style/stylesheets.rs index 99ec36762360..2bcb9b58a77a 100644 --- a/tests/unit/style/stylesheets.rs +++ b/tests/unit/style/stylesheets.rs @@ -81,7 +81,7 @@ fn test_parse_stylesheet() { prefix: None, url: NsAtom::from("http://www.w3.org/1999/xhtml") }))), - CssRule::Style(Arc::new(RwLock::new(StyleRule { + CssRule::Style(Arc::new(stylesheet.shared_lock.wrap(StyleRule { selectors: SelectorList(vec![ Selector { complex_selector: Arc::new(ComplexSelector { @@ -117,7 +117,7 @@ fn test_parse_stylesheet() { Importance::Important), ]))), }))), - CssRule::Style(Arc::new(RwLock::new(StyleRule { + CssRule::Style(Arc::new(stylesheet.shared_lock.wrap(StyleRule { selectors: SelectorList(vec![ Selector { complex_selector: Arc::new(ComplexSelector { @@ -159,7 +159,7 @@ fn test_parse_stylesheet() { Importance::Normal), ]))), }))), - CssRule::Style(Arc::new(RwLock::new(StyleRule { + CssRule::Style(Arc::new(stylesheet.shared_lock.wrap(StyleRule { selectors: SelectorList(vec![ Selector { complex_selector: Arc::new(ComplexSelector { diff --git a/tests/unit/style/stylist.rs b/tests/unit/style/stylist.rs index ab04f2e5dd5b..34a7feff9658 100644 --- a/tests/unit/style/stylist.rs +++ b/tests/unit/style/stylist.rs @@ -11,17 +11,19 @@ use style::properties::{PropertyDeclarationBlock, PropertyDeclaration}; use style::properties::{longhands, Importance}; use style::rule_tree::CascadeLevel; use style::selector_parser::SelectorParser; +use style::shared_lock::SharedRwLock; use style::stylesheets::StyleRule; use style::stylist::{Rule, SelectorMap}; use style::thread_state; /// Helper method to get some Rules from selector strings. /// Each sublist of the result contains the Rules for one StyleRule. -fn get_mock_rules(css_selectors: &[&str]) -> Vec> { - css_selectors.iter().enumerate().map(|(i, selectors)| { +fn get_mock_rules(css_selectors: &[&str]) -> (Vec>, SharedRwLock) { + let shared_lock = SharedRwLock::new(); + (css_selectors.iter().enumerate().map(|(i, selectors)| { let selectors = SelectorParser::parse_author_origin_no_namespace(selectors).unwrap(); - let rule = Arc::new(RwLock::new(StyleRule { + let locked = Arc::new(shared_lock.wrap(StyleRule { selectors: selectors, block: Arc::new(RwLock::new(PropertyDeclarationBlock::with_one( PropertyDeclaration::Display( @@ -30,21 +32,22 @@ fn get_mock_rules(css_selectors: &[&str]) -> Vec> { ))), })); - let guard = rule.read(); - guard.selectors.0.iter().map(|s| { + let guard = shared_lock.read(); + let rule = locked.read_with(&guard); + rule.selectors.0.iter().map(|s| { Rule { selector: s.complex_selector.clone(), - style_rule: rule.clone(), + style_rule: locked.clone(), specificity: s.specificity, source_order: i, } }).collect() - }).collect() + }).collect(), shared_lock) } -fn get_mock_map(selectors: &[&str]) -> SelectorMap { +fn get_mock_map(selectors: &[&str]) -> (SelectorMap, SharedRwLock) { let mut map = SelectorMap::new(); - let selector_rules = get_mock_rules(selectors); + let (selector_rules, shared_lock) = get_mock_rules(selectors); for rules in selector_rules.into_iter() { for rule in rules.into_iter() { @@ -52,12 +55,12 @@ fn get_mock_map(selectors: &[&str]) -> SelectorMap { } } - map + (map, shared_lock) } #[test] fn test_rule_ordering_same_specificity() { - let rules_list = get_mock_rules(&["a.intro", "img.sidebar"]); + let (rules_list, _) = get_mock_rules(&["a.intro", "img.sidebar"]); let a = &rules_list[0][0]; let b = &rules_list[1][0]; assert!((a.specificity, a.source_order) < ((b.specificity, b.source_order)), @@ -67,21 +70,21 @@ fn test_rule_ordering_same_specificity() { #[test] fn test_get_id_name() { - let rules_list = get_mock_rules(&[".intro", "#top"]); + let (rules_list, _) = get_mock_rules(&[".intro", "#top"]); assert_eq!(SelectorMap::get_id_name(&rules_list[0][0]), None); assert_eq!(SelectorMap::get_id_name(&rules_list[1][0]), Some(Atom::from("top"))); } #[test] fn test_get_class_name() { - let rules_list = get_mock_rules(&[".intro.foo", "#top"]); + let (rules_list, _) = get_mock_rules(&[".intro.foo", "#top"]); assert_eq!(SelectorMap::get_class_name(&rules_list[0][0]), Some(Atom::from("intro"))); assert_eq!(SelectorMap::get_class_name(&rules_list[1][0]), None); } #[test] fn test_get_local_name() { - let rules_list = get_mock_rules(&["img.foo", "#top", "IMG", "ImG"]); + let (rules_list, _) = get_mock_rules(&["img.foo", "#top", "IMG", "ImG"]); let check = |i: usize, names: Option<(&str, &str)>| { assert!(SelectorMap::get_local_name(&rules_list[i][0]) == names.map(|(name, lower_name)| LocalNameSelector { @@ -96,7 +99,7 @@ fn test_get_local_name() { #[test] fn test_insert() { - let rules_list = get_mock_rules(&[".intro.foo", "#top"]); + let (rules_list, _) = get_mock_rules(&[".intro.foo", "#top"]); let mut selector_map = SelectorMap::new(); selector_map.insert(rules_list[1][0].clone()); assert_eq!(1, selector_map.id_hash.get(&Atom::from("top")).unwrap()[0].source_order); @@ -108,10 +111,11 @@ fn test_insert() { #[test] fn test_get_universal_rules() { thread_state::initialize(thread_state::LAYOUT); - let map = get_mock_map(&["*|*", "#foo > *|*", ".klass", "#id"]); + let (map, shared_lock) = get_mock_map(&["*|*", "#foo > *|*", ".klass", "#id"]); - let decls = map.get_universal_rules(CascadeLevel::UserNormal, - CascadeLevel::UserImportant); + let guard = shared_lock.read(); + let decls = map.get_universal_rules( + &guard, CascadeLevel::UserNormal, CascadeLevel::UserImportant); assert_eq!(decls.len(), 1); }