Auto merge of #13172 - bholley:display_enum, r=emilio

stylo: avoid traversing non element/text nodes in style and layout

r? @emilio

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

6 files changed, 86 insertions, 52 deletions

diff --git a/components/script/layout_wrapper.rs b/components/script/layout_wrapper.rs
index 5411deb91a8..19e6c7c4e79 100644
--- a/components/script/layout_wrapper.rs
+++ b/components/script/layout_wrapper.rs
@@ -58,7 +58,8 @@ use style::attr::AttrValue;
 use style::computed_values::display;
 use style::context::SharedStyleContext;
 use style::data::PrivateStyleData;
-use style::dom::{OpaqueNode, PresentationalHintsSynthetizer, TDocument, TElement, TNode, UnsafeNode};
+use style::dom::{LayoutIterator, NodeInfo, OpaqueNode, PresentationalHintsSynthetizer, TDocument, TElement, TNode};
+use style::dom::UnsafeNode;
 use style::element_state::*;
 use style::properties::{ComputedValues, PropertyDeclarationBlock};
 use style::refcell::{Ref, RefCell, RefMut};
@@ -111,6 +112,18 @@ impl<'ln> ServoLayoutNode<'ln> {
     }
 }
 
+impl<'ln> NodeInfo for ServoLayoutNode<'ln> {
+    fn is_element(&self) -> bool {
+        unsafe {
+            self.node.is_element_for_layout()
+        }
+    }
+
+    fn is_text_node(&self) -> bool {
+        self.script_type_id() == NodeTypeId::CharacterData(CharacterDataTypeId::Text)
+    }
+}
+
 impl<'ln> TNode for ServoLayoutNode<'ln> {
     type ConcreteElement = ServoLayoutElement<'ln>;
     type ConcreteDocument = ServoLayoutDocument<'ln>;
@@ -128,16 +141,6 @@ impl<'ln> TNode for ServoLayoutNode<'ln> {
         transmute(node)
     }
 
-    fn is_text_node(&self) -> bool {
-        self.script_type_id() == NodeTypeId::CharacterData(CharacterDataTypeId::Text)
-    }
-
-    fn is_element(&self) -> bool {
-        unsafe {
-            self.node.is_element_for_layout()
-        }
-    }
-
     fn dump(self) {
         self.dump_indent(0);
     }
@@ -147,10 +150,10 @@ impl<'ln> TNode for ServoLayoutNode<'ln> {
         self.dump_style_indent(0);
     }
 
-    fn children(self) -> ServoChildrenIterator<'ln> {
-        ServoChildrenIterator {
+    fn children(self) -> LayoutIterator<ServoChildrenIterator<'ln>> {
+        LayoutIterator(ServoChildrenIterator {
             current: self.first_child(),
-        }
+        })
     }
 
     fn opaque(&self) -> OpaqueNode {
@@ -727,6 +730,20 @@ 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 }
+    }
+
+    fn is_text_node(&self) -> bool {
+        if let Some(LayoutNodeType::Text) = self.type_id() { true } else { false }
+    }
+
+    fn needs_layout(&self) -> bool {
+        self.pseudo != PseudoElementType::Normal || self.is_element() || self.is_text_node()
+    }
+}
+
 impl<'ln> ThreadSafeLayoutNode for ServoThreadSafeLayoutNode<'ln> {
     type ConcreteThreadSafeLayoutElement = ServoThreadSafeLayoutElement<'ln>;
     type ChildrenIterator = ThreadSafeLayoutNodeChildrenIterator<Self>;
@@ -760,8 +777,8 @@ impl<'ln> ThreadSafeLayoutNode for ServoThreadSafeLayoutNode<'ln> {
         self.node.debug_id()
     }
 
-    fn children(&self) -> Self::ChildrenIterator {
-        ThreadSafeLayoutNodeChildrenIterator::new(*self)
+    fn children(&self) -> LayoutIterator<Self::ChildrenIterator> {
+        LayoutIterator(ThreadSafeLayoutNodeChildrenIterator::new(*self))
     }
 
     fn as_element(&self) -> ServoThreadSafeLayoutElement<'ln> {
diff --git a/components/script_layout_interface/wrapper_traits.rs b/components/script_layout_interface/wrapper_traits.rs
index 20ddca94cf4..dcb1f68cfb4 100644
--- a/components/script_layout_interface/wrapper_traits.rs
+++ b/components/script_layout_interface/wrapper_traits.rs
@@ -15,7 +15,7 @@ use std::sync::Arc;
 use string_cache::{Atom, Namespace};
 use style::computed_values::display;
 use style::context::SharedStyleContext;
-use style::dom::{PresentationalHintsSynthetizer, TNode};
+use style::dom::{LayoutIterator, NodeInfo, PresentationalHintsSynthetizer, TNode};
 use style::dom::OpaqueNode;
 use style::properties::ServoComputedValues;
 use style::refcell::{Ref, RefCell};
@@ -81,10 +81,10 @@ pub trait LayoutNode: TNode {
     fn init_style_and_layout_data(&self, data: OpaqueStyleAndLayoutData);
     fn get_style_and_layout_data(&self) -> Option<OpaqueStyleAndLayoutData>;
 
-    fn rev_children(self) -> ReverseChildrenIterator<Self> {
-        ReverseChildrenIterator {
+    fn rev_children(self) -> LayoutIterator<ReverseChildrenIterator<Self>> {
+        LayoutIterator(ReverseChildrenIterator {
             current: self.last_child(),
-        }
+        })
     }
 
     fn traverse_preorder(self) -> TreeIterator<Self> {
@@ -137,7 +137,7 @@ impl<ConcreteNode> Iterator for TreeIterator<ConcreteNode>
 
 /// A thread-safe version of `LayoutNode`, used during flow construction. This type of layout
 /// node does not allow any parents or siblings of nodes to be accessed, to avoid races.
-pub trait ThreadSafeLayoutNode: Clone + Copy + Sized + PartialEq {
+pub trait ThreadSafeLayoutNode: Clone + Copy + NodeInfo + PartialEq + Sized {
     type ConcreteThreadSafeLayoutElement:
         ThreadSafeLayoutElement<ConcreteThreadSafeLayoutNode = Self>
         + ::selectors::Element<Impl=ServoSelectorImpl>;
@@ -169,10 +169,7 @@ pub trait ThreadSafeLayoutNode: Clone + Copy + Sized + PartialEq {
     fn debug_id(self) -> usize;
 
     /// Returns an iterator over this node's children.
-    fn children(&self) -> Self::ChildrenIterator;
-
-    #[inline]
-    fn is_element(&self) -> bool { if let Some(LayoutNodeType::Element(_)) = self.type_id() { true } else { false } }
+    fn children(&self) -> LayoutIterator<Self::ChildrenIterator>;
 
     /// If this is an element, accesses the element data. Fails if this is not an element node.
     #[inline]
diff --git a/components/style/dom.rs b/components/style/dom.rs
index 797675388fb..ec18ae65db6 100644
--- a/components/style/dom.rs
+++ b/components/style/dom.rs
@@ -64,7 +64,33 @@ pub trait TRestyleDamage : Debug + PartialEq + BitOr<Output=Self> + Copy {
     fn rebuild_and_reflow() -> Self;
 }
 
-pub trait TNode : Sized + Copy + Clone {
+/// Simple trait to provide basic information about the type of an element.
+///
+/// We avoid exposing the full type id, since computing it in the general case
+/// would be difficult for Gecko nodes.
+pub trait NodeInfo {
+    fn is_element(&self) -> bool;
+    fn is_text_node(&self) -> bool;
+
+    // Comments, doctypes, etc are ignored by layout algorithms.
+    fn needs_layout(&self) -> bool { self.is_element() || self.is_text_node() }
+}
+
+pub struct LayoutIterator<T>(pub T);
+impl<T, I> Iterator for LayoutIterator<T> where T: Iterator<Item=I>, I: NodeInfo {
+    type Item = I;
+    fn next(&mut self) -> Option<I> {
+        loop {
+            // Filter out nodes that layout should ignore.
+            let n = self.0.next();
+            if n.is_none() || n.as_ref().unwrap().needs_layout() {
+                return n
+            }
+        }
+    }
+}
+
+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;
@@ -73,19 +99,12 @@ pub trait TNode : Sized + Copy + Clone {
     fn to_unsafe(&self) -> UnsafeNode;
     unsafe fn from_unsafe(n: &UnsafeNode) -> Self;
 
-    /// Returns whether this is a text node. It turns out that this is all the style system cares
-    /// about, and thus obviates the need to compute the full type id, which would be expensive in
-    /// Gecko.
-    fn is_text_node(&self) -> bool;
-
-    fn is_element(&self) -> bool;
-
     fn dump(self);
 
     fn dump_style(self);
 
     /// Returns an iterator over this node's children.
-    fn children(self) -> Self::ConcreteChildrenIterator;
+    fn children(self) -> LayoutIterator<Self::ConcreteChildrenIterator>;
 
     /// Converts self into an `OpaqueNode`.
     fn opaque(&self) -> OpaqueNode;
diff --git a/components/style/matching.rs b/components/style/matching.rs
index 4edf85d1d3c..1ac8918bc1f 100644
--- a/components/style/matching.rs
+++ b/components/style/matching.rs
@@ -12,7 +12,7 @@ use cache::{LRUCache, SimpleHashCache};
 use cascade_info::CascadeInfo;
 use context::{SharedStyleContext, StyleContext};
 use data::PrivateStyleData;
-use dom::{TElement, TNode, TRestyleDamage, UnsafeNode};
+use dom::{NodeInfo, TElement, TNode, TRestyleDamage, UnsafeNode};
 use properties::{ComputedValues, PropertyDeclarationBlock, cascade};
 use properties::longhands::display::computed_value as display;
 use selector_impl::{PseudoElement, TheSelectorImpl};
diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs
index 6241616746a..d48d01f42e7 100644
--- a/ports/geckolib/glue.rs
+++ b/ports/geckolib/glue.rs
@@ -32,7 +32,7 @@ use std::sync::{Arc, Mutex};
 use std::sync::atomic::{AtomicBool, AtomicPtr, Ordering};
 use style::arc_ptr_eq;
 use style::context::{LocalStyleContextCreationInfo, ReflowGoal, SharedStyleContext};
-use style::dom::{TDocument, TElement, TNode};
+use style::dom::{NodeInfo, TDocument, TElement, TNode};
 use style::error_reporting::StdoutErrorReporter;
 use style::gecko_selector_impl::{GeckoSelectorImpl, PseudoElement};
 use style::parallel;
diff --git a/ports/geckolib/wrapper.rs b/ports/geckolib/wrapper.rs
index ed2da6f48b1..8255c13755c 100644
--- a/ports/geckolib/wrapper.rs
+++ b/ports/geckolib/wrapper.rs
@@ -39,8 +39,8 @@ use std::ops::BitOr;
 use std::ptr;
 use std::sync::Arc;
 use style::data::PrivateStyleData;
+use style::dom::{LayoutIterator, NodeInfo, TDocument, TElement, TNode, TRestyleDamage, UnsafeNode};
 use style::dom::{OpaqueNode, PresentationalHintsSynthetizer};
-use style::dom::{TDocument, TElement, TNode, TRestyleDamage, UnsafeNode};
 use style::element_state::ElementState;
 use style::error_reporting::StdoutErrorReporter;
 use style::gecko_selector_impl::{GeckoSelectorImpl, NonTSPseudoClass, PseudoElement};
@@ -125,6 +125,19 @@ impl BitOr for GeckoRestyleDamage {
 }
 
 
+impl<'ln> NodeInfo for GeckoNode<'ln> {
+    fn is_element(&self) -> bool {
+        unsafe {
+            Gecko_NodeIsElement(self.0)
+        }
+    }
+
+    fn is_text_node(&self) -> bool {
+        unsafe {
+            Gecko_IsTextNode(self.0)
+        }
+    }
+}
 
 impl<'ln> TNode for GeckoNode<'ln> {
     type ConcreteDocument = GeckoDocument<'ln>;
@@ -140,18 +153,6 @@ impl<'ln> TNode for GeckoNode<'ln> {
         GeckoNode(&*(n.0 as *mut RawGeckoNode))
     }
 
-    fn is_text_node(&self) -> bool {
-        unsafe {
-            Gecko_IsTextNode(self.0)
-        }
-    }
-
-    fn is_element(&self) -> bool {
-        unsafe {
-            Gecko_NodeIsElement(self.0)
-        }
-    }
-
     fn dump(self) {
         unimplemented!()
     }
@@ -160,12 +161,12 @@ impl<'ln> TNode for GeckoNode<'ln> {
         unimplemented!()
     }
 
-    fn children(self) -> GeckoChildrenIterator<'ln> {
+    fn children(self) -> LayoutIterator<GeckoChildrenIterator<'ln>> {
         let maybe_iter = unsafe { Gecko_MaybeCreateStyleChildrenIterator(self.0) };
         if let Some(iter) = maybe_iter.into_owned_opt() {
-            GeckoChildrenIterator::GeckoIterator(iter)
+            LayoutIterator(GeckoChildrenIterator::GeckoIterator(iter))
         } else {
-            GeckoChildrenIterator::Current(self.first_child())
+            LayoutIterator(GeckoChildrenIterator::Current(self.first_child()))
         }
     }