From d15acc219e7ed1f6cad34579206d2c76fff433dc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Mon, 10 Jul 2017 13:33:21 +0200
Subject: [PATCH] style: Allow calling GetBaseComputedStylesForElement for an
 unstyled element.

Before this refactoring, getComputedStyle could have side effects, and left the
style data in the element, so we could never arrive there without data.

There are a few crashtests that caught this, but this was already broken if you
called animate() on an element deep in a display: none subtree.

MozReview-Commit-ID: 1AvOvhAyOP3
---
 components/style/matching.rs | 31 ---------------------------
 ports/geckolib/glue.rs       | 41 +++++++++++++++++++++++++++++-------
 2 files changed, 33 insertions(+), 39 deletions(-)

diff --git a/components/style/matching.rs b/components/style/matching.rs
index 6b20226a620..bf31369ba3a 100644
--- a/components/style/matching.rs
+++ b/components/style/matching.rs
@@ -840,37 +840,6 @@ pub trait MatchMethods : TElement {
         // StyleChange::Changed conservatively.
         StyleDifference::new(damage, StyleChange::Changed)
     }
-
-    /// Returns computed values without animation and transition rules.
-    fn get_base_style(
-        &self,
-        context: &mut StyleContext<Self>,
-        style: &Arc<ComputedValues>,
-    ) -> Arc<ComputedValues> {
-        use style_resolver::StyleResolverForElement;
-
-        let rule_node = style.rules.as_ref().unwrap();
-        let without_animation_rules =
-            context.shared.stylist.rule_tree().remove_animation_rules(rule_node);
-        if without_animation_rules == *rule_node {
-            // Note that unwrapping here is fine, because the style is only
-            // incomplete during the styling process.
-            return style.clone();
-        }
-
-        // This currently ignores visited styles, which seems acceptable,
-        // as existing browsers don't appear to animate visited styles.
-        let inputs =
-            CascadeInputs {
-                rules: Some(without_animation_rules),
-                visited_rules: None,
-            };
-
-        let style =
-            StyleResolverForElement::new(*self, context, RuleInclusion::All)
-                .cascade_style_and_visited_with_default_parents(inputs);
-        style
-    }
 }
 
 impl<E: TElement> MatchMethods for E {}
diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs
index cb848fecc65..665d20716c0 100644
--- a/ports/geckolib/glue.rs
+++ b/ports/geckolib/glue.rs
@@ -654,19 +654,35 @@ pub extern "C" fn Servo_AnimationValue_Uncompute(value: RawServoAnimationValueBo
 #[no_mangle]
 pub extern "C" fn Servo_StyleSet_GetBaseComputedValuesForElement(raw_data: RawServoStyleSetBorrowed,
                                                                  element: RawGeckoElementBorrowed,
+                                                                 computed_values: ServoComputedValuesBorrowed,
                                                                  snapshots: *const ServoElementSnapshotTable,
                                                                  pseudo_type: CSSPseudoElementType)
                                                                  -> ServoComputedValuesStrong
 {
-    use style::matching::MatchMethods;
+    use style::style_resolver::StyleResolverForElement;
+
     debug_assert!(!snapshots.is_null());
+    let computed_values = ComputedValues::as_arc(&computed_values);
+
+    let rules = match computed_values.rules {
+        None => return computed_values.clone().into_strong(),
+        Some(ref rules) => rules,
+    };
 
     let doc_data = PerDocumentStyleData::from_ffi(raw_data).borrow();
-    let global_style_data = &*GLOBAL_STYLE_DATA;
-    let guard = global_style_data.shared_lock.read();
+    let without_animations =
+        doc_data.stylist.rule_tree().remove_animation_rules(rules);
+
+    if without_animations == *rules {
+        return computed_values.clone().into_strong();
+    }
 
     let element = GeckoElement(element);
-    let element_data = element.borrow_data().unwrap();
+
+    let element_data = match element.borrow_data() {
+        Some(data) => data,
+        None => return computed_values.clone().into_strong(),
+    };
     let styles = &element_data.styles;
 
     if let Some(pseudo) = PseudoElement::from_pseudo_type(pseudo_type) {
@@ -678,6 +694,8 @@ pub extern "C" fn Servo_StyleSet_GetBaseComputedValuesForElement(raw_data: RawSe
             .clone().into_strong();
     }
 
+    let global_style_data = &*GLOBAL_STYLE_DATA;
+    let guard = global_style_data.shared_lock.read();
     let shared = create_shared_context(&global_style_data,
                                        &guard,
                                        &doc_data,
@@ -689,10 +707,17 @@ pub extern "C" fn Servo_StyleSet_GetBaseComputedValuesForElement(raw_data: RawSe
         thread_local: &mut tlc,
     };
 
-    element.get_base_style(
-        &mut context,
-        styles.primary(),
-    ).into_strong()
+    // This currently ignores visited styles, which seems acceptable, as
+    // existing browsers don't appear to animate visited styles.
+    let inputs =
+        CascadeInputs {
+            rules: Some(without_animations),
+            visited_rules: None,
+        };
+
+    StyleResolverForElement::new(element, &mut context, RuleInclusion::All)
+        .cascade_style_and_visited_with_default_parents(inputs)
+        .into_strong()
 }
 
 #[no_mangle]