Bug 1368236: Kill StoredRestyleHint. r?bholley draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Tue, 13 Jun 2017 12:45:04 +0200
changeset 593574 6025fdb27f8a381d0c30c87118977304b648b006
parent 593423 0ea31832ab86804b68d0e0a6f4ac3484d657f95d
child 593575 8c622cfa9d8b75e444be10f5ce692018a9a120d3
push id63741
push userbmo:emilio+bugs@crisal.io
push dateTue, 13 Jun 2017 21:27:09 +0000
reviewersbholley
bugs1368236
milestone56.0a1
Bug 1368236: Kill StoredRestyleHint. r?bholley MozReview-Commit-ID: 43Cf0rJyhzO
servo/components/style/data.rs
servo/components/style/invalidation/element/restyle_hints.rs
servo/components/style/invalidation/stylesheets.rs
servo/components/style/traversal.rs
--- a/servo/components/style/data.rs
+++ b/servo/components/style/data.rs
@@ -11,17 +11,16 @@ use invalidation::element::restyle_hints
 use properties::{AnimationRules, ComputedValues, PropertyDeclarationBlock};
 use properties::longhands::display::computed_value as display;
 use rule_tree::StrongRuleNode;
 use selector_parser::{EAGER_PSEUDO_COUNT, PseudoElement, RestyleDamage};
 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,
 /// and the result of computing that rule node's rules, the `ComputedValues`.
 #[derive(Clone)]
 pub struct ComputedStyle {
     /// The rule node representing the ordered list of rules matched for this
     /// node.
@@ -344,115 +343,24 @@ impl ElementStyles {
     }
 
     /// Whether this element `display` value is `none`.
     pub fn is_display_none(&self) -> bool {
         self.primary.values().get_box().clone_display() == display::T::none
     }
 }
 
-/// 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.
 #[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,
+    pub hint: RestyleHint,
 
     /// 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.
@@ -578,17 +486,17 @@ impl ElementData {
                       "Should've stopped earlier");
         if !self.has_styles() {
             return RestyleKind::MatchAndCascade;
         }
 
         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 hint.match_self() {
             return RestyleKind::MatchAndCascade;
         }
 
         if hint.has_replacements() {
             return RestyleKind::CascadeWithReplacements(hint & RestyleHint::replacements());
         }
 
--- a/servo/components/style/invalidation/element/restyle_hints.rs
+++ b/servo/components/style/invalidation/element/restyle_hints.rs
@@ -1,16 +1,17 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 //! Restyle hints: an optimization to avoid unnecessarily matching selectors.
 
 #[cfg(feature = "gecko")]
 use gecko_bindings::structs::nsRestyleHint;
+use traversal::TraversalFlags;
 
 bitflags! {
     /// The kind of restyle we need to do for a given element.
     pub flags RestyleHint: u8 {
         /// Do a selector match of the element.
         const RESTYLE_SELF = 1 << 0,
 
         /// Do a selector match of the element's descendants.
@@ -52,19 +53,50 @@ impl RestyleHint {
     }
 
     /// Creates a new `RestyleHint` indicating that the current element and all
     /// its descendants must be recascaded.
     pub fn recascade_subtree() -> Self {
         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()
         }
         if self.contains(RECASCADE_DESCENDANTS) {
             return Self::recascade_subtree()
         }
         Self::empty()
     }
@@ -81,23 +113,16 @@ impl RestyleHint {
     }
 
     /// The replacements for the animation cascade levels.
     #[inline]
     pub fn for_animations() -> Self {
         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 {
         self.contains(RECASCADE_SELF)
     }
 
     /// Returns whether the hint specifies that an animation cascade level must
     /// be replaced.
@@ -140,16 +165,22 @@ impl RestyleHint {
         // 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.remove(RECASCADE_SELF);
     }
 }
 
+impl Default for RestyleHint {
+    fn default() -> Self {
+        Self::empty()
+    }
+}
+
 #[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;
--- a/servo/components/style/invalidation/stylesheets.rs
+++ b/servo/components/style/invalidation/stylesheets.rs
@@ -3,19 +3,19 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 //! A collection of invalidations due to changes in which stylesheets affect a
 //! document.
 
 #![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};
 use shared_lock::SharedRwLockReadGuard;
 use stylesheets::{CssRule, Stylesheet};
 use stylist::Stylist;
 
 /// An invalidation scope represents a kind of subtree that may need to be
@@ -129,17 +129,17 @@ impl StylesheetInvalidationSet {
             let mut data = match element.mutate_data() {
                 Some(data) => data,
                 None => return false,
             };
 
             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;
             }
         }
 
         if self.invalid_scopes.is_empty() {
             debug!("process_invalidations: empty invalidation set");
             return false;
         }
@@ -172,17 +172,17 @@ impl StylesheetInvalidationSet {
                 return false;
             }
         }
 
         for scope in &self.invalid_scopes {
             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;
             }
         }
 
 
         let mut any_children_invalid = false;
 
         for child in element.as_node().traversal_children() {
--- a/servo/components/style/traversal.rs
+++ b/servo/components/style/traversal.rs
@@ -1,17 +1,17 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 //! Traversing the DOM tree; the bloom filter.
 
 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};
 use selector_parser::RestyleDamage;
 use sharing::{StyleSharingBehavior, StyleSharingTarget};
 #[cfg(feature = "servo")] use servo_config::opts;
 use smallvec::SmallVec;
 use std::borrow::BorrowMut;
@@ -686,17 +686,17 @@ pub fn recalc_style_at<E, D>(traversal: 
                    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 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(),
                           "animation restyle hint should be handled during \
                            animation-only restyles");
             r.hint.propagate(&context.shared.traversal_flags)
         },
     };
@@ -832,17 +832,17 @@ fn compute_style<E, D>(_traversal: &D,
                 /* important_rules_changed = */ false
             )
         }
     }
 }
 
 fn preprocess_children<E, D>(context: &mut StyleContext<E>,
                              element: E,
-                             propagated_hint: StoredRestyleHint,
+                             propagated_hint: RestyleHint,
                              damage_handled: RestyleDamage)
     where E: TElement,
           D: DomTraversal<E>,
 {
     trace!("preprocess_children: {:?}", element);
 
     // Loop over all the traversal children.
     for child in element.as_node().traversal_children() {