style: Use RestyleDamage to determine whether we must continue cascading style changes to children. r?emilio draft
authorCameron McCormack <cam@mcc.id.au>
Mon, 15 May 2017 16:35:31 +0800
changeset 580308 fc5cc5167b096da17ae5ee0d1e6b9851f0fe98fa
parent 580307 783b3990df6d6df8d53d620c502ba47a557c755b
child 629236 cf6f30c09601d1d81dd2a4af627b514a2c2d7e6d
push id59502
push userbmo:cam@mcc.id.au
push dateThu, 18 May 2017 09:56:50 +0000
reviewersemilio
milestone55.0a1
style: Use RestyleDamage to determine whether we must continue cascading style changes to children. r?emilio MozReview-Commit-ID: 9kc1gWyB8Hj
servo/components/layout/animation.rs
servo/components/style/gecko/restyle_damage.rs
servo/components/style/matching.rs
servo/components/style/servo/restyle_damage.rs
servo/components/style/traversal.rs
--- a/servo/components/layout/animation.rs
+++ b/servo/components/layout/animation.rs
@@ -155,17 +155,19 @@ pub fn recalc_style_for_animations(conte
     flow.mutate_fragments(&mut |fragment| {
         if let Some(ref animations) = animations.get(&fragment.node) {
             for animation in animations.iter() {
                 let old_style = fragment.style.clone();
                 update_style_for_animation(&context.style_context,
                                            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;
             }
         }
     });
 
     let base = flow::mut_base(flow);
     base.restyle_damage.insert(damage);
     for kid in base.children.iter_mut() {
         recalc_style_for_animations(context, kid, animations)
--- a/servo/components/style/gecko/restyle_damage.rs
+++ b/servo/components/style/gecko/restyle_damage.rs
@@ -4,16 +4,17 @@
 
 //! Gecko's restyle damage computation (aka change hints, aka `nsChangeHint`).
 
 use gecko_bindings::bindings;
 use gecko_bindings::structs;
 use gecko_bindings::structs::{nsChangeHint, nsStyleContext};
 use gecko_bindings::sugar::ownership::FFIArcHelpers;
 use properties::ComputedValues;
+use matching::{StyleChange, StyleDifference};
 use std::ops::{BitAnd, BitOr, BitOrAssign, Not};
 use stylearc::Arc;
 
 /// The representation of Gecko's restyle damage is just a wrapper over
 /// `nsChangeHint`.
 #[derive(Clone, Copy, Debug, PartialEq)]
 pub struct GeckoRestyleDamage(nsChangeHint);
 
@@ -33,34 +34,37 @@ impl GeckoRestyleDamage {
         GeckoRestyleDamage(nsChangeHint(0))
     }
 
     /// Returns whether this restyle damage represents the empty damage.
     pub fn is_empty(&self) -> bool {
         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;
         let hint = unsafe {
             bindings::Gecko_CalcStyleDifference(context,
                                                 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|.
     pub fn contains(self, other: Self) -> bool {
         self & other == other
     }
 
     /// Gets restyle damage to reconstruct the entire frame, subsuming all
--- a/servo/components/style/matching.rs
+++ b/servo/components/style/matching.rs
@@ -46,16 +46,44 @@ enum InheritMode {
 fn relations_are_shareable(relations: &StyleRelations) -> bool {
     use selectors::matching::*;
     !relations.intersects(AFFECTED_BY_ID_SELECTOR |
                           AFFECTED_BY_PSEUDO_ELEMENTS |
                           AFFECTED_BY_STYLE_ATTRIBUTE |
                           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,
 /// and once here, the style information of the element is immutable, so it's
 /// safe to access.
 ///
 /// TODO: We can stick a lot more info here.
 #[derive(Debug)]
@@ -370,18 +398,47 @@ impl<E: TElement> StyleSharingCandidateC
     }
 }
 
 /// The results of attempting to share a style.
 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,
+        }
+    }
 }
 
 trait PrivateMatchMethods: TElement {
     /// Returns the closest parent element that doesn't have a display: contents
     /// style (and thus generates a box).
     ///
     /// This is needed to correctly handle blockification of flex and grid
     /// items.
@@ -531,17 +588,18 @@ trait PrivateMatchMethods: TElement {
                                 primary_style,
                                 inherit_mode)
     }
 
     /// Computes values and damage for the primary or pseudo style of an element,
     /// setting them on the ElementData.
     fn cascade_primary(&self,
                        context: &mut StyleContext<Self>,
-                       data: &mut ElementData) {
+                       data: &mut ElementData)
+                       -> ChildCascadeRequirement {
         // Collect some values.
         let (mut styles, restyle) = data.styles_and_restyle_mut();
         let mut primary_style = &mut styles.primary;
         let mut old_values = primary_style.values.take();
 
         // Compute the new values.
         let mut new_values = self.cascade_internal(context, primary_style, None);
 
@@ -549,50 +607,51 @@ trait PrivateMatchMethods: TElement {
         // traversing the pseudo-elements themselves.
         if !context.shared.traversal_flags.for_animation_only() {
             self.process_animations(context,
                                     &mut old_values,
                                     &mut new_values,
                                     primary_style);
         }
 
-        if let Some(old) = old_values {
+        let child_cascade_requirement =
             self.accumulate_damage(&context.shared,
                                    restyle.unwrap(),
-                                   &old,
+                                   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,
                             context: &mut StyleContext<Self>,
                             data: &mut ElementData,
                             pseudo: &PseudoElement) {
         debug_assert!(pseudo.is_eager());
         let (mut styles, restyle) = data.styles_and_restyle_mut();
         let mut pseudo_style = styles.pseudos.get_mut(pseudo).unwrap();
         let old_values = pseudo_style.values.take();
 
         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.
-            if cfg!(feature = "servo") || !pseudo.is_before_or_after() {
-                self.accumulate_damage(&context.shared,
-                                       restyle.unwrap(),
-                                       &old,
-                                       &new_values,
-                                       Some(pseudo));
-            }
+        // ::before and ::after are element-backed in Gecko, so they do
+        // the damage calculation for themselves.
+        if cfg!(feature = "servo") || !pseudo.is_before_or_after() {
+            self.accumulate_damage(&context.shared,
+                                   restyle.unwrap(),
+                                   old_values.as_ref().map(|v| v.as_ref()),
+                                   &new_values,
+                                   Some(pseudo));
         }
 
         pseudo_style.values = Some(new_values)
     }
 
 
     /// get_after_change_style removes the transition rules from the ComputedValues.
     /// If there is no transition rule in the ComputedValues, it returns None.
@@ -737,64 +796,75 @@ trait PrivateMatchMethods: TElement {
                 this_opaque,
                 &**values,
                 new_values,
                 &shared_context.timer,
                 &possibly_expired_animations);
         }
     }
 
-    /// 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,
+                         old_values: Option<&ComputedValues>,
                          new_values: &Arc<ComputedValues>,
-                         pseudo: Option<&PseudoElement>) {
+                         pseudo: Option<&PseudoElement>)
+                         -> ChildCascadeRequirement {
+        old_values.map_or(ChildCascadeRequirement::MustCascade, |old_values| {
+            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;
-            }
+        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")]
     fn update_animations_for_cascade(&self,
                                      context: &SharedStyleContext,
                                      style: &mut Arc<ComputedValues>,
                                      possibly_expired_animations: &mut Vec<::animation::PropertyAnimation>,
                                      font_metrics: &FontMetricsProvider) {
@@ -872,26 +942,26 @@ pub enum StyleSharingBehavior {
 
 /// The public API that elements expose for selector matching.
 pub trait MatchMethods : TElement {
     /// Performs selector matching and property cascading on an element and its
     /// eager pseudos.
     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 _rule_node_changed = self.match_primary(context,
                                                     data,
                                                     &mut relations);
 
         // Cascade properties and compute primary values.
-        self.cascade_primary(context, data);
+        let child_cascade_requirement = self.cascade_primary(context, data);
 
         // Match and cascade eager pseudo-elements.
         if !data.styles().is_display_none() {
             let _pseudo_rule_nodes_changed =
                 self.match_pseudos(context, data);
             self.cascade_pseudos(context, data);
         }
 
@@ -915,25 +985,28 @@ pub trait MatchMethods : TElement {
                                                     .take();
             context.thread_local
                    .style_sharing_candidate_cache
                    .insert_if_possible(self,
                                        data.styles().primary.values(),
                                        relations,
                                        revalidation_match_results);
         }
+
+        child_cascade_requirement
     }
 
     /// Performs the cascade, without matching.
     fn cascade_primary_and_pseudos(&self,
                                    context: &mut StyleContext<Self>,
-                                   mut data: &mut ElementData)
+                                   mut data: &mut ElementData) -> ChildCascadeRequirement
     {
-        self.cascade_primary(context, &mut data);
+        let child_cascade_requirement = self.cascade_primary(context, &mut data);
         self.cascade_pseudos(context, &mut data);
+        child_cascade_requirement
     }
 
     /// Runs selector matching to (re)compute the primary rule node for this element.
     ///
     /// Returns whether the primary rule node changed.
     fn match_primary(&self,
                      context: &mut StyleContext<Self>,
                      data: &mut ElementData,
@@ -1276,31 +1349,32 @@ pub trait MatchMethods : TElement {
             match sharing_result {
                 Ok(shared_style) => {
                     // Yay, cache hit. Share the style.
 
                     // Accumulate restyle damage.
                     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.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
                     // directly here.
                     //
                     // See https://bugzilla.mozilla.org/show_bug.cgi?id=1329361
                     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);
 
                     // Cache miss, let's see what kind of failure to decide
                     // whether we keep trying or not.
                     match miss {
                         // Cache miss because of parent, clear the candidate cache.
@@ -1374,42 +1448,45 @@ pub trait MatchMethods : TElement {
         self.each_class(|class| {
             bf.remove_hash(class.get_hash())
         });
     }
 
     /// 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
     {
         match self.existing_style_for_restyle_damage(old_values, pseudo) {
-            Some(ref source) => RestyleDamage::compute(source, new_values),
+            Some(ref source) => RestyleDamage::compute_style_difference(source, new_values),
             None => {
                 // If there's no style source, that likely means that Gecko
                 // couldn't find a style context. This happens with display:none
                 // elements, and probably a number of other edge cases that
                 // we don't handle well yet (like display:contents).
                 if new_values.get_box().clone_display() == display::T::none &&
                     old_values.get_box().clone_display() == display::T::none {
                     // The style remains display:none. No need for damage.
-                    RestyleDamage::empty()
+                    // (It doesn't matter what value we use for the StyleChange,
+                    // since we won't traverse into the display:none subtree
+                    // anyway.)
+                    StyleDifference::new(RestyleDamage::empty(), StyleChange::Unchanged)
                 } else {
                     // Something else. Be conservative for now.
-                    RestyleDamage::reconstruct()
+                    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)
     {
         // Note that we've already set up the map of matching pseudo-elements
         // in match_pseudos (and handled the damage implications of changing
         // which pseudos match), so now we can just iterate what we have. This
         // does mean collecting owned pseudos, so that the borrow checker will
--- a/servo/components/style/servo/restyle_damage.rs
+++ b/servo/components/style/servo/restyle_damage.rs
@@ -4,16 +4,17 @@
 
 //! The restyle damage is a hint that tells layout which kind of operations may
 //! be needed in presence of incremental style changes.
 
 #![deny(missing_docs)]
 
 use computed_values::display;
 use heapsize::HeapSizeOf;
+use matching::{StyleChange, StyleDifference};
 use properties::ServoComputedValues;
 use std::fmt;
 
 bitflags! {
     #[doc = "Individual layout actions that may be necessary after restyling."]
     pub flags ServoRestyleDamage: u8 {
         #[doc = "Repaint the node itself."]
         #[doc = "Currently unused; need to decide how this propagates."]
@@ -52,22 +53,24 @@ bitflags! {
     }
 }
 
 impl HeapSizeOf for ServoRestyleDamage {
     fn heap_size_of_children(&self) -> usize { 0 }
 }
 
 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
     /// reflowed.
     ///
     /// FIXME(bholley): Do we ever actually need this? Shouldn't
     /// RECONSTRUCT_FLOW imply everything else?
     pub fn rebuild_and_reflow() -> ServoRestyleDamage {
--- a/servo/components/style/traversal.rs
+++ b/servo/components/style/traversal.rs
@@ -3,17 +3,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 //! Traversing the DOM tree; the bloom filter.
 
 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;
 use std::borrow::BorrowMut;
 
 /// A per-traversal-level chunk of data. This is sent down by the traversal, and
 /// currently only holds the dom depth for the bloom filter.
 ///
@@ -606,28 +606,30 @@ pub fn recalc_style_at<E, D>(traversal: 
     let compute_self = !element.has_current_styles(data);
     let mut inherited_style_changed = false;
 
     debug!("recalc_style_at: {:?} (compute_self={:?}, dirty_descendants={:?}, data={:?})",
            element, compute_self, element.has_dirty_descendants(), data);
 
     // 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.
         if data.styles().is_display_none() {
             debug!("{:?} style is display:none - clearing data from descendants.",
                    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
     // those operations and compute the propagated restyle hint.
     let propagated_hint = match data.get_restyle_mut() {
         None => StoredRestyleHint::empty(),
         Some(r) => {
             debug_assert!(context.shared.traversal_flags.for_animation_only() ||
@@ -706,60 +708,60 @@ pub fn recalc_style_at<E, D>(traversal: 
         unsafe { element.unset_dirty_descendants(); }
     }
 }
 
 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>,
 {
     use data::RestyleKind::*;
     use matching::StyleSharingResult::*;
 
     context.thread_local.statistics.elements_styled += 1;
     let kind = data.restyle_kind();
 
     // First, try the style sharing cache. If we get a match we can skip the rest
     // of the work.
     if let MatchAndCascade = kind {
         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;
         }
     }
 
     match kind {
         MatchAndCascade => {
             // Ensure the bloom filter is up to date.
             context.thread_local.bloom_filter
                    .insert_parents_recovering(element, traversal_data.current_dom_depth);
 
             context.thread_local.bloom_filter.assert_complete(element);
             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 _rule_nodes_changed =
                 element.replace_rules(hint, context, &mut data);
-            element.cascade_primary_and_pseudos(context, &mut data);
+            element.cascade_primary_and_pseudos(context, &mut data)
         }
         CascadeOnly => {
-            element.cascade_primary_and_pseudos(context, &mut data);
+            element.cascade_primary_and_pseudos(context, &mut data)
         }
-    };
+    }
 }
 
 fn preprocess_children<E, D>(traversal: &D,
                              element: E,
                              mut propagated_hint: StoredRestyleHint,
                              damage_handled: RestyleDamage,
                              parent_inherited_style_changed: bool)
     where E: TElement,