Wait for actual paint before setting paint related metrics

components/metrics/lib.rs

@@ -3,15 +3,25 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 extern crate gfx;
+extern crate gfx_traits;
+extern crate ipc_channel;
+#[macro_use]
+extern crate log;
+extern crate msg;
 extern crate profile_traits;
+extern crate script_traits;
 extern crate servo_config;
-extern crate time;
 
 use gfx::display_list::{DisplayItem, DisplayList};
+use gfx_traits::Epoch;
+use ipc_channel::ipc::IpcSender;
+use msg::constellation_msg::PipelineId;
 use profile_traits::time::{ProfilerChan, ProfilerCategory, send_profile_data};
 use profile_traits::time::TimerMetadata;
+use script_traits::LayoutMsg;
 use servo_config::opts;
-use std::cell::Cell;
+use std::cell::{Cell, RefCell};
+use std::collections::HashMap;
 
 pub trait ProfilerMetadataFactory {
     fn new_metadata(&self) -> Option<TimerMetadata>;
@@ -19,23 +29,25 @@ pub trait ProfilerMetadataFactory {
 
 macro_rules! make_time_setter(
     ( $attr:ident, $func:ident, $category:ident, $label:expr ) => (
-        fn $func<T>(&self, profiler_metadata_factory: &T)
-            where T: ProfilerMetadataFactory {
+        fn $func(&self, profiler_metadata: Option<TimerMetadata>, paint_time: f64) {
+            if self.$attr.get().is_some() {
+                return;
+            }
+
             let navigation_start = match self.navigation_start {
                 Some(time) => time,
                 None => {
-                    println!("Trying to set metric before navigation start");
+                    warn!("Trying to set metric before navigation start");
                     return;
                 }
             };
 
-            let now = time::precise_time_ns() as f64;
-            let time = now - navigation_start;
+            let time = paint_time - navigation_start;
             self.$attr.set(Some(time));
 
             // Send the metric to the time profiler.
             send_profile_data(ProfilerCategory::$category,
-                              profiler_metadata_factory.new_metadata(),
+                              profiler_metadata,
                               &self.time_profiler_chan,
                               time as u64, time as u64, 0, 0);
 
@@ -48,20 +60,28 @@ macro_rules! make_time_setter(
 );
 
 pub struct PaintTimeMetrics {
+    pending_metrics: RefCell<HashMap<Epoch, (Option<TimerMetadata>, bool)>>,
     navigation_start: Option<f64>,
     first_paint: Cell<Option<f64>>,
     first_contentful_paint: Cell<Option<f64>>,
+    pipeline_id: PipelineId,
     time_profiler_chan: ProfilerChan,
+    constellation_chan: IpcSender<LayoutMsg>,
 }
 
 impl PaintTimeMetrics {
-    pub fn new(time_profiler_chan: ProfilerChan)
+    pub fn new(pipeline_id: PipelineId,
+               time_profiler_chan: ProfilerChan,
+               constellation_chan: IpcSender<LayoutMsg>)
         -> PaintTimeMetrics {
         PaintTimeMetrics {
+            pending_metrics: RefCell::new(HashMap::new()),
             navigation_start: None,
             first_paint: Cell::new(None),
             first_contentful_paint: Cell::new(None),
-            time_profiler_chan: time_profiler_chan,
+            pipeline_id,
+            time_profiler_chan,
+            constellation_chan,
         }
     }
 
@@ -76,39 +96,61 @@ impl PaintTimeMetrics {
                       TimeToFirstContentfulPaint,
                       "first-contentful-paint");
 
-    pub fn maybe_set_first_paint<T>(&self, profiler_metadata_factory: &T)
+    pub fn maybe_observe_paint_time<T>(&self,
+                                       profiler_metadata_factory: &T,
+                                       epoch: Epoch,
+                                       display_list: &DisplayList)
         where T: ProfilerMetadataFactory {
-        {
-            if self.first_paint.get().is_some() {
-                return;
-            }
+        if self.first_paint.get().is_some() && self.first_contentful_paint.get().is_some() {
+            // If we already set all paint metrics, we just bail out.
+            return;
         }
 
-        self.set_first_paint(profiler_metadata_factory);
-    }
-
-    pub fn maybe_set_first_contentful_paint<T>(&self, profiler_metadata_factory: &T,
-                                               display_list: &DisplayList)
-        where T: ProfilerMetadataFactory {
-        {
-            if self.first_contentful_paint.get().is_some() {
-                return;
-            }
-        }
-
-        // Analyze display list to figure out if this is the first contentful
-        // paint (i.e. the display list contains items of type text, image,
-        // non-white canvas or SVG)
+        let mut is_contentful = false;
+        // Analyze the display list to figure out if this may be the first
+        // contentful paint (i.e. the display list contains items of type text,
+        // image, non-white canvas or SVG).
         for item in &display_list.list {
             match item {
                 &DisplayItem::Text(_) |
                 &DisplayItem::Image(_) => {
-                    self.set_first_contentful_paint(profiler_metadata_factory);
-                    return;
+                    is_contentful = true;
+                    break;
                 },
                 _ => (),
             }
         }
+
+        self.pending_metrics.borrow_mut().insert(
+            epoch,
+            (profiler_metadata_factory.new_metadata(), is_contentful)
+        );
+
+        // Send the pending metric information to the compositor thread.
+        // The compositor will record the current time after painting the
+        // frame with the given ID and will send the metric back to us.
+        let msg = LayoutMsg::PendingPaintMetric(self.pipeline_id, epoch);
+        if let Err(e) = self.constellation_chan.send(msg) {
+            warn!("Failed to send PendingPaintMetric {:?}", e);
+        }
+    }
+
+    pub fn maybe_set_metric(&mut self, epoch: Epoch, paint_time: f64) {
+        if (self.first_paint.get().is_some() && self.first_contentful_paint.get().is_some()) ||
+           self.navigation_start.is_none() {
+            // If we already set all paint metrics or we have not set navigation start yet,
+            // we just bail out.
+            return;
+        }
+
+        if let Some(pending_metric) = self.pending_metrics.borrow_mut().remove(&epoch) {
+            let profiler_metadata = pending_metric.0;
+            self.set_first_paint(profiler_metadata.clone(), paint_time);
+            if pending_metric.1 {
+                self.set_first_contentful_paint(profiler_metadata, paint_time);
+            }
+        }
+
     }
 
     pub fn get_navigation_start(&self) -> Option<f64> {