diff --git a/components/compositing/compositor.rs b/components/compositing/compositor.rs index dc2a44fc0822..2b810536e192 100644 --- a/components/compositing/compositor.rs +++ b/components/compositing/compositor.rs @@ -21,7 +21,7 @@ use msg::constellation_msg::{Key, KeyModifiers, KeyState, CONTROL}; use msg::constellation_msg::{PipelineId, PipelineIndex, PipelineNamespaceId, TraversalDirection}; use net_traits::image::base::{Image, PixelFormat}; use profile_traits::time::{self, ProfilerCategory, profile}; -use script_traits::{AnimationState, AnimationTickType, ConstellationControlMsg}; +use script_traits::{AnimationState, ConstellationControlMsg}; use script_traits::{ConstellationMsg, DevicePixel, LayoutControlMsg, LoadData, MouseButton}; use script_traits::{MouseEventType, StackingContextScrollState}; use script_traits::{TouchpadPressurePhase, TouchEventType, TouchId, WindowSizeData, WindowSizeType}; @@ -1269,20 +1269,11 @@ 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; - 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 may need to tick animations in layout. (See #12749.) - let animations_running = self.pipeline_details(pipeline_id).animations_running; - if animations_running { - let msg = ConstellationMsg::TickAnimation(pipeline_id, AnimationTickType::Layout); - 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, animation_callbacks_running); + if let Err(e) = self.constellation_chan.send(msg) { + warn!("Sending tick to constellation failed ({}).", e); } } diff --git a/components/constellation/constellation.rs b/components/constellation/constellation.rs index 0682cbe526bc..d17d6d838299 100644 --- a/components/constellation/constellation.rs +++ b/components/constellation/constellation.rs @@ -94,7 +94,7 @@ use offscreen_gl_context::{GLContextAttributes, GLLimits}; use pipeline::{InitialPipelineState, Pipeline}; use profile_traits::mem; use profile_traits::time; -use script_traits::{AnimationState, AnimationTickType, CompositorEvent}; +use script_traits::{AnimationState, CompositorEvent}; use script_traits::{ConstellationControlMsg, ConstellationMsg as FromCompositorMsg, DiscardBrowsingContext}; use script_traits::{DocumentActivity, DocumentState, LayoutControlMsg, LoadData}; use script_traits::{IFrameLoadInfo, IFrameLoadInfoWithData, IFrameSandboxState, TimerEventRequest}; @@ -897,8 +897,8 @@ impl Constellation debug!("constellation got window resize message"); self.handle_window_size_msg(new_size, size_type); } - FromCompositorMsg::TickAnimation(pipeline_id, tick_type) => { - self.handle_tick_animation(pipeline_id, tick_type) + FromCompositorMsg::TickAnimation(pipeline_id, script_callbacks_present) => { + self.handle_tick_animation(pipeline_id, script_callbacks_present) } FromCompositorMsg::WebDriverCommand(command) => { debug!("constellation got webdriver command message"); @@ -1540,23 +1540,16 @@ impl Constellation animation_state)) } - fn handle_tick_animation(&mut self, pipeline_id: PipelineId, tick_type: AnimationTickType) { - let result = match tick_type { - AnimationTickType::Script => { - let msg = ConstellationControlMsg::TickAllAnimations(pipeline_id); - match self.pipelines.get(&pipeline_id) { - Some(pipeline) => pipeline.event_loop.send(msg), - None => return warn!("Pipeline {:?} got script tick after closure.", pipeline_id), - } - } - AnimationTickType::Layout => { - let msg = LayoutControlMsg::TickAnimations; - match self.pipelines.get(&pipeline_id) { - Some(pipeline) => pipeline.layout_chan.send(msg), - None => return warn!("Pipeline {:?} got layout tick after closure.", pipeline_id), - } - } + fn handle_tick_animation(&mut self, + pipeline_id: PipelineId, + script_callbacks_running: bool) { + let msg = LayoutControlMsg::TickAnimations(script_callbacks_running); + + let result = match self.pipelines.get(&pipeline_id) { + Some(pipeline) => pipeline.layout_chan.send(msg), + None => return warn!("Pipeline {:?} got layout tick after closure.", pipeline_id), }; + if let Err(e) = result { self.handle_send_error(pipeline_id, e); } diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs index 31500461fa4f..55f4d3e4ba4a 100644 --- a/components/layout_thread/lib.rs +++ b/components/layout_thread/lib.rs @@ -560,8 +560,10 @@ impl LayoutThread { self.handle_request_helper(Msg::SetStackingContextScrollStates(new_scroll_states), possibly_locked_rw_data) }, - Request::FromPipeline(LayoutControlMsg::TickAnimations) => { - self.handle_request_helper(Msg::TickAnimations, possibly_locked_rw_data) + Request::FromPipeline(LayoutControlMsg::TickAnimations( + script_animation_callbacks_present)) => { + self.handle_request_helper(Msg::TickAnimations(script_animation_callbacks_present), + possibly_locked_rw_data) }, Request::FromPipeline(LayoutControlMsg::GetCurrentEpoch(sender)) => { self.handle_request_helper(Msg::GetCurrentEpoch(sender), possibly_locked_rw_data) @@ -606,7 +608,9 @@ impl LayoutThread { self.time_profiler_chan.clone(), || self.handle_reflow(&data, possibly_locked_rw_data)); }, - Msg::TickAnimations => self.tick_all_animations(possibly_locked_rw_data), + Msg::TickAnimations(script_animation_callbacks_present) => { + self.tick_all_animations(possibly_locked_rw_data, script_animation_callbacks_present) + } Msg::SetStackingContextScrollStates(new_scroll_states) => { self.set_stacking_context_scroll_states(new_scroll_states, possibly_locked_rw_data); @@ -623,8 +627,11 @@ impl LayoutThread { let _rw_data = possibly_locked_rw_data.lock(); sender.send(self.epoch).unwrap(); }, - Msg::AdvanceClockMs(how_many, do_tick) => { - self.handle_advance_clock_ms(how_many, possibly_locked_rw_data, do_tick); + Msg::AdvanceClockMs(how_many, do_tick, script_callbacks) => { + self.handle_advance_clock_ms(how_many, + possibly_locked_rw_data, + do_tick, + script_callbacks); } Msg::GetWebFontLoadState(sender) => { let _rw_data = possibly_locked_rw_data.lock(); @@ -758,10 +765,11 @@ impl LayoutThread { fn handle_advance_clock_ms<'a, 'b>(&mut self, how_many_ms: i32, possibly_locked_rw_data: &mut RwData<'a, 'b>, - tick_animations: bool) { + tick_animations: bool, + script_animation_callbacks_present: bool) { self.timer.increment(how_many_ms as f64 / 1000.0); if tick_animations { - self.tick_all_animations(possibly_locked_rw_data); + self.tick_all_animations(possibly_locked_rw_data, script_animation_callbacks_present); } } @@ -1344,12 +1352,16 @@ impl LayoutThread { rw_data.stacking_context_scroll_offsets = layout_scroll_states } - fn tick_all_animations<'a, 'b>(&mut self, possibly_locked_rw_data: &mut RwData<'a, 'b>) { + fn tick_all_animations<'a, 'b>(&mut self, + possibly_locked_rw_data: &mut RwData<'a, 'b>, + script_animation_callbacks_present: bool) { let mut rw_data = possibly_locked_rw_data.lock(); - self.tick_animations(&mut rw_data); + self.tick_animations(&mut rw_data, script_animation_callbacks_present); } - fn tick_animations(&mut self, rw_data: &mut LayoutThreadData) { + fn tick_animations(&mut self, + rw_data: &mut LayoutThreadData, + script_animation_callbacks_present: bool) { if opts::get().relayout_event { println!("**** pipeline={}\tForDisplay\tSpecial\tAnimationTick", self.id); } @@ -1389,6 +1401,16 @@ impl LayoutThread { &mut *rw_data, &mut layout_context); assert!(layout_context.pending_images.is_none()); + + if script_animation_callbacks_present { + // Now that we're done sending transition events, etc, tell script + // it can run the animation callbacks. + // + // This order is defined by: + // https://html.spec.whatwg.org/multipage/#run-css-animations-and-send-events + let msg = ConstellationControlMsg::TickAllAnimations(self.id); + self.script_chan.send(msg).unwrap(); + } } } diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index 7a50fe5cf6c0..c01bb0e926d2 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -1558,6 +1558,10 @@ impl Document { } } + pub fn has_animation_callbacks(&self) -> bool { + !self.animation_frame_list.borrow().is_empty() + } + /// https://html.spec.whatwg.org/multipage/#run-the-animation-frame-callbacks pub fn run_the_animation_frame_callbacks(&self) { rooted_vec!(let mut animation_frame_list); @@ -1590,7 +1594,7 @@ impl Document { // constellation to stop giving us video refresh callbacks, to save energy. (A spurious // animation frame is one in which the callback did not mutate the DOM—that is, an // animation frame that wasn't actually used for animation.) - if self.animation_frame_list.borrow().is_empty() || + if !self.has_animation_callbacks() || (!was_faking_animation_frames && self.is_faking_animation_frames()) { mem::swap(&mut *self.animation_frame_list.borrow_mut(), &mut *animation_frame_list); diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index 3f89e7adf00c..af4282a095aa 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -80,7 +80,7 @@ use script_thread::{MainThreadScriptChan, MainThreadScriptMsg, Runnable, Runnabl use script_thread::{SendableMainThreadScriptChan, ImageCacheMsg}; use script_traits::{ConstellationControlMsg, LoadData, MozBrowserEvent, UntrustedNodeAddress}; use script_traits::{DocumentState, TimerEvent, TimerEventId}; -use script_traits::{ScriptMsg as ConstellationMsg, TimerEventRequest, WindowSizeData, WindowSizeType}; +use script_traits::{ScriptMsg as ConstellationMsg, TimerEventRequest, WindowSizeData}; use script_traits::webdriver_msg::{WebDriverJSError, WebDriverJSResult}; use servo_atoms::Atom; use servo_config::opts; @@ -188,9 +188,6 @@ pub struct Window { #[ignore_heap_size_of = "channels are hard"] devtools_marker_sender: DOMRefCell>>>, - /// Pending resize event, if any. - resize_event: Cell>, - /// Parent id associated with this page, if any. parent_info: Option<(PipelineId, FrameType)>, @@ -1123,7 +1120,9 @@ impl Window { /// Advances the layout animation clock by `delta` milliseconds, and then /// 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(); + let has_animation_callbacks = self.Document().has_animation_callbacks(); + self.layout_chan + .send(Msg::AdvanceClockMs(delta, tick, has_animation_callbacks)).unwrap(); } /// Reflows the page unconditionally if possible and not suppressed. This @@ -1547,16 +1546,6 @@ impl Window { self.pending_reflow_count.set(self.pending_reflow_count.get() + 1); } - pub fn set_resize_event(&self, event: WindowSizeData, event_type: WindowSizeType) { - self.resize_event.set(Some((event, event_type))); - } - - pub fn steal_resize_event(&self) -> Option<(WindowSizeData, WindowSizeType)> { - let event = self.resize_event.get(); - self.resize_event.set(None); - event - } - pub fn set_page_clip_rect_with_new_viewport(&self, viewport: Rect) -> bool { let rect = f32_rect_to_au_rect(viewport.clone()); self.current_viewport.set(rect); @@ -1768,7 +1757,6 @@ impl Window { bluetooth_thread: bluetooth_thread, bluetooth_extra_permission_data: BluetoothExtraPermissionData::new(), page_clip_rect: Cell::new(max_rect()), - resize_event: Cell::new(None), layout_chan: layout_chan, layout_rpc: layout_rpc, window_size: Cell::new(window_size), diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index ac96836d92ad..52c06ced38c9 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -750,24 +750,6 @@ impl ScriptThread { use self::MixedMessage::{FromConstellation, FromDevtools, FromImageCache}; use self::MixedMessage::{FromScheduler, FromScript}; - // Handle pending resize events. - // Gather them first to avoid a double mut borrow on self. - let mut resizes = vec!(); - - for (id, document) in self.documents.borrow().iter() { - // Only process a resize if layout is idle. - if let Some((size, size_type)) = document.window().steal_resize_event() { - resizes.push((id, size, size_type)); - } - } - - for (id, size, size_type) in resizes { - self.handle_event(id, ResizeEvent(size, size_type)); - } - - // Store new resizes, and gather all other events. - let mut sequential = vec![]; - // Receive at least one message so we don't spinloop. let mut event = { let sel = Select::new(); @@ -801,9 +783,18 @@ impl ScriptThread { } }; - // Squash any pending resize, reflow, animation tick, and mouse-move events in the queue. + // Sort any pending resize, reflow, animation tick, transition event, + // and mouse-move events in the queue. + // + // Dispatch them in order per: + // https://html.spec.whatwg.org/multipage/#run-css-animations-and-send-events + // + // FIXME: Lots to do here. + let mut sequential = vec![]; let mut mouse_move_event_index = None; - let mut animation_ticks = HashSet::new(); + let mut pipelines_to_tick = HashSet::new(); + let mut resizes = HashMap::new(); + let mut transition_events = vec![]; loop { // https://html.spec.whatwg.org/multipage/#event-loop-processing-model step 7 match event { @@ -818,10 +809,7 @@ impl ScriptThread { }) } FromConstellation(ConstellationControlMsg::Resize(id, size, size_type)) => { - // step 7.7 - self.profile_event(ScriptThreadEventCategory::Resize, || { - self.handle_resize(id, size, size_type); - }) + resizes.insert(id, (size, size_type)); } FromConstellation(ConstellationControlMsg::Viewport(id, rect)) => { self.profile_event(ScriptThreadEventCategory::SetViewport, || { @@ -835,11 +823,11 @@ impl ScriptThread { } FromConstellation(ConstellationControlMsg::TickAllAnimations( pipeline_id)) => { - // step 7.8 - if !animation_ticks.contains(&pipeline_id) { - animation_ticks.insert(pipeline_id); - sequential.push(event); - } + pipelines_to_tick.insert(pipeline_id); + } + FromConstellation(ConstellationControlMsg::TransitionEnd( + unsafe_node, name, duration)) => { + transition_events.push((unsafe_node, name, duration)); } FromConstellation(ConstellationControlMsg::SendEvent( _, @@ -880,6 +868,26 @@ impl ScriptThread { } } + // Step 5. Resize steps. + for (id, (size, size_type)) in resizes { + self.handle_resize(id, size, size_type); + } + + // FIXME Step 6. Scroll steps. + // FIXME Step 7. Media queries. + + // Step 8. CSS animation events. + for (unsafe_node, name, duration) in transition_events { + self.handle_transition_event(unsafe_node, name, duration); + } + + // FIXME Step 8: Fullscreen. + + // Step 9: animation frame callbacks. + for pipeline_id in pipelines_to_tick { + self.handle_tick_all_animations(pipeline_id); + } + // Process the gathered events. for msg in sequential { let category = self.categorize_msg(&msg); @@ -1037,10 +1045,6 @@ impl ScriptThread { self.handle_focus_iframe_msg(parent_pipeline_id, frame_id), ConstellationControlMsg::WebDriverScriptCommand(pipeline_id, msg) => self.handle_webdriver_msg(pipeline_id, msg), - ConstellationControlMsg::TickAllAnimations(pipeline_id) => - self.handle_tick_all_animations(pipeline_id), - ConstellationControlMsg::TransitionEnd(unsafe_node, name, duration) => - self.handle_transition_event(unsafe_node, name, duration), ConstellationControlMsg::WebFontLoaded(pipeline_id) => self.handle_web_font_loaded(pipeline_id), ConstellationControlMsg::DispatchFrameLoadEvent { @@ -1062,8 +1066,10 @@ impl ScriptThread { msg @ ConstellationControlMsg::Viewport(..) | msg @ ConstellationControlMsg::SetScrollState(..) | msg @ ConstellationControlMsg::Resize(..) | + msg @ ConstellationControlMsg::TickAllAnimations(..) | + msg @ ConstellationControlMsg::TransitionEnd(..) | msg @ ConstellationControlMsg::ExitScriptThread => - panic!("should have handled {:?} already", msg), + panic!("should have handled {:?} already", msg), } } @@ -1196,17 +1202,12 @@ impl ScriptThread { } fn handle_resize(&self, id: PipelineId, size: WindowSizeData, size_type: WindowSizeType) { - let window = self.documents.borrow().find_window(id); - if let Some(ref window) = window { - window.set_resize_event(size, size_type); - return; - } let mut loads = self.incomplete_loads.borrow_mut(); if let Some(ref mut load) = loads.iter_mut().find(|load| load.pipeline_id == id) { load.window_size = Some(size); - return; } - warn!("resize sent to nonexistent pipeline"); + + self.handle_event(id, ResizeEvent(size, size_type)); } fn handle_viewport(&self, id: PipelineId, rect: Rect) { diff --git a/components/script_layout_interface/message.rs b/components/script_layout_interface/message.rs index feb2b533fd2d..3ffb3d81d451 100644 --- a/components/script_layout_interface/message.rs +++ b/components/script_layout_interface/message.rs @@ -37,13 +37,14 @@ pub enum Msg { GetRPC(Sender>), /// Requests that the layout thread render the next frame of all animations. - TickAnimations, + TickAnimations(bool /* script_callbacks_running */), /// Updates layout's timer for animation testing from script. /// - /// The inner field is the number of *milliseconds* to advance, and the bool - /// field is whether animations should be force-ticked. - AdvanceClockMs(i32, bool), + /// The inner field is the number of *milliseconds* to advance, the first + /// bool field is whether animations should be force-ticked, and the second + /// one whether there are script animation callbacks or not. + AdvanceClockMs(i32, bool /* force_tick */, bool /* script_callbacks_present */), /// Destroys layout data associated with a DOM node. /// diff --git a/components/script_traits/lib.rs b/components/script_traits/lib.rs index 96042083d393..120acc27a1bf 100644 --- a/components/script_traits/lib.rs +++ b/components/script_traits/lib.rs @@ -117,8 +117,8 @@ pub enum LayoutControlMsg { ExitNow, /// Requests the current epoch (layout counter) from this layout. GetCurrentEpoch(IpcSender), - /// Asks layout to run another step in its animation. - TickAnimations, + /// Asks layout to run another step in its animations. + TickAnimations(bool /* script_animation_callbacks_present */), /// Tells layout about the new scrolling offsets of each scrollable stacking context. SetStackingContextScrollStates(Vec), /// Requests the current load state of Web fonts. `true` is returned if fonts are still loading @@ -638,15 +638,6 @@ impl MozBrowserErrorType { } } -/// Specifies whether the script or layout thread needs to be ticked for animation. -#[derive(Deserialize, Serialize)] -pub enum AnimationTickType { - /// The script thread. - Script, - /// The layout thread. - Layout, -} - /// The scroll state of a stacking context. #[derive(Copy, Clone, Debug, Deserialize, Serialize)] pub struct StackingContextScrollState { @@ -731,7 +722,7 @@ pub enum ConstellationMsg { /// Inform the constellation of a window being resized. WindowSize(WindowSizeData, WindowSizeType), /// Requests that the constellation instruct layout to begin a new tick of the animation. - TickAnimation(PipelineId, AnimationTickType), + TickAnimation(PipelineId, bool /* script_animation_callbacks_present */), /// Dispatch a webdriver command WebDriverCommand(WebDriverCommandMsg), /// Reload the current page. diff --git a/tests/wpt/mozilla/tests/css/animations/basic-transition.html b/tests/wpt/mozilla/tests/css/animations/basic-transition.html index b80e8a666a6e..32a168ee65fb 100644 --- a/tests/wpt/mozilla/tests/css/animations/basic-transition.html +++ b/tests/wpt/mozilla/tests/css/animations/basic-transition.html @@ -19,16 +19,16 @@ var div = document.getElementById('check-me'); var span = div.childNodes[0]; async_test(function(t) { - window.addEventListener('load', function() { + window.addEventListener('load', t.step_func(function() { assert_equals(getComputedStyle(div).getPropertyValue('background-color'), 'rgb(255, 0, 0)'); div.id = "check-me-2"; - requestAnimationFrame(function() { + requestAnimationFrame(t.step_func(function() { var test = new window.TestBinding(); test.advanceClock(2000); span.innerHTML = "a"; assert_equals(getComputedStyle(div).getPropertyValue('background-color'), 'rgb(0, 0, 0)'); t.done(); - }); - }) -}) + })); + })); +});