Hoist flags out of RestyleData.

MozReview-Commit-ID: 8emE83lykh3

components/style/data.rs

@@ -20,7 +20,8 @@ use std::fmt;
 use std::ops::{Deref, DerefMut};
 
 bitflags! {
-    flags RestyleFlags: u8 {
+    #[derive(Default)]
+    flags ElementDataFlags: u8 {
         /// Whether the styles changed for this restyle.
         const WAS_RESTYLED = 1 << 0,
         /// Whether the last traversal of this element did not do
@@ -42,16 +43,13 @@ bitflags! {
 /// processing.
 #[derive(Debug)]
 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,
-
-    /// A few flags to have in mind.
-    flags: RestyleFlags,
-
     /// The restyle damage, indicating what kind of layout changes are required
     /// afte restyling.
     pub damage: RestyleDamage,
+
+    /// The restyle hint, which indicates whether selectors need to be rematched
+    /// for this element, its children, and its descendants.
+    pub hint: RestyleHint,
 }
 
 impl Default for RestyleData {
@@ -63,91 +61,10 @@ impl Default for RestyleData {
 impl RestyleData {
     fn new() -> Self {
         Self {
-            hint: RestyleHint::empty(),
-            flags: RestyleFlags::empty(),
             damage: RestyleDamage::empty(),
+            hint: RestyleHint::empty(),
         }
     }
-
-    /// Clear all the restyle state associated with this element.
-    ///
-    /// FIXME(bholley): The only caller of this should probably just assert that
-    /// the hint is empty and call clear_flags_and_damage().
-    #[inline]
-    fn clear_restyle_state(&mut self) {
-        self.clear_restyle_flags_and_damage();
-        self.hint = RestyleHint::empty();
-    }
-
-    /// Clear restyle flags and damage.
-    ///
-    /// Note that we don't touch the TRAVERSED_WITHOUT_STYLING bit, which gets
-    /// set to the correct value on each traversal. There's no reason anyone
-    /// needs to clear it, and clearing it accidentally mid-traversal could
-    /// cause incorrect style sharing behavior.
-    #[inline]
-    fn clear_restyle_flags_and_damage(&mut self) {
-        self.damage = RestyleDamage::empty();
-        self.flags = self.flags & TRAVERSED_WITHOUT_STYLING;
-    }
-
-    /// Returns whether this element or any ancestor is going to be
-    /// reconstructed.
-    pub fn reconstructed_self_or_ancestor(&self) -> bool {
-        self.reconstructed_ancestor() || self.reconstructed_self()
-    }
-
-    /// Returns whether this element is going to be reconstructed.
-    pub fn reconstructed_self(&self) -> bool {
-        self.damage.contains(RestyleDamage::reconstruct())
-    }
-
-    /// Returns whether any ancestor of this element is going to be
-    /// reconstructed.
-    fn reconstructed_ancestor(&self) -> bool {
-        self.flags.contains(ANCESTOR_WAS_RECONSTRUCTED)
-    }
-
-    /// Sets the flag that tells us whether we've reconstructed an ancestor.
-    pub fn set_reconstructed_ancestor(&mut self, reconstructed: bool) {
-        if reconstructed {
-            // If it weren't for animation-only traversals, we could assert
-            // `!self.reconstructed_ancestor()` here.
-            self.flags.insert(ANCESTOR_WAS_RECONSTRUCTED);
-        } else {
-            self.flags.remove(ANCESTOR_WAS_RECONSTRUCTED);
-        }
-    }
-
-    /// Mark this element as restyled, which is useful to know whether we need
-    /// to do a post-traversal.
-    pub fn set_restyled(&mut self) {
-        self.flags.insert(WAS_RESTYLED);
-        self.flags.remove(TRAVERSED_WITHOUT_STYLING);
-    }
-
-    /// Returns true if this element was restyled.
-    #[inline]
-    pub fn is_restyle(&self) -> bool {
-        self.flags.contains(WAS_RESTYLED)
-    }
-
-    /// Mark that we traversed this element without computing any style for it.
-    pub fn set_traversed_without_styling(&mut self) {
-        self.flags.insert(TRAVERSED_WITHOUT_STYLING);
-    }
-
-    /// Returns whether the element was traversed without computing any style for
-    /// it.
-    pub fn traversed_without_styling(&self) -> bool {
-        self.flags.contains(TRAVERSED_WITHOUT_STYLING)
-    }
-
-    /// Returns whether this element has been part of a restyle.
-    #[inline]
-    pub fn contains_restyle_data(&self) -> bool {
-        self.is_restyle() || !self.hint.is_empty() || !self.damage.is_empty()
-    }
 }
 
 /// A lazily-allocated list of styles for eagerly-cascaded pseudo-elements.
@@ -305,6 +222,9 @@ pub struct ElementData {
 
     /// Restyle state.
     pub restyle: RestyleData,
+
+    /// Flags.
+    flags: ElementDataFlags,
 }
 
 /// The kind of restyle that a single element should do.
@@ -440,17 +360,94 @@ impl ElementData {
     }
 
     /// Drops any restyle state from the element.
+    ///
+    /// FIXME(bholley): The only caller of this should probably just assert that
+    /// the hint is empty and call clear_flags_and_damage().
     #[inline]
     pub fn clear_restyle_state(&mut self) {
-        self.restyle.clear_restyle_state();
+        self.restyle.hint = RestyleHint::empty();
+        self.clear_restyle_flags_and_damage();
     }
 
     /// Drops restyle flags and damage from the element.
     #[inline]
     pub fn clear_restyle_flags_and_damage(&mut self) {
-        self.restyle.clear_restyle_flags_and_damage();
+        self.restyle.damage = RestyleDamage::empty();
+        self.flags.remove(WAS_RESTYLED | ANCESTOR_WAS_RECONSTRUCTED)
     }
 
+    /// Returns whether this element or any ancestor is going to be
+    /// reconstructed.
+    pub fn reconstructed_self_or_ancestor(&self) -> bool {
+        self.reconstructed_ancestor() || self.reconstructed_self()
+    }
+
+    /// Returns whether this element is going to be reconstructed.
+    pub fn reconstructed_self(&self) -> bool {
+        self.restyle.damage.contains(RestyleDamage::reconstruct())
+    }
+
+    /// Returns whether any ancestor of this element is going to be
+    /// reconstructed.
+    fn reconstructed_ancestor(&self) -> bool {
+        self.flags.contains(ANCESTOR_WAS_RECONSTRUCTED)
+    }
+
+    /// Sets the flag that tells us whether we've reconstructed an ancestor.
+    pub fn set_reconstructed_ancestor(&mut self, reconstructed: bool) {
+        if reconstructed {
+            // If it weren't for animation-only traversals, we could assert
+            // `!self.reconstructed_ancestor()` here.
+            self.flags.insert(ANCESTOR_WAS_RECONSTRUCTED);
+        } else {
+            self.flags.remove(ANCESTOR_WAS_RECONSTRUCTED);
+        }
+    }
+
+    /// Mark this element as restyled, which is useful to know whether we need
+    /// to do a post-traversal.
+    pub fn set_restyled(&mut self) {
+        self.flags.insert(WAS_RESTYLED);
+        self.flags.remove(TRAVERSED_WITHOUT_STYLING);
+    }
+
+    /// Returns true if this element was restyled.
+    #[inline]
+    pub fn is_restyle(&self) -> bool {
+        self.flags.contains(WAS_RESTYLED)
+    }
+
+    /// Mark that we traversed this element without computing any style for it.
+    pub fn set_traversed_without_styling(&mut self) {
+        self.flags.insert(TRAVERSED_WITHOUT_STYLING);
+    }
+
+    /// Returns whether the element was traversed without computing any style for
+    /// it.
+    pub fn traversed_without_styling(&self) -> bool {
+        self.flags.contains(TRAVERSED_WITHOUT_STYLING)
+    }
+
+    /// Returns whether this element has been part of a restyle.
+    #[inline]
+    pub fn contains_restyle_data(&self) -> bool {
+        self.is_restyle() || !self.restyle.hint.is_empty() || !self.restyle.damage.is_empty()
+    }
+
+    /// 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.
+    #[cfg(feature = "gecko")]
+    pub fn skip_applying_damage(&self) -> bool { self.reconstructed_self_or_ancestor() }
+
+    /// N/A in Servo.
+    #[cfg(feature = "servo")]
+    pub fn skip_applying_damage(&self) -> bool { false }
+
     /// Measures memory usage.
     #[cfg(feature = "gecko")]
     pub fn size_of_excluding_cvs(&self, ops: &mut MallocSizeOfOps) -> usize {

components/style/gecko/wrapper.rs

@@ -510,7 +510,7 @@ impl<'le> GeckoElement<'le> {
                             ELEMENT_HAS_ANIMATION_ONLY_DIRTY_DESCENDANTS_FOR_SERVO as u32 |
                             NODE_DESCENDANTS_NEED_FRAMES as u32 |
                             NODE_NEEDS_FRAME as u32) != 0;
-        has_flag || self.borrow_data().unwrap().restyle.contains_restyle_data()
+        has_flag || self.borrow_data().unwrap().contains_restyle_data()
     }
 
     /// Returns true if this element has a shadow root.

components/style/matching.rs

@@ -352,10 +352,11 @@ trait PrivateMatchMethods: TElement {
     fn accumulate_damage_for(
         &self,
         shared_context: &SharedStyleContext,
+        skip_applying_damage: bool,
         restyle: &mut RestyleData,
         old_values: &ComputedValues,
         new_values: &ComputedValues,
-        pseudo: Option<&PseudoElement>
+        pseudo: Option<&PseudoElement>,
     ) -> ChildCascadeRequirement {
         debug!("accumulate_damage_for: {:?}", self);
 
@@ -365,16 +366,6 @@ trait PrivateMatchMethods: TElement {
             return ChildCascadeRequirement::MustCascadeChildren;
         }
 
-        // 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 =
-            cfg!(feature = "gecko") && restyle.reconstructed_self_or_ancestor();
-
         let difference =
             self.compute_style_difference(old_values, new_values, pseudo);
 
@@ -615,6 +606,7 @@ pub trait MatchMethods : TElement {
             cascade_requirement,
             self.accumulate_damage_for(
                 context.shared,
+                data.skip_applying_damage(),
                 &mut data.restyle,
                 &old_primary_style,
                 new_primary_style,
@@ -636,6 +628,7 @@ pub trait MatchMethods : TElement {
                 (&Some(ref old), &Some(ref new)) => {
                     self.accumulate_damage_for(
                         context.shared,
+                        data.skip_applying_damage(),
                         &mut data.restyle,
                         old,
                         new,

components/style/sharing/checks.rs

@@ -40,8 +40,7 @@ pub fn can_share_style_across_parents<E>(first: Option<E>, second: Option<E>) ->
     //
     // This is a somewhat conservative check. We could tighten it by having the
     // invalidation logic explicitly flag elements for which it ellided styling.
-    if first_data.restyle.traversed_without_styling() ||
+    if first_data.traversed_without_styling() || second_data.traversed_without_styling() {
-       second_data.restyle.traversed_without_styling() {
         return false;
     }
 

components/style/traversal.rs

@@ -161,7 +161,7 @@ pub trait DomTraversal<E: TElement> : Sync {
             // the last traversal (at a potentially-higher root). From the
             // perspective of this traversal, the root cannot have reconstructed
             // ancestors.
-            data.restyle.set_reconstructed_ancestor(false);
+            data.set_reconstructed_ancestor(false);
         };
 
         let parent = root.traversal_parent();
@@ -244,7 +244,7 @@ pub trait DomTraversal<E: TElement> : Sync {
         if el.is_native_anonymous() {
             if let Some(parent_data) = parent_data {
                 let going_to_reframe =
-                    parent_data.restyle.reconstructed_self_or_ancestor();
+                    parent_data.reconstructed_self_or_ancestor();
 
                 let mut is_before_or_after_pseudo = false;
                 if let Some(pseudo) = el.implemented_pseudo_element() {
@@ -499,7 +499,7 @@ where
         notify_paint_worklet(context, data);
     } else {
         debug_assert!(data.has_styles());
-        data.restyle.set_traversed_without_styling();
+        data.set_traversed_without_styling();
     }
 
     // Now that matching and cascading is done, clear the bits corresponding to
@@ -551,7 +551,7 @@ where
                                 !propagated_hint.is_empty() ||
                                 !child_cascade_requirement.can_skip_cascade() ||
                                 context.thread_local.is_initial_style() ||
-                                data.restyle.reconstructed_self() ||
+                                data.reconstructed_self() ||
                                 is_servo_nonincremental_layout();
 
     traverse_children = traverse_children &&
@@ -565,7 +565,7 @@ where
             data,
             propagated_hint,
             child_cascade_requirement,
-            data.restyle.reconstructed_self_or_ancestor(),
+            data.reconstructed_self_or_ancestor(),
             note_child
         );
     }
@@ -631,7 +631,7 @@ where
     debug!("compute_style: {:?} (kind={:?})", element, kind);
 
     if data.has_styles() {
-        data.restyle.set_restyled();
+        data.set_restyled();
     }
 
     let mut important_rules_changed = false;
@@ -811,7 +811,7 @@ where
         if let Some(ref mut child_data) = child_data {
             // Propagate the parent restyle hint, that may make us restyle the whole
             // subtree.
-            child_data.restyle.set_reconstructed_ancestor(reconstructed_ancestor);
+            child_data.set_reconstructed_ancestor(reconstructed_ancestor);
 
             let mut child_hint = propagated_hint;
             match cascade_requirement {

ports/geckolib/glue.rs

@@ -2980,7 +2980,7 @@ pub extern "C" fn Servo_TakeChangeHint(element: RawGeckoElementBorrowed,
 
     let damage = match element.mutate_data() {
         Some(mut data) => {
-            *was_restyled = data.restyle.is_restyle();
+            *was_restyled = data.is_restyle();
 
             let damage = data.restyle.damage;
             data.clear_restyle_state();