From 9272f395c5fa003e71548f590b40d3ee2c591236 Mon Sep 17 00:00:00 2001 From: Glenn Watson Date: Wed, 7 Oct 2015 18:36:35 -0600 Subject: [PATCH] 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 | 84 ++----------------- .../compositing/compositor_layer.rs | 28 +++---- .../components/compositing/compositor_task.rs | 7 +- servo/components/compositing/constellation.rs | 29 +++---- servo/components/compositing/headless.rs | 1 - .../components/layout/display_list_builder.rs | 1 - servo/components/layout/fragment.rs | 7 +- servo/components/layout/wrapper.rs | 9 +- servo/components/msg/compositor_msg.rs | 4 +- servo/components/msg/constellation_msg.rs | 11 +-- .../script/dom/htmliframeelement.rs | 5 ++ 11 files changed, 47 insertions(+), 139 deletions(-) diff --git a/servo/components/compositing/compositor.rs b/servo/components/compositing/compositor.rs index 42efc2b9cea3..dcacb440e667 100644 --- a/servo/components/compositing/compositor.rs +++ b/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 { /// 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, @@ -316,7 +312,6 @@ impl IOCompositor { 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 IOCompositor { 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 IOCompositor { (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 IOCompositor { 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, - pipeline_removed.parent_subpage_id)); - self.pending_subpages.remove(&pipeline_removed.child_pipeline_id); + self.pending_subpages.remove(&pipeline_removed); } } @@ -832,52 +811,21 @@ impl IOCompositor { 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) { - 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); - self.pending_subpages.insert(subpage_pipeline_id); + new_layer.add_child(subpage_layer); + 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 IOCompositor { 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 IOCompositor { } // 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, -} - diff --git a/servo/components/compositing/compositor_layer.rs b/servo/components/compositing/compositor_layer.rs index 91c8b0d121d9..0bcb35bd28dc 100644 --- a/servo/components/compositing/compositor_layer.rs +++ b/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, - /// The pipeline ID and subpage ID of this layer, if it represents a subpage. - pub subpage_info: Option<(PipelineId, SubpageId)>, + /// The pipeline ID of this layer, if it represents a subpage. + pub subpage_info: Option, } 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, pipeline_id: PipelineId, new_layers: &[LayerProperties], - pipelines_removed: &mut Vec) + pipelines_removed: &mut Vec) where Window: WindowMethods; } @@ -427,7 +427,7 @@ impl RcCompositorLayer for Rc> { compositor: &mut IOCompositor, pipeline_id: PipelineId, new_layers: &[LayerProperties], - pipelines_removed: &mut Vec) + pipelines_removed: &mut Vec) where Window: WindowMethods { fn find_root_layer_for_pipeline(layer: &Rc>, pipeline_id: PipelineId) -> Option>> { @@ -449,7 +449,7 @@ impl RcCompositorLayer for Rc> { compositor: &mut IOCompositor, pipeline_id: PipelineId, new_layers: &[LayerProperties], - pipelines_removed: &mut Vec) + pipelines_removed: &mut Vec) 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> { } } - 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 && - subpage_layer_info.subpage_id == - subpage_info_for_this_layer.1 { + if subpage_layer_info.pipeline_id == layer_pipeline_id { 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, - }); + pipelines_removed.push(extra_data.pipeline_id); } // When removing a layer, clear any tiles and surfaces associated with the layer. diff --git a/servo/components/compositing/compositor_task.rs b/servo/components/compositing/compositor_task.rs index f74813cdf334..2b3b572a0118 100644 --- a/servo/components/compositing/compositor_task.rs +++ b/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), /// 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), } 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"), } } } diff --git a/servo/components/compositing/constellation.rs b/servo/components/compositing/constellation.rs index 8fe576855ee3..0da135022d40 100644 --- a/servo/components/compositing/constellation.rs +++ b/servo/components/compositing/constellation.rs @@ -408,9 +408,9 @@ impl Constellation { } // 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 Constellation { 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 Constellation { } fn handle_frame_size_msg(&mut self, - containing_pipeline_id: PipelineId, - subpage_id: SubpageId, + pipeline_id: PipelineId, size: &TypedSize2D) { // 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 - .get(&(containing_pipeline_id, subpage_id)) - .map(|id| *id); - let pipeline = match pipeline_id { - Some(pipeline_id) => self.mut_pipeline(pipeline_id), + match self.pipelines.get_mut(&pipeline_id) { + Some(pipeline) => { + pipeline.size = Some(*size); + pipeline.script_chan.clone() + } None => return, - }; - pipeline.size = Some(*size); - (pipeline.id, pipeline.script_chan.clone()) + } }; script_chan.send(ConstellationControlMsg::Resize(pipeline_id, WindowSizeData { diff --git a/servo/components/compositing/headless.rs b/servo/components/compositing/headless.rs index 3a0ab67ea215..41bcc3811d2e 100644 --- a/servo/components/compositing/headless.rs +++ b/servo/components/compositing/headless.rs @@ -124,7 +124,6 @@ impl CompositorEventListener for NullCompositor { Msg::ReturnUnusedNativeSurfaces(..) => {} Msg::CollectMemoryReports(..) => {} Msg::PipelineExited(..) => {} - Msg::CreateLayerForSubpage(..) => {} } true } diff --git a/servo/components/layout/display_list_builder.rs b/servo/components/layout/display_list_builder.rs index 103b8a6b10e1..3edbab40e86f 100644 --- a/servo/components/layout/display_list_builder.rs +++ b/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), }) } diff --git a/servo/components/layout/fragment.rs b/servo/components/layout/fragment.rs index 63fec117a8cd..06a26863cff9 100644 --- a/servo/components/layout/fragment.rs +++ b/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, } } diff --git a/servo/components/layout/wrapper.rs b/servo/components/layout/wrapper.rs index 91ba3f958a86..74fdc8697591 100644 --- a/servo/components/layout/wrapper.rs +++ b/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()).subpage_id().unwrap()) + (*iframe_element.unsafe_get()).pipeline_id().unwrap() } } } diff --git a/servo/components/msg/compositor_msg.rs b/servo/components/msg/compositor_msg.rs index 681f5161f2c8..8c411148c9a1 100644 --- a/servo/components/msg/compositor_msg.rs +++ b/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, } diff --git a/servo/components/msg/constellation_msg.rs b/servo/components/msg/constellation_msg.rs index d62e3470f24e..857161c316d7 100644 --- a/servo/components/msg/constellation_msg.rs +++ b/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), + FrameSize(PipelineId, Size2D), LoadUrl(PipelineId, LoadData), ScriptLoadedURLInIFrame(IframeLoadInfo), Navigate(Option<(PipelineId, SubpageId)>, NavigationDirection), @@ -291,9 +291,6 @@ pub enum Msg { IpcSender, usize), String>>), /// Status message to be displayed in the chrome, eg. a link URL on mouseover. NodeStatus(Option), - /// 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> = 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, - index: PipelineIndex + pub namespace_id: PipelineNamespaceId, + pub index: PipelineIndex } impl PipelineId { diff --git a/servo/components/script/dom/htmliframeelement.rs b/servo/components/script/dom/htmliframeelement.rs index 115dd139854f..b8df7d9fd788 100644 --- a/servo/components/script/dom/htmliframeelement.rs +++ b/servo/components/script/dom/htmliframeelement.rs @@ -222,6 +222,11 @@ impl HTMLIFrameElement { pub fn subpage_id(&self) -> Option { self.subpage_id.get() } + + #[inline] + pub fn pipeline_id(&self) -> Option { + self.pipeline_id.get() + } } pub fn Navigate(iframe: &HTMLIFrameElement, direction: NavigationDirection) -> Fallible<()> {