diff options
author | Bobby Holley <bobbyholley@gmail.com> | 2016-10-09 13:37:57 -0700 |
---|---|---|
committer | Michael Howell <michael@notriddle.com> | 2016-10-10 20:36:31 -0700 |
commit | bfbbef6ecdfd8bdd0a58bd1a0c7ed95c182b98a7 (patch) | |
tree | 48a687153507447f1e43cb3a0b5ded1d510e23dc | |
parent | ddbc016f51e169369a6c25d68b453dc299cc8677 (diff) | |
download | servo-bfbbef6ecdfd8bdd0a58bd1a0c7ed95c182b98a7.tar.gz servo-bfbbef6ecdfd8bdd0a58bd1a0c7ed95c182b98a7.zip |
Remove borrow_data and mutate_data from TNode.
The new restyle architecture doesn't store these things in consistent
places, so we need a more abstract API.
-rw-r--r-- | components/layout/query.rs | 2 | ||||
-rw-r--r-- | components/layout/wrapper.rs | 2 | ||||
-rw-r--r-- | components/script/layout_wrapper.rs | 30 | ||||
-rw-r--r-- | components/style/data.rs | 5 | ||||
-rw-r--r-- | components/style/dom.rs | 32 | ||||
-rw-r--r-- | components/style/gecko/wrapper.rs | 38 | ||||
-rw-r--r-- | components/style/matching.rs | 62 | ||||
-rw-r--r-- | components/style/traversal.rs | 21 | ||||
-rw-r--r-- | ports/geckolib/glue.rs | 7 |
9 files changed, 115 insertions, 84 deletions
diff --git a/components/layout/query.rs b/components/layout/query.rs index c92af15582f..822377ef86f 100644 --- a/components/layout/query.rs +++ b/components/layout/query.rs @@ -627,7 +627,7 @@ pub fn process_node_scroll_area_request< N: LayoutNode>(requested_node: N, layou fn ensure_node_data_initialized<N: LayoutNode>(node: &N) { let mut cur = Some(node.clone()); while let Some(current) = cur { - if current.borrow_data().is_some() { + if current.borrow_layout_data().is_some() { break; } diff --git a/components/layout/wrapper.rs b/components/layout/wrapper.rs index 28d1196221b..2c8b3a175c3 100644 --- a/components/layout/wrapper.rs +++ b/components/layout/wrapper.rs @@ -68,7 +68,7 @@ impl<T: LayoutNode> LayoutNodeLayoutData for T { } fn initialize_data(self) { - if self.borrow_data().is_none() { + if self.borrow_layout_data().is_none() { let ptr: NonOpaqueStyleAndLayoutData = Box::into_raw(box AtomicRefCell::new(PersistentLayoutData::new())); let opaque = OpaqueStyleAndLayoutData { diff --git a/components/script/layout_wrapper.rs b/components/script/layout_wrapper.rs index efb3ca698ed..f2947bedc93 100644 --- a/components/script/layout_wrapper.rs +++ b/components/script/layout_wrapper.rs @@ -60,7 +60,7 @@ use style::atomic_refcell::{AtomicRef, AtomicRefCell, AtomicRefMut}; use style::attr::AttrValue; use style::computed_values::display; use style::context::SharedStyleContext; -use style::data::PersistentStyleData; +use style::data::{PersistentStyleData, PseudoStyles}; use style::dom::{LayoutIterator, NodeInfo, OpaqueNode, PresentationalHintsSynthetizer, TDocument, TElement, TNode}; use style::dom::UnsafeNode; use style::element_state::*; @@ -107,6 +107,14 @@ impl<'ln> ServoLayoutNode<'ln> { } } + pub fn borrow_data(&self) -> Option<AtomicRef<PersistentStyleData>> { + self.get_style_data().map(|d| d.borrow()) + } + + pub fn mutate_data(&self) -> Option<AtomicRefMut<PersistentStyleData>> { + self.get_style_data().map(|d| d.borrow_mut()) + } + fn script_type_id(&self) -> NodeTypeId { unsafe { self.node.type_id_for_layout() @@ -234,12 +242,24 @@ impl<'ln> TNode for ServoLayoutNode<'ln> { old_value - 1 } - fn borrow_data(&self) -> Option<AtomicRef<PersistentStyleData>> { - self.get_style_data().map(|d| d.borrow()) + fn get_existing_style(&self) -> Option<Arc<ComputedValues>> { + self.borrow_data().and_then(|x| x.style.clone()) } - fn mutate_data(&self) -> Option<AtomicRefMut<PersistentStyleData>> { - self.get_style_data().map(|d| d.borrow_mut()) + fn set_style(&self, style: Option<Arc<ComputedValues>>) { + self.mutate_data().unwrap().style = style; + } + + fn take_pseudo_styles(&self) -> PseudoStyles { + use std::mem; + let mut tmp = PseudoStyles::default(); + mem::swap(&mut tmp, &mut self.mutate_data().unwrap().per_pseudo); + tmp + } + + fn set_pseudo_styles(&self, styles: PseudoStyles) { + debug_assert!(self.borrow_data().unwrap().per_pseudo.is_empty()); + self.mutate_data().unwrap().per_pseudo = styles; } fn restyle_damage(self) -> RestyleDamage { diff --git a/components/style/data.rs b/components/style/data.rs index 1fc24808d03..c0b07ac340f 100644 --- a/components/style/data.rs +++ b/components/style/data.rs @@ -10,13 +10,14 @@ use std::collections::HashMap; use std::hash::BuildHasherDefault; use std::sync::Arc; +pub type PseudoStyles = HashMap<PseudoElement, Arc<ComputedValues>, + BuildHasherDefault<::fnv::FnvHasher>>; pub struct PersistentStyleData { /// The results of CSS styling for this node. pub style: Option<Arc<ComputedValues>>, /// The results of CSS styling for each pseudo-element (if any). - pub per_pseudo: HashMap<PseudoElement, Arc<ComputedValues>, - BuildHasherDefault<::fnv::FnvHasher>>, + pub per_pseudo: PseudoStyles, } impl PersistentStyleData { diff --git a/components/style/dom.rs b/components/style/dom.rs index 4e72e5745b6..5b2bbcb135e 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -6,8 +6,7 @@ #![allow(unsafe_code)] -use atomic_refcell::{AtomicRef, AtomicRefMut}; -use data::PersistentStyleData; +use data::PseudoStyles; use element_state::ElementState; use parking_lot::RwLock; use properties::{ComputedValues, PropertyDeclarationBlock}; @@ -147,13 +146,25 @@ pub trait TNode : Sized + Copy + Clone + NodeInfo { /// traversal. Returns the number of children left to process. fn did_process_child(&self) -> isize; - /// Borrows the style data immutably. Fails on a conflicting borrow. - #[inline(always)] - fn borrow_data(&self) -> Option<AtomicRef<PersistentStyleData>>; + /// Returns the computed style values corresponding to the existing style + /// for this node, if any. + /// + /// This returns an cloned Arc (rather than a borrow) to abstract over the + /// multitude of ways these values may be stored under the hood. By + /// returning an enum with various OwningRef/OwningHandle entries, we could + /// avoid the refcounting traffic here, but it's probably not worth the + /// complexity. + fn get_existing_style(&self) -> Option<Arc<ComputedValues>>; + + /// Sets the computed style for this node. + fn set_style(&self, style: Option<Arc<ComputedValues>>); + + /// Transfers ownership of the existing pseudo styles, if any, to the + /// caller. The stored pseudo styles are replaced with an empty map. + fn take_pseudo_styles(&self) -> PseudoStyles; - /// Borrows the style data mutably. Fails on a conflicting borrow. - #[inline(always)] - fn mutate_data(&self) -> Option<AtomicRefMut<PersistentStyleData>>; + /// Sets the pseudo styles on the element, replacing any existing styles. + fn set_pseudo_styles(&self, styles: PseudoStyles); /// Get the description of how to account for recent style changes. fn restyle_damage(self) -> Self::ConcreteRestyleDamage; @@ -171,11 +182,6 @@ pub trait TNode : Sized + Copy + Clone + NodeInfo { fn next_sibling(&self) -> Option<Self>; - /// Removes the style from this node. - fn unstyle(self) { - self.mutate_data().unwrap().style = None; - } - /// XXX: It's a bit unfortunate we need to pass the current computed values /// as an argument here, but otherwise Servo would crash due to double /// borrows to return it. diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index 407ff4694e4..eb5251a23f4 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -6,7 +6,7 @@ use atomic_refcell::{AtomicRef, AtomicRefCell, AtomicRefMut}; -use data::PersistentStyleData; +use data::{PersistentStyleData, PseudoStyles}; use dom::{LayoutIterator, NodeInfo, TDocument, TElement, TNode, TRestyleDamage, UnsafeNode}; use dom::{OpaqueNode, PresentationalHintsSynthetizer}; use element_state::ElementState; @@ -149,6 +149,20 @@ impl<'ln> GeckoNode<'ln> { self.0.mServoData.set(ptr::null_mut()); } } + + pub fn get_pseudo_style(&self, pseudo: &PseudoElement) -> Option<Arc<ComputedValues>> { + self.borrow_data().and_then(|data| data.per_pseudo.get(pseudo).map(|c| c.clone())) + } + + #[inline(always)] + fn borrow_data(&self) -> Option<AtomicRef<PersistentStyleData>> { + self.get_node_data().as_ref().map(|d| d.0.borrow()) + } + + #[inline(always)] + fn mutate_data(&self) -> Option<AtomicRefMut<PersistentStyleData>> { + self.get_node_data().as_ref().map(|d| d.0.borrow_mut()) + } } #[derive(Clone, Copy, Debug, PartialEq)] @@ -319,14 +333,24 @@ impl<'ln> TNode for GeckoNode<'ln> { panic!("Atomic child count not implemented in Gecko"); } - #[inline(always)] - fn borrow_data(&self) -> Option<AtomicRef<PersistentStyleData>> { - self.get_node_data().as_ref().map(|d| d.0.borrow()) + fn get_existing_style(&self) -> Option<Arc<ComputedValues>> { + self.borrow_data().and_then(|x| x.style.clone()) } - #[inline(always)] - fn mutate_data(&self) -> Option<AtomicRefMut<PersistentStyleData>> { - self.get_node_data().as_ref().map(|d| d.0.borrow_mut()) + fn set_style(&self, style: Option<Arc<ComputedValues>>) { + self.mutate_data().unwrap().style = style; + } + + fn take_pseudo_styles(&self) -> PseudoStyles { + use std::mem; + let mut tmp = PseudoStyles::default(); + mem::swap(&mut tmp, &mut self.mutate_data().unwrap().per_pseudo); + tmp + } + + fn set_pseudo_styles(&self, styles: PseudoStyles) { + debug_assert!(self.borrow_data().unwrap().per_pseudo.is_empty()); + self.mutate_data().unwrap().per_pseudo = styles; } fn restyle_damage(self) -> Self::ConcreteRestyleDamage { diff --git a/components/style/matching.rs b/components/style/matching.rs index 18d0623c0e3..18e45a32ece 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -11,7 +11,6 @@ use arc_ptr_eq; use cache::{LRUCache, SimpleHashCache}; use cascade_info::CascadeInfo; use context::{SharedStyleContext, StyleContext}; -use data::PersistentStyleData; use dom::{NodeInfo, TElement, TNode, TRestyleDamage, UnsafeNode}; use properties::{ComputedValues, cascade}; use properties::longhands::display::computed_value as display; @@ -441,8 +440,7 @@ impl StyleSharingCandidateCache { } let node = element.as_node(); - let data = node.borrow_data().unwrap(); - let style = data.style.as_ref().unwrap(); + let style = node.get_existing_style().unwrap(); let box_style = style.get_box(); if box_style.transition_property_count() > 0 { @@ -723,14 +721,13 @@ pub trait ElementMatchMethods : TElement { Ok(shared_style) => { // Yay, cache hit. Share the style. let node = self.as_node(); - let style = &mut node.mutate_data().unwrap().style; // TODO: add the display: none optimisation here too! Even // better, factor it out/make it a bit more generic so Gecko // can decide more easily if it knows that it's a child of // replaced content, or similar stuff! let damage = - match node.existing_style_for_restyle_damage((*style).as_ref(), None) { + match node.existing_style_for_restyle_damage(node.get_existing_style().as_ref(), None) { Some(ref source) => { <<Self as TElement>::ConcreteNode as TNode> ::ConcreteRestyleDamage::compute(source, &shared_style) @@ -747,7 +744,7 @@ pub trait ElementMatchMethods : TElement { RestyleResult::Continue }; - *style = Some(shared_style); + node.set_style(Some(shared_style)); return StyleSharingResult::StyleWasShared(i, damage, restyle_result) } @@ -881,8 +878,7 @@ pub trait MatchMethods : TNode { where Ctx: StyleContext<'a> { // Get our parent's style. - let parent_node_data = parent.as_ref().and_then(|x| x.borrow_data()); - let parent_style = parent_node_data.as_ref().map(|x| x.style.as_ref().unwrap()); + let parent_style = parent.as_ref().map(|x| x.get_existing_style().unwrap()); // In the case we're styling a text node, we don't need to compute the // restyle damage, since it's a subset of the restyle damage of the @@ -893,11 +889,9 @@ pub trait MatchMethods : TNode { // In Servo, this is also true, since text nodes generate UnscannedText // fragments, which aren't repairable by incremental layout. if self.is_text_node() { - let mut data_ref = self.mutate_data().unwrap(); - let mut data = &mut *data_ref; - let cloned_parent_style = ComputedValues::style_for_child_text_node(parent_style.unwrap()); + let cloned_parent_style = ComputedValues::style_for_child_text_node(parent_style.as_ref().unwrap()); - data.style = Some(cloned_parent_style); + self.set_style(Some(cloned_parent_style)); return RestyleResult::Continue; } @@ -906,11 +900,8 @@ pub trait MatchMethods : TNode { context.local_context().applicable_declarations_cache.borrow_mut(); let (damage, restyle_result) = { - let mut data_ref = self.mutate_data().unwrap(); - let mut data = &mut *data_ref; - // Compute the parameters for the cascade. - let mut old_style = data.style.clone(); + let mut old_style = self.get_existing_style(); let cacheable = match old_style { None => true, Some(ref mut old) => { @@ -923,7 +914,9 @@ pub trait MatchMethods : TNode { let new_style = - self.cascade_node_pseudo_element(context, parent_style, old_style.as_ref(), + self.cascade_node_pseudo_element(context, + parent_style.as_ref(), + old_style.as_ref(), &applicable_declarations.normal, &mut applicable_declarations_cache, CascadeBooleans { @@ -933,11 +926,12 @@ pub trait MatchMethods : TNode { }); let (damage, restyle_result) = - self.compute_damage_and_cascade_pseudos(new_style, old_style.as_ref(), - data, context, - applicable_declarations, + self.compute_damage_and_cascade_pseudos(&new_style, old_style.as_ref(), + context, applicable_declarations, &mut applicable_declarations_cache); + self.set_style(Some(new_style)); + self.set_can_be_fragmented(parent.map_or(false, |p| { p.can_be_fragmented() || parent_style.as_ref().unwrap().is_multicol() @@ -946,18 +940,14 @@ pub trait MatchMethods : TNode { (damage, restyle_result) }; - - // This method needs to borrow the data as mutable, so make sure - // data_ref goes out of scope first. self.set_restyle_damage(damage); restyle_result } fn compute_damage_and_cascade_pseudos<'a, Ctx>(&self, - final_style: Arc<ComputedValues>, + new_style: &Arc<ComputedValues>, old_style: Option<&Arc<ComputedValues>>, - data: &mut PersistentStyleData, context: &Ctx, applicable_declarations: &ApplicableDeclarations, mut applicable_declarations_cache: &mut ApplicableDeclarationsCache) @@ -968,7 +958,7 @@ pub trait MatchMethods : TNode { // previous and the new styles having display: none. In this // case, we can always optimize the traversal, regardless of the // restyle hint. - let this_display = final_style.get_box().clone_display(); + let this_display = new_style.get_box().clone_display(); if this_display == display::T::none { let old_display = old_style.map(|old_style| { old_style.get_box().clone_display() @@ -986,26 +976,19 @@ pub trait MatchMethods : TNode { debug!("Short-circuiting traversal: {:?} {:?} {:?}", this_display, old_display, damage); - data.style = Some(final_style); return (damage, RestyleResult::Stop); } // Otherwise, we just compute the damage normally, and sum up the damage // related to pseudo-elements. let mut damage = - self.compute_restyle_damage(old_style, &final_style, None); - - data.style = Some(final_style); - - let data_per_pseudo = &mut data.per_pseudo; - let new_style = data.style.as_ref(); - - debug_assert!(new_style.is_some()); + self.compute_restyle_damage(old_style, new_style, None); let rebuild_and_reflow = Self::ConcreteRestyleDamage::rebuild_and_reflow(); let no_damage = Self::ConcreteRestyleDamage::empty(); + let mut pseudo_styles = self.take_pseudo_styles(); <Self::ConcreteElement as MatchAttr>::Impl::each_eagerly_cascaded_pseudo_element(|pseudo| { let applicable_declarations_for_this_pseudo = applicable_declarations.per_pseudo.get(&pseudo).unwrap(); @@ -1015,7 +998,7 @@ pub trait MatchMethods : TNode { // The old entry will be replaced. Remove it from the map but keep // it for analysis. - let mut old_pseudo_style = data_per_pseudo.remove(&pseudo); + let mut old_pseudo_style = pseudo_styles.remove(&pseudo); if has_declarations { // We have declarations, so we need to cascade. Compute parameters. @@ -1031,7 +1014,8 @@ pub trait MatchMethods : TNode { }; let new_pseudo_style = - self.cascade_node_pseudo_element(context, new_style, old_pseudo_style.as_ref(), + self.cascade_node_pseudo_element(context, Some(new_style), + old_pseudo_style.as_ref(), &*applicable_declarations_for_this_pseudo, &mut applicable_declarations_cache, CascadeBooleans { @@ -1050,7 +1034,7 @@ pub trait MatchMethods : TNode { } // Insert the new entry into the map. - let existing = data_per_pseudo.insert(pseudo, new_pseudo_style); + let existing = pseudo_styles.insert(pseudo, new_pseudo_style); debug_assert!(existing.is_none()); } else { damage = damage | match old_pseudo_style { @@ -1060,6 +1044,8 @@ pub trait MatchMethods : TNode { } }); + self.set_pseudo_styles(pseudo_styles); + (damage, RestyleResult::Continue) } } diff --git a/components/style/traversal.rs b/components/style/traversal.rs index 5f041919596..ce248dbb410 100644 --- a/components/style/traversal.rs +++ b/components/style/traversal.rs @@ -229,14 +229,7 @@ fn ensure_node_styled_internal<'a, N, C>(node: N, { use properties::longhands::display::computed_value as display; - // Ensure we have style data available. This must be done externally because - // there's no way to initialize the style data from the style system - // (because in Servo it's coupled with the layout data too). - // - // Ideally we'd have an initialize_data() or something similar but just for - // style data. - debug_assert!(node.borrow_data().is_some(), - "Need to initialize the data before calling ensure_node_styled"); + // NB: The node data must be initialized here. // We need to go to the root and ensure their style is up to date. // @@ -257,7 +250,7 @@ fn ensure_node_styled_internal<'a, N, C>(node: N, // // We only need to mark whether we have display none, and forget about it, // our style is up to date. - if let Some(ref style) = node.borrow_data().unwrap().style { + if let Some(ref style) = node.get_existing_style() { if !*parents_had_display_none { *parents_had_display_none = style.get_box().clone_display() == display::T::none; return; @@ -308,7 +301,7 @@ pub fn recalc_style_at<'a, N, C>(context: &'a C, // Remove existing CSS styles from nodes whose content has changed (e.g. text changed), // to force non-incremental reflow. if node.has_changed() { - node.unstyle(); + node.set_style(None); } // Check to see whether we can share a style with someone. @@ -385,11 +378,15 @@ pub fn recalc_style_at<'a, N, C>(context: &'a C, } } else { // Finish any expired transitions. - animation::complete_expired_transitions( + let mut existing_style = node.get_existing_style().unwrap(); + let had_animations_to_expire = animation::complete_expired_transitions( node.opaque(), - node.mutate_data().unwrap().style.as_mut().unwrap(), + &mut existing_style, context.shared_context() ); + if had_animations_to_expire { + node.set_style(Some(existing_style)); + } } let unsafe_layout_node = node.to_unsafe(); diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index f7237b59561..b4bebcf7c59 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -260,7 +260,7 @@ pub extern "C" fn Servo_StyleSheet_Release(sheet: RawServoStyleSheetBorrowed) -> pub extern "C" fn Servo_ComputedValues_Get(node: RawGeckoNodeBorrowed) -> ServoComputedValuesStrong { let node = GeckoNode(node); - let arc_cv = match node.borrow_data().map_or(None, |data| data.style.clone()) { + let arc_cv = match node.get_existing_style() { Some(style) => style, None => { // FIXME(bholley): This case subverts the intended semantics of this @@ -322,10 +322,7 @@ pub extern "C" fn Servo_ComputedValues_GetForPseudoElement(parent_style: ServoCo match GeckoSelectorImpl::pseudo_element_cascade_type(&pseudo) { PseudoElementCascadeType::Eager => { let node = element.as_node(); - let maybe_computed = node.borrow_data() - .and_then(|data| { - data.per_pseudo.get(&pseudo).map(|c| c.clone()) - }); + let maybe_computed = node.get_pseudo_style(&pseudo); maybe_computed.map_or_else(parent_or_null, FFIArcHelpers::into_strong) } PseudoElementCascadeType::Lazy => { |