Auto merge of #12751 - emilio:transitions-raf, r=pcwalton

compositor: Send animation ticks to layout even if there are script animation frames.

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #12749 (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

The script tick ends up only processing JS callbacks related to animation
frames, so CSS transitions/animations end up not working as expected.

This could have accidentally worked before #12563 because we over-restyled, but
now this is no longer the case.

Other possible way to do it is making a layout reflow with RAF handle CSS
animations/transitions too, but that may not work if the reflow ends up being
suppressed (that could very well be the case), and we'd need to handle a lot
more state in the document, so this solution (assuming it doesn't break try)
seems a bit less flacky.

Missing a test, will add one soon. Fixes #12749.

<!-- 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/12751)
<!-- Reviewable:end -->

components/compositing/compositor.rs

@@ -1786,12 +1786,15 @@ impl<Window: WindowMethods> IOCompositor<Window> {
     fn tick_animations_for_pipeline(&mut self, pipeline_id: PipelineId) {
         self.schedule_delayed_composite_if_necessary();
         let animation_callbacks_running = self.pipeline_details(pipeline_id).animation_callbacks_running;
-        let animation_type = if animation_callbacks_running {
+        if animation_callbacks_running {
-            AnimationTickType::Script
+            let msg = ConstellationMsg::TickAnimation(pipeline_id, AnimationTickType::Script);
-        } else {
+            if let Err(e) = self.constellation_chan.send(msg) {
-            AnimationTickType::Layout
+                warn!("Sending tick to constellation failed ({}).", e);
-        };
+            }
-        let msg = ConstellationMsg::TickAnimation(pipeline_id, animation_type);
+        }
+
+        // We still need to tick layout unfortunately, see things like #12749.
+        let msg = ConstellationMsg::TickAnimation(pipeline_id, AnimationTickType::Layout);
         if let Err(e) = self.constellation_chan.send(msg) {
             warn!("Sending tick to constellation failed ({}).", e);
         }

components/layout_thread/lib.rs

@@ -674,8 +674,8 @@ impl LayoutThread {
                 let _rw_data = possibly_locked_rw_data.lock();
                 sender.send(self.epoch).unwrap();
             },
-            Msg::AdvanceClockMs(how_many) => {
+            Msg::AdvanceClockMs(how_many, do_tick) => {
-                self.handle_advance_clock_ms(how_many, possibly_locked_rw_data);
+                self.handle_advance_clock_ms(how_many, possibly_locked_rw_data, do_tick);
             }
             Msg::GetWebFontLoadState(sender) => {
                 let _rw_data = possibly_locked_rw_data.lock();
@@ -822,9 +822,12 @@ impl LayoutThread {
     /// Advances the animation clock of the document.
     fn handle_advance_clock_ms<'a, 'b>(&mut self,
                                        how_many_ms: i32,
-                                       possibly_locked_rw_data: &mut RwData<'a, 'b>) {
+                                       possibly_locked_rw_data: &mut RwData<'a, 'b>,
+                                       tick_animations: bool) {
         self.timer.increment(how_many_ms as f64 / 1000.0);
-        self.tick_all_animations(possibly_locked_rw_data);
+        if tick_animations {
+            self.tick_all_animations(possibly_locked_rw_data);
+        }
     }
 
     /// Sets quirks mode for the document, causing the quirks mode stylesheet to be used.

components/script/dom/testbinding.rs

@@ -613,8 +613,8 @@ impl TestBindingMethods for TestBinding {
         }
     }
 
-    fn AdvanceClock(&self, ms: i32) {
+    fn AdvanceClock(&self, ms: i32, tick: bool) {
-        self.global().r().as_window().advance_animation_clock(ms);
+        self.global().r().as_window().advance_animation_clock(ms, tick);
     }
 
     fn Panic(&self) { panic!("explicit panic from script") }

components/script/dom/webidls/TestBinding.webidl

@@ -439,7 +439,7 @@ interface TestBinding {
   [Pref="dom.testbinding.prefcontrolled.enabled"]
   const unsigned short prefControlledConstDisabled = 0;
   [Pref="layout.animations.test.enabled"]
-  void advanceClock(long millis);
+  void advanceClock(long millis, optional boolean forceLayoutTick = true);
 
   [Pref="dom.testbinding.prefcontrolled2.enabled"]
   readonly attribute boolean prefControlledAttributeEnabled;

components/script/dom/window.rs

@@ -1074,9 +1074,9 @@ impl Window {
     }
 
     /// Advances the layout animation clock by `delta` milliseconds, and then
-    /// forces a reflow.
+    /// forces a reflow if `tick` is true.
-    pub fn advance_animation_clock(&self, delta: i32) {
+    pub fn advance_animation_clock(&self, delta: i32, tick: bool) {
-        self.layout_chan.send(Msg::AdvanceClockMs(delta)).unwrap();
+        self.layout_chan.send(Msg::AdvanceClockMs(delta, tick)).unwrap();
     }
 
     /// Reflows the page unconditionally if possible and not suppressed. This

components/script_layout_interface/message.rs

@@ -42,8 +42,9 @@ pub enum Msg {
 
     /// Updates layout's timer for animation testing from script.
     ///
-    /// The inner field is the number of *milliseconds* to advance.
+    /// The inner field is the number of *milliseconds* to advance, and the bool
-    AdvanceClockMs(i32),
+    /// field is whether animations should be force-ticked.
+    AdvanceClockMs(i32, bool),
 
     /// Requests that the layout thread reflow with a newly-loaded Web font.
     ReflowWithNewlyLoadedWebFont,

tests/wpt/mozilla/meta/MANIFEST.json

@@ -6168,6 +6168,12 @@
             "url": "/_mozilla/css/animations/basic-transition.html"
           }
         ],
+        "css/animations/transition-raf.html": [
+          {
+            "path": "css/animations/transition-raf.html",
+            "url": "/_mozilla/css/animations/transition-raf.html"
+          }
+        ],
         "css/empty-keyframes.html": [
           {
             "path": "css/empty-keyframes.html",

tests/wpt/mozilla/tests/css/animations/transition-raf.html

@@ -0,0 +1,40 @@
+<!doctype html>
+<meta charset="utf-8">
+<title></title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<style>
+#box {
+  width: 100px;
+  height: 200px;
+  background: red;
+  transition: width 1s linear;
+}
+
+#box.expose {
+  width: 200px;
+}
+</style>
+<div id="box"></div>
+<script>
+var box = document.getElementById('box');
+var test = new window.TestBinding();
+async_test(function(t) {
+  // Dummy RAF loop.
+  (function onFrame() {
+    var style = getComputedStyle(box).getPropertyValue('width');
+    if (style === '150px') {
+      t.done();
+    } else {
+      window.requestAnimationFrame(onFrame);
+    }
+  }());
+
+  window.addEventListener('load', function() {
+    assert_equals(getComputedStyle(box).getPropertyValue('width'), '100px');
+    box.className = "expose";
+    // Let the first restyle run at zero, then advance the clock.
+    setTimeout(function() { test.advanceClock(500, false) }, 0);
+  });
+}, "Transitions should work during RAF loop")
+</script>