Bug 1325734 - Simplify ElementData and eliminate the concept of consuming styles. r=emilio

12 files changed, 195 insertions, 353 deletions

diff --git a/components/layout/traversal.rs b/components/layout/traversal.rs
index d5f85f767b9..0f7b1a3d89d 100644
--- a/components/layout/traversal.rs
+++ b/components/layout/traversal.rs
@@ -140,7 +140,6 @@ fn construct_flows_at<'a, N>(context: &LayoutContext<'a>,
     }
 
     if let Some(el) = node.as_element() {
-        el.mutate_data().unwrap().persist();
         unsafe { el.unset_dirty_descendants(); }
     }
 }
diff --git a/components/layout/wrapper.rs b/components/layout/wrapper.rs
index 817ff96bdb4..eb110a90e7a 100644
--- a/components/layout/wrapper.rs
+++ b/components/layout/wrapper.rs
@@ -172,7 +172,7 @@ impl<T: ThreadSafeLayoutNode> ThreadSafeLayoutNodeHelpers for T {
         }
 
         let data = node.borrow_layout_data().unwrap();
-        if let Some(r) = data.base.style_data.as_restyle() {
+        if let Some(r) = data.base.style_data.get_restyle() {
             // We're reflowing a node that just got a restyle, and so the
             // damage has been computed and stored in the RestyleData.
             r.damage
diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs
index 25192893290..133fcbf9093 100644
--- a/components/layout_thread/lib.rs
+++ b/components/layout_thread/lib.rs
@@ -1086,8 +1086,11 @@ impl LayoutThread {
                 while let Some(node) = next {
                     if node.needs_dirty_on_viewport_size_changed() {
                         let el = node.as_element().unwrap();
-                        el.mutate_data().map(|mut d| d.restyle()
-                                        .map(|mut r| r.hint.insert(&StoredRestyleHint::subtree())));
+                        if let Some(mut d) = element.mutate_data() {
+                            if d.has_styles() {
+                                d.ensure_restyle().hint.insert(&StoredRestyleHint::subtree());
+                            }
+                        }
                         if let Some(p) = el.parent_element() {
                             unsafe { p.note_dirty_descendant() };
                         }
@@ -1106,7 +1109,11 @@ impl LayoutThread {
                                                                              data.stylesheets_changed);
         let needs_reflow = viewport_size_changed && !needs_dirtying;
         if needs_dirtying {
-            element.mutate_data().map(|mut d| d.restyle().map(|mut r| r.hint.insert(&StoredRestyleHint::subtree())));
+            if let Some(mut d) = element.mutate_data() {
+                if d.has_styles() {
+                    d.ensure_restyle().hint.insert(&StoredRestyleHint::subtree());
+                }
+            }
         }
         if needs_reflow {
             if let Some(mut flow) = self.try_get_layout_root(element.as_node()) {
@@ -1132,10 +1139,7 @@ impl LayoutThread {
                 };
                 let mut style_data = &mut data.base.style_data;
                 debug_assert!(style_data.has_current_styles());
-                let mut restyle_data = match style_data.restyle() {
-                    Some(d) => d,
-                    None => continue,
-                };
+                let mut restyle_data = style_data.ensure_restyle();
 
                 // Stash the data on the element for processing by the style system.
                 restyle_data.hint = restyle.hint.into();
diff --git a/components/style/build_gecko.rs b/components/style/build_gecko.rs
index 161a0e86f1f..b696f0f123a 100644
--- a/components/style/build_gecko.rs
+++ b/components/style/build_gecko.rs
@@ -252,7 +252,6 @@ mod bindings {
             "RawGecko.*",
             "mozilla::ServoStyleSheet",
             "mozilla::ServoElementSnapshot.*",
-            "mozilla::ConsumeStyleBehavior",
             "mozilla::CSSPseudoClassType",
             "mozilla::css::SheetParsingMode",
             "mozilla::HalfCorner",
@@ -265,7 +264,6 @@ mod bindings {
             "AnonymousContent",
             "AudioContext",
             "CapturingContentInfo",
-            "ConsumeStyleBehavior",
             "DefaultDelete",
             "DOMIntersectionObserverEntry",
             "Element",
@@ -473,7 +471,6 @@ mod bindings {
             "RawGeckoPresContext",
             "ThreadSafeURIHolder",
             "ThreadSafePrincipalHolder",
-            "ConsumeStyleBehavior",
             "CSSPseudoClassType",
             "TraversalRootBehavior",
             "FontFamilyList",
diff --git a/components/style/context.rs b/components/style/context.rs
index 66310c752c7..7c9f0f0ab89 100644
--- a/components/style/context.rs
+++ b/components/style/context.rs
@@ -135,7 +135,7 @@ impl<E: TElement> ThreadLocalStyleContext<E> {
         debug_assert!(self.current_element_info.is_none());
         self.current_element_info = Some(CurrentElementInfo {
             element: element.as_node().opaque(),
-            is_initial_style: data.is_unstyled_initial(),
+            is_initial_style: !data.has_styles(),
         });
     }
 
diff --git a/components/style/data.rs b/components/style/data.rs
index ddd85ef9ad3..475a04b7023 100644
--- a/components/style/data.rs
+++ b/components/style/data.rs
@@ -15,7 +15,6 @@ use selector_parser::{PseudoElement, RestyleDamage, Snapshot};
 use std::collections::HashMap;
 use std::fmt;
 use std::hash::BuildHasherDefault;
-use std::mem;
 use std::ops::{Deref, DerefMut};
 use std::sync::Arc;
 use stylist::Stylist;
@@ -262,31 +261,37 @@ impl Deref for SnapshotOption {
 /// Transient data used by the restyle algorithm. This structure is instantiated
 /// either before or during restyle traversal, and is cleared at the end of node
 /// processing.
-///
-/// TODO(emilio): Tell bholley to document this more accurately. I can try (and
-/// the fields are certainly mostly self-explanatory), but it's better if he
-/// does, to avoid any misconception.
 #[derive(Debug)]
-#[allow(missing_docs)]
 pub struct RestyleData {
-    pub styles: ElementStyles,
+    /// The restyle hint, which indicates whether selectors need to be rematched
+    /// for this element, its children, and its descendants.
     pub hint: StoredRestyleHint,
+
+    /// Whether we need to recascade.
+    /// FIXME(bholley): This should eventually become more fine-grained.
     pub recascade: bool,
+
+    /// The restyle damage, indicating what kind of layout changes are required
+    /// afte restyling.
     pub damage: RestyleDamage,
+
+    /// An optional snapshot of the original state and attributes of the element,
+    /// from which we may compute additional restyle hints at traversal time.
     pub snapshot: SnapshotOption,
 }
 
-impl RestyleData {
-    fn new(styles: ElementStyles) -> Self {
+impl Default for RestyleData {
+    fn default() -> Self {
         RestyleData {
-            styles: styles,
             hint: StoredRestyleHint::default(),
             recascade: false,
             damage: RestyleDamage::empty(),
             snapshot: SnapshotOption::empty(),
         }
     }
+}
 
+impl RestyleData {
     /// Expands the snapshot (if any) into a restyle hint. Returns true if later
     /// siblings must be restyled.
     pub fn expand_snapshot<E: TElement>(&mut self, element: E, stylist: &Stylist) -> bool {
@@ -312,263 +317,116 @@ impl RestyleData {
         later_siblings
     }
 
-    /// Return if the element style's are up to date.
-    pub fn has_current_styles(&self) -> bool {
-        !(self.hint.restyle_self || self.recascade || self.snapshot.is_some())
-    }
-
-    /// Returns the element styles.
-    pub fn styles(&self) -> &ElementStyles {
-        &self.styles
-    }
-
-    /// Returns a mutable reference to the element styles.
-    pub fn styles_mut(&mut self) -> &mut ElementStyles {
-        &mut self.styles
-    }
-
-    fn finish_styling(&mut self, styles: ElementStyles, damage: RestyleDamage) {
-        debug_assert!(!self.has_current_styles());
-        debug_assert!(self.snapshot.is_none(), "Traversal should have expanded snapshots");
-        self.styles = styles;
-        self.damage |= damage;
-        // The hint and recascade bits get cleared by the traversal code. This
-        // is a bit confusing, and we should simplify it when we separate matching
-        // from cascading.
+    /// Returns true if this RestyleData might invalidate the current style.
+    pub fn has_invalidations(&self) -> bool {
+        self.hint.restyle_self || self.recascade || self.snapshot.is_some()
     }
 }
 
-/// Style system data associated with a node.
-///
-/// In Gecko, this hangs directly off a node, but is dropped when the frame takes
-/// ownership of the computed style data.
-///
-/// In Servo, this is embedded inside of layout data, which itself hangs directly
-/// off the node. Servo does not currently implement ownership transfer of the
-/// computed style data to the frame.
+/// Style system data associated with an Element.
 ///
-/// In both cases, it is wrapped inside an AtomicRefCell to ensure thread
-/// safety.
+/// In Gecko, this hangs directly off the Element. Servo, this is embedded
+/// inside of layout data, which itself hangs directly off the Element. In
+/// both cases, it is wrapped inside an AtomicRefCell to ensure thread safety.
 #[derive(Debug)]
-pub enum ElementData {
-    /// This is the first styling for this element.
-    Initial(Option<ElementStyles>),
-    /// This element has been restyled already, and all the relevant data is
-    /// inside the `RestyleData`.
-    Restyle(RestyleData),
-    /// This element has already been restyled, and only keeps its styles
-    /// around.
-    Persistent(ElementStyles),
+pub struct ElementData {
+    /// The computed styles for the element and its pseudo-elements.
+    styles: Option<ElementStyles>,
+
+    /// Restyle tracking. We separate this into a separate allocation so that
+    /// we can drop it when no restyles are pending on the elemnt.
+    restyle: Option<Box<RestyleData>>,
 }
 
 impl ElementData {
     /// Trivially construct an ElementData.
     pub fn new(existing: Option<ElementStyles>) -> Self {
-        if let Some(s) = existing {
-            ElementData::Persistent(s)
-        } else {
-            ElementData::Initial(None)
-        }
-    }
-
-    /// Return whether this data is from an initial restyle.
-    pub fn is_initial(&self) -> bool {
-        match *self {
-            ElementData::Initial(_) => true,
-            _ => false,
+        ElementData {
+            styles: existing,
+            restyle: None,
         }
     }
 
-    /// Return whether this data is from an element that hasn't been restyled.
-    pub fn is_unstyled_initial(&self) -> bool {
-        match *self {
-            ElementData::Initial(None) => true,
-            _ => false,
-        }
+    /// Returns true if this element has a computed styled.
+    pub fn has_styles(&self) -> bool {
+        self.styles.is_some()
     }
 
-    /// Return whether this data is from an element whose first restyle has just
-    /// been done.
-    pub fn is_styled_initial(&self) -> bool {
-        match *self {
-            ElementData::Initial(Some(_)) => true,
-            _ => false,
-        }
+    /// Returns true if this element's style is up-to-date and has no potential
+    /// invalidation.
+    pub fn has_current_styles(&self) -> bool {
+        self.has_styles() &&
+        self.restyle.as_ref().map_or(true, |r| !r.has_invalidations())
     }
 
-    /// Returns true if this element is being restyled and has been styled
-    /// before.
-    pub fn is_restyle(&self) -> bool {
-        match *self {
-            ElementData::Restyle(_) => true,
-            _ => false,
-        }
+    /// Gets the element styles, if any.
+    pub fn get_styles(&self) -> Option<&ElementStyles> {
+        self.styles.as_ref()
     }
 
-    /// Returns the `RestyleData` if it exists.
-    pub fn as_restyle(&self) -> Option<&RestyleData> {
-        match *self {
-            ElementData::Restyle(ref x) => Some(x),
-            _ => None,
-        }
+    /// Gets the element styles. Panic if the element has never been styled.
+    pub fn styles(&self) -> &ElementStyles {
+        self.styles.as_ref().expect("Calling styles() on unstyled ElementData")
     }
 
-    /// Returns a mutable reference to the RestyleData, if it exists.
-    pub fn as_restyle_mut(&mut self) -> Option<&mut RestyleData> {
-        match *self {
-            ElementData::Restyle(ref mut x) => Some(x),
-            _ => None,
-        }
+    /// Gets a mutable reference to the element styles, if any.
+    pub fn get_styles_mut(&mut self) -> Option<&mut ElementStyles> {
+        self.styles.as_mut()
     }
 
-    /// Returns whether this element's style is persistent.
-    pub fn is_persistent(&self) -> bool {
-        match *self {
-            ElementData::Persistent(_) => true,
-            _ => false,
-        }
+    /// Gets a mutable reference to the element styles. Panic if the element has
+    /// never been styled.
+    pub fn styles_mut(&mut self) -> &mut ElementStyles {
+        self.styles.as_mut().expect("Caling styles_mut() on unstyled ElementData")
     }
 
-    /// Sets an element up for restyle, returning None for an unstyled element.
-    pub fn restyle(&mut self) -> Option<&mut RestyleData> {
-        if self.is_unstyled_initial() {
-            return None;
-        }
-
-        // If the caller never consumed the initial style, make sure that the
-        // change hint represents the delta from zero, rather than a delta from
-        // a previous style that was never observed. Ideally this shouldn't
-        // happen, but we handle it for robustness' sake.
-        let damage_override = if self.is_styled_initial() {
-            RestyleDamage::rebuild_and_reflow()
-        } else {
-            RestyleDamage::empty()
-        };
-
-        if !self.is_restyle() {
-            // Play some tricks to reshape the enum without cloning ElementStyles.
-            let old = mem::replace(self, ElementData::new(None));
-            let styles = match old {
-                ElementData::Initial(Some(s)) => s,
-                ElementData::Persistent(s) => s,
-                _ => unreachable!()
-            };
-            *self = ElementData::Restyle(RestyleData::new(styles));
-        }
-
-        let restyle = self.as_restyle_mut().unwrap();
-        restyle.damage |= damage_override;
-        Some(restyle)
+    /// Sets the computed element styles.
+    pub fn set_styles(&mut self, styles: ElementStyles) {
+        debug_assert!(self.get_restyle().map_or(true, |r| r.snapshot.is_none()),
+                      "Traversal should have expanded snapshots");
+        self.styles = Some(styles);
     }
 
-    /// Converts Initial and Restyle to Persistent. No-op for Persistent.
-    pub fn persist(&mut self) {
-        if self.is_persistent() {
-            return;
-        }
-
-        // Play some tricks to reshape the enum without cloning ElementStyles.
-        let old = mem::replace(self, ElementData::new(None));
-        let styles = match old {
-            ElementData::Initial(i) => i.unwrap(),
-            ElementData::Restyle(r) => r.styles,
-            ElementData::Persistent(_) => unreachable!(),
-        };
-        *self = ElementData::Persistent(styles);
-    }
-
-    /// Return the restyle damage (if any).
-    pub fn damage(&self) -> RestyleDamage {
-        use self::ElementData::*;
-        match *self {
-            Initial(ref s) => {
-                debug_assert!(s.is_some());
-                RestyleDamage::rebuild_and_reflow()
-            },
-            Restyle(ref r) => {
-                debug_assert!(r.has_current_styles());
-                r.damage
-            },
-            Persistent(_) => RestyleDamage::empty(),
-        }
+    /// Returns true if the Element has a RestyleData.
+    pub fn has_restyle(&self) -> bool {
+        self.restyle.is_some()
     }
 
-    /// A version of the above, with the assertions replaced with warnings to
-    /// be more robust in corner-cases. This will go away soon.
-    #[cfg(feature = "gecko")]
-    pub fn damage_sloppy(&self) -> RestyleDamage {
-        use self::ElementData::*;
-        match *self {
-            Initial(ref s) => {
-                if s.is_none() {
-                    error!("Accessing damage on unstyled element");
-                }
-                RestyleDamage::rebuild_and_reflow()
-            },
-            Restyle(ref r) => {
-                if !r.has_current_styles() {
-                    error!("Accessing damage on dirty element");
-                }
-                r.damage
-            },
-            Persistent(_) => RestyleDamage::empty(),
-        }
+    /// Drops any RestyleData.
+    pub fn clear_restyle(&mut self) {
+        self.restyle = None;
     }
 
-    /// Returns true if this element's style is up-to-date and has no potential
-    /// invalidation.
-    pub fn has_current_styles(&self) -> bool {
-        use self::ElementData::*;
-        match *self {
-            Initial(ref x) => x.is_some(),
-            Restyle(ref x) => x.has_current_styles(),
-            Persistent(_) => true,
+    /// Creates a RestyleData if one doesn't exist.
+    ///
+    /// Asserts that the Element has been styled.
+    pub fn ensure_restyle(&mut self) -> &mut RestyleData {
+        debug_assert!(self.styles.is_some(), "restyling unstyled element");
+        if self.restyle.is_none() {
+            self.restyle = Some(Box::new(RestyleData::default()));
         }
+        self.restyle.as_mut().unwrap()
     }
 
-    /// Get the element styles, if any.
-    pub fn get_styles(&self) -> Option<&ElementStyles> {
-        use self::ElementData::*;
-        match *self {
-            Initial(ref x) => x.as_ref(),
-            Restyle(ref x) => Some(x.styles()),
-            Persistent(ref x) => Some(x),
-        }
+    /// Gets a reference to the restyle data, if any.
+    pub fn get_restyle(&self) -> Option<&RestyleData> {
+        self.restyle.as_ref().map(|r| &**r)
     }
 
-    /// Get the element styles. Panic if the element has never been styled.
-    pub fn styles(&self) -> &ElementStyles {
-        self.get_styles().expect("Calling styles() on unstyled ElementData")
+    /// Gets a reference to the restyle data. Panic if the element does not
+    /// have restyle data.
+    pub fn restyle(&self) -> &RestyleData {
+        self.get_restyle().expect("Calling restyle without RestyleData")
     }
 
-    /// Get a mutable reference to the element styles, if any.
-    pub fn get_styles_mut(&mut self) -> Option<&mut ElementStyles> {
-        use self::ElementData::*;
-        match *self {
-            Initial(ref mut x) => x.as_mut(),
-            Restyle(ref mut x) => Some(x.styles_mut()),
-            Persistent(ref mut x) => Some(x),
-        }
+    /// Gets a mutable reference to the restyle data, if any.
+    pub fn get_restyle_mut(&mut self) -> Option<&mut RestyleData> {
+        self.restyle.as_mut().map(|r| &mut **r)
     }
 
-    /// Get a mutable reference to the element styles. Panic if the element has
-    /// never been styled.
-    pub fn styles_mut(&mut self) -> &mut ElementStyles {
-        self.get_styles_mut().expect("Calling styles_mut() on unstyled ElementData")
-    }
-
-    /// Finishes the styling of the element, effectively setting the style in
-    /// the data.
-    pub fn finish_styling(&mut self, styles: ElementStyles, damage: RestyleDamage) {
-        use self::ElementData::*;
-        match *self {
-            Initial(ref mut x) => {
-                debug_assert!(x.is_none());
-                debug_assert!(damage == RestyleDamage::rebuild_and_reflow());
-                *x = Some(styles);
-            },
-            Restyle(ref mut x) => x.finish_styling(styles, damage),
-            Persistent(_) => panic!("Calling finish_styling on Persistent ElementData"),
-        };
+    /// Gets a mutable reference to the restyle data. Panic if the element does
+    /// not have restyle data.
+    pub fn restyle_mut(&mut self) -> &mut RestyleData {
+        self.get_restyle_mut().expect("Calling restyle_mut without RestyleData")
     }
 }
diff --git a/components/style/gecko_bindings/bindings.rs b/components/style/gecko_bindings/bindings.rs
index 1546e6a6950..ee1f7de87ae 100644
--- a/components/style/gecko_bindings/bindings.rs
+++ b/components/style/gecko_bindings/bindings.rs
@@ -9,7 +9,6 @@ use gecko_bindings::structs::RawGeckoNode;
 use gecko_bindings::structs::RawGeckoPresContext;
 use gecko_bindings::structs::ThreadSafeURIHolder;
 use gecko_bindings::structs::ThreadSafePrincipalHolder;
-use gecko_bindings::structs::ConsumeStyleBehavior;
 use gecko_bindings::structs::CSSPseudoClassType;
 use gecko_bindings::structs::TraversalRootBehavior;
 use gecko_bindings::structs::FontFamilyList;
@@ -1323,13 +1322,12 @@ extern "C" {
                                    change_hint: nsChangeHint);
 }
 extern "C" {
-    pub fn Servo_CheckChangeHint(element: RawGeckoElementBorrowed)
+    pub fn Servo_TakeChangeHint(element: RawGeckoElementBorrowed)
      -> nsChangeHint;
 }
 extern "C" {
     pub fn Servo_ResolveStyle(element: RawGeckoElementBorrowed,
-                              set: RawServoStyleSetBorrowed,
-                              consume: ConsumeStyleBehavior)
+                              set: RawServoStyleSetBorrowed)
      -> ServoComputedValuesStrong;
 }
 extern "C" {
@@ -1341,7 +1339,6 @@ extern "C" {
 extern "C" {
     pub fn Servo_ResolveStyleLazily(element: RawGeckoElementBorrowed,
                                     pseudo_tag: *mut nsIAtom,
-                                    consume: ConsumeStyleBehavior,
                                     set: RawServoStyleSetBorrowed)
      -> ServoComputedValuesStrong;
 }
diff --git a/components/style/gecko_bindings/structs_debug.rs b/components/style/gecko_bindings/structs_debug.rs
index 8560f3b6ccb..1fee5044fb4 100644
--- a/components/style/gecko_bindings/structs_debug.rs
+++ b/components/style/gecko_bindings/structs_debug.rs
@@ -2464,9 +2464,6 @@ pub mod root {
         }
         #[repr(i32)]
         #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
-        pub enum ConsumeStyleBehavior { Consume = 0, DontConsume = 1, }
-        #[repr(i32)]
-        #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
         pub enum TraversalRootBehavior {
             Normal = 0,
             UnstyledChildrenOnly = 1,
diff --git a/components/style/gecko_bindings/structs_release.rs b/components/style/gecko_bindings/structs_release.rs
index 45058c648c6..928a4b6deeb 100644
--- a/components/style/gecko_bindings/structs_release.rs
+++ b/components/style/gecko_bindings/structs_release.rs
@@ -2447,9 +2447,6 @@ pub mod root {
         }
         #[repr(i32)]
         #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
-        pub enum ConsumeStyleBehavior { Consume = 0, DontConsume = 1, }
-        #[repr(i32)]
-        #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
         pub enum TraversalRootBehavior {
             Normal = 0,
             UnstyledChildrenOnly = 1,
diff --git a/components/style/matching.rs b/components/style/matching.rs
index 93dedbd21f7..857926b5c93 100644
--- a/components/style/matching.rs
+++ b/components/style/matching.rs
@@ -660,16 +660,22 @@ pub trait MatchMethods : TElement {
                     // 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 = {
-                        debug_assert!(!data.has_current_styles());
-                        let previous_values = data.get_styles().map(|x| &x.primary.values);
-                        match self.existing_style_for_restyle_damage(previous_values, None) {
-                            Some(ref source) => RestyleDamage::compute(source, &shared_style.values),
-                            None => RestyleDamage::rebuild_and_reflow(),
-                        }
+                    let maybe_damage = {
+                        let previous = data.get_styles().map(|x| &x.primary.values);
+                        let existing = self.existing_style_for_restyle_damage(previous, None);
+                        existing.map(|e| RestyleDamage::compute(e, &shared_style.values))
                     };
+                    if let Some(d) = maybe_damage {
+                        data.restyle_mut().damage |= d;
+                    }
+
+                    // We never put elements with pseudo style into the style sharing cache,
+                    // so we can just mint an ElementStyles directly here.
+                    //
+                    // See https://bugzilla.mozilla.org/show_bug.cgi?id=1329361
+                    let styles = ElementStyles::new(shared_style);
+                    data.set_styles(styles);
 
-                    data.finish_styling(ElementStyles::new(shared_style), damage);
                     return StyleSharingResult::StyleWasShared(i)
                 }
                 Err(miss) => {
@@ -857,7 +863,10 @@ pub trait MatchMethods : TElement {
             damage
         };
 
-        data.finish_styling(new_styles, damage);
+        if data.has_styles() {
+            data.restyle_mut().damage |= damage;
+        }
+        data.set_styles(new_styles);
     }
 
     /// Given the old and new styling results, compute the final restyle damage.
diff --git a/components/style/traversal.rs b/components/style/traversal.rs
index 8e7bf465682..af943c50336 100644
--- a/components/style/traversal.rs
+++ b/components/style/traversal.rs
@@ -119,7 +119,7 @@ pub trait DomTraversal<E: TElement> : Sync {
         // we will drop on the floor. To prevent missed restyles, we assert against
         // restyling a root with later siblings.
         if let Some(mut data) = root.mutate_data() {
-            if let Some(r) = data.as_restyle_mut() {
+            if let Some(r) = data.get_restyle_mut() {
                 debug_assert!(root.next_sibling_element().is_none());
                 let _later_siblings = r.expand_snapshot(root, stylist);
             }
@@ -162,25 +162,31 @@ pub trait DomTraversal<E: TElement> : Sync {
                     None => return true,
                 };
 
-                // Check what kind of element data we have. If it's Initial or Persistent,
-                // we're done.
-                let restyle = match *data {
-                    ElementData::Initial(ref i) => return i.is_none(),
-                    ElementData::Persistent(_) => return false,
-                    ElementData::Restyle(ref r) => r,
-                };
-
-                // Check whether we have any selector matching or re-cascading to
-                // do in this subtree.
-                debug_assert!(restyle.snapshot.is_none(), "Snapshots should already be expanded");
-                if !restyle.hint.is_empty() || restyle.recascade {
+                // If we don't have any style data, we need to visit the element.
+                if !data.has_styles() {
                     return true;
                 }
 
+                // Check the restyle data.
+                if let Some(r) = data.get_restyle() {
+                    // If we have a restyle hint or need to recascade, we need to
+                    // visit the element.
+                    //
+                    // Note that this is different than checking has_current_styles(),
+                    // since that can return true even if we have a restyle hint
+                    // indicating that the element's descendants (but not necessarily
+                    // the element) need restyling.
+                    if !r.hint.is_empty() || r.recascade {
+                        return true;
+                    }
+                }
+
                 // Servo uses the post-order traversal for flow construction, so
                 // we need to traverse any element with damage so that we can perform
                 // fixup / reconstruction on our way back up the tree.
-                if cfg!(feature = "servo") && restyle.damage != RestyleDamage::empty() {
+                if cfg!(feature = "servo") &&
+                   data.get_restyle().map_or(false, |r| r.damage != RestyleDamage::empty())
+                {
                     return true;
                 }
 
@@ -258,7 +264,7 @@ pub trait DomTraversal<E: TElement> : Sync {
             if Self::node_needs_traversal(kid) {
                 let el = kid.as_element();
                 if el.as_ref().and_then(|el| el.borrow_data())
-                              .map_or(false, |d| d.is_restyle())
+                              .map_or(false, |d| d.has_styles())
                 {
                     unsafe { parent.set_dirty_descendants(); }
                 }
@@ -380,7 +386,7 @@ pub fn recalc_style_at<E, D>(traversal: &D,
           D: DomTraversal<E>
 {
     context.thread_local.begin_element(element, &data);
-    debug_assert!(data.as_restyle().map_or(true, |r| r.snapshot.is_none()),
+    debug_assert!(data.get_restyle().map_or(true, |r| r.snapshot.is_none()),
                   "Snapshots should be expanded by the caller");
 
     let compute_self = !data.has_current_styles();
@@ -397,7 +403,7 @@ pub fn recalc_style_at<E, D>(traversal: &D,
     // Now that matching and cascading is done, clear the bits corresponding to
     // those operations and compute the propagated restyle hint.
     let empty_hint = StoredRestyleHint::empty();
-    let propagated_hint = match data.as_restyle_mut() {
+    let propagated_hint = match data.get_restyle_mut() {
         None => empty_hint,
         Some(r) => {
             r.recascade = false;
@@ -523,14 +529,21 @@ fn preprocess_children<E, D>(traversal: &D,
         };
 
         let mut child_data = unsafe { D::ensure_element_data(&child).borrow_mut() };
-        if child_data.is_unstyled_initial() {
+
+        // If the child is unstyled, we don't need to set up any restyling.
+        if !child_data.has_styles() {
             continue;
         }
 
-        let mut restyle_data = match child_data.restyle() {
-            Some(d) => d,
-            None => continue,
-        };
+        // If the child doesn't have pre-existing RestyleData and we don't have
+        // any reason to create one, avoid the useless allocation and move on to
+        // the next child.
+        if propagated_hint.is_empty() && !parent_inherited_style_changed &&
+           !child_data.has_restyle()
+        {
+            continue;
+        }
+        let mut restyle_data = child_data.ensure_restyle();
 
         // Propagate the parent and sibling restyle hint.
         if !propagated_hint.is_empty() {
diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs
index 68a7b0f7c82..fb2faa5ae67 100644
--- a/ports/geckolib/glue.rs
+++ b/ports/geckolib/glue.rs
@@ -198,18 +198,19 @@ pub extern "C" fn Servo_Element_ClearData(element: RawGeckoElementBorrowed) -> (
 #[no_mangle]
 pub extern "C" fn Servo_Element_ShouldTraverse(element: RawGeckoElementBorrowed) -> bool {
     let element = GeckoElement(element);
-    if let Some(data) = element.get_data() {
-        debug_assert!(!element.has_dirty_descendants(),
-                      "only call Servo_Element_ShouldTraverse if you know the element \
-                       does not have dirty descendants");
-        match *data.borrow() {
-            ElementData::Initial(None) |
-            ElementData::Restyle(..) => true,
-            _ => false,
-        }
-    } else {
-        false
-    }
+    debug_assert!(!element.has_dirty_descendants(),
+                  "only call Servo_Element_ShouldTraverse if you know the element \
+                   does not have dirty descendants");
+    let result = match element.borrow_data() {
+        // Note that we check for has_restyle here rather than has_current_styles,
+        // because we also want the traversal code to trigger if there's restyle
+        // damage. We really only need the Gecko post-traversal in that case, so
+        // the servo traversal will be a no-op, but it's cheap enough that we
+        // don't bother distinguishing the two cases.
+        Some(d) => !d.has_styles() || d.has_restyle(),
+        None => true,
+    };
+    result
 }
 
 #[no_mangle]
@@ -852,17 +853,21 @@ pub extern "C" fn Servo_CSSSupports(property: *const nsACString, value: *const n
 unsafe fn maybe_restyle<'a>(data: &'a mut AtomicRefMut<ElementData>, element: GeckoElement)
     -> Option<&'a mut RestyleData>
 {
-    let r = data.restyle();
-    if r.is_some() {
-        // Propagate the bit up the chain.
-        let mut curr = element;
-        while let Some(parent) = curr.parent_element() {
-            curr = parent;
-            if curr.has_dirty_descendants() { break; }
-            curr.set_dirty_descendants();
-        }
+    // Don't generate a useless RestyleData if the element hasn't been styled.
+    if !data.has_styles() {
+        return None;
     }
-    r
+
+    // Propagate the bit up the chain.
+    let mut curr = element;
+    while let Some(parent) = curr.parent_element() {
+        curr = parent;
+        if curr.has_dirty_descendants() { break; }
+        curr.set_dirty_descendants();
+    }
+
+    // Ensure and return the RestyleData.
+    Some(data.ensure_restyle())
 }
 
 #[no_mangle]
@@ -908,62 +913,43 @@ pub extern "C" fn Servo_ImportRule_GetSheet(import_rule:
 }
 
 #[no_mangle]
-pub extern "C" fn Servo_CheckChangeHint(element: RawGeckoElementBorrowed) -> nsChangeHint
+pub extern "C" fn Servo_TakeChangeHint(element: RawGeckoElementBorrowed) -> nsChangeHint
 {
     let element = GeckoElement(element);
-    if element.get_data().is_none() {
+    let damage = if let Some(mut data) = element.mutate_data() {
+        let d = data.get_restyle().map_or(GeckoRestyleDamage::empty(), |r| r.damage);
+        data.clear_restyle();
+        d
+    } else {
         error!("Trying to get change hint from unstyled element");
-        return nsChangeHint(0);
-    }
-
-    let mut data = element.get_data().unwrap().borrow_mut();
-    let damage = data.damage_sloppy();
-
-    // If there's no change hint, the caller won't consume the new style. Do that
-    // ourselves.
-    //
-    // FIXME(bholley): Once we start storing style data on frames, we'll want to
-    // drop the data here instead.
-    if damage.is_empty() {
-        data.persist();
-    }
+        GeckoRestyleDamage::empty()
+    };
 
-    debug!("Servo_GetChangeHint: {:?}, damage={:?}", element, damage);
+    debug!("Servo_TakeChangeHint: {:?}, damage={:?}", element, damage);
     damage.as_change_hint()
 }
 
 #[no_mangle]
 pub extern "C" fn Servo_ResolveStyle(element: RawGeckoElementBorrowed,
-                                     raw_data: RawServoStyleSetBorrowed,
-                                     consume: structs::ConsumeStyleBehavior)
+                                     raw_data: RawServoStyleSetBorrowed)
                                      -> ServoComputedValuesStrong
 {
     let element = GeckoElement(element);
-    debug!("Servo_ResolveStyle: {:?}, consume={:?}", element, consume);
-
-    let mut data = unsafe { element.ensure_data() }.borrow_mut();
-    let per_doc_data = PerDocumentStyleData::from_ffi(raw_data).borrow();
+    debug!("Servo_ResolveStyle: {:?}", element);
+    let data = unsafe { element.ensure_data() }.borrow_mut();
 
     if !data.has_current_styles() {
         error!("Resolving style on unstyled element with lazy computation forbidden.");
+        let per_doc_data = PerDocumentStyleData::from_ffi(raw_data).borrow();
         return per_doc_data.default_computed_values.clone().into_strong();
     }
 
-    let values = data.styles().primary.values.clone();
-
-    if consume == structs::ConsumeStyleBehavior::Consume {
-        // FIXME(bholley): Once we start storing style data on frames, we'll want to
-        // drop the data here instead.
-        data.persist();
-    }
-
-    values.into_strong()
+    data.styles().primary.values.clone().into_strong()
 }
 
 #[no_mangle]
 pub extern "C" fn Servo_ResolveStyleLazily(element: RawGeckoElementBorrowed,
                                            pseudo_tag: *mut nsIAtom,
-                                           consume: structs::ConsumeStyleBehavior,
                                            raw_data: RawServoStyleSetBorrowed)
      -> ServoComputedValuesStrong
 {
@@ -983,15 +969,6 @@ pub extern "C" fn Servo_ResolveStyleLazily(element: RawGeckoElementBorrowed,
     let mut result = element.mutate_data()
                             .and_then(|d| d.get_styles().map(&finish));
     if result.is_some() {
-        if consume == structs::ConsumeStyleBehavior::Consume {
-            let mut d = element.mutate_data().unwrap();
-            if !d.is_persistent() {
-                // XXXheycam is it right to persist an ElementData::Restyle?
-                // Couldn't we lose restyle hints that would cause us to
-                // restyle descendants?
-                d.persist();
-            }
-        }
         return result.unwrap().into_strong();
     }
 
@@ -1007,12 +984,6 @@ pub extern "C" fn Servo_ResolveStyleLazily(element: RawGeckoElementBorrowed,
     resolve_style(&mut context, element, &ensure, &clear,
                   |styles| result = Some(finish(styles)));
 
-    // Consume the style if requested, though it may not exist anymore if the
-    // element is in a display:none subtree.
-    if consume == structs::ConsumeStyleBehavior::Consume {
-        element.mutate_data().map(|mut d| d.persist());
-    }
-
     result.unwrap().into_strong()
 }