Use a trait object to handle notifications.

webrender/src/render_backend.rs

@@ -9,7 +9,7 @@ use api::{DeviceIntPoint, DevicePixelScale, DeviceUintPoint, DeviceUintRect, Dev
 use api::{DocumentId, DocumentLayer, ExternalScrollId, FrameMsg, HitTestFlags, HitTestResult};
 use api::{IdNamespace, LayoutPoint, PipelineId, RenderNotifier, SceneMsg, ScrollClamping};
 use api::{ScrollLocation, ScrollNodeState, TransactionMsg, ResourceUpdate, ImageKey};
-use api::ExternalEvent;
+use api::{NotificationRequest, Checkpoint};
 use api::channel::{MsgReceiver, Payload};
 #[cfg(feature = "capture")]
 use api::CaptureBits;
@@ -45,6 +45,7 @@ use std::u32;
 #[cfg(feature = "replay")]
 use tiling::Frame;
 use time::precise_time_ns;
+use util::drain_filter;
 
 #[cfg_attr(feature = "capture", derive(Serialize))]
 #[cfg_attr(feature = "replay", derive(Deserialize))]
@@ -803,7 +804,7 @@ impl RenderBackend {
             resource_updates: transaction_msg.resource_updates,
             frame_ops: transaction_msg.frame_ops,
             rasterized_blobs: Vec::new(),
-            notifications: Vec::new(),
+            notifications: transaction_msg.notifications,
             set_root_pipeline: None,
             build_frame: transaction_msg.generate_frame,
             render_frame: transaction_msg.generate_frame,
@@ -876,7 +877,7 @@ impl RenderBackend {
         document_id: DocumentId,
         resource_updates: Vec<ResourceUpdate>,
         mut frame_ops: Vec<FrameMsg>,
-        notifications: Vec<ExternalEvent>,
+        mut notifications: Vec<NotificationRequest>,
         mut build_frame: bool,
         mut render_frame: bool,
         frame_counter: &mut u32,
@@ -986,9 +987,11 @@ impl RenderBackend {
             self.result_tx.send(msg).unwrap();
         }
 
-        for evt in notifications {
-            self.notifier.external_event(evt)
-        }
+        drain_filter(
+            &mut notifications,
+            |n| { n.when() == Checkpoint::FrameBuilt },
+            |n| { n.notify(); },
+        );
 
         // Always forward the transaction to the renderer if a frame was requested,
         // otherwise gecko can get into a state where it waits (forever) for the

webrender/src/scene_builder.rs

@@ -4,7 +4,7 @@
 
 use api::{AsyncBlobImageRasterizer, BlobImageRequest, BlobImageParams, BlobImageResult};
 use api::{DocumentId, PipelineId, ApiMsg, FrameMsg, ResourceUpdate, Epoch};
-use api::{BuiltDisplayList, ColorF, LayoutSize, ExternalEvent};
+use api::{BuiltDisplayList, ColorF, LayoutSize, NotificationRequest, Checkpoint};
 use api::channel::MsgSender;
 use frame_builder::{FrameBuilderConfig, FrameBuilder};
 use clip_scroll_tree::ClipScrollTree;
@@ -17,6 +17,7 @@ use scene::Scene;
 use std::sync::mpsc::{channel, Receiver, Sender};
 use std::mem::replace;
 use time::precise_time_ns;
+use util::drain_filter;
 
 /// Represents the work associated to a transaction before scene building.
 pub struct Transaction {
@@ -30,7 +31,7 @@ pub struct Transaction {
     pub rasterized_blobs: Vec<(BlobImageRequest, BlobImageResult)>,
     pub resource_updates: Vec<ResourceUpdate>,
     pub frame_ops: Vec<FrameMsg>,
-    pub notifications: Vec<ExternalEvent>,
+    pub notifications: Vec<NotificationRequest>,
     pub set_root_pipeline: Option<PipelineId>,
     pub build_frame: bool,
     pub render_frame: bool,
@@ -62,7 +63,7 @@ pub struct BuiltTransaction {
     pub blob_rasterizer: Option<Box<AsyncBlobImageRasterizer>>,
     pub frame_ops: Vec<FrameMsg>,
     pub removed_pipelines: Vec<PipelineId>,
-    pub notifications: Vec<ExternalEvent>,
+    pub notifications: Vec<NotificationRequest>,
     pub scene_build_start_time: u64,
     pub scene_build_end_time: u64,
     pub build_frame: bool,
@@ -325,6 +326,12 @@ impl SceneBuilder {
         );
         rasterized_blobs.append(&mut txn.rasterized_blobs);
 
+        drain_filter(
+            &mut txn.notifications,
+            |n| { n.when() == Checkpoint::SceneBuilt },
+            |n| { n.notify(); },
+        );
+
         Box::new(BuiltTransaction {
             document_id: txn.document_id,
             build_frame: txn.build_frame || built_scene.is_some(),

webrender_api/src/api.rs

@@ -10,6 +10,7 @@ use std::cell::Cell;
 use std::fmt;
 use std::marker::PhantomData;
 use std::path::PathBuf;
+use std::sync::Arc;
 use std::u32;
 use {BuiltDisplayList, BuiltDisplayListDescriptor, ColorF, DeviceIntPoint, DeviceUintRect};
 use {DeviceUintSize, ExternalScrollId, FontInstanceKey, FontInstanceOptions};
@@ -48,7 +49,7 @@ pub struct Transaction {
     // Additional display list data.
     payloads: Vec<Payload>,
 
-    notifications: Vec<ExternalEvent>,
+    notifications: Vec<NotificationRequest>,
 
     // Resource updates are applied after scene building.
     pub resource_updates: Vec<ResourceUpdate>,
@@ -178,12 +179,15 @@ impl Transaction {
     // Note: Gecko uses this to get notified when a transaction that contains
     // potentially long blob rasterization or scene build is ready to be rendered.
     // so that the tab-switching integration can react adequately when tab
-    // switching takes too long. For this use case what matters is that the
+    // switching takes too long. For this use case when matters is that the
     // notification doesn't fire before scene building and blob rasterization.
 
-    /// Trigger the external event notification when the transaction gets passed
-    /// the frame building stage.
-    pub fn notify(&mut self, event: ExternalEvent) {
+    /// Trigger a notification at a certain stage of the rendering pipeline.
+    ///
+    /// Not that notification requests are skipped during serialization, so is is
+    /// best to use them for synchronization purposes and not for things that could
+    /// affect the WebRender's state.
+    pub fn notify(&mut self, event: NotificationRequest) {
         self.notifications.push(event);
     }
 
@@ -235,7 +239,7 @@ impl Transaction {
     /// in `webrender::Renderer`, [new_frame_ready()][notifier] gets called.
     /// Note that the notifier is called even if the frame generation was a
     /// no-op; the arguments passed to `new_frame_ready` will provide information
-    /// as to what happened.
+    /// as to when happened.
     ///
     /// [notifier]: trait.RenderNotifier.html#tymethod.new_frame_ready
     pub fn generate_frame(&mut self) {
@@ -385,10 +389,12 @@ pub struct TransactionMsg {
     pub scene_ops: Vec<SceneMsg>,
     pub frame_ops: Vec<FrameMsg>,
     pub resource_updates: Vec<ResourceUpdate>,
-    pub notifications: Vec<ExternalEvent>,
     pub generate_frame: bool,
     pub use_scene_builder_thread: bool,
     pub low_priority: bool,
+
+    #[serde(skip)]
+    pub notifications: Vec<NotificationRequest>,
 }
 
 impl TransactionMsg {
@@ -1161,3 +1167,48 @@ pub trait RenderNotifier: Send {
     }
     fn shut_down(&self) {}
 }
+
+#[derive(Copy, Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
+pub enum Checkpoint {
+    SceneBuilt,
+    FrameBuilt,
+    /// NotificationRequests get notified with this if they get dropped without having been
+    /// notified. This provides the guarantee that if a request is created it will get notified.
+    TransactionDropped,
+}
+
+pub trait NotificationHandler : Send + Sync {
+    fn notify(&self, when: Checkpoint);
+}
+
+#[derive(Clone)]
+pub struct NotificationRequest {
+    handler: Arc<NotificationHandler>,
+    when: Checkpoint,
+    done: bool,
+}
+
+impl NotificationRequest {
+    pub fn new(when: Checkpoint, handler: Arc<NotificationHandler>) -> Self {
+        NotificationRequest {
+            handler,
+            when,
+            done: false,
+        }
+    }
+
+    pub fn when(&self) -> Checkpoint { self.when }
+
+    pub fn notify(mut self) {
+        self.handler.notify(self.when);
+        self.done = true;
+    }
+}
+
+impl Drop for NotificationRequest {
+    fn drop(&mut self) {
+        if !self.done {
+            self.handler.notify(Checkpoint::TransactionDropped);
+        }
+    }
+}