Auto merge of #18904 - emilio:invalidator-less-dependencies, r=heycam,jdm,nox

style: Stop threading the ElementData around the invalidator.

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

components/layout_thread/dom_wrapper.rs

@@ -68,7 +68,7 @@ use style::attr::AttrValue;
 use style::computed_values::display;
 use style::context::SharedStyleContext;
 use style::data::ElementData;
-use style::dom::{LayoutIterator, NodeInfo, OpaqueNode};
+use style::dom::{DomChildren, LayoutIterator, NodeInfo, OpaqueNode};
 use style::dom::{PresentationalHintsSynthesizer, TElement, TNode};
 use style::element_state::*;
 use style::font_metrics::ServoMetricsProvider;
@@ -159,7 +159,6 @@ impl<'ln> NodeInfo for ServoLayoutNode<'ln> {
 
 impl<'ln> TNode for ServoLayoutNode<'ln> {
     type ConcreteElement = ServoLayoutElement<'ln>;
-    type ConcreteChildrenIterator = ServoChildrenIterator<'ln>;
 
     fn parent_node(&self) -> Option<Self> {
         unsafe {
@@ -167,20 +166,34 @@ impl<'ln> TNode for ServoLayoutNode<'ln> {
         }
     }
 
-    fn children(&self) -> LayoutIterator<ServoChildrenIterator<'ln>> {
+    fn first_child(&self) -> Option<Self> {
-        LayoutIterator(ServoChildrenIterator {
+        unsafe {
-            current: self.first_child(),
+            self.node.first_child_ref().map(|node| self.new_with_this_lifetime(&node))
-        })
+        }
+    }
+
+    fn last_child(&self) -> Option<Self> {
+        unsafe {
+            self.node.last_child_ref().map(|node| self.new_with_this_lifetime(&node))
+        }
+    }
+
+    fn prev_sibling(&self) -> Option<Self> {
+        unsafe {
+            self.node.prev_sibling_ref().map(|node| self.new_with_this_lifetime(&node))
+        }
+    }
+
+    fn next_sibling(&self) -> Option<Self> {
+        unsafe {
+            self.node.next_sibling_ref().map(|node| self.new_with_this_lifetime(&node))
+        }
     }
 
     fn traversal_parent(&self) -> Option<ServoLayoutElement<'ln>> {
         self.parent_element()
     }
 
-    fn traversal_children(&self) -> LayoutIterator<ServoChildrenIterator<'ln>> {
-        self.children()
-    }
-
     fn opaque(&self) -> OpaqueNode {
         unsafe { self.get_jsmanaged().opaque() }
     }
@@ -200,23 +213,6 @@ impl<'ln> TNode for ServoLayoutNode<'ln> {
     unsafe fn set_can_be_fragmented(&self, value: bool) {
         self.node.set_flag(CAN_BE_FRAGMENTED, value)
     }
-
-    fn is_in_doc(&self) -> bool {
-        unsafe { (*self.node.unsafe_get()).is_in_doc() }
-    }
-}
-
-pub struct ServoChildrenIterator<'a> {
-    current: Option<ServoLayoutNode<'a>>,
-}
-
-impl<'a> Iterator for ServoChildrenIterator<'a> {
-    type Item = ServoLayoutNode<'a>;
-    fn next(&mut self) -> Option<ServoLayoutNode<'a>> {
-        let node = self.current;
-        self.current = node.and_then(|node| node.next_sibling());
-        node
-    }
 }
 
 impl<'ln> LayoutNode for ServoLayoutNode<'ln> {
@@ -248,30 +244,6 @@ impl<'ln> LayoutNode for ServoLayoutNode<'ln> {
     unsafe fn take_style_and_layout_data(&self) -> OpaqueStyleAndLayoutData {
         self.get_jsmanaged().take_style_and_layout_data()
     }
-
-    fn first_child(&self) -> Option<ServoLayoutNode<'ln>> {
-        unsafe {
-            self.node.first_child_ref().map(|node| self.new_with_this_lifetime(&node))
-        }
-    }
-
-    fn last_child(&self) -> Option<ServoLayoutNode<'ln>> {
-        unsafe {
-            self.node.last_child_ref().map(|node| self.new_with_this_lifetime(&node))
-        }
-    }
-
-    fn prev_sibling(&self) -> Option<ServoLayoutNode<'ln>> {
-        unsafe {
-            self.node.prev_sibling_ref().map(|node| self.new_with_this_lifetime(&node))
-        }
-    }
-
-    fn next_sibling(&self) -> Option<ServoLayoutNode<'ln>> {
-        unsafe {
-            self.node.next_sibling_ref().map(|node| self.new_with_this_lifetime(&node))
-        }
-    }
 }
 
 impl<'ln> GetLayoutData for ServoLayoutNode<'ln> {
@@ -320,8 +292,8 @@ impl<'ld> ServoLayoutDocument<'ld> {
         ServoLayoutNode::from_layout_js(self.document.upcast())
     }
 
-    pub fn root_node(&self) -> Option<ServoLayoutNode<'ld>> {
+    pub fn root_element(&self) -> Option<ServoLayoutElement<'ld>> {
-        self.as_node().children().find(ServoLayoutNode::is_element)
+        self.as_node().dom_children().flat_map(|n| n.as_element()).next()
     }
 
     pub fn drain_pending_restyles(&self) -> Vec<(ServoLayoutElement<'ld>, PendingRestyle)> {
@@ -380,6 +352,7 @@ impl<'le> PresentationalHintsSynthesizer for ServoLayoutElement<'le> {
 
 impl<'le> TElement for ServoLayoutElement<'le> {
     type ConcreteNode = ServoLayoutNode<'le>;
+    type TraversalChildrenIterator = DomChildren<Self::ConcreteNode>;
 
     type FontMetricsProvider = ServoMetricsProvider;
 
@@ -387,6 +360,10 @@ impl<'le> TElement for ServoLayoutElement<'le> {
         ServoLayoutNode::from_layout_js(self.element.upcast())
     }
 
+    fn traversal_children(&self) -> LayoutIterator<Self::TraversalChildrenIterator> {
+        LayoutIterator(self.as_node().dom_children())
+    }
+
     fn style_attribute(&self) -> Option<ArcBorrow<StyleLocked<PropertyDeclarationBlock>>> {
         unsafe {
             (*self.element.style_attribute()).as_ref().map(|x| x.borrow_arc())
@@ -629,7 +606,7 @@ impl<'le> ::selectors::Element for ServoLayoutElement<'le> {
     }
 
     fn first_child_element(&self) -> Option<ServoLayoutElement<'le>> {
-        self.as_node().children().filter_map(|n| n.as_element()).next()
+        self.as_node().dom_children().filter_map(|n| n.as_element()).next()
     }
 
     fn last_child_element(&self) -> Option<ServoLayoutElement<'le>> {
@@ -690,7 +667,7 @@ impl<'le> ::selectors::Element for ServoLayoutElement<'le> {
     }
 
     fn is_empty(&self) -> bool {
-        self.as_node().children().all(|node| match node.script_type_id() {
+        self.as_node().dom_children().all(|node| match node.script_type_id() {
             NodeTypeId::Element(..) => false,
             NodeTypeId::CharacterData(CharacterDataTypeId::Text) => unsafe {
                 node.node.downcast().unwrap().data_for_layout().is_empty()
@@ -850,20 +827,14 @@ impl<'ln> ServoThreadSafeLayoutNode<'ln> {
     }
 }
 
-// NB: The implementation here is a bit tricky because elements implementing
-// pseudos are supposed to return false for is_element().
 impl<'ln> NodeInfo for ServoThreadSafeLayoutNode<'ln> {
     fn is_element(&self) -> bool {
-        self.pseudo == PseudoElementType::Normal && self.node.is_element()
+        self.node.is_element()
     }
 
     fn is_text_node(&self) -> bool {
         self.node.is_text_node()
     }
-
-    fn needs_layout(&self) -> bool {
-        self.node.is_text_node() || self.node.is_element()
-    }
 }
 
 impl<'ln> ThreadSafeLayoutNode for ServoThreadSafeLayoutNode<'ln> {
@@ -883,11 +854,6 @@ impl<'ln> ThreadSafeLayoutNode for ServoThreadSafeLayoutNode<'ln> {
         }
     }
 
-    #[inline]
-    fn type_id_without_excluding_pseudo_elements(&self) -> LayoutNodeType {
-        self.node.type_id()
-    }
-
     fn parent_style(&self) -> Arc<ComputedValues> {
         let parent = self.node.parent_node().unwrap().as_element().unwrap();
         let parent_data = parent.get_data().unwrap().borrow();

components/layout_thread/lib.rs

@@ -1067,7 +1067,7 @@ impl LayoutThread {
 
         let mut rw_data = possibly_locked_rw_data.lock();
 
-        let element: ServoLayoutElement = match document.root_node() {
+        let element = match document.root_element() {
             None => {
                 // Since we cannot compute anything, give spec-required placeholders.
                 debug!("layout: No root node: bailing");
@@ -1112,7 +1112,7 @@ impl LayoutThread {
                 }
                 return;
             },
-            Some(x) => x.as_element().unwrap(),
+            Some(x) => x,
         };
 
         debug!("layout: processing reflow request for: {:?} ({}) (query={:?})",

components/script_layout_interface/wrapper_traits.rs

@@ -100,14 +100,6 @@ pub trait LayoutNode: Debug + GetLayoutData + TNode {
     fn traverse_preorder(self) -> TreeIterator<Self> {
         TreeIterator::new(self)
     }
-
-    fn first_child(&self) -> Option<Self>;
-
-    fn last_child(&self) -> Option<Self>;
-
-    fn prev_sibling(&self) -> Option<Self>;
-
-    fn next_sibling(&self) -> Option<Self>;
 }
 
 pub struct ReverseChildrenIterator<ConcreteNode> where ConcreteNode: LayoutNode {
@@ -169,10 +161,6 @@ pub trait ThreadSafeLayoutNode: Clone + Copy + Debug + GetLayoutData + NodeInfo
     /// Returns `None` if this is a pseudo-element; otherwise, returns `Some`.
     fn type_id(&self) -> Option<LayoutNodeType>;
 
-    /// Returns the type ID of this node, without discarding pseudo-elements as
-    /// `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.
     ///
@@ -183,14 +171,6 @@ pub trait ThreadSafeLayoutNode: Clone + Copy + Debug + GetLayoutData + NodeInfo
     /// the parent until all the children have been processed.
     fn parent_style(&self) -> Arc<ComputedValues>;
 
-    #[inline]
-    fn is_element_or_elements_pseudo(&self) -> bool {
-        match self.type_id_without_excluding_pseudo_elements() {
-            LayoutNodeType::Element(..) => true,
-            _ => false,
-        }
-    }
-
     fn get_before_pseudo(&self) -> Option<Self> {
         self.as_element().and_then(|el| el.get_before_pseudo()).map(|el| el.as_node())
     }

components/style/data.rs

@@ -259,11 +259,15 @@ impl ElementData {
             return InvalidationResult::empty();
         }
 
-        let mut processor = StateAndAttrInvalidationProcessor;
+        let mut processor = StateAndAttrInvalidationProcessor::new(
+            shared_context,
+            element,
+            self,
+        );
+
         let invalidator = TreeStyleInvalidator::new(
             element,
-            Some(self),
+            shared_context.quirks_mode(),
-            shared_context,
             stack_limit_checker,
             nth_index_cache,
             &mut processor,

components/style/dom.rs

@@ -64,61 +64,86 @@ pub trait NodeInfo {
     fn is_element(&self) -> bool;
     /// Whether this node is a text node.
     fn is_text_node(&self) -> bool;
-
-    /// Whether this node needs layout.
-    ///
-    /// Comments, doctypes, etc are ignored by layout algorithms.
-    fn needs_layout(&self) -> bool { self.is_element() || self.is_text_node() }
 }
 
 /// A node iterator that only returns node that don't need layout.
 pub struct LayoutIterator<T>(pub T);
 
-impl<T, I> Iterator for LayoutIterator<T>
+impl<T, N> Iterator for LayoutIterator<T>
-    where T: Iterator<Item=I>,
+where
-          I: NodeInfo,
+    T: Iterator<Item = N>,
+    N: NodeInfo,
 {
-    type Item = I;
+    type Item = N;
-    fn next(&mut self) -> Option<I> {
+
+    fn next(&mut self) -> Option<N> {
         loop {
-            // Filter out nodes that layout should ignore.
+            match self.0.next() {
-            let n = self.0.next();
+                Some(n) => {
-            if n.is_none() || n.as_ref().unwrap().needs_layout() {
+                    // Filter out nodes that layout should ignore.
-                return n
+                    if n.is_text_node() || n.is_element() {
+                        return Some(n)
+                    }
+                }
+                None => return None,
             }
         }
     }
 }
 
+/// An iterator over the DOM children of a node.
+pub struct DomChildren<N>(Option<N>);
+impl<N> Iterator for DomChildren<N>
+where
+    N: TNode
+{
+    type Item = N;
+
+    fn next(&mut self) -> Option<N> {
+        match self.0.take() {
+            Some(n) => {
+                self.0 = n.next_sibling();
+                Some(n)
+            }
+            None => None,
+        }
+    }
+}
+
 /// The `TNode` trait. This is the main generic trait over which the style
 /// system can be implemented.
 pub trait TNode : Sized + Copy + Clone + Debug + NodeInfo {
     /// The concrete `TElement` type.
     type ConcreteElement: TElement<ConcreteNode = Self>;
 
-    /// A concrete children iterator type in order to iterate over the `Node`s.
-    ///
-    /// TODO(emilio): We should eventually replace this with the `impl Trait`
-    /// syntax.
-    type ConcreteChildrenIterator: Iterator<Item = Self>;
-
     /// Get this node's parent node.
     fn parent_node(&self) -> Option<Self>;
 
-    /// Get this node's parent element if present.
+    /// Get this node's first child.
-    fn parent_element(&self) -> Option<Self::ConcreteElement> {
+    fn first_child(&self) -> Option<Self>;
-        self.parent_node().and_then(|n| n.as_element())
-    }
 
-    /// Returns an iterator over this node's children.
+    /// Get this node's first child.
-    fn children(&self) -> LayoutIterator<Self::ConcreteChildrenIterator>;
+    fn last_child(&self) -> Option<Self>;
+
+    /// Get this node's previous sibling.
+    fn prev_sibling(&self) -> Option<Self>;
+
+    /// Get this node's next sibling.
+    fn next_sibling(&self) -> Option<Self>;
+
+    /// Iterate over the DOM children of an element.
+    fn dom_children(&self) -> DomChildren<Self> {
+        DomChildren(self.first_child())
+    }
 
     /// Get this node's parent element from the perspective of a restyle
     /// traversal.
     fn traversal_parent(&self) -> Option<Self::ConcreteElement>;
 
-    /// Get this node's children from the perspective of a restyle traversal.
+    /// Get this node's parent element if present.
-    fn traversal_children(&self) -> LayoutIterator<Self::ConcreteChildrenIterator>;
+    fn parent_element(&self) -> Option<Self::ConcreteElement> {
+        self.parent_node().and_then(|n| n.as_element())
+    }
 
     /// Converts self into an `OpaqueNode`.
     fn opaque(&self) -> OpaqueNode;
@@ -135,10 +160,6 @@ pub trait TNode : Sized + Copy + Clone + Debug + NodeInfo {
 
     /// Set whether this node can be fragmented.
     unsafe fn set_can_be_fragmented(&self, value: bool);
-
-    /// Whether this node is in the document right now needed to clear the
-    /// restyle data appropriately on some forced restyles.
-    fn is_in_doc(&self) -> bool;
 }
 
 /// Wrapper to output the ElementData along with the node when formatting for
@@ -223,9 +244,11 @@ fn fmt_subtree<F, N: TNode>(f: &mut fmt::Formatter, stringify: &F, n: N, indent:
         write!(f, "  ")?;
     }
     stringify(f, n)?;
-    for kid in n.traversal_children() {
+    if let Some(e) = n.as_element() {
-        writeln!(f, "")?;
+        for kid in e.traversal_children() {
-        fmt_subtree(f, stringify, kid, indent + 1)?;
+            writeln!(f, "")?;
+            fmt_subtree(f, stringify, kid, indent + 1)?;
+        }
     }
 
     Ok(())
@@ -256,6 +279,12 @@ pub trait TElement
     /// The concrete node type.
     type ConcreteNode: TNode<ConcreteElement = Self>;
 
+    /// A concrete children iterator type in order to iterate over the `Node`s.
+    ///
+    /// TODO(emilio): We should eventually replace this with the `impl Trait`
+    /// syntax.
+    type TraversalChildrenIterator: Iterator<Item = Self::ConcreteNode>;
+
     /// Type of the font metrics provider
     ///
     /// XXXManishearth It would be better to make this a type parameter on
@@ -295,6 +324,9 @@ pub trait TElement
         self.as_node().traversal_parent()
     }
 
+    /// Get this node's children from the perspective of a restyle traversal.
+    fn traversal_children(&self) -> LayoutIterator<Self::TraversalChildrenIterator>;
+
     /// Returns the parent element we should inherit from.
     ///
     /// This is pretty much always the parent element itself, except in the case

components/style/gecko/wrapper.rs

@@ -158,32 +158,6 @@ impl<'ln> GeckoNode<'ln> {
         unsafe { &*self.node_info().mDocument }
     }
 
-    #[inline]
-    fn first_child(&self) -> Option<GeckoNode<'ln>> {
-        unsafe { self.0.mFirstChild.as_ref().map(GeckoNode::from_content) }
-    }
-
-    #[inline]
-    fn last_child(&self) -> Option<GeckoNode<'ln>> {
-        unsafe { Gecko_GetLastChild(self.0).map(GeckoNode) }
-    }
-
-    #[inline]
-    fn prev_sibling(&self) -> Option<GeckoNode<'ln>> {
-        unsafe { self.0.mPreviousSibling.as_ref().map(GeckoNode::from_content) }
-    }
-
-    #[inline]
-    fn next_sibling(&self) -> Option<GeckoNode<'ln>> {
-        unsafe { self.0.mNextSibling.as_ref().map(GeckoNode::from_content) }
-    }
-
-    /// Simple iterator over all this node's children.  Unlike `.children()`, this iterator does
-    /// not filter out nodes that don't need layout.
-    fn dom_children(self) -> GeckoChildrenIterator<'ln> {
-        GeckoChildrenIterator::Current(self.first_child())
-    }
-
     /// WARNING: This logic is duplicated in Gecko's FlattenedTreeParentIsParent.
     /// Make sure to mirror any modifications in both places.
     fn flattened_tree_parent_is_parent(&self) -> bool {
@@ -222,11 +196,6 @@ impl<'ln> GeckoNode<'ln> {
     fn contains_non_whitespace_content(&self) -> bool {
         unsafe { Gecko_IsSignificantChild(self.0, true, false) }
     }
-
-    #[inline]
-    fn may_have_anonymous_children(&self) -> bool {
-        self.get_bool_flag(nsINode_BooleanFlag::ElementMayHaveAnonymousChildren)
-    }
 }
 
 impl<'ln> NodeInfo for GeckoNode<'ln> {
@@ -244,40 +213,35 @@ impl<'ln> NodeInfo for GeckoNode<'ln> {
 
 impl<'ln> TNode for GeckoNode<'ln> {
     type ConcreteElement = GeckoElement<'ln>;
-    type ConcreteChildrenIterator = GeckoChildrenIterator<'ln>;
 
     fn parent_node(&self) -> Option<Self> {
         unsafe { self.0.mParent.as_ref().map(GeckoNode) }
     }
 
-    fn children(&self) -> LayoutIterator<GeckoChildrenIterator<'ln>> {
+    #[inline]
-        LayoutIterator(self.dom_children())
+    fn first_child(&self) -> Option<Self> {
+        unsafe { self.0.mFirstChild.as_ref().map(GeckoNode::from_content) }
+    }
+
+    #[inline]
+    fn last_child(&self) -> Option<Self> {
+        unsafe { Gecko_GetLastChild(self.0).map(GeckoNode) }
+    }
+
+    #[inline]
+    fn prev_sibling(&self) -> Option<Self> {
+        unsafe { self.0.mPreviousSibling.as_ref().map(GeckoNode::from_content) }
+    }
+
+    #[inline]
+    fn next_sibling(&self) -> Option<Self> {
+        unsafe { self.0.mNextSibling.as_ref().map(GeckoNode::from_content) }
     }
 
     fn traversal_parent(&self) -> Option<GeckoElement<'ln>> {
         self.flattened_tree_parent().and_then(|n| n.as_element())
     }
 
-    fn traversal_children(&self) -> LayoutIterator<GeckoChildrenIterator<'ln>> {
-        if let Some(element) = self.as_element() {
-            // This condition is similar to the check that
-            // StyleChildrenIterator::IsNeeded does, except that it might return
-            // true if we used to (but no longer) have anonymous content from
-            // ::before/::after, XBL bindings, or nsIAnonymousContentCreators.
-            if element.is_in_anonymous_subtree() ||
-               element.has_xbl_binding_with_content() ||
-               self.may_have_anonymous_children() {
-                unsafe {
-                    let mut iter: structs::StyleChildrenIterator = ::std::mem::zeroed();
-                    Gecko_ConstructStyleChildrenIterator(element.0, &mut iter);
-                    return LayoutIterator(GeckoChildrenIterator::GeckoIterator(iter));
-                }
-            }
-        }
-
-        LayoutIterator(self.dom_children())
-    }
-
     fn opaque(&self) -> OpaqueNode {
         let ptr: usize = self.0 as *const _ as usize;
         OpaqueNode(ptr)
@@ -306,10 +270,6 @@ impl<'ln> TNode for GeckoNode<'ln> {
         // FIXME(SimonSapin): Servo uses this to implement CSS multicol / fragmentation
         // Maybe this isn’t useful for Gecko?
     }
-
-    fn is_in_doc(&self) -> bool {
-        unsafe { bindings::Gecko_IsInDocument(self.0) }
-    }
 }
 
 /// A wrapper on top of two kind of iterators, depending on the parent being
@@ -447,6 +407,11 @@ impl<'le> fmt::Debug for GeckoElement<'le> {
 }
 
 impl<'le> GeckoElement<'le> {
+    #[inline]
+    fn may_have_anonymous_children(&self) -> bool {
+        self.as_node().get_bool_flag(nsINode_BooleanFlag::ElementMayHaveAnonymousChildren)
+    }
+
     /// Parse the style attribute of an element.
     pub fn parse_style_attribute<R>(
         value: &str,
@@ -883,6 +848,7 @@ impl structs::FontSizePrefs {
 impl<'le> TElement for GeckoElement<'le> {
     type ConcreteNode = GeckoNode<'le>;
     type FontMetricsProvider = GeckoFontMetricsProvider;
+    type TraversalChildrenIterator = GeckoChildrenIterator<'le>;
 
     fn inheritance_parent(&self) -> Option<Self> {
         if self.is_native_anonymous() {
@@ -894,6 +860,24 @@ impl<'le> TElement for GeckoElement<'le> {
         }
     }
 
+    fn traversal_children(&self) -> LayoutIterator<GeckoChildrenIterator<'le>> {
+        // This condition is similar to the check that
+        // StyleChildrenIterator::IsNeeded does, except that it might return
+        // true if we used to (but no longer) have anonymous content from
+        // ::before/::after, XBL bindings, or nsIAnonymousContentCreators.
+        if self.is_in_anonymous_subtree() ||
+           self.has_xbl_binding_with_content() ||
+           self.may_have_anonymous_children() {
+            unsafe {
+                let mut iter: structs::StyleChildrenIterator = ::std::mem::zeroed();
+                Gecko_ConstructStyleChildrenIterator(self.0, &mut iter);
+                return LayoutIterator(GeckoChildrenIterator::GeckoIterator(iter));
+            }
+        }
+
+        LayoutIterator(GeckoChildrenIterator::Current(self.as_node().first_child()))
+    }
+
     fn before_pseudo_element(&self) -> Option<Self> {
         self.get_before_or_after_pseudo(/* is_before = */ true)
     }

components/style/invalidation/element/collector.rs

@@ -51,9 +51,24 @@ where
 
 /// An invalidation processor for style changes due to state and attribute
 /// changes.
-pub struct StateAndAttrInvalidationProcessor;
+pub struct StateAndAttrInvalidationProcessor<'a, 'b: 'a, E> {
+    shared_context: &'a SharedStyleContext<'b>,
+    element: E,
+    data: &'a mut ElementData,
+}
 
-impl<E> InvalidationProcessor<E> for StateAndAttrInvalidationProcessor
+impl<'a, 'b: 'a, E> StateAndAttrInvalidationProcessor<'a, 'b, E> {
+    /// Creates a new StateAndAttrInvalidationProcessor.
+    pub fn new(
+        shared_context: &'a SharedStyleContext<'b>,
+        element: E,
+        data: &'a mut ElementData,
+    ) -> Self {
+        Self { shared_context, element, data }
+    }
+}
+
+impl<'a, 'b: 'a, E> InvalidationProcessor<E> for StateAndAttrInvalidationProcessor<'a, 'b, E>
 where
     E: TElement,
 {
@@ -65,17 +80,16 @@ where
     fn collect_invalidations(
         &mut self,
         element: E,
-        mut data: Option<&mut ElementData>,
         nth_index_cache: Option<&mut NthIndexCache>,
-        shared_context: &SharedStyleContext,
+        quirks_mode: QuirksMode,
         descendant_invalidations: &mut InvalidationVector,
         sibling_invalidations: &mut InvalidationVector,
     ) -> bool {
         debug_assert!(element.has_snapshot(), "Why bothering?");
-        debug_assert!(data.is_some(), "How exactly?");
+        debug_assert_eq!(quirks_mode, self.shared_context.quirks_mode(), "How exactly?");
 
         let wrapper =
-            ElementWrapper::new(element, &*shared_context.snapshot_map);
+            ElementWrapper::new(element, &*self.shared_context.snapshot_map);
 
         let state_changes = wrapper.state_changes();
         let snapshot = wrapper.snapshot().expect("has_snapshot lied");
@@ -92,8 +106,7 @@ where
             trace!(" > visitedness change, force subtree restyle");
             // We can't just return here because there may also be attribute
             // changes as well that imply additional hints.
-            let data = data.as_mut().unwrap();
+            self.data.hint.insert(RestyleHint::restyle_subtree());
-            data.hint.insert(RestyleHint::restyle_subtree());
         }
 
         let mut classes_removed = SmallVec::<[Atom; 8]>::new();
@@ -140,7 +153,7 @@ where
                 state_changes,
                 element,
                 snapshot: &snapshot,
-                quirks_mode: shared_context.quirks_mode(),
+                quirks_mode: self.shared_context.quirks_mode(),
                 removed_id: id_removed.as_ref(),
                 added_id: id_added.as_ref(),
                 classes_removed: &classes_removed,
@@ -150,7 +163,7 @@ where
                 invalidates_self: false,
             };
 
-            shared_context.stylist.each_invalidation_map(|invalidation_map| {
+            self.shared_context.stylist.each_invalidation_map(|invalidation_map| {
                 collector.collect_dependencies_in_invalidation_map(invalidation_map);
             });
 
@@ -173,52 +186,43 @@ where
         };
 
         if invalidated_self {
-            if let Some(ref mut data) = data {
+            self.data.hint.insert(RESTYLE_SELF);
-                data.hint.insert(RESTYLE_SELF);
-            }
         }
 
         invalidated_self
     }
 
-    fn should_process_descendants(
+    fn should_process_descendants(&mut self, element: E) -> bool {
-        &mut self,
+        if element == self.element {
-        _element: E,
+            return !self.data.styles.is_display_none() &&
-        data: Option<&mut ElementData>,
+                !self.data.hint.contains(RESTYLE_DESCENDANTS)
-    ) -> bool {
+        }
-        let data = match data {
+
+        let data = match element.borrow_data() {
             None => return false,
-            Some(ref data) => data,
+            Some(data) => data,
         };
 
         !data.styles.is_display_none() &&
             !data.hint.contains(RESTYLE_DESCENDANTS)
     }
 
-    fn recursion_limit_exceeded(
+    fn recursion_limit_exceeded(&mut self, element: E) {
-        &mut self,
+        if element == self.element {
-        _element: E,
+            self.data.hint.insert(RESTYLE_DESCENDANTS);
-        data: Option<&mut ElementData>,
+            return;
-    ) {
+        }
-        if let Some(data) = data {
+
+        if let Some(mut data) = element.mutate_data() {
             data.hint.insert(RESTYLE_DESCENDANTS);
         }
     }
 
-    fn invalidated_descendants(
+    fn invalidated_descendants(&mut self, element: E, child: E) {
-        &mut self,
-        element: E,
-        data: Option<&mut ElementData>,
-        child: E,
-    ) {
         if child.get_data().is_none() {
             return;
         }
 
-        if data.as_ref().map_or(true, |d| d.styles.is_display_none()) {
-            return;
-        }
-
         // The child may not be a flattened tree child of the current element,
         // but may be arbitrarily deep.
         //
@@ -235,12 +239,9 @@ where
         }
     }
 
-    fn invalidated_self(
+    fn invalidated_self(&mut self, element: E) {
-        &mut self,
+        debug_assert_ne!(element, self.element);
-        _element: E,
+        if let Some(mut data) = element.mutate_data() {
-        data: Option<&mut ElementData>,
-    ) {
-        if let Some(data) = data {
             data.hint.insert(RESTYLE_SELF);
         }
     }

components/style/invalidation/element/invalidator.rs

@@ -5,12 +5,11 @@
 //! The struct that takes care of encapsulating all the logic on where and how
 //! element styles need to be invalidated.
 
-use context::{SharedStyleContext, StackLimitChecker};
+use context::StackLimitChecker;
-use data::ElementData;
 use dom::{TElement, TNode};
 use selector_parser::SelectorImpl;
 use selectors::NthIndexCache;
-use selectors::matching::{MatchingContext, MatchingMode, VisitedHandlingMode};
+use selectors::matching::{MatchingContext, MatchingMode, QuirksMode, VisitedHandlingMode};
 use selectors::matching::CompoundSelectorMatchingResult;
 use selectors::matching::matches_compound_selector;
 use selectors::parser::{Combinator, Component, Selector};
@@ -18,9 +17,6 @@ use smallvec::SmallVec;
 use std::fmt;
 
 /// A trait to abstract the collection of invalidations for a given pass.
-///
-/// The `data` argument is a mutable reference to the element's style data, if
-/// any.
 pub trait InvalidationProcessor<E>
 where
     E: TElement,
@@ -36,64 +32,36 @@ where
     fn collect_invalidations(
         &mut self,
         element: E,
-        data: Option<&mut ElementData>,
         nth_index_cache: Option<&mut NthIndexCache>,
-        shared_context: &SharedStyleContext,
+        quirks_mode: QuirksMode,
         descendant_invalidations: &mut InvalidationVector,
         sibling_invalidations: &mut InvalidationVector,
     ) -> bool;
 
     /// Returns whether the invalidation process should process the descendants
     /// of the given element.
-    fn should_process_descendants(
+    fn should_process_descendants(&mut self, element: E) -> bool;
-        &mut self,
-        element: E,
-        data: Option<&mut ElementData>,
-    ) -> bool;
 
     /// Executes an arbitrary action when the recursion limit is exceded (if
     /// any).
-    fn recursion_limit_exceeded(
+    fn recursion_limit_exceeded(&mut self, element: E);
-        &mut self,
-        element: E,
-        data: Option<&mut ElementData>,
-    );
 
     /// Executes an action when `Self` is invalidated.
-    fn invalidated_self(
+    fn invalidated_self(&mut self, element: E);
-        &mut self,
-        element: E,
-        data: Option<&mut ElementData>,
-    );
 
     /// Executes an action when any descendant of `Self` is invalidated.
-    fn invalidated_descendants(
+    fn invalidated_descendants(&mut self, element: E, child: E);
-        &mut self,
-        element: E,
-        data: Option<&mut ElementData>,
-        child: E,
-    );
 }
 
 /// The struct that takes care of encapsulating all the logic on where and how
 /// element styles need to be invalidated.
-pub struct TreeStyleInvalidator<'a, 'b: 'a, E, P: 'a>
+pub struct TreeStyleInvalidator<'a, E, P: 'a>
 where
     E: TElement,
     P: InvalidationProcessor<E>
 {
     element: E,
-    // TODO(emilio): It's tempting enough to just avoid running invalidation for
+    quirks_mode: QuirksMode,
-    // elements without data.
-    //
-    // But that's be wrong for sibling invalidations when a new element has been
-    // inserted in the tree and still has no data (though I _think_ the slow
-    // selector bits save us, it'd be nice not to depend on them).
-    //
-    // Seems like we could at least avoid running invalidation for the
-    // descendants if an element has no data, though.
-    data: Option<&'a mut ElementData>,
-    shared_context: &'a SharedStyleContext<'b>,
     stack_limit_checker: Option<&'a StackLimitChecker>,
     nth_index_cache: Option<&'a mut NthIndexCache>,
     processor: &'a mut P,
@@ -224,7 +192,7 @@ impl InvalidationResult {
     }
 }
 
-impl<'a, 'b: 'a, E, P: 'a> TreeStyleInvalidator<'a, 'b, E, P>
+impl<'a, E, P: 'a> TreeStyleInvalidator<'a, E, P>
 where
     E: TElement,
     P: InvalidationProcessor<E>,
@@ -232,16 +200,14 @@ where
     /// Trivially constructs a new `TreeStyleInvalidator`.
     pub fn new(
         element: E,
-        data: Option<&'a mut ElementData>,
+        quirks_mode: QuirksMode,
-        shared_context: &'a SharedStyleContext<'b>,
         stack_limit_checker: Option<&'a StackLimitChecker>,
         nth_index_cache: Option<&'a mut NthIndexCache>,
         processor: &'a mut P,
     ) -> Self {
         Self {
             element,
-            data,
+            quirks_mode,
-            shared_context,
             stack_limit_checker,
             nth_index_cache,
             processor,
@@ -257,9 +223,8 @@ where
 
         let invalidated_self = self.processor.collect_invalidations(
             self.element,
-            self.data.as_mut().map(|d| &mut **d),
             self.nth_index_cache.as_mut().map(|c| &mut **c),
-            self.shared_context,
+            self.quirks_mode,
             &mut descendant_invalidations,
             &mut sibling_invalidations,
         );
@@ -291,12 +256,9 @@ where
         let mut any_invalidated = false;
 
         while let Some(sibling) = current {
-            let mut sibling_data = sibling.mutate_data();
-
             let mut sibling_invalidator = TreeStyleInvalidator::new(
                 sibling,
-                sibling_data.as_mut().map(|d| &mut **d),
+                self.quirks_mode,
-                self.shared_context,
                 self.stack_limit_checker,
                 self.nth_index_cache.as_mut().map(|c| &mut **c),
                 self.processor,
@@ -359,12 +321,9 @@ where
 
         let mut invalidated_child = false;
         let invalidated_descendants = {
-            let mut child_data = child.mutate_data();
-
             let mut child_invalidator = TreeStyleInvalidator::new(
                 child,
-                child_data.as_mut().map(|d| &mut **d),
+                self.quirks_mode,
-                self.shared_context,
                 self.stack_limit_checker,
                 self.nth_index_cache.as_mut().map(|c| &mut **c),
                 self.processor,
@@ -392,11 +351,7 @@ where
         // Since we keep the traversal flags in terms of the flattened tree,
         // we need to propagate it as appropriate.
         if invalidated_child || invalidated_descendants {
-            self.processor.invalidated_descendants(
+            self.processor.invalidated_descendants(self.element, child);
-                self.element,
-                self.data.as_mut().map(|d| &mut **d),
-                child,
-            );
         }
 
         invalidated_child || invalidated_descendants
@@ -427,7 +382,7 @@ where
         let mut any_descendant = false;
 
         let mut sibling_invalidations = InvalidationVector::new();
-        for child in parent.children() {
+        for child in parent.dom_children() {
             // TODO(emilio): We handle <xbl:children> fine, because they appear
             // in selector-matching (note bug 1374247, though).
             //
@@ -467,10 +422,7 @@ where
         debug!(" > {:?}", invalidations);
 
         let should_process =
-            self.processor.should_process_descendants(
+            self.processor.should_process_descendants(self.element);
-                self.element,
-                self.data.as_mut().map(|d| &mut **d),
-            );
 
         if !should_process {
             return false;
@@ -478,10 +430,7 @@ where
 
         if let Some(checker) = self.stack_limit_checker {
             if checker.limit_exceeded() {
-                self.processor.recursion_limit_exceeded(
+                self.processor.recursion_limit_exceeded(self.element);
-                    self.element,
-                    self.data.as_mut().map(|d| &mut **d)
-                );
                 return true;
             }
         }
@@ -609,7 +558,7 @@ where
                 None,
                 self.nth_index_cache.as_mut().map(|c| &mut **c),
                 VisitedHandlingMode::AllLinksVisitedAndUnvisited,
-                self.shared_context.quirks_mode(),
+                self.quirks_mode,
             );
 
             matches_compound_selector(
@@ -771,10 +720,7 @@ where
         }
 
         if invalidated_self {
-            self.processor.invalidated_self(
+            self.processor.invalidated_self(self.element);
-                self.element,
-                self.data.as_mut().map(|d| &mut **d),
-            );
         }
 
         SingleInvalidationResult { invalidated_self, matched, }

components/style/invalidation/stylesheets.rs

@@ -237,7 +237,7 @@ impl StylesheetInvalidationSet {
 
         let mut any_children_invalid = false;
 
-        for child in element.as_node().traversal_children() {
+        for child in element.traversal_children() {
             let child = match child.as_element() {
                 Some(e) => e,
                 None => continue,

components/style/traversal.rs

@@ -841,7 +841,7 @@ where
     let is_initial_style = context.thread_local.is_initial_style();
 
     // Loop over all the traversal children.
-    for child_node in element.as_node().traversal_children() {
+    for child_node in element.traversal_children() {
         let child = match child_node.as_element() {
             Some(el) => el,
             None => {
@@ -933,7 +933,7 @@ where
     let mut parents = SmallVec::<[E; 32]>::new();
     parents.push(root);
     while let Some(p) = parents.pop() {
-        for kid in p.as_node().traversal_children() {
+        for kid in p.traversal_children() {
             if let Some(kid) = kid.as_element() {
                 // We maintain an invariant that, if an element has data, all its
                 // ancestors have data as well.

ports/geckolib/glue.rs

@@ -3623,7 +3623,7 @@ pub extern "C" fn Servo_AssertTreeIsClean(root: RawGeckoElementBorrowed) {
         debug_assert!(!el.has_dirty_descendants() && !el.has_animation_only_dirty_descendants(),
                       "{:?} has still dirty bit {:?} or animation-only dirty bit {:?}",
                       el, el.has_dirty_descendants(), el.has_animation_only_dirty_descendants());
-        for child in el.as_node().traversal_children() {
+        for child in el.traversal_children() {
             if let Some(child) = child.as_element() {
                 assert_subtree_is_clean(child);
             }