servo: Merge #7899 - Remove constellation round trip for subpage mapping in compositor (from glennw:subpage-fixes-1); r=pcwalton

This makes use of the new functionality that allows iframes to generate their own pipeline IDs in order to remove any knowledge of subpage ids from the compositor.

(This is the first of several commits removing subpage from parts of servo).

Source-Repo: https://github.com/servo/servo
Source-Revision: 1d617f332edd0036ca4cbc3890f1f44f57597906

servo/components/compositing/compositor.rs

@@ -30,7 +30,7 @@ use msg::compositor_msg::{LayerProperties, ScrollPolicy};
 use msg::constellation_msg::AnimationState;
 use msg::constellation_msg::Msg as ConstellationMsg;
 use msg::constellation_msg::{ConstellationChan, Key, KeyModifiers, KeyState, LoadData};
-use msg::constellation_msg::{NavigationDirection, PipelineId, SubpageId, WindowSizeData};
+use msg::constellation_msg::{NavigationDirection, PipelineId, WindowSizeData};
 use pipeline::CompositionPipeline;
 use png;
 use profile_traits::mem::{self, ReportKind, Reporter, ReporterRequest};
@@ -165,10 +165,6 @@ pub struct IOCompositor<Window: WindowMethods> {
     /// A data structure to cache unused NativeSurfaces.
     surface_map: SurfaceMap,
 
-    /// Information about subpage layers that are pending. The keys in this map are the
-    /// pipeline/subpage IDs of the parent.
-    pending_subpage_layers: HashMap<(PipelineId, SubpageId), PendingSubpageLayerInfo>,
-
     /// Pipeline IDs of subpages that the compositor has seen in a layer tree but which have not
     /// yet been painted.
     pending_subpages: HashSet<PipelineId>,
@@ -316,7 +312,6 @@ impl<Window: WindowMethods> IOCompositor<Window> {
             has_seen_quit_event: false,
             ready_to_save_state: ReadyState::Unknown,
             surface_map: SurfaceMap::new(BUFFER_MAP_SIZE),
-            pending_subpage_layers: HashMap::new(),
             pending_subpages: HashSet::new(),
         }
     }
@@ -520,15 +515,6 @@ impl<Window: WindowMethods> IOCompositor<Window> {
                 self.window.head_parsed();
             }
 
-            (Msg::CreateLayerForSubpage(parent_pipeline_id,
-                                        parent_subpage_id,
-                                        subpage_pipeline_id),
-             ShutdownState::NotShuttingDown) => {
-                 self.create_layer_for_subpage(parent_pipeline_id,
-                                               parent_subpage_id,
-                                               subpage_pipeline_id);
-            }
-
             (Msg::CollectMemoryReports(reports_chan), ShutdownState::NotShuttingDown) => {
                 let mut reports = vec![];
                 let name = "compositor-task";
@@ -549,11 +535,6 @@ impl<Window: WindowMethods> IOCompositor<Window> {
 
             (Msg::PipelineExited(pipeline_id), _) => {
                 self.pending_subpages.remove(&pipeline_id);
-
-                // FIXME(pcwalton): This is a total hack. But it'll get complicated to do this
-                // properly, since we need to get rid of the pending subpage layers if either the
-                // parent or the child layer goes away.
-                self.pending_subpage_layers.clear();
             }
 
             // When we are shutting_down, we need to avoid performing operations
@@ -716,9 +697,7 @@ impl<Window: WindowMethods> IOCompositor<Window> {
         root_layer.collect_old_layers(self, pipeline_id, new_layers, &mut pipelines_removed);
 
         for pipeline_removed in pipelines_removed.into_iter() {
-            self.pending_subpage_layers.remove(&(pipeline_removed.parent_pipeline_id,
+            self.pending_subpages.remove(&pipeline_removed);
-                                                 pipeline_removed.parent_subpage_id));
-            self.pending_subpages.remove(&pipeline_removed.child_pipeline_id);
         }
     }
 
@@ -832,52 +811,21 @@ impl<Window: WindowMethods> IOCompositor<Window> {
                     scrolls_overflow_area: true,
                 };
 
-                // We need to map from the (pipeline ID, subpage ID) pair to the pipeline ID of
-                // the subpage itself. The constellation is the source of truth for that
-                // information, so go ask it. In the meantime, store the information in the list of
-                // pending subpage layers.
-                self.pending_subpage_layers.insert((pipeline_id, subpage_layer_info.subpage_id),
-                                                   PendingSubpageLayerInfo {
-                    subpage_layer_properties: subpage_layer_properties,
-                    container_layer_id: layer_properties.id,
-                });
-                self.constellation_chan.0.send(ConstellationMsg::PrepareForSubpageLayerCreation(
-                        pipeline_id,
-                        subpage_layer_info.subpage_id)).unwrap();
-            }
-
-            parent_layer.add_child(new_layer.clone());
-        }
-    }
-
-    fn create_layer_for_subpage(&mut self,
-                                parent_pipeline_id: PipelineId,
-                                parent_subpage_id: SubpageId,
-                                subpage_pipeline_id: Option<PipelineId>) {
-        let subpage_pipeline_id = match subpage_pipeline_id {
-            Some(subpage_pipeline_id) => subpage_pipeline_id,
-            None => return,
-        };
-        if let Some(PendingSubpageLayerInfo {
-            subpage_layer_properties,
-            container_layer_id
-        }) = self.pending_subpage_layers.remove(&(parent_pipeline_id, parent_subpage_id)) {
-            if let Some(container_layer) =
-                    self.find_layer_with_pipeline_and_layer_id(parent_pipeline_id,
-                                                               container_layer_id) {
                 let wants_scroll_events = if subpage_layer_properties.scrolls_overflow_area {
                     WantsScrollEventsFlag::WantsScrollEvents
                 } else {
                     WantsScrollEventsFlag::DoesntWantScrollEvents
                 };
-                let subpage_layer = CompositorData::new_layer(subpage_pipeline_id,
+                let subpage_layer = CompositorData::new_layer(subpage_layer_info.pipeline_id,
                                                               subpage_layer_properties,
                                                               wants_scroll_events,
-                                                              container_layer.tile_size);
+                                                              new_layer.tile_size);
                 *subpage_layer.masks_to_bounds.borrow_mut() = true;
-                container_layer.add_child(subpage_layer);
+                new_layer.add_child(subpage_layer);
-                self.pending_subpages.insert(subpage_pipeline_id);
+                self.pending_subpages.insert(subpage_layer_info.pipeline_id);
             }
+
+            parent_layer.add_child(new_layer.clone());
         }
 
         self.dump_layer_tree();
@@ -906,7 +854,6 @@ impl<Window: WindowMethods> IOCompositor<Window> {
 
         let ConstellationChan(ref chan) = self.constellation_chan;
         chan.send(ConstellationMsg::FrameSize(subpage_layer_info.pipeline_id,
-                                              subpage_layer_info.subpage_id,
                                               layer_properties.rect.size)).unwrap();
     }
 
@@ -1500,7 +1447,7 @@ impl<Window: WindowMethods> IOCompositor<Window> {
                 }
 
                 // Check if there are any pending frames. If so, the image is not stable yet.
-                if self.pending_subpage_layers.len() > 0 || self.pending_subpages.len() > 0 {
+                if self.pending_subpages.len() > 0 {
                     return false
                 }
 
@@ -1962,16 +1909,3 @@ pub enum CompositingReason {
     /// The window has been zoomed.
     Zoom,
 }
-
-struct PendingSubpageLayerInfo {
-    subpage_layer_properties: LayerProperties,
-    container_layer_id: LayerId,
-}
-
-#[derive(Copy, Clone)]
-pub struct RemovedPipelineInfo {
-    pub parent_pipeline_id: PipelineId,
-    pub parent_subpage_id: SubpageId,
-    pub child_pipeline_id: PipelineId,
-}
-

servo/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, RemovedPipelineInfo};
+use compositor::IOCompositor;
 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, SubpageId};
+use msg::constellation_msg::{PipelineId};
 use script_traits::CompositorEvent::{ClickEvent, MouseDownEvent, MouseMoveEvent, MouseUpEvent};
 use script_traits::ConstellationControlMsg;
 use std::rc::Rc;
@@ -43,8 +43,8 @@ pub struct CompositorData {
     /// 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.
+    /// The pipeline ID of this layer, if it represents a subpage.
-    pub subpage_info: Option<(PipelineId, SubpageId)>,
+    pub subpage_info: Option<PipelineId>,
 }
 
 impl CompositorData {
@@ -62,7 +62,7 @@ impl CompositorData {
             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)
+                subpage_layer_info.pipeline_id
             }),
         };
 
@@ -156,7 +156,7 @@ pub trait RcCompositorLayer {
                                   compositor: &mut IOCompositor<Window>,
                                   pipeline_id: PipelineId,
                                   new_layers: &[LayerProperties],
-                                  pipelines_removed: &mut Vec<RemovedPipelineInfo>)
+                                  pipelines_removed: &mut Vec<PipelineId>)
                                   where Window: WindowMethods;
 }
 
@@ -427,7 +427,7 @@ impl RcCompositorLayer for Rc<Layer<CompositorData>> {
                                   compositor: &mut IOCompositor<Window>,
                                   pipeline_id: PipelineId,
                                   new_layers: &[LayerProperties],
-                                  pipelines_removed: &mut Vec<RemovedPipelineInfo>)
+                                  pipelines_removed: &mut Vec<PipelineId>)
                                   where Window: WindowMethods {
         fn find_root_layer_for_pipeline(layer: &Rc<Layer<CompositorData>>, pipeline_id: PipelineId)
                                         -> Option<Rc<Layer<CompositorData>>> {
@@ -449,7 +449,7 @@ impl RcCompositorLayer for Rc<Layer<CompositorData>> {
                 compositor: &mut IOCompositor<Window>,
                 pipeline_id: PipelineId,
                 new_layers: &[LayerProperties],
-                pipelines_removed: &mut Vec<RemovedPipelineInfo>)
+                pipelines_removed: &mut Vec<PipelineId>)
                 where Window: WindowMethods {
             // Traverse children first so that layers are removed
             // bottom up - allowing each layer being removed to properly
@@ -477,23 +477,17 @@ impl RcCompositorLayer for Rc<Layer<CompositorData>> {
                     }
                 }
 
-                if let Some(ref subpage_info_for_this_layer) = extra_data.subpage_info {
+                if let Some(layer_pipeline_id) = 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 &&
+                            if subpage_layer_info.pipeline_id == layer_pipeline_id {
-                                    subpage_layer_info.subpage_id ==
-                                    subpage_info_for_this_layer.1 {
                                 return true
                             }
                         }
                     }
 
-                    pipelines_removed.push(RemovedPipelineInfo {
+                    pipelines_removed.push(extra_data.pipeline_id);
-                        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.

servo/components/compositing/compositor_task.rs

@@ -12,7 +12,7 @@ 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, SubpageId};
+use msg::constellation_msg::{AnimationState, ConstellationChan, PipelineId};
 use msg::constellation_msg::{Key, KeyModifiers, KeyState};
 use png;
 use profile_traits::mem;
@@ -219,10 +219,6 @@ pub enum Msg {
     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 {
@@ -257,7 +253,6 @@ impl Debug for Msg {
             Msg::MoveTo(..) => write!(f, "MoveTo"),
             Msg::ResizeTo(..) => write!(f, "ResizeTo"),
             Msg::PipelineExited(..) => write!(f, "PipelineExited"),
-            Msg::CreateLayerForSubpage(..) => write!(f, "CreateLayerForSubpage"),
         }
     }
 }

servo/components/compositing/constellation.rs

@@ -408,9 +408,9 @@ impl<LTF: LayoutTaskFactory, STF: ScriptTaskFactory> Constellation<LTF, STF> {
             }
             // The compositor discovered the size of a subframe. This needs to be reflected by all
             // frame trees in the navigation context containing the subframe.
-            ConstellationMsg::FrameSize(pipeline_id, subpage_id, size) => {
+            ConstellationMsg::FrameSize(pipeline_id, size) => {
                 debug!("constellation got frame size message");
-                self.handle_frame_size_msg(pipeline_id, subpage_id, &Size2D::from_untyped(&size));
+                self.handle_frame_size_msg(pipeline_id, &Size2D::from_untyped(&size));
             }
             ConstellationMsg::ScriptLoadedURLInIFrame(load_info) => {
                 debug!("constellation got iframe URL load message {:?} {:?} {:?}",
@@ -541,12 +541,6 @@ impl<LTF: LayoutTaskFactory, STF: ScriptTaskFactory> Constellation<LTF, STF> {
                 debug!("constellation got NodeStatus message");
                 self.compositor_proxy.send(CompositorMsg::Status(message));
             }
-            ConstellationMsg::PrepareForSubpageLayerCreation(pipeline_id, subpage_id) => {
-                self.compositor_proxy.send(CompositorMsg::CreateLayerForSubpage(
-                        pipeline_id,
-                        subpage_id,
-                        self.subpage_map.get(&(pipeline_id, subpage_id)).map(|x| *x)))
-            }
         }
         true
     }
@@ -617,23 +611,20 @@ impl<LTF: LayoutTaskFactory, STF: ScriptTaskFactory> Constellation<LTF, STF> {
     }
 
     fn handle_frame_size_msg(&mut self,
-                             containing_pipeline_id: PipelineId,
+                             pipeline_id: PipelineId,
-                             subpage_id: SubpageId,
                              size: &TypedSize2D<PagePx, f32>) {
         // Store the new rect inside the pipeline
-        let (pipeline_id, script_chan) = {
+        let script_chan = {
             // Find the pipeline that corresponds to this rectangle. It's possible that this
             // pipeline may have already exited before we process this message, so just
             // early exit if that occurs.
-            let pipeline_id = self.subpage_map
+            match self.pipelines.get_mut(&pipeline_id) {
-                                  .get(&(containing_pipeline_id, subpage_id))
+                Some(pipeline) => {
-                                  .map(|id| *id);
+                    pipeline.size = Some(*size);
-            let pipeline = match pipeline_id {
+                    pipeline.script_chan.clone()
-                Some(pipeline_id) => self.mut_pipeline(pipeline_id),
+                }
                 None => return,
-            };
+            }
-            pipeline.size = Some(*size);
-            (pipeline.id, pipeline.script_chan.clone())
         };
 
         script_chan.send(ConstellationControlMsg::Resize(pipeline_id, WindowSizeData {

servo/components/compositing/headless.rs

@@ -124,7 +124,6 @@ impl CompositorEventListener for NullCompositor {
             Msg::ReturnUnusedNativeSurfaces(..) => {}
             Msg::CollectMemoryReports(..) => {}
             Msg::PipelineExited(..) => {}
-            Msg::CreateLayerForSubpage(..) => {}
         }
         true
     }

servo/components/layout/display_list_builder.rs

@@ -1289,7 +1289,6 @@ impl FragmentDisplayListBuilding for Fragment {
                 let border_padding = self.border_padding.to_physical(self.style().writing_mode);
                 Some(SubpageLayerInfo {
                     pipeline_id: iframe_fragment_info.pipeline_id,
-                    subpage_id: iframe_fragment_info.subpage_id,
                     origin: Point2D::new(border_padding.left, border_padding.top),
                 })
             }

servo/components/layout/fragment.rs

@@ -24,7 +24,7 @@ use ipc_channel::ipc::IpcSender;
 use layout_debug;
 use model::{self, IntrinsicISizes, IntrinsicISizesContribution, MaybeAuto, specified};
 use msg::compositor_msg::{LayerId, LayerType};
-use msg::constellation_msg::{PipelineId, SubpageId};
+use msg::constellation_msg::PipelineId;
 use net_traits::image::base::Image;
 use net_traits::image_cache_task::UsePlaceholder;
 use rustc_serialize::{Encodable, Encoder};
@@ -578,17 +578,14 @@ impl ReplacedImageFragmentInfo {
 pub struct IframeFragmentInfo {
     /// The pipeline ID of this iframe.
     pub pipeline_id: PipelineId,
-    /// The subpage ID of this iframe.
-    pub subpage_id: SubpageId,
 }
 
 impl IframeFragmentInfo {
     /// Creates the information specific to an iframe fragment.
     pub fn new(node: &ThreadSafeLayoutNode) -> IframeFragmentInfo {
-        let (pipeline_id, subpage_id) = node.iframe_pipeline_and_subpage_ids();
+        let pipeline_id = node.iframe_pipeline_id();
         IframeFragmentInfo {
             pipeline_id: pipeline_id,
-            subpage_id: subpage_id,
         }
     }
 

servo/components/layout/wrapper.rs

@@ -37,7 +37,7 @@ use gfx::display_list::OpaqueNode;
 use gfx::text::glyph::CharIndex;
 use incremental::RestyleDamage;
 use ipc_channel::ipc::IpcSender;
-use msg::constellation_msg::{PipelineId, SubpageId};
+use msg::constellation_msg::PipelineId;
 use opaque_node::OpaqueNodeMethods;
 use script::dom::attr::AttrValue;
 use script::dom::bindings::codegen::InheritTypes::{CharacterDataCast, ElementCast};
@@ -1011,14 +1011,13 @@ impl<'ln> ThreadSafeLayoutNode<'ln> {
         }
     }
 
-    /// If this node is an iframe element, returns its pipeline and subpage IDs. If this node is
+    /// If this node is an iframe element, returns its pipeline ID. If this node is
     /// not an iframe element, fails.
-    pub fn iframe_pipeline_and_subpage_ids(&self) -> (PipelineId, SubpageId) {
+    pub fn iframe_pipeline_id(&self) -> PipelineId {
         unsafe {
             let iframe_element = HTMLIFrameElementCast::to_layout_js(self.get_jsmanaged())
                 .expect("not an iframe element!");
-            ((*iframe_element.unsafe_get()).containing_page_pipeline_id().unwrap(),
+            (*iframe_element.unsafe_get()).pipeline_id().unwrap()
-             (*iframe_element.unsafe_get()).subpage_id().unwrap())
         }
     }
 }

servo/components/msg/compositor_msg.rs

@@ -4,7 +4,7 @@
 
 use app_units::Au;
 use azure::azure_hl::Color;
-use constellation_msg::{Key, KeyModifiers, KeyState, PipelineId, SubpageId};
+use constellation_msg::{Key, KeyModifiers, KeyState, PipelineId};
 use euclid::{Matrix4, Point2D, Rect, Size2D};
 use ipc_channel::ipc::IpcSender;
 use layers::layers::{BufferRequest, LayerBufferSet};
@@ -169,8 +169,6 @@ pub enum ScriptToCompositorMsg {
 pub struct SubpageLayerInfo {
     /// The ID of the pipeline.
     pub pipeline_id: PipelineId,
-    /// The ID of the subpage.
-    pub subpage_id: SubpageId,
     /// The offset of the subpage within this layer (to account for borders).
     pub origin: Point2D<Au>,
 }

servo/components/msg/constellation_msg.rs

@@ -239,7 +239,7 @@ pub enum Msg {
     LoadComplete(PipelineId),
     /// Dispatched after the DOM load event has fired on a document
     DOMLoad(PipelineId),
-    FrameSize(PipelineId, SubpageId, Size2D<f32>),
+    FrameSize(PipelineId, Size2D<f32>),
     LoadUrl(PipelineId, LoadData),
     ScriptLoadedURLInIFrame(IframeLoadInfo),
     Navigate(Option<(PipelineId, SubpageId)>, NavigationDirection),
@@ -291,9 +291,6 @@ pub enum Msg {
                          IpcSender<Result<(IpcSender<CanvasMsg>, usize), String>>),
     /// Status message to be displayed in the chrome, eg. a link URL on mouseover.
     NodeStatus(Option<String>),
-    /// Requests that the pipeline ID of the subpage identified by a (pipeline ID, subpage ID)
-    /// pair be sent to the compositor via a `CreateLayerForSubpage` message.
-    PrepareForSubpageLayerCreation(PipelineId, SubpageId),
 }
 
 #[derive(Clone, Eq, PartialEq, Deserialize, Serialize, Debug)]
@@ -463,12 +460,12 @@ thread_local!(pub static PIPELINE_NAMESPACE: Cell<Option<PipelineNamespace>> = C
 pub struct PipelineNamespaceId(pub u32);
 
 #[derive(Clone, PartialEq, Eq, Copy, Hash, Debug, Deserialize, Serialize, HeapSizeOf)]
-pub struct PipelineIndex(u32);
+pub struct PipelineIndex(pub u32);
 
 #[derive(Clone, PartialEq, Eq, Copy, Hash, Debug, Deserialize, Serialize, HeapSizeOf)]
 pub struct PipelineId {
-    namespace_id: PipelineNamespaceId,
+    pub namespace_id: PipelineNamespaceId,
-    index: PipelineIndex
+    pub index: PipelineIndex
 }
 
 impl PipelineId {

servo/components/script/dom/htmliframeelement.rs

@@ -222,6 +222,11 @@ impl HTMLIFrameElement {
     pub fn subpage_id(&self) -> Option<SubpageId> {
         self.subpage_id.get()
     }
+
+    #[inline]
+    pub fn pipeline_id(&self) -> Option<PipelineId> {
+        self.pipeline_id.get()
+    }
 }
 
 pub fn Navigate(iframe: &HTMLIFrameElement, direction: NavigationDirection) -> Fallible<()> {