Improve decisions in compositor over when to draw a frame.

components/compositing/compositor.rs

@@ -6,7 +6,6 @@ use CompositionPipeline;
 use SendableFrameTree;
 use compositor_thread::{CompositorProxy, CompositorReceiver};
 use compositor_thread::{InitialCompositorState, Msg, RenderListener};
-use delayed_composition::DelayedCompositionTimerProxy;
 use euclid::{Point2D, TypedPoint2D, TypedVector2D, TypedRect, ScaleFactor, TypedSize2D};
 use gfx_traits::Epoch;
 use gleam::gl;
@@ -131,9 +130,6 @@ pub struct IOCompositor<Window: WindowMethods> {
 
     channel_to_self: CompositorProxy,
 
-    /// A handle to the delayed composition timer.
-    delayed_composition_timer: DelayedCompositionTimerProxy,
-
     /// The type of composition to perform
     composite_target: CompositeTarget,
 
@@ -207,7 +203,6 @@ struct ScrollZoomEvent {
 #[derive(PartialEq, Debug)]
 enum CompositionRequest {
     NoCompositingNecessary,
-    DelayedComposite(u64),
     CompositeNow(CompositingReason),
 }
 
@@ -363,7 +358,6 @@ impl<Window: WindowMethods> IOCompositor<Window> {
             scale: ScaleFactor::new(1.0),
             scale_factor: scale_factor,
             channel_to_self: state.sender.clone_compositor_proxy(),
-            delayed_composition_timer: DelayedCompositionTimerProxy::new(state.sender),
             composition_request: CompositionRequest::NoCompositingNecessary,
             touch_handler: TouchHandler::new(),
             pending_scroll_zoom_events: Vec::new(),
@@ -437,8 +431,6 @@ impl<Window: WindowMethods> IOCompositor<Window> {
             let _ = receiver.recv();
         }
 
-        self.delayed_composition_timer.shutdown();
-
         self.shutdown_state = ShutdownState::FinishedShuttingDown;
     }
 
@@ -524,16 +516,6 @@ impl<Window: WindowMethods> IOCompositor<Window> {
                 }
             }
 
-            (Msg::DelayedCompositionTimeout(timestamp), ShutdownState::NotShuttingDown) => {
-                if let CompositionRequest::DelayedComposite(this_timestamp) =
-                    self.composition_request {
-                    if timestamp == this_timestamp {
-                        self.composition_request = CompositionRequest::CompositeNow(
-                            CompositingReason::DelayedCompositeTimeout)
-                    }
-                }
-            }
-
             (Msg::Recomposite(reason), ShutdownState::NotShuttingDown) => {
                 self.composition_request = CompositionRequest::CompositeNow(reason)
             }
@@ -753,18 +735,6 @@ impl<Window: WindowMethods> IOCompositor<Window> {
         }
     }
 
-    fn schedule_delayed_composite_if_necessary(&mut self) {
-        match self.composition_request {
-            CompositionRequest::CompositeNow(_) => return,
-            CompositionRequest::DelayedComposite(_) |
-            CompositionRequest::NoCompositingNecessary => {}
-        }
-
-        let timestamp = precise_time_ns();
-        self.delayed_composition_timer.schedule_composite(timestamp);
-        self.composition_request = CompositionRequest::DelayedComposite(timestamp);
-    }
-
     fn scroll_fragment_to_point(&mut self, id: ClipId, point: Point2D<f32>) {
         self.webrender_api.scroll_node_with_id(LayoutPoint::from_untyped(&point), id,
                                                ScrollClamping::ToContentBounds);
@@ -1235,13 +1205,18 @@ impl<Window: WindowMethods> IOCompositor<Window> {
                    pipeline_ids.push(*pipeline_id);
             }
         }
+        let animation_state = if pipeline_ids.is_empty() {
+            windowing::AnimationState::Idle
+        } else {
+            windowing::AnimationState::Animating
+        };
+        self.window.set_animation_state(animation_state);
         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;
         if animation_callbacks_running {
             let msg = ConstellationMsg::TickAnimation(pipeline_id, AnimationTickType::Script);
@@ -1635,14 +1610,6 @@ impl<Window: WindowMethods> IOCompositor<Window> {
                 _ => compositor_messages.push(msg),
             }
         }
-        if found_recomposite_msg {
-            compositor_messages.retain(|msg| {
-                match *msg {
-                    Msg::DelayedCompositionTimeout(_) => false,
-                    _ => true,
-                }
-            })
-        }
         for msg in compositor_messages {
             if !self.handle_browser_message(msg) {
                 break
@@ -1664,8 +1631,7 @@ impl<Window: WindowMethods> IOCompositor<Window> {
         }
 
         match self.composition_request {
-            CompositionRequest::NoCompositingNecessary |
-            CompositionRequest::DelayedComposite(_) => {}
+            CompositionRequest::NoCompositingNecessary => {}
             CompositionRequest::CompositeNow(_) => {
                 self.composite()
             }

components/compositing/compositor_thread.rs

@@ -100,8 +100,6 @@ pub enum Msg {
     HistoryChanged(Vec<LoadData>, usize),
     /// Wether or not to follow a link
     AllowNavigation(ServoUrl, IpcSender<bool>),
-    /// We hit the delayed composition timeout. (See `delayed_composition.rs`.)
-    DelayedCompositionTimeout(u64),
     /// Composite.
     Recomposite(CompositingReason),
     /// Sends an unconsumed key event back to the compositor.
@@ -160,7 +158,6 @@ impl Debug for Msg {
             Msg::AllowNavigation(..) => write!(f, "AllowNavigation"),
             Msg::LoadStart => write!(f, "LoadStart"),
             Msg::HistoryChanged(..) => write!(f, "HistoryChanged"),
-            Msg::DelayedCompositionTimeout(..) => write!(f, "DelayedCompositionTimeout"),
             Msg::Recomposite(..) => write!(f, "Recomposite"),
             Msg::KeyEvent(..) => write!(f, "KeyEvent"),
             Msg::TouchEventProcessed(..) => write!(f, "TouchEventProcessed"),

components/compositing/lib.rs

@@ -35,7 +35,6 @@ use style_traits::CSSPixel;
 
 mod compositor;
 pub mod compositor_thread;
-mod delayed_composition;
 mod touch;
 pub mod windowing;
 

components/compositing/windowing.rs

@@ -102,6 +102,12 @@ impl Debug for WindowEvent {
     }
 }
 
+#[derive(Debug, Copy, Clone, PartialEq)]
+pub enum AnimationState {
+    Idle,
+    Animating,
+}
+
 pub trait WindowMethods {
     /// Returns the rendering area size in hardware pixels.
     fn framebuffer_size(&self) -> TypedSize2D<u32, DevicePixel>;
@@ -163,4 +169,10 @@ pub trait WindowMethods {
 
     /// Return the GL function pointer trait.
     fn gl(&self) -> Rc<gl::Gl>;
+
+    /// Set whether the application is currently animating.
+    /// Typically, when animations are active, the window
+    /// will want to avoid blocking on UI events, and just
+    /// run the event loop at the vsync interval.
+    fn set_animation_state(&self, _state: AnimationState) {}
 }

components/script/dom/document.rs

@@ -1538,28 +1538,29 @@ impl Document {
         self.animation_frame_ident.set(ident);
         self.animation_frame_list.borrow_mut().push((ident, Some(callback)));
 
-        // No need to send a `ChangeRunningAnimationsState` if we're running animation callbacks:
-        // we're guaranteed to already be in the "animation callbacks present" state.
-        //
-        // This reduces CPU usage by avoiding needless thread wakeups in the common case of
-        // repeated rAF.
-        //
         // TODO: Should tick animation only when document is visible
-        if !self.running_animation_callbacks.get() {
-            if !self.is_faking_animation_frames() {
-                let global_scope = self.window.upcast::<GlobalScope>();
-                let event = ConstellationMsg::ChangeRunningAnimationsState(
-                    global_scope.pipeline_id(),
-                    AnimationState::AnimationCallbacksPresent);
-                global_scope.constellation_chan().send(event).unwrap();
-            } else {
-                let callback = FakeRequestAnimationFrameCallback {
-                    document: Trusted::new(self),
-                };
-                self.global()
-                    .schedule_callback(OneshotTimerCallback::FakeRequestAnimationFrame(callback),
-                                       MsDuration::new(FAKE_REQUEST_ANIMATION_FRAME_DELAY));
-            }
+
+        // If we are running 'fake' animation frames, we unconditionally
+        // set up a one-shot timer for script to execute the rAF callbacks.
+        if self.is_faking_animation_frames() {
+            let callback = FakeRequestAnimationFrameCallback {
+                document: Trusted::new(self),
+            };
+            self.global()
+                .schedule_callback(OneshotTimerCallback::FakeRequestAnimationFrame(callback),
+                                   MsDuration::new(FAKE_REQUEST_ANIMATION_FRAME_DELAY));
+        } else if !self.running_animation_callbacks.get() {
+            // No need to send a `ChangeRunningAnimationsState` if we're running animation callbacks:
+            // we're guaranteed to already be in the "animation callbacks present" state.
+            //
+            // This reduces CPU usage by avoiding needless thread wakeups in the common case of
+            // repeated rAF.
+
+            let global_scope = self.window.upcast::<GlobalScope>();
+            let event = ConstellationMsg::ChangeRunningAnimationsState(
+                global_scope.pipeline_id(),
+                AnimationState::AnimationCallbacksPresent);
+            global_scope.constellation_chan().send(event).unwrap();
         }
 
         ident
@@ -1596,6 +1597,22 @@ impl Document {
                                            ReflowQueryType::NoQuery,
                                            ReflowReason::RequestAnimationFrame);
 
+        if spurious && !was_faking_animation_frames {
+            // If the rAF callbacks did not mutate the DOM, then the
+            // reflow call above means that layout will not be invoked,
+            // and therefore no new frame will be sent to the compositor.
+            // If this happens, the compositor will not tick the animation
+            // and the next rAF will never be called! When this happens
+            // for several frames, then the spurious rAF detection below
+            // will kick in and use a timer to tick the callbacks. However,
+            // for the interim frames where we are deciding whether this rAF
+            // is considered spurious, we need to ensure that the layout
+            // and compositor *do* tick the animation.
+            self.window.force_reflow(ReflowGoal::ForDisplay,
+                                     ReflowQueryType::NoQuery,
+                                     ReflowReason::RequestAnimationFrame);
+        }
+
         // Only send the animation change state message after running any callbacks.
         // This means that if the animation callback adds a new callback for
         // the next frame (which is the common case), we won't send a NoAnimationCallbacksPresent