servo: Merge #13172 - stylo: avoid traversing non element/text nodes in style and layout (from bholley:display_enum); r=emilio
authorBobby Holley <bobbyholley@gmail.com>
Wed, 21 Sep 2016 19:59:52 -0500
changeset 339723 f463a3bce76d13280bbcbdc1840d89339aaed1a9
parent 339722 b5b07ccd7852f0ce7d239a9774734a3e90fb2319
child 339724 f58fc8ebdd683601b4d630f4e0a607bcdeb8b867
push id31307
push usergszorc@mozilla.com
push dateSat, 04 Feb 2017 00:59:06 +0000
treeherdermozilla-central@94079d43835f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
servo: Merge #13172 - stylo: avoid traversing non element/text nodes in style and layout (from bholley:display_enum); r=emilio r? @emilio Source-Repo: https://github.com/servo/servo Source-Revision: 614e9ca840baacfa427ec78db99258a83b75e135
servo/components/script/layout_wrapper.rs
servo/components/script_layout_interface/wrapper_traits.rs
servo/components/style/dom.rs
servo/components/style/matching.rs
servo/ports/geckolib/glue.rs
servo/ports/geckolib/wrapper.rs
--- a/servo/components/script/layout_wrapper.rs
+++ b/servo/components/script/layout_wrapper.rs
@@ -53,17 +53,18 @@ use std::fmt;
 use std::marker::PhantomData;
 use std::mem::transmute;
 use std::sync::Arc;
 use string_cache::{Atom, Namespace};
 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};
 use style::selector_impl::{ElementSnapshot, NonTSPseudoClass, PseudoElement, ServoSelectorImpl};
 use style::selector_matching::ApplicableDeclarationBlock;
 use style::sink::Push;
 use style::str::is_whitespace;
 use url::Url;
@@ -106,16 +107,28 @@ impl<'ln> ServoLayoutNode<'ln> {
 
     fn script_type_id(&self) -> NodeTypeId {
         unsafe {
             self.node.type_id_for_layout()
         }
     }
 }
 
+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>;
     type ConcreteRestyleDamage = RestyleDamage;
     type ConcreteChildrenIterator = ServoChildrenIterator<'ln>;
 
     fn to_unsafe(&self) -> UnsafeNode {
         unsafe {
@@ -123,39 +136,29 @@ impl<'ln> TNode for ServoLayoutNode<'ln>
         }
     }
 
     unsafe fn from_unsafe(n: &UnsafeNode) -> Self {
         let (node, _) = *n;
         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);
     }
 
     fn dump_style(self) {
         println!("\nDOM with computed styles:");
         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 {
         unsafe { self.get_jsmanaged().opaque() }
     }
 
     fn layout_parent_node(self, reflow_root: OpaqueNode) -> Option<ServoLayoutNode<'ln>> {
         if self.opaque() == reflow_root {
@@ -722,16 +725,30 @@ impl<'ln> ServoThreadSafeLayoutNode<'ln>
 
     /// Returns the interior of this node as a `LayoutJS`. This is highly unsafe for layout to
     /// call and as such is marked `unsafe`.
     unsafe fn get_jsmanaged(&self) -> &LayoutJS<Node> {
         self.node.get_jsmanaged()
     }
 }
 
+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>;
 
     fn with_pseudo(&self,
                    pseudo: PseudoElementType<Option<display::T>>) -> ServoThreadSafeLayoutNode<'ln> {
         ServoThreadSafeLayoutNode {
             node: self.node.clone(),
@@ -755,18 +772,18 @@ impl<'ln> ThreadSafeLayoutNode for Servo
     fn type_id_without_excluding_pseudo_elements(&self) -> LayoutNodeType {
         self.node.type_id()
     }
 
     fn debug_id(self) -> usize {
         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> {
         unsafe {
             let element = match self.get_jsmanaged().downcast() {
                 Some(e) => e.unsafe_get(),
                 None => panic!("not an element")
             };
--- a/servo/components/script_layout_interface/wrapper_traits.rs
+++ b/servo/components/script_layout_interface/wrapper_traits.rs
@@ -10,17 +10,17 @@ use gfx_traits::{ByteIndex, LayerId, Lay
 use msg::constellation_msg::PipelineId;
 use range::Range;
 use restyle_damage::RestyleDamage;
 use std::fmt::Debug;
 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};
 use style::selector_impl::{PseudoElement, PseudoElementCascadeType, ServoSelectorImpl};
 use url::Url;
 
 #[derive(Copy, PartialEq, Clone)]
 pub enum PseudoElementType<T> {
@@ -76,20 +76,20 @@ pub trait LayoutNode: TNode {
     /// Returns the type ID of this node.
     fn type_id(&self) -> LayoutNodeType;
 
     fn get_style_data(&self) -> Option<&RefCell<PartialStyleAndLayoutData>>;
 
     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> {
         TreeIterator::new(self)
     }
 }
 
 pub struct ReverseChildrenIterator<ConcreteNode> where ConcreteNode: TNode {
@@ -132,17 +132,17 @@ impl<ConcreteNode> Iterator for TreeIter
         ret.map(|node| self.stack.extend(node.rev_children()));
         ret
     }
 }
 
 
 /// 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>;
     type ChildrenIterator: Iterator<Item = Self> + Sized;
 
     /// Creates a new `ThreadSafeLayoutNode` for the same `LayoutNode`
     /// with a different pseudo-element type.
     fn with_pseudo(&self, pseudo: PseudoElementType<Option<display::T>>) -> Self;
@@ -164,20 +164,17 @@ pub trait ThreadSafeLayoutNode: Clone + 
             LayoutNodeType::Element(..) => true,
             _ => false,
         }
     }
 
     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]
     fn as_element(&self) -> Self::ConcreteThreadSafeLayoutElement;
 
     #[inline]
     fn get_pseudo_element_type(&self) -> PseudoElementType<Option<display::T>>;
 
--- a/servo/components/style/dom.rs
+++ b/servo/components/style/dom.rs
@@ -59,38 +59,57 @@ pub trait TRestyleDamage : Debug + Parti
     fn compute(old: &Self::PreExistingComputedValues,
                new: &Arc<ComputedValues>) -> Self;
 
     fn empty() -> Self;
 
     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;
     type ConcreteChildrenIterator: Iterator<Item = Self>;
 
     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;
 
     /// While doing a reflow, the node at the root has no parent, as far as we're
     /// concerned. This method returns `None` at the reflow root.
     fn layout_parent_node(self, reflow_root: OpaqueNode) -> Option<Self>;
 
--- a/servo/components/style/matching.rs
+++ b/servo/components/style/matching.rs
@@ -7,17 +7,17 @@
 #![allow(unsafe_code)]
 
 use animation;
 use arc_ptr_eq;
 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};
 use selector_matching::{ApplicableDeclarationBlock, Stylist};
 use selectors::{Element, MatchAttr};
 use selectors::bloom::BloomFilter;
 use selectors::matching::{AFFECTED_BY_PSEUDO_ELEMENTS, MatchingReason, StyleRelations};
 use sink::ForgetfulSink;
--- a/servo/ports/geckolib/glue.rs
+++ b/servo/ports/geckolib/glue.rs
@@ -27,17 +27,17 @@ use snapshot::GeckoElementSnapshot;
 use std::mem::transmute;
 use std::ptr;
 use std::slice;
 use std::str::from_utf8_unchecked;
 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;
 use style::parser::ParserContextExtraData;
 use style::properties::{ComputedValues, PropertyDeclarationBlock, parse_one_declaration};
 use style::selector_impl::PseudoElementCascadeType;
 use style::sequential;
 use style::stylesheets::{Origin, Stylesheet};
--- a/servo/ports/geckolib/wrapper.rs
+++ b/servo/ports/geckolib/wrapper.rs
@@ -34,18 +34,18 @@ use selectors::Element;
 use selectors::parser::{AttrSelector, NamespaceConstraint};
 use snapshot::GeckoElementSnapshot;
 use snapshot_helpers;
 use std::fmt;
 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};
 use style::parser::ParserContextExtraData;
 use style::properties::{ComputedValues, parse_style_attribute};
 use style::properties::PropertyDeclarationBlock;
 use style::refcell::{Ref, RefCell, RefMut};
 use style::selector_impl::ElementExt;
@@ -120,57 +120,58 @@ impl BitOr for GeckoRestyleDamage {
 
     fn bitor(self, other: Self) -> Self {
         use std::mem;
         GeckoRestyleDamage(unsafe { mem::transmute(self.0 as u32 | other.0 as u32) })
     }
 }
 
 
+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>;
     type ConcreteElement = GeckoElement<'ln>;
     type ConcreteRestyleDamage = GeckoRestyleDamage;
     type ConcreteChildrenIterator = GeckoChildrenIterator<'ln>;
 
     fn to_unsafe(&self) -> UnsafeNode {
         (self.0 as *const _ as usize, 0)
     }
 
     unsafe fn from_unsafe(n: &UnsafeNode) -> Self {
         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!()
     }
 
     fn dump_style(self) {
         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()))
         }
     }
 
     fn opaque(&self) -> OpaqueNode {
         let ptr: uintptr_t = self.0 as *const _ as uintptr_t;
         OpaqueNode(ptr)
     }