Replace DisplayList::is_contentful with tracking during conversion to WR display lists

8 files changed, 53 insertions, 42 deletions

diff --git a/components/gfx_traits/lib.rs b/components/gfx_traits/lib.rs
index a3a85499a9e..165059c7996 100644
--- a/components/gfx_traits/lib.rs
+++ b/components/gfx_traits/lib.rs
@@ -103,8 +103,3 @@ pub fn node_id_from_scroll_id(id: usize) -> Option<usize> {
     }
     None
 }
-
-pub trait DisplayList {
-    /// Returns true if this display list contains meaningful content.
-    fn is_contentful(&self) -> bool;
-}
diff --git a/components/layout/display_list/items.rs b/components/layout/display_list/items.rs
index de7e49d306e..f838a95bf08 100644
--- a/components/layout/display_list/items.rs
+++ b/components/layout/display_list/items.rs
@@ -137,22 +137,6 @@ impl DisplayList {
     }
 }
 
-impl gfx_traits::DisplayList for DisplayList {
-    /// 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). Used by metrics.
-    fn is_contentful(&self) -> bool {
-        for item in &self.list {
-            match item {
-                &DisplayItem::Text(_) | &DisplayItem::Image(_) => return true,
-                _ => (),
-            }
-        }
-
-        false
-    }
-}
-
 /// Display list sections that make up a stacking context. Each section  here refers
 /// to the steps in CSS 2.1 Appendix E.
 ///
diff --git a/components/layout/display_list/webrender_helpers.rs b/components/layout/display_list/webrender_helpers.rs
index 64333f41977..993e2b38cef 100644
--- a/components/layout/display_list/webrender_helpers.rs
+++ b/components/layout/display_list/webrender_helpers.rs
@@ -24,8 +24,17 @@ struct ClipScrollState {
     active_spatial_id: SpatialId,
 }
 
+/// Contentful paint, for the purpose of
+/// https://w3c.github.io/paint-timing/#first-contentful-paint
+/// (i.e. the display list contains items of type text,
+/// image, non-white canvas or SVG). Used by metrics.
+pub struct IsContentful(pub bool);
+
 impl DisplayList {
-    pub fn convert_to_webrender(&mut self, pipeline_id: PipelineId) -> DisplayListBuilder {
+    pub fn convert_to_webrender(
+        &mut self,
+        pipeline_id: PipelineId,
+    ) -> (DisplayListBuilder, IsContentful) {
         let mut clip_ids = vec![None; self.clip_scroll_nodes.len()];
         let mut spatial_ids = vec![None; self.clip_scroll_nodes.len()];
 
@@ -54,11 +63,14 @@ impl DisplayList {
             1024 * 1024, // 1 MB of space
         );
 
+        let mut is_contentful = IsContentful(false);
         for item in &mut self.list {
-            item.convert_to_webrender(&self.clip_scroll_nodes, &mut state, &mut builder);
+            is_contentful.0 |= item
+                .convert_to_webrender(&self.clip_scroll_nodes, &mut state, &mut builder)
+                .0;
         }
 
-        builder
+        (builder, is_contentful)
     }
 }
 
@@ -68,7 +80,7 @@ impl DisplayItem {
         clip_scroll_nodes: &[ClipScrollNode],
         state: &mut ClipScrollState,
         builder: &mut DisplayListBuilder,
-    ) {
+    ) -> IsContentful {
         // Note: for each time of a display item, if we register one of `clip_ids` or `spatial_ids`,
         // we also register the other one as inherited from the current state or the stack.
         // This is not an ideal behavior, but it is compatible with the old WebRender model
@@ -96,15 +108,18 @@ impl DisplayItem {
             DisplayItem::Rectangle(ref mut item) => {
                 item.item.common = build_common_item_properties(&item.base, state);
                 builder.push_item(&WrDisplayItem::Rectangle(item.item));
+                IsContentful(false)
             },
             DisplayItem::Text(ref mut item) => {
                 item.item.common = build_common_item_properties(&item.base, state);
                 builder.push_item(&WrDisplayItem::Text(item.item));
                 builder.push_iter(item.data.iter());
+                IsContentful(true)
             },
             DisplayItem::Image(ref mut item) => {
                 item.item.common = build_common_item_properties(&item.base, state);
                 builder.push_item(&WrDisplayItem::Image(item.item));
+                IsContentful(true)
             },
             DisplayItem::Border(ref mut item) => {
                 item.item.common = build_common_item_properties(&item.base, state);
@@ -112,24 +127,29 @@ impl DisplayItem {
                     builder.push_stops(item.data.as_ref());
                 }
                 builder.push_item(&WrDisplayItem::Border(item.item));
+                IsContentful(false)
             },
             DisplayItem::Gradient(ref mut item) => {
                 item.item.common = build_common_item_properties(&item.base, state);
                 builder.push_stops(item.data.as_ref());
                 builder.push_item(&WrDisplayItem::Gradient(item.item));
+                IsContentful(false)
             },
             DisplayItem::RadialGradient(ref mut item) => {
                 item.item.common = build_common_item_properties(&item.base, state);
                 builder.push_stops(item.data.as_ref());
                 builder.push_item(&WrDisplayItem::RadialGradient(item.item));
+                IsContentful(false)
             },
             DisplayItem::Line(ref mut item) => {
                 item.item.common = build_common_item_properties(&item.base, state);
                 builder.push_item(&WrDisplayItem::Line(item.item));
+                IsContentful(false)
             },
             DisplayItem::BoxShadow(ref mut item) => {
                 item.item.common = build_common_item_properties(&item.base, state);
                 builder.push_item(&WrDisplayItem::BoxShadow(item.item));
+                IsContentful(false)
             },
             DisplayItem::PushTextShadow(ref mut item) => {
                 let common = build_common_item_properties(&item.base, state);
@@ -141,9 +161,11 @@ impl DisplayItem {
                     item.shadow,
                     true,
                 );
+                IsContentful(false)
             },
             DisplayItem::PopAllTextShadows(_) => {
                 builder.push_item(&WrDisplayItem::PopAllShadows);
+                IsContentful(false)
             },
             DisplayItem::Iframe(ref mut item) => {
                 let common = build_common_item_properties(&item.base, state);
@@ -157,6 +179,7 @@ impl DisplayItem {
                     item.iframe.to_webrender(),
                     true,
                 );
+                IsContentful(false)
             },
             DisplayItem::PushStackingContext(ref mut item) => {
                 let stacking_context = &item.stacking_context;
@@ -222,8 +245,12 @@ impl DisplayItem {
                 };
 
                 builder.push_item(&WrDisplayItem::PushStackingContext(wr_item));
+                IsContentful(false)
+            },
+            DisplayItem::PopStackingContext(_) => {
+                builder.pop_stacking_context();
+                IsContentful(false)
             },
-            DisplayItem::PopStackingContext(_) => builder.pop_stacking_context(),
             DisplayItem::DefineClipScrollNode(ref mut item) => {
                 let node = &clip_scroll_nodes[item.node_index.to_index()];
                 let item_rect = node.clip.main;
@@ -285,6 +312,7 @@ impl DisplayItem {
                         unreachable!("Found DefineClipScrollNode for Placeholder type node.");
                     },
                 };
+                IsContentful(false)
             },
         }
     }
diff --git a/components/layout_2020/display_list/items.rs b/components/layout_2020/display_list/items.rs
index 68ec3fcc188..8797480329d 100644
--- a/components/layout_2020/display_list/items.rs
+++ b/components/layout_2020/display_list/items.rs
@@ -3,7 +3,6 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
 use euclid::Vector2D;
-use gfx_traits;
 use std::collections::HashMap;
 use std::f32;
 use webrender_api::units::LayoutPixel;
@@ -14,11 +13,5 @@ pub use style::dom::OpaqueNode;
 #[derive(Serialize)]
 pub struct DisplayList {}
 
-impl gfx_traits::DisplayList for DisplayList {
-    fn is_contentful(&self) -> bool {
-        false
-    }
-}
-
 /// The type of the scroll offset list. This is only populated if WebRender is in use.
 pub type ScrollOffsetMap = HashMap<ExternalScrollId, Vector2D<f32, LayoutPixel>>;
diff --git a/components/layout_2020/display_list/webrender_helpers.rs b/components/layout_2020/display_list/webrender_helpers.rs
index c93bc05d269..951a0d61f95 100644
--- a/components/layout_2020/display_list/webrender_helpers.rs
+++ b/components/layout_2020/display_list/webrender_helpers.rs
@@ -7,8 +7,17 @@ use msg::constellation_msg::PipelineId;
 use webrender_api::units::LayoutSize;
 use webrender_api::{self, DisplayListBuilder};
 
+/// Contentful paint, for the purpose of
+/// https://w3c.github.io/paint-timing/#first-contentful-paint
+/// (i.e. the display list contains items of type text,
+/// image, non-white canvas or SVG). Used by metrics.
+pub struct IsContentful(pub bool);
+
 impl DisplayList {
-    pub fn convert_to_webrender(&mut self, pipeline_id: PipelineId) -> DisplayListBuilder {
+    pub fn convert_to_webrender(
+        &mut self,
+        pipeline_id: PipelineId,
+    ) -> (DisplayListBuilder, IsContentful) {
         let webrender_pipeline = pipeline_id.to_webrender();
 
         let builder = DisplayListBuilder::with_capacity(
@@ -17,6 +26,8 @@ impl DisplayList {
             1024 * 1024, // 1 MB of space
         );
 
-        builder
+        let is_contentful = IsContentful(false);
+
+        (builder, is_contentful)
     }
 }
diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs
index 5f65adcc245..e24360db2dc 100644
--- a/components/layout_thread/lib.rs
+++ b/components/layout_thread/lib.rs
@@ -1233,7 +1233,7 @@ impl LayoutThread {
                 debug!("Layout done!");
 
                 // TODO: Avoid the temporary conversion and build webrender sc/dl directly!
-                let builder = display_list.convert_to_webrender(self.id);
+                let (builder, is_contentful) = display_list.convert_to_webrender(self.id);
 
                 let viewport_size = Size2D::new(
                     self.viewport_size.width.to_f32_px(),
@@ -1250,7 +1250,7 @@ impl LayoutThread {
                 // sending the display list to WebRender in order to set time related
                 // Progressive Web Metrics.
                 self.paint_time_metrics
-                    .maybe_observe_paint_time(self, epoch, &*display_list);
+                    .maybe_observe_paint_time(self, epoch, is_contentful.0);
 
                 let mut txn = webrender_api::Transaction::new();
                 txn.set_display_list(
diff --git a/components/layout_thread_2020/lib.rs b/components/layout_thread_2020/lib.rs
index 6db5e2e8681..9c3e03c5ee0 100644
--- a/components/layout_thread_2020/lib.rs
+++ b/components/layout_thread_2020/lib.rs
@@ -1342,7 +1342,7 @@ impl LayoutThread {
         debug!("Layout done!");
 
         // TODO: Avoid the temporary conversion and build webrender sc/dl directly!
-        let builder = display_list.convert_to_webrender(self.id);
+        let (builder, is_contentful) = display_list.convert_to_webrender(self.id);
 
         let viewport_size = Size2D::new(
             self.viewport_size.width.to_f32_px(),
@@ -1359,7 +1359,7 @@ impl LayoutThread {
         // sending the display list to WebRender in order to set time related
         // Progressive Web Metrics.
         self.paint_time_metrics
-            .maybe_observe_paint_time(self, epoch, &display_list);
+            .maybe_observe_paint_time(self, epoch, is_contentful.0);
 
         let mut txn = webrender_api::Transaction::new();
         txn.set_display_list(
diff --git a/components/metrics/lib.rs b/components/metrics/lib.rs
index bfe4ad831c0..1165c8f45ca 100644
--- a/components/metrics/lib.rs
+++ b/components/metrics/lib.rs
@@ -7,7 +7,7 @@ extern crate log;
 #[macro_use]
 extern crate malloc_size_of_derive;
 
-use gfx_traits::{DisplayList, Epoch};
+use gfx_traits::Epoch;
 use ipc_channel::ipc::IpcSender;
 use msg::constellation_msg::PipelineId;
 use profile_traits::time::TimerMetadata;
@@ -306,7 +306,7 @@ impl PaintTimeMetrics {
         &self,
         profiler_metadata_factory: &T,
         epoch: Epoch,
-        display_list: &dyn DisplayList,
+        display_list_is_contentful: bool,
     ) where
         T: ProfilerMetadataFactory,
     {
@@ -319,7 +319,7 @@ impl PaintTimeMetrics {
             epoch,
             (
                 profiler_metadata_factory.new_metadata(),
-                display_list.is_contentful(),
+                display_list_is_contentful,
             ),
         );