diff options
| -rw-r--r-- | components/layout/display_list_builder.rs | 13 | ||||
| -rw-r--r-- | components/layout/fragment.rs | 8 | ||||
| -rw-r--r-- | components/layout_thread/dom_wrapper.rs | 6 | ||||
| -rw-r--r-- | components/script/dom/node.rs | 12 | ||||
| -rw-r--r-- | components/script/script_thread.rs | 41 | ||||
| -rw-r--r-- | components/script_layout_interface/wrapper_traits.rs | 8 |
6 files changed, 58 insertions, 30 deletions
diff --git a/components/layout/display_list_builder.rs b/components/layout/display_list_builder.rs index 057966c1000..08b19d46a87 100644 --- a/components/layout/display_list_builder.rs +++ b/components/layout/display_list_builder.rs @@ -2022,6 +2022,15 @@ impl FragmentDisplayListBuilding for Fragment { } SpecificFragmentInfo::Iframe(ref fragment_info) => { if !stacking_relative_content_box.is_empty() { + let browsing_context_id = match fragment_info.browsing_context_id { + Some(browsing_context_id) => browsing_context_id, + None => return warn!("No browsing context id for iframe."), + }; + let pipeline_id = match fragment_info.pipeline_id { + Some(pipeline_id) => pipeline_id, + None => return warn!("No pipeline id for iframe {}.", browsing_context_id), + }; + let base = state.create_base_display_item( &stacking_relative_content_box, build_local_clip(&self.style), @@ -2030,12 +2039,12 @@ impl FragmentDisplayListBuilding for Fragment { DisplayListSection::Content); let item = DisplayItem::Iframe(box IframeDisplayItem { base: base, - iframe: fragment_info.pipeline_id, + iframe: pipeline_id, }); let size = Size2D::new(item.bounds().size.width.to_f32_px(), item.bounds().size.height.to_f32_px()); - state.iframe_sizes.push((fragment_info.browsing_context_id, + state.iframe_sizes.push((browsing_context_id, TypedSize2D::from_untyped(&size))); state.add_display_item(item); diff --git a/components/layout/fragment.rs b/components/layout/fragment.rs index 5dcfea6da53..c7ef0a74e14 100644 --- a/components/layout/fragment.rs +++ b/components/layout/fragment.rs @@ -488,10 +488,10 @@ impl ImageFragmentInfo { /// size of this iframe can be communicated via the constellation to the iframe's own layout thread. #[derive(Clone)] pub struct IframeFragmentInfo { - /// The frame ID of this iframe. - pub browsing_context_id: BrowsingContextId, - /// The pipelineID of this iframe. - pub pipeline_id: PipelineId, + /// The frame ID of this iframe. None if there is no nested browsing context. + pub browsing_context_id: Option<BrowsingContextId>, + /// The pipelineID of this iframe. None if there is no nested browsing context. + pub pipeline_id: Option<PipelineId>, } impl IframeFragmentInfo { diff --git a/components/layout_thread/dom_wrapper.rs b/components/layout_thread/dom_wrapper.rs index a9596f099a2..af999c60efc 100644 --- a/components/layout_thread/dom_wrapper.rs +++ b/components/layout_thread/dom_wrapper.rs @@ -979,12 +979,14 @@ impl<'ln> ThreadSafeLayoutNode for ServoThreadSafeLayoutNode<'ln> { this.svg_data() } - fn iframe_browsing_context_id(&self) -> BrowsingContextId { + // Can return None if the iframe has no nested browsing context + fn iframe_browsing_context_id(&self) -> Option<BrowsingContextId> { let this = unsafe { self.get_jsmanaged() }; this.iframe_browsing_context_id() } - fn iframe_pipeline_id(&self) -> PipelineId { + // Can return None if the iframe has no nested browsing context + fn iframe_pipeline_id(&self) -> Option<PipelineId> { let this = unsafe { self.get_jsmanaged() }; this.iframe_pipeline_id() } diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs index b83cdcbd37e..f5b318b3178 100644 --- a/components/script/dom/node.rs +++ b/components/script/dom/node.rs @@ -1021,8 +1021,8 @@ pub trait LayoutNodeHelpers { fn image_url(&self) -> Option<ServoUrl>; fn canvas_data(&self) -> Option<HTMLCanvasData>; fn svg_data(&self) -> Option<SVGSVGData>; - fn iframe_browsing_context_id(&self) -> BrowsingContextId; - fn iframe_pipeline_id(&self) -> PipelineId; + fn iframe_browsing_context_id(&self) -> Option<BrowsingContextId>; + fn iframe_pipeline_id(&self) -> Option<PipelineId>; fn opaque(&self) -> OpaqueNode; } @@ -1172,16 +1172,16 @@ impl LayoutNodeHelpers for LayoutJS<Node> { .map(|svg| svg.data()) } - fn iframe_browsing_context_id(&self) -> BrowsingContextId { + fn iframe_browsing_context_id(&self) -> Option<BrowsingContextId> { let iframe_element = self.downcast::<HTMLIFrameElement>() .expect("not an iframe element!"); - iframe_element.browsing_context_id().unwrap() + iframe_element.browsing_context_id() } - fn iframe_pipeline_id(&self) -> PipelineId { + fn iframe_pipeline_id(&self) -> Option<PipelineId> { let iframe_element = self.downcast::<HTMLIFrameElement>() .expect("not an iframe element!"); - iframe_element.pipeline_id().unwrap() + iframe_element.pipeline_id() } #[allow(unsafe_code)] diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index 2fa0071ed65..b9bcf5be7c8 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -1750,10 +1750,35 @@ impl ScriptThread { load.pipeline_id == id }); + let document = self.documents.borrow_mut().remove(id); + + // We should never have a pipeline that's still an incomplete load, + // but also has a Document. + debug_assert!(idx.is_none() || document.is_none()); + + // Remove any incomplete load. let chan = if let Some(idx) = idx { let load = self.incomplete_loads.borrow_mut().remove(idx); load.layout_chan.clone() - } else if let Some(document) = self.documents.borrow_mut().remove(id) { + } else if let Some(ref document) = document { + document.window().layout_chan().clone() + } else { + return warn!("Exiting nonexistant pipeline {}.", id); + }; + + // We shut down layout before removing the document, + // since layout might still be in the middle of laying it out. + debug!("preparing to shut down layout for page {}", id); + let (response_chan, response_port) = channel(); + chan.send(message::Msg::PrepareToExit(response_chan)).ok(); + let _ = response_port.recv(); + + debug!("shutting down layout for page {}", id); + chan.send(message::Msg::ExitNow).ok(); + self.script_sender.send((id, ScriptMsg::PipelineExited)).ok(); + + // Now that layout is shut down, it's OK to remove the document. + if let Some(document) = document { // We don't want to dispatch `mouseout` event pointing to non-existing element if let Some(target) = self.topmost_mouse_over_target.get() { if target.upcast::<Node>().owner_doc() == document { @@ -1761,22 +1786,14 @@ impl ScriptThread { } } + // We discard the browsing context after requesting layout shut down, + // to avoid running layout on detached iframes. let window = document.window(); if discard_bc == DiscardBrowsingContext::Yes { window.window_proxy().discard_browsing_context(); } window.clear_js_runtime(); - window.layout_chan().clone() - } else { - return warn!("Exiting nonexistant pipeline {}.", id); - }; - - let (response_chan, response_port) = channel(); - chan.send(message::Msg::PrepareToExit(response_chan)).ok(); - debug!("shutting down layout for page {}", id); - let _ = response_port.recv(); - chan.send(message::Msg::ExitNow).ok(); - self.script_sender.send((id, ScriptMsg::PipelineExited)).ok(); + } debug!("Exited pipeline {}.", id); } diff --git a/components/script_layout_interface/wrapper_traits.rs b/components/script_layout_interface/wrapper_traits.rs index c58b6292ef0..d022360305b 100644 --- a/components/script_layout_interface/wrapper_traits.rs +++ b/components/script_layout_interface/wrapper_traits.rs @@ -275,12 +275,12 @@ pub trait ThreadSafeLayoutNode: Clone + Copy + Debug + GetLayoutData + NodeInfo fn svg_data(&self) -> Option<SVGSVGData>; /// If this node is an iframe element, returns its browsing context ID. If this node is - /// not an iframe element, fails. - fn iframe_browsing_context_id(&self) -> BrowsingContextId; + /// not an iframe element, fails. Returns None if there is no nested browsing context. + fn iframe_browsing_context_id(&self) -> Option<BrowsingContextId>; /// If this node is an iframe element, returns its pipeline ID. If this node is - /// not an iframe element, fails. - fn iframe_pipeline_id(&self) -> PipelineId; + /// not an iframe element, fails. Returns None if there is no nested browsing context. + fn iframe_pipeline_id(&self) -> Option<PipelineId>; fn get_colspan(&self) -> u32; |