From 05c1f1e01699aa974aa2d39ec06594081724326c Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Fri, 21 Oct 2016 16:34:23 -0700 Subject: [PATCH] Introduce StylingMode and deprecate explicit dirtiness. MozReview-Commit-ID: 5tF075EJKBa --- components/layout/traversal.rs | 14 +++-- components/layout_thread/lib.rs | 2 +- components/script/layout_wrapper.rs | 32 +++++++++--- components/style/data.rs | 15 +++--- components/style/dom.rs | 78 +++++++++++++++++++++++++--- components/style/gecko/traversal.rs | 8 ++- components/style/gecko/wrapper.rs | 38 ++++++-------- components/style/parallel.rs | 28 +++------- components/style/sequential.rs | 16 ++---- components/style/traversal.rs | 80 ++++++++++++++++++----------- ports/geckolib/glue.rs | 9 ++-- 11 files changed, 204 insertions(+), 116 deletions(-) diff --git a/components/layout/traversal.rs b/components/layout/traversal.rs index bec162f0010..5cd1a236f5c 100644 --- a/components/layout/traversal.rs +++ b/components/layout/traversal.rs @@ -13,8 +13,10 @@ use gfx::display_list::OpaqueNode; use script_layout_interface::restyle_damage::{BUBBLE_ISIZES, REFLOW, REFLOW_OUT_OF_FLOW, REPAINT, RestyleDamage}; use script_layout_interface::wrapper_traits::{LayoutNode, ThreadSafeLayoutNode}; use std::mem; +use style::atomic_refcell::AtomicRefCell; use style::context::{LocalStyleContext, SharedStyleContext, StyleContext}; -use style::dom::TNode; +use style::data::NodeData; +use style::dom::{TNode, TRestyleDamage}; use style::selector_impl::ServoSelectorImpl; use style::traversal::{DomTraversalContext, recalc_style_at, remove_from_bloom_filter}; use style::traversal::RestyleResult; @@ -75,13 +77,18 @@ impl<'lc, N> DomTraversalContext for RecalcStyleAndConstructFlows<'lc> // done by the HTML parser. node.initialize_data(); - recalc_style_at(&self.context, self.root, node) + recalc_style_at::<_, _, Self>(&self.context, self.root, node) } fn process_postorder(&self, node: N) { construct_flows_at(&self.context, self.root, node); } + fn ensure_node_data(node: &N) -> &AtomicRefCell { + node.initialize_data(); + node.get_style_data().unwrap() + } + fn local_context(&self) -> &LocalStyleContext { self.context.local_context() } @@ -103,7 +110,8 @@ 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.is_dirty() || node.has_dirty_descendants() { + if nonincremental_layout || node.has_dirty_descendants() || + node.restyle_damage() != N::ConcreteRestyleDamage::empty() { let mut flow_constructor = FlowConstructor::new(context); if nonincremental_layout || !flow_constructor.repair_if_possible(&tnode) { flow_constructor.process(&tnode); diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs index 24c59c1b339..51ec52cb0d0 100644 --- a/components/layout_thread/lib.rs +++ b/components/layout_thread/lib.rs @@ -1146,7 +1146,7 @@ impl LayoutThread { if !needs_dirtying { for (el, snapshot) in modified_elements { let hint = rw_data.stylist.compute_restyle_hint(&el, &snapshot, el.get_state()); - el.note_restyle_hint(hint); + el.note_restyle_hint::(hint); } } diff --git a/components/script/layout_wrapper.rs b/components/script/layout_wrapper.rs index 653bcae478b..4d615d2a50e 100644 --- a/components/script/layout_wrapper.rs +++ b/components/script/layout_wrapper.rs @@ -186,14 +186,10 @@ impl<'ln> TNode for ServoLayoutNode<'ln> { self.node.downcast().map(ServoLayoutDocument::from_layout_js) } - fn is_dirty(&self) -> bool { + fn deprecated_dirty_bit_is_set(&self) -> bool { unsafe { self.node.get_flag(IS_DIRTY) } } - unsafe fn set_dirty(&self) { - self.node.set_flag(IS_DIRTY, true) - } - fn has_dirty_descendants(&self) -> bool { unsafe { self.node.get_flag(HAS_DIRTY_DESCENDANTS) } } @@ -242,6 +238,20 @@ impl<'ln> TNode for ServoLayoutNode<'ln> { 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; } } @@ -358,6 +368,14 @@ impl<'ln> LayoutNode for ServoLayoutNode<'ln> { } impl<'ln> ServoLayoutNode<'ln> { + pub fn is_dirty(&self) -> bool { + unsafe { self.node.get_flag(IS_DIRTY) } + } + + pub unsafe fn set_dirty(&self) { + self.node.set_flag(IS_DIRTY, true) + } + fn get_partial_layout_data(&self) -> Option<&AtomicRefCell> { unsafe { self.get_jsmanaged().get_style_and_layout_data().map(|d| { @@ -397,7 +415,9 @@ impl<'ln> ServoLayoutNode<'ln> { fn debug_str(self) -> String { format!("{:?}: changed={} dirty={} dirty_descendants={}", - self.script_type_id(), self.has_changed(), self.is_dirty(), self.has_dirty_descendants()) + self.script_type_id(), self.has_changed(), + self.deprecated_dirty_bit_is_set(), + self.has_dirty_descendants()) } fn debug_style_str(self) -> String { diff --git a/components/style/data.rs b/components/style/data.rs index 511b1e174c3..9e6d6054fb0 100644 --- a/components/style/data.rs +++ b/components/style/data.rs @@ -115,14 +115,14 @@ impl RestyleData { #[derive(Debug)] pub struct NodeData { styles: NodeDataStyles, - pub restyle: Option, + pub restyle_data: Option, } impl NodeData { pub fn new() -> Self { NodeData { styles: NodeDataStyles::Uninitialized, - restyle: None, + restyle_data: None, } } @@ -175,25 +175,24 @@ impl NodeData { self.styles = match mem::replace(&mut self.styles, Uninitialized) { Uninitialized => Previous(f()), Current(x) => Previous(Some(x)), - _ => panic!("Already have previous styles"), + Previous(x) => Previous(x), }; } - // FIXME(bholley): Called in future patches. pub fn ensure_restyle_data(&mut self) { - if self.restyle.is_none() { - self.restyle = Some(RestyleData::new()); + if self.restyle_data.is_none() { + self.restyle_data = Some(RestyleData::new()); } } pub fn style_text_node(&mut self, style: Arc) { - debug_assert!(self.restyle.is_none()); self.styles = NodeDataStyles::Current(NodeStyles::new(style)); + self.restyle_data = None; } pub fn finish_styling(&mut self, styles: NodeStyles) { debug_assert!(self.styles.is_previous()); self.styles = NodeDataStyles::Current(styles); - self.restyle = None; + self.restyle_data = None; } } diff --git a/components/style/dom.rs b/components/style/dom.rs index d8b82d94d04..8f8a53f2c1f 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -7,7 +7,7 @@ #![allow(unsafe_code)] use atomic_refcell::{AtomicRef, AtomicRefMut}; -use data::NodeData; +use data::{NodeStyles, NodeData}; use element_state::ElementState; use parking_lot::RwLock; use properties::{ComputedValues, PropertyDeclarationBlock}; @@ -19,6 +19,8 @@ use std::fmt::Debug; use std::ops::BitOr; use std::sync::Arc; use string_cache::{Atom, Namespace}; +use traversal::DomTraversalContext; +use util::opts; pub use style_traits::UnsafeNode; @@ -43,6 +45,19 @@ impl OpaqueNode { } } +#[derive(Clone, Copy, PartialEq)] +pub enum StylingMode { + /// The node has never been styled before, and needs a full style computation. + Initial, + /// The node has been styled before, but needs some amount of recomputation. + Restyle, + /// The node does not need any style processing, but one or more of its + /// descendants do. + Traverse, + /// No nodes in this subtree require style processing. + Stop, +} + pub trait TRestyleDamage : Debug + PartialEq + BitOr + Copy { /// The source for our current computed values in the cascade. This is a /// ComputedValues in Servo and a StyleContext in Gecko. @@ -117,9 +132,11 @@ pub trait TNode : Sized + Copy + Clone + NodeInfo { fn as_document(&self) -> Option; - fn is_dirty(&self) -> bool; - - unsafe fn set_dirty(&self); + /// The concept of a dirty bit doesn't exist in our new restyle algorithm. + /// Instead, we associate restyle and change hints with nodes. However, we + /// continue to allow the dirty bit to trigger unconditional restyles while + /// we transition both Servo and Stylo to the new architecture. + fn deprecated_dirty_bit_is_set(&self) -> bool; fn has_dirty_descendants(&self) -> bool; @@ -141,6 +158,51 @@ pub trait TNode : Sized + Copy + Clone + NodeInfo { /// traversal. Returns the number of children left to process. fn did_process_child(&self) -> isize; + /// Returns true if this node has a styled layout frame that owns the style. + fn frame_has_style(&self) -> bool { false } + + /// Returns the styles from the layout frame that owns them, if any. + /// + /// FIXME(bholley): Once we start dropping NodeData from nodes when + /// creating frames, we'll want to teach this method to actually get + /// style data from the frame. + fn get_styles_from_frame(&self) -> Option { None } + + /// Returns the styling mode for this node. This is only valid to call before + /// and during restyling, before finish_styling is invoked. + /// + /// See the comments around StylingMode. + fn styling_mode(&self) -> StylingMode { + use self::StylingMode::*; + + // Non-incremental layout impersonates Initial. + if opts::get().nonincremental_layout { + return Initial; + } + + // Compute the default result if this node doesn't require processing. + let mode_for_descendants = if self.has_dirty_descendants() { + Traverse + } else { + Stop + }; + + match self.borrow_data() { + // No node data, no style on the frame. + None if !self.frame_has_style() => Initial, + // No node data, style on the frame. + None => mode_for_descendants, + Some(d) => { + if d.restyle_data.is_some() || self.deprecated_dirty_bit_is_set() { + Restyle + } else { + debug_assert!(!self.frame_has_style()); // display:none etc + mode_for_descendants + } + }, + } + } + /// Sets up the appropriate data structures to style a node, returing a /// mutable handle to the node data upon which further style calculations /// can be performed. @@ -212,7 +274,7 @@ pub trait TElement : PartialEq + Debug + Sized + Copy + Clone + ElementExt + Pre fn attr_equals(&self, namespace: &Namespace, attr: &Atom, value: &Atom) -> bool; /// Properly marks nodes as dirty in response to restyle hints. - fn note_restyle_hint(&self, hint: RestyleHint) { + fn note_restyle_hint>(&self, hint: RestyleHint) { // Bail early if there's no restyling to do. if hint.is_empty() { return; @@ -230,13 +292,13 @@ pub trait TElement : PartialEq + Debug + Sized + Copy + Clone + ElementExt + Pre // Process hints. if hint.contains(RESTYLE_SELF) { - unsafe { node.set_dirty(); } + unsafe { C::ensure_node_data(&node).borrow_mut().ensure_restyle_data(); } // XXX(emilio): For now, dirty implies dirty descendants if found. } else if hint.contains(RESTYLE_DESCENDANTS) { unsafe { node.set_dirty_descendants(); } let mut current = node.first_child(); while let Some(node) = current { - unsafe { node.set_dirty(); } + unsafe { C::ensure_node_data(&node).borrow_mut().ensure_restyle_data(); } current = node.next_sibling(); } } @@ -245,7 +307,7 @@ pub trait TElement : PartialEq + Debug + Sized + Copy + Clone + ElementExt + Pre let mut next = ::selectors::Element::next_sibling_element(self); while let Some(sib) = next { let sib_node = sib.as_node(); - unsafe { sib_node.set_dirty() }; + unsafe { C::ensure_node_data(&sib_node).borrow_mut().ensure_restyle_data() }; next = ::selectors::Element::next_sibling_element(&sib); } } diff --git a/components/style/gecko/traversal.rs b/components/style/gecko/traversal.rs index 2325bd1d303..61a9bd32de8 100644 --- a/components/style/gecko/traversal.rs +++ b/components/style/gecko/traversal.rs @@ -2,7 +2,9 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +use atomic_refcell::AtomicRefCell; use context::{LocalStyleContext, SharedStyleContext, StyleContext}; +use data::NodeData; use dom::OpaqueNode; use gecko::context::StandaloneStyleContext; use gecko::wrapper::GeckoNode; @@ -29,7 +31,7 @@ impl<'lc, 'ln> DomTraversalContext> for RecalcStyleOnly<'lc> { } fn process_preorder(&self, node: GeckoNode<'ln>) -> RestyleResult { - recalc_style_at(&self.context, self.root, node) + recalc_style_at::<_, _, Self>(&self.context, self.root, node) } fn process_postorder(&self, _: GeckoNode<'ln>) { @@ -39,6 +41,10 @@ impl<'lc, 'ln> DomTraversalContext> for RecalcStyleOnly<'lc> { /// We don't use the post-order traversal for anything. fn needs_postorder_traversal(&self) -> bool { false } + fn ensure_node_data<'a>(node: &'a GeckoNode<'ln>) -> &'a AtomicRefCell { + node.ensure_data() + } + fn local_context(&self) -> &LocalStyleContext { self.context.local_context() } diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index 9340dc68776..187aed0acd8 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -6,7 +6,7 @@ use atomic_refcell::{AtomicRef, AtomicRefCell, AtomicRefMut}; -use data::{NodeData, NodeStyles}; +use data::NodeData; use dom::{LayoutIterator, NodeInfo, TDocument, TElement, TNode, TRestyleDamage, UnsafeNode}; use dom::{OpaqueNode, PresentationalHintsSynthetizer}; use element_state::ElementState; @@ -95,18 +95,11 @@ impl<'ln> GeckoNode<'ln> { .get(pseudo).map(|c| c.clone())) } - fn styles_from_frame(&self) -> Option { - // FIXME(bholley): Once we start dropping NodeData from nodes when - // creating frames, we'll want to teach this method to actually get - // style data from the frame. - None - } - fn get_node_data(&self) -> Option<&AtomicRefCell> { unsafe { self.0.mServoData.get().as_ref() } } - fn ensure_node_data(&self) -> &AtomicRefCell { + pub fn ensure_data(&self) -> &AtomicRefCell { match self.get_node_data() { Some(x) => x, None => { @@ -224,20 +217,10 @@ impl<'ln> TNode for GeckoNode<'ln> { unimplemented!() } - fn is_dirty(&self) -> bool { - // Return true unconditionally if we're not yet styled. This is a hack - // and should go away soon. - if self.get_node_data().is_none() { - return true; - } - + fn deprecated_dirty_bit_is_set(&self) -> bool { self.flags() & (NODE_IS_DIRTY_FOR_SERVO as u32) != 0 } - unsafe fn set_dirty(&self) { - self.set_flags(NODE_IS_DIRTY_FOR_SERVO as u32) - } - fn has_dirty_descendants(&self) -> bool { // Return true unconditionally if we're not yet styled. This is a hack // and should go away soon. @@ -271,14 +254,19 @@ impl<'ln> TNode for GeckoNode<'ln> { } fn begin_styling(&self) -> AtomicRefMut { - let mut data = self.ensure_node_data().borrow_mut(); - data.gather_previous_styles(|| self.styles_from_frame()); + let mut data = self.ensure_data().borrow_mut(); + data.gather_previous_styles(|| self.get_styles_from_frame()); data } fn style_text_node(&self, style: Arc) { debug_assert!(self.is_text_node()); - self.ensure_node_data().borrow_mut().style_text_node(style); + + // 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); } + + self.ensure_data().borrow_mut().style_text_node(style); } fn borrow_data(&self) -> Option> { @@ -291,6 +279,10 @@ impl<'ln> TNode for GeckoNode<'ln> { } 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) } } diff --git a/components/style/parallel.rs b/components/style/parallel.rs index d8dc89d43de..ad83d3ac475 100644 --- a/components/style/parallel.rs +++ b/components/style/parallel.rs @@ -8,7 +8,7 @@ #![allow(unsafe_code)] -use dom::{OpaqueNode, TNode, UnsafeNode}; +use dom::{OpaqueNode, StylingMode, TNode, UnsafeNode}; use std::mem; use std::sync::atomic::Ordering; use traversal::{RestyleResult, DomTraversalContext}; @@ -47,6 +47,7 @@ pub fn traverse_dom(root: N, where N: TNode, C: DomTraversalContext { + debug_assert!(root.styling_mode() != StylingMode::Stop); if opts::get().style_sharing_stats { STYLE_SHARING_CACHE_HITS.store(0, Ordering::SeqCst); STYLE_SHARING_CACHE_MISSES.store(0, Ordering::SeqCst); @@ -80,28 +81,13 @@ fn top_down_dom(unsafe_nodes: UnsafeNodeList, // Get a real layout node. let node = unsafe { N::from_unsafe(&unsafe_node) }; - if !context.should_process(node) { - continue; - } - - // Possibly enqueue the children. - let mut children_to_process = 0isize; // Perform the appropriate traversal. + let mut children_to_process = 0isize; if let RestyleResult::Continue = context.process_preorder(node) { - for kid in node.children() { - // Trigger the hook pre-adding the kid to the list. This can - // (and in fact uses to) change the result of the should_process - // operation. - // - // As of right now, this hook takes care of propagating the - // restyle flag down the tree. In the future, more accurate - // behavior is probably going to be needed. - context.pre_process_child_hook(node, kid); - if context.should_process(kid) { - children_to_process += 1; - discovered_child_nodes.push(kid.to_unsafe()) - } - } + C::traverse_children(node, |kid| { + children_to_process += 1; + discovered_child_nodes.push(kid.to_unsafe()) + }); } // Reset the count of children if we need to do a bottom-up traversal diff --git a/components/style/sequential.rs b/components/style/sequential.rs index d3db166b446..87925b4d5f7 100644 --- a/components/style/sequential.rs +++ b/components/style/sequential.rs @@ -4,7 +4,7 @@ //! Implements sequential traversal over the DOM tree. -use dom::TNode; +use dom::{StylingMode, TNode}; use traversal::{RestyleResult, DomTraversalContext}; pub fn traverse_dom(root: N, @@ -16,14 +16,8 @@ pub fn traverse_dom(root: N, where N: TNode, C: DomTraversalContext { - debug_assert!(context.should_process(node)); if let RestyleResult::Continue = context.process_preorder(node) { - for kid in node.children() { - context.pre_process_child_hook(node, kid); - if context.should_process(kid) { - doit::(context, kid); - } - } + C::traverse_children(node, |kid| doit::(context, kid)); } if context.needs_postorder_traversal() { @@ -31,10 +25,10 @@ pub fn traverse_dom(root: N, } } + debug_assert!(root.styling_mode() != StylingMode::Stop); let context = C::new(shared, root.opaque()); - if context.should_process(root) { - doit::(&context, root); - } + doit::(&context, root); + // Clear the local LRU cache since we store stateful elements inside. context.local_context().style_sharing_candidate_cache.borrow_mut().clear(); } diff --git a/components/style/traversal.rs b/components/style/traversal.rs index 0405c2f90bd..1efe9721b6c 100644 --- a/components/style/traversal.rs +++ b/components/style/traversal.rs @@ -4,8 +4,10 @@ //! Traversing the DOM tree; the bloom filter. +use atomic_refcell::AtomicRefCell; use context::{LocalStyleContext, SharedStyleContext, StyleContext}; -use dom::{OpaqueNode, TNode, UnsafeNode}; +use data::NodeData; +use dom::{OpaqueNode, StylingMode, TNode, UnsafeNode}; use matching::{ApplicableDeclarations, ElementMatchMethods, MatchMethods, StyleSharingResult}; use selectors::bloom::BloomFilter; use selectors::matching::StyleRelations; @@ -22,6 +24,7 @@ pub type Generation = u32; /// an element. /// /// So far this only happens where a display: none node is found. +#[derive(Clone, Copy, PartialEq)] pub enum RestyleResult { Continue, Stop, @@ -172,30 +175,30 @@ pub trait DomTraversalContext { /// If it's false, then process_postorder has no effect at all. fn needs_postorder_traversal(&self) -> bool { true } - /// Returns if the node should be processed by the preorder traversal (and - /// then by the post-order one). - /// - /// Note that this is true unconditionally for servo, since it requires to - /// bubble the widths bottom-up for all the DOM. - fn should_process(&self, node: N) -> bool { - opts::get().nonincremental_layout || node.is_dirty() || node.has_dirty_descendants() - } + /// Helper for the traversal implementations to select the children that + /// should be enqueued for processing. + fn traverse_children(parent: N, mut f: F) + { + // If we enqueue any children for traversal, we need to set the dirty + // descendants bit. Avoid doing it more than once. + let mut marked_dirty_descendants = false; - /// Do an action over the child before pushing him to the work queue. - /// - /// By default, propagate the IS_DIRTY flag down the tree. - #[allow(unsafe_code)] - fn pre_process_child_hook(&self, parent: N, kid: N) { - // NOTE: At this point is completely safe to modify either the parent or - // the child, since we have exclusive access to both of them. - if parent.is_dirty() { - unsafe { - kid.set_dirty(); - parent.set_dirty_descendants(); + for kid in parent.children() { + if kid.styling_mode() != StylingMode::Stop { + if !marked_dirty_descendants { + unsafe { parent.set_dirty_descendants(); } + marked_dirty_descendants = true; + } + f(kid); } } } + /// Ensures the existence of the NodeData, and returns it. This can't live + /// on TNode because of the trait-based separation between Servo's script + /// and layout crates. + fn ensure_node_data(node: &N) -> &AtomicRefCell; + fn local_context(&self) -> &LocalStyleContext; } @@ -281,11 +284,12 @@ 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>(context: &'a C, - root: OpaqueNode, - node: N) -> RestyleResult +pub fn recalc_style_at<'a, N, C, D>(context: &'a C, + root: OpaqueNode, + node: N) -> RestyleResult where N: TNode, - C: StyleContext<'a> + C: StyleContext<'a>, + D: DomTraversalContext { // Get the parent node. let parent_opt = match node.parent_node() { @@ -296,9 +300,10 @@ pub fn recalc_style_at<'a, N, C>(context: &'a C, // Get the style bloom filter. let mut bf = take_thread_local_bloom_filter(parent_opt, root, context.shared_context()); - let nonincremental_layout = opts::get().nonincremental_layout; let mut restyle_result = RestyleResult::Continue; - if nonincremental_layout || node.is_dirty() { + let mode = 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(); @@ -370,6 +375,23 @@ pub fn recalc_style_at<'a, N, C>(context: &'a C, } } + // If we restyled this node, conservatively mark all our children as needing + // 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() { + let mut data = D::ensure_node_data(&kid).borrow_mut(); + if kid.is_text_node() { + data.ensure_restyle_data(); + } else { + data.gather_previous_styles(|| kid.get_styles_from_frame()); + if data.previous_styles().is_some() { + data.ensure_restyle_data(); + } + } + } + } + let unsafe_layout_node = node.to_unsafe(); // Before running the children, we need to insert our nodes into the bloom @@ -380,9 +402,5 @@ pub fn recalc_style_at<'a, N, C>(context: &'a C, // NB: flow construction updates the bloom filter on the way up. put_thread_local_bloom_filter(bf, &unsafe_layout_node, context.shared_context()); - if nonincremental_layout { - RestyleResult::Continue - } else { - restyle_result - } + restyle_result } diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index 679295aa040..ed9caef877f 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -13,7 +13,7 @@ use std::str::from_utf8_unchecked; use std::sync::{Arc, Mutex}; use style::arc_ptr_eq; use style::context::{LocalStyleContextCreationInfo, ReflowGoal, SharedStyleContext}; -use style::dom::{NodeInfo, TElement, TNode}; +use style::dom::{NodeInfo, StylingMode, TElement, TNode}; use style::error_reporting::StdoutErrorReporter; use style::gecko::data::{NUM_THREADS, PerDocumentStyleData}; use style::gecko::selector_impl::{GeckoSelectorImpl, PseudoElement}; @@ -107,8 +107,11 @@ fn restyle_subtree(node: GeckoNode, raw_data: RawServoStyleSetBorrowed) { timer: Timer::new(), }; - // We ensure this is true before calling Servo_RestyleSubtree() - debug_assert!(node.is_dirty() || node.has_dirty_descendants()); + if node.styling_mode() == StylingMode::Stop { + error!("Unnecessary call to restyle_subtree"); + return; + } + if per_doc_data.num_threads == 1 || per_doc_data.work_queue.is_none() { sequential::traverse_dom::(node, &shared_style_context); } else {