diff options
| -rw-r--r-- | components/layout/animation.rs | 2 | ||||
| -rw-r--r-- | components/layout/construct.rs | 8 | ||||
| -rw-r--r-- | components/layout/query.rs | 37 | ||||
| -rw-r--r-- | components/layout/traversal.rs | 9 | ||||
| -rw-r--r-- | components/layout_thread/lib.rs | 7 | ||||
| -rw-r--r-- | components/script/layout_wrapper.rs | 5 | ||||
| -rw-r--r-- | components/script_layout_interface/restyle_damage.rs | 14 | ||||
| -rw-r--r-- | components/style/dom.rs | 12 | ||||
| -rw-r--r-- | components/style/matching.rs | 337 | ||||
| -rw-r--r-- | components/style/parallel.rs | 53 | ||||
| -rw-r--r-- | components/style/selector_impl.rs | 6 | ||||
| -rw-r--r-- | components/style/sequential.rs | 18 | ||||
| -rw-r--r-- | components/style/traversal.rs | 117 | ||||
| -rw-r--r-- | ports/geckolib/traversal.rs | 12 | ||||
| -rw-r--r-- | ports/geckolib/wrapper.rs | 34 | ||||
| -rw-r--r-- | tests/wpt/metadata-css/css-transitions-1_dev/html/hidden-container-001.htm.ini | 5 | ||||
| -rw-r--r-- | tests/wpt/mozilla/meta/MANIFEST.json | 24 | ||||
| -rw-r--r-- | tests/wpt/mozilla/tests/css/pseudo_element_restyle_no_rules.html | 20 | ||||
| -rw-r--r-- | tests/wpt/mozilla/tests/css/pseudo_element_restyle_no_rules_ref.html | 5 |
19 files changed, 544 insertions, 181 deletions
diff --git a/components/layout/animation.rs b/components/layout/animation.rs index 5222d4ba70a..71a388dcf0c 100644 --- a/components/layout/animation.rs +++ b/components/layout/animation.rs @@ -135,7 +135,7 @@ pub fn recalc_style_for_animations(context: &SharedLayoutContext, update_style_for_animation(&context.style_context, animation, &mut fragment.style); - damage |= RestyleDamage::compute(Some(&old_style), &fragment.style); + damage |= RestyleDamage::compute(&old_style, &fragment.style); } } }); diff --git a/components/layout/construct.rs b/components/layout/construct.rs index 809ecaa406a..33b88f6b8c0 100644 --- a/components/layout/construct.rs +++ b/components/layout/construct.rs @@ -1374,12 +1374,20 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode> // We visit the kids first and reset their HAS_NEWLY_CONSTRUCTED_FLOW flags after checking // them. NOTE: Make sure not to bail out early before resetting all the flags! let mut need_to_reconstruct = false; + + // If the node has display: none, it's possible that we haven't even + // styled the children once, so we need to bailout early here. + if node.style(self.style_context()).get_box().clone_display() == display::T::none { + return false; + } + for kid in node.children() { if kid.flags().contains(HAS_NEWLY_CONSTRUCTED_FLOW) { kid.remove_flags(HAS_NEWLY_CONSTRUCTED_FLOW); need_to_reconstruct = true } } + if need_to_reconstruct { return false } diff --git a/components/layout/query.rs b/components/layout/query.rs index ee06f3865ba..d8742973460 100644 --- a/components/layout/query.rs +++ b/components/layout/query.rs @@ -30,6 +30,7 @@ use std::ops::Deref; use std::sync::{Arc, Mutex}; use string_cache::Atom; use style::computed_values; +use style::context::StyleContext; use style::logical_geometry::{WritingMode, BlockFlowDirection, InlineBaseDirection}; use style::properties::longhands::{display, position}; use style::properties::style_structs; @@ -37,7 +38,7 @@ use style::selector_impl::PseudoElement; use style::selector_matching::Stylist; use style::values::LocalToCss; use style_traits::cursor::Cursor; -use wrapper::ThreadSafeLayoutNodeHelpers; +use wrapper::{LayoutNodeLayoutData, ThreadSafeLayoutNodeHelpers}; /// Mutable data belonging to the LayoutThread. /// @@ -620,11 +621,39 @@ pub fn process_node_scroll_area_request< N: LayoutNode>(requested_node: N, layou } } +/// Ensures that a node's data, and all its parents' is initialized. This is +/// needed to resolve style lazily. +fn ensure_node_data_initialized<N: LayoutNode>(node: &N) { + let mut cur = Some(node.clone()); + while let Some(current) = cur { + if current.borrow_data().is_some() { + break; + } + + current.initialize_data(); + cur = current.parent_node(); + } +} + /// Return the resolved value of property for a given (pseudo)element. /// https://drafts.csswg.org/cssom/#resolved-value -pub fn process_resolved_style_request<N: LayoutNode>( - requested_node: N, pseudo: &Option<PseudoElement>, - property: &Atom, layout_root: &mut FlowRef) -> Option<String> { +pub fn process_resolved_style_request<'a, N, C>(requested_node: N, + style_context: &'a C, + pseudo: &Option<PseudoElement>, + property: &Atom, + layout_root: &mut FlowRef) -> Option<String> + where N: LayoutNode, + C: StyleContext<'a> +{ + use style::traversal::ensure_node_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); + let layout_node = requested_node.to_threadsafe(); let layout_node = match *pseudo { Some(PseudoElement::Before) => layout_node.get_before_pseudo(), diff --git a/components/layout/traversal.rs b/components/layout/traversal.rs index f9dcd975f9c..a8c94c52715 100644 --- a/components/layout/traversal.rs +++ b/components/layout/traversal.rs @@ -16,6 +16,7 @@ use std::mem; use style::context::SharedStyleContext; use style::dom::TNode; use style::selector_impl::ServoSelectorImpl; +use style::traversal::RestyleResult; use style::traversal::{DomTraversalContext, remove_from_bloom_filter, recalc_style_at}; use util::opts; use wrapper::{LayoutNodeLayoutData, ThreadSafeLayoutNodeHelpers}; @@ -69,12 +70,12 @@ impl<'lc, N> DomTraversalContext<N> for RecalcStyleAndConstructFlows<'lc> } } - fn process_preorder(&self, node: N) { - // FIXME(pcwalton): Stop allocating here. Ideally this should just be done by the HTML - // parser. + fn process_preorder(&self, node: N) -> RestyleResult { + // FIXME(pcwalton): Stop allocating here. Ideally this should just be + // done by the HTML parser. node.initialize_data(); - recalc_style_at(&self.context, self.root, node); + recalc_style_at(&self.context, self.root, node) } fn process_postorder(&self, node: N) { diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs index c31e2de54ac..da5d2ec5a75 100644 --- a/components/layout_thread/lib.rs +++ b/components/layout_thread/lib.rs @@ -1237,8 +1237,13 @@ impl LayoutThread { }, ReflowQueryType::ResolvedStyleQuery(node, ref pseudo, ref property) => { let node = unsafe { ServoLayoutNode::new(&node) }; + let layout_context = LayoutContext::new(&shared_layout_context); rw_data.resolved_style_response = - process_resolved_style_request(node, pseudo, property, &mut root_flow); + process_resolved_style_request(node, + &layout_context, + pseudo, + property, + &mut root_flow); }, ReflowQueryType::OffsetParentQuery(node) => { let node = unsafe { ServoLayoutNode::new(&node) }; diff --git a/components/script/layout_wrapper.rs b/components/script/layout_wrapper.rs index d4518b015e8..c13db423982 100644 --- a/components/script/layout_wrapper.rs +++ b/components/script/layout_wrapper.rs @@ -61,7 +61,7 @@ use style::dom::{PresentationalHintsSynthetizer, OpaqueNode, TDocument, TElement use style::element_state::*; use style::properties::{ComputedValues, PropertyDeclaration, PropertyDeclarationBlock}; use style::refcell::{Ref, RefCell, RefMut}; -use style::selector_impl::{ElementSnapshot, NonTSPseudoClass, ServoSelectorImpl}; +use style::selector_impl::{ElementSnapshot, NonTSPseudoClass, PseudoElement, ServoSelectorImpl}; use style::sink::Push; use style::str::is_whitespace; use url::Url; @@ -266,7 +266,8 @@ impl<'ln> TNode for ServoLayoutNode<'ln> { #[inline] fn existing_style_for_restyle_damage<'a>(&'a self, - current_cv: Option<&'a Arc<ComputedValues>>) + current_cv: Option<&'a Arc<ComputedValues>>, + _pseudo_element: Option<&PseudoElement>) -> Option<&'a Arc<ComputedValues>> { current_cv } diff --git a/components/script_layout_interface/restyle_damage.rs b/components/script_layout_interface/restyle_damage.rs index fc7c11d4259..9e2cd1db514 100644 --- a/components/script_layout_interface/restyle_damage.rs +++ b/components/script_layout_interface/restyle_damage.rs @@ -47,7 +47,11 @@ impl TRestyleDamage for RestyleDamage { /// For Servo the style source is always the computed values. type PreExistingComputedValues = Arc<ServoComputedValues>; - fn compute(old: Option<&Arc<ServoComputedValues>>, + fn empty() -> Self { + RestyleDamage::empty() + } + + fn compute(old: &Arc<ServoComputedValues>, new: &Arc<ServoComputedValues>) -> RestyleDamage { compute_damage(old, new) } @@ -150,13 +154,7 @@ macro_rules! add_if_not_equal( }) ); -fn compute_damage(old: Option<&Arc<ServoComputedValues>>, new: &Arc<ServoComputedValues>) -> RestyleDamage { - let new = &**new; - let old: &ServoComputedValues = match old { - None => return RestyleDamage::rebuild_and_reflow(), - Some(cv) => &**cv, - }; - +fn compute_damage(old: &ServoComputedValues, new: &ServoComputedValues) -> RestyleDamage { let mut damage = RestyleDamage::empty(); // This should check every CSS property, as enumerated in the fields of diff --git a/components/style/dom.rs b/components/style/dom.rs index 510f139b401..c1d444d2729 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -12,9 +12,10 @@ use element_state::ElementState; use properties::{ComputedValues, PropertyDeclaration, PropertyDeclarationBlock}; use refcell::{Ref, RefMut}; use restyle_hints::{RESTYLE_DESCENDANTS, RESTYLE_LATER_SIBLINGS, RESTYLE_SELF, RestyleHint}; -use selector_impl::ElementExt; +use selector_impl::{ElementExt, PseudoElement}; use selectors::matching::DeclarationBlock; use sink::Push; +use std::fmt::Debug; use std::ops::BitOr; use std::sync::Arc; use string_cache::{Atom, Namespace}; @@ -44,7 +45,7 @@ impl OpaqueNode { } } -pub trait TRestyleDamage : BitOr<Output=Self> + Copy { +pub trait TRestyleDamage : Debug + PartialEq + BitOr<Output=Self> + Copy { /// The source for our current computed values in the cascade. This is a /// ComputedValues in Servo and a StyleContext in Gecko. /// @@ -55,9 +56,11 @@ pub trait TRestyleDamage : BitOr<Output=Self> + Copy { /// This should be obtained via TNode::existing_style_for_restyle_damage type PreExistingComputedValues; - fn compute(old: Option<&Self::PreExistingComputedValues>, + fn compute(old: &Self::PreExistingComputedValues, new: &Arc<ComputedValues>) -> Self; + fn empty() -> Self; + fn rebuild_and_reflow() -> Self; } @@ -174,7 +177,8 @@ pub trait TNode : Sized + Copy + Clone { /// 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<ComputedValues>>) + current_computed_values: Option<&'a Arc<ComputedValues>>, + pseudo: Option<&PseudoElement>) -> Option<&'a <Self::ConcreteRestyleDamage as TRestyleDamage>::PreExistingComputedValues>; } diff --git a/components/style/matching.rs b/components/style/matching.rs index 170534940a7..16f0b7571b1 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -12,6 +12,7 @@ use cache::{LRUCache, SimpleHashCache}; use context::{StyleContext, SharedStyleContext}; use data::PrivateStyleData; use dom::{TElement, TNode, TRestyleDamage}; +use properties::longhands::display::computed_value as display; use properties::{ComputedValues, PropertyDeclaration, cascade}; use selector_impl::{ElementExt, TheSelectorImpl, PseudoElement}; use selector_matching::{DeclarationBlock, Stylist}; @@ -25,6 +26,7 @@ use std::hash::{BuildHasherDefault, Hash, Hasher}; use std::slice::Iter; use std::sync::Arc; use string_cache::{Atom, Namespace}; +use traversal::RestyleResult; use util::opts; fn create_common_style_affecting_attributes_from_element<E: TElement>(element: &E) @@ -410,9 +412,11 @@ impl StyleSharingCandidateCache { pub enum StyleSharingResult<ConcreteRestyleDamage: TRestyleDamage> { /// We didn't find anybody to share the style with. CannotShare, - /// The node's style can be shared. The integer specifies the index in the LRU cache that was - /// hit and the damage that was done. - StyleWasShared(usize, ConcreteRestyleDamage), + /// The node's style can be shared. The integer specifies the index in the + /// LRU cache that was hit and the damage that was done, and the restyle + /// result the original result of the candidate's styling, that is, whether + /// it should stop the traversal or not. + StyleWasShared(usize, ConcreteRestyleDamage, RestyleResult), } trait PrivateMatchMethods: TNode { @@ -424,22 +428,22 @@ trait PrivateMatchMethods: TNode { context: &Ctx, parent_style: Option<&Arc<ComputedValues>>, applicable_declarations: &[DeclarationBlock], - mut style: Option<&mut Arc<ComputedValues>>, + mut old_style: Option<&mut Arc<ComputedValues>>, applicable_declarations_cache: &mut ApplicableDeclarationsCache, shareable: bool, animate_properties: bool) - -> (Self::ConcreteRestyleDamage, Arc<ComputedValues>) - where Ctx: StyleContext<'a> { + -> Arc<ComputedValues> + where Ctx: StyleContext<'a> + { let mut cacheable = true; let shared_context = context.shared_context(); if animate_properties { cacheable = !self.update_animations_for_cascade(shared_context, - &mut style) && cacheable; + &mut old_style) && cacheable; } - let this_style; - match parent_style { + let (this_style, is_cacheable) = match parent_style { Some(ref parent_style) => { let cache_entry = applicable_declarations_cache.find(applicable_declarations); let cached_computed_values = match cache_entry { @@ -447,27 +451,25 @@ trait PrivateMatchMethods: TNode { None => None, }; - let (the_style, is_cacheable) = cascade(shared_context.viewport_size, - applicable_declarations, - shareable, - Some(&***parent_style), - cached_computed_values, - shared_context.error_reporter.clone()); - cacheable = cacheable && is_cacheable; - this_style = the_style + cascade(shared_context.viewport_size, + applicable_declarations, + shareable, + Some(&***parent_style), + cached_computed_values, + shared_context.error_reporter.clone()) } None => { - let (the_style, is_cacheable) = cascade(shared_context.viewport_size, - applicable_declarations, - shareable, - None, - None, - shared_context.error_reporter.clone()); - cacheable = cacheable && is_cacheable; - this_style = the_style + cascade(shared_context.viewport_size, + applicable_declarations, + shareable, + None, + None, + shared_context.error_reporter.clone()) } }; + cacheable = cacheable && is_cacheable; + let mut this_style = Arc::new(this_style); if animate_properties { @@ -482,7 +484,7 @@ trait PrivateMatchMethods: TNode { // Trigger transitions if necessary. This will reset `this_style` back // to its old value if it did trigger a transition. - if let Some(ref style) = style { + if let Some(ref style) = old_style { animations_started |= animation::start_transitions_if_applicable( new_animations_sender, @@ -496,21 +498,13 @@ trait PrivateMatchMethods: TNode { } - let existing_style = - self.existing_style_for_restyle_damage(style.map(|s| &*s)); - - // Calculate style difference. - let damage = - Self::ConcreteRestyleDamage::compute(existing_style, &this_style); - // Cache the resolved style if it was cacheable. if cacheable { applicable_declarations_cache.insert(applicable_declarations.to_vec(), this_style.clone()); } - // Return the final style and the damage done to our caller. - (damage, this_style) + this_style } fn update_animations_for_cascade(&self, @@ -646,16 +640,27 @@ pub trait ElementMatchMethods : TElement { let style = &mut node.mutate_data().unwrap().style; - let damage = { - let source = - node.existing_style_for_restyle_damage((*style).as_ref()); - let damage = <<Self as TElement>::ConcreteNode as TNode> - ::ConcreteRestyleDamage::compute(source, &shared_style); - damage + let damage = + match node.existing_style_for_restyle_damage((*style).as_ref(), None) { + Some(ref source) => { + <<Self as TElement>::ConcreteNode as TNode> + ::ConcreteRestyleDamage::compute(source, &shared_style) + } + None => { + <<Self as TElement>::ConcreteNode as TNode> + ::ConcreteRestyleDamage::rebuild_and_reflow() + } + }; + + let restyle_result = if shared_style.get_box().clone_display() == display::T::none { + RestyleResult::Stop + } else { + RestyleResult::Continue }; *style = Some(shared_style); - return StyleSharingResult::StyleWasShared(i, damage) + + return StyleSharingResult::StyleWasShared(i, damage, restyle_result) } } @@ -714,11 +719,54 @@ pub trait MatchMethods : TNode { } } + fn compute_restyle_damage(&self, + old_style: Option<&Arc<ComputedValues>>, + new_style: &Arc<ComputedValues>, + pseudo: Option<&PseudoElement>) + -> Self::ConcreteRestyleDamage + { + match self.existing_style_for_restyle_damage(old_style, pseudo) { + Some(ref source) => { + Self::ConcreteRestyleDamage::compute(source, + new_style) + } + None => { + // If there's no style source, two things can happen: + // + // 1. This is not an incremental restyle (old_style is none). + // In this case we can't do too much than sending + // rebuild_and_reflow. + // + // 2. This is an incremental restyle, but the old display value + // is none, so there's no effective way for Gecko to get the + // style source. In this case, we could return either + // RestyleDamage::empty(), in the case both displays are + // none, or rebuild_and_reflow, otherwise. The first case + // should be already handled when calling this function, so + // we can assert that the new display value is not none. + // + // Also, this can be a text node (in which case we don't + // care of watching the new display value). + // + // Unfortunately we can't strongly assert part of this, since + // we style some nodes that in Gecko never generate a frame, + // like children of replaced content. Arguably, we shouldn't be + // styling those here, but until we implement that we'll have to + // stick without the assertions. + debug_assert!(pseudo.is_none() || + new_style.get_box().clone_display() != display::T::none); + Self::ConcreteRestyleDamage::rebuild_and_reflow() + } + } + } + unsafe fn cascade_node<'a, Ctx>(&self, context: &Ctx, parent: Option<Self>, applicable_declarations: &ApplicableDeclarations) - where Ctx: StyleContext<'a> { + -> RestyleResult + where Ctx: StyleContext<'a> + { // Get our parent's style. This must be unsafe so that we don't touch the parent's // borrow flags. // @@ -735,72 +783,171 @@ pub trait MatchMethods : TNode { let mut applicable_declarations_cache = context.local_context().applicable_declarations_cache.borrow_mut(); - let damage; - if self.is_text_node() { + let (damage, restyle_result) = if self.is_text_node() { let mut data_ref = self.mutate_data().unwrap(); let mut data = &mut *data_ref; let cloned_parent_style = ComputedValues::style_for_child_text_node(parent_style.unwrap()); - { - let existing_style = - self.existing_style_for_restyle_damage(data.style.as_ref()); - damage = Self::ConcreteRestyleDamage::compute(existing_style, - &cloned_parent_style); - } + let damage = + self.compute_restyle_damage(data.style.as_ref(), &cloned_parent_style, None); data.style = Some(cloned_parent_style); - } else { - damage = { - let mut data_ref = self.mutate_data().unwrap(); - let mut data = &mut *data_ref; - let (mut damage, final_style) = self.cascade_node_pseudo_element( - context, - parent_style, - &applicable_declarations.normal, - data.style.as_mut(), - &mut applicable_declarations_cache, - applicable_declarations.normal_shareable, - true); - - data.style = Some(final_style); - - <Self::ConcreteElement as MatchAttr>::Impl::each_eagerly_cascaded_pseudo_element(|pseudo| { - let applicable_declarations_for_this_pseudo = - applicable_declarations.per_pseudo.get(&pseudo).unwrap(); - - if !applicable_declarations_for_this_pseudo.is_empty() { - // NB: Transitions and animations should only work for - // pseudo-elements ::before and ::after - let should_animate_properties = - <Self::ConcreteElement as MatchAttr>::Impl::pseudo_is_before_or_after(&pseudo); - let (new_damage, style) = self.cascade_node_pseudo_element( - context, - Some(data.style.as_ref().unwrap()), - &*applicable_declarations_for_this_pseudo, - data.per_pseudo.get_mut(&pseudo), - &mut applicable_declarations_cache, - false, - should_animate_properties); - data.per_pseudo.insert(pseudo, style); - - damage = damage | new_damage; - } - }); - damage - }; + (damage, RestyleResult::Continue) + } else { + let mut data_ref = self.mutate_data().unwrap(); + let mut data = &mut *data_ref; + let final_style = + self.cascade_node_pseudo_element(context, parent_style, + &applicable_declarations.normal, + data.style.as_mut(), + &mut applicable_declarations_cache, + applicable_declarations.normal_shareable, + /* should_animate = */ true); + + let (damage, restyle_result) = + self.compute_damage_and_cascade_pseudos(final_style, + data, + context, + applicable_declarations, + &mut applicable_declarations_cache); - // This method needs to borrow the data as mutable, so make sure data_ref goes out of - // scope first. self.set_can_be_fragmented(parent.map_or(false, |p| { p.can_be_fragmented() || parent_style.as_ref().unwrap().is_multicol() })); - } - // This method needs to borrow the data as mutable, so make sure data_ref goes out of - // scope first. + (damage, restyle_result) + }; + + + // This method needs to borrow the data as mutable, so make sure + // data_ref goes out of scope first. self.set_restyle_damage(damage); + + restyle_result + } + + fn compute_damage_and_cascade_pseudos<'a, Ctx>(&self, + final_style: Arc<ComputedValues>, + data: &mut PrivateStyleData, + context: &Ctx, + applicable_declarations: &ApplicableDeclarations, + mut applicable_declarations_cache: &mut ApplicableDeclarationsCache) + -> (Self::ConcreteRestyleDamage, RestyleResult) + where Ctx: StyleContext<'a> + { + // Here we optimise the case of the style changing but both the + // previous and the new styles having display: none. In this + // case, we can always optimize the traversal, regardless of the + // restyle hint. + let this_display = final_style.get_box().clone_display(); + if this_display == display::T::none { + let old_display = data.style.as_ref().map(|old_style| { + old_style.get_box().clone_display() + }); + + // If display passed from none to something, then we need to reflow, + // otherwise, we don't do anything. + let damage = match old_display { + Some(display) if display == this_display => { + Self::ConcreteRestyleDamage::empty() + } + _ => Self::ConcreteRestyleDamage::rebuild_and_reflow() + }; + + debug!("Short-circuiting traversal: {:?} {:?} {:?}", + this_display, old_display, damage); + + data.style = Some(final_style); + return (damage, RestyleResult::Stop); + } + + // Otherwise, we just compute the damage normally, and sum up the damage + // related to pseudo-elements. + let mut damage = + self.compute_restyle_damage(data.style.as_ref(), &final_style, None); + + data.style = Some(final_style); + + let data_per_pseudo = &mut data.per_pseudo; + let new_style = data.style.as_ref(); + + debug_assert!(new_style.is_some()); + + let rebuild_and_reflow = + Self::ConcreteRestyleDamage::rebuild_and_reflow(); + + <Self::ConcreteElement as MatchAttr>::Impl::each_eagerly_cascaded_pseudo_element(|pseudo| { + use std::collections::hash_map::Entry; + + let applicable_declarations_for_this_pseudo = + applicable_declarations.per_pseudo.get(&pseudo).unwrap(); + + let has_declarations = + !applicable_declarations_for_this_pseudo.is_empty(); + + // If there are declarations matching, we're going to need to + // recompute the style anyway, so do it now to simplify the logic + // below. + let pseudo_style_if_declarations = if has_declarations { + // NB: Transitions and animations should only work for + // pseudo-elements ::before and ::after + let should_animate_properties = + <Self::ConcreteElement as MatchAttr>::Impl::pseudo_is_before_or_after(&pseudo); + + Some(self.cascade_node_pseudo_element(context, + new_style, + &*applicable_declarations_for_this_pseudo, + data_per_pseudo.get_mut(&pseudo), + &mut applicable_declarations_cache, + /* shareable = */ false, + should_animate_properties)) + } else { + None + }; + + // Let's see what we had before. + match data_per_pseudo.entry(pseudo.clone()) { + Entry::Vacant(vacant_entry) => { + // If we had a vacant entry, and no rules that match, we're + // fine so far. + if !has_declarations { + return; + } + + // Otherwise, we need to insert the new computed styles, and + // generate a rebuild_and_reflow damage. + damage = damage | Self::ConcreteRestyleDamage::rebuild_and_reflow(); + vacant_entry.insert(pseudo_style_if_declarations.unwrap()); + } + Entry::Occupied(mut occupied_entry) => { + // If there was an existing style, and no declarations, we + // need to remove us from the map, and ensure we're + // reconstructing. + if !has_declarations { + damage = damage | Self::ConcreteRestyleDamage::rebuild_and_reflow(); + occupied_entry.remove(); + return; + } + + // If there's a new style, we need to diff it and add the + // damage, except if the damage was already + // rebuild_and_reflow, in which case we can avoid it. + if damage != rebuild_and_reflow { + damage = damage | + self.compute_restyle_damage(Some(occupied_entry.get()), + pseudo_style_if_declarations.as_ref().unwrap(), + Some(&pseudo)); + } + + // And now, of course, use the new style. + occupied_entry.insert(pseudo_style_if_declarations.unwrap()); + } + } + }); + + (damage, RestyleResult::Continue) } } diff --git a/components/style/parallel.rs b/components/style/parallel.rs index b82727657c3..532bde8540a 100644 --- a/components/style/parallel.rs +++ b/components/style/parallel.rs @@ -11,7 +11,7 @@ use dom::{OpaqueNode, TNode, UnsafeNode}; use std::mem; use std::sync::atomic::Ordering; -use traversal::DomTraversalContext; +use traversal::{RestyleResult, DomTraversalContext}; use workqueue::{WorkQueue, WorkUnit, WorkerProxy}; #[allow(dead_code)] @@ -67,37 +67,38 @@ fn top_down_dom<N, C>(unsafe_nodes: UnsafeNodeList, continue; } - // Perform the appropriate traversal. - context.process_preorder(node); - // Possibly enqueue the children. let mut children_to_process = 0isize; - 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()) + // Perform the appropriate traversal. + 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()) + } } } - // Reset the count of children. - { - let data = node.mutate_data().unwrap(); - data.parallel.children_to_process + // Reset the count of children if we need to do a bottom-up traversal + // after the top up. + if context.needs_postorder_traversal() { + node.mutate_data().unwrap() + .parallel.children_to_process .store(children_to_process, Ordering::Relaxed); - } - - // If there were no more children, start walking back up. - if children_to_process == 0 { - bottom_up_dom::<N, C>(unsafe_nodes.1, unsafe_node, proxy) + // If there were no more children, start walking back up. + if children_to_process == 0 { + bottom_up_dom::<N, C>(unsafe_nodes.1, unsafe_node, proxy) + } } } @@ -123,7 +124,9 @@ fn top_down_dom<N, C>(unsafe_nodes: UnsafeNodeList, fn bottom_up_dom<N, C>(root: OpaqueNode, unsafe_node: UnsafeNode, proxy: &mut WorkerProxy<C::SharedContext, UnsafeNodeList>) - where N: TNode, C: DomTraversalContext<N> { + where N: TNode, + C: DomTraversalContext<N> +{ let context = C::new(proxy.user_data(), root); // Get a real layout node. diff --git a/components/style/selector_impl.rs b/components/style/selector_impl.rs index 276b721ba7f..6c38fecec63 100644 --- a/components/style/selector_impl.rs +++ b/components/style/selector_impl.rs @@ -79,7 +79,8 @@ pub trait ElementExt: Element<Impl=TheSelectorImpl> { impl TheSelectorImpl { #[inline] pub fn each_eagerly_cascaded_pseudo_element<F>(mut fun: F) - where F: FnMut(<Self as SelectorImpl>::PseudoElement) { + where F: FnMut(PseudoElement) + { Self::each_pseudo_element(|pseudo| { if Self::pseudo_element_cascade_type(&pseudo).is_eager() { fun(pseudo) @@ -89,7 +90,8 @@ impl TheSelectorImpl { #[inline] pub fn each_precomputed_pseudo_element<F>(mut fun: F) - where F: FnMut(<Self as SelectorImpl>::PseudoElement) { + where F: FnMut(PseudoElement) + { Self::each_pseudo_element(|pseudo| { if Self::pseudo_element_cascade_type(&pseudo).is_precomputed() { fun(pseudo) diff --git a/components/style/sequential.rs b/components/style/sequential.rs index 2ae7a18048b..334eecc5959 100644 --- a/components/style/sequential.rs +++ b/components/style/sequential.rs @@ -5,7 +5,7 @@ //! Implements sequential traversal over the DOM tree. use dom::TNode; -use traversal::DomTraversalContext; +use traversal::{RestyleResult, DomTraversalContext}; pub fn traverse_dom<N, C>(root: N, shared: &C::SharedContext) @@ -17,16 +17,18 @@ pub fn traverse_dom<N, C>(root: N, C: DomTraversalContext<N> { debug_assert!(context.should_process(node)); - context.process_preorder(node); - - for kid in node.children() { - context.pre_process_child_hook(node, kid); - if context.should_process(kid) { - doit::<N, C>(context, kid); + 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::<N, C>(context, kid); + } } } - context.process_postorder(node); + if context.needs_postorder_traversal() { + context.process_postorder(node); + } } let context = C::new(shared, root.opaque()); diff --git a/components/style/traversal.rs b/components/style/traversal.rs index ded29719397..9bab63b2692 100644 --- a/components/style/traversal.rs +++ b/components/style/traversal.rs @@ -18,6 +18,16 @@ use values::HasViewportPercentage; /// detected by ticking a generation number every layout. pub type Generation = u32; +/// This enum tells us about whether we can stop restyling or not after styling +/// an element. +/// +/// So far this only happens where a display: none node is found. +pub enum RestyleResult { + Continue, + Stop, +} + + /// A pair of the bloom filter used for css selector matching, and the node to /// which it applies. This is used to efficiently do `Descendant` selector /// matches. Thanks to the bloom filter, we can avoid walking up the tree @@ -141,12 +151,23 @@ pub fn remove_from_bloom_filter<'a, N, C>(context: &C, root: OpaqueNode, node: N pub trait DomTraversalContext<N: TNode> { type SharedContext: Sync + 'static; + fn new<'a>(&'a Self::SharedContext, OpaqueNode) -> Self; + /// Process `node` on the way down, before its children have been processed. - fn process_preorder(&self, node: N); + fn process_preorder(&self, node: N) -> RestyleResult; + /// Process `node` on the way up, after its children have been processed. + /// + /// This is only executed if `needs_postorder_traversal` returns true. fn process_postorder(&self, node: N); + /// Boolean that specifies whether a bottom up traversal should be + /// performed. + /// + /// 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). /// @@ -172,14 +193,88 @@ pub trait DomTraversalContext<N: TNode> { } } +pub fn ensure_node_styled<'a, N, C>(node: N, + context: &'a C) + where N: TNode, + C: StyleContext<'a> +{ + let mut display_none = false; + ensure_node_styled_internal(node, 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, + C: StyleContext<'a> +{ + use properties::longhands::display::computed_value as display; + + // Ensure we have style data available. This must be done externally because + // there's no way to initialize the style data from the style system + // (because in Servo it's coupled with the layout data too). + // + // Ideally we'd have an initialize_data() or something similar but just for + // style data. + debug_assert!(node.borrow_data().is_some(), + "Need to initialize the data before calling ensure_node_styled"); + + // We need to go to the root and ensure their style is up to date. + // + // 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, + }; + + if let Some(parent) = parent { + ensure_node_styled_internal(parent, context, parents_had_display_none); + } + + // Common case: our style is already resolved and none of our ancestors had + // display: none. + // + // We only need to mark whether we have display none, and forget about it, + // our style is up to date. + if let Some(ref style) = node.borrow_data().unwrap().style { + if !*parents_had_display_none { + *parents_had_display_none = style.get_box().clone_display() == display::T::none; + return; + } + } + + // Otherwise, our style might be out of date. Time to do selector matching + // if appropriate and cascade the node. + // + // Note that we could add the bloom filter's complexity here, but that's + // 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; + + element.match_element(&**stylist, + None, + &mut applicable_declarations); + } + + unsafe { + node.cascade_node(context, parent, &applicable_declarations); + } +} + /// 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) + node: N) -> RestyleResult where N: TNode, - C: StyleContext<'a> { + C: StyleContext<'a> +{ // Get the parent node. let parent_opt = match node.parent_node() { Some(parent) if parent.is_element() => Some(parent), @@ -190,6 +285,7 @@ pub fn recalc_style_at<'a, N, C>(context: &'a C, 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() { // Remove existing CSS styles from nodes whose content has changed (e.g. text changed), // to force non-incremental reflow. @@ -239,9 +335,9 @@ pub fn recalc_style_at<'a, N, C>(context: &'a C, // Perform the CSS cascade. unsafe { - node.cascade_node(context, - parent_opt, - &applicable_declarations); + restyle_result = node.cascade_node(context, + parent_opt, + &applicable_declarations); } // Add ourselves to the LRU cache. @@ -249,7 +345,8 @@ pub fn recalc_style_at<'a, N, C>(context: &'a C, style_sharing_candidate_cache.insert_if_possible::<'ln, N>(&element); } } - StyleSharingResult::StyleWasShared(index, damage) => { + StyleSharingResult::StyleWasShared(index, damage, restyle_result_cascade) => { + restyle_result = restyle_result_cascade; style_sharing_candidate_cache.touch(index); node.set_restyle_damage(damage); } @@ -286,4 +383,10 @@ pub fn recalc_style_at<'a, N, C>(context: &'a C, } } } + + if nonincremental_layout { + RestyleResult::Continue + } else { + restyle_result + } } diff --git a/ports/geckolib/traversal.rs b/ports/geckolib/traversal.rs index 87b63e83e15..50aa40dbd79 100644 --- a/ports/geckolib/traversal.rs +++ b/ports/geckolib/traversal.rs @@ -6,6 +6,7 @@ use context::StandaloneStyleContext; use std::mem; use style::context::SharedStyleContext; use style::dom::OpaqueNode; +use style::traversal::RestyleResult; use style::traversal::{DomTraversalContext, recalc_style_at}; use wrapper::GeckoNode; @@ -27,13 +28,18 @@ impl<'lc, 'ln> DomTraversalContext<GeckoNode<'ln>> for RecalcStyleOnly<'lc> { } } - fn process_preorder(&self, node: GeckoNode<'ln>) { + fn process_preorder(&self, node: GeckoNode<'ln>) -> RestyleResult { // FIXME(pcwalton): Stop allocating here. Ideally this should just be done by the HTML // parser. node.initialize_data(); - recalc_style_at(&self.context, self.root, node); + recalc_style_at(&self.context, self.root, node) } - fn process_postorder(&self, _: GeckoNode<'ln>) {} + fn process_postorder(&self, _: GeckoNode<'ln>) { + unreachable!(); + } + + /// We don't use the post-order traversal for anything. + fn needs_postorder_traversal(&self) -> bool { false } } diff --git a/ports/geckolib/wrapper.rs b/ports/geckolib/wrapper.rs index b1299ff7bc5..6b82cc8a226 100644 --- a/ports/geckolib/wrapper.rs +++ b/ports/geckolib/wrapper.rs @@ -44,7 +44,7 @@ use style::dom::{TDocument, TElement, TNode, TRestyleDamage, UnsafeNode}; use style::element_state::ElementState; use style::error_reporting::StdoutErrorReporter; use style::gecko_glue::ArcHelpers; -use style::gecko_selector_impl::{GeckoSelectorImpl, NonTSPseudoClass}; +use style::gecko_selector_impl::{GeckoSelectorImpl, NonTSPseudoClass, PseudoElement}; use style::parser::ParserContextExtraData; use style::properties::{ComputedValues, parse_style_attribute}; use style::properties::{PropertyDeclaration, PropertyDeclarationBlock}; @@ -95,18 +95,21 @@ impl<'ln> GeckoNode<'ln> { } } -#[derive(Clone, Copy)] +#[derive(Clone, Copy, Debug, PartialEq)] pub struct GeckoRestyleDamage(nsChangeHint); impl TRestyleDamage for GeckoRestyleDamage { type PreExistingComputedValues = nsStyleContext; - fn compute(source: Option<&nsStyleContext>, + + fn empty() -> Self { + use std::mem; + GeckoRestyleDamage(unsafe { mem::transmute(0u32) }) + } + + fn compute(source: &nsStyleContext, new_style: &Arc<ComputedValues>) -> Self { type Helpers = ArcHelpers<ServoComputedValues, ComputedValues>; - let context = match source { - Some(ctx) => ctx as *const nsStyleContext as *mut nsStyleContext, - None => return Self::rebuild_and_reflow(), - }; + let context = source as *const nsStyleContext as *mut nsStyleContext; Helpers::borrow(new_style, |new_style| { let hint = unsafe { Gecko_CalcStyleDifference(context, new_style) }; @@ -194,10 +197,9 @@ impl<'ln> TNode for GeckoNode<'ln> { unimplemented!() } - fn has_changed(&self) -> bool { - // FIXME(bholley) - Implement this to allow incremental reflows! - true - } + // NOTE: This is not relevant for Gecko, since we get explicit restyle hints + // when a content has changed. + fn has_changed(&self) -> bool { false } unsafe fn set_changed(&self, _value: bool) { unimplemented!() @@ -310,13 +312,21 @@ impl<'ln> TNode for GeckoNode<'ln> { } fn existing_style_for_restyle_damage<'a>(&'a self, - current_cv: Option<&'a Arc<ComputedValues>>) + current_cv: Option<&'a Arc<ComputedValues>>, + 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; } + if pseudo.is_some() { + // FIXME(emilio): This makes us reconstruct frame for pseudos every + // restyle, add a FFI call to get the style context associated with + // a PE. + return None; + } + unsafe { let context_ptr = Gecko_GetStyleContext(self.node); context_ptr.as_ref() diff --git a/tests/wpt/metadata-css/css-transitions-1_dev/html/hidden-container-001.htm.ini b/tests/wpt/metadata-css/css-transitions-1_dev/html/hidden-container-001.htm.ini deleted file mode 100644 index 27e2c118109..00000000000 --- a/tests/wpt/metadata-css/css-transitions-1_dev/html/hidden-container-001.htm.ini +++ /dev/null @@ -1,5 +0,0 @@ -[hidden-container-001.htm] - type: testharness - [transition within display:none / values] - expected: FAIL - diff --git a/tests/wpt/mozilla/meta/MANIFEST.json b/tests/wpt/mozilla/meta/MANIFEST.json index 4291bf726aa..5aa73e95581 100644 --- a/tests/wpt/mozilla/meta/MANIFEST.json +++ b/tests/wpt/mozilla/meta/MANIFEST.json @@ -4420,6 +4420,18 @@ "url": "/_mozilla/css/pseudo_element_a.html" } ], + "css/pseudo_element_restyle_no_rules.html": [ + { + "path": "css/pseudo_element_restyle_no_rules.html", + "references": [ + [ + "/_mozilla/css/pseudo_element_restyle_no_rules_ref.html", + "==" + ] + ], + "url": "/_mozilla/css/pseudo_element_restyle_no_rules.html" + } + ], "css/pseudo_inherit.html": [ { "path": "css/pseudo_inherit.html", @@ -13586,6 +13598,18 @@ "url": "/_mozilla/css/pseudo_element_a.html" } ], + "css/pseudo_element_restyle_no_rules.html": [ + { + "path": "css/pseudo_element_restyle_no_rules.html", + "references": [ + [ + "/_mozilla/css/pseudo_element_restyle_no_rules_ref.html", + "==" + ] + ], + "url": "/_mozilla/css/pseudo_element_restyle_no_rules.html" + } + ], "css/pseudo_inherit.html": [ { "path": "css/pseudo_inherit.html", diff --git a/tests/wpt/mozilla/tests/css/pseudo_element_restyle_no_rules.html b/tests/wpt/mozilla/tests/css/pseudo_element_restyle_no_rules.html new file mode 100644 index 00000000000..f309fab8312 --- /dev/null +++ b/tests/wpt/mozilla/tests/css/pseudo_element_restyle_no_rules.html @@ -0,0 +1,20 @@ +<!doctype html> +<meta charset="utf-8"> +<title>Pseudo-Element is restyled correctly when matching rules become empty</title> +<link rel="match" href="pseudo_element_restyle_no_rules_ref.html"> +<style> + p.whatever::before { content: " hey "; } +</style> +<p> + test. + +<script> +var p = document.querySelector('p'); +var toggle = function() { + p.className = p.className == "whatever" ? "" : "whatever"; +}; +toggle(); +// Ensure a restyle happens. +p.offsetTop; +toggle() +</script> diff --git a/tests/wpt/mozilla/tests/css/pseudo_element_restyle_no_rules_ref.html b/tests/wpt/mozilla/tests/css/pseudo_element_restyle_no_rules_ref.html new file mode 100644 index 00000000000..f09f5d074e8 --- /dev/null +++ b/tests/wpt/mozilla/tests/css/pseudo_element_restyle_no_rules_ref.html @@ -0,0 +1,5 @@ +<!doctype html> +<meta charset="utf-8"> +<title>CSS Test reference.</title> +<p> + test. |