Auto merge of #17317 - emilio:die-storedrestylehint-die, r=bholley

Kill StoredRestyleHint, and shrink RestyleData.

These are the already-reviewed parts of bug 1368236.

components/layout_thread/lib.rs

@@ -111,9 +111,9 @@ use std::thread;
 use style::animation::Animation;
 use style::context::{QuirksMode, ReflowGoal, SharedStyleContext};
 use style::context::{StyleSystemOptions, ThreadLocalStyleContextCreationInfo};
-use style::data::StoredRestyleHint;
 use style::dom::{ShowSubtree, ShowSubtreeDataAndPrimaryValues, TElement, TNode};
 use style::error_reporting::{NullReporter, RustLogReporter};
+use style::invalidation::element::restyle_hints::RestyleHint;
 use style::logical_geometry::LogicalPoint;
 use style::media_queries::{Device, MediaList, MediaType};
 use style::selector_parser::SnapshotMap;
@@ -1119,7 +1119,7 @@ impl LayoutThread {
                         let el = node.as_element().unwrap();
                         if let Some(mut d) = element.mutate_data() {
                             if d.has_styles() {
-                                d.ensure_restyle().hint.insert(StoredRestyleHint::subtree());
+                                d.ensure_restyle().hint.insert(RestyleHint::restyle_subtree());
                             }
                         }
                         if let Some(p) = el.parent_element() {
@@ -1155,7 +1155,7 @@ impl LayoutThread {
         if needs_dirtying {
             if let Some(mut d) = element.mutate_data() {
                 if d.has_styles() {
-                    d.ensure_restyle().hint.insert(StoredRestyleHint::subtree());
+                    d.ensure_restyle().hint.insert(RestyleHint::restyle_subtree());
                 }
             }
         }

components/style/data.rs

@@ -16,7 +16,6 @@ use selectors::matching::VisitedHandlingMode;
 use shared_lock::{Locked, StylesheetGuards};
 use std::fmt;
 use stylearc::Arc;
-use traversal::TraversalFlags;
 
 /// The structure that represents the result of style computation. This is
 /// effectively a tuple of rules and computed values, that is, the rule node,
@@ -349,97 +348,6 @@ impl ElementStyles {
     }
 }
 
-/// Restyle hint for storing on ElementData.
-///
-/// We wrap it in a newtype to force the encapsulation of the complexity of
-/// handling the correct invalidations in this file.
-///
-/// TODO(emilio): This will probably be a non-issue in a bit.
-#[derive(Clone, Copy, Debug)]
-pub struct StoredRestyleHint(pub RestyleHint);
-
-impl StoredRestyleHint {
-    /// Propagates this restyle hint to a child element.
-    pub fn propagate(&mut self, traversal_flags: &TraversalFlags) -> Self {
-        use std::mem;
-
-        // In the middle of an animation only restyle, we don't need to
-        // propagate any restyle hints, and we need to remove ourselves.
-        if traversal_flags.for_animation_only() {
-            self.0.remove_animation_hints();
-            return Self::empty();
-        }
-
-        debug_assert!(!self.0.has_animation_hint(),
-                      "There should not be any animation restyle hints \
-                       during normal traversal");
-
-        // Else we should clear ourselves, and return the propagated hint.
-        let new_hint = mem::replace(&mut self.0, RestyleHint::empty())
-                       .propagate_for_non_animation_restyle();
-        StoredRestyleHint(new_hint)
-    }
-
-    /// Creates an empty `StoredRestyleHint`.
-    pub fn empty() -> Self {
-        StoredRestyleHint(RestyleHint::empty())
-    }
-
-    /// Creates a restyle hint that forces the whole subtree to be restyled,
-    /// including the element.
-    pub fn subtree() -> Self {
-        StoredRestyleHint(RestyleHint::restyle_subtree())
-    }
-
-    /// 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()
-    }
-
-    /// Whether the restyle hint is empty (nothing requires to be restyled).
-    pub fn is_empty(&self) -> bool {
-        self.0.is_empty()
-    }
-
-    /// Insert another restyle hint, effectively resulting in the union of both.
-    pub fn insert(&mut self, other: Self) {
-        self.0.insert(other.0)
-    }
-
-    /// Contains whether the whole subtree is invalid.
-    pub fn contains_subtree(&self) -> bool {
-        self.0.contains(RestyleHint::restyle_subtree())
-    }
-
-    /// 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 {
-        Self::empty()
-    }
-}
-
-impl From<RestyleHint> for StoredRestyleHint {
-    fn from(hint: RestyleHint) -> Self {
-        StoredRestyleHint(hint)
-    }
-}
-
 /// Transient data used by the restyle algorithm. This structure is instantiated
 /// either before or during restyle traversal, and is cleared at the end of node
 /// processing.
@@ -447,21 +355,14 @@ impl From<RestyleHint> for StoredRestyleHint {
 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,
+    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 {
@@ -469,28 +370,6 @@ impl RestyleData {
     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.
@@ -588,8 +467,8 @@ impl ElementData {
 
         debug_assert!(self.restyle.is_some());
         let restyle_data = self.restyle.as_ref().unwrap();
-        let hint = restyle_data.hint.0;
 
+        let hint = restyle_data.hint;
         if shared_context.traversal_flags.for_animation_only() {
             // return either CascadeWithReplacements or CascadeOnly in case of
             // animation-only restyle.

components/style/invalidation/element/restyle_hints.rs

@@ -6,6 +6,7 @@
 
 #[cfg(feature = "gecko")]
 use gecko_bindings::structs::nsRestyleHint;
+use traversal::TraversalFlags;
 
 bitflags! {
     /// The kind of restyle we need to do for a given element.
@@ -57,9 +58,40 @@ impl RestyleHint {
         RECASCADE_SELF | RECASCADE_DESCENDANTS
     }
 
+    /// Returns whether this hint invalidates the element and all its
+    /// descendants.
+    pub fn contains_subtree(&self) -> bool {
+        self.contains(RESTYLE_SELF | RESTYLE_DESCENDANTS)
+    }
+
+    /// Returns whether we need to restyle this element.
+    pub fn has_self_invalidations(&self) -> bool {
+        self.intersects(RESTYLE_SELF | RECASCADE_SELF | Self::replacements())
+    }
+
+    /// Propagates this restyle hint to a child element.
+    pub fn propagate(&mut self, traversal_flags: &TraversalFlags) -> Self {
+        use std::mem;
+
+        // In the middle of an animation only restyle, we don't need to
+        // propagate any restyle hints, and we need to remove ourselves.
+        if traversal_flags.for_animation_only() {
+            self.remove_animation_hints();
+            return Self::empty();
+        }
+
+        debug_assert!(!self.has_animation_hint(),
+                      "There should not be any animation restyle hints \
+                       during normal traversal");
+
+        // Else we should clear ourselves, and return the propagated hint.
+        mem::replace(self, Self::empty())
+            .propagate_for_non_animation_restyle()
+    }
+
     /// Returns a new `CascadeHint` appropriate for children of the current
     /// element.
-    pub fn propagate_for_non_animation_restyle(&self) -> Self {
+    fn propagate_for_non_animation_restyle(&self) -> Self {
         if self.contains(RESTYLE_DESCENDANTS) {
             return Self::restyle_subtree()
         }
@@ -86,13 +118,6 @@ impl RestyleHint {
         RESTYLE_SMIL | RESTYLE_CSS_ANIMATIONS | RESTYLE_CSS_TRANSITIONS
     }
 
-    /// Returns whether the hint specifies that some work must be performed on
-    /// the current element.
-    #[inline]
-    pub fn affects_self(&self) -> bool {
-        self.intersects(RESTYLE_SELF | RECASCADE_SELF | Self::replacements())
-    }
-
     /// Returns whether the hint specifies that the currently element must be
     /// recascaded.
     pub fn has_recascade_self(&self) -> bool {
@@ -145,6 +170,12 @@ impl RestyleHint {
     }
 }
 
+impl Default for RestyleHint {
+    fn default() -> Self {
+        Self::empty()
+    }
+}
+
 #[cfg(feature = "gecko")]
 impl From<nsRestyleHint> for RestyleHint {
     fn from(raw: nsRestyleHint) -> Self {

components/style/invalidation/stylesheets.rs

@@ -8,9 +8,9 @@
 #![deny(unsafe_code)]
 
 use Atom;
-use data::StoredRestyleHint;
 use dom::{TElement, TNode};
 use fnv::FnvHashSet;
+use invalidation::element::restyle_hints::RestyleHint;
 use selector_parser::SelectorImpl;
 use selectors::attr::CaseSensitivity;
 use selectors::parser::{Component, Selector};
@@ -134,7 +134,7 @@ impl StylesheetInvalidationSet {
             if self.fully_invalid {
                 debug!("process_invalidations: fully_invalid({:?})",
                        element);
-                data.ensure_restyle().hint.insert(StoredRestyleHint::subtree());
+                data.ensure_restyle().hint.insert(RestyleHint::restyle_subtree());
                 return true;
             }
         }
@@ -177,7 +177,7 @@ impl StylesheetInvalidationSet {
             if scope.matches(element) {
                 debug!("process_invalidations_in_subtree: {:?} matched {:?}",
                        element, scope);
-                data.ensure_restyle().hint.insert(StoredRestyleHint::subtree());
+                data.ensure_restyle().hint.insert(RestyleHint::restyle_subtree());
                 return true;
             }
         }

components/style/matching.rs

@@ -697,8 +697,8 @@ trait PrivateMatchMethods: TElement {
         // 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,

components/style/traversal.rs

@@ -6,7 +6,7 @@
 
 use atomic_refcell::AtomicRefCell;
 use context::{SharedStyleContext, StyleContext, ThreadLocalStyleContext};
-use data::{ElementData, ElementStyles, StoredRestyleHint};
+use data::{ElementData, ElementStyles};
 use dom::{DirtyDescendants, NodeInfo, OpaqueNode, TElement, TNode};
 use invalidation::element::restyle_hints::{RECASCADE_SELF, RECASCADE_DESCENDANTS, RestyleHint};
 use matching::{ChildCascadeRequirement, MatchMethods};
@@ -297,8 +297,8 @@ pub trait DomTraversal<E: TElement> : Sync {
             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())
+                    r.reconstructed_ancestor ||
-                        .contains(RestyleDamage::reconstruct())
+                    r.damage.contains(RestyleDamage::reconstruct())
                 });
 
                 let mut is_before_or_after_pseudo = false;
@@ -691,7 +691,7 @@ 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 mut propagated_hint = match data.get_restyle_mut() {
-        None => StoredRestyleHint::empty(),
+        None => RestyleHint::empty(),
         Some(r) => {
             debug_assert!(context.shared.traversal_flags.for_animation_only() ||
                           !r.hint.has_animation_hint(),
@@ -730,14 +730,16 @@ pub fn recalc_style_at<E, D>(traversal: &D,
                                           DontLog) &&
         (has_dirty_descendants_for_this_restyle ||
          !propagated_hint.is_empty()) {
-        let damage_handled = data.get_restyle().map_or(RestyleDamage::empty(), |r| {
+        let reconstructed_ancestor = data.get_restyle().map_or(false, |r| {
-            r.damage_handled() | r.damage.handled_for_descendants()
+            r.reconstructed_ancestor ||
+            r.damage.contains(RestyleDamage::reconstruct())
         });
-
+        preprocess_children::<E, D>(
-        preprocess_children::<E, D>(context,
+            context,
-                                    element,
+            element,
-                                    propagated_hint,
+            propagated_hint,
-                                    damage_handled);
+            reconstructed_ancestor,
+        )
     }
 
     // If we are in a restyle for reconstruction, drop the existing restyle
@@ -840,12 +842,15 @@ fn compute_style<E, D>(_traversal: &D,
     }
 }
 
-fn preprocess_children<E, D>(context: &mut StyleContext<E>,
+fn preprocess_children<E, D>(
-                             element: E,
+    context: &mut StyleContext<E>,
-                             propagated_hint: StoredRestyleHint,
+    element: E,
-                             damage_handled: RestyleDamage)
+    propagated_hint: RestyleHint,
-    where E: TElement,
+    reconstructed_ancestor: bool,
-          D: DomTraversal<E>,
+)
+where
+    E: TElement,
+    D: DomTraversal<E>,
 {
     trace!("preprocess_children: {:?}", element);
 
@@ -880,9 +885,7 @@ 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() &&
+        if !reconstructed_ancestor && propagated_hint.is_empty() && !child_data.has_restyle() {
-            damage_handled.is_empty() &&
-            !child_data.has_restyle() {
             continue;
         }
 
@@ -890,10 +893,8 @@ fn preprocess_children<E, D>(context: &mut StyleContext<E>,
 
         // 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);
     }
 }