style: Support a restyle hint that indicates all descendants must be recascaded. draft
authorCameron McCormack <cam@mcc.id.au>
Sun, 28 May 2017 19:31:54 +0800
changeset 585856 e0d437a2f1e2ce9611b8d5c4770ee4386b9821bb
parent 585803 4541134e973a6bd5e667a603e844854c8e5361da
child 585857 87fd53879ad3017260cde1781dc63b104fa2ecbf
push id61210
push userbmo:cam@mcc.id.au
push dateMon, 29 May 2017 05:49:35 +0000
milestone55.0a1
style: Support a restyle hint that indicates all descendants must be recascaded. This also moves the result of deciding whether to recascade from the RestyleData into the RestyleHint. MozReview-Commit-ID: GoCdUebD7j3
servo/components/style/data.rs
servo/components/style/restyle_hints.rs
servo/components/style/traversal.rs
--- a/servo/components/style/data.rs
+++ b/servo/components/style/data.rs
@@ -362,16 +362,21 @@ impl StoredRestyleHint {
 
     /// Creates a restyle hint that forces the element and all its later
     /// siblings to have their whole subtrees restyled, including the elements
     /// themselves.
     pub fn subtree_and_later_siblings() -> Self {
         StoredRestyleHint(RestyleHint::subtree_and_later_siblings())
     }
 
+    /// Creates a restyle hint that indicates the element must be recascaded.
+    pub fn recascade_self() -> Self {
+        StoredRestyleHint(RestyleHint::recascade_self())
+    }
+
     /// Returns true if the hint indicates that our style may be invalidated.
     pub fn has_self_invalidations(&self) -> bool {
         self.0.affects_self()
     }
 
     /// Returns true if the hint indicates that our sibling's style may be
     /// invalidated.
     pub fn has_sibling_invalidations(&self) -> bool {
@@ -392,16 +397,22 @@ impl StoredRestyleHint {
     pub fn insert_from(&mut self, other: &Self) {
         self.0.insert_from(&other.0)
     }
 
     /// Returns true if the hint has animation-only restyle.
     pub fn has_animation_hint(&self) -> bool {
         self.0.has_animation_hint()
     }
+
+    /// Returns true if the hint indicates the current element must be
+    /// recascaded.
+    pub fn has_recascade_self(&self) -> bool {
+        self.0.has_recascade_self()
+    }
 }
 
 impl Default for StoredRestyleHint {
     fn default() -> Self {
         StoredRestyleHint::empty()
     }
 }
 
@@ -415,20 +426,16 @@ impl From<RestyleHint> for StoredRestyle
 /// 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: StoredRestyleHint,
 
-    /// Whether we need to recascade.
-    /// FIXME(bholley): This should eventually become more fine-grained.
-    pub recascade: 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.
     ///
@@ -437,17 +444,17 @@ pub struct RestyleData {
     /// 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() || self.recascade
+        self.hint.has_self_invalidations()
     }
 
     /// Returns true if this RestyleData might invalidate sibling styles.
     pub fn has_sibling_invalidations(&self) -> bool {
         self.hint.has_sibling_invalidations()
     }
 
     /// Returns damage handled.
@@ -588,22 +595,21 @@ impl ElementData {
         debug_assert!(self.restyle.is_some());
         let restyle_data = self.restyle.as_ref().unwrap();
 
         let hint = &restyle_data.hint.0;
         if hint.match_self() {
             return RestyleKind::MatchAndCascade;
         }
 
-        if !hint.is_empty() {
+        if hint.has_replacements() {
             return RestyleKind::CascadeWithReplacements(hint.replacements);
         }
 
-        debug_assert!(restyle_data.recascade,
-                      "We definitely need to do something!");
+        debug_assert!(hint.has_recascade_self(), "We definitely need to do something!");
         return RestyleKind::CascadeOnly;
     }
 
     /// Gets the element styles, if any.
     pub fn get_styles(&self) -> Option<&ElementStyles> {
         self.styles.as_ref()
     }
 
--- a/servo/components/style/restyle_hints.rs
+++ b/servo/components/style/restyle_hints.rs
@@ -40,16 +40,19 @@ use std::cmp;
 pub struct RestyleHint {
     /// Depths at which selector matching must be re-run.
     match_under_self: RestyleDepths,
 
     /// Rerun selector matching on all later siblings of the element and all
     /// of their descendants.
     match_later_siblings: bool,
 
+    /// Whether the current element and/or all descendants must be recascade.
+    recascade: CascadeHint,
+
     /// Levels of the cascade whose rule nodes should be recomputed and
     /// replaced.
     pub replacements: RestyleReplacements,
 }
 
 bitflags! {
     /// Cascade levels that can be updated for an element by simply replacing
     /// their rule node.
@@ -150,16 +153,47 @@ impl RestyleDepths {
 
     /// Returns whether this `RestyleDepths` includes all depths represented
     /// by `other`.
     fn contains(&self, other: RestyleDepths) -> bool {
         (self.0 & other.0) == other.0
     }
 }
 
+bitflags! {
+    /// Flags representing whether the current element or its descendants
+    /// must be recascaded.
+    ///
+    /// FIXME(bholley): This should eventually become more fine-grained.
+    pub flags CascadeHint: u8 {
+        /// Recascade the current element.
+        const RECASCADE_SELF = 0x01,
+        /// Recascade all descendant elements.
+        const RECASCADE_DESCENDANTS = 0x02,
+    }
+}
+
+impl CascadeHint {
+    /// Creates a new `CascadeHint` indicating that the current element and all
+    /// its descendants must be recascaded.
+    fn subtree() -> CascadeHint {
+        RECASCADE_SELF | RECASCADE_DESCENDANTS
+    }
+
+    /// Returns a new `CascadeHint` appropriate for children of the current
+    /// element.
+    fn propagate(&self) -> Self {
+        if self.contains(RECASCADE_DESCENDANTS) {
+            CascadeHint::subtree()
+        } else {
+            CascadeHint::empty()
+        }
+    }
+}
+
 /// Asserts that all RestyleReplacements have a matching nsRestyleHint value.
 #[cfg(feature = "gecko")]
 #[inline]
 pub fn assert_restyle_hints_match() {
     use gecko_bindings::structs;
 
     macro_rules! check_restyle_hints {
         ( $( $a:ident => $b:ident ),*, ) => {
@@ -186,120 +220,146 @@ pub fn assert_restyle_hints_match() {
 impl RestyleHint {
     /// Creates a new, empty `RestyleHint`, which represents no restyling work
     /// needs to be done.
     #[inline]
     pub fn empty() -> Self {
         RestyleHint {
             match_under_self: RestyleDepths::empty(),
             match_later_siblings: false,
+            recascade: CascadeHint::empty(),
             replacements: RestyleReplacements::empty(),
         }
     }
 
     /// Creates a new `RestyleHint` that indicates selector matching must be
     /// re-run on the element.
     #[inline]
     pub fn for_self() -> Self {
         RestyleHint {
             match_under_self: RestyleDepths::for_self(),
             match_later_siblings: false,
+            recascade: CascadeHint::empty(),
             replacements: RestyleReplacements::empty(),
         }
     }
 
     /// Creates a new `RestyleHint` that indicates selector matching must be
     /// re-run on all of the element's descendants.
     #[inline]
     pub fn descendants() -> Self {
         RestyleHint {
             match_under_self: RestyleDepths::for_descendants(),
             match_later_siblings: false,
+            recascade: CascadeHint::empty(),
             replacements: RestyleReplacements::empty(),
         }
     }
 
     /// Creates a new `RestyleHint` that indicates selector matching must be
     /// re-run on the descendants of element at the specified depth, where 0
     /// indicates the element itself, 1 is its children, 2 its grandchildren,
     /// etc.
     #[inline]
     pub fn descendants_at_depth(depth: u32) -> Self {
         RestyleHint {
             match_under_self: RestyleDepths::for_depth(depth),
             match_later_siblings: false,
+            recascade: CascadeHint::empty(),
             replacements: RestyleReplacements::empty(),
         }
     }
 
     /// Creates a new `RestyleHint` that indicates selector matching must be
     /// re-run on all of the element's later siblings and their descendants.
     #[inline]
     pub fn later_siblings() -> Self {
         RestyleHint {
             match_under_self: RestyleDepths::empty(),
             match_later_siblings: true,
+            recascade: CascadeHint::empty(),
             replacements: RestyleReplacements::empty(),
         }
     }
 
     /// Creates a new `RestyleHint` that indicates selector matching must be
     /// re-run on the element and all of its descendants.
     #[inline]
     pub fn subtree() -> Self {
         RestyleHint {
             match_under_self: RestyleDepths::for_self_and_descendants(),
             match_later_siblings: false,
+            recascade: CascadeHint::empty(),
             replacements: RestyleReplacements::empty(),
         }
     }
 
     /// Creates a new `RestyleHint` that indicates selector matching must be
     /// re-run on the element, its descendants, its later siblings, and
     /// their descendants.
     #[inline]
     pub fn subtree_and_later_siblings() -> Self {
         RestyleHint {
             match_under_self: RestyleDepths::for_self_and_descendants(),
             match_later_siblings: true,
+            recascade: CascadeHint::empty(),
             replacements: RestyleReplacements::empty(),
         }
     }
 
     /// Creates a new `RestyleHint` that indicates the specified rule node
     /// replacements must be performed on the element.
     #[inline]
     pub fn for_replacements(replacements: RestyleReplacements) -> Self {
         RestyleHint {
             match_under_self: RestyleDepths::empty(),
             match_later_siblings: false,
+            recascade: CascadeHint::empty(),
             replacements: replacements,
         }
     }
 
+    /// Creates a new `RestyleHint` that indicates the element must be
+    /// recascaded.
+    pub fn recascade_self() -> Self {
+        RestyleHint {
+            match_under_self: RestyleDepths::empty(),
+            match_later_siblings: false,
+            recascade: RECASCADE_SELF,
+            replacements: RestyleReplacements::empty(),
+        }
+    }
+
     /// Returns whether this `RestyleHint` represents no needed restyle work.
     #[inline]
     pub fn is_empty(&self) -> bool {
         *self == RestyleHint::empty()
     }
 
     /// Returns whether this `RestyleHint` represents the maximum possible
     /// restyle work, and thus any `insert()` calls will have no effect.
     #[inline]
     pub fn is_maximum(&self) -> bool {
         self.match_under_self.is_self_and_descendants() &&
             self.match_later_siblings &&
+            self.recascade.is_all() &&
             self.replacements.is_all()
     }
 
     /// Returns whether the hint specifies that some work must be performed on
     /// the current element.
     #[inline]
     pub fn affects_self(&self) -> bool {
-        self.match_self() || !self.replacements.is_empty()
+        self.match_self() || self.has_recascade_self() || !self.replacements.is_empty()
+    }
+
+    /// Returns whether the hint specifies that the currently element must be
+    /// recascaded.
+    pub fn has_recascade_self(&self) -> bool {
+        self.recascade.contains(RECASCADE_SELF)
     }
 
     /// Returns whether the hint specifies that later siblings must be restyled.
     #[inline]
     pub fn affects_later_siblings(&self) -> bool {
         self.match_later_siblings
     }
 
@@ -310,55 +370,76 @@ impl RestyleHint {
         self.replacements.intersects(RestyleReplacements::for_animations())
     }
 
     /// Returns whether the hint specifies some restyle work other than an
     /// animation cascade level replacement.
     #[inline]
     pub fn has_non_animation_hint(&self) -> bool {
         self.match_under_self.is_any() || self.match_later_siblings ||
+            !self.recascade.is_empty() ||
             self.replacements.contains(RESTYLE_STYLE_ATTRIBUTE)
     }
 
     /// Returns whether the hint specifies that selector matching must be re-run
     /// for the element.
     #[inline]
     pub fn match_self(&self) -> bool {
         self.match_under_self.has_self()
     }
 
+    /// Returns whether the hint specifies that some cascade levels must be
+    /// replaced.
+    #[inline]
+    pub fn has_replacements(&self) -> bool {
+        !self.replacements.is_empty()
+    }
+
     /// Returns a new `RestyleHint` appropriate for children of the current
     /// element.
     #[inline]
     pub fn propagate_for_non_animation_restyle(&self) -> Self {
         RestyleHint {
             match_under_self: self.match_under_self.propagate(),
             match_later_siblings: false,
+            recascade: self.recascade.propagate(),
             replacements: RestyleReplacements::empty(),
         }
     }
 
     /// Removes all of the animation-related hints.
     #[inline]
     pub fn remove_animation_hints(&mut self) {
         self.replacements.remove(RestyleReplacements::for_animations());
+
+        // While RECASCADE_SELF is not animation-specific, we only ever add and
+        // process it during traversal.  If we are here, removing animation
+        // hints, then we are in an animation-only traversal, and we know that
+        // any RECASCADE_SELF flag must have been set due to changes in
+        // inherited values after restyling for animations, and thus we
+        // want to remove it so that we don't later try to restyle the element
+        // during a normal restyle.  (We could have separate
+        // RECASCADE_SELF_NORMAL and RECASCADE_SELF_ANIMATIONS flags to make it
+        // clear, but this isn't currently necessary.)
+        self.recascade.remove(RECASCADE_SELF);
     }
 
     /// Removes the later siblings hint, and returns whether it was present.
     pub fn remove_later_siblings_hint(&mut self) -> bool {
         let later_siblings = self.match_later_siblings;
         self.match_later_siblings = false;
         later_siblings
     }
 
     /// Unions the specified `RestyleHint` into this one.
     #[inline]
     pub fn insert_from(&mut self, other: &Self) {
         self.match_under_self.insert(other.match_under_self);
         self.match_later_siblings |= other.match_later_siblings;
+        self.recascade.insert(other.recascade);
         self.replacements.insert(other.replacements);
     }
 
     /// Unions the specified `RestyleHint` into this one.
     #[inline]
     pub fn insert(&mut self, other: Self) {
         // A later patch should make it worthwhile to have an insert() function
         // that consumes its argument.
@@ -366,16 +447,17 @@ impl RestyleHint {
     }
 
     /// Returns whether this `RestyleHint` represents at least as much restyle
     /// work as the specified one.
     #[inline]
     pub fn contains(&mut self, other: &Self) -> bool {
         self.match_under_self.contains(other.match_under_self) &&
         (self.match_later_siblings & other.match_later_siblings) == other.match_later_siblings &&
+        self.recascade.contains(other.recascade) &&
         self.replacements.contains(other.replacements)
     }
 }
 
 impl RestyleReplacements {
     /// The replacements for the animation cascade levels.
     #[inline]
     pub fn for_animations() -> Self {
@@ -388,32 +470,39 @@ impl From<nsRestyleHint> for RestyleRepl
     fn from(raw: nsRestyleHint) -> Self {
         Self::from_bits_truncate(raw.0 as u8)
     }
 }
 
 #[cfg(feature = "gecko")]
 impl From<nsRestyleHint> for RestyleHint {
     fn from(raw: nsRestyleHint) -> Self {
+        use gecko_bindings::structs::nsRestyleHint_eRestyle_ForceDescendants as eRestyle_ForceDescendants;
         use gecko_bindings::structs::nsRestyleHint_eRestyle_LaterSiblings as eRestyle_LaterSiblings;
         use gecko_bindings::structs::nsRestyleHint_eRestyle_Self as eRestyle_Self;
         use gecko_bindings::structs::nsRestyleHint_eRestyle_SomeDescendants as eRestyle_SomeDescendants;
         use gecko_bindings::structs::nsRestyleHint_eRestyle_Subtree as eRestyle_Subtree;
 
         let mut match_under_self = RestyleDepths::empty();
         if (raw.0 & (eRestyle_Self.0 | eRestyle_Subtree.0)) != 0 {
             match_under_self.insert(RestyleDepths::for_self());
         }
         if (raw.0 & (eRestyle_Subtree.0 | eRestyle_SomeDescendants.0)) != 0 {
             match_under_self.insert(RestyleDepths::for_descendants());
         }
 
+        let mut recascade = CascadeHint::empty();
+        if (raw.0 & eRestyle_ForceDescendants.0) != 0 {
+            recascade.insert(CascadeHint::subtree())
+        }
+
         RestyleHint {
             match_under_self: match_under_self,
             match_later_siblings: (raw.0 & eRestyle_LaterSiblings.0) != 0,
+            recascade: recascade,
             replacements: raw.into(),
         }
     }
 }
 
 #[cfg(feature = "servo")]
 impl HeapSizeOf for RestyleHint {
     fn heap_size_of_children(&self) -> usize { 0 }
--- a/servo/components/style/traversal.rs
+++ b/servo/components/style/traversal.rs
@@ -346,17 +346,18 @@ pub trait DomTraversal<E: TElement> : Sy
                 return true;
             }
 
             let data = match el.borrow_data() {
                 Some(d) => d,
                 None => return false,
             };
             return data.get_restyle()
-                       .map_or(false, |r| r.hint.has_animation_hint() || r.recascade);
+                       .map_or(false, |r| r.hint.has_animation_hint() ||
+                                          r.hint.has_recascade_self());
         }
 
         // If the dirty descendants bit is set, we need to traverse no
         // matter what. Skip examining the ElementData.
         if el.has_dirty_descendants() {
             return true;
         }
 
@@ -376,17 +377,17 @@ pub trait DomTraversal<E: TElement> : Sy
         if let Some(r) = data.get_restyle() {
             // If we have a restyle hint or need to recascade, we need to
             // visit the element.
             //
             // Note that this is different than checking has_current_styles(),
             // since that can return true even if we have a restyle hint
             // indicating that the element's descendants (but not necessarily
             // the element) need restyling.
-            if !r.hint.is_empty() || r.recascade {
+            if !r.hint.is_empty() {
                 return true;
             }
         }
 
         // Servo uses the post-order traversal for flow construction, so
         // we need to traverse any element with damage so that we can perform
         // fixup / reconstruction on our way back up the tree.
         //
@@ -691,27 +692,33 @@ pub fn recalc_style_at<E, D>(traversal: 
             debug!("{:?} style is display:none - clearing data from descendants.",
                    element);
             clear_descendant_data(element, &|e| unsafe { D::clear_element_data(&e) });
         }
     }
 
     // Now that matching and cascading is done, clear the bits corresponding to
     // those operations and compute the propagated restyle hint.
-    let propagated_hint = match data.get_restyle_mut() {
+    let mut propagated_hint = match data.get_restyle_mut() {
         None => StoredRestyleHint::empty(),
         Some(r) => {
             debug_assert!(context.shared.traversal_flags.for_animation_only() ||
                           !r.hint.has_animation_hint(),
                           "animation restyle hint should be handled during \
                            animation-only restyles");
-            r.recascade = false;
             r.hint.propagate(&context.shared.traversal_flags)
         },
     };
+
+    if inherited_style_changed {
+        // FIXME(bholley): Need to handle explicitly-inherited reset properties
+        // somewhere.
+        propagated_hint.insert(StoredRestyleHint::recascade_self());
+    }
+
     trace!("propagated_hint={:?}, inherited_style_changed={:?}, \
             is_display_none={:?}, implementing_pseudo={:?}",
            propagated_hint, inherited_style_changed,
            data.styles().is_display_none(),
            element.implemented_pseudo_element());
     debug_assert!(element.has_current_styles(data) ||
                   context.shared.traversal_flags.for_animation_only(),
                   "Should have computed style or haven't yet valid computed \
@@ -725,28 +732,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() ||
-         inherited_style_changed) {
+         !propagated_hint.is_empty()) {
         let damage_handled = data.get_restyle().map_or(RestyleDamage::empty(), |r| {
             r.damage_handled() | r.damage.handled_for_descendants()
         });
 
         preprocess_children::<E, D>(context,
                                     traversal_data,
                                     element,
                                     propagated_hint,
-                                    damage_handled,
-                                    inherited_style_changed);
+                                    damage_handled);
     }
 
     // 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();
     }
@@ -839,18 +844,17 @@ fn compute_style<E, D>(_traversal: &D,
         }
     }
 }
 
 fn preprocess_children<E, D>(context: &mut StyleContext<E>,
                              parent_traversal_data: &PerLevelTraversalData,
                              element: E,
                              mut propagated_hint: StoredRestyleHint,
-                             damage_handled: RestyleDamage,
-                             parent_inherited_style_changed: bool)
+                             damage_handled: RestyleDamage)
     where E: TElement,
           D: DomTraversal<E>,
 {
     trace!("preprocess_children: {:?}", element);
 
     // Loop over all the children.
     for child in element.as_node().children() {
         // FIXME(bholley): Add TElement::element_children instead of this.
@@ -883,43 +887,31 @@ fn preprocess_children<E, D>(context: &m
                child_data.get_restyle().map(|r| &r.hint),
                propagated_hint,
                child.implemented_pseudo_element(),
                later_siblings);
 
         // 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() && !parent_inherited_style_changed &&
-           damage_handled.is_empty() && !child_data.has_restyle() {
+        if propagated_hint.is_empty() && damage_handled.is_empty() && !child_data.has_restyle() {
             continue;
         }
 
         let mut restyle_data = child_data.ensure_restyle();
 
         // Propagate the parent and sibling restyle hint.
-        if !propagated_hint.is_empty() {
-            restyle_data.hint.insert_from(&propagated_hint);
-        }
+        restyle_data.hint.insert_from(&propagated_hint);
 
         if later_siblings {
             propagated_hint.insert(RestyleHint::subtree().into());
         }
 
         // Store the damage already handled by ancestors.
         restyle_data.set_damage_handled(damage_handled);
-
-        // If properties that we inherited from the parent changed, we need to
-        // recascade.
-        //
-        // FIXME(bholley): Need to handle explicitly-inherited reset properties
-        // somewhere.
-        if parent_inherited_style_changed {
-            restyle_data.recascade = true;
-        }
     }
 }
 
 /// 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().children() {
         if let Some(kid) = kid.as_element() {
             // We maintain an invariant that, if an element has data, all its ancestors