Address review comments.

webrender/src/frame.rs

@@ -23,10 +23,10 @@ use profiler::{GpuCacheProfileCounters, TextureCacheProfileCounters};
 use resource_cache::{FontInstanceMap,ResourceCache, TiledImageMap};
 use scene::{Scene, StackingContextHelpers, ScenePipeline, SceneProperties};
 use scene_builder::{SceneRequest, BuiltScene};
+use std::sync::Arc;
 use tiling::{CompositeOps, Frame};
 use renderer::PipelineInfo;
 
-use std::sync::Arc;
 
 #[derive(Copy, Clone, PartialEq, PartialOrd, Debug, Eq, Ord)]
 #[cfg_attr(feature = "capture", derive(Serialize))]
@@ -1201,7 +1201,7 @@ impl FrameContext {
         pan: WorldPoint,
         texture_cache_profile: &mut TextureCacheProfileCounters,
         gpu_cache_profile: &mut GpuCacheProfileCounters,
-		scene_properties: &SceneProperties,
+        scene_properties: &SceneProperties,
         removed_pipelines: Vec<PipelineId>,
     ) -> (HitTester, RenderedDocument) {
         let frame = frame_builder.build(
@@ -1252,7 +1252,7 @@ pub fn build_scene(
 
         let mut roller = FlattenContext {
             scene,
-            // WIP, we're not really recycling anything here, clean this up.
+            // TODO, we're not really recycling anything here, clean this up.
             builder: FrameBuilder::empty().recycle(
                 inner_rect,
                 background_color,

webrender/src/record.rs

@@ -67,13 +67,6 @@ pub fn should_record_msg(msg: &ApiMsg) -> bool {
         ApiMsg::AddDocument { .. } |
         ApiMsg::DeleteDocument(..) => true,
         ApiMsg::UpdateDocument(_, ref msgs) => {
-            for msg in &msgs.scene_ops {
-                match *msg {
-                    DocumentMsg::GetScrollNodeState(..) |
-                    DocumentMsg::HitTest(..) => {}
-                    _ => { return true; }
-                }
-            }
             for msg in &msgs.frame_ops {
                 match *msg {
                     DocumentMsg::GetScrollNodeState(..) |

webrender/src/render_backend.rs

@@ -43,9 +43,6 @@ use std::sync::mpsc::{Sender, Receiver};
 use std::u32;
 use time::precise_time_ns;
 
-// WIP: I realize we don't really want to send the entire struct to the scene
-// building thread, this will be most likely a private struct again by the time
-// I figure the bigger picture out.
 #[cfg_attr(feature = "capture", derive(Serialize))]
 #[cfg_attr(feature = "replay", derive(Deserialize))]
 #[derive(Clone)]
@@ -164,14 +161,17 @@ impl Document {
             self.view.accumulated_scale_factor(),
             &self.output_pipelines,
         );
+        if !self.current.removed_pipelines.is_empty() {
+            warn!("Built the scene several times without rendering it.");
+        }
         self.current.removed_pipelines.extend(self.pending.removed_pipelines.drain(..));
         self.frame_builder = Some(frame_builder);
         self.current.scene = self.pending.scene.clone();
     }
 
     fn forward_transaction_to_scene_builder(
         &mut self,
-        txn: TransactionMsg,
+        transaction_msg: TransactionMsg,
         document_ops: &DocumentOps,
         document_id: DocumentId,
         resource_cache: &ResourceCache,
@@ -203,9 +203,9 @@ impl Document {
 
         scene_tx.send(SceneBuilderRequest::Transaction {
             scene: scene_request,
-            resource_updates: txn.resource_updates,
-            frame_ops: txn.frame_ops,
-            render: txn.generate_frame,
+            resource_updates: transaction_msg.resource_updates,
+            frame_ops: transaction_msg.frame_ops,
+            render: transaction_msg.generate_frame,
             document_id,
         }).unwrap();
     }
@@ -294,7 +294,7 @@ pub struct RenderBackend {
     payload_tx: PayloadSender,
     result_tx: Sender<ResultMsg>,
     scene_tx: Sender<SceneBuilderRequest>,
-    scene_rx: Receiver<SceneBuilderMsg>,
+    scene_rx: Receiver<SceneBuilderResult>,
 
     default_device_pixel_ratio: f32,
 
@@ -317,7 +317,7 @@ impl RenderBackend {
         payload_tx: PayloadSender,
         result_tx: Sender<ResultMsg>,
         scene_tx: Sender<SceneBuilderRequest>,
-        scene_rx: Receiver<SceneBuilderMsg>,
+        scene_rx: Receiver<SceneBuilderResult>,
         default_device_pixel_ratio: f32,
         resource_cache: ResourceCache,
         notifier: Box<RenderNotifier>,
@@ -551,7 +551,7 @@ impl RenderBackend {
 
             while let Ok(msg) = self.scene_rx.try_recv() {
                 match msg {
-                    SceneBuilderMsg::Transaction {
+                    SceneBuilderResult::Transaction {
                         document_id,
                         mut built_scene,
                         resource_updates,
@@ -576,18 +576,18 @@ impl RenderBackend {
                             continue;
                         }
 
-                        let txn = TransactionMsg {
+                        let transaction_msg = TransactionMsg {
                             scene_ops: Vec::new(),
                             frame_ops,
                             resource_updates,
                             generate_frame: render,
                             use_scene_builder_thread: false,
                         };
 
-                        if !txn.is_empty() {
+                        if !transaction_msg.is_empty() {
                             self.update_document(
                                 document_id,
-                                txn,
+                                transaction_msg,
                                 &mut frame_counter,
                                 &mut profile_counters
                             );
@@ -774,13 +774,13 @@ impl RenderBackend {
     fn update_document(
         &mut self,
         document_id: DocumentId,
-        mut txn: TransactionMsg,
+        mut transaction_msg: TransactionMsg,
         frame_counter: &mut u32,
         profile_counters: &mut BackendProfileCounters,
     ) {
         let mut op = DocumentOps::nop();
 
-        for doc_msg in replace(&mut txn.scene_ops, Vec::new()) {
+        for doc_msg in transaction_msg.scene_ops.drain(..) {
             let _timer = profile_counters.total_time.timer();
             op.combine(
                 self.process_document(
@@ -792,10 +792,10 @@ impl RenderBackend {
             );
         }
 
-        if txn.use_scene_builder_thread && !txn.is_empty() {
+        if transaction_msg.use_scene_builder_thread && !transaction_msg.is_empty() {
             let doc = self.documents.get_mut(&document_id).unwrap();
             doc.forward_transaction_to_scene_builder(
-                txn,
+                transaction_msg,
                 &op,
                 document_id,
                 &self.resource_cache,
@@ -806,7 +806,7 @@ impl RenderBackend {
         }
 
         self.resource_cache.update_resources(
-            txn.resource_updates,
+            transaction_msg.resource_updates,
             &mut profile_counters.resources,
         );
 
@@ -819,7 +819,7 @@ impl RenderBackend {
             doc.render_on_hittest = true;
         }
 
-        for doc_msg in txn.frame_ops {
+        for doc_msg in transaction_msg.frame_ops {
             let _timer = profile_counters.total_time.timer();
             op.combine(
                 self.process_document(
@@ -834,14 +834,14 @@ impl RenderBackend {
         let doc = self.documents.get_mut(&document_id).unwrap();
 
         if !doc.can_render() {
-            // WIP: this happens if we are building the first scene asynchronously and
+            // TODO: this happens if we are building the first scene asynchronously and
             // scroll at the same time. we should keep track of the fact that we skipped
             // composition here and do it as soon as we receive the scene.
             op.render = false;
             op.composite = false;
         }
 
-        if txn.generate_frame {
+        if transaction_msg.generate_frame {
             if let Some(ref mut ros) = doc.render_on_scroll {
                 *ros = true;
             }

webrender/src/scene.rs

@@ -97,8 +97,6 @@ pub struct ScenePipeline {
     pub display_list: BuiltDisplayList,
 }
 
-// WIP: cloning the whole scene struct is probably more than wat we structly need to
-// do the async stuff, will look into this in more details later.
 /// A complete representation of the layout bundling visible pipelines together.
 #[cfg_attr(feature = "capture", derive(Serialize))]
 #[cfg_attr(feature = "replay", derive(Deserialize))]

webrender/src/scene_builder.rs

@@ -26,7 +26,7 @@ pub enum SceneBuilderRequest {
 }
 
 // Message from scene builder to render backend.
-pub enum SceneBuilderMsg {
+pub enum SceneBuilderResult {
     Transaction {
         document_id: DocumentId,
         built_scene: Option<BuiltScene>,
@@ -56,7 +56,7 @@ pub struct BuiltScene {
 
 pub struct SceneBuilder {
     rx: Receiver<SceneBuilderRequest>,
-    tx: Sender<SceneBuilderMsg>,
+    tx: Sender<SceneBuilderResult>,
     api_tx: MsgSender<ApiMsg>,
     config: FrameBuilderConfig,
 }
@@ -65,7 +65,7 @@ impl SceneBuilder {
     pub fn new(
         config: FrameBuilderConfig,
         api_tx: MsgSender<ApiMsg>
-    ) -> (Self, Sender<SceneBuilderRequest>, Receiver<SceneBuilderMsg>) {
+    ) -> (Self, Sender<SceneBuilderRequest>, Receiver<SceneBuilderResult>) {
         let (in_tx, in_rx) = channel();
         let (out_tx, out_rx) = channel();
         (
@@ -110,7 +110,7 @@ impl SceneBuilder {
 
                 // TODO: pre-rasterization.
 
-                self.tx.send(SceneBuilderMsg::Transaction {
+                self.tx.send(SceneBuilderResult::Transaction {
                     document_id,
                     built_scene,
                     resource_updates,

webrender_api/src/api.rs

@@ -131,7 +131,9 @@ impl ResourceUpdates {
 ///  - no redundant work is performed if two commands in the same transaction cause the scene or
 ///    the frame to be rebuilt.
 pub struct Transaction {
+    // Operations affecting the scene (applied before scene building).
     scene_ops: Vec<DocumentMsg>,
+    // Operations affecting the generation of frames (applied after scene building).
     frame_ops: Vec<DocumentMsg>,
 
     // Additional display list data.
@@ -431,7 +433,7 @@ pub struct AddFontInstance {
 }
 
 // TODO: Split this in two emums.
-// I am postponing this because while it is a trivial change, it is a hard one to rebase.
+// Review note: this will be done just before landing.
 #[derive(Clone, Deserialize, Serialize)]
 pub enum DocumentMsg {
     // Scene messages (apply before building the scene).
@@ -879,11 +881,7 @@ impl RenderApi {
     ) {
         self.send(
             document_id,
-            DocumentMsg::SetWindowParameters {
-                window_size,
-                inner_rect,
-                device_pixel_ratio,
-            },
+            DocumentMsg::SetWindowParameters { window_size, inner_rect, device_pixel_ratio, },
         );
     }
 

wrench/src/yaml_frame_writer.rs

@@ -1162,7 +1162,6 @@ impl webrender::ApiRecordingReceiver for YamlFrameWriterReceiver {
                         _ => {}
                     }
                 }
-
             }
             _ => {}
         }