servo: Merge #12751 - compositor: Send animation ticks to layout even if there are script animation frames (from emilio:transitions-raf); r=pcwalton
authorEmilio Cobos Álvarez <ecoal95@gmail.com>
Sun, 07 Aug 2016 22:52:32 -0500
changeset 368478 fbff82db52716111ef49973d0185f1c63e647941
parent 368477 3478aa8b5033c884bdb72670183941a31e3833dd
child 368479 aac1e5c61fc01e735bd1365e601b8086a28a0b37
push id10863
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 23:02:23 +0000
treeherdermozilla-aurora@0931190cd725 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspcwalton
servo: Merge #12751 - compositor: Send animation ticks to layout even if there are script animation frames (from emilio:transitions-raf); r=pcwalton <!-- 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. Source-Repo: https://github.com/servo/servo Source-Revision: 0b9832119e9d42bc3ba4d8e4a4e573a03705de3e
servo/components/compositing/compositor.rs
servo/components/layout_thread/lib.rs
servo/components/script/dom/testbinding.rs
servo/components/script/dom/webidls/TestBinding.webidl
servo/components/script/dom/window.rs
servo/components/script_layout_interface/message.rs
--- a/servo/components/compositing/compositor.rs
+++ b/servo/components/compositing/compositor.rs
@@ -1781,22 +1781,25 @@ impl<Window: WindowMethods> IOCompositor
         for pipeline_id in &pipeline_ids {
             self.tick_animations_for_pipeline(*pipeline_id)
         }
     }
 
     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 {
-            AnimationTickType::Script
-        } else {
-            AnimationTickType::Layout
-        };
-        let msg = ConstellationMsg::TickAnimation(pipeline_id, animation_type);
+        if animation_callbacks_running {
+            let msg = ConstellationMsg::TickAnimation(pipeline_id, AnimationTickType::Script);
+            if let Err(e) = self.constellation_chan.send(msg) {
+                warn!("Sending tick to constellation failed ({}).", e);
+            }
+        }
+
+        // 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);
         }
     }
 
     fn constrain_viewport(&mut self, pipeline_id: PipelineId, constraints: ViewportConstraints) {
         let is_root = self.root_pipeline.as_ref().map_or(false, |root_pipeline| {
             root_pipeline.id == pipeline_id
--- a/servo/components/layout_thread/lib.rs
+++ b/servo/components/layout_thread/lib.rs
@@ -669,18 +669,18 @@ impl LayoutThread {
             }
             Msg::CollectReports(reports_chan) => {
                 self.collect_reports(reports_chan, possibly_locked_rw_data);
             },
             Msg::GetCurrentEpoch(sender) => {
                 let _rw_data = possibly_locked_rw_data.lock();
                 sender.send(self.epoch).unwrap();
             },
-            Msg::AdvanceClockMs(how_many) => {
-                self.handle_advance_clock_ms(how_many, possibly_locked_rw_data);
+            Msg::AdvanceClockMs(how_many, do_tick) => {
+                self.handle_advance_clock_ms(how_many, possibly_locked_rw_data, do_tick);
             }
             Msg::GetWebFontLoadState(sender) => {
                 let _rw_data = possibly_locked_rw_data.lock();
                 let outstanding_web_fonts = self.outstanding_web_fonts.load(Ordering::SeqCst);
                 sender.send(outstanding_web_fonts != 0).unwrap();
             },
             Msg::CreateLayoutThread(info) => {
                 self.create_layout_thread(info)
@@ -817,19 +817,22 @@ impl LayoutThread {
         }
 
         possibly_locked_rw_data.block(rw_data);
     }
 
     /// 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.
     fn handle_set_quirks_mode<'a, 'b>(&self, possibly_locked_rw_data: &mut RwData<'a, 'b>) {
         let mut rw_data = possibly_locked_rw_data.lock();
         Arc::get_mut(&mut rw_data.stylist).unwrap().set_quirks_mode(true);
         possibly_locked_rw_data.block(rw_data);
     }
--- a/servo/components/script/dom/testbinding.rs
+++ b/servo/components/script/dom/testbinding.rs
@@ -608,18 +608,18 @@ impl TestBindingMethods for TestBinding 
     fn CrashHard(&self) {
         static READ_ONLY_VALUE: i32 = 0;
         unsafe {
             let p: *mut u32 = &READ_ONLY_VALUE as *const _ as *mut _;
             ptr::write_volatile(p, 0xbaadc0de);
         }
     }
 
-    fn AdvanceClock(&self, ms: i32) {
-        self.global().r().as_window().advance_animation_clock(ms);
+    fn AdvanceClock(&self, ms: i32, tick: bool) {
+        self.global().r().as_window().advance_animation_clock(ms, tick);
     }
 
     fn Panic(&self) { panic!("explicit panic from script") }
 }
 
 impl TestBinding {
     pub fn BooleanAttributeStatic(_: GlobalRef) -> bool { false }
     pub fn SetBooleanAttributeStatic(_: GlobalRef, _: bool) {}
--- a/servo/components/script/dom/webidls/TestBinding.webidl
+++ b/servo/components/script/dom/webidls/TestBinding.webidl
@@ -434,17 +434,17 @@ interface TestBinding {
   static readonly attribute boolean prefControlledStaticAttributeDisabled;
   [Pref="dom.testbinding.prefcontrolled.enabled"]
   void prefControlledMethodDisabled();
   [Pref="dom.testbinding.prefcontrolled.enabled"]
   static void prefControlledStaticMethodDisabled();
   [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;
   [Pref="dom.testbinding.prefcontrolled2.enabled"]
   static readonly attribute boolean prefControlledStaticAttributeEnabled;
   [Pref="dom.testbinding.prefcontrolled2.enabled"]
   void prefControlledMethodEnabled();
   [Pref="dom.testbinding.prefcontrolled2.enabled"]
--- a/servo/components/script/dom/window.rs
+++ b/servo/components/script/dom/window.rs
@@ -1069,19 +1069,19 @@ impl Window {
 
     pub fn client_window(&self) -> (Size2D<u32>, Point2D<i32>) {
         let (send, recv) = ipc::channel::<(Size2D<u32>, Point2D<i32>)>().unwrap();
         self.constellation_chan.send(ConstellationMsg::GetClientWindow(send)).unwrap();
         recv.recv().unwrap_or((Size2D::zero(), Point2D::zero()))
     }
 
     /// Advances the layout animation clock by `delta` milliseconds, and then
-    /// forces a reflow.
-    pub fn advance_animation_clock(&self, delta: i32) {
-        self.layout_chan.send(Msg::AdvanceClockMs(delta)).unwrap();
+    /// forces a reflow if `tick` is true.
+    pub fn advance_animation_clock(&self, delta: i32, tick: bool) {
+        self.layout_chan.send(Msg::AdvanceClockMs(delta, tick)).unwrap();
     }
 
     /// Reflows the page unconditionally if possible and not suppressed. This
     /// method will wait for the layout thread to complete (but see the `TODO`
     /// below). If there is no window size yet, the page is presumed invisible
     /// and no reflow is performed. If reflow is suppressed, no reflow will be
     /// performed for ForDisplay goals.
     ///
--- a/servo/components/script_layout_interface/message.rs
+++ b/servo/components/script_layout_interface/message.rs
@@ -37,18 +37,19 @@ pub enum Msg {
     /// Get an RPC interface.
     GetRPC(Sender<Box<LayoutRPC + Send>>),
 
     /// Requests that the layout thread render the next frame of all animations.
     TickAnimations,
 
     /// Updates layout's timer for animation testing from script.
     ///
-    /// The inner field is the number of *milliseconds* to advance.
-    AdvanceClockMs(i32),
+    /// The inner field is the number of *milliseconds* to advance, and the bool
+    /// field is whether animations should be force-ticked.
+    AdvanceClockMs(i32, bool),
 
     /// Requests that the layout thread reflow with a newly-loaded Web font.
     ReflowWithNewlyLoadedWebFont,
 
     /// Updates the layout visible rects, affecting the area that display lists will be constructed
     /// for.
     SetVisibleRects(Vec<(LayerId, Rect<Au>)>),