From ddbc016f51e169369a6c25d68b453dc299cc8677 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Sat, 8 Oct 2016 23:46:37 -0700 Subject: [PATCH 1/2] Refactor cascade to limit and clarify the mutability of the old style. The current semantics make it very difficult to figure out when the existing style is mutated and what code depends on it. This is problematic for the new incremental restyle architecture, where the cascade logic no longer has direct access to |data|. --- components/style/matching.rs | 165 ++++++++++++++++++----------------- 1 file changed, 83 insertions(+), 82 deletions(-) diff --git a/components/style/matching.rs b/components/style/matching.rs index 24334512cf0..18d0623c0e3 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -486,6 +486,14 @@ pub enum StyleSharingResult { StyleWasShared(usize, ConcreteRestyleDamage, RestyleResult), } +// Callers need to pass several boolean flags to cascade_node_pseudo_element. +// We encapsulate them in this struct to avoid mixing them up. +struct CascadeBooleans { + shareable: bool, + cacheable: bool, + animate: bool, +} + trait PrivateMatchMethods: TNode { /// Actually cascades style for a node or a pseudo-element of a node. /// @@ -494,27 +502,24 @@ trait PrivateMatchMethods: TNode { fn cascade_node_pseudo_element<'a, Ctx>(&self, context: &Ctx, parent_style: Option<&Arc>, + old_style: Option<&Arc>, applicable_declarations: &[ApplicableDeclarationBlock], - mut old_style: Option<&mut Arc>, applicable_declarations_cache: &mut ApplicableDeclarationsCache, - shareable: bool, - animate_properties: bool) + booleans: CascadeBooleans) -> Arc where Ctx: StyleContext<'a> { + let mut cacheable = booleans.cacheable; + let shared_context = context.shared_context(); + // Don’t cache applicable declarations for elements with a style attribute. // Since the style attribute contributes to that set, no other element would have the same set // 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 mut cacheable = !has_style_attribute; - let shared_context = context.shared_context(); - if animate_properties { - cacheable = !self.update_animations_for_cascade(shared_context, - &mut old_style) && cacheable; - } + cacheable = cacheable && !has_style_attribute; let mut cascade_info = CascadeInfo::new(); let (this_style, is_cacheable) = match parent_style { @@ -527,7 +532,7 @@ trait PrivateMatchMethods: TNode { cascade(shared_context.viewport_size, applicable_declarations, - shareable, + booleans.shareable, Some(&***parent_style), cached_computed_values, Some(&mut cascade_info), @@ -536,7 +541,7 @@ trait PrivateMatchMethods: TNode { None => { cascade(shared_context.viewport_size, applicable_declarations, - shareable, + booleans.shareable, None, None, Some(&mut cascade_info), @@ -549,7 +554,7 @@ trait PrivateMatchMethods: TNode { let mut this_style = Arc::new(this_style); - if animate_properties { + if booleans.animate { let new_animations_sender = &context.local_context().new_animations_sender; let this_opaque = self.opaque(); // Trigger any present animations if necessary. @@ -585,13 +590,7 @@ trait PrivateMatchMethods: TNode { fn update_animations_for_cascade(&self, context: &SharedStyleContext, - style: &mut Option<&mut Arc>) - -> bool { - let style = match *style { - None => return false, - Some(ref mut style) => style, - }; - + style: &mut Arc) -> bool { // Finish any expired transitions. let this_opaque = self.opaque(); let had_animations_to_expire = @@ -909,18 +908,33 @@ pub trait MatchMethods : TNode { let (damage, restyle_result) = { 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, + + // Compute the parameters for the cascade. + let mut old_style = data.style.clone(); + let cacheable = match old_style { + None => true, + Some(ref mut old) => { + // Update animations before the cascade. This may modify + // the value of old_style. + !self.update_animations_for_cascade(context.shared_context(), old) + }, + }; + let shareable = applicable_declarations.normal_shareable; + + + let new_style = + self.cascade_node_pseudo_element(context, parent_style, old_style.as_ref(), &applicable_declarations.normal, - data.style.as_mut(), &mut applicable_declarations_cache, - applicable_declarations.normal_shareable, - /* should_animate = */ true); + CascadeBooleans { + shareable: shareable, + cacheable: cacheable, + animate: true, + }); let (damage, restyle_result) = - self.compute_damage_and_cascade_pseudos(final_style, - data, - context, + self.compute_damage_and_cascade_pseudos(new_style, old_style.as_ref(), + data, context, applicable_declarations, &mut applicable_declarations_cache); @@ -942,6 +956,7 @@ pub trait MatchMethods : TNode { fn compute_damage_and_cascade_pseudos<'a, Ctx>(&self, final_style: Arc, + old_style: Option<&Arc>, data: &mut PersistentStyleData, context: &Ctx, applicable_declarations: &ApplicableDeclarations, @@ -955,7 +970,7 @@ pub trait MatchMethods : TNode { // 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| { + let old_display = old_style.map(|old_style| { old_style.get_box().clone_display() }); @@ -978,7 +993,7 @@ pub trait MatchMethods : TNode { // 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); + self.compute_restyle_damage(old_style, &final_style, None); data.style = Some(final_style); @@ -989,72 +1004,58 @@ pub trait MatchMethods : TNode { let rebuild_and_reflow = Self::ConcreteRestyleDamage::rebuild_and_reflow(); + let no_damage = Self::ConcreteRestyleDamage::empty(); ::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 = - ::Impl::pseudo_is_before_or_after(&pseudo); + // The old entry will be replaced. Remove it from the map but keep + // it for analysis. + let mut old_pseudo_style = data_per_pseudo.remove(&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 - }; + if has_declarations { + // We have declarations, so we need to cascade. Compute parameters. + 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. + !self.update_animations_for_cascade(context.shared_context(), + old_pseudo_style.as_mut().unwrap()) + } else { + true + }; - // 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; - } + let new_pseudo_style = + self.cascade_node_pseudo_element(context, new_style, old_pseudo_style.as_ref(), + &*applicable_declarations_for_this_pseudo, + &mut applicable_declarations_cache, + CascadeBooleans { + shareable: false, + cacheable: cacheable, + animate: animate, + }); - // 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()); + // Compute restyle damage unless we've already maxed it out. + if damage != rebuild_and_reflow { + damage = damage | match old_pseudo_style { + None => rebuild_and_reflow, + Some(ref old) => self.compute_restyle_damage(Some(old), &new_pseudo_style, + Some(&pseudo)), + }; } - 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()); + // Insert the new entry into the map. + let existing = data_per_pseudo.insert(pseudo, new_pseudo_style); + debug_assert!(existing.is_none()); + } else { + damage = damage | match old_pseudo_style { + Some(_) => rebuild_and_reflow, + None => no_damage, } } }); From bfbbef6ecdfd8bdd0a58bd1a0c7ed95c182b98a7 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Sun, 9 Oct 2016 13:37:57 -0700 Subject: [PATCH 2/2] Remove borrow_data and mutate_data from TNode. The new restyle architecture doesn't store these things in consistent places, so we need a more abstract API. --- components/layout/query.rs | 2 +- components/layout/wrapper.rs | 2 +- components/script/layout_wrapper.rs | 30 +++++++++++--- components/style/data.rs | 5 ++- components/style/dom.rs | 32 +++++++++------ components/style/gecko/wrapper.rs | 38 ++++++++++++++---- components/style/matching.rs | 62 +++++++++++------------------ components/style/traversal.rs | 21 +++++----- ports/geckolib/glue.rs | 7 +--- 9 files changed, 115 insertions(+), 84 deletions(-) diff --git a/components/layout/query.rs b/components/layout/query.rs index c92af15582f..822377ef86f 100644 --- a/components/layout/query.rs +++ b/components/layout/query.rs @@ -627,7 +627,7 @@ pub fn process_node_scroll_area_request< N: LayoutNode>(requested_node: N, layou fn ensure_node_data_initialized(node: &N) { let mut cur = Some(node.clone()); while let Some(current) = cur { - if current.borrow_data().is_some() { + if current.borrow_layout_data().is_some() { break; } diff --git a/components/layout/wrapper.rs b/components/layout/wrapper.rs index 28d1196221b..2c8b3a175c3 100644 --- a/components/layout/wrapper.rs +++ b/components/layout/wrapper.rs @@ -68,7 +68,7 @@ impl LayoutNodeLayoutData for T { } fn initialize_data(self) { - if self.borrow_data().is_none() { + if self.borrow_layout_data().is_none() { let ptr: NonOpaqueStyleAndLayoutData = Box::into_raw(box AtomicRefCell::new(PersistentLayoutData::new())); let opaque = OpaqueStyleAndLayoutData { diff --git a/components/script/layout_wrapper.rs b/components/script/layout_wrapper.rs index efb3ca698ed..f2947bedc93 100644 --- a/components/script/layout_wrapper.rs +++ b/components/script/layout_wrapper.rs @@ -60,7 +60,7 @@ use style::atomic_refcell::{AtomicRef, AtomicRefCell, AtomicRefMut}; use style::attr::AttrValue; use style::computed_values::display; use style::context::SharedStyleContext; -use style::data::PersistentStyleData; +use style::data::{PersistentStyleData, PseudoStyles}; use style::dom::{LayoutIterator, NodeInfo, OpaqueNode, PresentationalHintsSynthetizer, TDocument, TElement, TNode}; use style::dom::UnsafeNode; use style::element_state::*; @@ -107,6 +107,14 @@ impl<'ln> ServoLayoutNode<'ln> { } } + pub fn borrow_data(&self) -> Option> { + self.get_style_data().map(|d| d.borrow()) + } + + pub fn mutate_data(&self) -> Option> { + self.get_style_data().map(|d| d.borrow_mut()) + } + fn script_type_id(&self) -> NodeTypeId { unsafe { self.node.type_id_for_layout() @@ -234,12 +242,24 @@ impl<'ln> TNode for ServoLayoutNode<'ln> { old_value - 1 } - fn borrow_data(&self) -> Option> { - self.get_style_data().map(|d| d.borrow()) + fn get_existing_style(&self) -> Option> { + self.borrow_data().and_then(|x| x.style.clone()) } - fn mutate_data(&self) -> Option> { - self.get_style_data().map(|d| d.borrow_mut()) + fn set_style(&self, style: Option>) { + self.mutate_data().unwrap().style = style; + } + + fn take_pseudo_styles(&self) -> PseudoStyles { + use std::mem; + let mut tmp = PseudoStyles::default(); + mem::swap(&mut tmp, &mut self.mutate_data().unwrap().per_pseudo); + tmp + } + + fn set_pseudo_styles(&self, styles: PseudoStyles) { + debug_assert!(self.borrow_data().unwrap().per_pseudo.is_empty()); + self.mutate_data().unwrap().per_pseudo = styles; } fn restyle_damage(self) -> RestyleDamage { diff --git a/components/style/data.rs b/components/style/data.rs index 1fc24808d03..c0b07ac340f 100644 --- a/components/style/data.rs +++ b/components/style/data.rs @@ -10,13 +10,14 @@ use std::collections::HashMap; use std::hash::BuildHasherDefault; use std::sync::Arc; +pub type PseudoStyles = HashMap, + BuildHasherDefault<::fnv::FnvHasher>>; pub struct PersistentStyleData { /// The results of CSS styling for this node. pub style: Option>, /// The results of CSS styling for each pseudo-element (if any). - pub per_pseudo: HashMap, - BuildHasherDefault<::fnv::FnvHasher>>, + pub per_pseudo: PseudoStyles, } impl PersistentStyleData { diff --git a/components/style/dom.rs b/components/style/dom.rs index 4e72e5745b6..5b2bbcb135e 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -6,8 +6,7 @@ #![allow(unsafe_code)] -use atomic_refcell::{AtomicRef, AtomicRefMut}; -use data::PersistentStyleData; +use data::PseudoStyles; use element_state::ElementState; use parking_lot::RwLock; use properties::{ComputedValues, PropertyDeclarationBlock}; @@ -147,13 +146,25 @@ pub trait TNode : Sized + Copy + Clone + NodeInfo { /// traversal. Returns the number of children left to process. fn did_process_child(&self) -> isize; - /// Borrows the style data immutably. Fails on a conflicting borrow. - #[inline(always)] - fn borrow_data(&self) -> Option>; + /// Returns the computed style values corresponding to the existing style + /// for this node, if any. + /// + /// This returns an cloned Arc (rather than a borrow) to abstract over the + /// multitude of ways these values may be stored under the hood. By + /// returning an enum with various OwningRef/OwningHandle entries, we could + /// avoid the refcounting traffic here, but it's probably not worth the + /// complexity. + fn get_existing_style(&self) -> Option>; - /// Borrows the style data mutably. Fails on a conflicting borrow. - #[inline(always)] - fn mutate_data(&self) -> Option>; + /// Sets the computed style for this node. + fn set_style(&self, style: Option>); + + /// Transfers ownership of the existing pseudo styles, if any, to the + /// caller. The stored pseudo styles are replaced with an empty map. + fn take_pseudo_styles(&self) -> PseudoStyles; + + /// Sets the pseudo styles on the element, replacing any existing styles. + fn set_pseudo_styles(&self, styles: PseudoStyles); /// Get the description of how to account for recent style changes. fn restyle_damage(self) -> Self::ConcreteRestyleDamage; @@ -171,11 +182,6 @@ pub trait TNode : Sized + Copy + Clone + NodeInfo { fn next_sibling(&self) -> Option; - /// Removes the style from this node. - fn unstyle(self) { - self.mutate_data().unwrap().style = None; - } - /// 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. diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index 407ff4694e4..eb5251a23f4 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::PersistentStyleData; +use data::{PersistentStyleData, PseudoStyles}; use dom::{LayoutIterator, NodeInfo, TDocument, TElement, TNode, TRestyleDamage, UnsafeNode}; use dom::{OpaqueNode, PresentationalHintsSynthetizer}; use element_state::ElementState; @@ -149,6 +149,20 @@ impl<'ln> GeckoNode<'ln> { self.0.mServoData.set(ptr::null_mut()); } } + + pub fn get_pseudo_style(&self, pseudo: &PseudoElement) -> Option> { + self.borrow_data().and_then(|data| data.per_pseudo.get(pseudo).map(|c| c.clone())) + } + + #[inline(always)] + fn borrow_data(&self) -> Option> { + self.get_node_data().as_ref().map(|d| d.0.borrow()) + } + + #[inline(always)] + fn mutate_data(&self) -> Option> { + self.get_node_data().as_ref().map(|d| d.0.borrow_mut()) + } } #[derive(Clone, Copy, Debug, PartialEq)] @@ -319,14 +333,24 @@ impl<'ln> TNode for GeckoNode<'ln> { panic!("Atomic child count not implemented in Gecko"); } - #[inline(always)] - fn borrow_data(&self) -> Option> { - self.get_node_data().as_ref().map(|d| d.0.borrow()) + fn get_existing_style(&self) -> Option> { + self.borrow_data().and_then(|x| x.style.clone()) } - #[inline(always)] - fn mutate_data(&self) -> Option> { - self.get_node_data().as_ref().map(|d| d.0.borrow_mut()) + fn set_style(&self, style: Option>) { + self.mutate_data().unwrap().style = style; + } + + fn take_pseudo_styles(&self) -> PseudoStyles { + use std::mem; + let mut tmp = PseudoStyles::default(); + mem::swap(&mut tmp, &mut self.mutate_data().unwrap().per_pseudo); + tmp + } + + fn set_pseudo_styles(&self, styles: PseudoStyles) { + debug_assert!(self.borrow_data().unwrap().per_pseudo.is_empty()); + self.mutate_data().unwrap().per_pseudo = styles; } fn restyle_damage(self) -> Self::ConcreteRestyleDamage { diff --git a/components/style/matching.rs b/components/style/matching.rs index 18d0623c0e3..18e45a32ece 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -11,7 +11,6 @@ use arc_ptr_eq; use cache::{LRUCache, SimpleHashCache}; use cascade_info::CascadeInfo; use context::{SharedStyleContext, StyleContext}; -use data::PersistentStyleData; use dom::{NodeInfo, TElement, TNode, TRestyleDamage, UnsafeNode}; use properties::{ComputedValues, cascade}; use properties::longhands::display::computed_value as display; @@ -441,8 +440,7 @@ impl StyleSharingCandidateCache { } let node = element.as_node(); - let data = node.borrow_data().unwrap(); - let style = data.style.as_ref().unwrap(); + let style = node.get_existing_style().unwrap(); let box_style = style.get_box(); if box_style.transition_property_count() > 0 { @@ -723,14 +721,13 @@ pub trait ElementMatchMethods : TElement { Ok(shared_style) => { // Yay, cache hit. Share the style. let node = self.as_node(); - let style = &mut node.mutate_data().unwrap().style; // TODO: add the display: none optimisation here too! Even // better, factor it out/make it a bit more generic so Gecko // 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((*style).as_ref(), None) { + match node.existing_style_for_restyle_damage(node.get_existing_style().as_ref(), None) { Some(ref source) => { <::ConcreteNode as TNode> ::ConcreteRestyleDamage::compute(source, &shared_style) @@ -747,7 +744,7 @@ pub trait ElementMatchMethods : TElement { RestyleResult::Continue }; - *style = Some(shared_style); + node.set_style(Some(shared_style)); return StyleSharingResult::StyleWasShared(i, damage, restyle_result) } @@ -881,8 +878,7 @@ pub trait MatchMethods : TNode { where Ctx: StyleContext<'a> { // Get our parent's style. - let parent_node_data = parent.as_ref().and_then(|x| x.borrow_data()); - let parent_style = parent_node_data.as_ref().map(|x| x.style.as_ref().unwrap()); + let parent_style = parent.as_ref().map(|x| x.get_existing_style().unwrap()); // 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 @@ -893,11 +889,9 @@ pub trait MatchMethods : TNode { // In Servo, this is also true, since text nodes generate UnscannedText // fragments, which aren't repairable by incremental layout. 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 cloned_parent_style = ComputedValues::style_for_child_text_node(parent_style.as_ref().unwrap()); - data.style = Some(cloned_parent_style); + self.set_style(Some(cloned_parent_style)); return RestyleResult::Continue; } @@ -906,11 +900,8 @@ pub trait MatchMethods : TNode { context.local_context().applicable_declarations_cache.borrow_mut(); let (damage, restyle_result) = { - let mut data_ref = self.mutate_data().unwrap(); - let mut data = &mut *data_ref; - // Compute the parameters for the cascade. - let mut old_style = data.style.clone(); + let mut old_style = self.get_existing_style(); let cacheable = match old_style { None => true, Some(ref mut old) => { @@ -923,7 +914,9 @@ pub trait MatchMethods : TNode { let new_style = - self.cascade_node_pseudo_element(context, parent_style, old_style.as_ref(), + self.cascade_node_pseudo_element(context, + parent_style.as_ref(), + old_style.as_ref(), &applicable_declarations.normal, &mut applicable_declarations_cache, CascadeBooleans { @@ -933,11 +926,12 @@ pub trait MatchMethods : TNode { }); let (damage, restyle_result) = - self.compute_damage_and_cascade_pseudos(new_style, old_style.as_ref(), - data, context, - applicable_declarations, + self.compute_damage_and_cascade_pseudos(&new_style, old_style.as_ref(), + context, applicable_declarations, &mut applicable_declarations_cache); + self.set_style(Some(new_style)); + self.set_can_be_fragmented(parent.map_or(false, |p| { p.can_be_fragmented() || parent_style.as_ref().unwrap().is_multicol() @@ -946,18 +940,14 @@ pub trait MatchMethods : TNode { (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, + new_style: &Arc, old_style: Option<&Arc>, - data: &mut PersistentStyleData, context: &Ctx, applicable_declarations: &ApplicableDeclarations, mut applicable_declarations_cache: &mut ApplicableDeclarationsCache) @@ -968,7 +958,7 @@ pub trait MatchMethods : TNode { // 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(); + let this_display = new_style.get_box().clone_display(); if this_display == display::T::none { let old_display = old_style.map(|old_style| { old_style.get_box().clone_display() @@ -986,26 +976,19 @@ pub trait MatchMethods : TNode { 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(old_style, &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()); + self.compute_restyle_damage(old_style, new_style, None); let rebuild_and_reflow = Self::ConcreteRestyleDamage::rebuild_and_reflow(); let no_damage = Self::ConcreteRestyleDamage::empty(); + let mut pseudo_styles = self.take_pseudo_styles(); ::Impl::each_eagerly_cascaded_pseudo_element(|pseudo| { let applicable_declarations_for_this_pseudo = applicable_declarations.per_pseudo.get(&pseudo).unwrap(); @@ -1015,7 +998,7 @@ pub trait MatchMethods : TNode { // The old entry will be replaced. Remove it from the map but keep // it for analysis. - let mut old_pseudo_style = data_per_pseudo.remove(&pseudo); + let mut old_pseudo_style = pseudo_styles.remove(&pseudo); if has_declarations { // We have declarations, so we need to cascade. Compute parameters. @@ -1031,7 +1014,8 @@ pub trait MatchMethods : TNode { }; let new_pseudo_style = - self.cascade_node_pseudo_element(context, new_style, old_pseudo_style.as_ref(), + self.cascade_node_pseudo_element(context, Some(new_style), + old_pseudo_style.as_ref(), &*applicable_declarations_for_this_pseudo, &mut applicable_declarations_cache, CascadeBooleans { @@ -1050,7 +1034,7 @@ pub trait MatchMethods : TNode { } // Insert the new entry into the map. - let existing = data_per_pseudo.insert(pseudo, new_pseudo_style); + let existing = pseudo_styles.insert(pseudo, new_pseudo_style); debug_assert!(existing.is_none()); } else { damage = damage | match old_pseudo_style { @@ -1060,6 +1044,8 @@ pub trait MatchMethods : TNode { } }); + self.set_pseudo_styles(pseudo_styles); + (damage, RestyleResult::Continue) } } diff --git a/components/style/traversal.rs b/components/style/traversal.rs index 5f041919596..ce248dbb410 100644 --- a/components/style/traversal.rs +++ b/components/style/traversal.rs @@ -229,14 +229,7 @@ fn ensure_node_styled_internal<'a, N, C>(node: N, { 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"); + // NB: The node data must be initialized here. // We need to go to the root and ensure their style is up to date. // @@ -257,7 +250,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. - if let Some(ref style) = node.borrow_data().unwrap().style { + if let Some(ref style) = node.get_existing_style() { if !*parents_had_display_none { *parents_had_display_none = style.get_box().clone_display() == display::T::none; return; @@ -308,7 +301,7 @@ pub fn recalc_style_at<'a, N, C>(context: &'a C, // Remove existing CSS styles from nodes whose content has changed (e.g. text changed), // to force non-incremental reflow. if node.has_changed() { - node.unstyle(); + node.set_style(None); } // Check to see whether we can share a style with someone. @@ -385,11 +378,15 @@ pub fn recalc_style_at<'a, N, C>(context: &'a C, } } else { // Finish any expired transitions. - animation::complete_expired_transitions( + let mut existing_style = node.get_existing_style().unwrap(); + let had_animations_to_expire = animation::complete_expired_transitions( node.opaque(), - node.mutate_data().unwrap().style.as_mut().unwrap(), + &mut existing_style, context.shared_context() ); + if had_animations_to_expire { + node.set_style(Some(existing_style)); + } } let unsafe_layout_node = node.to_unsafe(); diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index f7237b59561..b4bebcf7c59 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -260,7 +260,7 @@ pub extern "C" fn Servo_StyleSheet_Release(sheet: RawServoStyleSheetBorrowed) -> pub extern "C" fn Servo_ComputedValues_Get(node: RawGeckoNodeBorrowed) -> ServoComputedValuesStrong { let node = GeckoNode(node); - let arc_cv = match node.borrow_data().map_or(None, |data| data.style.clone()) { + let arc_cv = match node.get_existing_style() { Some(style) => style, None => { // FIXME(bholley): This case subverts the intended semantics of this @@ -322,10 +322,7 @@ pub extern "C" fn Servo_ComputedValues_GetForPseudoElement(parent_style: ServoCo match GeckoSelectorImpl::pseudo_element_cascade_type(&pseudo) { PseudoElementCascadeType::Eager => { let node = element.as_node(); - let maybe_computed = node.borrow_data() - .and_then(|data| { - data.per_pseudo.get(&pseudo).map(|c| c.clone()) - }); + let maybe_computed = node.get_pseudo_style(&pseudo); maybe_computed.map_or_else(parent_or_null, FFIArcHelpers::into_strong) } PseudoElementCascadeType::Lazy => {