diff --git a/components/layout_thread/dom_wrapper.rs b/components/layout_thread/dom_wrapper.rs index 08d628a6d6c..45db07f43de 100644 --- a/components/layout_thread/dom_wrapper.rs +++ b/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 { unsafe { @@ -167,20 +166,34 @@ impl<'ln> TNode for ServoLayoutNode<'ln> { } } - fn children(&self) -> LayoutIterator> { - LayoutIterator(ServoChildrenIterator { - current: self.first_child(), - }) + fn first_child(&self) -> Option { + unsafe { + self.node.first_child_ref().map(|node| self.new_with_this_lifetime(&node)) + } + } + + fn last_child(&self) -> Option { + unsafe { + self.node.last_child_ref().map(|node| self.new_with_this_lifetime(&node)) + } + } + + fn prev_sibling(&self) -> Option { + unsafe { + self.node.prev_sibling_ref().map(|node| self.new_with_this_lifetime(&node)) + } + } + + fn next_sibling(&self) -> Option { + unsafe { + self.node.next_sibling_ref().map(|node| self.new_with_this_lifetime(&node)) + } } fn traversal_parent(&self) -> Option> { self.parent_element() } - fn traversal_children(&self) -> LayoutIterator> { - self.children() - } - fn opaque(&self) -> OpaqueNode { unsafe { self.get_jsmanaged().opaque() } } @@ -206,19 +219,6 @@ impl<'ln> TNode for ServoLayoutNode<'ln> { } } -pub struct ServoChildrenIterator<'a> { - current: Option>, -} - -impl<'a> Iterator for ServoChildrenIterator<'a> { - type Item = ServoLayoutNode<'a>; - fn next(&mut self) -> Option> { - let node = self.current; - self.current = node.and_then(|node| node.next_sibling()); - node - } -} - impl<'ln> LayoutNode for ServoLayoutNode<'ln> { type ConcreteThreadSafeLayoutNode = ServoThreadSafeLayoutNode<'ln>; @@ -248,30 +248,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> { - unsafe { - self.node.first_child_ref().map(|node| self.new_with_this_lifetime(&node)) - } - } - - fn last_child(&self) -> Option> { - unsafe { - self.node.last_child_ref().map(|node| self.new_with_this_lifetime(&node)) - } - } - - fn prev_sibling(&self) -> Option> { - unsafe { - self.node.prev_sibling_ref().map(|node| self.new_with_this_lifetime(&node)) - } - } - - fn next_sibling(&self) -> Option> { - unsafe { - self.node.next_sibling_ref().map(|node| self.new_with_this_lifetime(&node)) - } - } } impl<'ln> GetLayoutData for ServoLayoutNode<'ln> { @@ -320,8 +296,8 @@ impl<'ld> ServoLayoutDocument<'ld> { ServoLayoutNode::from_layout_js(self.document.upcast()) } - pub fn root_node(&self) -> Option> { - self.as_node().children().find(ServoLayoutNode::is_element) + pub fn root_element(&self) -> Option> { + self.as_node().dom_children().flat_map(|n| n.as_element()).next() } pub fn drain_pending_restyles(&self) -> Vec<(ServoLayoutElement<'ld>, PendingRestyle)> { @@ -380,6 +356,7 @@ impl<'le> PresentationalHintsSynthesizer for ServoLayoutElement<'le> { impl<'le> TElement for ServoLayoutElement<'le> { type ConcreteNode = ServoLayoutNode<'le>; + type TraversalChildrenIterator = DomChildren; type FontMetricsProvider = ServoMetricsProvider; @@ -387,6 +364,10 @@ impl<'le> TElement for ServoLayoutElement<'le> { ServoLayoutNode::from_layout_js(self.element.upcast()) } + fn traversal_children(&self) -> LayoutIterator { + LayoutIterator(self.as_node().dom_children()) + } + fn style_attribute(&self) -> Option>> { unsafe { (*self.element.style_attribute()).as_ref().map(|x| x.borrow_arc()) @@ -629,7 +610,7 @@ impl<'le> ::selectors::Element for ServoLayoutElement<'le> { } fn first_child_element(&self) -> Option> { - 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> { @@ -690,7 +671,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 +831,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> { diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs index 3132eccf68a..528e06c6424 100644 --- a/components/layout_thread/lib.rs +++ b/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={:?})", diff --git a/components/script_layout_interface/wrapper_traits.rs b/components/script_layout_interface/wrapper_traits.rs index 8221a3f885c..a0571c66ff0 100644 --- a/components/script_layout_interface/wrapper_traits.rs +++ b/components/script_layout_interface/wrapper_traits.rs @@ -100,14 +100,6 @@ pub trait LayoutNode: Debug + GetLayoutData + TNode { fn traverse_preorder(self) -> TreeIterator { TreeIterator::new(self) } - - fn first_child(&self) -> Option; - - fn last_child(&self) -> Option; - - fn prev_sibling(&self) -> Option; - - fn next_sibling(&self) -> Option; } pub struct ReverseChildrenIterator where ConcreteNode: LayoutNode { diff --git a/components/style/dom.rs b/components/style/dom.rs index 1733b580184..91f51fdd7c2 100644 --- a/components/style/dom.rs +++ b/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(pub T); -impl Iterator for LayoutIterator - where T: Iterator, - I: NodeInfo, +impl Iterator for LayoutIterator +where + T: Iterator, + N: NodeInfo, { - type Item = I; - fn next(&mut self) -> Option { + type Item = N; + + fn next(&mut self) -> Option { 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 + match self.0.next() { + Some(n) => { + // Filter out nodes that layout should ignore. + 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(Option); +impl Iterator for DomChildren +where + N: TNode +{ + type Item = N; + + fn next(&mut self) -> Option { + 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; - /// 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; - /// Get this node's parent node. fn parent_node(&self) -> Option; - /// Get this node's parent element if present. - fn parent_element(&self) -> Option { - self.parent_node().and_then(|n| n.as_element()) - } + /// Get this node's first child. + fn first_child(&self) -> Option; - /// Returns an iterator over this node's children. - fn children(&self) -> LayoutIterator; + /// Get this node's first child. + fn last_child(&self) -> Option; + + /// Get this node's previous sibling. + fn prev_sibling(&self) -> Option; + + /// Get this node's next sibling. + fn next_sibling(&self) -> Option; + + /// Iterate over the DOM children of an element. + fn dom_children(&self) -> DomChildren { + DomChildren(self.first_child()) + } /// Get this node's parent element from the perspective of a restyle /// traversal. fn traversal_parent(&self) -> Option; - /// Get this node's children from the perspective of a restyle traversal. - fn traversal_children(&self) -> LayoutIterator; + /// Get this node's parent element if present. + fn parent_element(&self) -> Option { + self.parent_node().and_then(|n| n.as_element()) + } /// Converts self into an `OpaqueNode`. fn opaque(&self) -> OpaqueNode; @@ -223,9 +248,11 @@ fn fmt_subtree(f: &mut fmt::Formatter, stringify: &F, n: N, indent: write!(f, " ")?; } stringify(f, n)?; - for kid in n.traversal_children() { - writeln!(f, "")?; - fmt_subtree(f, stringify, kid, indent + 1)?; + if let Some(e) = n.as_element() { + for kid in e.traversal_children() { + writeln!(f, "")?; + fmt_subtree(f, stringify, kid, indent + 1)?; + } } Ok(()) @@ -256,6 +283,12 @@ pub trait TElement /// The concrete node type. type ConcreteNode: TNode; + /// 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; + /// Type of the font metrics provider /// /// XXXManishearth It would be better to make this a type parameter on @@ -295,6 +328,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; + /// Returns the parent element we should inherit from. /// /// This is pretty much always the parent element itself, except in the case diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index e259422c629..5b70f6207a0 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -158,32 +158,6 @@ impl<'ln> GeckoNode<'ln> { unsafe { &*self.node_info().mDocument } } - #[inline] - fn first_child(&self) -> Option> { - unsafe { self.0.mFirstChild.as_ref().map(GeckoNode::from_content) } - } - - #[inline] - fn last_child(&self) -> Option> { - unsafe { Gecko_GetLastChild(self.0).map(GeckoNode) } - } - - #[inline] - fn prev_sibling(&self) -> Option> { - unsafe { self.0.mPreviousSibling.as_ref().map(GeckoNode::from_content) } - } - - #[inline] - fn next_sibling(&self) -> Option> { - 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 { unsafe { self.0.mParent.as_ref().map(GeckoNode) } } - fn children(&self) -> LayoutIterator> { - LayoutIterator(self.dom_children()) + #[inline] + fn first_child(&self) -> Option { + unsafe { self.0.mFirstChild.as_ref().map(GeckoNode::from_content) } + } + + #[inline] + fn last_child(&self) -> Option { + unsafe { Gecko_GetLastChild(self.0).map(GeckoNode) } + } + + #[inline] + fn prev_sibling(&self) -> Option { + unsafe { self.0.mPreviousSibling.as_ref().map(GeckoNode::from_content) } + } + + #[inline] + fn next_sibling(&self) -> Option { + unsafe { self.0.mNextSibling.as_ref().map(GeckoNode::from_content) } } fn traversal_parent(&self) -> Option> { self.flattened_tree_parent().and_then(|n| n.as_element()) } - fn traversal_children(&self) -> LayoutIterator> { - 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) @@ -447,6 +411,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( value: &str, @@ -883,6 +852,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 { if self.is_native_anonymous() { @@ -894,6 +864,24 @@ impl<'le> TElement for GeckoElement<'le> { } } + fn traversal_children(&self) -> LayoutIterator> { + // 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.get_before_or_after_pseudo(/* is_before = */ true) } diff --git a/components/style/invalidation/element/invalidator.rs b/components/style/invalidation/element/invalidator.rs index 388fb36aae8..99e42e6d860 100644 --- a/components/style/invalidation/element/invalidator.rs +++ b/components/style/invalidation/element/invalidator.rs @@ -382,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 fine, because they appear // in selector-matching (note bug 1374247, though). // diff --git a/components/style/invalidation/stylesheets.rs b/components/style/invalidation/stylesheets.rs index c65dfc056bc..2e2abdf4960 100644 --- a/components/style/invalidation/stylesheets.rs +++ b/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, diff --git a/components/style/traversal.rs b/components/style/traversal.rs index d31d1abff0e..7737ce044f0 100644 --- a/components/style/traversal.rs +++ b/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. diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index 9e7c2f22f2d..33b95b53a09 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -3621,7 +3621,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); }