Auto merge of #17087 - heycam:recascade, r=emilio

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.

Reviewed in https://bugzilla.mozilla.org/show_bug.cgi?id=1367647.

components/style/data.rs

@@ -367,6 +367,11 @@ impl StoredRestyleHint {
         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()
@@ -402,6 +407,12 @@ impl StoredRestyleHint {
     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 {
@@ -425,10 +436,6 @@ pub struct RestyleData {
     /// 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,
@@ -447,7 +454,7 @@ pub struct RestyleData {
 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.
@@ -598,12 +605,11 @@ impl ElementData {
             return RestyleKind::MatchAndCascade;
         }
 
-        if !hint.is_empty() {
+        if hint.has_replacements() {
             return RestyleKind::CascadeWithReplacements(hint.replacements);
         }
 
-        debug_assert!(restyle_data.recascade,
+        debug_assert!(hint.has_recascade_self(), "We definitely need to do something!");
-                      "We definitely need to do something!");
         return RestyleKind::CascadeOnly;
     }
 

components/style/restyle_hints.rs

@@ -45,6 +45,9 @@ pub struct RestyleHint {
     /// 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,
@@ -155,6 +158,37 @@ impl RestyleDepths {
     }
 }
 
+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]
@@ -191,6 +225,7 @@ impl RestyleHint {
         RestyleHint {
             match_under_self: RestyleDepths::empty(),
             match_later_siblings: false,
+            recascade: CascadeHint::empty(),
             replacements: RestyleReplacements::empty(),
         }
     }
@@ -202,6 +237,7 @@ impl RestyleHint {
         RestyleHint {
             match_under_self: RestyleDepths::for_self(),
             match_later_siblings: false,
+            recascade: CascadeHint::empty(),
             replacements: RestyleReplacements::empty(),
         }
     }
@@ -213,6 +249,7 @@ impl RestyleHint {
         RestyleHint {
             match_under_self: RestyleDepths::for_descendants(),
             match_later_siblings: false,
+            recascade: CascadeHint::empty(),
             replacements: RestyleReplacements::empty(),
         }
     }
@@ -226,6 +263,7 @@ impl RestyleHint {
         RestyleHint {
             match_under_self: RestyleDepths::for_depth(depth),
             match_later_siblings: false,
+            recascade: CascadeHint::empty(),
             replacements: RestyleReplacements::empty(),
         }
     }
@@ -237,6 +275,7 @@ impl RestyleHint {
         RestyleHint {
             match_under_self: RestyleDepths::empty(),
             match_later_siblings: true,
+            recascade: CascadeHint::empty(),
             replacements: RestyleReplacements::empty(),
         }
     }
@@ -248,6 +287,7 @@ impl RestyleHint {
         RestyleHint {
             match_under_self: RestyleDepths::for_self_and_descendants(),
             match_later_siblings: false,
+            recascade: CascadeHint::empty(),
             replacements: RestyleReplacements::empty(),
         }
     }
@@ -260,6 +300,7 @@ impl RestyleHint {
         RestyleHint {
             match_under_self: RestyleDepths::for_self_and_descendants(),
             match_later_siblings: true,
+            recascade: CascadeHint::empty(),
             replacements: RestyleReplacements::empty(),
         }
     }
@@ -271,10 +312,22 @@ impl RestyleHint {
         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 {
@@ -287,6 +340,7 @@ impl RestyleHint {
     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()
     }
 
@@ -294,7 +348,13 @@ impl RestyleHint {
     /// 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.
@@ -315,6 +375,7 @@ impl RestyleHint {
     #[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)
     }
 
@@ -325,6 +386,13 @@ impl RestyleHint {
         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]
@@ -332,6 +400,7 @@ impl RestyleHint {
         RestyleHint {
             match_under_self: self.match_under_self.propagate(),
             match_later_siblings: false,
+            recascade: self.recascade.propagate(),
             replacements: RestyleReplacements::empty(),
         }
     }
@@ -340,6 +409,17 @@ impl RestyleHint {
     #[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.
@@ -354,6 +434,7 @@ impl RestyleHint {
     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);
     }
 
@@ -371,6 +452,7 @@ impl RestyleHint {
     pub fn contains(&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)
     }
 }
@@ -393,6 +475,7 @@ impl From<nsRestyleHint> for RestyleReplacements {
 #[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;
@@ -406,9 +489,15 @@ impl From<nsRestyleHint> for RestyleHint {
             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(),
         }
     }

components/style/traversal.rs

@@ -351,7 +351,8 @@ pub trait DomTraversal<E: TElement> : Sync {
                 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
@@ -381,7 +382,7 @@ pub trait DomTraversal<E: TElement> : Sync {
             // 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;
             }
         }
@@ -696,17 +697,23 @@ pub fn recalc_style_at<E, D>(traversal: &D,
 
     // 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,
@@ -730,8 +737,7 @@ pub fn recalc_style_at<E, D>(traversal: &D,
                                           &data,
                                           DontLog) &&
         (has_dirty_descendants_for_this_restyle ||
-         !propagated_hint.is_empty() ||
+         !propagated_hint.is_empty()) {
-         inherited_style_changed) {
         let damage_handled = data.get_restyle().map_or(RestyleDamage::empty(), |r| {
             r.damage_handled() | r.damage.handled_for_descendants()
         });
@@ -740,8 +746,7 @@ pub fn recalc_style_at<E, D>(traversal: &D,
                                     traversal_data,
                                     element,
                                     propagated_hint,
-                                    damage_handled,
+                                    damage_handled);
-                                    inherited_style_changed);
     }
 
     // If we are in a restyle for reconstruction, drop the existing restyle
@@ -844,8 +849,7 @@ fn preprocess_children<E, D>(context: &mut StyleContext<E>,
                              parent_traversal_data: &PerLevelTraversalData,
                              element: E,
                              mut propagated_hint: StoredRestyleHint,
-                             damage_handled: RestyleDamage,
+                             damage_handled: RestyleDamage)
-                             parent_inherited_style_changed: bool)
     where E: TElement,
           D: DomTraversal<E>,
 {
@@ -888,17 +892,14 @@ fn preprocess_children<E, D>(context: &mut StyleContext<E>,
         // 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 &&
+        if propagated_hint.is_empty() && damage_handled.is_empty() && !child_data.has_restyle() {
-           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());
@@ -906,15 +907,6 @@ fn preprocess_children<E, D>(context: &mut StyleContext<E>,
 
         // 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;
-        }
     }
 }