From 3b7b21ebb5f769789fc65fb90e98636b53f3860a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Wed, 17 Oct 2018 15:46:21 +0200
Subject: [PATCH] Revert "style: Make the transitions code make sense again."

This reverts commit d6092fae27a55345ac9fa82f4d2195130ec01318.

This change actually makes transitions start, and our code for stopping
transitions is just bogus, so we just keep re-starting them over and over, which
is not good.
---
 components/style/animation.rs                 |  4 ++--
 components/style/matching.rs                  | 20 ++++++++++---------
 .../font-style-interpolation.html.ini         |  2 ++
 ...nsition-property-all-before-value.html.ini |  3 ---
 ...ue-before-transition-property-all.html.ini |  3 ---
 5 files changed, 15 insertions(+), 17 deletions(-)
 delete mode 100644 tests/wpt/metadata/css/css-variables/variable-transitions-transition-property-all-before-value.html.ini
 delete mode 100644 tests/wpt/metadata/css/css-variables/variable-transitions-value-before-transition-property-all.html.ini

diff --git a/components/style/animation.rs b/components/style/animation.rs
index 17e8b0981fb..5d96f5cacd1 100644
--- a/components/style/animation.rs
+++ b/components/style/animation.rs
@@ -345,8 +345,8 @@ impl PropertyAnimation {
 
         let property_animation = PropertyAnimation {
             property: animated_property,
-            timing_function,
-            duration,
+            timing_function: timing_function,
+            duration: duration,
         };
 
         if property_animation.does_animate() {
diff --git a/components/style/matching.rs b/components/style/matching.rs
index 24c9e9bbf11..9d840a5bbd9 100644
--- a/components/style/matching.rs
+++ b/components/style/matching.rs
@@ -331,7 +331,7 @@ trait PrivateMatchMethods: TElement {
     fn process_animations(
         &self,
         context: &mut StyleContext<Self>,
-        old_values: Option<&Arc<ComputedValues>>,
+        old_values: &mut Option<Arc<ComputedValues>>,
         new_values: &mut Arc<ComputedValues>,
         restyle_hint: RestyleHint,
         important_rules_changed: bool,
@@ -413,7 +413,7 @@ trait PrivateMatchMethods: TElement {
     fn process_animations(
         &self,
         context: &mut StyleContext<Self>,
-        old_values: Option<&Arc<ComputedValues>>,
+        old_values: &mut Option<Arc<ComputedValues>>,
         new_values: &mut Arc<ComputedValues>,
         _restyle_hint: RestyleHint,
         _important_rules_changed: bool,
@@ -423,10 +423,11 @@ trait PrivateMatchMethods: TElement {
 
         let mut possibly_expired_animations = vec![];
         let shared_context = context.shared;
-        if old_values.is_some() {
+        if let Some(ref mut old) = *old_values {
+            // FIXME(emilio, #20116): This makes no sense.
             self.update_animations_for_cascade(
                 shared_context,
-                new_values,
+                old,
                 &mut possibly_expired_animations,
                 &context.thread_local.font_metrics_provider,
             );
@@ -445,10 +446,7 @@ trait PrivateMatchMethods: TElement {
 
         // Trigger transitions if necessary. This will reset `new_values` back
         // to its old value if it did trigger a transition.
-        //
-        // FIXME(emilio): We need logic to also stop the old transitions
-        // from running if transition-property / transition-duration changed.
-        if let Some(ref values) = old_values {
+        if let Some(ref values) = *old_values {
             animation::start_transitions_if_applicable(
                 new_animations_sender,
                 this_opaque,
@@ -575,6 +573,10 @@ trait PrivateMatchMethods: TElement {
 
     // FIXME(emilio, #20116): It's not clear to me that the name of this method
     // represents anything of what it does.
+    //
+    // Also, this function gets the old style, for some reason I don't really
+    // get, but the functions called (mainly update_style_for_animation) expects
+    // the new style, wtf?
     #[cfg(feature = "servo")]
     fn update_animations_for_cascade(
         &self,
@@ -677,7 +679,7 @@ pub trait MatchMethods: TElement {
 
         self.process_animations(
             context,
-            data.styles.primary.as_ref(),
+            &mut data.styles.primary,
             &mut new_styles.primary.style.0,
             data.hint,
             important_rules_changed,
diff --git a/tests/wpt/metadata/css/css-fonts/variations/font-style-interpolation.html.ini b/tests/wpt/metadata/css/css-fonts/variations/font-style-interpolation.html.ini
index 2e7944f8327..6c98bee99a6 100644
--- a/tests/wpt/metadata/css/css-fonts/variations/font-style-interpolation.html.ini
+++ b/tests/wpt/metadata/css/css-fonts/variations/font-style-interpolation.html.ini
@@ -3,4 +3,6 @@
   expected: TIMEOUT
   [font-style transition]
     expected: TIMEOUT
+  [font-style animation]
+    expected: TIMEOUT
 
diff --git a/tests/wpt/metadata/css/css-variables/variable-transitions-transition-property-all-before-value.html.ini b/tests/wpt/metadata/css/css-variables/variable-transitions-transition-property-all-before-value.html.ini
deleted file mode 100644
index 282cefe525a..00000000000
--- a/tests/wpt/metadata/css/css-variables/variable-transitions-transition-property-all-before-value.html.ini
+++ /dev/null
@@ -1,3 +0,0 @@
-[variable-transitions-transition-property-all-before-value.html]
-  [Verify substituted color value after transition]
-    expected: FAIL
diff --git a/tests/wpt/metadata/css/css-variables/variable-transitions-value-before-transition-property-all.html.ini b/tests/wpt/metadata/css/css-variables/variable-transitions-value-before-transition-property-all.html.ini
deleted file mode 100644
index e1fc8c9c968..00000000000
--- a/tests/wpt/metadata/css/css-variables/variable-transitions-value-before-transition-property-all.html.ini
+++ /dev/null
@@ -1,3 +0,0 @@
-[variable-transitions-value-before-transition-property-all.html]
-  [Verify substituted color value after transition]
-    expected: FAIL