Auto merge of #9663 - pcwalton:browser-html-jank-fix, r=glennw

compositing: Stop compositing unnecessarily after each animation frame.

Instead, schedule a delayed composite after each frame of an animation.

The previous code would cause jank, because the following sequence
frequently occurred:

1. The page uses `requestAnimationFrame()` to request a frame.

2. The compositor receives the message, schedules a composite,
dispatches the rAF message to the script thread, composites, and goes to
sleep waiting for vblank (frame 1).

3. The script makes a change and sends it through the pipeline.
Eventually it gets painted and is sent to the compositor, but the
compositor is sleeping.

4. The compositor wakes up, sees the new painted content, page flips,
and goes to sleep (frame 2). Repeat from step 1.

The problem is that we have two composition frames, not just one. This
halves Web apps' framerate!

This commit fixes the problem by scheduling the composite in step 2 to
12 ms in the future. We already have this delayed-composition
functionality in the form of the scrolling timer, which I repurposed and
renamed to the "delayed composition timer" for this task. This change
gives the page 12 ms to prepare the frame, which seems to usually be
enough, especially with WebRender.

Note that simply removing the scheduled composite after rAF is not the
correct solution. If this is done, then pages that call rAF and don't
modify the page won't receive future rAFs, since the compositor will be
sleeping and won't be notified of vblank.

Fixes a bunch of jank in browser.html. The remaining jank seems to be a
problem with browser.html itself.

r? @glennw

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9663)
<!-- Reviewable:end -->

7 files changed, 146 insertions, 115 deletions

diff --git a/components/compositing/compositor.rs b/components/compositing/compositor.rs
index 78609998112..1e9f29b014c 100644
--- a/components/compositing/compositor.rs
+++ b/components/compositing/compositor.rs
@@ -8,6 +8,7 @@ use compositor_layer::{CompositorData, CompositorLayer, RcCompositorLayer, Wants
 use compositor_thread::{CompositorEventListener, CompositorProxy};
 use compositor_thread::{CompositorReceiver, InitialCompositorState, Msg, RenderListener};
 use constellation::SendableFrameTree;
+use delayed_composition::DelayedCompositionTimerProxy;
 use euclid::point::TypedPoint2D;
 use euclid::rect::TypedRect;
 use euclid::scale_factor::ScaleFactor;
@@ -36,7 +37,6 @@ use profile_traits::time::{self, ProfilerCategory, profile};
 use script_traits::CompositorEvent::{MouseMoveEvent, MouseButtonEvent, TouchEvent};
 use script_traits::{AnimationState, ConstellationControlMsg, LayoutControlMsg};
 use script_traits::{MouseButton, MouseEventType, TouchEventType, TouchId};
-use scrolling::ScrollingTimerProxy;
 use std::collections::hash_map::Entry::{Occupied, Vacant};
 use std::collections::{HashMap, HashSet};
 use std::fs::File;
@@ -133,8 +133,8 @@ pub struct IOCompositor<Window: WindowMethods> {
 
     channel_to_self: Box<CompositorProxy + Send>,
 
-    /// A handle to the scrolling timer.
-    scrolling_timer: ScrollingTimerProxy,
+    /// A handle to the delayed composition timer.
+    delayed_composition_timer: DelayedCompositionTimerProxy,
 
     /// The type of composition to perform
     composite_target: CompositeTarget,
@@ -214,7 +214,7 @@ pub struct ScrollZoomEvent {
 #[derive(PartialEq, Debug)]
 enum CompositionRequest {
     NoCompositingNecessary,
-    CompositeOnScrollTimeout(u64),
+    DelayedComposite(u64),
     CompositeNow(CompositingReason),
 }
 
@@ -346,7 +346,7 @@ impl RenderNotifier {
 
 impl webrender_traits::RenderNotifier for RenderNotifier {
     fn new_frame_ready(&mut self) {
-        self.compositor_proxy.recomposite();
+        self.compositor_proxy.recomposite(CompositingReason::NewWebRenderFrame);
     }
 
     fn pipeline_size_changed(&mut self,
@@ -402,7 +402,7 @@ impl<Window: WindowMethods> IOCompositor<Window> {
             viewport: None,
             hidpi_factor: hidpi_factor,
             channel_to_self: state.sender.clone_compositor_proxy(),
-            scrolling_timer: ScrollingTimerProxy::new(state.sender),
+            delayed_composition_timer: DelayedCompositionTimerProxy::new(state.sender),
             composition_request: CompositionRequest::NoCompositingNecessary,
             touch_handler: TouchHandler::new(),
             pending_scroll_zoom_events: Vec::new(),
@@ -430,8 +430,7 @@ impl<Window: WindowMethods> IOCompositor<Window> {
         }
     }
 
-    pub fn create(window: Rc<Window>, state: InitialCompositorState)
-                  -> IOCompositor<Window> {
+    pub fn create(window: Rc<Window>, state: InitialCompositorState) -> IOCompositor<Window> {
         let mut compositor = IOCompositor::new(window, state);
 
         if let Some(ref mut webrender) = compositor.webrender {
@@ -475,7 +474,7 @@ impl<Window: WindowMethods> IOCompositor<Window> {
         // Tell the profiler, memory profiler, and scrolling timer to shut down.
         self.time_profiler_chan.send(time::ProfilerMsg::Exit);
         self.mem_profiler_chan.send(mem::ProfilerMsg::Exit);
-        self.scrolling_timer.shutdown();
+        self.delayed_composition_timer.shutdown();
 
         self.shutdown_state = ShutdownState::FinishedShuttingDown;
     }
@@ -598,20 +597,19 @@ impl<Window: WindowMethods> IOCompositor<Window> {
                 self.window.load_end(back, forward);
             }
 
-            (Msg::ScrollTimeout(timestamp), ShutdownState::NotShuttingDown) => {
-                debug!("scroll timeout, drawing unpainted content!");
-                if let CompositionRequest::CompositeOnScrollTimeout(this_timestamp) =
+            (Msg::DelayedCompositionTimeout(timestamp), ShutdownState::NotShuttingDown) => {
+                debug!("delayed composition timeout!");
+                if let CompositionRequest::DelayedComposite(this_timestamp) =
                     self.composition_request {
                     if timestamp == this_timestamp {
                         self.composition_request = CompositionRequest::CompositeNow(
-                            CompositingReason::HitScrollTimeout)
+                            CompositingReason::DelayedCompositeTimeout)
                     }
                 }
             }
 
-            (Msg::RecompositeAfterScroll, ShutdownState::NotShuttingDown) => {
-                self.composition_request =
-                    CompositionRequest::CompositeNow(CompositingReason::ContinueScroll)
+            (Msg::Recomposite(reason), ShutdownState::NotShuttingDown) => {
+                self.composition_request = CompositionRequest::CompositeNow(reason)
             }
 
             (Msg::KeyEvent(key, state, modified), ShutdownState::NotShuttingDown) => {
@@ -707,13 +705,11 @@ impl<Window: WindowMethods> IOCompositor<Window> {
                 self.composite_if_necessary(CompositingReason::Animation);
             }
             AnimationState::AnimationCallbacksPresent => {
-                if self.pipeline_details(pipeline_id).animation_callbacks_running {
-                    return
+                if !self.pipeline_details(pipeline_id).animation_callbacks_running {
+                    self.pipeline_details(pipeline_id).animation_callbacks_running =
+                        true;
+                    self.tick_animations_for_pipeline(pipeline_id);
                 }
-                self.pipeline_details(pipeline_id).animation_callbacks_running =
-                    true;
-                self.tick_animations_for_pipeline(pipeline_id);
-                self.composite_if_necessary(CompositingReason::Animation);
             }
             AnimationState::NoAnimationsPresent => {
                 self.pipeline_details(pipeline_id).animations_running = false;
@@ -1073,16 +1069,16 @@ impl<Window: WindowMethods> IOCompositor<Window> {
         }
     }
 
-    fn start_scrolling_timer_if_necessary(&mut self) {
+    fn schedule_delayed_composite_if_necessary(&mut self) {
         match self.composition_request {
             CompositionRequest::CompositeNow(_) |
-            CompositionRequest::CompositeOnScrollTimeout(_) => return,
+            CompositionRequest::DelayedComposite(_) => return,
             CompositionRequest::NoCompositingNecessary => {}
         }
 
         let timestamp = precise_time_ns();
-        self.scrolling_timer.scroll_event_processed(timestamp);
-        self.composition_request = CompositionRequest::CompositeOnScrollTimeout(timestamp);
+        self.delayed_composition_timer.schedule_composite(timestamp);
+        self.composition_request = CompositionRequest::DelayedComposite(timestamp);
     }
 
     fn assign_painted_buffers(&mut self,
@@ -1531,23 +1527,28 @@ impl<Window: WindowMethods> IOCompositor<Window> {
     fn perform_updates_after_scroll(&mut self) {
         self.send_updated_display_ports_to_layout();
         if self.send_buffer_requests_for_all_layers() {
-            self.start_scrolling_timer_if_necessary();
+            self.schedule_delayed_composite_if_necessary();
         } else {
-            self.channel_to_self.send(Msg::RecompositeAfterScroll);
+            self.channel_to_self.send(Msg::Recomposite(CompositingReason::ContinueScroll));
         }
     }
 
     /// If there are any animations running, dispatches appropriate messages to the constellation.
     fn process_animations(&mut self) {
+        let mut pipeline_ids = vec![];
         for (pipeline_id, pipeline_details) in &self.pipeline_details {
             if pipeline_details.animations_running ||
                pipeline_details.animation_callbacks_running {
-                self.tick_animations_for_pipeline(*pipeline_id)
+                pipeline_ids.push(*pipeline_id);
             }
         }
+        for pipeline_id in &pipeline_ids {
+            self.tick_animations_for_pipeline(*pipeline_id)
+        }
     }
 
-    fn tick_animations_for_pipeline(&self, pipeline_id: PipelineId) {
+    fn tick_animations_for_pipeline(&mut self, pipeline_id: PipelineId) {
+        self.schedule_delayed_composite_if_necessary();
         self.constellation_chan.send(ConstellationMsg::TickAnimation(pipeline_id)).unwrap()
     }
 
@@ -2262,7 +2263,7 @@ impl<Window> CompositorEventListener for IOCompositor<Window> where Window: Wind
 
         match self.composition_request {
             CompositionRequest::NoCompositingNecessary |
-            CompositionRequest::CompositeOnScrollTimeout(_) => {}
+            CompositionRequest::DelayedComposite(_) => {}
             CompositionRequest::CompositeNow(_) => {
                 self.composite()
             }
@@ -2296,7 +2297,7 @@ impl<Window> CompositorEventListener for IOCompositor<Window> where Window: Wind
             while self.shutdown_state != ShutdownState::ShuttingDown {
                 let msg = self.port.recv_compositor_msg();
                 let need_recomposite = match msg {
-                    Msg::RecompositeAfterScroll => true,
+                    Msg::Recomposite(_) => true,
                     _ => false,
                 };
                 let keep_going = self.handle_browser_message(msg);
@@ -2327,8 +2328,8 @@ impl<Window> CompositorEventListener for IOCompositor<Window> where Window: Wind
 /// Why we performed a composite. This is used for debugging.
 #[derive(Copy, Clone, PartialEq, Debug)]
 pub enum CompositingReason {
-    /// We hit the scroll timeout and are therefore drawing unrendered content.
-    HitScrollTimeout,
+    /// We hit the delayed composition timeout. (See `delayed_composition.rs`.)
+    DelayedCompositeTimeout,
     /// The window has been scrolled and we're starting the first recomposite.
     Scroll,
     /// A scroll has continued and we need to recomposite again.
@@ -2343,4 +2344,6 @@ pub enum CompositingReason {
     NewPaintedBuffers,
     /// The window has been zoomed.
     Zoom,
+    /// A new WebRender frame has arrived.
+    NewWebRenderFrame,
 }
diff --git a/components/compositing/compositor_thread.rs b/components/compositing/compositor_thread.rs
index f9f0c03d1e3..526f65a5d12 100644
--- a/components/compositing/compositor_thread.rs
+++ b/components/compositing/compositor_thread.rs
@@ -5,7 +5,7 @@
 //! Communication with the compositor thread.
 
 use CompositorMsg as ConstellationMsg;
-use compositor;
+use compositor::{self, CompositingReason};
 use euclid::point::Point2D;
 use euclid::size::Size2D;
 use gfx_traits::{Epoch, FrameTreeId, LayerId, LayerProperties, PaintListener};
@@ -103,12 +103,12 @@ pub fn run_script_listener_thread(compositor_proxy: Box<CompositorProxy + 'stati
 }
 
 pub trait RenderListener {
-    fn recomposite(&mut self);
+    fn recomposite(&mut self, reason: CompositingReason);
 }
 
 impl RenderListener for Box<CompositorProxy + 'static> {
-    fn recomposite(&mut self) {
-        self.send(Msg::RecompositeAfterScroll);
+    fn recomposite(&mut self, reason: CompositingReason) {
+        self.send(Msg::Recomposite(reason));
     }
 }
 
@@ -197,10 +197,10 @@ pub enum Msg {
     LoadStart(bool, bool),
     /// The load of a page has completed: (can go back, can go forward).
     LoadComplete(bool, bool),
-    /// Indicates that the scrolling timeout with the given starting timestamp has happened and a
-    /// composite should happen. (See the `scrolling` module.)
-    ScrollTimeout(u64),
-    RecompositeAfterScroll,
+    /// We hit the delayed composition timeout. (See `delayed_composition.rs`.)
+    DelayedCompositionTimeout(u64),
+    /// Composite.
+    Recomposite(CompositingReason),
     /// Sends an unconsumed key event back to the compositor.
     KeyEvent(Key, KeyState, KeyModifiers),
     /// Script has handled a touch event, and either prevented or allowed default actions.
@@ -251,8 +251,8 @@ impl Debug for Msg {
             Msg::SetFrameTree(..) => write!(f, "SetFrameTree"),
             Msg::LoadComplete(..) => write!(f, "LoadComplete"),
             Msg::LoadStart(..) => write!(f, "LoadStart"),
-            Msg::ScrollTimeout(..) => write!(f, "ScrollTimeout"),
-            Msg::RecompositeAfterScroll => write!(f, "RecompositeAfterScroll"),
+            Msg::DelayedCompositionTimeout(..) => write!(f, "DelayedCompositionTimeout"),
+            Msg::Recomposite(..) => write!(f, "Recomposite"),
             Msg::KeyEvent(..) => write!(f, "KeyEvent"),
             Msg::TouchEventProcessed(..) => write!(f, "TouchEventProcessed"),
             Msg::SetCursor(..) => write!(f, "SetCursor"),
diff --git a/components/compositing/delayed_composition.rs b/components/compositing/delayed_composition.rs
new file mode 100644
index 00000000000..bc4b933f1bd
--- /dev/null
+++ b/components/compositing/delayed_composition.rs
@@ -0,0 +1,94 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+//! A timer thread that composites near the end of the frame.
+//!
+//! This is useful when we need to composite next frame but we want to opportunistically give the
+//! painting thread time to paint if it can.
+
+use compositor_thread::{CompositorProxy, Msg};
+use std::sync::mpsc::{Receiver, Sender, channel};
+use std::thread::{self, Builder};
+use time;
+use util::time::duration_from_nanoseconds;
+
+/// The amount of time in nanoseconds that we give to the painting thread to paint. When this
+/// expires, we give up and composite anyway.
+static TIMEOUT: u64 = 12_000_000;
+
+pub struct DelayedCompositionTimerProxy {
+    sender: Sender<ToDelayedCompositionTimerMsg>,
+}
+
+pub struct DelayedCompositionTimer {
+    compositor_proxy: Box<CompositorProxy>,
+    receiver: Receiver<ToDelayedCompositionTimerMsg>,
+}
+
+enum ToDelayedCompositionTimerMsg {
+    Exit,
+    ScheduleComposite(u64),
+}
+
+impl DelayedCompositionTimerProxy {
+    pub fn new(compositor_proxy: Box<CompositorProxy + Send>) -> DelayedCompositionTimerProxy {
+        let (to_timer_sender, to_timer_receiver) = channel();
+        Builder::new().spawn(move || {
+            let mut timer = DelayedCompositionTimer {
+                compositor_proxy: compositor_proxy,
+                receiver: to_timer_receiver,
+            };
+            timer.run();
+        }).unwrap();
+        DelayedCompositionTimerProxy {
+            sender: to_timer_sender,
+        }
+    }
+
+    pub fn schedule_composite(&mut self, timestamp: u64) {
+        self.sender.send(ToDelayedCompositionTimerMsg::ScheduleComposite(timestamp)).unwrap()
+    }
+
+    pub fn shutdown(&mut self) {
+        self.sender.send(ToDelayedCompositionTimerMsg::Exit).unwrap()
+    }
+}
+
+impl DelayedCompositionTimer {
+    pub fn run(&mut self) {
+        'outer: loop {
+            let mut timestamp;
+            loop {
+                match self.receiver.recv() {
+                    Ok(ToDelayedCompositionTimerMsg::ScheduleComposite(this_timestamp)) => {
+                        timestamp = this_timestamp;
+                        break
+                    }
+                    Ok(ToDelayedCompositionTimerMsg::Exit) => break 'outer,
+                    _ => break 'outer,
+                }
+            }
+
+            // Drain all messages from the queue.
+            loop {
+                match self.receiver.try_recv() {
+                    Ok(ToDelayedCompositionTimerMsg::ScheduleComposite(this_timestamp)) => {
+                        timestamp = this_timestamp;
+                        break
+                    }
+                    _ => break,
+                }
+            }
+
+            let target = timestamp + TIMEOUT;
+            let now = time::precise_time_ns();
+            if target > now {
+                let delta_ns = target - now;
+                thread::sleep(duration_from_nanoseconds(delta_ns));
+            }
+            self.compositor_proxy.send(Msg::DelayedCompositionTimeout(timestamp));
+        }
+    }
+}
+
diff --git a/components/compositing/headless.rs b/components/compositing/headless.rs
index e575baade2a..78be2671f5c 100644
--- a/components/compositing/headless.rs
+++ b/components/compositing/headless.rs
@@ -114,8 +114,8 @@ impl CompositorEventListener for NullCompositor {
             Msg::Status(..) |
             Msg::LoadStart(..) |
             Msg::LoadComplete(..) |
-            Msg::ScrollTimeout(..) |
-            Msg::RecompositeAfterScroll |
+            Msg::DelayedCompositionTimeout(..) |
+            Msg::Recomposite(..) |
             Msg::ChangePageTitle(..) |
             Msg::ChangePageUrl(..) |
             Msg::KeyEvent(..) |
diff --git a/components/compositing/lib.rs b/components/compositing/lib.rs
index 410b007a3a6..bfeecd734e3 100644
--- a/components/compositing/lib.rs
+++ b/components/compositing/lib.rs
@@ -66,11 +66,11 @@ mod compositor;
 mod compositor_layer;
 pub mod compositor_thread;
 pub mod constellation;
+mod delayed_composition;
 mod headless;
 pub mod pipeline;
 #[cfg(not(target_os = "windows"))]
 pub mod sandboxing;
-mod scrolling;
 mod surface_map;
 mod timer_scheduler;
 mod touch;
diff --git a/components/compositing/scrolling.rs b/components/compositing/scrolling.rs
deleted file mode 100644
index b0551b6722c..00000000000
--- a/components/compositing/scrolling.rs
+++ /dev/null
@@ -1,66 +0,0 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-//! A timer thread that gives the painting thread a little time to catch up when the user scrolls.
-
-use compositor_thread::{CompositorProxy, Msg};
-use std::sync::mpsc::{Receiver, Sender, channel};
-use std::thread::{self, Builder};
-use time;
-use util::time::duration_from_nanoseconds;
-
-/// The amount of time in nanoseconds that we give to the painting thread to paint new tiles upon
-/// processing a scroll event that caused new tiles to be revealed. When this expires, we give up
-/// and composite anyway (showing a "checkerboard") to avoid dropping the frame.
-static TIMEOUT: u64 = 12_000_000;
-
-pub struct ScrollingTimerProxy {
-    sender: Sender<ToScrollingTimerMsg>,
-}
-
-pub struct ScrollingTimer {
-    compositor_proxy: Box<CompositorProxy>,
-    receiver: Receiver<ToScrollingTimerMsg>,
-}
-
-enum ToScrollingTimerMsg {
-    ExitMsg,
-    ScrollEventProcessedMsg(u64),
-}
-
-impl ScrollingTimerProxy {
-    pub fn new(compositor_proxy: Box<CompositorProxy + Send>) -> ScrollingTimerProxy {
-        let (to_scrolling_timer_sender, to_scrolling_timer_receiver) = channel();
-        Builder::new().spawn(move || {
-            let mut scrolling_timer = ScrollingTimer {
-                compositor_proxy: compositor_proxy,
-                receiver: to_scrolling_timer_receiver,
-            };
-            scrolling_timer.run();
-        }).unwrap();
-        ScrollingTimerProxy {
-            sender: to_scrolling_timer_sender,
-        }
-    }
-
-    pub fn scroll_event_processed(&mut self, timestamp: u64) {
-        self.sender.send(ToScrollingTimerMsg::ScrollEventProcessedMsg(timestamp)).unwrap()
-    }
-
-    pub fn shutdown(&mut self) {
-        self.sender.send(ToScrollingTimerMsg::ExitMsg).unwrap()
-    }
-}
-
-impl ScrollingTimer {
-    pub fn run(&mut self) {
-        while let Ok(ToScrollingTimerMsg::ScrollEventProcessedMsg(timestamp)) = self.receiver.recv() {
-            let target = timestamp + TIMEOUT;
-            let delta_ns = target - time::precise_time_ns();
-            thread::sleep(duration_from_nanoseconds(delta_ns));
-            self.compositor_proxy.send(Msg::ScrollTimeout(timestamp));
-        }
-    }
-}
-
diff --git a/components/layout/layout_thread.rs b/components/layout/layout_thread.rs
index b574e2d62fd..999a979f028 100644
--- a/components/layout/layout_thread.rs
+++ b/components/layout/layout_thread.rs
@@ -1196,8 +1196,8 @@ impl LayoutThread {
         self.tick_animations(&mut rw_data);
 
         self.script_chan
-          .send(ConstellationControlMsg::TickAllAnimations(self.id))
-          .unwrap();
+            .send(ConstellationControlMsg::TickAllAnimations(self.id))
+            .unwrap();
     }
 
     pub fn tick_animations(&mut self, rw_data: &mut LayoutThreadData) {