From 97fd61f51260c273882e37cad76156d84ce909a3 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Tue, 25 Oct 2016 13:52:27 -0700 Subject: [PATCH 1/5] Eliminate untraversed node types from LayoutNodeType. MozReview-Commit-ID: 926ReI1BSsf --- components/layout/construct.rs | 12 ------------ components/script/dom/node.rs | 11 +---------- components/script/layout_wrapper.rs | 7 ++++--- components/script_layout_interface/lib.rs | 5 ----- 4 files changed, 5 insertions(+), 30 deletions(-) diff --git a/components/layout/construct.rs b/components/layout/construct.rs index d3a0c63e7a6..7c7eff8bc0c 100644 --- a/components/layout/construct.rs +++ b/components/layout/construct.rs @@ -1472,13 +1472,6 @@ impl<'a, ConcreteThreadSafeLayoutNode> PostorderNodeMutTraversal (display::T::inline, float::T::none, position::T::static_), - Some(LayoutNodeType::Comment) | - Some(LayoutNodeType::ProcessingInstruction) | - Some(LayoutNodeType::DocumentType) | - Some(LayoutNodeType::DocumentFragment) | - Some(LayoutNodeType::Document) => { - (display::T::none, float::T::none, position::T::static_) - } }; debug!("building flow for node: {:?} {:?} {:?} {:?}", display, float, positioning, node.type_id()); @@ -1615,12 +1608,7 @@ impl NodeUtils for ConcreteThreadSafeLayoutNode where ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode { fn is_replaced_content(&self) -> bool { match self.type_id() { - Some(LayoutNodeType::Comment) | - Some(LayoutNodeType::ProcessingInstruction) | Some(LayoutNodeType::Text) | - Some(LayoutNodeType::DocumentType) | - Some(LayoutNodeType::DocumentFragment) | - Some(LayoutNodeType::Document) | Some(LayoutNodeType::Element(LayoutElementType::HTMLImageElement)) | Some(LayoutNodeType::Element(LayoutElementType::HTMLIFrameElement)) | Some(LayoutNodeType::Element(LayoutElementType::HTMLCanvasElement)) | diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs index c9275306e53..ec2c45bfe8c 100644 --- a/components/script/dom/node.rs +++ b/components/script/dom/node.rs @@ -2654,20 +2654,11 @@ impl Into for NodeTypeId { #[inline(always)] fn into(self) -> LayoutNodeType { match self { - NodeTypeId::CharacterData(CharacterDataTypeId::Comment) => - LayoutNodeType::Comment, - NodeTypeId::Document(..) => - LayoutNodeType::Document, - NodeTypeId::DocumentFragment => - LayoutNodeType::DocumentFragment, - NodeTypeId::DocumentType => - LayoutNodeType::DocumentType, NodeTypeId::Element(e) => LayoutNodeType::Element(e.into()), - NodeTypeId::CharacterData(CharacterDataTypeId::ProcessingInstruction) => - LayoutNodeType::ProcessingInstruction, NodeTypeId::CharacterData(CharacterDataTypeId::Text) => LayoutNodeType::Text, + x => unreachable!("Layout should not traverse nodes of type {:?}", x), } } } diff --git a/components/script/layout_wrapper.rs b/components/script/layout_wrapper.rs index 4d615d2a50e..cad48e36a64 100644 --- a/components/script/layout_wrapper.rs +++ b/components/script/layout_wrapper.rs @@ -783,15 +783,16 @@ 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 } + self.pseudo == PseudoElementType::Normal && self.node.is_element() } fn is_text_node(&self) -> bool { - if let Some(LayoutNodeType::Text) = self.type_id() { true } else { false } + self.pseudo == PseudoElementType::Normal && self.node.is_text_node() } fn needs_layout(&self) -> bool { - self.pseudo != PseudoElementType::Normal || self.is_element() || self.is_text_node() + self.pseudo != PseudoElementType::Normal || + self.node.is_element() || self.node.is_text_node() } } diff --git a/components/script_layout_interface/lib.rs b/components/script_layout_interface/lib.rs index 120f794c249..3ad565d0c78 100644 --- a/components/script_layout_interface/lib.rs +++ b/components/script_layout_interface/lib.rs @@ -107,12 +107,7 @@ impl DomParallelInfo { #[derive(Copy, Clone, PartialEq, Eq, Debug)] pub enum LayoutNodeType { - Comment, - Document, - DocumentFragment, - DocumentType, Element(LayoutElementType), - ProcessingInstruction, Text, } From a5cabda484a145738ccdb720568862d41b616fb1 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Tue, 25 Oct 2016 15:56:29 -0700 Subject: [PATCH 2/5] Teach Servo to compute text node style from layout. MozReview-Commit-ID: IolVN5puF1i --- components/script/layout_wrapper.rs | 8 +++++ .../script_layout_interface/wrapper_traits.rs | 31 ++++++++++++++++--- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/components/script/layout_wrapper.rs b/components/script/layout_wrapper.rs index cad48e36a64..1f715399d4a 100644 --- a/components/script/layout_wrapper.rs +++ b/components/script/layout_wrapper.rs @@ -825,6 +825,14 @@ impl<'ln> ThreadSafeLayoutNode for ServoThreadSafeLayoutNode<'ln> { self.node.type_id() } + fn style_for_text_node(&self) -> Arc { + debug_assert!(self.is_text_node()); + let parent = self.node.parent_node().unwrap(); + let parent_data = parent.get_style_data().unwrap().borrow(); + let parent_style = &parent_data.current_styles().primary; + ComputedValues::style_for_child_text_node(parent_style) + } + fn debug_id(self) -> usize { self.node.debug_id() } diff --git a/components/script_layout_interface/wrapper_traits.rs b/components/script_layout_interface/wrapper_traits.rs index 5ea15926f97..a4463070689 100644 --- a/components/script_layout_interface/wrapper_traits.rs +++ b/components/script_layout_interface/wrapper_traits.rs @@ -165,6 +165,16 @@ pub trait ThreadSafeLayoutNode: Clone + Copy + NodeInfo + PartialEq + Sized { /// `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. + /// + /// Note that this does require accessing the parent, which this interface + /// technically forbids. But accessing the parent is only unsafe insofar as + /// it can be used to reach siblings and cousins. A simple immutable borrow + /// of the parent data is fine, since the bottom-up traversal will not process + /// the parent until all the children have been processed. + fn style_for_text_node(&self) -> Arc; + #[inline] fn is_element_or_elements_pseudo(&self) -> bool { match self.type_id_without_excluding_pseudo_elements() { @@ -187,7 +197,8 @@ pub trait ThreadSafeLayoutNode: Clone + Copy + NodeInfo + PartialEq + Sized { #[inline] fn get_before_pseudo(&self) -> Option { - if self.get_style_data() + if self.is_element() && + self.get_style_data() .unwrap() .borrow() .current_styles().pseudos @@ -200,7 +211,8 @@ pub trait ThreadSafeLayoutNode: Clone + Copy + NodeInfo + PartialEq + Sized { #[inline] fn get_after_pseudo(&self) -> Option { - if self.get_style_data() + if self.is_element() && + self.get_style_data() .unwrap() .borrow() .current_styles().pseudos @@ -248,8 +260,11 @@ pub trait ThreadSafeLayoutNode: Clone + Copy + NodeInfo + PartialEq + Sized { fn style(&self, context: &SharedStyleContext) -> Arc { match self.get_pseudo_element_type() { PseudoElementType::Normal => { - self.get_style_data().unwrap().borrow() - .current_styles().primary.clone() + match self.type_id().unwrap() { + LayoutNodeType::Text => self.style_for_text_node(), + LayoutNodeType::Element(_) => self.get_style_data().unwrap().borrow() + .current_styles().primary.clone(), + } }, other => { // Precompute non-eagerly-cascaded pseudo-element styles if not @@ -308,6 +323,10 @@ pub trait ThreadSafeLayoutNode: Clone + Copy + NodeInfo + PartialEq + Sized { /// element style is precomputed, not from general layout itself. #[inline] fn resolved_style(&self) -> Arc { + if self.is_text_node() { + return self.style_for_text_node(); + } + let data = self.get_style_data().unwrap().borrow(); match self.get_pseudo_element_type() { PseudoElementType::Normal @@ -319,6 +338,10 @@ pub trait ThreadSafeLayoutNode: Clone + Copy + NodeInfo + PartialEq + Sized { #[inline] fn selected_style(&self, _context: &SharedStyleContext) -> Arc { + if self.is_text_node() { + return self.style_for_text_node(); + } + let data = self.get_style_data().unwrap().borrow(); data.current_styles().pseudos .get(&PseudoElement::Selection) From 1cfd5e81725c0bf6f9ec59f3d80646303bad2555 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Tue, 25 Oct 2016 19:29:32 -0700 Subject: [PATCH 3/5] Hoist RestyleDamage onto TElement. Instead of maintaining a dummy restyle damage field for text nodes, we can just perform the necessary parent inheritance and is_changed adjustments on the fly in ThreadSafeLayoutNode, simplifying the requirements on the style system. MozReview-Commit-ID: DCqiCRLsLUF --- components/layout/traversal.rs | 8 +-- components/script/layout_wrapper.rs | 67 ++++++++----------- .../script_layout_interface/wrapper_traits.rs | 2 +- components/style/dom.rs | 25 +++---- components/style/gecko/wrapper.rs | 55 +++++++-------- components/style/matching.rs | 6 +- components/style/traversal.rs | 4 +- 7 files changed, 72 insertions(+), 95 deletions(-) diff --git a/components/layout/traversal.rs b/components/layout/traversal.rs index 5cd1a236f5c..b44ef4e17f8 100644 --- a/components/layout/traversal.rs +++ b/components/layout/traversal.rs @@ -16,7 +16,7 @@ use std::mem; use style::atomic_refcell::AtomicRefCell; use style::context::{LocalStyleContext, SharedStyleContext, StyleContext}; use style::data::NodeData; -use style::dom::{TNode, TRestyleDamage}; +use style::dom::TNode; use style::selector_impl::ServoSelectorImpl; use style::traversal::{DomTraversalContext, recalc_style_at, remove_from_bloom_filter}; use style::traversal::RestyleResult; @@ -111,7 +111,7 @@ fn construct_flows_at<'a, N: LayoutNode>(context: &'a LayoutContext<'a>, root: O // Always reconstruct if incremental layout is turned off. let nonincremental_layout = opts::get().nonincremental_layout; if nonincremental_layout || node.has_dirty_descendants() || - node.restyle_damage() != N::ConcreteRestyleDamage::empty() { + tnode.restyle_damage() != RestyleDamage::empty() { let mut flow_constructor = FlowConstructor::new(context); if nonincremental_layout || !flow_constructor.repair_if_possible(&tnode) { flow_constructor.process(&tnode); @@ -121,9 +121,7 @@ fn construct_flows_at<'a, N: LayoutNode>(context: &'a LayoutContext<'a>, root: O } } - // Reset the layout damage in this node. It's been propagated to the - // flow by the flow constructor. - tnode.set_restyle_damage(RestyleDamage::empty()); + tnode.clear_restyle_damage(); } unsafe { node.clear_dirty_bits(); } diff --git a/components/script/layout_wrapper.rs b/components/script/layout_wrapper.rs index 1f715399d4a..0cc2e7f3c78 100644 --- a/components/script/layout_wrapper.rs +++ b/components/script/layout_wrapper.rs @@ -236,41 +236,12 @@ impl<'ln> TNode for ServoLayoutNode<'ln> { debug_assert!(self.is_text_node()); let mut data = self.get_partial_layout_data().unwrap().borrow_mut(); data.style_data.style_text_node(style); - if self.has_changed() { - data.restyle_damage = RestyleDamage::rebuild_and_reflow(); - } else { - // FIXME(bholley): This is necessary to make it correct to use restyle - // damage in construct_flows_at to determine whether to reconstruct - // text nodes. Without it, we fail cascade-import-dynamic-002.htm. - // - // Long-term, We should teach layout how to correctly propagate - // style changes from elements to child text nodes so that we don't - // need to do this explicitly here. This will likely all be rolled - // into a patch where we stop styling text nodes from the style - // system and instead generate the styles on the fly during frame - // construction / repair. - let parent = self.parent_node().unwrap(); - let parent_data = parent.get_partial_layout_data().unwrap().borrow(); - data.restyle_damage = parent_data.restyle_damage; - } } fn borrow_data(&self) -> Option> { self.get_style_data().map(|d| d.borrow()) } - fn restyle_damage(self) -> RestyleDamage { - self.get_partial_layout_data().unwrap().borrow().restyle_damage - } - - fn set_restyle_damage(self, damage: RestyleDamage) { - let mut damage = damage; - if self.has_changed() { - damage = RestyleDamage::rebuild_and_reflow(); - } - self.get_partial_layout_data().unwrap().borrow_mut().restyle_damage = damage; - } - fn parent_node(&self) -> Option> { unsafe { self.node.parent_node_ref().map(|node| self.new_with_this_lifetime(&node)) @@ -300,14 +271,6 @@ impl<'ln> TNode for ServoLayoutNode<'ln> { self.node.next_sibling_ref().map(|node| self.new_with_this_lifetime(&node)) } } - - #[inline] - fn existing_style_for_restyle_damage<'a>(&'a self, - current_cv: Option<&'a Arc>, - _pseudo_element: Option<&PseudoElement>) - -> Option<&'a Arc> { - current_cv - } } pub struct ServoChildrenIterator<'a> { @@ -531,6 +494,19 @@ impl<'le> TElement for ServoLayoutElement<'le> { fn attr_equals(&self, namespace: &Namespace, attr: &Atom, val: &Atom) -> bool { self.get_attr(namespace, attr).map_or(false, |x| x == val) } + + fn set_restyle_damage(self, damage: RestyleDamage) { + let node = self.as_node(); + node.get_partial_layout_data().unwrap().borrow_mut().restyle_damage = damage; + } + + #[inline] + fn existing_style_for_restyle_damage<'a>(&'a self, + current_cv: Option<&'a Arc>, + _pseudo_element: Option<&PseudoElement>) + -> Option<&'a Arc> { + current_cv + } } impl<'le> PartialEq for ServoLayoutElement<'le> { @@ -885,11 +861,22 @@ impl<'ln> ThreadSafeLayoutNode for ServoThreadSafeLayoutNode<'ln> { } fn restyle_damage(self) -> RestyleDamage { - self.node.restyle_damage() + if self.node.has_changed() { + RestyleDamage::rebuild_and_reflow() + } else if self.is_text_node() { + let parent = self.node.parent_node().unwrap(); + let parent_data = parent.get_partial_layout_data().unwrap().borrow(); + parent_data.restyle_damage + } else { + self.node.get_partial_layout_data().unwrap().borrow().restyle_damage + } } - fn set_restyle_damage(self, damage: RestyleDamage) { - self.node.set_restyle_damage(damage) + fn clear_restyle_damage(self) { + if self.is_element() { + let mut data = self.node.get_partial_layout_data().unwrap().borrow_mut(); + data.restyle_damage = RestyleDamage::empty(); + } } fn can_be_fragmented(&self) -> bool { diff --git a/components/script_layout_interface/wrapper_traits.rs b/components/script_layout_interface/wrapper_traits.rs index a4463070689..40fdf33847d 100644 --- a/components/script_layout_interface/wrapper_traits.rs +++ b/components/script_layout_interface/wrapper_traits.rs @@ -353,7 +353,7 @@ pub trait ThreadSafeLayoutNode: Clone + Copy + NodeInfo + PartialEq + Sized { fn restyle_damage(self) -> RestyleDamage; - fn set_restyle_damage(self, damage: RestyleDamage); + fn clear_restyle_damage(self); /// Returns true if this node contributes content. This is used in the implementation of /// `empty_cells` per CSS 2.1 ยง 17.6.1.1. diff --git a/components/style/dom.rs b/components/style/dom.rs index 8f8a53f2c1f..c824457dbcb 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -215,12 +215,6 @@ pub trait TNode : Sized + Copy + Clone + NodeInfo { /// Immutable borrows the NodeData. fn borrow_data(&self) -> Option>; - /// Get the description of how to account for recent style changes. - fn restyle_damage(self) -> Self::ConcreteRestyleDamage; - - /// Set the restyle damage field. - fn set_restyle_damage(self, damage: Self::ConcreteRestyleDamage); - fn parent_node(&self) -> Option; fn first_child(&self) -> Option; @@ -230,14 +224,6 @@ pub trait TNode : Sized + Copy + Clone + NodeInfo { fn prev_sibling(&self) -> Option; fn next_sibling(&self) -> Option; - - /// XXX: It's a bit unfortunate we need to pass the current computed values - /// as an argument here, but otherwise Servo would crash due to double - /// borrows to return it. - fn existing_style_for_restyle_damage<'a>(&'a self, - current_computed_values: Option<&'a Arc>, - pseudo: Option<&PseudoElement>) - -> Option<&'a ::PreExistingComputedValues>; } pub trait TDocument : Sized + Copy + Clone { @@ -273,6 +259,17 @@ pub trait TElement : PartialEq + Debug + Sized + Copy + Clone + ElementExt + Pre fn has_attr(&self, namespace: &Namespace, attr: &Atom) -> bool; fn attr_equals(&self, namespace: &Namespace, attr: &Atom, value: &Atom) -> bool; + /// Set the restyle damage field. + fn set_restyle_damage(self, damage: ::ConcreteRestyleDamage); + + /// XXX: It's a bit unfortunate we need to pass the current computed values + /// as an argument here, but otherwise Servo would crash due to double + /// borrows to return it. + fn existing_style_for_restyle_damage<'a>(&'a self, + current_computed_values: Option<&'a Arc>, + pseudo: Option<&PseudoElement>) + -> Option<&'a <::ConcreteRestyleDamage as TRestyleDamage>::PreExistingComputedValues>; + /// Properly marks nodes as dirty in response to restyle hints. fn note_restyle_hint>(&self, hint: RestyleHint) { // Bail early if there's no restyling to do. diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index 187aed0acd8..a1128768d1b 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -273,19 +273,6 @@ impl<'ln> TNode for GeckoNode<'ln> { self.get_node_data().map(|x| x.borrow()) } - fn restyle_damage(self) -> Self::ConcreteRestyleDamage { - // Not called from style, only for layout. - unimplemented!(); - } - - fn set_restyle_damage(self, damage: Self::ConcreteRestyleDamage) { - // FIXME(bholley): Gecko currently relies on the dirty bit being set to - // drive the post-traversal. This will go away soon. - unsafe { self.set_flags(NODE_IS_DIRTY_FOR_SERVO as u32) } - - unsafe { Gecko_StoreStyleDifference(self.0, damage.0) } - } - fn parent_node(&self) -> Option> { unsafe { self.0.mParent.as_ref().map(GeckoNode) } } @@ -306,23 +293,6 @@ impl<'ln> TNode for GeckoNode<'ln> { unsafe { self.0.mNextSibling.as_ref().map(GeckoNode::from_content) } } - fn existing_style_for_restyle_damage<'a>(&'a self, - current_cv: Option<&'a Arc>, - pseudo: Option<&PseudoElement>) - -> Option<&'a nsStyleContext> { - if current_cv.is_none() { - // Don't bother in doing an ffi call to get null back. - return None; - } - - unsafe { - let atom_ptr = pseudo.map(|p| p.as_atom().as_ptr()) - .unwrap_or(ptr::null_mut()); - let context_ptr = Gecko_GetStyleContext(self.0, atom_ptr); - context_ptr.as_ref() - } - } - fn needs_dirty_on_viewport_size_changed(&self) -> bool { // Gecko's node doesn't have the DIRTY_ON_VIEWPORT_SIZE_CHANGE flag, // so we force them to be dirtied on viewport size change, regardless if @@ -464,6 +434,31 @@ impl<'le> TElement for GeckoElement<'le> { /* ignoreCase = */ false) } } + + fn set_restyle_damage(self, damage: GeckoRestyleDamage) { + // FIXME(bholley): Gecko currently relies on the dirty bit being set to + // drive the post-traversal. This will go away soon. + unsafe { self.as_node().set_flags(NODE_IS_DIRTY_FOR_SERVO as u32) } + + unsafe { Gecko_StoreStyleDifference(self.as_node().0, damage.0) } + } + + fn existing_style_for_restyle_damage<'a>(&'a self, + current_cv: Option<&'a Arc>, + pseudo: Option<&PseudoElement>) + -> Option<&'a nsStyleContext> { + if current_cv.is_none() { + // Don't bother in doing an ffi call to get null back. + return None; + } + + unsafe { + let atom_ptr = pseudo.map(|p| p.as_atom().as_ptr()) + .unwrap_or(ptr::null_mut()); + let context_ptr = Gecko_GetStyleContext(self.as_node().0, atom_ptr); + context_ptr.as_ref() + } + } } impl<'le> PartialEq for GeckoElement<'le> { diff --git a/components/style/matching.rs b/components/style/matching.rs index a7c7769788d..82add15f335 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -739,7 +739,7 @@ pub trait ElementMatchMethods : TElement { // can decide more easily if it knows that it's a child of // replaced content, or similar stuff! let damage = - match node.existing_style_for_restyle_damage(data.previous_styles().map(|x| &x.primary), None) { + match self.existing_style_for_restyle_damage(data.previous_styles().map(|x| &x.primary), None) { Some(ref source) => { <::ConcreteNode as TNode> ::ConcreteRestyleDamage::compute(source, &shared_style) @@ -847,7 +847,7 @@ pub trait MatchMethods : TNode { pseudo: Option<&PseudoElement>) -> Self::ConcreteRestyleDamage { - match self.existing_style_for_restyle_damage(old_style, pseudo) { + match self.as_element().unwrap().existing_style_for_restyle_damage(old_style, pseudo) { Some(ref source) => { Self::ConcreteRestyleDamage::compute(source, new_style) @@ -955,7 +955,7 @@ pub trait MatchMethods : TNode { data.finish_styling(new_styles); // Drop the mutable borrow early, since Servo's set_restyle_damage also borrows. mem::drop(data); - self.set_restyle_damage(damage); + self.as_element().unwrap().set_restyle_damage(damage); restyle_result } diff --git a/components/style/traversal.rs b/components/style/traversal.rs index 1efe9721b6c..085ce8888c6 100644 --- a/components/style/traversal.rs +++ b/components/style/traversal.rs @@ -7,7 +7,7 @@ use atomic_refcell::AtomicRefCell; use context::{LocalStyleContext, SharedStyleContext, StyleContext}; use data::NodeData; -use dom::{OpaqueNode, StylingMode, TNode, UnsafeNode}; +use dom::{OpaqueNode, StylingMode, TElement, TNode, UnsafeNode}; use matching::{ApplicableDeclarations, ElementMatchMethods, MatchMethods, StyleSharingResult}; use selectors::bloom::BloomFilter; use selectors::matching::StyleRelations; @@ -370,7 +370,7 @@ pub fn recalc_style_at<'a, N, C, D>(context: &'a C, STYLE_SHARING_CACHE_HITS.fetch_add(1, Ordering::Relaxed); } style_sharing_candidate_cache.touch(index); - node.set_restyle_damage(damage); + node.as_element().unwrap().set_restyle_damage(damage); } } } From 1090abae877bb6624e49b34540682a389e7daaac Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Tue, 25 Oct 2016 19:50:31 -0700 Subject: [PATCH 4/5] Don't traverse text nodes during styling. MozReview-Commit-ID: 6CtQMxbcLnF --- components/layout/traversal.rs | 35 +++++++++++++++++-- components/script/layout_wrapper.rs | 14 ++++++-- .../script_layout_interface/wrapper_traits.rs | 2 ++ components/style/dom.rs | 3 +- components/style/gecko/traversal.rs | 11 ++++-- .../style/properties/properties.mako.rs | 12 ------- components/style/traversal.rs | 26 +++++++------- 7 files changed, 72 insertions(+), 31 deletions(-) diff --git a/components/layout/traversal.rs b/components/layout/traversal.rs index b44ef4e17f8..36130aba4c4 100644 --- a/components/layout/traversal.rs +++ b/components/layout/traversal.rs @@ -18,8 +18,10 @@ use style::context::{LocalStyleContext, SharedStyleContext, StyleContext}; use style::data::NodeData; use style::dom::TNode; use style::selector_impl::ServoSelectorImpl; -use style::traversal::{DomTraversalContext, recalc_style_at, remove_from_bloom_filter}; +use style::traversal::{DomTraversalContext, put_thread_local_bloom_filter}; +use style::traversal::{recalc_style_at, remove_from_bloom_filter}; use style::traversal::RestyleResult; +use style::traversal::take_thread_local_bloom_filter; use util::opts; use wrapper::{LayoutNodeLayoutData, ThreadSafeLayoutNodeHelpers}; @@ -77,7 +79,36 @@ impl<'lc, N> DomTraversalContext for RecalcStyleAndConstructFlows<'lc> // done by the HTML parser. node.initialize_data(); - recalc_style_at::<_, _, Self>(&self.context, self.root, node) + if node.is_text_node() { + // FIXME(bholley): Stop doing this silly work to maintain broken bloom filter + // invariants. + // + // Longer version: The bloom filter is entirely busted for parallel traversal. Because + // parallel traversal is breadth-first, each sibling rejects the bloom filter set up + // by the previous sibling (which is valid for children, not siblings) and recreates + // it. Similarly, the fixup performed in the bottom-up traversal is useless, because + // threads perform flow construction up the parent chain until they find a parent with + // other unprocessed children, at which point they bail to the work queue and find a + // different node. + // + // Nevertheless, the remove_from_bloom_filter call at the end of flow construction + // asserts that the bloom filter is valid for the current node. This breaks when we + // stop calling recalc_style_at for text nodes, because the recursive chain of + // construct_flows_at calls is no longer necessarily rooted in a call that sets up the + // thread-local bloom filter for the leaf node. + // + // The bloom filter stuff is all going to be rewritten, so we just hackily duplicate + // the bloom filter manipulation from recalc_style_at to maintain invariants. + let parent = node.parent_node(); + debug_assert!(parent.unwrap().is_element()); + let bf = take_thread_local_bloom_filter(parent, self.root, self.context.shared_context()); + put_thread_local_bloom_filter(bf, &node.to_unsafe(), self.context.shared_context()); + + RestyleResult::Stop + } else { + let el = node.as_element().unwrap(); + recalc_style_at::<_, _, Self>(&self.context, self.root, el) + } } fn process_postorder(&self, node: N) { diff --git a/components/script/layout_wrapper.rs b/components/script/layout_wrapper.rs index 0cc2e7f3c78..b5e1c5a4f60 100644 --- a/components/script/layout_wrapper.rs +++ b/components/script/layout_wrapper.rs @@ -763,6 +763,9 @@ impl<'ln> NodeInfo for ServoThreadSafeLayoutNode<'ln> { } fn is_text_node(&self) -> bool { + // It's unlikely that text nodes will ever be used to implement a + // pseudo-element, but the type system doesn't really enforce that, + // so we check to be safe. self.pseudo == PseudoElementType::Normal && self.node.is_text_node() } @@ -802,11 +805,18 @@ impl<'ln> ThreadSafeLayoutNode for ServoThreadSafeLayoutNode<'ln> { } fn style_for_text_node(&self) -> Arc { + // Text nodes get a copy of the parent style. Inheriting all non- + // inherited properties into the text node is odd from a CSS + // perspective, but makes fragment construction easier (by making + // properties like vertical-align on fragments have values that + // match the parent element). This is an implementation detail of + // Servo layout that is not central to how fragment construction + // works, but would be difficult to change. (Text node style is + // also not visible to script.) debug_assert!(self.is_text_node()); let parent = self.node.parent_node().unwrap(); let parent_data = parent.get_style_data().unwrap().borrow(); - let parent_style = &parent_data.current_styles().primary; - ComputedValues::style_for_child_text_node(parent_style) + parent_data.current_styles().primary.clone() } fn debug_id(self) -> usize { diff --git a/components/script_layout_interface/wrapper_traits.rs b/components/script_layout_interface/wrapper_traits.rs index 40fdf33847d..200aeaa03af 100644 --- a/components/script_layout_interface/wrapper_traits.rs +++ b/components/script_layout_interface/wrapper_traits.rs @@ -323,6 +323,7 @@ pub trait ThreadSafeLayoutNode: Clone + Copy + NodeInfo + PartialEq + Sized { /// element style is precomputed, not from general layout itself. #[inline] fn resolved_style(&self) -> Arc { + // FIXME(bholley): This should move to Element and lose the text node check. if self.is_text_node() { return self.style_for_text_node(); } @@ -338,6 +339,7 @@ pub trait ThreadSafeLayoutNode: Clone + Copy + NodeInfo + PartialEq + Sized { #[inline] fn selected_style(&self, _context: &SharedStyleContext) -> Arc { + // FIXME(bholley): This should move to Element and lose the text node check. if self.is_text_node() { return self.style_for_text_node(); } diff --git a/components/style/dom.rs b/components/style/dom.rs index c824457dbcb..aa5d0829001 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -268,7 +268,8 @@ pub trait TElement : PartialEq + Debug + Sized + Copy + Clone + ElementExt + Pre fn existing_style_for_restyle_damage<'a>(&'a self, current_computed_values: Option<&'a Arc>, pseudo: Option<&PseudoElement>) - -> Option<&'a <::ConcreteRestyleDamage as TRestyleDamage>::PreExistingComputedValues>; + -> Option<&'a <::ConcreteRestyleDamage as TRestyleDamage> + ::PreExistingComputedValues>; /// Properly marks nodes as dirty in response to restyle hints. fn note_restyle_hint>(&self, hint: RestyleHint) { diff --git a/components/style/gecko/traversal.rs b/components/style/gecko/traversal.rs index 61a9bd32de8..c9ddca69d73 100644 --- a/components/style/gecko/traversal.rs +++ b/components/style/gecko/traversal.rs @@ -5,7 +5,7 @@ use atomic_refcell::AtomicRefCell; use context::{LocalStyleContext, SharedStyleContext, StyleContext}; use data::NodeData; -use dom::OpaqueNode; +use dom::{NodeInfo, OpaqueNode, TNode}; use gecko::context::StandaloneStyleContext; use gecko::wrapper::GeckoNode; use std::mem; @@ -31,7 +31,14 @@ impl<'lc, 'ln> DomTraversalContext> for RecalcStyleOnly<'lc> { } fn process_preorder(&self, node: GeckoNode<'ln>) -> RestyleResult { - recalc_style_at::<_, _, Self>(&self.context, self.root, node) + if node.is_text_node() { + // Text nodes don't have children, so save the traversal algorithm + // the trouble of iterating the children. + RestyleResult::Stop + } else { + let el = node.as_element().unwrap(); + recalc_style_at::<_, _, Self>(&self.context, self.root, el) + } } fn process_postorder(&self, _: GeckoNode<'ln>) { diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 63e7b2fe5a4..40871299659 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -1084,18 +1084,6 @@ impl ComputedValues { } } - pub fn style_for_child_text_node(parent: &Arc) -> Arc { - // Text nodes get a copy of the parent style. Inheriting all non- - // inherited properties into the text node is odd from a CSS - // perspective, but makes fragment construction easier (by making - // properties like vertical-align on fragments have values that - // match the parent element). This is an implementation detail of - // Servo layout that is not central to how fragment construction - // works, but would be difficult to change. (Text node style is - // also not visible to script.) - parent.clone() - } - pub fn initial_values() -> &'static Self { &*INITIAL_SERVO_VALUES } #[inline] diff --git a/components/style/traversal.rs b/components/style/traversal.rs index 085ce8888c6..03405e59360 100644 --- a/components/style/traversal.rs +++ b/components/style/traversal.rs @@ -7,7 +7,7 @@ use atomic_refcell::AtomicRefCell; use context::{LocalStyleContext, SharedStyleContext, StyleContext}; use data::NodeData; -use dom::{OpaqueNode, StylingMode, TElement, TNode, UnsafeNode}; +use dom::{NodeInfo, OpaqueNode, StylingMode, TElement, TNode, UnsafeNode}; use matching::{ApplicableDeclarations, ElementMatchMethods, MatchMethods, StyleSharingResult}; use selectors::bloom::BloomFilter; use selectors::matching::StyleRelations; @@ -62,11 +62,11 @@ thread_local!( /// /// If one does not exist, a new one will be made for you. If it is out of date, /// it will be cleared and reused. -fn take_thread_local_bloom_filter(parent_node: Option, - root: OpaqueNode, - context: &SharedStyleContext) - -> Box - where N: TNode { +pub fn take_thread_local_bloom_filter(parent_node: Option, + root: OpaqueNode, + context: &SharedStyleContext) + -> Box + where N: TNode { STYLE_BLOOM.with(|style_bloom| { match (parent_node, style_bloom.borrow_mut().take()) { // Root node. Needs new bloom filter. @@ -98,8 +98,8 @@ fn take_thread_local_bloom_filter(parent_node: Option, }) } -fn put_thread_local_bloom_filter(bf: Box, unsafe_node: &UnsafeNode, - context: &SharedStyleContext) { +pub fn put_thread_local_bloom_filter(bf: Box, unsafe_node: &UnsafeNode, + context: &SharedStyleContext) { STYLE_BLOOM.with(move |style_bloom| { assert!(style_bloom.borrow().is_none(), "Putting into a never-taken thread-local bloom filter"); @@ -284,13 +284,15 @@ fn ensure_node_styled_internal<'a, N, C>(node: N, /// Calculates the style for a single node. #[inline] #[allow(unsafe_code)] -pub fn recalc_style_at<'a, N, C, D>(context: &'a C, +pub fn recalc_style_at<'a, E, C, D>(context: &'a C, root: OpaqueNode, - node: N) -> RestyleResult - where N: TNode, + element: E) -> RestyleResult + where E: TElement, C: StyleContext<'a>, - D: DomTraversalContext + D: DomTraversalContext { + let node = element.as_node(); + // Get the parent node. let parent_opt = match node.parent_node() { Some(parent) if parent.is_element() => Some(parent), From 2fce2fbb0c5a6d85af76d6d74c51bf9c85b901f2 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Wed, 26 Oct 2016 16:54:36 -0700 Subject: [PATCH 5/5] Move all *MatchMethods to TElement. MozReview-Commit-ID: 3V6JIlOVVhw --- components/layout/query.rs | 4 +- components/layout/traversal.rs | 3 +- components/script/layout_wrapper.rs | 2 +- components/style/dom.rs | 7 +- components/style/gecko/wrapper.rs | 2 +- components/style/matching.rs | 111 ++++++++--------------- components/style/traversal.rs | 132 ++++++++++++---------------- 7 files changed, 98 insertions(+), 163 deletions(-) diff --git a/components/layout/query.rs b/components/layout/query.rs index a71bc246113..b491f08632d 100644 --- a/components/layout/query.rs +++ b/components/layout/query.rs @@ -631,14 +631,14 @@ pub fn process_resolved_style_request<'a, N, C>(requested_node: N, where N: LayoutNode, C: StyleContext<'a> { - use style::traversal::ensure_node_styled; + use style::traversal::ensure_element_styled; // This node might have display: none, or it's style might be not up to // date, so we might need to do style recalc. // // FIXME(emilio): Is a bit shame we have to do this instead of in style. ensure_node_data_initialized(&requested_node); - ensure_node_styled(requested_node, style_context); + ensure_element_styled(requested_node.as_element().unwrap(), style_context); let layout_node = requested_node.to_threadsafe(); let layout_node = match *pseudo { diff --git a/components/layout/traversal.rs b/components/layout/traversal.rs index 36130aba4c4..2ab1e21cebe 100644 --- a/components/layout/traversal.rs +++ b/components/layout/traversal.rs @@ -99,8 +99,7 @@ impl<'lc, N> DomTraversalContext for RecalcStyleAndConstructFlows<'lc> // // The bloom filter stuff is all going to be rewritten, so we just hackily duplicate // the bloom filter manipulation from recalc_style_at to maintain invariants. - let parent = node.parent_node(); - debug_assert!(parent.unwrap().is_element()); + let parent = node.parent_node().unwrap().as_element(); let bf = take_thread_local_bloom_filter(parent, self.root, self.context.shared_context()); put_thread_local_bloom_filter(bf, &node.to_unsafe(), self.context.shared_context()); diff --git a/components/script/layout_wrapper.rs b/components/script/layout_wrapper.rs index b5e1c5a4f60..75bdd55aedc 100644 --- a/components/script/layout_wrapper.rs +++ b/components/script/layout_wrapper.rs @@ -133,7 +133,6 @@ impl<'ln> NodeInfo for ServoLayoutNode<'ln> { 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 { @@ -470,6 +469,7 @@ impl<'le> PresentationalHintsSynthetizer for ServoLayoutElement<'le> { impl<'le> TElement for ServoLayoutElement<'le> { type ConcreteNode = ServoLayoutNode<'le>; type ConcreteDocument = ServoLayoutDocument<'le>; + type ConcreteRestyleDamage = RestyleDamage; fn as_node(&self) -> ServoLayoutNode<'le> { ServoLayoutNode::from_layout_js(self.element.upcast()) diff --git a/components/style/dom.rs b/components/style/dom.rs index aa5d0829001..7ead2605416 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -106,7 +106,6 @@ impl Iterator for LayoutIterator where T: Iterator, I: NodeInfo pub trait TNode : Sized + Copy + Clone + NodeInfo { type ConcreteElement: TElement; type ConcreteDocument: TDocument; - type ConcreteRestyleDamage: TRestyleDamage; type ConcreteChildrenIterator: Iterator; fn to_unsafe(&self) -> UnsafeNode; @@ -249,6 +248,7 @@ pub trait PresentationalHintsSynthetizer { pub trait TElement : PartialEq + Debug + Sized + Copy + Clone + ElementExt + PresentationalHintsSynthetizer { type ConcreteNode: TNode; type ConcreteDocument: TDocument; + type ConcreteRestyleDamage: TRestyleDamage; fn as_node(&self) -> Self::ConcreteNode; @@ -260,7 +260,7 @@ pub trait TElement : PartialEq + Debug + Sized + Copy + Clone + ElementExt + Pre fn attr_equals(&self, namespace: &Namespace, attr: &Atom, value: &Atom) -> bool; /// Set the restyle damage field. - fn set_restyle_damage(self, damage: ::ConcreteRestyleDamage); + fn set_restyle_damage(self, damage: Self::ConcreteRestyleDamage); /// XXX: It's a bit unfortunate we need to pass the current computed values /// as an argument here, but otherwise Servo would crash due to double @@ -268,8 +268,7 @@ pub trait TElement : PartialEq + Debug + Sized + Copy + Clone + ElementExt + Pre fn existing_style_for_restyle_damage<'a>(&'a self, current_computed_values: Option<&'a Arc>, pseudo: Option<&PseudoElement>) - -> Option<&'a <::ConcreteRestyleDamage as TRestyleDamage> - ::PreExistingComputedValues>; + -> Option<&'a ::PreExistingComputedValues>; /// Properly marks nodes as dirty in response to restyle hints. fn note_restyle_hint>(&self, hint: RestyleHint) { diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index a1128768d1b..e1ca9ab6723 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -160,7 +160,6 @@ impl<'ln> NodeInfo for GeckoNode<'ln> { 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 { @@ -399,6 +398,7 @@ lazy_static! { impl<'le> TElement for GeckoElement<'le> { type ConcreteNode = GeckoNode<'le>; type ConcreteDocument = GeckoDocument<'le>; + type ConcreteRestyleDamage = GeckoRestyleDamage; fn as_node(&self) -> Self::ConcreteNode { unsafe { GeckoNode(&*(self.0 as *const _ as *const RawGeckoNode)) } diff --git a/components/style/matching.rs b/components/style/matching.rs index 82add15f335..794e18e2c7b 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -12,12 +12,12 @@ use cache::{LRUCache, SimpleHashCache}; use cascade_info::CascadeInfo; use context::{SharedStyleContext, StyleContext}; use data::{NodeStyles, PseudoStyles}; -use dom::{NodeInfo, TElement, TNode, TRestyleDamage, UnsafeNode}; +use dom::{TElement, TNode, TRestyleDamage, UnsafeNode}; use properties::{CascadeFlags, ComputedValues, SHAREABLE, 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::MatchAttr; use selectors::bloom::BloomFilter; use selectors::matching::{AFFECTED_BY_PSEUDO_ELEMENTS, MatchingReason, StyleRelations}; use sink::ForgetfulSink; @@ -497,7 +497,7 @@ struct CascadeBooleans { animate: bool, } -trait PrivateMatchMethods: TNode { +trait PrivateMatchMethods: TElement { /// Actually cascades style for a node or a pseudo-element of a node. /// /// Note that animations only apply to nodes or ::before or ::after @@ -521,7 +521,7 @@ trait PrivateMatchMethods: TNode { // and the cache would not be effective anyway. // This also works around the test failures at // https://github.com/servo/servo/pull/13459#issuecomment-250717584 - let has_style_attribute = self.as_element().map_or(false, |e| e.style_attribute().is_some()); + let has_style_attribute = self.style_attribute().is_some(); cacheable = cacheable && !has_style_attribute; let mut cascade_info = CascadeInfo::new(); @@ -556,7 +556,7 @@ trait PrivateMatchMethods: TNode { cascade_flags) } }; - cascade_info.finish(self); + cascade_info.finish(&self.as_node()); cacheable = cacheable && is_cacheable; @@ -564,7 +564,7 @@ trait PrivateMatchMethods: TNode { if booleans.animate { let new_animations_sender = &context.local_context().new_animations_sender; - let this_opaque = self.opaque(); + let this_opaque = self.as_node().opaque(); // Trigger any present animations if necessary. let mut animations_started = animation::maybe_start_animations( &shared_context, @@ -579,7 +579,7 @@ trait PrivateMatchMethods: TNode { animation::start_transitions_if_applicable( new_animations_sender, this_opaque, - self.to_unsafe(), + self.as_node().to_unsafe(), &**style, &mut this_style, &shared_context.timer); @@ -601,7 +601,7 @@ trait PrivateMatchMethods: TNode { context: &SharedStyleContext, style: &mut Arc) -> bool { // Finish any expired transitions. - let this_opaque = self.opaque(); + let this_opaque = self.as_node().opaque(); let had_animations_to_expire = animation::complete_expired_transitions(this_opaque, style, context); @@ -636,18 +636,11 @@ trait PrivateMatchMethods: TNode { had_animations_to_expire || had_running_animations } -} -impl PrivateMatchMethods for N {} - -trait PrivateElementMatchMethods: TElement { fn share_style_with_candidate_if_possible(&self, - parent_node: Self::ConcreteNode, shared_context: &SharedStyleContext, candidate: &mut StyleSharingCandidate) -> Result, CacheMiss> { - debug_assert!(parent_node.is_element()); - let candidate_element = unsafe { Self::ConcreteNode::from_unsafe(&candidate.node).as_element().unwrap() }; @@ -657,9 +650,9 @@ trait PrivateElementMatchMethods: TElement { } } -impl PrivateElementMatchMethods for E {} +impl PrivateMatchMethods for E {} -pub trait ElementMatchMethods : TElement { +pub trait MatchMethods : TElement { fn match_element(&self, stylist: &Stylist, parent_bf: Option<&BloomFilter>, @@ -703,9 +696,8 @@ pub trait ElementMatchMethods : TElement { unsafe fn share_style_if_possible(&self, style_sharing_candidate_cache: &mut StyleSharingCandidateCache, - shared_context: &SharedStyleContext, - parent: Option) - -> StyleSharingResult<::ConcreteRestyleDamage> { + shared_context: &SharedStyleContext) + -> StyleSharingResult { if opts::get().disable_share_style_cache { return StyleSharingResult::CannotShare } @@ -718,16 +710,9 @@ pub trait ElementMatchMethods : TElement { return StyleSharingResult::CannotShare } - let parent = match parent { - Some(parent) if parent.is_element() => parent, - _ => return StyleSharingResult::CannotShare, - }; - let mut should_clear_cache = false; for (i, &mut (ref mut candidate, ())) in style_sharing_candidate_cache.iter_mut().enumerate() { - let sharing_result = self.share_style_with_candidate_if_possible(parent, - shared_context, - candidate); + let sharing_result = self.share_style_with_candidate_if_possible(shared_context, candidate); match sharing_result { Ok(shared_style) => { // Yay, cache hit. Share the style. @@ -741,12 +726,10 @@ pub trait ElementMatchMethods : TElement { let damage = match self.existing_style_for_restyle_damage(data.previous_styles().map(|x| &x.primary), None) { Some(ref source) => { - <::ConcreteNode as TNode> - ::ConcreteRestyleDamage::compute(source, &shared_style) + Self::ConcreteRestyleDamage::compute(source, &shared_style) } None => { - <::ConcreteNode as TNode> - ::ConcreteRestyleDamage::rebuild_and_reflow() + Self::ConcreteRestyleDamage::rebuild_and_reflow() } }; @@ -788,11 +771,7 @@ pub trait ElementMatchMethods : TElement { StyleSharingResult::CannotShare } -} -impl ElementMatchMethods for E {} - -pub trait MatchMethods : TNode { // The below two functions are copy+paste because I can't figure out how to // write a function which takes a generic function. I don't think it can // be done. @@ -816,29 +795,23 @@ pub trait MatchMethods : TNode { /// Therefore, each node must have its matching selectors inserted _after_ /// its own selector matching and _before_ its children start. fn insert_into_bloom_filter(&self, bf: &mut BloomFilter) { - // Only elements are interesting. - if let Some(element) = self.as_element() { - bf.insert(&*element.get_local_name()); - bf.insert(&*element.get_namespace()); - element.get_id().map(|id| bf.insert(&id)); + bf.insert(&*self.get_local_name()); + bf.insert(&*self.get_namespace()); + self.get_id().map(|id| bf.insert(&id)); - // TODO: case-sensitivity depends on the document type and quirks mode - element.each_class(|class| bf.insert(class)); - } + // TODO: case-sensitivity depends on the document type and quirks mode + self.each_class(|class| bf.insert(class)); } /// After all the children are done css selector matching, this must be /// called to reset the bloom filter after an `insert`. fn remove_from_bloom_filter(&self, bf: &mut BloomFilter) { - // Only elements are interesting. - if let Some(element) = self.as_element() { - bf.remove(&*element.get_local_name()); - bf.remove(&*element.get_namespace()); - element.get_id().map(|id| bf.remove(&id)); + bf.remove(&*self.get_local_name()); + bf.remove(&*self.get_namespace()); + self.get_id().map(|id| bf.remove(&id)); - // TODO: case-sensitivity depends on the document type and quirks mode - element.each_class(|class| bf.remove(class)); - } + // TODO: case-sensitivity depends on the document type and quirks mode + self.each_class(|class| bf.remove(class)); } fn compute_restyle_damage(&self, @@ -847,7 +820,7 @@ pub trait MatchMethods : TNode { pseudo: Option<&PseudoElement>) -> Self::ConcreteRestyleDamage { - match self.as_element().unwrap().existing_style_for_restyle_damage(old_style, pseudo) { + match self.existing_style_for_restyle_damage(old_style, pseudo) { Some(ref source) => { Self::ConcreteRestyleDamage::compute(source, new_style) @@ -890,23 +863,12 @@ pub trait MatchMethods : TNode { where Ctx: StyleContext<'a> { // Get our parent's style. - let parent_data = parent.as_ref().map(|x| x.borrow_data().unwrap()); + let parent_as_node = parent.map(|x| x.as_node()); + let parent_data = parent_as_node.as_ref().map(|x| x.borrow_data().unwrap()); let parent_style = parent_data.as_ref().map(|x| &x.current_styles().primary); - // In the case we're styling a text node, we don't need to compute the - // restyle damage, since it's a subset of the restyle damage of the - // parent. - // - // In Gecko, we're done, we don't need anything else from text nodes. - // - // In Servo, this is also true, since text nodes generate UnscannedText - // fragments, which aren't repairable by incremental layout. - if self.is_text_node() { - self.style_text_node(ComputedValues::style_for_child_text_node(parent_style.clone().unwrap())); - return RestyleResult::Continue; - } - - let mut data = self.begin_styling(); + let node = self.as_node(); + let mut data = node.begin_styling(); let mut new_styles; let mut applicable_declarations_cache = @@ -944,8 +906,8 @@ pub trait MatchMethods : TNode { context, applicable_declarations, &mut applicable_declarations_cache); - self.set_can_be_fragmented(parent.map_or(false, |p| { - p.can_be_fragmented() || + self.as_node().set_can_be_fragmented(parent.map_or(false, |p| { + p.as_node().can_be_fragmented() || parent_style.unwrap().is_multicol() })); @@ -955,7 +917,7 @@ pub trait MatchMethods : TNode { data.finish_styling(new_styles); // Drop the mutable borrow early, since Servo's set_restyle_damage also borrows. mem::drop(data); - self.as_element().unwrap().set_restyle_damage(damage); + self.set_restyle_damage(damage); restyle_result } @@ -1006,7 +968,7 @@ pub trait MatchMethods : TNode { let no_damage = Self::ConcreteRestyleDamage::empty(); debug_assert!(new_pseudos.is_empty()); - ::Impl::each_eagerly_cascaded_pseudo_element(|pseudo| { + ::Impl::each_eagerly_cascaded_pseudo_element(|pseudo| { let applicable_declarations_for_this_pseudo = applicable_declarations.per_pseudo.get(&pseudo).unwrap(); @@ -1018,8 +980,7 @@ pub trait MatchMethods : TNode { if has_declarations { // We have declarations, so we need to cascade. Compute parameters. - let animate = ::Impl - ::pseudo_is_before_or_after(&pseudo); + let animate = ::Impl::pseudo_is_before_or_after(&pseudo); let cacheable = if animate && old_pseudo_style.is_some() { // Update animations before the cascade. This may modify // the value of old_pseudo_style. @@ -1064,4 +1025,4 @@ pub trait MatchMethods : TNode { } } -impl MatchMethods for N {} +impl MatchMethods for E {} diff --git a/components/style/traversal.rs b/components/style/traversal.rs index 03405e59360..48b598ba93a 100644 --- a/components/style/traversal.rs +++ b/components/style/traversal.rs @@ -8,7 +8,7 @@ use atomic_refcell::AtomicRefCell; use context::{LocalStyleContext, SharedStyleContext, StyleContext}; use data::NodeData; use dom::{NodeInfo, OpaqueNode, StylingMode, TElement, TNode, UnsafeNode}; -use matching::{ApplicableDeclarations, ElementMatchMethods, MatchMethods, StyleSharingResult}; +use matching::{ApplicableDeclarations, MatchMethods, StyleSharingResult}; use selectors::bloom::BloomFilter; use selectors::matching::StyleRelations; use std::cell::RefCell; @@ -62,13 +62,13 @@ thread_local!( /// /// If one does not exist, a new one will be made for you. If it is out of date, /// it will be cleared and reused. -pub fn take_thread_local_bloom_filter(parent_node: Option, +pub fn take_thread_local_bloom_filter(parent_element: Option, root: OpaqueNode, context: &SharedStyleContext) -> Box - where N: TNode { + where E: TElement { STYLE_BLOOM.with(|style_bloom| { - match (parent_node, style_bloom.borrow_mut().take()) { + match (parent_element, style_bloom.borrow_mut().take()) { // Root node. Needs new bloom filter. (None, _ ) => { debug!("[{}] No parent, but new bloom filter!", tid()); @@ -82,7 +82,7 @@ pub fn take_thread_local_bloom_filter(parent_node: Option, } // Found cached bloom filter. (Some(parent), Some((mut bloom_filter, old_node, old_generation))) => { - if old_node == parent.to_unsafe() && + if old_node == parent.as_node().to_unsafe() && old_generation == context.generation { // Hey, the cached parent is our parent! We can reuse the bloom filter. debug!("[{}] Parent matches (={}). Reusing bloom filter.", tid(), old_node.0); @@ -108,17 +108,17 @@ pub fn put_thread_local_bloom_filter(bf: Box, unsafe_node: &UnsafeN } /// "Ancestors" in this context is inclusive of ourselves. -fn insert_ancestors_into_bloom_filter(bf: &mut Box, - mut n: N, +fn insert_ancestors_into_bloom_filter(bf: &mut Box, + mut el: E, root: OpaqueNode) - where N: TNode { + where E: TElement { debug!("[{}] Inserting ancestors.", tid()); let mut ancestors = 0; loop { ancestors += 1; - n.insert_into_bloom_filter(&mut **bf); - n = match n.layout_parent_node(root) { + el.insert_into_bloom_filter(&mut **bf); + el = match el.as_node().layout_parent_node(root).and_then(|x| x.as_element()) { None => break, Some(p) => p, }; @@ -149,7 +149,7 @@ pub fn remove_from_bloom_filter<'a, N, C>(context: &C, root: OpaqueNode, node: N } Some(parent) => { // Otherwise, put it back, but remove this node. - node.remove_from_bloom_filter(&mut *bf); + node.as_element().map(|x| x.remove_from_bloom_filter(&mut *bf)); let unsafe_parent = parent.to_unsafe(); put_thread_local_bloom_filter(bf, &unsafe_parent, &context.shared_context()); }, @@ -213,20 +213,20 @@ pub fn relations_are_shareable(relations: &StyleRelations) -> bool { AFFECTED_BY_PRESENTATIONAL_HINTS) } -pub fn ensure_node_styled<'a, N, C>(node: N, - context: &'a C) - where N: TNode, +pub fn ensure_element_styled<'a, E, C>(element: E, + context: &'a C) + where E: TElement, C: StyleContext<'a> { let mut display_none = false; - ensure_node_styled_internal(node, context, &mut display_none); + ensure_element_styled_internal(element, context, &mut display_none); } #[allow(unsafe_code)] -fn ensure_node_styled_internal<'a, N, C>(node: N, - context: &'a C, - parents_had_display_none: &mut bool) - where N: TNode, +fn ensure_element_styled_internal<'a, E, C>(element: E, + context: &'a C, + parents_had_display_none: &mut bool) + where E: TElement, C: StyleContext<'a> { use properties::longhands::display::computed_value as display; @@ -238,13 +238,9 @@ fn ensure_node_styled_internal<'a, N, C>(node: N, // This means potentially a bit of wasted work (usually not much). We could // add a flag at the node at which point we stopped the traversal to know // where should we stop, but let's not add that complication unless needed. - let parent = match node.parent_node() { - Some(parent) if parent.is_element() => Some(parent), - _ => None, - }; - + let parent = element.parent_element(); if let Some(parent) = parent { - ensure_node_styled_internal(parent, context, parents_had_display_none); + ensure_element_styled_internal(parent, context, parents_had_display_none); } // Common case: our style is already resolved and none of our ancestors had @@ -252,6 +248,7 @@ fn ensure_node_styled_internal<'a, N, C>(node: N, // // We only need to mark whether we have display none, and forget about it, // our style is up to date. + let node = element.as_node(); if let Some(data) = node.borrow_data() { if let Some(style) = data.get_current_styles().map(|x| &x.primary) { if !*parents_had_display_none { @@ -268,16 +265,14 @@ fn ensure_node_styled_internal<'a, N, C>(node: N, // probably not necessary since we're likely to be matching only a few // nodes, at best. let mut applicable_declarations = ApplicableDeclarations::new(); - if let Some(element) = node.as_element() { - let stylist = &context.shared_context().stylist; + let stylist = &context.shared_context().stylist; - element.match_element(&**stylist, - None, - &mut applicable_declarations); - } + element.match_element(&**stylist, + None, + &mut applicable_declarations); unsafe { - node.cascade_node(context, parent, &applicable_declarations); + element.cascade_node(context, parent, &applicable_declarations); } } @@ -291,34 +286,21 @@ pub fn recalc_style_at<'a, E, C, D>(context: &'a C, C: StyleContext<'a>, D: DomTraversalContext { - let node = element.as_node(); - - // Get the parent node. - let parent_opt = match node.parent_node() { - Some(parent) if parent.is_element() => Some(parent), - _ => None, - }; - // Get the style bloom filter. - let mut bf = take_thread_local_bloom_filter(parent_opt, root, context.shared_context()); + let mut bf = take_thread_local_bloom_filter(element.parent_element(), root, context.shared_context()); let mut restyle_result = RestyleResult::Continue; - let mode = node.styling_mode(); + let mode = element.as_node().styling_mode(); debug_assert!(mode != StylingMode::Stop, "Parent should not have enqueued us"); if mode != StylingMode::Traverse { // Check to see whether we can share a style with someone. let style_sharing_candidate_cache = &mut context.local_context().style_sharing_candidate_cache.borrow_mut(); - let sharing_result = match node.as_element() { - Some(element) => { - unsafe { - element.share_style_if_possible(style_sharing_candidate_cache, - context.shared_context(), - parent_opt.clone()) - } - }, - None => StyleSharingResult::CannotShare, + let sharing_result = if element.parent_element().is_none() { + StyleSharingResult::CannotShare + } else { + unsafe { element.share_style_if_possible(style_sharing_candidate_cache, context.shared_context()) } }; // Otherwise, match and cascade selectors. @@ -327,38 +309,32 @@ pub fn recalc_style_at<'a, E, C, D>(context: &'a C, let mut applicable_declarations = ApplicableDeclarations::new(); let relations; - let shareable_element = match node.as_element() { - Some(element) => { - if opts::get().style_sharing_stats { - STYLE_SHARING_CACHE_MISSES.fetch_add(1, Ordering::Relaxed); - } + let shareable_element = { + if opts::get().style_sharing_stats { + STYLE_SHARING_CACHE_MISSES.fetch_add(1, Ordering::Relaxed); + } - // Perform the CSS selector matching. - let stylist = &context.shared_context().stylist; + // Perform the CSS selector matching. + let stylist = &context.shared_context().stylist; - relations = element.match_element(&**stylist, - Some(&*bf), - &mut applicable_declarations); + relations = element.match_element(&**stylist, + Some(&*bf), + &mut applicable_declarations); - debug!("Result of selector matching: {:?}", relations); + debug!("Result of selector matching: {:?}", relations); - if relations_are_shareable(&relations) { - Some(element) - } else { - None - } - }, - None => { - relations = StyleRelations::empty(); + if relations_are_shareable(&relations) { + Some(element) + } else { None - }, + } }; // Perform the CSS cascade. unsafe { - restyle_result = node.cascade_node(context, - parent_opt, - &applicable_declarations); + restyle_result = element.cascade_node(context, + element.parent_element(), + &applicable_declarations); } // Add ourselves to the LRU cache. @@ -372,7 +348,7 @@ pub fn recalc_style_at<'a, E, C, D>(context: &'a C, STYLE_SHARING_CACHE_HITS.fetch_add(1, Ordering::Relaxed); } style_sharing_candidate_cache.touch(index); - node.as_element().unwrap().set_restyle_damage(damage); + element.set_restyle_damage(damage); } } } @@ -381,7 +357,7 @@ pub fn recalc_style_at<'a, E, C, D>(context: &'a C, // processing. The eventual algorithm we're designing does this in a more granular // fashion. if mode == StylingMode::Restyle && restyle_result == RestyleResult::Continue { - for kid in node.children() { + for kid in element.as_node().children() { let mut data = D::ensure_node_data(&kid).borrow_mut(); if kid.is_text_node() { data.ensure_restyle_data(); @@ -394,12 +370,12 @@ pub fn recalc_style_at<'a, E, C, D>(context: &'a C, } } - let unsafe_layout_node = node.to_unsafe(); + let unsafe_layout_node = element.as_node().to_unsafe(); // Before running the children, we need to insert our nodes into the bloom // filter. debug!("[{}] + {:X}", tid(), unsafe_layout_node.0); - node.insert_into_bloom_filter(&mut *bf); + element.insert_into_bloom_filter(&mut *bf); // NB: flow construction updates the bloom filter on the way up. put_thread_local_bloom_filter(bf, &unsafe_layout_node, context.shared_context());