layout: Make the compositor rather than layout determine the position of

each iframe.

The old code that attempted to do this during layout wasn't able to work
for multiple reasons: it couldn't know where the iframe was going to be
on the page (because of nested iframes), and at the time it was building
the display list for a fragment it couldn't know where that fragment was
going to be in page coordinates.

This patch rewrites that code so that both the sizes and positions of
iframes are determined by the compositor. Layout layerizes all iframes
and marks the iframe layers with the appropriate pipeline and subpage
IDs so that the compositor can place them correctly. This approach is
similar in spirit to Gecko's `RefLayer` infrastructure. The logic that
determines when it is time to take the screenshot for reftests has been
significantly revamped to deal with this change in delegation of
responsibility.

Additionally, this code removes the infrastructure that sends layout
data back to the layout task to be destroyed, since it is now all
thread-safe and can be destroyed on the script task.

The failing tests now fail because of a pre-existing bug related to
intrinsic heights and borders on inline replaced elements. They happened
to pass before because we never rendered the iframes at all, which meant
they never had a chance to draw the red border the tests expect to not
render!

Closes #7377.

components/compositing/compositor_layer.rs

@@ -3,7 +3,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use azure::azure_hl;
-use compositor::IOCompositor;
+use compositor::{IOCompositor, RemovedPipelineInfo};
 use euclid::length::Length;
 use euclid::point::{Point2D, TypedPoint2D};
 use euclid::rect::Rect;
@@ -12,7 +12,7 @@ use layers::color::Color;
 use layers::geometry::LayerPixel;
 use layers::layers::{Layer, LayerBufferSet};
 use msg::compositor_msg::{Epoch, LayerId, LayerProperties, ScrollPolicy};
-use msg::constellation_msg::PipelineId;
+use msg::constellation_msg::{PipelineId, SubpageId};
 use script_traits::CompositorEvent::{ClickEvent, MouseDownEvent, MouseMoveEvent, MouseUpEvent};
 use script_traits::ConstellationControlMsg;
 use std::rc::Rc;
@@ -42,6 +42,9 @@ pub struct CompositorData {
     /// The scroll offset originating from this scrolling root. This allows scrolling roots
     /// to track their current scroll position even while their content_offset does not change.
     pub scroll_offset: TypedPoint2D<LayerPixel, f32>,
+
+    /// The pipeline ID and subpage ID of this layer, if it represents a subpage.
+    pub subpage_info: Option<(PipelineId, SubpageId)>,
 }
 
 impl CompositorData {
@@ -58,6 +61,9 @@ impl CompositorData {
             requested_epoch: Epoch(0),
             painted_epoch: Epoch(0),
             scroll_offset: Point2D::typed(0., 0.),
+            subpage_info: layer_properties.subpage_layer_info.map(|subpage_layer_info| {
+                (subpage_layer_info.pipeline_id, subpage_layer_info.subpage_id)
+            }),
         };
 
         Rc::new(Layer::new(Rect::from_untyped(&layer_properties.rect),
@@ -97,14 +103,6 @@ pub trait CompositorLayer {
                                                   pipeline_id: PipelineId)
                                                   where Window: WindowMethods;
 
-    /// Traverses the existing layer hierarchy and removes any layers that
-    /// currently exist but which are no longer required.
-    fn collect_old_layers<Window>(&self,
-                                  compositor: &mut IOCompositor<Window>,
-                                  pipeline_id: PipelineId,
-                                  new_layers: &[LayerProperties])
-                                  where Window: WindowMethods;
-
     /// Destroys all tiles of all layers, including children, *without* sending them back to the
     /// painter. You must call this only when the paint task is destined to be going down;
     /// otherwise, you will leak tiles.
@@ -151,6 +149,17 @@ pub trait CompositorLayer {
     fn pipeline_id(&self) -> PipelineId;
 }
 
+pub trait RcCompositorLayer {
+    /// Traverses the existing layer hierarchy and removes any layers that
+    /// currently exist but which are no longer required.
+    fn collect_old_layers<Window>(&self,
+                                  compositor: &mut IOCompositor<Window>,
+                                  pipeline_id: PipelineId,
+                                  new_layers: &[LayerProperties],
+                                  pipelines_removed: &mut Vec<RemovedPipelineInfo>)
+                                  where Window: WindowMethods;
+}
+
 #[derive(Copy, PartialEq, Clone, Debug)]
 pub enum WantsScrollEventsFlag {
     WantsScrollEvents,
@@ -279,43 +288,6 @@ impl CompositorLayer for Layer<CompositorData> {
         }
     }
 
-    fn collect_old_layers<Window>(&self,
-                                  compositor: &mut IOCompositor<Window>,
-                                  pipeline_id: PipelineId,
-                                  new_layers: &[LayerProperties])
-                                  where Window: WindowMethods {
-        // Traverse children first so that layers are removed
-        // bottom up - allowing each layer being removed to properly
-        // clean up any tiles it owns.
-        for kid in &*self.children() {
-            kid.collect_old_layers(compositor, pipeline_id, new_layers);
-        }
-
-        // Retain child layers that also exist in the new layer list.
-        self.children().retain(|child| {
-            let extra_data = child.extra_data.borrow();
-
-            // Never remove root layers or layers from other pipelines.
-            if pipeline_id != extra_data.pipeline_id ||
-               extra_data.id == LayerId::null() {
-                true
-            } else {
-                // Keep this layer if it exists in the new layer list.
-                let keep_layer = new_layers.iter().any(|properties| {
-                    properties.id == extra_data.id
-                });
-
-                // When removing a layer, clear any tiles and surfaces
-                // associated with the layer.
-                if !keep_layer {
-                    child.clear_all_tiles(compositor);
-                }
-
-                keep_layer
-            }
-        });
-    }
-
     /// Destroys all tiles of all layers, including children, *without* sending them back to the
     /// painter. You must call this only when the paint task is destined to be going down;
     /// otherwise, you will leak tiles.
@@ -403,17 +375,23 @@ impl CompositorLayer for Layer<CompositorData> {
                 MouseUpEvent(button, event_point),
         };
 
-        let pipeline = compositor.pipeline(self.pipeline_id());
-        let _ = pipeline.script_chan.send(ConstellationControlMsg::SendEvent(pipeline.id.clone(), message));
+        if let Some(pipeline) = compositor.pipeline(self.pipeline_id()) {
+            pipeline.script_chan
+                    .send(ConstellationControlMsg::SendEvent(pipeline.id.clone(), message))
+                    .unwrap();
+        }
     }
 
     fn send_mouse_move_event<Window>(&self,
                                      compositor: &IOCompositor<Window>,
                                      cursor: TypedPoint2D<LayerPixel, f32>)
                                      where Window: WindowMethods {
         let message = MouseMoveEvent(cursor.to_untyped());
-        let pipeline = compositor.pipeline(self.pipeline_id());
-        let _ = pipeline.script_chan.send(ConstellationControlMsg::SendEvent(pipeline.id.clone(), message));
+        if let Some(pipeline) = compositor.pipeline(self.pipeline_id()) {
+            pipeline.script_chan
+                    .send(ConstellationControlMsg::SendEvent(pipeline.id.clone(), message))
+                    .unwrap();
+        }
     }
 
     fn scroll_layer_and_all_child_layers(&self, new_offset: TypedPoint2D<LayerPixel, f32>)
@@ -443,3 +421,97 @@ impl CompositorLayer for Layer<CompositorData> {
         self.extra_data.borrow().pipeline_id
     }
 }
+
+impl RcCompositorLayer for Rc<Layer<CompositorData>> {
+    fn collect_old_layers<Window>(&self,
+                                  compositor: &mut IOCompositor<Window>,
+                                  pipeline_id: PipelineId,
+                                  new_layers: &[LayerProperties],
+                                  pipelines_removed: &mut Vec<RemovedPipelineInfo>)
+                                  where Window: WindowMethods {
+        fn find_root_layer_for_pipeline(layer: &Rc<Layer<CompositorData>>, pipeline_id: PipelineId)
+                                        -> Option<Rc<Layer<CompositorData>>> {
+            let extra_data = layer.extra_data.borrow();
+            if extra_data.pipeline_id == pipeline_id {
+                return Some((*layer).clone())
+            }
+
+            for kid in &*layer.children() {
+                if let Some(layer) = find_root_layer_for_pipeline(kid, pipeline_id) {
+                    return Some(layer.clone())
+                }
+            }
+            None
+        }
+
+        fn collect_old_layers_for_pipeline<Window>(
+                layer: &Layer<CompositorData>,
+                compositor: &mut IOCompositor<Window>,
+                pipeline_id: PipelineId,
+                new_layers: &[LayerProperties],
+                pipelines_removed: &mut Vec<RemovedPipelineInfo>)
+                where Window: WindowMethods {
+            // Traverse children first so that layers are removed
+            // bottom up - allowing each layer being removed to properly
+            // clean up any tiles it owns.
+            for kid in &*layer.children() {
+                collect_old_layers_for_pipeline(kid,
+                                                compositor,
+                                                pipeline_id,
+                                                new_layers,
+                                                pipelines_removed);
+            }
+
+            // Retain child layers that also exist in the new layer list.
+            layer.children().retain(|child| {
+                let extra_data = child.extra_data.borrow();
+                if pipeline_id == extra_data.pipeline_id {
+                    // Never remove our own root layer.
+                    if extra_data.id == LayerId::null() {
+                        return true
+                    }
+
+                    // Keep this layer if it exists in the new layer list.
+                    return new_layers.iter().any(|properties| properties.id == extra_data.id);
+                }
+
+                if let Some(ref subpage_info_for_this_layer) = extra_data.subpage_info {
+                    for layer_properties in new_layers.iter() {
+                        // Keep this layer if a reference to it exists.
+                        if let Some(ref subpage_layer_info) = layer_properties.subpage_layer_info {
+                            if subpage_layer_info.pipeline_id == subpage_info_for_this_layer.0 &&
+                                    subpage_layer_info.subpage_id ==
+                                    subpage_info_for_this_layer.1 {
+                                return true
+                            }
+                        }
+                    }
+
+                    pipelines_removed.push(RemovedPipelineInfo {
+                        parent_pipeline_id: subpage_info_for_this_layer.0,
+                        parent_subpage_id: subpage_info_for_this_layer.1,
+                        child_pipeline_id: extra_data.pipeline_id,
+                    });
+                }
+
+                // When removing a layer, clear any tiles and surfaces associated with the layer.
+                child.clear_all_tiles(compositor);
+                false
+            });
+        }
+
+        // First, find the root layer with the given pipeline ID.
+        let root_layer = match find_root_layer_for_pipeline(self, pipeline_id) {
+            Some(root_layer) => root_layer,
+            None => return,
+        };
+
+        // Then collect all old layers underneath that layer.
+        collect_old_layers_for_pipeline(&root_layer,
+                                        compositor,
+                                        pipeline_id,
+                                        new_layers,
+                                        pipelines_removed);
+    }
+}
+

components/compositing/compositor_task.rs

@@ -5,14 +5,14 @@
 //! Communication with the compositor task.
 
 use compositor;
-use euclid::{Point2D, Rect, Size2D};
+use euclid::{Point2D, Size2D};
 use headless;
 use ipc_channel::ipc::{IpcReceiver, IpcSender};
 use layers::layers::{BufferRequest, LayerBufferSet};
 use layers::platform::surface::{NativeDisplay, NativeSurface};
 use msg::compositor_msg::{Epoch, FrameTreeId, LayerId, LayerProperties};
 use msg::compositor_msg::{PaintListener, ScriptToCompositorMsg};
-use msg::constellation_msg::{AnimationState, ConstellationChan, PipelineId};
+use msg::constellation_msg::{AnimationState, ConstellationChan, PipelineId, SubpageId};
 use msg::constellation_msg::{Key, KeyModifiers, KeyState};
 use png;
 use profile_traits::mem;
@@ -64,7 +64,10 @@ pub fn run_script_listener_thread(compositor_proxy: Box<CompositorProxy + 'stati
     while let Ok(msg) = receiver.recv() {
         match msg {
             ScriptToCompositorMsg::ScrollFragmentPoint(pipeline_id, layer_id, point, _smooth) => {
-                compositor_proxy.send(Msg::ScrollFragmentPoint(pipeline_id, layer_id, point, _smooth));
+                compositor_proxy.send(Msg::ScrollFragmentPoint(pipeline_id,
+                                                               layer_id,
+                                                               point,
+                                                               _smooth));
             }
 
             ScriptToCompositorMsg::GetClientWindow(send) => {
@@ -165,8 +168,6 @@ pub enum Msg {
     /// Tells the compositor to create or update the layers for a pipeline if necessary
     /// (i.e. if no layer with that ID exists).
     InitializeLayersForPipeline(PipelineId, Epoch, Vec<LayerProperties>),
-    /// Alerts the compositor that the specified layer's rect has changed.
-    SetLayerRect(PipelineId, LayerId, Rect<f32>),
     /// Scroll a page in a window
     ScrollFragmentPoint(PipelineId, LayerId, Point2D<f32>, bool),
     /// Requests that the compositor assign the painted buffers to the given layers.
@@ -216,6 +217,12 @@ pub enum Msg {
     MoveTo(Point2D<i32>),
     /// Resize the window to size
     ResizeTo(Size2D<u32>),
+    /// A pipeline was shut down.
+    PipelineExited(PipelineId),
+    /// The layer for a subpage should be created. The first two IDs are the IDs of the *parent*
+    /// pipeline and subpage, respectively, while the last ID is the pipeline ID of the subpage
+    /// itself (or `None` if it has shut down).
+    CreateLayerForSubpage(PipelineId, SubpageId, Option<PipelineId>),
 }
 
 impl Debug for Msg {
@@ -225,7 +232,6 @@ impl Debug for Msg {
             Msg::ShutdownComplete(..) => write!(f, "ShutdownComplete"),
             Msg::GetNativeDisplay(..) => write!(f, "GetNativeDisplay"),
             Msg::InitializeLayersForPipeline(..) => write!(f, "InitializeLayersForPipeline"),
-            Msg::SetLayerRect(..) => write!(f, "SetLayerRect"),
             Msg::ScrollFragmentPoint(..) => write!(f, "ScrollFragmentPoint"),
             Msg::AssignPaintedBuffers(..) => write!(f, "AssignPaintedBuffers"),
             Msg::ChangeRunningAnimationsState(..) => write!(f, "ChangeRunningAnimationsState"),
@@ -250,6 +256,8 @@ impl Debug for Msg {
             Msg::GetClientWindow(..) => write!(f, "GetClientWindow"),
             Msg::MoveTo(..) => write!(f, "MoveTo"),
             Msg::ResizeTo(..) => write!(f, "ResizeTo"),
+            Msg::PipelineExited(..) => write!(f, "PipelineExited"),
+            Msg::CreateLayerForSubpage(..) => write!(f, "CreateLayerForSubpage"),
         }
     }
 }