From 3c160750e2a3943f73019f7d4884630d9a205f56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 17 Oct 2017 05:18:29 -0500 Subject: [PATCH] servo: Merge #18904 - style: Stop threading the ElementData around the invalidator (from emilio:invalidator-less-dependencies); r=heycam,jdm,nox style: Stop threading the ElementData around the invalidator. Source-Repo: https://github.com/servo/servo Source-Revision: c1e0889971582488ed7a4d3a3af21a49bf497abc --HG-- extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear extra : subtree_revision : e7648a93a0b2ee03a02355cff9d6a4d7af9169b8 --- servo/components/layout_thread/dom_wrapper.rs | 100 ++++++----------- servo/components/layout_thread/lib.rs | 4 +- .../script_layout_interface/wrapper_traits.rs | 20 ---- servo/components/style/data.rs | 10 +- servo/components/style/dom.rs | 102 ++++++++++++------ servo/components/style/gecko/wrapper.rs | 100 ++++++++--------- .../style/invalidation/element/collector.rs | 85 +++++++-------- .../style/invalidation/element/invalidator.rs | 96 ++++------------- .../style/invalidation/stylesheets.rs | 2 +- servo/components/style/traversal.rs | 4 +- servo/ports/geckolib/glue.rs | 2 +- 11 files changed, 219 insertions(+), 306 deletions(-) diff --git a/servo/components/layout_thread/dom_wrapper.rs b/servo/components/layout_thread/dom_wrapper.rs index 08d628a6d6ce..f19e5e13b787 100644 --- a/servo/components/layout_thread/dom_wrapper.rs +++ b/servo/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() } } @@ -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>, -} - -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> { @@ -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> { - 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 +292,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 +352,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 +360,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 +606,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 +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 { let parent = self.node.parent_node().unwrap().as_element().unwrap(); let parent_data = parent.get_data().unwrap().borrow(); diff --git a/servo/components/layout_thread/lib.rs b/servo/components/layout_thread/lib.rs index 3132eccf68ac..528e06c6424d 100644 --- a/servo/components/layout_thread/lib.rs +++ b/servo/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/servo/components/script_layout_interface/wrapper_traits.rs b/servo/components/script_layout_interface/wrapper_traits.rs index 8221a3f885c3..b94ee469ecb6 100644 --- a/servo/components/script_layout_interface/wrapper_traits.rs +++ b/servo/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 { @@ -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; - /// 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; - #[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.as_element().and_then(|el| el.get_before_pseudo()).map(|el| el.as_node()) } diff --git a/servo/components/style/data.rs b/servo/components/style/data.rs index 7e50a67dbb30..b9aed397f235 100644 --- a/servo/components/style/data.rs +++ b/servo/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, + shared_context.quirks_mode(), stack_limit_checker, nth_index_cache, &mut processor, diff --git a/servo/components/style/dom.rs b/servo/components/style/dom.rs index 1733b580184f..766b217863fd 100644 --- a/servo/components/style/dom.rs +++ b/servo/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; @@ -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: &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 +279,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 +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; + /// Returns the parent element we should inherit from. /// /// This is pretty much always the parent element itself, except in the case diff --git a/servo/components/style/gecko/wrapper.rs b/servo/components/style/gecko/wrapper.rs index e259422c6298..3ba4b5d470f6 100644 --- a/servo/components/style/gecko/wrapper.rs +++ b/servo/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) @@ -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( 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 { if self.is_native_anonymous() { @@ -894,6 +860,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/servo/components/style/invalidation/element/collector.rs b/servo/components/style/invalidation/element/collector.rs index b1971bfab196..ff74ad9f3a24 100644 --- a/servo/components/style/invalidation/element/collector.rs +++ b/servo/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 InvalidationProcessor 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 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(); - data.hint.insert(RestyleHint::restyle_subtree()); + self.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 { - data.hint.insert(RESTYLE_SELF); - } + self.data.hint.insert(RESTYLE_SELF); } invalidated_self } - fn should_process_descendants( - &mut self, - _element: E, - data: Option<&mut ElementData>, - ) -> bool { - let data = match data { + fn should_process_descendants(&mut self, element: E) -> bool { + if element == self.element { + return !self.data.styles.is_display_none() && + !self.data.hint.contains(RESTYLE_DESCENDANTS) + } + + 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( - &mut self, - _element: E, - data: Option<&mut ElementData>, - ) { - if let Some(data) = data { + fn recursion_limit_exceeded(&mut self, element: E) { + if element == self.element { + self.data.hint.insert(RESTYLE_DESCENDANTS); + return; + } + + if let Some(mut data) = element.mutate_data() { data.hint.insert(RESTYLE_DESCENDANTS); } } - fn invalidated_descendants( - &mut self, - element: E, - data: Option<&mut ElementData>, - child: E, - ) { + fn invalidated_descendants(&mut self, element: E, 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( - &mut self, - _element: E, - data: Option<&mut ElementData>, - ) { - if let Some(data) = data { + fn invalidated_self(&mut self, element: E) { + debug_assert_ne!(element, self.element); + if let Some(mut data) = element.mutate_data() { data.hint.insert(RESTYLE_SELF); } } diff --git a/servo/components/style/invalidation/element/invalidator.rs b/servo/components/style/invalidation/element/invalidator.rs index 97edd7a42032..99e42e6d8600 100644 --- a/servo/components/style/invalidation/element/invalidator.rs +++ b/servo/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 data::ElementData; +use context::StackLimitChecker; 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 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( - &mut self, - element: E, - data: Option<&mut ElementData>, - ) -> bool; + fn should_process_descendants(&mut self, element: E) -> bool; /// Executes an arbitrary action when the recursion limit is exceded (if /// any). - fn recursion_limit_exceeded( - &mut self, - element: E, - data: Option<&mut ElementData>, - ); + fn recursion_limit_exceeded(&mut self, element: E); /// Executes an action when `Self` is invalidated. - fn invalidated_self( - &mut self, - element: E, - data: Option<&mut ElementData>, - ); + fn invalidated_self(&mut self, element: E); /// Executes an action when any descendant of `Self` is invalidated. - fn invalidated_descendants( - &mut self, - element: E, - data: Option<&mut ElementData>, - child: E, - ); + fn invalidated_descendants(&mut self, element: E, 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 { element: E, - // TODO(emilio): It's tempting enough to just avoid running invalidation for - // 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>, + quirks_mode: QuirksMode, 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, @@ -232,16 +200,14 @@ where /// Trivially constructs a new `TreeStyleInvalidator`. pub fn new( element: E, - data: Option<&'a mut ElementData>, - shared_context: &'a SharedStyleContext<'b>, + quirks_mode: QuirksMode, stack_limit_checker: Option<&'a StackLimitChecker>, nth_index_cache: Option<&'a mut NthIndexCache>, processor: &'a mut P, ) -> Self { Self { element, - data, - shared_context, + quirks_mode, 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.shared_context, + self.quirks_mode, 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.shared_context, + self.quirks_mode, 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.element, - self.data.as_mut().map(|d| &mut **d), - child, - ); + self.processor.invalidated_descendants(self.element, 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 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.element, - self.data.as_mut().map(|d| &mut **d), - ); + self.processor.should_process_descendants(self.element); 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.element, - self.data.as_mut().map(|d| &mut **d) - ); + self.processor.recursion_limit_exceeded(self.element); 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.element, - self.data.as_mut().map(|d| &mut **d), - ); + self.processor.invalidated_self(self.element); } SingleInvalidationResult { invalidated_self, matched, } diff --git a/servo/components/style/invalidation/stylesheets.rs b/servo/components/style/invalidation/stylesheets.rs index c65dfc056bcc..2e2abdf49608 100644 --- a/servo/components/style/invalidation/stylesheets.rs +++ b/servo/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/servo/components/style/traversal.rs b/servo/components/style/traversal.rs index d31d1abff0ec..7737ce044f01 100644 --- a/servo/components/style/traversal.rs +++ b/servo/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/servo/ports/geckolib/glue.rs b/servo/ports/geckolib/glue.rs index c1b0f7a8445c..924c629ae511 100644 --- a/servo/ports/geckolib/glue.rs +++ b/servo/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); }