Auto merge of #13934 - bholley:skip_text_nodes, r=emilio

Only traverse elements from style

This is part of the new incremental restyle architecture work. I have patches to element-ify things more, but want to get this in the tree first.

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

components/layout/construct.rs

@@ -1472,13 +1472,6 @@ impl<'a, ConcreteThreadSafeLayoutNode> PostorderNodeMutTraversal<ConcreteThreadS
             }
             Some(LayoutNodeType::Text) =>
                 (display::T::inline, float::T::none, position::T::static_),
-            Some(LayoutNodeType::Comment) |
-            Some(LayoutNodeType::ProcessingInstruction) |
-            Some(LayoutNodeType::DocumentType) |
-            Some(LayoutNodeType::DocumentFragment) |
-            Some(LayoutNodeType::Document) => {
-                (display::T::none, float::T::none, position::T::static_)
-            }
         };
 
         debug!("building flow for node: {:?} {:?} {:?} {:?}", display, float, positioning, node.type_id());
@@ -1615,12 +1608,7 @@ impl<ConcreteThreadSafeLayoutNode> NodeUtils for ConcreteThreadSafeLayoutNode
                                              where ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode {
     fn is_replaced_content(&self) -> bool {
         match self.type_id() {
-            Some(LayoutNodeType::Comment) |
-            Some(LayoutNodeType::ProcessingInstruction) |
             Some(LayoutNodeType::Text) |
-            Some(LayoutNodeType::DocumentType) |
-            Some(LayoutNodeType::DocumentFragment) |
-            Some(LayoutNodeType::Document) |
             Some(LayoutNodeType::Element(LayoutElementType::HTMLImageElement)) |
             Some(LayoutNodeType::Element(LayoutElementType::HTMLIFrameElement)) |
             Some(LayoutNodeType::Element(LayoutElementType::HTMLCanvasElement)) |

components/layout/query.rs

@@ -631,14 +631,14 @@ pub fn process_resolved_style_request<'a, N, C>(requested_node: N,
     where N: LayoutNode,
           C: StyleContext<'a>
 {
-    use style::traversal::ensure_node_styled;
+    use style::traversal::ensure_element_styled;
 
     // This node might have display: none, or it's style might be not up to
     // date, so we might need to do style recalc.
     //
     // FIXME(emilio): Is a bit shame we have to do this instead of in style.
     ensure_node_data_initialized(&requested_node);
-    ensure_node_styled(requested_node, style_context);
+    ensure_element_styled(requested_node.as_element().unwrap(), style_context);
 
     let layout_node = requested_node.to_threadsafe();
     let layout_node = match *pseudo {

components/layout/traversal.rs

@@ -16,10 +16,12 @@ use std::mem;
 use style::atomic_refcell::AtomicRefCell;
 use style::context::{LocalStyleContext, SharedStyleContext, StyleContext};
 use style::data::NodeData;
-use style::dom::{TNode, TRestyleDamage};
+use style::dom::TNode;
 use style::selector_impl::ServoSelectorImpl;
-use style::traversal::{DomTraversalContext, recalc_style_at, remove_from_bloom_filter};
+use style::traversal::{DomTraversalContext, put_thread_local_bloom_filter};
+use style::traversal::{recalc_style_at, remove_from_bloom_filter};
 use style::traversal::RestyleResult;
+use style::traversal::take_thread_local_bloom_filter;
 use util::opts;
 use wrapper::{LayoutNodeLayoutData, ThreadSafeLayoutNodeHelpers};
 
@@ -77,7 +79,35 @@ impl<'lc, N> DomTraversalContext<N> for RecalcStyleAndConstructFlows<'lc>
         // done by the HTML parser.
         node.initialize_data();
 
-        recalc_style_at::<_, _, Self>(&self.context, self.root, node)
+        if node.is_text_node() {
+            // FIXME(bholley): Stop doing this silly work to maintain broken bloom filter
+            // invariants.
+            //
+            // Longer version: The bloom filter is entirely busted for parallel traversal. Because
+            // parallel traversal is breadth-first, each sibling rejects the bloom filter set up
+            // by the previous sibling (which is valid for children, not siblings) and recreates
+            // it. Similarly, the fixup performed in the bottom-up traversal is useless, because
+            // threads perform flow construction up the parent chain until they find a parent with
+            // other unprocessed children, at which point they bail to the work queue and find a
+            // different node.
+            //
+            // Nevertheless, the remove_from_bloom_filter call at the end of flow construction
+            // asserts that the bloom filter is valid for the current node. This breaks when we
+            // stop calling recalc_style_at for text nodes, because the recursive chain of
+            // construct_flows_at calls is no longer necessarily rooted in a call that sets up the
+            // thread-local bloom filter for the leaf node.
+            //
+            // The bloom filter stuff is all going to be rewritten, so we just hackily duplicate
+            // the bloom filter manipulation from recalc_style_at to maintain invariants.
+            let parent = node.parent_node().unwrap().as_element();
+            let bf = take_thread_local_bloom_filter(parent, self.root, self.context.shared_context());
+            put_thread_local_bloom_filter(bf, &node.to_unsafe(), self.context.shared_context());
+
+            RestyleResult::Stop
+        } else {
+            let el = node.as_element().unwrap();
+            recalc_style_at::<_, _, Self>(&self.context, self.root, el)
+        }
     }
 
     fn process_postorder(&self, node: N) {
@@ -111,7 +141,7 @@ fn construct_flows_at<'a, N: LayoutNode>(context: &'a LayoutContext<'a>, root: O
         // Always reconstruct if incremental layout is turned off.
         let nonincremental_layout = opts::get().nonincremental_layout;
         if nonincremental_layout || node.has_dirty_descendants() ||
-           node.restyle_damage() != N::ConcreteRestyleDamage::empty() {
+           tnode.restyle_damage() != RestyleDamage::empty() {
             let mut flow_constructor = FlowConstructor::new(context);
             if nonincremental_layout || !flow_constructor.repair_if_possible(&tnode) {
                 flow_constructor.process(&tnode);
@@ -121,9 +151,7 @@ fn construct_flows_at<'a, N: LayoutNode>(context: &'a LayoutContext<'a>, root: O
             }
         }
 
-        // Reset the layout damage in this node. It's been propagated to the
+        tnode.clear_restyle_damage();
-        // flow by the flow constructor.
-        tnode.set_restyle_damage(RestyleDamage::empty());
     }
 
     unsafe { node.clear_dirty_bits(); }

components/script/dom/node.rs

@@ -2654,20 +2654,11 @@ impl Into<LayoutNodeType> for NodeTypeId {
     #[inline(always)]
     fn into(self) -> LayoutNodeType {
         match self {
-            NodeTypeId::CharacterData(CharacterDataTypeId::Comment) =>
-                LayoutNodeType::Comment,
-            NodeTypeId::Document(..) =>
-                LayoutNodeType::Document,
-            NodeTypeId::DocumentFragment =>
-                LayoutNodeType::DocumentFragment,
-            NodeTypeId::DocumentType =>
-                LayoutNodeType::DocumentType,
             NodeTypeId::Element(e) =>
                 LayoutNodeType::Element(e.into()),
-            NodeTypeId::CharacterData(CharacterDataTypeId::ProcessingInstruction) =>
-                LayoutNodeType::ProcessingInstruction,
             NodeTypeId::CharacterData(CharacterDataTypeId::Text) =>
                 LayoutNodeType::Text,
+            x => unreachable!("Layout should not traverse nodes of type {:?}", x),
         }
     }
 }

components/script/layout_wrapper.rs

@@ -133,7 +133,6 @@ impl<'ln> NodeInfo for ServoLayoutNode<'ln> {
 impl<'ln> TNode for ServoLayoutNode<'ln> {
     type ConcreteElement = ServoLayoutElement<'ln>;
     type ConcreteDocument = ServoLayoutDocument<'ln>;
-    type ConcreteRestyleDamage = RestyleDamage;
     type ConcreteChildrenIterator = ServoChildrenIterator<'ln>;
 
     fn to_unsafe(&self) -> UnsafeNode {
@@ -236,41 +235,12 @@ impl<'ln> TNode for ServoLayoutNode<'ln> {
         debug_assert!(self.is_text_node());
         let mut data = self.get_partial_layout_data().unwrap().borrow_mut();
         data.style_data.style_text_node(style);
-        if self.has_changed() {
-            data.restyle_damage = RestyleDamage::rebuild_and_reflow();
-        } else {
-            // FIXME(bholley): This is necessary to make it correct to use restyle
-            // damage in construct_flows_at to determine whether to reconstruct
-            // text nodes. Without it, we fail cascade-import-dynamic-002.htm.
-            //
-            // Long-term, We should teach layout how to correctly propagate
-            // style changes from elements to child text nodes so that we don't
-            // need to do this explicitly here. This will likely all be rolled
-            // into a patch where we stop styling text nodes from the style
-            // system and instead generate the styles on the fly during frame
-            // construction / repair.
-            let parent = self.parent_node().unwrap();
-            let parent_data = parent.get_partial_layout_data().unwrap().borrow();
-            data.restyle_damage = parent_data.restyle_damage;
-        }
     }
 
     fn borrow_data(&self) -> Option<AtomicRef<NodeData>> {
         self.get_style_data().map(|d| d.borrow())
     }
 
-    fn restyle_damage(self) -> RestyleDamage {
-        self.get_partial_layout_data().unwrap().borrow().restyle_damage
-    }
-
-    fn set_restyle_damage(self, damage: RestyleDamage) {
-        let mut damage = damage;
-        if self.has_changed() {
-            damage = RestyleDamage::rebuild_and_reflow();
-        }
-        self.get_partial_layout_data().unwrap().borrow_mut().restyle_damage = damage;
-    }
-
     fn parent_node(&self) -> Option<ServoLayoutNode<'ln>> {
         unsafe {
             self.node.parent_node_ref().map(|node| self.new_with_this_lifetime(&node))
@@ -300,14 +270,6 @@ impl<'ln> TNode for ServoLayoutNode<'ln> {
             self.node.next_sibling_ref().map(|node| self.new_with_this_lifetime(&node))
         }
     }
-
-    #[inline]
-    fn existing_style_for_restyle_damage<'a>(&'a self,
-                                             current_cv: Option<&'a Arc<ComputedValues>>,
-                                             _pseudo_element: Option<&PseudoElement>)
-                                             -> Option<&'a Arc<ComputedValues>> {
-        current_cv
-    }
 }
 
 pub struct ServoChildrenIterator<'a> {
@@ -507,6 +469,7 @@ impl<'le> PresentationalHintsSynthetizer for ServoLayoutElement<'le> {
 impl<'le> TElement for ServoLayoutElement<'le> {
     type ConcreteNode = ServoLayoutNode<'le>;
     type ConcreteDocument = ServoLayoutDocument<'le>;
+    type ConcreteRestyleDamage = RestyleDamage;
 
     fn as_node(&self) -> ServoLayoutNode<'le> {
         ServoLayoutNode::from_layout_js(self.element.upcast())
@@ -531,6 +494,19 @@ impl<'le> TElement for ServoLayoutElement<'le> {
     fn attr_equals(&self, namespace: &Namespace, attr: &Atom, val: &Atom) -> bool {
         self.get_attr(namespace, attr).map_or(false, |x| x == val)
     }
+
+    fn set_restyle_damage(self, damage: RestyleDamage) {
+        let node = self.as_node();
+        node.get_partial_layout_data().unwrap().borrow_mut().restyle_damage = damage;
+    }
+
+    #[inline]
+    fn existing_style_for_restyle_damage<'a>(&'a self,
+                                             current_cv: Option<&'a Arc<ComputedValues>>,
+                                             _pseudo_element: Option<&PseudoElement>)
+                                             -> Option<&'a Arc<ComputedValues>> {
+        current_cv
+    }
 }
 
 impl<'le> PartialEq for ServoLayoutElement<'le> {
@@ -783,15 +759,19 @@ impl<'ln> ServoThreadSafeLayoutNode<'ln> {
 
 impl<'ln> NodeInfo for ServoThreadSafeLayoutNode<'ln> {
     fn is_element(&self) -> bool {
-        if let Some(LayoutNodeType::Element(_)) = self.type_id() { true } else { false }
+        self.pseudo == PseudoElementType::Normal && self.node.is_element()
     }
 
     fn is_text_node(&self) -> bool {
-        if let Some(LayoutNodeType::Text) = self.type_id() { true } else { false }
+        // It's unlikely that text nodes will ever be used to implement a
+        // pseudo-element, but the type system doesn't really enforce that,
+        // so we check to be safe.
+        self.pseudo == PseudoElementType::Normal && self.node.is_text_node()
     }
 
     fn needs_layout(&self) -> bool {
-        self.pseudo != PseudoElementType::Normal || self.is_element() || self.is_text_node()
+        self.pseudo != PseudoElementType::Normal ||
+        self.node.is_element() || self.node.is_text_node()
     }
 }
 
@@ -824,6 +804,21 @@ impl<'ln> ThreadSafeLayoutNode for ServoThreadSafeLayoutNode<'ln> {
         self.node.type_id()
     }
 
+    fn style_for_text_node(&self) -> Arc<ComputedValues> {
+        // Text nodes get a copy of the parent style. Inheriting all non-
+        // inherited properties into the text node is odd from a CSS
+        // perspective, but makes fragment construction easier (by making
+        // properties like vertical-align on fragments have values that
+        // match the parent element). This is an implementation detail of
+        // Servo layout that is not central to how fragment construction
+        // works, but would be difficult to change. (Text node style is
+        // also not visible to script.)
+        debug_assert!(self.is_text_node());
+        let parent = self.node.parent_node().unwrap();
+        let parent_data = parent.get_style_data().unwrap().borrow();
+        parent_data.current_styles().primary.clone()
+    }
+
     fn debug_id(self) -> usize {
         self.node.debug_id()
     }
@@ -876,11 +871,22 @@ impl<'ln> ThreadSafeLayoutNode for ServoThreadSafeLayoutNode<'ln> {
     }
 
     fn restyle_damage(self) -> RestyleDamage {
-        self.node.restyle_damage()
+        if self.node.has_changed() {
+            RestyleDamage::rebuild_and_reflow()
+        } else if self.is_text_node() {
+            let parent = self.node.parent_node().unwrap();
+            let parent_data = parent.get_partial_layout_data().unwrap().borrow();
+            parent_data.restyle_damage
+        } else {
+            self.node.get_partial_layout_data().unwrap().borrow().restyle_damage
+        }
     }
 
-    fn set_restyle_damage(self, damage: RestyleDamage) {
+    fn clear_restyle_damage(self) {
-        self.node.set_restyle_damage(damage)
+        if self.is_element() {
+            let mut data = self.node.get_partial_layout_data().unwrap().borrow_mut();
+            data.restyle_damage = RestyleDamage::empty();
+        }
     }
 
     fn can_be_fragmented(&self) -> bool {

components/script_layout_interface/lib.rs

@@ -107,12 +107,7 @@ impl DomParallelInfo {
 
 #[derive(Copy, Clone, PartialEq, Eq, Debug)]
 pub enum LayoutNodeType {
-    Comment,
-    Document,
-    DocumentFragment,
-    DocumentType,
     Element(LayoutElementType),
-    ProcessingInstruction,
     Text,
 }
 

components/script_layout_interface/wrapper_traits.rs

@@ -165,6 +165,16 @@ pub trait ThreadSafeLayoutNode: Clone + Copy + NodeInfo + PartialEq + Sized {
     /// `type_id` does.
     fn type_id_without_excluding_pseudo_elements(&self) -> LayoutNodeType;
 
+    /// Returns the style for a text node. This is computed on the fly from the
+    /// parent style to avoid traversing text nodes in the style system.
+    ///
+    /// Note that this does require accessing the parent, which this interface
+    /// technically forbids. But accessing the parent is only unsafe insofar as
+    /// it can be used to reach siblings and cousins. A simple immutable borrow
+    /// of the parent data is fine, since the bottom-up traversal will not process
+    /// the parent until all the children have been processed.
+    fn style_for_text_node(&self) -> Arc<ServoComputedValues>;
+
     #[inline]
     fn is_element_or_elements_pseudo(&self) -> bool {
         match self.type_id_without_excluding_pseudo_elements() {
@@ -187,7 +197,8 @@ pub trait ThreadSafeLayoutNode: Clone + Copy + NodeInfo + PartialEq + Sized {
 
     #[inline]
     fn get_before_pseudo(&self) -> Option<Self> {
-        if self.get_style_data()
+        if self.is_element() &&
+           self.get_style_data()
                .unwrap()
                .borrow()
                .current_styles().pseudos
@@ -200,7 +211,8 @@ pub trait ThreadSafeLayoutNode: Clone + Copy + NodeInfo + PartialEq + Sized {
 
     #[inline]
     fn get_after_pseudo(&self) -> Option<Self> {
-        if self.get_style_data()
+        if self.is_element() &&
+           self.get_style_data()
                .unwrap()
                .borrow()
                .current_styles().pseudos
@@ -248,8 +260,11 @@ pub trait ThreadSafeLayoutNode: Clone + Copy + NodeInfo + PartialEq + Sized {
     fn style(&self, context: &SharedStyleContext) -> Arc<ServoComputedValues> {
         match self.get_pseudo_element_type() {
             PseudoElementType::Normal => {
-                self.get_style_data().unwrap().borrow()
+                match self.type_id().unwrap() {
-                    .current_styles().primary.clone()
+                    LayoutNodeType::Text => self.style_for_text_node(),
+                    LayoutNodeType::Element(_) => self.get_style_data().unwrap().borrow()
+                                                      .current_styles().primary.clone(),
+                }
             },
             other => {
                 // Precompute non-eagerly-cascaded pseudo-element styles if not
@@ -308,6 +323,11 @@ pub trait ThreadSafeLayoutNode: Clone + Copy + NodeInfo + PartialEq + Sized {
     /// element style is precomputed, not from general layout itself.
     #[inline]
     fn resolved_style(&self) -> Arc<ServoComputedValues> {
+        // FIXME(bholley): This should move to Element and lose the text node check.
+        if self.is_text_node() {
+            return self.style_for_text_node();
+        }
+
         let data = self.get_style_data().unwrap().borrow();
         match self.get_pseudo_element_type() {
             PseudoElementType::Normal
@@ -319,6 +339,11 @@ pub trait ThreadSafeLayoutNode: Clone + Copy + NodeInfo + PartialEq + Sized {
 
     #[inline]
     fn selected_style(&self, _context: &SharedStyleContext) -> Arc<ServoComputedValues> {
+        // FIXME(bholley): This should move to Element and lose the text node check.
+        if self.is_text_node() {
+            return self.style_for_text_node();
+        }
+
         let data = self.get_style_data().unwrap().borrow();
         data.current_styles().pseudos
             .get(&PseudoElement::Selection)
@@ -330,7 +355,7 @@ pub trait ThreadSafeLayoutNode: Clone + Copy + NodeInfo + PartialEq + Sized {
 
     fn restyle_damage(self) -> RestyleDamage;
 
-    fn set_restyle_damage(self, damage: RestyleDamage);
+    fn clear_restyle_damage(self);
 
     /// Returns true if this node contributes content. This is used in the implementation of
     /// `empty_cells` per CSS 2.1 § 17.6.1.1.

components/style/dom.rs

@@ -106,7 +106,6 @@ impl<T, I> Iterator for LayoutIterator<T> where T: Iterator<Item=I>, I: NodeInfo
 pub trait TNode : Sized + Copy + Clone + NodeInfo {
     type ConcreteElement: TElement<ConcreteNode = Self, ConcreteDocument = Self::ConcreteDocument>;
     type ConcreteDocument: TDocument<ConcreteNode = Self, ConcreteElement = Self::ConcreteElement>;
-    type ConcreteRestyleDamage: TRestyleDamage;
     type ConcreteChildrenIterator: Iterator<Item = Self>;
 
     fn to_unsafe(&self) -> UnsafeNode;
@@ -215,12 +214,6 @@ pub trait TNode : Sized + Copy + Clone + NodeInfo {
     /// Immutable borrows the NodeData.
     fn borrow_data(&self) -> Option<AtomicRef<NodeData>>;
 
-    /// Get the description of how to account for recent style changes.
-    fn restyle_damage(self) -> Self::ConcreteRestyleDamage;
-
-    /// Set the restyle damage field.
-    fn set_restyle_damage(self, damage: Self::ConcreteRestyleDamage);
-
     fn parent_node(&self) -> Option<Self>;
 
     fn first_child(&self) -> Option<Self>;
@@ -230,14 +223,6 @@ pub trait TNode : Sized + Copy + Clone + NodeInfo {
     fn prev_sibling(&self) -> Option<Self>;
 
     fn next_sibling(&self) -> Option<Self>;
-
-    /// XXX: It's a bit unfortunate we need to pass the current computed values
-    /// as an argument here, but otherwise Servo would crash due to double
-    /// borrows to return it.
-    fn existing_style_for_restyle_damage<'a>(&'a self,
-                                             current_computed_values: Option<&'a Arc<ComputedValues>>,
-                                             pseudo: Option<&PseudoElement>)
-        -> Option<&'a <Self::ConcreteRestyleDamage as TRestyleDamage>::PreExistingComputedValues>;
 }
 
 pub trait TDocument : Sized + Copy + Clone {
@@ -263,6 +248,7 @@ pub trait PresentationalHintsSynthetizer {
 pub trait TElement : PartialEq + Debug + Sized + Copy + Clone + ElementExt + PresentationalHintsSynthetizer {
     type ConcreteNode: TNode<ConcreteElement = Self, ConcreteDocument = Self::ConcreteDocument>;
     type ConcreteDocument: TDocument<ConcreteNode = Self::ConcreteNode, ConcreteElement = Self>;
+    type ConcreteRestyleDamage: TRestyleDamage;
 
     fn as_node(&self) -> Self::ConcreteNode;
 
@@ -273,6 +259,17 @@ pub trait TElement : PartialEq + Debug + Sized + Copy + Clone + ElementExt + Pre
     fn has_attr(&self, namespace: &Namespace, attr: &Atom) -> bool;
     fn attr_equals(&self, namespace: &Namespace, attr: &Atom, value: &Atom) -> bool;
 
+    /// Set the restyle damage field.
+    fn set_restyle_damage(self, damage: Self::ConcreteRestyleDamage);
+
+    /// XXX: It's a bit unfortunate we need to pass the current computed values
+    /// as an argument here, but otherwise Servo would crash due to double
+    /// borrows to return it.
+    fn existing_style_for_restyle_damage<'a>(&'a self,
+                                             current_computed_values: Option<&'a Arc<ComputedValues>>,
+                                             pseudo: Option<&PseudoElement>)
+        -> Option<&'a <Self::ConcreteRestyleDamage as TRestyleDamage> ::PreExistingComputedValues>;
+
     /// Properly marks nodes as dirty in response to restyle hints.
     fn note_restyle_hint<C: DomTraversalContext<Self::ConcreteNode>>(&self, hint: RestyleHint) {
         // Bail early if there's no restyling to do.

components/style/gecko/traversal.rs

@@ -5,7 +5,7 @@
 use atomic_refcell::AtomicRefCell;
 use context::{LocalStyleContext, SharedStyleContext, StyleContext};
 use data::NodeData;
-use dom::OpaqueNode;
+use dom::{NodeInfo, OpaqueNode, TNode};
 use gecko::context::StandaloneStyleContext;
 use gecko::wrapper::GeckoNode;
 use std::mem;
@@ -31,7 +31,14 @@ impl<'lc, 'ln> DomTraversalContext<GeckoNode<'ln>> for RecalcStyleOnly<'lc> {
     }
 
     fn process_preorder(&self, node: GeckoNode<'ln>) -> RestyleResult {
-        recalc_style_at::<_, _, Self>(&self.context, self.root, node)
+        if node.is_text_node() {
+            // Text nodes don't have children, so save the traversal algorithm
+            // the trouble of iterating the children.
+            RestyleResult::Stop
+        } else {
+            let el = node.as_element().unwrap();
+            recalc_style_at::<_, _, Self>(&self.context, self.root, el)
+        }
     }
 
     fn process_postorder(&self, _: GeckoNode<'ln>) {

components/style/gecko/wrapper.rs

@@ -160,7 +160,6 @@ impl<'ln> NodeInfo for GeckoNode<'ln> {
 impl<'ln> TNode for GeckoNode<'ln> {
     type ConcreteDocument = GeckoDocument<'ln>;
     type ConcreteElement = GeckoElement<'ln>;
-    type ConcreteRestyleDamage = GeckoRestyleDamage;
     type ConcreteChildrenIterator = GeckoChildrenIterator<'ln>;
 
     fn to_unsafe(&self) -> UnsafeNode {
@@ -273,19 +272,6 @@ impl<'ln> TNode for GeckoNode<'ln> {
         self.get_node_data().map(|x| x.borrow())
     }
 
-    fn restyle_damage(self) -> Self::ConcreteRestyleDamage {
-        // Not called from style, only for layout.
-        unimplemented!();
-    }
-
-    fn set_restyle_damage(self, damage: Self::ConcreteRestyleDamage) {
-        // FIXME(bholley): Gecko currently relies on the dirty bit being set to
-        // drive the post-traversal. This will go away soon.
-        unsafe { self.set_flags(NODE_IS_DIRTY_FOR_SERVO as u32) }
-
-        unsafe { Gecko_StoreStyleDifference(self.0, damage.0) }
-    }
-
     fn parent_node(&self) -> Option<GeckoNode<'ln>> {
         unsafe { self.0.mParent.as_ref().map(GeckoNode) }
     }
@@ -306,23 +292,6 @@ impl<'ln> TNode for GeckoNode<'ln> {
         unsafe { self.0.mNextSibling.as_ref().map(GeckoNode::from_content) }
     }
 
-    fn existing_style_for_restyle_damage<'a>(&'a self,
-                                             current_cv: Option<&'a Arc<ComputedValues>>,
-                                             pseudo: Option<&PseudoElement>)
-                                             -> Option<&'a nsStyleContext> {
-        if current_cv.is_none() {
-            // Don't bother in doing an ffi call to get null back.
-            return None;
-        }
-
-        unsafe {
-            let atom_ptr = pseudo.map(|p| p.as_atom().as_ptr())
-                                 .unwrap_or(ptr::null_mut());
-            let context_ptr = Gecko_GetStyleContext(self.0, atom_ptr);
-            context_ptr.as_ref()
-        }
-    }
-
     fn needs_dirty_on_viewport_size_changed(&self) -> bool {
         // Gecko's node doesn't have the DIRTY_ON_VIEWPORT_SIZE_CHANGE flag,
         // so we force them to be dirtied on viewport size change, regardless if
@@ -429,6 +398,7 @@ lazy_static! {
 impl<'le> TElement for GeckoElement<'le> {
     type ConcreteNode = GeckoNode<'le>;
     type ConcreteDocument = GeckoDocument<'le>;
+    type ConcreteRestyleDamage = GeckoRestyleDamage;
 
     fn as_node(&self) -> Self::ConcreteNode {
         unsafe { GeckoNode(&*(self.0 as *const _ as *const RawGeckoNode)) }
@@ -464,6 +434,31 @@ impl<'le> TElement for GeckoElement<'le> {
                                        /* ignoreCase = */ false)
         }
     }
+
+    fn set_restyle_damage(self, damage: GeckoRestyleDamage) {
+        // FIXME(bholley): Gecko currently relies on the dirty bit being set to
+        // drive the post-traversal. This will go away soon.
+        unsafe { self.as_node().set_flags(NODE_IS_DIRTY_FOR_SERVO as u32) }
+
+        unsafe { Gecko_StoreStyleDifference(self.as_node().0, damage.0) }
+    }
+
+    fn existing_style_for_restyle_damage<'a>(&'a self,
+                                             current_cv: Option<&'a Arc<ComputedValues>>,
+                                             pseudo: Option<&PseudoElement>)
+                                             -> Option<&'a nsStyleContext> {
+        if current_cv.is_none() {
+            // Don't bother in doing an ffi call to get null back.
+            return None;
+        }
+
+        unsafe {
+            let atom_ptr = pseudo.map(|p| p.as_atom().as_ptr())
+                                 .unwrap_or(ptr::null_mut());
+            let context_ptr = Gecko_GetStyleContext(self.as_node().0, atom_ptr);
+            context_ptr.as_ref()
+        }
+    }
 }
 
 impl<'le> PartialEq for GeckoElement<'le> {

components/style/matching.rs

@@ -12,12 +12,12 @@ use cache::{LRUCache, SimpleHashCache};
 use cascade_info::CascadeInfo;
 use context::{SharedStyleContext, StyleContext};
 use data::{NodeStyles, PseudoStyles};
-use dom::{NodeInfo, TElement, TNode, TRestyleDamage, UnsafeNode};
+use dom::{TElement, TNode, TRestyleDamage, UnsafeNode};
 use properties::{CascadeFlags, ComputedValues, SHAREABLE, cascade};
 use properties::longhands::display::computed_value as display;
 use selector_impl::{PseudoElement, TheSelectorImpl};
 use selector_matching::{ApplicableDeclarationBlock, Stylist};
-use selectors::{Element, MatchAttr};
+use selectors::MatchAttr;
 use selectors::bloom::BloomFilter;
 use selectors::matching::{AFFECTED_BY_PSEUDO_ELEMENTS, MatchingReason, StyleRelations};
 use sink::ForgetfulSink;
@@ -497,7 +497,7 @@ struct CascadeBooleans {
     animate: bool,
 }
 
-trait PrivateMatchMethods: TNode {
+trait PrivateMatchMethods: TElement {
     /// Actually cascades style for a node or a pseudo-element of a node.
     ///
     /// Note that animations only apply to nodes or ::before or ::after
@@ -521,7 +521,7 @@ trait PrivateMatchMethods: TNode {
         // and the cache would not be effective anyway.
         // This also works around the test failures at
         // https://github.com/servo/servo/pull/13459#issuecomment-250717584
-        let has_style_attribute = self.as_element().map_or(false, |e| e.style_attribute().is_some());
+        let has_style_attribute = self.style_attribute().is_some();
         cacheable = cacheable && !has_style_attribute;
 
         let mut cascade_info = CascadeInfo::new();
@@ -556,7 +556,7 @@ trait PrivateMatchMethods: TNode {
                         cascade_flags)
             }
         };
-        cascade_info.finish(self);
+        cascade_info.finish(&self.as_node());
 
         cacheable = cacheable && is_cacheable;
 
@@ -564,7 +564,7 @@ trait PrivateMatchMethods: TNode {
 
         if booleans.animate {
             let new_animations_sender = &context.local_context().new_animations_sender;
-            let this_opaque = self.opaque();
+            let this_opaque = self.as_node().opaque();
             // Trigger any present animations if necessary.
             let mut animations_started = animation::maybe_start_animations(
                 &shared_context,
@@ -579,7 +579,7 @@ trait PrivateMatchMethods: TNode {
                     animation::start_transitions_if_applicable(
                         new_animations_sender,
                         this_opaque,
-                        self.to_unsafe(),
+                        self.as_node().to_unsafe(),
                         &**style,
                         &mut this_style,
                         &shared_context.timer);
@@ -601,7 +601,7 @@ trait PrivateMatchMethods: TNode {
                                      context: &SharedStyleContext,
                                      style: &mut Arc<ComputedValues>) -> bool {
         // Finish any expired transitions.
-        let this_opaque = self.opaque();
+        let this_opaque = self.as_node().opaque();
         let had_animations_to_expire =
             animation::complete_expired_transitions(this_opaque, style, context);
 
@@ -636,18 +636,11 @@ trait PrivateMatchMethods: TNode {
 
         had_animations_to_expire || had_running_animations
     }
-}
 
-impl<N: TNode> PrivateMatchMethods for N {}
-
-trait PrivateElementMatchMethods: TElement {
     fn share_style_with_candidate_if_possible(&self,
-                                              parent_node: Self::ConcreteNode,
                                               shared_context: &SharedStyleContext,
                                               candidate: &mut StyleSharingCandidate)
                                               -> Result<Arc<ComputedValues>, CacheMiss> {
-        debug_assert!(parent_node.is_element());
-
         let candidate_element = unsafe {
             Self::ConcreteNode::from_unsafe(&candidate.node).as_element().unwrap()
         };
@@ -657,9 +650,9 @@ trait PrivateElementMatchMethods: TElement {
     }
 }
 
-impl<E: TElement> PrivateElementMatchMethods for E {}
+impl<E: TElement> PrivateMatchMethods for E {}
 
-pub trait ElementMatchMethods : TElement {
+pub trait MatchMethods : TElement {
     fn match_element(&self,
                      stylist: &Stylist,
                      parent_bf: Option<&BloomFilter>,
@@ -703,9 +696,8 @@ pub trait ElementMatchMethods : TElement {
     unsafe fn share_style_if_possible(&self,
                                       style_sharing_candidate_cache:
                                         &mut StyleSharingCandidateCache,
-                                      shared_context: &SharedStyleContext,
+                                      shared_context: &SharedStyleContext)
-                                      parent: Option<Self::ConcreteNode>)
+                                      -> StyleSharingResult<Self::ConcreteRestyleDamage> {
-                                      -> StyleSharingResult<<Self::ConcreteNode as TNode>::ConcreteRestyleDamage> {
         if opts::get().disable_share_style_cache {
             return StyleSharingResult::CannotShare
         }
@@ -718,16 +710,9 @@ pub trait ElementMatchMethods : TElement {
             return StyleSharingResult::CannotShare
         }
 
-        let parent = match parent {
-            Some(parent) if parent.is_element() => parent,
-            _ => return StyleSharingResult::CannotShare,
-        };
-
         let mut should_clear_cache = false;
         for (i, &mut (ref mut candidate, ())) in style_sharing_candidate_cache.iter_mut().enumerate() {
-            let sharing_result = self.share_style_with_candidate_if_possible(parent,
+            let sharing_result = self.share_style_with_candidate_if_possible(shared_context, candidate);
-                                                                             shared_context,
-                                                                             candidate);
             match sharing_result {
                 Ok(shared_style) => {
                     // Yay, cache hit. Share the style.
@@ -739,14 +724,12 @@ pub trait ElementMatchMethods : TElement {
                     // can decide more easily if it knows that it's a child of
                     // replaced content, or similar stuff!
                     let damage =
-                        match node.existing_style_for_restyle_damage(data.previous_styles().map(|x| &x.primary), None) {
+                        match self.existing_style_for_restyle_damage(data.previous_styles().map(|x| &x.primary), None) {
                             Some(ref source) => {
-                                <<Self as TElement>::ConcreteNode as TNode>
+                                Self::ConcreteRestyleDamage::compute(source, &shared_style)
-                                ::ConcreteRestyleDamage::compute(source, &shared_style)
                             }
                             None => {
-                                <<Self as TElement>::ConcreteNode as TNode>
+                                Self::ConcreteRestyleDamage::rebuild_and_reflow()
-                                ::ConcreteRestyleDamage::rebuild_and_reflow()
                             }
                         };
 
@@ -788,11 +771,7 @@ pub trait ElementMatchMethods : TElement {
 
         StyleSharingResult::CannotShare
     }
-}
 
-impl<E: TElement> ElementMatchMethods for E {}
-
-pub trait MatchMethods : TNode {
     // The below two functions are copy+paste because I can't figure out how to
     // write a function which takes a generic function. I don't think it can
     // be done.
@@ -816,29 +795,23 @@ pub trait MatchMethods : TNode {
     /// Therefore, each node must have its matching selectors inserted _after_
     /// its own selector matching and _before_ its children start.
     fn insert_into_bloom_filter(&self, bf: &mut BloomFilter) {
-        // Only elements are interesting.
+        bf.insert(&*self.get_local_name());
-        if let Some(element) = self.as_element() {
+        bf.insert(&*self.get_namespace());
-            bf.insert(&*element.get_local_name());
+        self.get_id().map(|id| bf.insert(&id));
-            bf.insert(&*element.get_namespace());
-            element.get_id().map(|id| bf.insert(&id));
 
-            // TODO: case-sensitivity depends on the document type and quirks mode
+        // TODO: case-sensitivity depends on the document type and quirks mode
-            element.each_class(|class| bf.insert(class));
+        self.each_class(|class| bf.insert(class));
-        }
     }
 
     /// After all the children are done css selector matching, this must be
     /// called to reset the bloom filter after an `insert`.
     fn remove_from_bloom_filter(&self, bf: &mut BloomFilter) {
-        // Only elements are interesting.
+        bf.remove(&*self.get_local_name());
-        if let Some(element) = self.as_element() {
+        bf.remove(&*self.get_namespace());
-            bf.remove(&*element.get_local_name());
+        self.get_id().map(|id| bf.remove(&id));
-            bf.remove(&*element.get_namespace());
-            element.get_id().map(|id| bf.remove(&id));
 
-            // TODO: case-sensitivity depends on the document type and quirks mode
+        // TODO: case-sensitivity depends on the document type and quirks mode
-            element.each_class(|class| bf.remove(class));
+        self.each_class(|class| bf.remove(class));
-        }
     }
 
     fn compute_restyle_damage(&self,
@@ -890,23 +863,12 @@ pub trait MatchMethods : TNode {
         where Ctx: StyleContext<'a>
     {
         // Get our parent's style.
-        let parent_data = parent.as_ref().map(|x| x.borrow_data().unwrap());
+        let parent_as_node = parent.map(|x| x.as_node());
+        let parent_data = parent_as_node.as_ref().map(|x| x.borrow_data().unwrap());
         let parent_style = parent_data.as_ref().map(|x| &x.current_styles().primary);
 
-        // In the case we're styling a text node, we don't need to compute the
+        let node = self.as_node();
-        // restyle damage, since it's a subset of the restyle damage of the
+        let mut data = node.begin_styling();
-        // parent.
-        //
-        // In Gecko, we're done, we don't need anything else from text nodes.
-        //
-        // In Servo, this is also true, since text nodes generate UnscannedText
-        // fragments, which aren't repairable by incremental layout.
-        if self.is_text_node() {
-            self.style_text_node(ComputedValues::style_for_child_text_node(parent_style.clone().unwrap()));
-            return RestyleResult::Continue;
-        }
-
-        let mut data = self.begin_styling();
         let mut new_styles;
 
         let mut applicable_declarations_cache =
@@ -944,8 +906,8 @@ pub trait MatchMethods : TNode {
                                                         context, applicable_declarations,
                                                         &mut applicable_declarations_cache);
 
-            self.set_can_be_fragmented(parent.map_or(false, |p| {
+            self.as_node().set_can_be_fragmented(parent.map_or(false, |p| {
-                p.can_be_fragmented() ||
+                p.as_node().can_be_fragmented() ||
                 parent_style.unwrap().is_multicol()
             }));
 
@@ -1006,7 +968,7 @@ pub trait MatchMethods : TNode {
         let no_damage = Self::ConcreteRestyleDamage::empty();
 
         debug_assert!(new_pseudos.is_empty());
-        <Self::ConcreteElement as MatchAttr>::Impl::each_eagerly_cascaded_pseudo_element(|pseudo| {
+        <Self as MatchAttr>::Impl::each_eagerly_cascaded_pseudo_element(|pseudo| {
             let applicable_declarations_for_this_pseudo =
                 applicable_declarations.per_pseudo.get(&pseudo).unwrap();
 
@@ -1018,8 +980,7 @@ pub trait MatchMethods : TNode {
 
             if has_declarations {
                 // We have declarations, so we need to cascade. Compute parameters.
-                let animate = <Self::ConcreteElement as MatchAttr>::Impl
+                let animate = <Self as MatchAttr>::Impl::pseudo_is_before_or_after(&pseudo);
-                                ::pseudo_is_before_or_after(&pseudo);
                 let cacheable = if animate && old_pseudo_style.is_some() {
                     // Update animations before the cascade. This may modify
                     // the value of old_pseudo_style.
@@ -1064,4 +1025,4 @@ pub trait MatchMethods : TNode {
     }
 }
 
-impl<N: TNode> MatchMethods for N {}
+impl<E: TElement> MatchMethods for E {}

components/style/properties/properties.mako.rs

@@ -1084,18 +1084,6 @@ impl ComputedValues {
         }
     }
 
-    pub fn style_for_child_text_node(parent: &Arc<Self>) -> Arc<Self> {
-        // Text nodes get a copy of the parent style. Inheriting all non-
-        // inherited properties into the text node is odd from a CSS
-        // perspective, but makes fragment construction easier (by making
-        // properties like vertical-align on fragments have values that
-        // match the parent element). This is an implementation detail of
-        // Servo layout that is not central to how fragment construction
-        // works, but would be difficult to change. (Text node style is
-        // also not visible to script.)
-        parent.clone()
-    }
-
     pub fn initial_values() -> &'static Self { &*INITIAL_SERVO_VALUES }
 
     #[inline]

components/style/traversal.rs

@@ -7,8 +7,8 @@
 use atomic_refcell::AtomicRefCell;
 use context::{LocalStyleContext, SharedStyleContext, StyleContext};
 use data::NodeData;
-use dom::{OpaqueNode, StylingMode, TNode, UnsafeNode};
+use dom::{NodeInfo, OpaqueNode, StylingMode, TElement, TNode, UnsafeNode};
-use matching::{ApplicableDeclarations, ElementMatchMethods, MatchMethods, StyleSharingResult};
+use matching::{ApplicableDeclarations, MatchMethods, StyleSharingResult};
 use selectors::bloom::BloomFilter;
 use selectors::matching::StyleRelations;
 use std::cell::RefCell;
@@ -62,13 +62,13 @@ thread_local!(
 ///
 /// If one does not exist, a new one will be made for you. If it is out of date,
 /// it will be cleared and reused.
-fn take_thread_local_bloom_filter<N>(parent_node: Option<N>,
+pub fn take_thread_local_bloom_filter<E>(parent_element: Option<E>,
-                                     root: OpaqueNode,
+                                         root: OpaqueNode,
-                                     context: &SharedStyleContext)
+                                         context: &SharedStyleContext)
-                                     -> Box<BloomFilter>
+                                         -> Box<BloomFilter>
-                                     where N: TNode {
+                                         where E: TElement {
     STYLE_BLOOM.with(|style_bloom| {
-        match (parent_node, style_bloom.borrow_mut().take()) {
+        match (parent_element, style_bloom.borrow_mut().take()) {
             // Root node. Needs new bloom filter.
             (None,     _  ) => {
                 debug!("[{}] No parent, but new bloom filter!", tid());
@@ -82,7 +82,7 @@ fn take_thread_local_bloom_filter<N>(parent_node: Option<N>,
             }
             // Found cached bloom filter.
             (Some(parent), Some((mut bloom_filter, old_node, old_generation))) => {
-                if old_node == parent.to_unsafe() &&
+                if old_node == parent.as_node().to_unsafe() &&
                     old_generation == context.generation {
                     // Hey, the cached parent is our parent! We can reuse the bloom filter.
                     debug!("[{}] Parent matches (={}). Reusing bloom filter.", tid(), old_node.0);
@@ -98,8 +98,8 @@ fn take_thread_local_bloom_filter<N>(parent_node: Option<N>,
     })
 }
 
-fn put_thread_local_bloom_filter(bf: Box<BloomFilter>, unsafe_node: &UnsafeNode,
+pub fn put_thread_local_bloom_filter(bf: Box<BloomFilter>, unsafe_node: &UnsafeNode,
-                                 context: &SharedStyleContext) {
+                                     context: &SharedStyleContext) {
     STYLE_BLOOM.with(move |style_bloom| {
         assert!(style_bloom.borrow().is_none(),
                 "Putting into a never-taken thread-local bloom filter");
@@ -108,17 +108,17 @@ fn put_thread_local_bloom_filter(bf: Box<BloomFilter>, unsafe_node: &UnsafeNode,
 }
 
 /// "Ancestors" in this context is inclusive of ourselves.
-fn insert_ancestors_into_bloom_filter<N>(bf: &mut Box<BloomFilter>,
+fn insert_ancestors_into_bloom_filter<E>(bf: &mut Box<BloomFilter>,
-                                         mut n: N,
+                                         mut el: E,
                                          root: OpaqueNode)
-                                         where N: TNode {
+                                         where E: TElement {
     debug!("[{}] Inserting ancestors.", tid());
     let mut ancestors = 0;
     loop {
         ancestors += 1;
 
-        n.insert_into_bloom_filter(&mut **bf);
+        el.insert_into_bloom_filter(&mut **bf);
-        n = match n.layout_parent_node(root) {
+        el = match el.as_node().layout_parent_node(root).and_then(|x| x.as_element()) {
             None => break,
             Some(p) => p,
         };
@@ -149,7 +149,7 @@ pub fn remove_from_bloom_filter<'a, N, C>(context: &C, root: OpaqueNode, node: N
         }
         Some(parent) => {
             // Otherwise, put it back, but remove this node.
-            node.remove_from_bloom_filter(&mut *bf);
+            node.as_element().map(|x| x.remove_from_bloom_filter(&mut *bf));
             let unsafe_parent = parent.to_unsafe();
             put_thread_local_bloom_filter(bf, &unsafe_parent, &context.shared_context());
         },
@@ -213,20 +213,20 @@ pub fn relations_are_shareable(relations: &StyleRelations) -> bool {
                           AFFECTED_BY_PRESENTATIONAL_HINTS)
 }
 
-pub fn ensure_node_styled<'a, N, C>(node: N,
+pub fn ensure_element_styled<'a, E, C>(element: E,
-                                    context: &'a C)
+                                       context: &'a C)
-    where N: TNode,
+    where E: TElement,
           C: StyleContext<'a>
 {
     let mut display_none = false;
-    ensure_node_styled_internal(node, context, &mut display_none);
+    ensure_element_styled_internal(element, context, &mut display_none);
 }
 
 #[allow(unsafe_code)]
-fn ensure_node_styled_internal<'a, N, C>(node: N,
+fn ensure_element_styled_internal<'a, E, C>(element: E,
-                                         context: &'a C,
+                                            context: &'a C,
-                                         parents_had_display_none: &mut bool)
+                                            parents_had_display_none: &mut bool)
-    where N: TNode,
+    where E: TElement,
           C: StyleContext<'a>
 {
     use properties::longhands::display::computed_value as display;
@@ -238,13 +238,9 @@ fn ensure_node_styled_internal<'a, N, C>(node: N,
     // This means potentially a bit of wasted work (usually not much). We could
     // add a flag at the node at which point we stopped the traversal to know
     // where should we stop, but let's not add that complication unless needed.
-    let parent = match node.parent_node() {
+    let parent = element.parent_element();
-        Some(parent) if parent.is_element() => Some(parent),
-        _ => None,
-    };
-
     if let Some(parent) = parent {
-        ensure_node_styled_internal(parent, context, parents_had_display_none);
+        ensure_element_styled_internal(parent, context, parents_had_display_none);
     }
 
     // Common case: our style is already resolved and none of our ancestors had
@@ -252,6 +248,7 @@ fn ensure_node_styled_internal<'a, N, C>(node: N,
     //
     // We only need to mark whether we have display none, and forget about it,
     // our style is up to date.
+    let node = element.as_node();
     if let Some(data) = node.borrow_data() {
         if let Some(style) = data.get_current_styles().map(|x| &x.primary) {
             if !*parents_had_display_none {
@@ -268,55 +265,42 @@ fn ensure_node_styled_internal<'a, N, C>(node: N,
     // probably not necessary since we're likely to be matching only a few
     // nodes, at best.
     let mut applicable_declarations = ApplicableDeclarations::new();
-    if let Some(element) = node.as_element() {
+    let stylist = &context.shared_context().stylist;
-        let stylist = &context.shared_context().stylist;
 
-        element.match_element(&**stylist,
+    element.match_element(&**stylist,
-                              None,
+                          None,
-                              &mut applicable_declarations);
+                          &mut applicable_declarations);
-    }
 
     unsafe {
-        node.cascade_node(context, parent, &applicable_declarations);
+        element.cascade_node(context, parent, &applicable_declarations);
     }
 }
 
 /// Calculates the style for a single node.
 #[inline]
 #[allow(unsafe_code)]
-pub fn recalc_style_at<'a, N, C, D>(context: &'a C,
+pub fn recalc_style_at<'a, E, C, D>(context: &'a C,
                                     root: OpaqueNode,
-                                    node: N) -> RestyleResult
+                                    element: E) -> RestyleResult
-    where N: TNode,
+    where E: TElement,
           C: StyleContext<'a>,
-          D: DomTraversalContext<N>
+          D: DomTraversalContext<E::ConcreteNode>
 {
-    // Get the parent node.
-    let parent_opt = match node.parent_node() {
-        Some(parent) if parent.is_element() => Some(parent),
-        _ => None,
-    };
-
     // Get the style bloom filter.
-    let mut bf = take_thread_local_bloom_filter(parent_opt, root, context.shared_context());
+    let mut bf = take_thread_local_bloom_filter(element.parent_element(), root, context.shared_context());
 
     let mut restyle_result = RestyleResult::Continue;
-    let mode = node.styling_mode();
+    let mode = element.as_node().styling_mode();
     debug_assert!(mode != StylingMode::Stop, "Parent should not have enqueued us");
     if mode != StylingMode::Traverse {
         // Check to see whether we can share a style with someone.
         let style_sharing_candidate_cache =
             &mut context.local_context().style_sharing_candidate_cache.borrow_mut();
 
-        let sharing_result = match node.as_element() {
+        let sharing_result = if element.parent_element().is_none() {
-            Some(element) => {
+            StyleSharingResult::CannotShare
-                unsafe {
+        } else {
-                    element.share_style_if_possible(style_sharing_candidate_cache,
+            unsafe { element.share_style_if_possible(style_sharing_candidate_cache, context.shared_context()) }
-                                                    context.shared_context(),
-                                                    parent_opt.clone())
-                }
-            },
-            None => StyleSharingResult::CannotShare,
         };
 
         // Otherwise, match and cascade selectors.
@@ -325,38 +309,32 @@ pub fn recalc_style_at<'a, N, C, D>(context: &'a C,
                 let mut applicable_declarations = ApplicableDeclarations::new();
 
                 let relations;
-                let shareable_element = match node.as_element() {
+                let shareable_element = {
-                    Some(element) => {
+                    if opts::get().style_sharing_stats {
-                        if opts::get().style_sharing_stats {
+                        STYLE_SHARING_CACHE_MISSES.fetch_add(1, Ordering::Relaxed);
-                            STYLE_SHARING_CACHE_MISSES.fetch_add(1, Ordering::Relaxed);
+                    }
-                        }
 
-                        // Perform the CSS selector matching.
+                    // Perform the CSS selector matching.
-                        let stylist = &context.shared_context().stylist;
+                    let stylist = &context.shared_context().stylist;
 
-                        relations = element.match_element(&**stylist,
+                    relations = element.match_element(&**stylist,
-                                                          Some(&*bf),
+                                                      Some(&*bf),
-                                                          &mut applicable_declarations);
+                                                      &mut applicable_declarations);
 
-                        debug!("Result of selector matching: {:?}", relations);
+                    debug!("Result of selector matching: {:?}", relations);
 
-                        if relations_are_shareable(&relations) {
+                    if relations_are_shareable(&relations) {
-                            Some(element)
+                        Some(element)
-                        } else {
+                    } else {
-                            None
-                        }
-                    },
-                    None => {
-                        relations = StyleRelations::empty();
                         None
-                    },
+                    }
                 };
 
                 // Perform the CSS cascade.
                 unsafe {
-                    restyle_result = node.cascade_node(context,
+                    restyle_result = element.cascade_node(context,
-                                                       parent_opt,
+                                                          element.parent_element(),
-                                                       &applicable_declarations);
+                                                          &applicable_declarations);
                 }
 
                 // Add ourselves to the LRU cache.
@@ -370,7 +348,7 @@ pub fn recalc_style_at<'a, N, C, D>(context: &'a C,
                     STYLE_SHARING_CACHE_HITS.fetch_add(1, Ordering::Relaxed);
                 }
                 style_sharing_candidate_cache.touch(index);
-                node.set_restyle_damage(damage);
+                element.set_restyle_damage(damage);
             }
         }
     }
@@ -379,7 +357,7 @@ pub fn recalc_style_at<'a, N, C, D>(context: &'a C,
     // processing. The eventual algorithm we're designing does this in a more granular
     // fashion.
     if mode == StylingMode::Restyle && restyle_result == RestyleResult::Continue {
-        for kid in node.children() {
+        for kid in element.as_node().children() {
             let mut data = D::ensure_node_data(&kid).borrow_mut();
             if kid.is_text_node() {
                 data.ensure_restyle_data();
@@ -392,12 +370,12 @@ pub fn recalc_style_at<'a, N, C, D>(context: &'a C,
         }
     }
 
-    let unsafe_layout_node = node.to_unsafe();
+    let unsafe_layout_node = element.as_node().to_unsafe();
 
     // Before running the children, we need to insert our nodes into the bloom
     // filter.
     debug!("[{}] + {:X}", tid(), unsafe_layout_node.0);
-    node.insert_into_bloom_filter(&mut *bf);
+    element.insert_into_bloom_filter(&mut *bf);
 
     // NB: flow construction updates the bloom filter on the way up.
     put_thread_local_bloom_filter(bf, &unsafe_layout_node, context.shared_context());