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.

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;
         }
 

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 {

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>> {
+    fn get_existing_style(&self) -> Option<Arc<ComputedValues>> {
-        self.get_style_data().map(|d| d.borrow())
+        self.borrow_data().and_then(|x| x.style.clone())
     }
 
-    fn mutate_data(&self) -> Option<AtomicRefMut<PersistentStyleData>> {
+    fn set_style(&self, style: Option<Arc<ComputedValues>>) {
-        self.get_style_data().map(|d| d.borrow_mut())
+        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 {

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>,
+    pub per_pseudo: PseudoStyles,
-                            BuildHasherDefault<::fnv::FnvHasher>>,
 }
 
 impl PersistentStyleData {

components/style/dom.rs

@@ -6,8 +6,7 @@
 
 #![allow(unsafe_code)]
 
-use atomic_refcell::{AtomicRef, AtomicRefMut};
+use data::PseudoStyles;
-use data::PersistentStyleData;
 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.
+    /// Returns the computed style values corresponding to the existing style
-    #[inline(always)]
+    /// for this node, if any.
-    fn borrow_data(&self) -> Option<AtomicRef<PersistentStyleData>>;
+    ///
+    /// 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>>;
 
-    /// Borrows the style data mutably. Fails on a conflicting borrow.
+    /// Sets the computed style for this node.
-    #[inline(always)]
+    fn set_style(&self, style: Option<Arc<ComputedValues>>);
-    fn mutate_data(&self) -> Option<AtomicRefMut<PersistentStyleData>>;
+
+    /// 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;
+
+    /// 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.

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 get_existing_style(&self) -> Option<Arc<ComputedValues>> {
-    fn borrow_data(&self) -> Option<AtomicRef<PersistentStyleData>> {
+        self.borrow_data().and_then(|x| x.style.clone())
-        self.get_node_data().as_ref().map(|d| d.0.borrow())
     }
 
-    #[inline(always)]
+    fn set_style(&self, style: Option<Arc<ComputedValues>>) {
-    fn mutate_data(&self) -> Option<AtomicRefMut<PersistentStyleData>> {
+        self.mutate_data().unwrap().style = style;
-        self.get_node_data().as_ref().map(|d| d.0.borrow_mut())
+    }
+
+    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 {

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 = node.get_existing_style().unwrap();
-        let style = data.style.as_ref().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.as_ref().map(|x| x.get_existing_style().unwrap());
-        let parent_style = parent_node_data.as_ref().map(|x| x.style.as_ref().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 cloned_parent_style = ComputedValues::style_for_child_text_node(parent_style.as_ref().unwrap());
-            let mut data = &mut *data_ref;
-            let cloned_parent_style = ComputedValues::style_for_child_text_node(parent_style.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(),
+                self.compute_damage_and_cascade_pseudos(&new_style, old_style.as_ref(),
-                                                        data, context,
+                                                        context, applicable_declarations,
-                                                        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);
+            self.compute_restyle_damage(old_style, new_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());
 
         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)
     }
 }

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
+    // NB: The node data must be initialized here.
-    // 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");
 
     // 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();

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()
+            let maybe_computed = node.get_pseudo_style(&pseudo);
-                                     .and_then(|data| {
-                                         data.per_pseudo.get(&pseudo).map(|c| c.clone())
-                                     });
             maybe_computed.map_or_else(parent_or_null, FFIArcHelpers::into_strong)
         }
         PseudoElementCascadeType::Lazy => {