Auto merge of #14907 - bholley:eliminate_consume, r=emilio

Give up on hoisting ElementData into the frame and eliminate the concept of consuming styles

Servo PR for the work in https://bugzilla.mozilla.org/show_bug.cgi?id=1325734

<!-- 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/14907)
<!-- Reviewable:end -->

components/layout/context.rs

@@ -16,6 +16,7 @@ use net_traits::image_cache_thread::{ImageOrMetadataAvailable, UsePlaceholder};
 use parking_lot::RwLock;
 use servo_config::opts;
 use servo_url::ServoUrl;
+use std::borrow::{Borrow, BorrowMut};
 use std::cell::{RefCell, RefMut};
 use std::collections::HashMap;
 use std::hash::BuildHasherDefault;
@@ -37,6 +38,18 @@ impl<E: TElement> ScopedThreadLocalLayoutContext<E> {
     }
 }
 
+impl<E: TElement> Borrow<ThreadLocalStyleContext<E>> for ScopedThreadLocalLayoutContext<E> {
+    fn borrow(&self) -> &ThreadLocalStyleContext<E> {
+        &self.style_context
+    }
+}
+
+impl<E: TElement> BorrowMut<ThreadLocalStyleContext<E>> for ScopedThreadLocalLayoutContext<E> {
+    fn borrow_mut(&mut self) -> &mut ThreadLocalStyleContext<E> {
+        &mut self.style_context
+    }
+}
+
 /// TLS data that persists across traversals.
 pub struct PersistentThreadLocalLayoutContext {
     // FontContext uses Rc all over the place and so isn't Send, which means we

components/layout/data.rs

@@ -46,6 +46,8 @@ impl PersistentLayoutData {
 bitflags! {
     pub flags LayoutDataFlags: u8 {
         #[doc = "Whether a flow has been newly constructed."]
-        const HAS_NEWLY_CONSTRUCTED_FLOW = 0x01
+        const HAS_NEWLY_CONSTRUCTED_FLOW = 0x01,
+        #[doc = "Whether this node has been traversed by layout."]
+        const HAS_BEEN_TRAVERSED = 0x02,
     }
 }

components/layout/fragment.rs

@@ -53,6 +53,7 @@ use style::values::{self, Either};
 use style::values::computed::{LengthOrPercentage, LengthOrPercentageOrAuto};
 use text;
 use text::TextRunScanner;
+use wrapper::ThreadSafeLayoutNodeHelpers;
 
 // From gfxFontConstants.h in Firefox.
 static FONT_SUBSCRIPT_OFFSET_RATIO: f32 = 0.20;

components/layout/traversal.rs

@@ -14,12 +14,13 @@ use script_layout_interface::wrapper_traits::{LayoutNode, ThreadSafeLayoutNode};
 use servo_config::opts;
 use style::context::{SharedStyleContext, StyleContext};
 use style::data::ElementData;
-use style::dom::{TElement, TNode};
+use style::dom::{NodeInfo, TElement, TNode};
 use style::selector_parser::RestyleDamage;
 use style::servo::restyle_damage::{BUBBLE_ISIZES, REFLOW, REFLOW_OUT_OF_FLOW, REPAINT};
 use style::traversal::{DomTraversal, recalc_style_at};
 use style::traversal::PerLevelTraversalData;
 use wrapper::{GetRawData, LayoutNodeHelpers, LayoutNodeLayoutData};
+use wrapper::ThreadSafeLayoutNodeHelpers;
 
 pub struct RecalcStyleAndConstructFlows {
     shared: SharedLayoutContext,
@@ -47,14 +48,14 @@ impl RecalcStyleAndConstructFlows {
 }
 
 #[allow(unsafe_code)]
-impl<N> DomTraversal<N> for RecalcStyleAndConstructFlows
+impl<E> DomTraversal<E> for RecalcStyleAndConstructFlows
-    where N: LayoutNode + TNode,
+    where E: TElement,
-          N::ConcreteElement: TElement
+          E::ConcreteNode: LayoutNode,
 {
-    type ThreadLocalContext = ScopedThreadLocalLayoutContext<N::ConcreteElement>;
+    type ThreadLocalContext = ScopedThreadLocalLayoutContext<E>;
 
     fn process_preorder(&self, traversal_data: &mut PerLevelTraversalData,
-                        thread_local: &mut Self::ThreadLocalContext, node: N) {
+                        thread_local: &mut Self::ThreadLocalContext, node: E::ConcreteNode) {
         // FIXME(pcwalton): Stop allocating here. Ideally this should just be
         // done by the HTML parser.
         node.initialize_data();
@@ -70,12 +71,12 @@ impl<N> DomTraversal<N> for RecalcStyleAndConstructFlows
         }
     }
 
-    fn process_postorder(&self, thread_local: &mut Self::ThreadLocalContext, node: N) {
+    fn process_postorder(&self, thread_local: &mut Self::ThreadLocalContext, node: E::ConcreteNode) {
         let context = LayoutContext::new(&self.shared);
         construct_flows_at(&context, thread_local, node);
     }
 
-    fn text_node_needs_traversal(node: N) -> bool {
+    fn text_node_needs_traversal(node: E::ConcreteNode) -> bool {
         // Text nodes never need styling. However, there are two cases they may need
         // flow construction:
         // (1) They child doesn't yet have layout data (preorder traversal initializes it).
@@ -84,12 +85,12 @@ impl<N> DomTraversal<N> for RecalcStyleAndConstructFlows
         node.parent_node().unwrap().to_threadsafe().restyle_damage() != RestyleDamage::empty()
     }
 
-    unsafe fn ensure_element_data(element: &N::ConcreteElement) -> &AtomicRefCell<ElementData> {
+    unsafe fn ensure_element_data(element: &E) -> &AtomicRefCell<ElementData> {
         element.as_node().initialize_data();
         element.get_data().unwrap()
     }
 
-    unsafe fn clear_element_data(element: &N::ConcreteElement) {
+    unsafe fn clear_element_data(element: &E) {
         element.as_node().clear_data();
     }
 
@@ -134,10 +135,11 @@ fn construct_flows_at<'a, N>(context: &LayoutContext<'a>,
                        tnode.flow_debug_id());
             }
         }
+
+        tnode.mutate_layout_data().unwrap().flags.insert(::data::HAS_BEEN_TRAVERSED);
     }
 
     if let Some(el) = node.as_element() {
-        el.mutate_data().unwrap().persist();
         unsafe { el.unset_dirty_descendants(); }
     }
 }

components/layout/wrapper.rs

@@ -37,6 +37,8 @@ use script_layout_interface::{OpaqueStyleAndLayoutData, PartialPersistentLayoutD
 use script_layout_interface::wrapper_traits::{LayoutNode, ThreadSafeLayoutElement, ThreadSafeLayoutNode};
 use script_layout_interface::wrapper_traits::GetLayoutData;
 use style::computed_values::content::{self, ContentItem};
+use style::dom::{NodeInfo, TNode};
+use style::selector_parser::RestyleDamage;
 
 pub type NonOpaqueStyleAndLayoutData = AtomicRefCell<PersistentLayoutData>;
 
@@ -120,6 +122,12 @@ pub trait ThreadSafeLayoutNodeHelpers {
     ///
     /// FIXME(pcwalton): This might have too much copying and/or allocation. Profile this.
     fn text_content(&self) -> TextContent;
+
+    /// The RestyleDamage from any restyling, or RestyleDamage::rebuild_and_reflow() if this
+    /// is the first time layout is visiting this node. We implement this here, rather than
+    /// with the rest of the wrapper layer, because we need layout code to determine whether
+    /// layout has visited the node.
+    fn restyle_damage(self) -> RestyleDamage;
 }
 
 impl<T: ThreadSafeLayoutNode> ThreadSafeLayoutNodeHelpers for T {
@@ -149,6 +157,37 @@ impl<T: ThreadSafeLayoutNode> ThreadSafeLayoutNodeHelpers for T {
 
         return TextContent::Text(self.node_text_content());
     }
+
+    fn restyle_damage(self) -> RestyleDamage {
+        // We need the underlying node to potentially access the parent in the
+        // case of text nodes. This is safe as long as we don't let the parent
+        // escape and never access its descendants.
+        let mut node = unsafe { self.unsafe_get() };
+
+        // If this is a text node, use the parent element, since that's what
+        // controls our style.
+        if node.is_text_node() {
+            node = node.parent_node().unwrap();
+            debug_assert!(node.is_element());
+        }
+
+        let data = node.borrow_layout_data().unwrap();
+        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
+        } else if !data.flags.contains(::data::HAS_BEEN_TRAVERSED) {
+            // We're reflowing a node that was styled for the first time and
+            // has never been visited by layout. Return rebuild_and_reflow,
+            // because that's what the code expects.
+            RestyleDamage::rebuild_and_reflow()
+        } else {
+            // We're reflowing a node whose style data didn't change, but whose
+            // layout may change due to changes in ancestors or descendants.
+            RestyleDamage::empty()
+        }
+    }
+
 }
 
 pub enum TextContent {

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()
+                        if let Some(mut d) = element.mutate_data() {
-                                        .map(|mut r| r.hint.insert(&StoredRestyleHint::subtree())));
+                            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() {
+                let mut restyle_data = style_data.ensure_restyle();
-                    Some(d) => d,
-                    None => continue,
-                };
 
                 // Stash the data on the element for processing by the style system.
                 restyle_data.hint = restyle.hint.into();
@@ -1157,9 +1161,9 @@ impl LayoutThread {
         let dom_depth = Some(0); // This is always the root node.
         let token = {
             let stylist = &<RecalcStyleAndConstructFlows as
-                            DomTraversal<ServoLayoutNode>>::shared_context(&traversal).stylist;
+                            DomTraversal<ServoLayoutElement>>::shared_context(&traversal).stylist;
             <RecalcStyleAndConstructFlows as
-             DomTraversal<ServoLayoutNode>>::pre_traverse(element, stylist, /* skip_root = */ false)
+             DomTraversal<ServoLayoutElement>>::pre_traverse(element, stylist, /* skip_root = */ false)
         };
 
         if token.should_traverse() {
@@ -1171,11 +1175,11 @@ impl LayoutThread {
                 // Perform CSS selector matching and flow construction.
                 if let (true, Some(pool)) = (self.parallel_flag, self.parallel_traversal.as_mut()) {
                     // Parallel mode
-                    parallel::traverse_dom::<ServoLayoutNode, RecalcStyleAndConstructFlows>(
+                    parallel::traverse_dom::<ServoLayoutElement, RecalcStyleAndConstructFlows>(
                         &traversal, element, dom_depth, token, pool);
                 } else {
                     // Sequential mode
-                    sequential::traverse_dom::<ServoLayoutNode, RecalcStyleAndConstructFlows>(
+                    sequential::traverse_dom::<ServoLayoutElement, RecalcStyleAndConstructFlows>(
                         &traversal, element, token);
                 }
             });

components/script/layout_wrapper.rs

@@ -67,7 +67,7 @@ use style::dom::{LayoutIterator, NodeInfo, OpaqueNode, PresentationalHintsSynthe
 use style::dom::UnsafeNode;
 use style::element_state::*;
 use style::properties::{ComputedValues, PropertyDeclarationBlock};
-use style::selector_parser::{NonTSPseudoClass, PseudoElement, RestyleDamage, SelectorImpl};
+use style::selector_parser::{NonTSPseudoClass, PseudoElement, SelectorImpl};
 use style::sink::Push;
 use style::str::is_whitespace;
 use style::stylist::ApplicableDeclarationBlock;
@@ -740,6 +740,7 @@ impl<'ln> NodeInfo for ServoThreadSafeLayoutNode<'ln> {
 }
 
 impl<'ln> ThreadSafeLayoutNode for ServoThreadSafeLayoutNode<'ln> {
+    type ConcreteNode = ServoLayoutNode<'ln>;
     type ConcreteThreadSafeLayoutElement = ServoThreadSafeLayoutElement<'ln>;
     type ChildrenIterator = ThreadSafeLayoutNodeChildrenIterator<Self>;
 
@@ -815,15 +816,8 @@ impl<'ln> ThreadSafeLayoutNode for ServoThreadSafeLayoutNode<'ln> {
         }
     }
 
-    fn restyle_damage(self) -> RestyleDamage {
+    unsafe fn unsafe_get(self) -> Self::ConcreteNode {
-        let element = if self.is_text_node() {
+        self.node
-            self.node.parent_node().unwrap().as_element().unwrap()
-        } else {
-            self.node.as_element().unwrap()
-        };
-
-        let damage = element.borrow_data().unwrap().damage();
-        damage
     }
 
     fn can_be_fragmented(&self) -> bool {

components/script_layout_interface/wrapper_traits.rs

@@ -151,6 +151,7 @@ impl<ConcreteNode> Iterator for TreeIterator<ConcreteNode>
 /// A thread-safe version of `LayoutNode`, used during flow construction. This type of layout
 /// node does not allow any parents or siblings of nodes to be accessed, to avoid races.
 pub trait ThreadSafeLayoutNode: Clone + Copy + Debug + GetLayoutData + NodeInfo + PartialEq + Sized {
+    type ConcreteNode: LayoutNode<ConcreteThreadSafeLayoutNode = Self>;
     type ConcreteThreadSafeLayoutElement:
         ThreadSafeLayoutElement<ConcreteThreadSafeLayoutNode = Self>
         + ::selectors::Element<Impl=SelectorImpl>;
@@ -237,14 +238,20 @@ pub trait ThreadSafeLayoutNode: Clone + Copy + Debug + GetLayoutData + NodeInfo
 
     fn is_ignorable_whitespace(&self, context: &SharedStyleContext) -> bool;
 
-    fn restyle_damage(self) -> RestyleDamage;
-
     /// Returns true if this node contributes content. This is used in the implementation of
     /// `empty_cells` per CSS 2.1 § 17.6.1.1.
     fn is_content(&self) -> bool {
         self.type_id().is_some()
     }
 
+    /// Returns access to the underlying LayoutNode. This is breaks the abstraction
+    /// barrier of ThreadSafeLayout wrapper layer, and can lead to races if not used
+    /// carefully.
+    ///
+    /// We need this because the implementation of some methods need to access the layout
+    /// data flags, and we have this annoying trait separation between script and layout :-(
+    unsafe fn unsafe_get(self) -> Self::ConcreteNode;
+
     fn can_be_fragmented(&self) -> bool;
 
     fn node_text_content(&self) -> String;

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",

components/style/context.rs

@@ -8,7 +8,8 @@
 use animation::Animation;
 use app_units::Au;
 use bloom::StyleBloom;
-use dom::{OpaqueNode, TElement};
+use data::ElementData;
+use dom::{OpaqueNode, TNode, TElement};
 use error_reporting::ParseErrorReporter;
 use euclid::Size2D;
 use matching::StyleSharingCandidateCache;
@@ -89,6 +90,18 @@ pub struct SharedStyleContext {
     pub default_computed_values: Arc<ComputedValues>,
 }
 
+/// Information about the current element being processed. We group this together
+/// into a single struct within ThreadLocalStyleContext so that we can instantiate
+/// and destroy it easily at the beginning and end of element processing.
+struct CurrentElementInfo {
+    /// The element being processed. Currently we use an OpaqueNode since we only
+    /// use this for identity checks, but we could use SendElement if there were
+    /// a good reason to.
+    element: OpaqueNode,
+    /// Whether the element is being styled for the first time.
+    is_initial_style: bool,
+}
+
 /// A thread-local style context.
 ///
 /// This context contains data that needs to be used during restyling, but is
@@ -102,17 +115,52 @@ pub struct ThreadLocalStyleContext<E: TElement> {
     /// A channel on which new animations that have been triggered by style
     /// recalculation can be sent.
     pub new_animations_sender: Sender<Animation>,
+    /// Information related to the current element, non-None during processing.
+    current_element_info: Option<CurrentElementInfo>,
 }
 
 impl<E: TElement> ThreadLocalStyleContext<E> {
-    /// Create a new `ThreadLocalStyleContext` from a shared one.
+    /// Creates a new `ThreadLocalStyleContext` from a shared one.
     pub fn new(shared: &SharedStyleContext) -> Self {
         ThreadLocalStyleContext {
             style_sharing_candidate_cache: StyleSharingCandidateCache::new(),
             bloom_filter: StyleBloom::new(),
             new_animations_sender: shared.local_context_creation_data.lock().unwrap().new_animations_sender.clone(),
+            current_element_info: None,
         }
     }
+
+    /// Notes when the style system starts traversing an element.
+    pub fn begin_element(&mut self, element: E, data: &ElementData) {
+        debug_assert!(self.current_element_info.is_none());
+        self.current_element_info = Some(CurrentElementInfo {
+            element: element.as_node().opaque(),
+            is_initial_style: !data.has_styles(),
+        });
+    }
+
+    /// Notes when the style system finishes traversing an element.
+    pub fn end_element(&mut self, element: E) {
+        debug_assert!(self.current_element_info.is_some());
+        debug_assert!(self.current_element_info.as_ref().unwrap().element ==
+                      element.as_node().opaque());
+        self.current_element_info = None;
+    }
+
+    /// Returns true if the current element being traversed is being styled for
+    /// the first time.
+    ///
+    /// Panics if called while no element is being traversed.
+    pub fn is_initial_style(&self) -> bool {
+        self.current_element_info.as_ref().unwrap().is_initial_style
+    }
+}
+
+#[cfg(debug_assertions)]
+impl<E: TElement> Drop for ThreadLocalStyleContext<E> {
+    fn drop(&mut self) {
+        debug_assert!(self.current_element_info.is_none());
+    }
 }
 
 /// A `StyleContext` is just a simple container for a immutable reference to a

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 {
+impl Default for RestyleData {
-    fn new(styles: ElementStyles) -> Self {
+    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.
+    /// Returns true if this RestyleData might invalidate the current style.
-    pub fn has_current_styles(&self) -> bool {
+    pub fn has_invalidations(&self) -> bool {
-        !(self.hint.restyle_self || self.recascade || self.snapshot.is_some())
+        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.
     }
 }
 
-/// Style system data associated with a node.
+/// Style system data associated with an Element.
 ///
-/// In Gecko, this hangs directly off a node, but is dropped when the frame takes
+/// In Gecko, this hangs directly off the Element. Servo, this is embedded
-/// ownership of the computed style data.
+/// inside of layout data, which itself hangs directly off the Element. In
-///
+/// both cases, it is wrapped inside an AtomicRefCell to ensure thread safety.
-/// 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.
-///
-/// In both cases, it is wrapped inside an AtomicRefCell to ensure thread
-/// safety.
 #[derive(Debug)]
-pub enum ElementData {
+pub struct ElementData {
-    /// This is the first styling for this element.
+    /// The computed styles for the element and its pseudo-elements.
-    Initial(Option<ElementStyles>),
+    styles: Option<ElementStyles>,
-    /// This element has been restyled already, and all the relevant data is
+
-    /// inside the `RestyleData`.
+    /// Restyle tracking. We separate this into a separate allocation so that
-    Restyle(RestyleData),
+    /// we can drop it when no restyles are pending on the elemnt.
-    /// This element has already been restyled, and only keeps its styles
+    restyle: Option<Box<RestyleData>>,
-    /// around.
-    Persistent(ElementStyles),
 }
 
 impl ElementData {
     /// Trivially construct an ElementData.
     pub fn new(existing: Option<ElementStyles>) -> Self {
-        if let Some(s) = existing {
+        ElementData {
-            ElementData::Persistent(s)
+            styles: existing,
-        } else {
+            restyle: None,
-            ElementData::Initial(None)
         }
     }
 
-    /// Return whether this data is from an initial restyle.
+    /// Returns true if this element has a computed styled.
-    pub fn is_initial(&self) -> bool {
+    pub fn has_styles(&self) -> bool {
-        match *self {
+        self.styles.is_some()
-            ElementData::Initial(_) => true,
-            _ => false,
-        }
-    }
-
-    /// 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,
-        }
-    }
-
-    /// 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 is being restyled and has been styled
-    /// before.
-    pub fn is_restyle(&self) -> bool {
-        match *self {
-            ElementData::Restyle(_) => true,
-            _ => false,
-        }
-    }
-
-    /// Returns the `RestyleData` if it exists.
-    pub fn as_restyle(&self) -> Option<&RestyleData> {
-        match *self {
-            ElementData::Restyle(ref x) => Some(x),
-            _ => None,
-        }
-    }
-
-    /// 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,
-        }
-    }
-
-    /// Returns whether this element's style is persistent.
-    pub fn is_persistent(&self) -> bool {
-        match *self {
-            ElementData::Persistent(_) => true,
-            _ => false,
-        }
-    }
-
-    /// 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)
-    }
-
-    /// 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(),
-        }
-    }
-
-    /// 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(),
-        }
     }
 
     /// 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::*;
+        self.has_styles() &&
-        match *self {
+        self.restyle.as_ref().map_or(true, |r| !r.has_invalidations())
-            Initial(ref x) => x.is_some(),
-            Restyle(ref x) => x.has_current_styles(),
-            Persistent(_) => true,
-        }
     }
 
-    /// Get the element styles, if any.
+    /// Gets the element styles, if any.
     pub fn get_styles(&self) -> Option<&ElementStyles> {
-        use self::ElementData::*;
+        self.styles.as_ref()
-        match *self {
-            Initial(ref x) => x.as_ref(),
-            Restyle(ref x) => Some(x.styles()),
-            Persistent(ref x) => Some(x),
-        }
     }
 
-    /// Get the element styles. Panic if the element has never been styled.
+    /// Gets 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")
+        self.styles.as_ref().expect("Calling styles() on unstyled ElementData")
     }
 
-    /// Get a mutable reference to the element styles, if any.
+    /// Gets a mutable reference to the element styles, if any.
     pub fn get_styles_mut(&mut self) -> Option<&mut ElementStyles> {
-        use self::ElementData::*;
+        self.styles.as_mut()
-        match *self {
-            Initial(ref mut x) => x.as_mut(),
-            Restyle(ref mut x) => Some(x.styles_mut()),
-            Persistent(ref mut x) => Some(x),
-        }
     }
 
-    /// Get a mutable reference to the element styles. Panic if the element has
+    /// 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.get_styles_mut().expect("Calling styles_mut() on unstyled ElementData")
+        self.styles.as_mut().expect("Caling styles_mut() on unstyled ElementData")
     }
 
-    /// Finishes the styling of the element, effectively setting the style in
+    /// Sets the computed element styles.
-    /// the data.
+    pub fn set_styles(&mut self, styles: ElementStyles) {
-    pub fn finish_styling(&mut self, styles: ElementStyles, damage: RestyleDamage) {
+        debug_assert!(self.get_restyle().map_or(true, |r| r.snapshot.is_none()),
-        use self::ElementData::*;
+                      "Traversal should have expanded snapshots");
-        match *self {
+        self.styles = Some(styles);
-            Initial(ref mut x) => {
+    }
-                debug_assert!(x.is_none());
+
-                debug_assert!(damage == RestyleDamage::rebuild_and_reflow());
+    /// Returns true if the Element has a RestyleData.
-                *x = Some(styles);
+    pub fn has_restyle(&self) -> bool {
-            },
+        self.restyle.is_some()
-            Restyle(ref mut x) => x.finish_styling(styles, damage),
+    }
-            Persistent(_) => panic!("Calling finish_styling on Persistent ElementData"),
+
-        };
+    /// Drops any RestyleData.
+    pub fn clear_restyle(&mut self) {
+        self.restyle = None;
+    }
+
+    /// 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()
+    }
+
+    /// Gets a reference to the restyle data, if any.
+    pub fn get_restyle(&self) -> Option<&RestyleData> {
+        self.restyle.as_ref().map(|r| &**r)
+    }
+
+    /// 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")
+    }
+
+    /// 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)
+    }
+
+    /// 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")
     }
 }

components/style/gecko/traversal.rs

@@ -26,12 +26,12 @@ impl RecalcStyleOnly {
     }
 }
 
-impl<'ln> DomTraversal<GeckoNode<'ln>> for RecalcStyleOnly {
+impl<'le> DomTraversal<GeckoElement<'le>> for RecalcStyleOnly {
-    type ThreadLocalContext = ThreadLocalStyleContext<GeckoElement<'ln>>;
+    type ThreadLocalContext = ThreadLocalStyleContext<GeckoElement<'le>>;
 
     fn process_preorder(&self, traversal_data: &mut PerLevelTraversalData,
                         thread_local: &mut Self::ThreadLocalContext,
-                        node: GeckoNode<'ln>)
+                        node: GeckoNode<'le>)
     {
         if node.is_element() {
             let el = node.as_element().unwrap();
@@ -44,18 +44,18 @@ impl<'ln> DomTraversal<GeckoNode<'ln>> for RecalcStyleOnly {
         }
     }
 
-    fn process_postorder(&self, _: &mut Self::ThreadLocalContext, _: GeckoNode<'ln>) {
+    fn process_postorder(&self, _: &mut Self::ThreadLocalContext, _: GeckoNode<'le>) {
         unreachable!();
     }
 
     /// We don't use the post-order traversal for anything.
     fn needs_postorder_traversal() -> bool { false }
 
-    unsafe fn ensure_element_data<'a>(element: &'a GeckoElement<'ln>) -> &'a AtomicRefCell<ElementData> {
+    unsafe fn ensure_element_data<'a>(element: &'a GeckoElement<'le>) -> &'a AtomicRefCell<ElementData> {
         element.ensure_data()
     }
 
-    unsafe fn clear_element_data<'a>(element: &'a GeckoElement<'ln>) {
+    unsafe fn clear_element_data<'a>(element: &'a GeckoElement<'le>) {
         element.clear_data()
     }
 

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,
+                              set: RawServoStyleSetBorrowed)
-                              consume: ConsumeStyleBehavior)
      -> 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;
 }

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,

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,

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 = {
+                    let maybe_damage = {
-                        debug_assert!(!data.has_current_styles());
+                        let previous = data.get_styles().map(|x| &x.primary.values);
-                        let previous_values = data.get_styles().map(|x| &x.primary.values);
+                        let existing = self.existing_style_for_restyle_damage(previous, None);
-                        match self.existing_style_for_restyle_damage(previous_values, None) {
+                        existing.map(|e| RestyleDamage::compute(e, &shared_style.values))
-                            Some(ref source) => RestyleDamage::compute(source, &shared_style.values),
-                            None => RestyleDamage::rebuild_and_reflow(),
-                        }
                     };
+                    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.

components/style/parallel.rs

@@ -37,13 +37,13 @@ pub const CHUNK_SIZE: usize = 64;
 
 /// A parallel top down traversal, generic over `D`.
 #[allow(unsafe_code)]
-pub fn traverse_dom<N, D>(traversal: &D,
+pub fn traverse_dom<E, D>(traversal: &D,
-                          root: N::ConcreteElement,
+                          root: E,
                           known_root_dom_depth: Option<usize>,
                           token: PreTraverseToken,
                           queue: &rayon::ThreadPool)
-    where N: TNode,
+    where E: TElement,
-          D: DomTraversal<N>,
+          D: DomTraversal<E>,
 {
     if opts::get().style_sharing_stats {
         STYLE_SHARING_CACHE_HITS.store(0, Ordering::SeqCst);
@@ -91,14 +91,14 @@ pub fn traverse_dom<N, D>(traversal: &D,
 /// A parallel top-down DOM traversal.
 #[inline(always)]
 #[allow(unsafe_code)]
-fn top_down_dom<'a, 'scope, N, D>(nodes: &'a [SendNode<N>],
+fn top_down_dom<'a, 'scope, E, D>(nodes: &'a [SendNode<E::ConcreteNode>],
                                   root: OpaqueNode,
                                   mut traversal_data: PerLevelTraversalData,
                                   scope: &'a rayon::Scope<'scope>,
                                   traversal: &'scope D,
                                   tls: &'scope ScopedTLS<'scope, D::ThreadLocalContext>)
-    where N: TNode + 'scope,
+    where E: TElement + 'scope,
-          D: DomTraversal<N>,
+          D: DomTraversal<E>,
 {
     let mut discovered_child_nodes = vec![];
     {
@@ -112,7 +112,7 @@ fn top_down_dom<'a, 'scope, N, D>(nodes: &'a [SendNode<N>],
             let mut children_to_process = 0isize;
             traversal.process_preorder(&mut traversal_data, &mut *tlc, node);
             if let Some(el) = node.as_element() {
-                D::traverse_children(el, |kid| {
+                traversal.traverse_children(&mut *tlc, el, |_tlc, kid| {
                     children_to_process += 1;
                     discovered_child_nodes.push(unsafe { SendNode::new(kid) })
                 });
@@ -140,13 +140,13 @@ fn top_down_dom<'a, 'scope, N, D>(nodes: &'a [SendNode<N>],
     traverse_nodes(discovered_child_nodes, root, traversal_data, scope, traversal, tls);
 }
 
-fn traverse_nodes<'a, 'scope, N, D>(nodes: Vec<SendNode<N>>, root: OpaqueNode,
+fn traverse_nodes<'a, 'scope, E, D>(nodes: Vec<SendNode<E::ConcreteNode>>, root: OpaqueNode,
                                     traversal_data: PerLevelTraversalData,
                                     scope: &'a rayon::Scope<'scope>,
                                     traversal: &'scope D,
                                     tls: &'scope ScopedTLS<'scope, D::ThreadLocalContext>)
-    where N: TNode + 'scope,
+    where E: TElement + 'scope,
-          D: DomTraversal<N>,
+          D: DomTraversal<E>,
 {
     if nodes.is_empty() {
         return;
@@ -182,12 +182,12 @@ fn traverse_nodes<'a, 'scope, N, D>(nodes: Vec<SendNode<N>>, root: OpaqueNode,
 ///
 /// The only communication between siblings is that they both
 /// fetch-and-subtract the parent's children count.
-fn bottom_up_dom<N, D>(traversal: &D,
+fn bottom_up_dom<E, D>(traversal: &D,
                        thread_local: &mut D::ThreadLocalContext,
                        root: OpaqueNode,
-                       mut node: N)
+                       mut node: E::ConcreteNode)
-    where N: TNode,
+    where E: TElement,
-          D: DomTraversal<N>,
+          D: DomTraversal<E>,
 {
     loop {
         // Perform the appropriate operation.

components/style/sequential.rs

@@ -10,18 +10,18 @@ use dom::{TElement, TNode};
 use traversal::{DomTraversal, PerLevelTraversalData, PreTraverseToken};
 
 /// Do a sequential DOM traversal for layout or styling, generic over `D`.
-pub fn traverse_dom<N, D>(traversal: &D,
+pub fn traverse_dom<E, D>(traversal: &D,
-                          root: N::ConcreteElement,
+                          root: E,
                           token: PreTraverseToken)
-    where N: TNode,
+    where E: TElement,
-          D: DomTraversal<N>,
+          D: DomTraversal<E>,
 {
     debug_assert!(token.should_traverse());
 
-    fn doit<N, D>(traversal: &D, traversal_data: &mut PerLevelTraversalData,
+    fn doit<E, D>(traversal: &D, traversal_data: &mut PerLevelTraversalData,
-                  thread_local: &mut D::ThreadLocalContext, node: N)
+                  thread_local: &mut D::ThreadLocalContext, node: E::ConcreteNode)
-        where N: TNode,
+        where E: TElement,
-              D: DomTraversal<N>
+              D: DomTraversal<E>
     {
         traversal.process_preorder(traversal_data, thread_local, node);
         if let Some(el) = node.as_element() {
@@ -29,7 +29,9 @@ pub fn traverse_dom<N, D>(traversal: &D,
                 *depth += 1;
             }
 
-            D::traverse_children(el, |kid| doit(traversal, traversal_data, thread_local, kid));
+            traversal.traverse_children(thread_local, el, |tlc, kid| {
+                doit(traversal, traversal_data, tlc, kid)
+            });
 
             if let Some(ref mut depth) = traversal_data.current_dom_depth {
                 *depth -= 1;

components/style/traversal.rs

@@ -7,14 +7,14 @@
 #![deny(missing_docs)]
 
 use atomic_refcell::{AtomicRefCell, AtomicRefMut};
-use context::{SharedStyleContext, StyleContext};
+use context::{SharedStyleContext, StyleContext, ThreadLocalStyleContext};
 use data::{ElementData, ElementStyles, StoredRestyleHint};
-use dom::{TElement, TNode};
+use dom::{NodeInfo, TElement, TNode};
 use matching::{MatchMethods, StyleSharingResult};
 use restyle_hints::{RESTYLE_DESCENDANTS, RESTYLE_SELF};
 use selector_parser::RestyleDamage;
-use selectors::Element;
 use servo_config::opts;
+use std::borrow::BorrowMut;
 use std::mem;
 use std::sync::atomic::{AtomicUsize, ATOMIC_USIZE_INIT, Ordering};
 use stylist::Stylist;
@@ -72,20 +72,23 @@ impl LogBehavior {
 
 /// A DOM Traversal trait, that is used to generically implement styling for
 /// Gecko and Servo.
-pub trait DomTraversal<N: TNode> : Sync {
+pub trait DomTraversal<E: TElement> : Sync {
     /// The thread-local context, used to store non-thread-safe stuff that needs
     /// to be used in the traversal, and of which we use one per worker, like
     /// the bloom filter, for example.
-    type ThreadLocalContext: Send;
+    type ThreadLocalContext: Send + BorrowMut<ThreadLocalStyleContext<E>>;
 
     /// Process `node` on the way down, before its children have been processed.
     fn process_preorder(&self, data: &mut PerLevelTraversalData,
-                        thread_local: &mut Self::ThreadLocalContext, node: N);
+                        thread_local: &mut Self::ThreadLocalContext,
+                        node: E::ConcreteNode);
 
     /// Process `node` on the way up, after its children have been processed.
     ///
     /// This is only executed if `needs_postorder_traversal` returns true.
-    fn process_postorder(&self, thread_local: &mut Self::ThreadLocalContext, node: N);
+    fn process_postorder(&self,
+                         thread_local: &mut Self::ThreadLocalContext,
+                         node: E::ConcreteNode);
 
     /// Boolean that specifies whether a bottom up traversal should be
     /// performed.
@@ -99,8 +102,7 @@ pub trait DomTraversal<N: TNode> : Sync {
     ///
     /// The unstyled_children_only parameter is used in Gecko to style newly-
     /// appended children without restyling the parent.
-    fn pre_traverse(root: N::ConcreteElement, stylist: &Stylist,
+    fn pre_traverse(root: E, stylist: &Stylist, unstyled_children_only: bool)
-                    unstyled_children_only: bool)
                     -> PreTraverseToken
     {
         if unstyled_children_only {
@@ -117,7 +119,7 @@ pub trait DomTraversal<N: TNode> : 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);
             }
@@ -132,10 +134,13 @@ pub trait DomTraversal<N: TNode> : Sync {
     /// Returns true if traversal should visit a text node. The style system never
     /// processes text nodes, but Servo overrides this to visit them for flow
     /// construction when necessary.
-    fn text_node_needs_traversal(node: N) -> bool { debug_assert!(node.is_text_node()); false }
+    fn text_node_needs_traversal(node: E::ConcreteNode) -> bool {
+        debug_assert!(node.is_text_node());
+        false
+    }
 
     /// Returns true if traversal is needed for the given node and subtree.
-    fn node_needs_traversal(node: N) -> bool {
+    fn node_needs_traversal(node: E::ConcreteNode) -> bool {
         // Non-incremental layout visits every node.
         if cfg!(feature = "servo") && opts::get().nonincremental_layout {
             return true;
@@ -157,25 +162,31 @@ pub trait DomTraversal<N: TNode> : Sync {
                     None => return true,
                 };
 
-                // Check what kind of element data we have. If it's Initial or Persistent,
+                // If we don't have any style data, we need to visit the element.
-                // we're done.
+                if !data.has_styles() {
-                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 {
                     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;
                 }
 
@@ -189,7 +200,10 @@ pub trait DomTraversal<N: TNode> : Sync {
     ///
     /// This may be called multiple times when processing an element, so we pass
     /// a parameter to keep the logs tidy.
-    fn should_traverse_children(parent: N::ConcreteElement, parent_data: &ElementData,
+    fn should_traverse_children(&self,
+                                thread_local: &mut ThreadLocalStyleContext<E>,
+                                parent: E,
+                                parent_data: &ElementData,
                                 log: LogBehavior) -> bool
     {
         // See the comment on `cascade_node` for why we allow this on Gecko.
@@ -222,7 +236,7 @@ pub trait DomTraversal<N: TNode> : Sync {
         // happens, we may just end up doing wasted work, since Gecko
         // recursively drops Servo ElementData when the XBL insertion parent of
         // an Element is changed.
-        if cfg!(feature = "gecko") && parent_data.is_styled_initial() &&
+        if cfg!(feature = "gecko") && thread_local.is_initial_style() &&
            parent_data.styles().primary.values.has_moz_binding() {
             if log.allow() { debug!("Parent {:?} has XBL binding, deferring traversal", parent); }
             return false;
@@ -234,10 +248,15 @@ pub trait DomTraversal<N: TNode> : Sync {
 
     /// Helper for the traversal implementations to select the children that
     /// should be enqueued for processing.
-    fn traverse_children<F: FnMut(N)>(parent: N::ConcreteElement, mut f: F)
+    fn traverse_children<F>(&self, thread_local: &mut Self::ThreadLocalContext, parent: E, mut f: F)
+        where F: FnMut(&mut Self::ThreadLocalContext, E::ConcreteNode)
     {
         // Check if we're allowed to traverse past this element.
-        if !Self::should_traverse_children(parent, &parent.borrow_data().unwrap(), MayLog) {
+        let should_traverse =
+            self.should_traverse_children(thread_local.borrow_mut(), parent,
+                                          &parent.borrow_data().unwrap(), MayLog);
+        thread_local.borrow_mut().end_element(parent);
+        if !should_traverse {
             return;
         }
 
@@ -245,11 +264,11 @@ pub trait DomTraversal<N: TNode> : 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(); }
                 }
-                f(kid);
+                f(thread_local, kid);
             }
         }
     }
@@ -260,18 +279,18 @@ pub trait DomTraversal<N: TNode> : Sync {
     ///
     /// This is only safe to call in top-down traversal before processing the
     /// children of |element|.
-    unsafe fn ensure_element_data(element: &N::ConcreteElement) -> &AtomicRefCell<ElementData>;
+    unsafe fn ensure_element_data(element: &E) -> &AtomicRefCell<ElementData>;
 
     /// Clears the ElementData attached to this element, if any.
     ///
     /// This is only safe to call in top-down traversal before processing the
     /// children of |element|.
-    unsafe fn clear_element_data(element: &N::ConcreteElement);
+    unsafe fn clear_element_data(element: &E);
 
     /// Return the shared style context common to all worker threads.
     fn shared_context(&self) -> &SharedStyleContext;
 
-    /// Create a thread-local context.
+    /// Creates a thread-local context.
     fn create_thread_local_context(&self) -> Self::ThreadLocalContext;
 }
 
@@ -364,9 +383,10 @@ pub fn recalc_style_at<E, D>(traversal: &D,
                              element: E,
                              mut data: &mut AtomicRefMut<ElementData>)
     where E: TElement,
-          D: DomTraversal<E::ConcreteNode>
+          D: DomTraversal<E>
 {
-    debug_assert!(data.as_restyle().map_or(true, |r| r.snapshot.is_none()),
+    context.thread_local.begin_element(element, &data);
+    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();
@@ -383,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;
@@ -394,7 +414,7 @@ pub fn recalc_style_at<E, D>(traversal: &D,
     trace!("propagated_hint={:?}, inherited_style_changed={:?}", propagated_hint, inherited_style_changed);
 
     // Preprocess children, propagating restyle hints and handling sibling relationships.
-    if D::should_traverse_children(element, &data, DontLog) &&
+    if traversal.should_traverse_children(&mut context.thread_local, element, &data, DontLog) &&
        (element.has_dirty_descendants() || !propagated_hint.is_empty() || inherited_style_changed) {
         preprocess_children(traversal, element, propagated_hint, inherited_style_changed);
     }
@@ -411,7 +431,7 @@ fn compute_style<E, D>(_traversal: &D,
                        element: E,
                        mut data: &mut AtomicRefMut<ElementData>) -> bool
     where E: TElement,
-          D: DomTraversal<E::ConcreteNode>,
+          D: DomTraversal<E>,
 {
     let shared_context = context.shared;
     // Ensure the bloom filter is up to date.
@@ -498,7 +518,7 @@ fn preprocess_children<E, D>(traversal: &D,
                              mut propagated_hint: StoredRestyleHint,
                              parent_inherited_style_changed: bool)
     where E: TElement,
-          D: DomTraversal<E::ConcreteNode>
+          D: DomTraversal<E>
 {
     // Loop over all the children.
     for child in element.as_node().children() {
@@ -509,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() {
+        // If the child doesn't have pre-existing RestyleData and we don't have
-            Some(d) => d,
+        // any reason to create one, avoid the useless allocation and move on to
-            None => continue,
+        // 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() {

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() {
+    let result = match element.borrow_data() {
-            ElementData::Initial(None) |
+        // Note that we check for has_restyle here rather than has_current_styles,
-            ElementData::Restyle(..) => true,
+        // because we also want the traversal code to trigger if there's restyle
-            _ => false,
+        // 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
-    } else {
+        // don't bother distinguishing the two cases.
-        false
+        Some(d) => !d.has_styles() || d.has_restyle(),
-    }
+        None => true,
+    };
+    result
 }
 
 #[no_mangle]
@@ -852,8 +853,11 @@ 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();
+    // Don't generate a useless RestyleData if the element hasn't been styled.
-    if r.is_some() {
+    if !data.has_styles() {
+        return None;
+    }
+
     // Propagate the bit up the chain.
     let mut curr = element;
     while let Some(parent) = curr.parent_element() {
@@ -861,8 +865,9 @@ unsafe fn maybe_restyle<'a>(data: &'a mut AtomicRefMut<ElementData>, element: Ge
         if curr.has_dirty_descendants() { break; }
         curr.set_dirty_descendants();
     }
-    }
+
-    r
+    // 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);
+        GeckoRestyleDamage::empty()
-    }
+    };
 
-    let mut data = element.get_data().unwrap().borrow_mut();
+    debug!("Servo_TakeChangeHint: {:?}, damage={:?}", element, damage);
-    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();
-    }
-
-    debug!("Servo_GetChangeHint: {:?}, damage={:?}", element, damage);
     damage.as_change_hint()
 }
 
 #[no_mangle]
 pub extern "C" fn Servo_ResolveStyle(element: RawGeckoElementBorrowed,
-                                     raw_data: RawServoStyleSetBorrowed,
+                                     raw_data: RawServoStyleSetBorrowed)
-                                     consume: structs::ConsumeStyleBehavior)
                                      -> ServoComputedValuesStrong
 {
     let element = GeckoElement(element);
-    debug!("Servo_ResolveStyle: {:?}, consume={:?}", element, consume);
+    debug!("Servo_ResolveStyle: {:?}", element);
-
+    let data = unsafe { element.ensure_data() }.borrow_mut();
-    let mut data = unsafe { element.ensure_data() }.borrow_mut();
-    let per_doc_data = PerDocumentStyleData::from_ffi(raw_data).borrow();
 
     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();
+    data.styles().primary.values.clone().into_strong()
-
-    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()
 }
 
 #[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()
 }