Bug 1368236: Remove damage_handled, and use a reconstructed_ancestor bit instead. r?bholley
MozReview-Commit-ID: 8KK0YfhiS2
--- a/servo/components/style/data.rs
+++ b/servo/components/style/data.rs
@@ -352,58 +352,29 @@ impl ElementStyles {
/// either before or during restyle traversal, and is cleared at the end of node
/// processing.
#[derive(Debug, Default)]
pub struct RestyleData {
/// The restyle hint, which indicates whether selectors need to be rematched
/// for this element, its children, and its descendants.
pub hint: RestyleHint,
+ /// Whether we reframed/reconstructed any ancestor or self.
+ pub reconstructed_ancestor: bool,
+
/// The restyle damage, indicating what kind of layout changes are required
/// afte restyling.
pub damage: RestyleDamage,
-
- /// The restyle damage that has already been handled by our ancestors, and does
- /// not need to be applied again at this element. Only non-empty during the
- /// traversal, once ancestor damage has been calculated.
- ///
- /// Note that this optimization mostly makes sense in terms of Gecko's top-down
- /// frame constructor and change list processing model. We don't bother with it
- /// for Servo for now.
- #[cfg(feature = "gecko")]
- pub damage_handled: RestyleDamage,
}
impl RestyleData {
/// Returns true if this RestyleData might invalidate the current style.
pub fn has_invalidations(&self) -> bool {
self.hint.has_self_invalidations()
}
-
- /// Returns damage handled.
- #[cfg(feature = "gecko")]
- pub fn damage_handled(&self) -> RestyleDamage {
- self.damage_handled
- }
-
- /// Returns damage handled (always empty for servo).
- #[cfg(feature = "servo")]
- pub fn damage_handled(&self) -> RestyleDamage {
- RestyleDamage::empty()
- }
-
- /// Sets damage handled.
- #[cfg(feature = "gecko")]
- pub fn set_damage_handled(&mut self, d: RestyleDamage) {
- self.damage_handled = d;
- }
-
- /// Sets damage handled. No-op for Servo.
- #[cfg(feature = "servo")]
- pub fn set_damage_handled(&mut self, _: RestyleDamage) {}
}
/// Style system data associated with an Element.
///
/// In Gecko, this hangs directly off the Element. Servo, this is embedded
/// inside of layout data, which itself hangs directly off the Element. In
/// both cases, it is wrapped inside an AtomicRefCell to ensure thread safety.
#[derive(Debug)]
--- a/servo/components/style/matching.rs
+++ b/servo/components/style/matching.rs
@@ -692,18 +692,18 @@ trait PrivateMatchMethods: TElement {
// If an ancestor is already getting reconstructed by Gecko's top-down
// frame constructor, no need to apply damage. Similarly if we already
// have an explicitly stored ReconstructFrame hint.
//
// See https://bugzilla.mozilla.org/show_bug.cgi?id=1301258#c12
// for followup work to make the optimization here more optimal by considering
// each bit individually.
let skip_applying_damage =
- restyle.damage_handled.contains(RestyleDamage::reconstruct()) ||
- restyle.damage.contains(RestyleDamage::reconstruct());
+ restyle.damage.contains(RestyleDamage::reconstruct()) ||
+ restyle.reconstructed_ancestor;
let difference = self.compute_style_difference(&old_values,
&new_values,
pseudo);
if !skip_applying_damage {
restyle.damage |= difference.damage;
}
difference.change.into()
--- a/servo/components/style/traversal.rs
+++ b/servo/components/style/traversal.rs
@@ -292,18 +292,18 @@ pub trait DomTraversal<E: TElement> : Sy
// transitions that we won't see otherwise.
//
// But it may be that we no longer match, so detect that case
// and act appropriately here.
if el.is_native_anonymous() {
if let Some(parent) = el.traversal_parent() {
let parent_data = parent.borrow_data().unwrap();
let going_to_reframe = parent_data.get_restyle().map_or(false, |r| {
- (r.damage | r.damage_handled())
- .contains(RestyleDamage::reconstruct())
+ r.reconstructed_ancestor ||
+ r.damage.contains(RestyleDamage::reconstruct())
});
let mut is_before_or_after_pseudo = false;
if let Some(pseudo) = el.implemented_pseudo_element() {
if pseudo.is_before_or_after() {
is_before_or_after_pseudo = true;
let still_match =
parent_data.styles().pseudos.get(&pseudo).is_some();
@@ -725,24 +725,26 @@ pub fn recalc_style_at<E, D>(traversal:
// Preprocess children, propagating restyle hints and handling sibling
// relationships.
if traversal.should_traverse_children(&mut context.thread_local,
element,
&data,
DontLog) &&
(has_dirty_descendants_for_this_restyle ||
!propagated_hint.is_empty()) {
- let damage_handled = data.get_restyle().map_or(RestyleDamage::empty(), |r| {
- r.damage_handled() | r.damage.handled_for_descendants()
+ let reconstructed_ancestor = data.get_restyle().map_or(false, |r| {
+ r.reconstructed_ancestor ||
+ r.damage.contains(RestyleDamage::reconstruct())
});
-
- preprocess_children::<E, D>(context,
- element,
- propagated_hint,
- damage_handled);
+ preprocess_children::<E, D>(
+ context,
+ element,
+ propagated_hint,
+ reconstructed_ancestor,
+ )
}
// If we are in a restyle for reconstruction, drop the existing restyle
// data here, since we won't need to perform a post-traversal to pick up
// any change hints.
if context.shared.traversal_flags.for_reconstruct() {
data.clear_restyle();
}
@@ -830,22 +832,25 @@ fn compute_style<E, D>(_traversal: &D,
context,
data,
/* important_rules_changed = */ false
)
}
}
}
-fn preprocess_children<E, D>(context: &mut StyleContext<E>,
- element: E,
- propagated_hint: RestyleHint,
- damage_handled: RestyleDamage)
- where E: TElement,
- D: DomTraversal<E>,
+fn preprocess_children<E, D>(
+ context: &mut StyleContext<E>,
+ element: E,
+ propagated_hint: RestyleHint,
+ reconstructed_ancestor: bool,
+)
+where
+ E: TElement,
+ D: DomTraversal<E>,
{
trace!("preprocess_children: {:?}", element);
// Loop over all the traversal children.
for child in element.as_node().traversal_children() {
// FIXME(bholley): Add TElement::element_children instead of this.
let child = match child.as_element() {
Some(el) => el,
@@ -870,30 +875,29 @@ fn preprocess_children<E, D>(context: &m
child,
child_data.get_restyle().map(|r| &r.hint),
propagated_hint,
child.implemented_pseudo_element());
// If the child doesn't have pre-existing RestyleData and we don't have
// any reason to create one, avoid the useless allocation and move on to
// the next child.
- if propagated_hint.is_empty() &&
- damage_handled.is_empty() &&
- !child_data.has_restyle() {
+ if !reconstructed_ancestor && propagated_hint.is_empty() && !child_data.has_restyle() {
continue;
}
let mut restyle_data = child_data.ensure_restyle();
+ debug_assert!(!restyle_data.reconstructed_ancestor,
+ "How did we know this already?");
+
// Propagate the parent restyle hint, that may make us restyle the whole
// subtree.
+ restyle_data.reconstructed_ancestor = reconstructed_ancestor;
restyle_data.hint.insert(propagated_hint);
-
- // Store the damage already handled by ancestors.
- restyle_data.set_damage_handled(damage_handled);
}
}
/// Clear style data for all the subtree under `el`.
pub fn clear_descendant_data<E: TElement, F: Fn(E)>(el: E, clear_data: &F) {
for kid in el.as_node().traversal_children() {
if let Some(kid) = kid.as_element() {
// We maintain an invariant that, if an element has data, all its ancestors