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|.
This commit is contained in:
Bobby Holley 2016-10-08 23:46:37 -07:00
parent b091ada480
commit ddbc016f51

View file

@ -486,6 +486,14 @@ pub enum StyleSharingResult<ConcreteRestyleDamage: TRestyleDamage> {
StyleWasShared(usize, ConcreteRestyleDamage, RestyleResult), 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 { trait PrivateMatchMethods: TNode {
/// Actually cascades style for a node or a pseudo-element of a node. /// 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, fn cascade_node_pseudo_element<'a, Ctx>(&self,
context: &Ctx, context: &Ctx,
parent_style: Option<&Arc<ComputedValues>>, parent_style: Option<&Arc<ComputedValues>>,
old_style: Option<&Arc<ComputedValues>>,
applicable_declarations: &[ApplicableDeclarationBlock], applicable_declarations: &[ApplicableDeclarationBlock],
mut old_style: Option<&mut Arc<ComputedValues>>,
applicable_declarations_cache: applicable_declarations_cache:
&mut ApplicableDeclarationsCache, &mut ApplicableDeclarationsCache,
shareable: bool, booleans: CascadeBooleans)
animate_properties: bool)
-> Arc<ComputedValues> -> Arc<ComputedValues>
where Ctx: StyleContext<'a> where Ctx: StyleContext<'a>
{ {
let mut cacheable = booleans.cacheable;
let shared_context = context.shared_context();
// Dont cache applicable declarations for elements with a style attribute. // Dont 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 // Since the style attribute contributes to that set, no other element would have the same set
// and the cache would not be effective anyway. // and the cache would not be effective anyway.
// This also works around the test failures at // This also works around the test failures at
// https://github.com/servo/servo/pull/13459#issuecomment-250717584 // https://github.com/servo/servo/pull/13459#issuecomment-250717584
let has_style_attribute = self.as_element().map_or(false, |e| e.style_attribute().is_some()); let has_style_attribute = self.as_element().map_or(false, |e| e.style_attribute().is_some());
let mut cacheable = !has_style_attribute; cacheable = 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;
}
let mut cascade_info = CascadeInfo::new(); let mut cascade_info = CascadeInfo::new();
let (this_style, is_cacheable) = match parent_style { let (this_style, is_cacheable) = match parent_style {
@ -527,7 +532,7 @@ trait PrivateMatchMethods: TNode {
cascade(shared_context.viewport_size, cascade(shared_context.viewport_size,
applicable_declarations, applicable_declarations,
shareable, booleans.shareable,
Some(&***parent_style), Some(&***parent_style),
cached_computed_values, cached_computed_values,
Some(&mut cascade_info), Some(&mut cascade_info),
@ -536,7 +541,7 @@ trait PrivateMatchMethods: TNode {
None => { None => {
cascade(shared_context.viewport_size, cascade(shared_context.viewport_size,
applicable_declarations, applicable_declarations,
shareable, booleans.shareable,
None, None,
None, None,
Some(&mut cascade_info), Some(&mut cascade_info),
@ -549,7 +554,7 @@ trait PrivateMatchMethods: TNode {
let mut this_style = Arc::new(this_style); 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 new_animations_sender = &context.local_context().new_animations_sender;
let this_opaque = self.opaque(); let this_opaque = self.opaque();
// Trigger any present animations if necessary. // Trigger any present animations if necessary.
@ -585,13 +590,7 @@ trait PrivateMatchMethods: TNode {
fn update_animations_for_cascade(&self, fn update_animations_for_cascade(&self,
context: &SharedStyleContext, context: &SharedStyleContext,
style: &mut Option<&mut Arc<ComputedValues>>) style: &mut Arc<ComputedValues>) -> bool {
-> bool {
let style = match *style {
None => return false,
Some(ref mut style) => style,
};
// Finish any expired transitions. // Finish any expired transitions.
let this_opaque = self.opaque(); let this_opaque = self.opaque();
let had_animations_to_expire = let had_animations_to_expire =
@ -909,18 +908,33 @@ pub trait MatchMethods : TNode {
let (damage, restyle_result) = { let (damage, restyle_result) = {
let mut data_ref = self.mutate_data().unwrap(); let mut data_ref = self.mutate_data().unwrap();
let mut data = &mut *data_ref; 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, &applicable_declarations.normal,
data.style.as_mut(),
&mut applicable_declarations_cache, &mut applicable_declarations_cache,
applicable_declarations.normal_shareable, CascadeBooleans {
/* should_animate = */ true); shareable: shareable,
cacheable: cacheable,
animate: true,
});
let (damage, restyle_result) = let (damage, restyle_result) =
self.compute_damage_and_cascade_pseudos(final_style, self.compute_damage_and_cascade_pseudos(new_style, old_style.as_ref(),
data, data, context,
context,
applicable_declarations, applicable_declarations,
&mut applicable_declarations_cache); &mut applicable_declarations_cache);
@ -942,6 +956,7 @@ pub trait MatchMethods : TNode {
fn compute_damage_and_cascade_pseudos<'a, Ctx>(&self, fn compute_damage_and_cascade_pseudos<'a, Ctx>(&self,
final_style: Arc<ComputedValues>, final_style: Arc<ComputedValues>,
old_style: Option<&Arc<ComputedValues>>,
data: &mut PersistentStyleData, data: &mut PersistentStyleData,
context: &Ctx, context: &Ctx,
applicable_declarations: &ApplicableDeclarations, applicable_declarations: &ApplicableDeclarations,
@ -955,7 +970,7 @@ pub trait MatchMethods : TNode {
// restyle hint. // restyle hint.
let this_display = final_style.get_box().clone_display(); let this_display = final_style.get_box().clone_display();
if this_display == display::T::none { 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() 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 // Otherwise, we just compute the damage normally, and sum up the damage
// related to pseudo-elements. // related to pseudo-elements.
let mut damage = 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); data.style = Some(final_style);
@ -989,72 +1004,58 @@ pub trait MatchMethods : TNode {
let rebuild_and_reflow = let rebuild_and_reflow =
Self::ConcreteRestyleDamage::rebuild_and_reflow(); Self::ConcreteRestyleDamage::rebuild_and_reflow();
let no_damage = Self::ConcreteRestyleDamage::empty();
<Self::ConcreteElement as MatchAttr>::Impl::each_eagerly_cascaded_pseudo_element(|pseudo| { <Self::ConcreteElement as MatchAttr>::Impl::each_eagerly_cascaded_pseudo_element(|pseudo| {
use std::collections::hash_map::Entry;
let applicable_declarations_for_this_pseudo = let applicable_declarations_for_this_pseudo =
applicable_declarations.per_pseudo.get(&pseudo).unwrap(); applicable_declarations.per_pseudo.get(&pseudo).unwrap();
let has_declarations = let has_declarations =
!applicable_declarations_for_this_pseudo.is_empty(); !applicable_declarations_for_this_pseudo.is_empty();
// If there are declarations matching, we're going to need to // The old entry will be replaced. Remove it from the map but keep
// recompute the style anyway, so do it now to simplify the logic // it for analysis.
// below. let mut old_pseudo_style = data_per_pseudo.remove(&pseudo);
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, if has_declarations {
new_style, // We have declarations, so we need to cascade. Compute parameters.
&*applicable_declarations_for_this_pseudo, let animate = <Self::ConcreteElement as MatchAttr>::Impl
data_per_pseudo.get_mut(&pseudo), ::pseudo_is_before_or_after(&pseudo);
&mut applicable_declarations_cache, let cacheable = if animate && old_pseudo_style.is_some() {
/* shareable = */ false, // Update animations before the cascade. This may modify
should_animate_properties)) // the value of old_pseudo_style.
} else { !self.update_animations_for_cascade(context.shared_context(),
None old_pseudo_style.as_mut().unwrap())
}; } else {
true
};
// Let's see what we had before. let new_pseudo_style =
match data_per_pseudo.entry(pseudo.clone()) { self.cascade_node_pseudo_element(context, new_style, old_pseudo_style.as_ref(),
Entry::Vacant(vacant_entry) => { &*applicable_declarations_for_this_pseudo,
// If we had a vacant entry, and no rules that match, we're &mut applicable_declarations_cache,
// fine so far. CascadeBooleans {
if !has_declarations { shareable: false,
return; cacheable: cacheable,
} animate: animate,
});
// Otherwise, we need to insert the new computed styles, and // Compute restyle damage unless we've already maxed it out.
// generate a rebuild_and_reflow damage. if damage != rebuild_and_reflow {
damage = damage | Self::ConcreteRestyleDamage::rebuild_and_reflow(); damage = damage | match old_pseudo_style {
vacant_entry.insert(pseudo_style_if_declarations.unwrap()); 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 // Insert the new entry into the map.
// damage, except if the damage was already let existing = data_per_pseudo.insert(pseudo, new_pseudo_style);
// rebuild_and_reflow, in which case we can avoid it. debug_assert!(existing.is_none());
if damage != rebuild_and_reflow { } else {
damage = damage | damage = damage | match old_pseudo_style {
self.compute_restyle_damage(Some(occupied_entry.get()), Some(_) => rebuild_and_reflow,
pseudo_style_if_declarations.as_ref().unwrap(), None => no_damage,
Some(&pseudo));
}
// And now, of course, use the new style.
occupied_entry.insert(pseudo_style_if_declarations.unwrap());
} }
} }
}); });