Auto merge of #16909 - emilio:simplify-cascade, r=bholley

style: Refactor the cascade function.

The `cascade_primary_or_pseudo` function was nice when we shared more code, but
right now I think it just makes it harder to understand what's going on.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16909)
<!-- Reviewable:end -->

components/script/layout_wrapper.rs

@@ -404,9 +404,9 @@ impl<'le> TElement for ServoLayoutElement<'le> {
 
     #[inline]
     fn existing_style_for_restyle_damage<'a>(&'a self,
-                                             current_cv: &'a Arc<ComputedValues>,
+                                             current_cv: &'a ComputedValues,
                                              _pseudo_element: Option<&PseudoElement>)
-                                             -> Option<&'a Arc<ComputedValues>> {
+                                             -> Option<&'a ComputedValues> {
         Some(current_cv)
     }
 

components/style/dom.rs

@@ -389,7 +389,7 @@ pub trait TElement : Eq + PartialEq + Debug + Hash + Sized + Copy + Clone +
     /// values as an argument here, but otherwise Servo would crash due to
     /// double borrows to return it.
     fn existing_style_for_restyle_damage<'a>(&'a self,
-                                             current_computed_values: &'a Arc<ComputedValues>,
+                                             current_computed_values: &'a ComputedValues,
                                              pseudo: Option<&PseudoElement>)
                                              -> Option<&'a PreExistingComputedValues>;
 

components/style/gecko/wrapper.rs

@@ -624,7 +624,7 @@ impl<'le> TElement for GeckoElement<'le> {
     }
 
     fn existing_style_for_restyle_damage<'a>(&'a self,
-                                             _existing_values: &'a Arc<ComputedValues>,
+                                             _existing_values: &'a ComputedValues,
                                              pseudo: Option<&PseudoElement>)
                                              -> Option<&'a nsStyleContext> {
         // TODO(emilio): Migrate this to CSSPseudoElementType.

components/style/matching.rs

@@ -496,6 +496,27 @@ trait PrivateMatchMethods: TElement {
                         primary_style: &ComputedStyle,
                         eager_pseudo_style: Option<&ComputedStyle>)
                         -> Arc<ComputedValues> {
+        if let Some(pseudo) = self.implemented_pseudo_element() {
+            debug_assert!(eager_pseudo_style.is_none());
+
+            // This is an element-backed pseudo, just grab the styles from the
+            // parent if it's eager, and recascade otherwise.
+            //
+            // We also recascade if the eager pseudo-style has any animation
+            // rules, because we don't cascade those during the eager traversal.
+            //
+            // We could make that a bit better if the complexity cost is not too
+            // big, but given further restyles are posted directly to
+            // pseudo-elements, it doesn't seem worth the effort at a glance.
+            if pseudo.is_eager() && self.get_animation_rules().is_empty() {
+                let parent = self.parent_element().unwrap();
+                let parent_data = parent.borrow_data().unwrap();
+                let pseudo_style =
+                    parent_data.styles().pseudos.get(&pseudo).unwrap();
+                return pseudo_style.values().clone()
+            }
+        }
+
         // Grab the rule node.
         let rule_node = &eager_pseudo_style.unwrap_or(primary_style).rules;
         let inherit_mode = if eager_pseudo_style.is_some() {
@@ -513,97 +534,66 @@ trait PrivateMatchMethods: TElement {
 
     /// Computes values and damage for the primary or pseudo style of an element,
     /// setting them on the ElementData.
-    fn cascade_primary_or_pseudo(&self,
+    fn cascade_primary(&self,
-                                 context: &mut StyleContext<Self>,
+                       context: &mut StyleContext<Self>,
-                                 data: &mut ElementData,
+                       data: &mut ElementData) {
-                                 pseudo: Option<&PseudoElement>) {
-        debug_assert!(pseudo.is_none() || self.implemented_pseudo_element().is_none(),
-                      "Pseudo-element-implementing elements can't have pseudos!");
         // Collect some values.
         let (mut styles, restyle) = data.styles_and_restyle_mut();
         let mut primary_style = &mut styles.primary;
-        let pseudos = &mut styles.pseudos;
+        let mut old_values = primary_style.values.take();
-        let mut pseudo_style = match pseudo {
-            Some(p) => {
-                let style = pseudos.get_mut(p);
-                debug_assert!(style.is_some());
-                style
-            }
-            None => None,
-        };
-
-        let mut old_values = match pseudo_style {
-            Some(ref mut s) => s.values.take(),
-            None => primary_style.values.take(),
-        };
 
         // Compute the new values.
-        let mut new_values = match self.implemented_pseudo_element() {
+        let mut new_values = self.cascade_internal(context, primary_style, None);
-            Some(ref pseudo) => {
-                // This is an element-backed pseudo, just grab the styles from
-                // the parent if it's eager, and recascade otherwise.
-                //
-                // We also recascade if the eager pseudo-style has any animation
-                // rules, because we don't cascade those during the eager
-                // traversal. We could make that a bit better if the complexity
-                // cost is not too big, but given further restyles are posted
-                // directly to pseudo-elements, it doesn't seem worth the effort
-                // at a glance.
-                if pseudo.is_eager() &&
-                   self.get_animation_rules().is_empty() {
-                    let parent = self.parent_element().unwrap();
-
-                    let parent_data = parent.borrow_data().unwrap();
-                    let pseudo_style =
-                        parent_data.styles().pseudos.get(pseudo).unwrap();
-                    pseudo_style.values().clone()
-                } else {
-                    self.cascade_internal(context,
-                                          primary_style,
-                                          None)
-                }
-            }
-            None => {
-                // Else it's an eager pseudo or a normal element, do the cascade
-                // work.
-                self.cascade_internal(context,
-                                      primary_style,
-                                      pseudo_style.as_ref().map(|s| &**s))
-            }
-        };
 
         // NB: Animations for pseudo-elements in Gecko are handled while
         // traversing the pseudo-elements themselves.
-        if pseudo.is_none() &&
+        if !context.shared.traversal_flags.for_animation_only() {
-           !context.shared.traversal_flags.for_animation_only() {
             self.process_animations(context,
                                     &mut old_values,
                                     &mut new_values,
                                     primary_style);
         }
 
-        // Accumulate restyle damage.
+        if let Some(old) = old_values {
+            self.accumulate_damage(&context.shared,
+                                   restyle.unwrap(),
+                                   &old,
+                                   &new_values,
+                                   None);
+        }
+
+        // Set the new computed values.
+        primary_style.values = Some(new_values);
+    }
+
+    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() {
-            // FIXME(emilio): We have more element-backed stuff, and this is
-            // redundant for them right now.
-            if cfg!(feature = "servo") ||
-               pseudo.map_or(true, |p| !p.is_before_or_after()) {
                 self.accumulate_damage(&context.shared,
                                        restyle.unwrap(),
                                        &old,
                                        &new_values,
-                                       pseudo);
+                                       Some(pseudo));
             }
         }
 
-        // Set the new computed values.
+        pseudo_style.values = Some(new_values)
-        let mut relevant_style = pseudo_style.unwrap_or(primary_style);
-        relevant_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.
     #[cfg(feature = "gecko")]
@@ -753,15 +743,11 @@ trait PrivateMatchMethods: TElement {
     }
 
     /// Computes and applies non-redundant damage.
-    ///
-    /// FIXME(emilio): Damage for non-::before and non-::after element-backed
-    /// pseudo-elements should be refactored to go on themselves (right now they
-    /// do, but we apply this twice).
     #[cfg(feature = "gecko")]
     fn accumulate_damage(&self,
                          shared_context: &SharedStyleContext,
                          restyle: &mut RestyleData,
-                         old_values: &Arc<ComputedValues>,
+                         old_values: &ComputedValues,
                          new_values: &Arc<ComputedValues>,
                          pseudo: Option<&PseudoElement>) {
         // Don't accumulate damage if we're in a restyle for reconstruction.
@@ -797,12 +783,12 @@ trait PrivateMatchMethods: TElement {
     fn accumulate_damage(&self,
                          _shared_context: &SharedStyleContext,
                          restyle: &mut RestyleData,
-                         old_values: &Arc<ComputedValues>,
+                         old_values: &ComputedValues,
                          new_values: &Arc<ComputedValues>,
                          pseudo: Option<&PseudoElement>) {
         if restyle.damage != RestyleDamage::rebuild_and_reflow() {
-            let d = self.compute_restyle_damage(&old_values, &new_values, pseudo);
+            restyle.damage |=
-            restyle.damage |= d;
+                self.compute_restyle_damage(&old_values, &new_values, pseudo);
         }
     }
 
@@ -1394,7 +1380,7 @@ pub trait MatchMethods : TElement {
     /// 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: &Arc<ComputedValues>,
+                              old_values: &ComputedValues,
                               new_values: &Arc<ComputedValues>,
                               pseudo: Option<&PseudoElement>)
                               -> RestyleDamage
@@ -1418,15 +1404,7 @@ pub trait MatchMethods : TElement {
         }
     }
 
-    /// Performs the cascade for the element's primary style.
+    /// Cascade the eager pseudo-elements of this element.
-    fn cascade_primary(&self,
-                       context: &mut StyleContext<Self>,
-                       mut data: &mut ElementData)
-    {
-        self.cascade_primary_or_pseudo(context, &mut data, None);
-    }
-
-    /// Performs the cascade for the element's eager pseudos.
     fn cascade_pseudos(&self,
                        context: &mut StyleContext<Self>,
                        mut data: &mut ElementData)
@@ -1438,7 +1416,7 @@ pub trait MatchMethods : TElement {
         // let us pass the mutable |data| to the cascade function.
         let matched_pseudos = data.styles().pseudos.keys();
         for pseudo in matched_pseudos {
-            self.cascade_primary_or_pseudo(context, data, Some(&pseudo));
+            self.cascade_eager_pseudo(context, data, &pseudo);
         }
     }
 

components/style/selector_parser.rs

@@ -37,7 +37,7 @@ pub use gecko::restyle_damage::GeckoRestyleDamage as RestyleDamage;
 /// A type that represents the previous computed values needed for restyle
 /// damage calculation.
 #[cfg(feature = "servo")]
-pub type PreExistingComputedValues = ::stylearc::Arc<::properties::ServoComputedValues>;
+pub type PreExistingComputedValues = ::properties::ServoComputedValues;
 
 /// A type that represents the previous computed values needed for restyle
 /// damage calculation.

components/style/servo/restyle_damage.rs

@@ -11,7 +11,6 @@ use computed_values::display;
 use heapsize::HeapSizeOf;
 use properties::ServoComputedValues;
 use std::fmt;
-use stylearc::Arc;
 
 bitflags! {
     #[doc = "Individual layout actions that may be necessary after restyling."]
@@ -60,16 +59,17 @@ impl HeapSizeOf for ServoRestyleDamage {
 impl ServoRestyleDamage {
     /// Compute the appropriate restyle damage for a given style change between
     /// `old` and `new`.
-    pub fn compute(old: &Arc<ServoComputedValues>,
+    pub fn compute(old: &ServoComputedValues,
-                   new: &Arc<ServoComputedValues>) -> ServoRestyleDamage {
+                   new: &ServoComputedValues)
+                   -> ServoRestyleDamage {
         compute_damage(old, new)
     }
 
     /// 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
+    /// FIXME(bholley): Do we ever actually need this? Shouldn't
-    /// imply everything else?
+    /// RECONSTRUCT_FLOW imply everything else?
     pub fn rebuild_and_reflow() -> ServoRestyleDamage {
         REPAINT | REPOSITION | STORE_OVERFLOW | BUBBLE_ISIZES | REFLOW_OUT_OF_FLOW | REFLOW |
             RECONSTRUCT_FLOW