Auto merge of #13656 - bholley:existing_style, r=emilio

Refactor style logic to avoid direct access to the node data during the cascade

The new restyle architecture doesn't store these things in consistent places, so we need a more abstract API.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13656)
<!-- Reviewable:end -->

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 {
@@ -486,6 +484,14 @@ pub enum StyleSharingResult<ConcreteRestyleDamage: TRestyleDamage> {
     StyleWasShared(usize, ConcreteRestyleDamage, RestyleResult),
 }
 
+// Callers need to pass several boolean flags to cascade_node_pseudo_element.
+// We encapsulate them in this struct to avoid mixing them up.
+struct CascadeBooleans {
+    shareable: bool,
+    cacheable: bool,
+    animate: bool,
+}
+
 trait PrivateMatchMethods: TNode {
     /// Actually cascades style for a node or a pseudo-element of a node.
     ///
@@ -494,27 +500,24 @@ trait PrivateMatchMethods: TNode {
     fn cascade_node_pseudo_element<'a, Ctx>(&self,
                                             context: &Ctx,
                                             parent_style: Option<&Arc<ComputedValues>>,
+                                            old_style: Option<&Arc<ComputedValues>>,
                                             applicable_declarations: &[ApplicableDeclarationBlock],
-                                            mut old_style: Option<&mut Arc<ComputedValues>>,
                                             applicable_declarations_cache:
                                              &mut ApplicableDeclarationsCache,
-                                            shareable: bool,
+                                            booleans: CascadeBooleans)
-                                            animate_properties: bool)
                                             -> Arc<ComputedValues>
         where Ctx: StyleContext<'a>
     {
+        let mut cacheable = booleans.cacheable;
+        let shared_context = context.shared_context();
+
         // Don’t cache applicable declarations for elements with a style attribute.
         // Since the style attribute contributes to that set, no other element would have the same set
         // and the cache would not be effective anyway.
         // This also works around the test failures at
         // https://github.com/servo/servo/pull/13459#issuecomment-250717584
         let has_style_attribute = self.as_element().map_or(false, |e| e.style_attribute().is_some());
-        let mut cacheable = !has_style_attribute;
+        cacheable = cacheable && !has_style_attribute;
-        let shared_context = context.shared_context();
-        if animate_properties {
-            cacheable = !self.update_animations_for_cascade(shared_context,
-                                                            &mut old_style) && cacheable;
-        }
 
         let mut cascade_info = CascadeInfo::new();
         let (this_style, is_cacheable) = match parent_style {
@@ -527,7 +530,7 @@ trait PrivateMatchMethods: TNode {
 
                 cascade(shared_context.viewport_size,
                         applicable_declarations,
-                        shareable,
+                        booleans.shareable,
                         Some(&***parent_style),
                         cached_computed_values,
                         Some(&mut cascade_info),
@@ -536,7 +539,7 @@ trait PrivateMatchMethods: TNode {
             None => {
                 cascade(shared_context.viewport_size,
                         applicable_declarations,
-                        shareable,
+                        booleans.shareable,
                         None,
                         None,
                         Some(&mut cascade_info),
@@ -549,7 +552,7 @@ trait PrivateMatchMethods: TNode {
 
         let mut this_style = Arc::new(this_style);
 
-        if animate_properties {
+        if booleans.animate {
             let new_animations_sender = &context.local_context().new_animations_sender;
             let this_opaque = self.opaque();
             // Trigger any present animations if necessary.
@@ -585,13 +588,7 @@ trait PrivateMatchMethods: TNode {
 
     fn update_animations_for_cascade(&self,
                                      context: &SharedStyleContext,
-                                     style: &mut Option<&mut Arc<ComputedValues>>)
+                                     style: &mut Arc<ComputedValues>) -> bool {
-                                     -> bool {
-        let style = match *style {
-            None => return false,
-            Some(ref mut style) => style,
-        };
-
         // Finish any expired transitions.
         let this_opaque = self.opaque();
         let had_animations_to_expire =
@@ -724,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)
@@ -748,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)
                 }
@@ -882,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
@@ -894,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;
         }
@@ -907,23 +900,38 @@ pub trait MatchMethods : TNode {
             context.local_context().applicable_declarations_cache.borrow_mut();
 
         let (damage, restyle_result) = {
-            let mut data_ref = self.mutate_data().unwrap();
+            // Compute the parameters for the cascade.
-            let mut data = &mut *data_ref;
+            let mut old_style = self.get_existing_style();
-            let final_style =
+            let cacheable = match old_style {
-                self.cascade_node_pseudo_element(context, parent_style,
+                None => true,
+                Some(ref mut old) => {
+                    // Update animations before the cascade. This may modify
+                    // the value of old_style.
+                    !self.update_animations_for_cascade(context.shared_context(), old)
+                },
+            };
+            let shareable = applicable_declarations.normal_shareable;
+
+
+            let new_style =
+                self.cascade_node_pseudo_element(context,
+                                                 parent_style.as_ref(),
+                                                 old_style.as_ref(),
                                                  &applicable_declarations.normal,
-                                                 data.style.as_mut(),
                                                  &mut applicable_declarations_cache,
-                                                 applicable_declarations.normal_shareable,
+                                                 CascadeBooleans {
-                                                 /* should_animate = */ true);
+                                                     shareable: shareable,
+                                                     cacheable: cacheable,
+                                                     animate: true,
+                                                 });
 
             let (damage, restyle_result) =
-                self.compute_damage_and_cascade_pseudos(final_style,
+                self.compute_damage_and_cascade_pseudos(&new_style, old_style.as_ref(),
-                                                        data,
+                                                        context, applicable_declarations,
-                                                        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()
@@ -932,17 +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>,
-                                                   data: &mut PersistentStyleData,
+                                                   old_style: Option<&Arc<ComputedValues>>,
                                                    context: &Ctx,
                                                    applicable_declarations: &ApplicableDeclarations,
                                                    mut applicable_declarations_cache: &mut ApplicableDeclarationsCache)
@@ -953,9 +958,9 @@ 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 = data.style.as_ref().map(|old_style| {
+            let old_display = old_style.map(|old_style| {
                 old_style.get_box().clone_display()
             });
 
@@ -971,94 +976,76 @@ 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(data.style.as_ref(), &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| {
-            use std::collections::hash_map::Entry;
-
             let applicable_declarations_for_this_pseudo =
                 applicable_declarations.per_pseudo.get(&pseudo).unwrap();
 
             let has_declarations =
                 !applicable_declarations_for_this_pseudo.is_empty();
 
-            // If there are declarations matching, we're going to need to
+            // The old entry will be replaced. Remove it from the map but keep
-            // recompute the style anyway, so do it now to simplify the logic
+            // it for analysis.
-            // below.
+            let mut old_pseudo_style = pseudo_styles.remove(&pseudo);
-            let pseudo_style_if_declarations = if has_declarations {
-                // NB: Transitions and animations should only work for
-                // pseudo-elements ::before and ::after
-                let should_animate_properties =
-                    <Self::ConcreteElement as MatchAttr>::Impl::pseudo_is_before_or_after(&pseudo);
 
-                Some(self.cascade_node_pseudo_element(context,
+            if has_declarations {
-                                                      new_style,
+                // We have declarations, so we need to cascade. Compute parameters.
-                                                      &*applicable_declarations_for_this_pseudo,
+                let animate = <Self::ConcreteElement as MatchAttr>::Impl
-                                                      data_per_pseudo.get_mut(&pseudo),
+                                ::pseudo_is_before_or_after(&pseudo);
-                                                      &mut applicable_declarations_cache,
+                let cacheable = if animate && old_pseudo_style.is_some() {
-                                                      /* shareable = */ false,
+                    // Update animations before the cascade. This may modify
-                                                      should_animate_properties))
+                    // the value of old_pseudo_style.
-            } else {
+                    !self.update_animations_for_cascade(context.shared_context(),
-                None
+                                                        old_pseudo_style.as_mut().unwrap())
-            };
+                } else {
+                    true
+                };
 
-            // Let's see what we had before.
+                let new_pseudo_style =
-            match data_per_pseudo.entry(pseudo.clone()) {
+                    self.cascade_node_pseudo_element(context, Some(new_style),
-                Entry::Vacant(vacant_entry) => {
+                                                     old_pseudo_style.as_ref(),
-                    // If we had a vacant entry, and no rules that match, we're
+                                                     &*applicable_declarations_for_this_pseudo,
-                    // fine so far.
+                                                     &mut applicable_declarations_cache,
-                    if !has_declarations {
+                                                     CascadeBooleans {
-                        return;
+                                                         shareable: false,
-                    }
+                                                         cacheable: cacheable,
+                                                         animate: animate,
+                                                     });
 
-                    // Otherwise, we need to insert the new computed styles, and
+                // Compute restyle damage unless we've already maxed it out.
-                    // generate a rebuild_and_reflow damage.
+                if damage != rebuild_and_reflow {
-                    damage = damage | Self::ConcreteRestyleDamage::rebuild_and_reflow();
+                    damage = damage | match old_pseudo_style {
-                    vacant_entry.insert(pseudo_style_if_declarations.unwrap());
+                        None => rebuild_and_reflow,
+                        Some(ref old) => self.compute_restyle_damage(Some(old), &new_pseudo_style,
+                                                                     Some(&pseudo)),
+                    };
                 }
-                Entry::Occupied(mut occupied_entry) => {
-                    // If there was an existing style, and no declarations, we
-                    // need to remove us from the map, and ensure we're
-                    // reconstructing.
-                    if !has_declarations {
-                        damage = damage | Self::ConcreteRestyleDamage::rebuild_and_reflow();
-                        occupied_entry.remove();
-                        return;
-                    }
 
-                    // If there's a new style, we need to diff it and add the
+                // Insert the new entry into the map.
-                    // damage, except if the damage was already
+                let existing = pseudo_styles.insert(pseudo, new_pseudo_style);
-                    // rebuild_and_reflow, in which case we can avoid it.
+                debug_assert!(existing.is_none());
-                    if damage != rebuild_and_reflow {
+            } else {
-                        damage = damage |
+                damage = damage | match old_pseudo_style {
-                                 self.compute_restyle_damage(Some(occupied_entry.get()),
+                    Some(_) => rebuild_and_reflow,
-                                                             pseudo_style_if_declarations.as_ref().unwrap(),
+                    None => no_damage,
-                                                             Some(&pseudo));
-                    }
-
-                    // And now, of course, use the new style.
-                    occupied_entry.insert(pseudo_style_if_declarations.unwrap());
                 }
             }
         });
 
+        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 => {