From 805988e839217ccea19e9bdec86462cd8739bf14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Fri, 5 Aug 2016 13:38:58 -0700 Subject: [PATCH 1/3] compositor: Send animation ticks to layout even if there are script animation frames. 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. Fixes #12749. --- components/compositing/compositor.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/components/compositing/compositor.rs b/components/compositing/compositor.rs index 60b64a03047..653fb219447 100644 --- a/components/compositing/compositor.rs +++ b/components/compositing/compositor.rs @@ -1786,12 +1786,15 @@ impl IOCompositor { 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); } From 0e3d4ab4079878dbb96f14f23d5b189eaa1fca42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Fri, 5 Aug 2016 14:38:33 -0700 Subject: [PATCH 2/3] layout: Expand animation test mode to support not force-ticking layout. --- components/layout_thread/lib.rs | 11 +++++++---- components/script/dom/testbinding.rs | 4 ++-- components/script/dom/webidls/TestBinding.webidl | 2 +- components/script/dom/window.rs | 6 +++--- components/script_layout_interface/message.rs | 5 +++-- 5 files changed, 16 insertions(+), 12 deletions(-) diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs index 403f30a96f5..958a2acc71e 100644 --- a/components/layout_thread/lib.rs +++ b/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) => { - 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(); @@ -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. diff --git a/components/script/dom/testbinding.rs b/components/script/dom/testbinding.rs index 13579d71330..372055483c7 100644 --- a/components/script/dom/testbinding.rs +++ b/components/script/dom/testbinding.rs @@ -613,8 +613,8 @@ impl TestBindingMethods for TestBinding { } } - 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") } diff --git a/components/script/dom/webidls/TestBinding.webidl b/components/script/dom/webidls/TestBinding.webidl index 9fc8a9e3bb6..9945f983ad7 100644 --- a/components/script/dom/webidls/TestBinding.webidl +++ b/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; diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index c25c826b4ec..8aaa0adcde3 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -1074,9 +1074,9 @@ impl Window { } /// 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 diff --git a/components/script_layout_interface/message.rs b/components/script_layout_interface/message.rs index b46703e778e..14580be4881 100644 --- a/components/script_layout_interface/message.rs +++ b/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. - 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, From a3d1457809f76355764a43f6f36f10ddd9ff0948 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Fri, 5 Aug 2016 14:40:36 -0700 Subject: [PATCH 3/3] Add test for #12749 --- tests/wpt/mozilla/meta/MANIFEST.json | 6 +++ .../tests/css/animations/transition-raf.html | 40 +++++++++++++++++++ 2 files changed, 46 insertions(+) create mode 100644 tests/wpt/mozilla/tests/css/animations/transition-raf.html diff --git a/tests/wpt/mozilla/meta/MANIFEST.json b/tests/wpt/mozilla/meta/MANIFEST.json index 4d0e9b89ac2..d9d08e602d6 100644 --- a/tests/wpt/mozilla/meta/MANIFEST.json +++ b/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", diff --git a/tests/wpt/mozilla/tests/css/animations/transition-raf.html b/tests/wpt/mozilla/tests/css/animations/transition-raf.html new file mode 100644 index 00000000000..6159bb9ab33 --- /dev/null +++ b/tests/wpt/mozilla/tests/css/animations/transition-raf.html @@ -0,0 +1,40 @@ + + + + + + +
+