style: Use RestyleDamage to determine whether we must continue cascading style changes to children.

5 files changed, 221 insertions, 109 deletions

diff --git a/components/layout/animation.rs b/components/layout/animation.rs
index d1673ac44a1..a054927190c 100644
--- a/components/layout/animation.rs
+++ b/components/layout/animation.rs
@@ -160,7 +160,9 @@ pub fn recalc_style_for_animations(context: &LayoutContext,
                                            animation,
                                            &mut fragment.style,
                                            &ServoMetricsProvider);
-                damage |= RestyleDamage::compute(&old_style, &fragment.style);
+                let difference =
+                    RestyleDamage::compute_style_difference(&old_style, &fragment.style);
+                damage |= difference.damage;
             }
         }
     });
diff --git a/components/style/gecko/restyle_damage.rs b/components/style/gecko/restyle_damage.rs
index 7cb66470403..9d443b23baf 100644
--- a/components/style/gecko/restyle_damage.rs
+++ b/components/style/gecko/restyle_damage.rs
@@ -8,6 +8,7 @@ use gecko_bindings::bindings;
 use gecko_bindings::structs;
 use gecko_bindings::structs::{nsChangeHint, nsStyleContext};
 use gecko_bindings::sugar::ownership::FFIArcHelpers;
+use matching::{StyleChange, StyleDifference};
 use properties::ComputedValues;
 use std::ops::{BitAnd, BitOr, BitOrAssign, Not};
 use stylearc::Arc;
@@ -38,15 +39,17 @@ impl GeckoRestyleDamage {
         self.0 == nsChangeHint(0)
     }
 
-    /// Computes a change hint given an old style (in the form of a
-    /// `nsStyleContext`, and a new style (in the form of `ComputedValues`).
+    /// Computes the `StyleDifference` (including the appropriate change hint)
+    /// given an old style (in the form of a `nsStyleContext`, and a new style
+    /// (in the form of `ComputedValues`).
     ///
     /// Note that we could in theory just get two `ComputedValues` here and diff
     /// them, but Gecko has an interesting optimization when they mark accessed
     /// structs, so they effectively only diff structs that have ever been
     /// accessed from layout.
-    pub fn compute(source: &nsStyleContext,
-                   new_style: &Arc<ComputedValues>) -> Self {
+    pub fn compute_style_difference(source: &nsStyleContext,
+                                    new_style: &Arc<ComputedValues>)
+                                    -> StyleDifference {
         // TODO(emilio): Const-ify this?
         let context = source as *const nsStyleContext as *mut nsStyleContext;
         let mut any_style_changed: bool = false;
@@ -55,7 +58,8 @@ impl GeckoRestyleDamage {
                                                 new_style.as_borrowed_opt().unwrap(),
                                                 &mut any_style_changed)
         };
-        GeckoRestyleDamage(hint)
+        let change = if any_style_changed { StyleChange::Changed } else { StyleChange::Unchanged };
+        StyleDifference::new(GeckoRestyleDamage(hint), change)
     }
 
     /// Returns true if this restyle damage contains all the damage of |other|.
diff --git a/components/style/matching.rs b/components/style/matching.rs
index 9b1711f3d48..0af85c7d0e3 100644
--- a/components/style/matching.rs
+++ b/components/style/matching.rs
@@ -51,6 +51,34 @@ fn relations_are_shareable(relations: &StyleRelations) -> bool {
                           AFFECTED_BY_PRESENTATIONAL_HINTS)
 }
 
+/// Represents the result of comparing an element's old and new style.
+pub struct StyleDifference {
+    /// The resulting damage.
+    pub damage: RestyleDamage,
+
+    /// Whether any styles changed.
+    pub change: StyleChange,
+}
+
+impl StyleDifference {
+    /// Creates a new `StyleDifference`.
+    pub fn new(damage: RestyleDamage, change: StyleChange) -> Self {
+        StyleDifference {
+            change: change,
+            damage: damage,
+        }
+    }
+}
+
+/// Represents whether or not the style of an element has changed.
+#[derive(Copy, Clone)]
+pub enum StyleChange {
+    /// The style hasn't changed.
+    Unchanged,
+    /// The style has changed.
+    Changed,
+}
+
 /// Information regarding a style sharing candidate.
 ///
 /// Note that this information is stored in TLS and cleared after the traversal,
@@ -375,8 +403,37 @@ pub enum StyleSharingResult {
     /// We didn't find anybody to share the style with.
     CannotShare,
     /// The node's style can be shared. The integer specifies the index in the
-    /// LRU cache that was hit and the damage that was done.
-    StyleWasShared(usize),
+    /// LRU cache that was hit and the damage that was done. The
+    /// `ChildCascadeRequirement` indicates whether style changes due to using
+    /// the shared style mean we need to recascade to children.
+    StyleWasShared(usize, ChildCascadeRequirement),
+}
+
+/// Whether or not newly computed values for an element need to be cascade
+/// to children.
+pub enum ChildCascadeRequirement {
+    /// Old and new computed values were the same, or we otherwise know that
+    /// we won't bother recomputing style for children, so we can skip cascading
+    /// the new values into child elements.
+    CanSkipCascade,
+    /// Old and new computed values were different, so we must cascade the
+    /// new values to children.
+    ///
+    /// FIXME(heycam) Although this is "must" cascade, in the future we should
+    /// track whether child elements rely specifically on inheriting particular
+    /// property values.  When we do that, we can treat `MustCascade` as "must
+    /// cascade unless we know that changes to these properties can be
+    /// ignored".
+    MustCascade,
+}
+
+impl From<StyleChange> for ChildCascadeRequirement {
+    fn from(change: StyleChange) -> ChildCascadeRequirement {
+        match change {
+            StyleChange::Unchanged => ChildCascadeRequirement::CanSkipCascade,
+            StyleChange::Changed => ChildCascadeRequirement::MustCascade,
+        }
+    }
 }
 
 /// The result status for match primary rules.
@@ -570,7 +627,8 @@ trait PrivateMatchMethods: TElement {
     fn cascade_primary(&self,
                        context: &mut StyleContext<Self>,
                        data: &mut ElementData,
-                       important_rules_changed: bool) {
+                       important_rules_changed: bool)
+                       -> ChildCascadeRequirement {
         // Collect some values.
         let (mut styles, restyle) = data.styles_and_restyle_mut();
         let mut primary_style = &mut styles.primary;
@@ -589,16 +647,19 @@ trait PrivateMatchMethods: TElement {
                                     important_rules_changed);
         }
 
-        if let Some(old) = old_values {
+        let child_cascade_requirement =
             self.accumulate_damage(&context.shared,
-                                   restyle.unwrap(),
-                                   &old,
+                                   restyle,
+                                   old_values.as_ref().map(|v| v.as_ref()),
                                    &new_values,
                                    None);
-        }
 
         // Set the new computed values.
         primary_style.values = Some(new_values);
+
+        // Return whether the damage indicates we must cascade new inherited
+        // values into children.
+        child_cascade_requirement
     }
 
     fn cascade_eager_pseudo(&self,
@@ -613,29 +674,11 @@ trait PrivateMatchMethods: TElement {
         let new_values =
             self.cascade_internal(context, &styles.primary, Some(pseudo_style));
 
-        if let Some(old) = old_values {
-            // ::before and ::after are element-backed in Gecko, so they do the
-            // damage calculation for themselves, when there's an actual pseudo.
-            //
-            // NOTE(emilio): An alternative to this check is to just remove the
-            // pseudo-style entry here if new_values is display: none or has
-            // an ineffective content property...
-            //
-            // I think it's nice to handle it here instead for symmetry with how
-            // we handle display: none elements, but the other approach may be
-            // ok too?
-            let is_unexisting_before_or_after =
-                pseudo.is_before_or_after() &&
-                self.existing_style_for_restyle_damage(&old, Some(pseudo)).is_none();
-
-            if cfg!(feature = "servo") || is_unexisting_before_or_after {
-                self.accumulate_damage(&context.shared,
-                                       restyle.unwrap(),
-                                       &old,
-                                       &new_values,
-                                       Some(pseudo));
-            }
-        }
+        self.accumulate_damage(&context.shared,
+                               restyle,
+                               old_values.as_ref().map(|v| &**v),
+                               &new_values,
+                               Some(pseudo));
 
         pseudo_style.values = Some(new_values)
     }
@@ -794,54 +837,89 @@ trait PrivateMatchMethods: TElement {
         }
     }
 
-    /// Computes and applies non-redundant damage.
-    #[cfg(feature = "gecko")]
+    /// Computes and applies restyle damage.
     fn accumulate_damage(&self,
                          shared_context: &SharedStyleContext,
-                         restyle: &mut RestyleData,
-                         old_values: &ComputedValues,
+                         restyle: Option<&mut RestyleData>,
+                         old_values: Option<&ComputedValues>,
                          new_values: &Arc<ComputedValues>,
-                         pseudo: Option<&PseudoElement>) {
+                         pseudo: Option<&PseudoElement>)
+                         -> ChildCascadeRequirement {
+        let restyle = match restyle {
+            Some(r) => r,
+            None => return ChildCascadeRequirement::MustCascade,
+        };
+
+        let old_values = match old_values {
+            Some(v) => v,
+            None => return ChildCascadeRequirement::MustCascade,
+        };
+
+        // ::before and ::after are element-backed in Gecko, so they do the
+        // damage calculation for themselves, when there's an actual pseudo.
+        let is_existing_before_or_after =
+            cfg!(feature = "gecko") &&
+            pseudo.map_or(false, |p| p.is_before_or_after()) &&
+            self.existing_style_for_restyle_damage(old_values, pseudo)
+                .is_some();
+
+        if is_existing_before_or_after {
+            return ChildCascadeRequirement::CanSkipCascade;
+        }
+
+        self.accumulate_damage_for(shared_context,
+                                   restyle,
+                                   old_values,
+                                   new_values,
+                                   pseudo)
+    }
+
+    /// Computes and applies non-redundant damage.
+    #[cfg(feature = "gecko")]
+    fn accumulate_damage_for(&self,
+                             shared_context: &SharedStyleContext,
+                             restyle: &mut RestyleData,
+                             old_values: &ComputedValues,
+                             new_values: &Arc<ComputedValues>,
+                             pseudo: Option<&PseudoElement>)
+                             -> ChildCascadeRequirement {
         // Don't accumulate damage if we're in a restyle for reconstruction.
         if shared_context.traversal_flags.for_reconstruct() {
-            return;
+            return ChildCascadeRequirement::MustCascade;
         }
 
         // If an ancestor is already getting reconstructed by Gecko's top-down
-        // frame constructor, no need to apply damage.
-        if restyle.damage_handled.contains(RestyleDamage::reconstruct()) {
-            restyle.damage = RestyleDamage::empty();
-            return;
-        }
-
-        // Add restyle damage, but only the bits that aren't redundant with
-        // respect to damage applied on our ancestors.
+        // 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.
-        if !restyle.damage.contains(RestyleDamage::reconstruct()) {
-            let new_damage = self.compute_restyle_damage(&old_values,
-                                                         &new_values,
-                                                         pseudo);
-            if !restyle.damage_handled.contains(new_damage) {
-                restyle.damage |= new_damage;
-            }
+        // 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());
+
+        let difference = self.compute_style_difference(&old_values,
+                                                       &new_values,
+                                                       pseudo);
+        if !skip_applying_damage {
+            restyle.damage |= difference.damage;
         }
+        difference.change.into()
     }
 
     /// Computes and applies restyle damage unless we've already maxed it out.
     #[cfg(feature = "servo")]
-    fn accumulate_damage(&self,
-                         _shared_context: &SharedStyleContext,
-                         restyle: &mut RestyleData,
-                         old_values: &ComputedValues,
-                         new_values: &Arc<ComputedValues>,
-                         pseudo: Option<&PseudoElement>) {
-        if restyle.damage != RestyleDamage::rebuild_and_reflow() {
-            restyle.damage |=
-                self.compute_restyle_damage(&old_values, &new_values, pseudo);
-        }
+    fn accumulate_damage_for(&self,
+                             _shared_context: &SharedStyleContext,
+                             restyle: &mut RestyleData,
+                             old_values: &ComputedValues,
+                             new_values: &Arc<ComputedValues>,
+                             pseudo: Option<&PseudoElement>)
+                             -> ChildCascadeRequirement {
+        let difference = self.compute_style_difference(&old_values, &new_values, pseudo);
+        restyle.damage |= difference.damage;
+        difference.change.into()
     }
 
     #[cfg(feature = "servo")]
@@ -929,14 +1007,19 @@ pub trait MatchMethods : TElement {
     fn match_and_cascade(&self,
                          context: &mut StyleContext<Self>,
                          data: &mut ElementData,
-                         sharing: StyleSharingBehavior)
+                         sharing: StyleSharingBehavior) -> ChildCascadeRequirement
     {
         // Perform selector matching for the primary style.
         let mut relations = StyleRelations::empty();
         let result = self.match_primary(context, data, &mut relations);
 
         // Cascade properties and compute primary values.
-        self.cascade_primary(context, data, result.important_rules_overriding_animation_changed);
+        let child_cascade_requirement =
+            self.cascade_primary(
+                context,
+                data,
+                result.important_rules_overriding_animation_changed
+            );
 
         // Match and cascade eager pseudo-elements.
         if !data.styles().is_display_none() {
@@ -969,6 +1052,8 @@ pub trait MatchMethods : TElement {
                                        relations,
                                        revalidation_match_results);
         }
+
+        child_cascade_requirement
     }
 
     /// Performs the cascade, without matching.
@@ -976,9 +1061,12 @@ pub trait MatchMethods : TElement {
                                    context: &mut StyleContext<Self>,
                                    mut data: &mut ElementData,
                                    important_rules_changed: bool)
+                                   -> ChildCascadeRequirement
     {
-        self.cascade_primary(context, &mut data, important_rules_changed);
+        let child_cascade_requirement =
+            self.cascade_primary(context, &mut data, important_rules_changed);
         self.cascade_pseudos(context, &mut data);
+        child_cascade_requirement
     }
 
     /// Runs selector matching to (re)compute the primary rule node for this element.
@@ -1354,11 +1442,12 @@ pub trait MatchMethods : TElement {
                     debug_assert_eq!(data.has_styles(), data.has_restyle());
                     let old_values = data.get_styles_mut()
                                          .and_then(|s| s.primary.values.take());
-                    if let Some(old) = old_values {
+                    let child_cascade_requirement =
                         self.accumulate_damage(&context.shared,
-                                               data.restyle_mut(), &old,
-                                               shared_style.values(), None);
-                    }
+                                               data.get_restyle_mut(),
+                                               old_values.as_ref().map(|v| v.as_ref()),
+                                               shared_style.values(),
+                                               None);
 
                     // We never put elements with pseudo style into the style
                     // sharing cache, so we can just mint an ElementStyles
@@ -1368,7 +1457,7 @@ pub trait MatchMethods : TElement {
                     let styles = ElementStyles::new(shared_style);
                     data.set_styles(styles);
 
-                    return StyleSharingResult::StyleWasShared(i)
+                    return StyleSharingResult::StyleWasShared(i, child_cascade_requirement)
                 }
                 Err(miss) => {
                     debug!("Cache miss: {:?}", miss);
@@ -1452,14 +1541,14 @@ pub trait MatchMethods : TElement {
     /// Given the old and new style of this element, and whether it's a
     /// pseudo-element, compute the restyle damage used to determine which
     /// kind of layout or painting operations we'll need.
-    fn compute_restyle_damage(&self,
-                              old_values: &ComputedValues,
-                              new_values: &Arc<ComputedValues>,
-                              pseudo: Option<&PseudoElement>)
-                              -> RestyleDamage
+    fn compute_style_difference(&self,
+                                old_values: &ComputedValues,
+                                new_values: &Arc<ComputedValues>,
+                                pseudo: Option<&PseudoElement>)
+                                -> StyleDifference
     {
         if let Some(source) = self.existing_style_for_restyle_damage(old_values, pseudo) {
-            return RestyleDamage::compute(source, new_values);
+            return RestyleDamage::compute_style_difference(source, new_values)
         }
 
         let new_style_is_display_none =
@@ -1474,7 +1563,7 @@ pub trait MatchMethods : TElement {
         // pseudo-elements.
         if new_style_is_display_none && old_style_is_display_none {
             // The style remains display:none. No need for damage.
-            return RestyleDamage::empty()
+            return StyleDifference::new(RestyleDamage::empty(), StyleChange::Unchanged)
         }
 
         if pseudo.map_or(false, |p| p.is_before_or_after()) {
@@ -1484,18 +1573,19 @@ pub trait MatchMethods : TElement {
                 new_values.ineffective_content_property()) {
                 // The pseudo-element will remain undisplayed, so just avoid
                 // triggering any change.
-                return RestyleDamage::empty()
+                return StyleDifference::new(RestyleDamage::empty(), StyleChange::Unchanged)
             }
-            return RestyleDamage::reconstruct()
+            return StyleDifference::new(RestyleDamage::reconstruct(), StyleChange::Changed)
         }
 
         // Something else. Be conservative for now.
         warn!("Reframing due to lack of old style source: {:?}, pseudo: {:?}",
                self, pseudo);
-        RestyleDamage::reconstruct()
+        // Something else. Be conservative for now.
+        StyleDifference::new(RestyleDamage::reconstruct(), StyleChange::Changed)
     }
 
-    /// Cascade the eager pseudo-elements of this element.
+    /// Performs the cascade for the element's eager pseudos.
     fn cascade_pseudos(&self,
                        context: &mut StyleContext<Self>,
                        mut data: &mut ElementData)
diff --git a/components/style/servo/restyle_damage.rs b/components/style/servo/restyle_damage.rs
index 69199d6e982..59a03a3f085 100644
--- a/components/style/servo/restyle_damage.rs
+++ b/components/style/servo/restyle_damage.rs
@@ -9,6 +9,7 @@
 
 use computed_values::display;
 use heapsize::HeapSizeOf;
+use matching::{StyleChange, StyleDifference};
 use properties::ServoComputedValues;
 use std::fmt;
 
@@ -57,12 +58,14 @@ impl HeapSizeOf for ServoRestyleDamage {
 }
 
 impl ServoRestyleDamage {
-    /// Compute the appropriate restyle damage for a given style change between
-    /// `old` and `new`.
-    pub fn compute(old: &ServoComputedValues,
-                   new: &ServoComputedValues)
-                   -> ServoRestyleDamage {
-        compute_damage(old, new)
+    /// Compute the `StyleDifference` (including the appropriate restyle damage)
+    /// for a given style change between `old` and `new`.
+    pub fn compute_style_difference(old: &ServoComputedValues,
+                                    new: &ServoComputedValues)
+                                    -> StyleDifference {
+        let damage = compute_damage(old, new);
+        let change = if damage.is_empty() { StyleChange::Unchanged } else { StyleChange::Changed };
+        StyleDifference::new(damage, change)
     }
 
     /// Returns a bitmask that represents a flow that needs to be rebuilt and
diff --git a/components/style/traversal.rs b/components/style/traversal.rs
index d91b81149fb..ab12776eeb8 100644
--- a/components/style/traversal.rs
+++ b/components/style/traversal.rs
@@ -8,7 +8,7 @@ use atomic_refcell::{AtomicRefCell, AtomicRefMut};
 use context::{SharedStyleContext, StyleContext, ThreadLocalStyleContext};
 use data::{ElementData, ElementStyles, StoredRestyleHint};
 use dom::{DirtyDescendants, NodeInfo, OpaqueNode, TElement, TNode};
-use matching::{MatchMethods, StyleSharingBehavior};
+use matching::{ChildCascadeRequirement, MatchMethods, StyleSharingBehavior};
 use restyle_hints::{RESTYLE_DESCENDANTS, RESTYLE_SELF};
 use selector_parser::RestyleDamage;
 #[cfg(feature = "servo")] use servo_config::opts;
@@ -611,7 +611,12 @@ pub fn recalc_style_at<E, D>(traversal: &D,
 
     // Compute style for this element if necessary.
     if compute_self {
-        compute_style(traversal, traversal_data, context, element, &mut data);
+        match compute_style(traversal, traversal_data, context, element, &mut data) {
+            ChildCascadeRequirement::MustCascade => {
+                inherited_style_changed = true;
+            }
+            ChildCascadeRequirement::CanSkipCascade => {}
+        };
 
         // If we're restyling this element to display:none, throw away all style
         // data in the subtree, notify the caller to early-return.
@@ -620,9 +625,6 @@ pub fn recalc_style_at<E, D>(traversal: &D,
                    element);
             clear_descendant_data(element, &|e| unsafe { D::clear_element_data(&e) });
         }
-
-        // FIXME(bholley): Compute this accurately from the call to CalcStyleDifference.
-        inherited_style_changed = true;
     }
 
     // Now that matching and cascading is done, clear the bits corresponding to
@@ -711,7 +713,7 @@ fn compute_style<E, D>(_traversal: &D,
                        traversal_data: &PerLevelTraversalData,
                        context: &mut StyleContext<E>,
                        element: E,
-                       mut data: &mut AtomicRefMut<ElementData>)
+                       mut data: &mut AtomicRefMut<ElementData>) -> ChildCascadeRequirement
     where E: TElement,
           D: DomTraversal<E>,
 {
@@ -727,10 +729,10 @@ fn compute_style<E, D>(_traversal: &D,
         let sharing_result = unsafe {
             element.share_style_if_possible(context, &mut data)
         };
-        if let StyleWasShared(index) = sharing_result {
+        if let StyleWasShared(index, had_damage) = sharing_result {
             context.thread_local.statistics.styles_shared += 1;
             context.thread_local.style_sharing_candidate_cache.touch(index);
-            return;
+            return had_damage;
         }
     }
 
@@ -744,17 +746,28 @@ fn compute_style<E, D>(_traversal: &D,
             context.thread_local.statistics.elements_matched += 1;
 
             // Perform the matching and cascading.
-            element.match_and_cascade(context, &mut data, StyleSharingBehavior::Allow);
+            element.match_and_cascade(
+                context,
+                &mut data,
+                StyleSharingBehavior::Allow
+            )
         }
-        CascadeWithReplacements(hint) => {
-            let rules_changed = element.replace_rules(hint, context, &mut data);
-            element.cascade_primary_and_pseudos(context, &mut data,
-                                                rules_changed.important_rules_changed());
+        CascadeWithReplacements(flags) => {
+            let rules_changed = element.replace_rules(flags, context, &mut data);
+            element.cascade_primary_and_pseudos(
+                context,
+                &mut data,
+                rules_changed.important_rules_changed()
+            )
         }
         CascadeOnly => {
-            element.cascade_primary_and_pseudos(context, &mut data, false);
+            element.cascade_primary_and_pseudos(
+                context,
+                &mut data,
+                /* important_rules_changed = */ false
+            )
         }
-    };
+    }
 }
 
 fn preprocess_children<E, D>(traversal: &D,