From 8d508dff44f494f52b59f2bb3074f829cc526269 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Tue, 12 Jan 2016 20:57:23 -0800 Subject: [PATCH] compositing: Fix a couple of bugs that prevented iframes from painting after navigation. The first bug was that iframes were not reflowed in their parent DOM when the child page navigated. This is fixed by simply having the constellation notify the appropriate script thread when navigation occurs. The second bug was that the compositor was unable to adjust the pipeline for existing iframe layers, only new ones. This patch adds logic to do that. Closes #8081. --- components/compositing/compositor.rs | 76 ++++++++++++------- components/compositing/compositor_layer.rs | 1 + components/compositing/constellation.rs | 11 +++ components/script/dom/window.rs | 2 + components/script/script_thread.rs | 18 +++++ components/script_traits/lib.rs | 2 + tests/wpt/mozilla/meta/MANIFEST.json | 12 +++ .../mozilla/tests/css/iframe/navigation.html | 36 +++++++++ .../tests/css/iframe/navigation_ref.html | 17 +++++ 9 files changed, 149 insertions(+), 26 deletions(-) create mode 100644 tests/wpt/mozilla/tests/css/iframe/navigation.html create mode 100644 tests/wpt/mozilla/tests/css/iframe/navigation_ref.html diff --git a/components/compositing/compositor.rs b/components/compositing/compositor.rs index 84d101ac9d13..3ada78486cd0 100644 --- a/components/compositing/compositor.rs +++ b/components/compositing/compositor.rs @@ -790,6 +790,13 @@ impl IOCompositor { match self.find_layer_with_pipeline_and_layer_id(pipeline_id, properties.id) { Some(existing_layer) => { + // If this layer contains a subpage, then create the root layer for that subpage + // now. + if properties.subpage_pipeline_id.is_some() { + self.create_root_layer_for_subpage_if_necessary(properties, + existing_layer.clone()) + } + existing_layer.update_layer(properties); true } @@ -868,32 +875,9 @@ impl IOCompositor { } // If this layer contains a subpage, then create the root layer for that subpage now. - if let Some(ref subpage_pipeline_id) = layer_properties.subpage_pipeline_id { - let subpage_layer_properties = LayerProperties { - id: LayerId::null(), - parent_id: None, - rect: Rect::new(Point2D::zero(), layer_properties.rect.size), - background_color: layer_properties.background_color, - scroll_policy: ScrollPolicy::Scrollable, - transform: Matrix4::identity(), - perspective: Matrix4::identity(), - subpage_pipeline_id: layer_properties.subpage_pipeline_id, - establishes_3d_context: true, - scrolls_overflow_area: true, - }; - - 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, - subpage_layer_properties, - wants_scroll_events, - new_layer.tile_size); - *subpage_layer.masks_to_bounds.borrow_mut() = true; - new_layer.add_child(subpage_layer); - self.pending_subpages.insert(*subpage_pipeline_id); + if layer_properties.subpage_pipeline_id.is_some() { + self.create_root_layer_for_subpage_if_necessary(layer_properties, + new_layer.clone()) } parent_layer.add_child(new_layer.clone()); @@ -902,6 +886,46 @@ impl IOCompositor { self.dump_layer_tree(); } + fn create_root_layer_for_subpage_if_necessary(&mut self, + layer_properties: LayerProperties, + parent_layer: Rc>) { + if parent_layer.children + .borrow() + .iter() + .any(|child| child.extra_data.borrow().subpage_info.is_some()) { + return + } + + let subpage_pipeline_id = + layer_properties.subpage_pipeline_id + .expect("create_root_layer_for_subpage() called for non-subpage?!"); + let subpage_layer_properties = LayerProperties { + id: LayerId::null(), + parent_id: None, + rect: Rect::new(Point2D::zero(), layer_properties.rect.size), + background_color: layer_properties.background_color, + scroll_policy: ScrollPolicy::Scrollable, + transform: Matrix4::identity(), + perspective: Matrix4::identity(), + subpage_pipeline_id: Some(subpage_pipeline_id), + establishes_3d_context: true, + scrolls_overflow_area: true, + }; + + 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, + subpage_layer_properties, + wants_scroll_events, + parent_layer.tile_size); + *subpage_layer.masks_to_bounds.borrow_mut() = true; + parent_layer.add_child(subpage_layer); + self.pending_subpages.insert(subpage_pipeline_id); + } + fn send_window_size(&self) { let dppx = self.page_zoom * self.device_pixels_per_screen_px(); let initial_viewport = self.window_size.as_f32() / dppx; diff --git a/components/compositing/compositor_layer.rs b/components/compositing/compositor_layer.rs index 9c7d567c43aa..a59e67eb5762 100644 --- a/components/compositing/compositor_layer.rs +++ b/components/compositing/compositor_layer.rs @@ -211,6 +211,7 @@ pub enum ScrollEventResult { impl CompositorLayer for Layer { fn update_layer_except_bounds(&self, layer_properties: LayerProperties) { self.extra_data.borrow_mut().scroll_policy = layer_properties.scroll_policy; + self.extra_data.borrow_mut().subpage_info = layer_properties.subpage_pipeline_id; *self.transform.borrow_mut() = layer_properties.transform; *self.perspective.borrow_mut() = layer_properties.perspective; diff --git a/components/compositing/constellation.rs b/components/compositing/constellation.rs index 4f74a90aedb5..0bf7911f8afc 100644 --- a/components/compositing/constellation.rs +++ b/components/compositing/constellation.rs @@ -1311,6 +1311,17 @@ impl Constellation fn handle_activate_document_msg(&mut self, pipeline_id: PipelineId) { debug!("Document ready to activate {:?}", pipeline_id); + if let Some(ref child_pipeline) = self.pipelines.get(&pipeline_id) { + if let Some(ref parent_info) = child_pipeline.parent_info { + if let Some(parent_pipeline) = self.pipelines.get(&parent_info.0) { + let _ = parent_pipeline.script_chan + .send(ConstellationControlMsg::FramedContentChanged( + parent_info.0, + parent_info.1)); + } + } + } + // If this pipeline is already part of the current frame tree, // we don't need to do anything. if self.pipeline_is_in_current_frame(pipeline_id) { diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index 46cda6c4aa45..4c26c15e6f52 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -107,6 +107,7 @@ pub enum ReflowReason { ImageLoaded, RequestAnimationFrame, WebFontLoaded, + FramedContentChanged, } pub type ScrollPoint = Point2D; @@ -1425,6 +1426,7 @@ fn debug_reflow_events(goal: &ReflowGoal, query_type: &ReflowQueryType, reason: ReflowReason::ImageLoaded => "\tImageLoaded", ReflowReason::RequestAnimationFrame => "\tRequestAnimationFrame", ReflowReason::WebFontLoaded => "\tWebFontLoaded", + ReflowReason::FramedContentChanged => "\tFramedContentChanged", }); println!("{}", debug_msg); diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index d4339cb2b05a..0ce6d883a4c0 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -1127,6 +1127,8 @@ impl ScriptThread { ConstellationControlMsg::DispatchFrameLoadEvent { target: pipeline_id, parent: containing_id } => self.handle_frame_load_event(containing_id, pipeline_id), + ConstellationControlMsg::FramedContentChanged(containing_pipeline_id, subpage_id) => + self.handle_framed_content_changed(containing_pipeline_id, subpage_id), ConstellationControlMsg::ReportCSSError(pipeline_id, filename, line, column, msg) => self.handle_css_error_reporting(pipeline_id, filename, line, column, msg), } @@ -1483,6 +1485,22 @@ impl ScriptThread { } } + fn handle_framed_content_changed(&self, + parent_pipeline_id: PipelineId, + subpage_id: SubpageId) { + let borrowed_page = self.root_page(); + let page = borrowed_page.find(parent_pipeline_id).unwrap(); + let doc = page.document(); + let frame_element = doc.find_iframe(subpage_id); + if let Some(ref frame_element) = frame_element { + frame_element.upcast::().dirty(NodeDamage::OtherNodeDamage); + let window = page.window(); + window.reflow(ReflowGoal::ForDisplay, + ReflowQueryType::NoQuery, + ReflowReason::FramedContentChanged); + } + } + /// Handles a mozbrowser event, for example see: /// https://developer.mozilla.org/en-US/docs/Web/Events/mozbrowserloadstart fn handle_mozbrowser_event_msg(&self, diff --git a/components/script_traits/lib.rs b/components/script_traits/lib.rs index a4d07966028e..9229c626ab67 100644 --- a/components/script_traits/lib.rs +++ b/components/script_traits/lib.rs @@ -143,6 +143,8 @@ pub enum ConstellationControlMsg { /// The pipeline that contains a frame loading the target pipeline. parent: PipelineId }, + /// Notifies a parent frame that one of its child frames is now active. + FramedContentChanged(PipelineId, SubpageId), /// Report an error from a CSS parser for the given pipeline ReportCSSError(PipelineId, String, u32, u32, String), } diff --git a/tests/wpt/mozilla/meta/MANIFEST.json b/tests/wpt/mozilla/meta/MANIFEST.json index 44620f9b2262..fa0b73ee9204 100644 --- a/tests/wpt/mozilla/meta/MANIFEST.json +++ b/tests/wpt/mozilla/meta/MANIFEST.json @@ -1688,6 +1688,18 @@ "url": "/_mozilla/css/iframe/multiple_external.html" } ], + "css/iframe/navigation.html": [ + { + "path": "css/iframe/navigation.html", + "references": [ + [ + "/_mozilla/css/iframe/navigation_ref.html", + "==" + ] + ], + "url": "/_mozilla/css/iframe/navigation.html" + } + ], "css/iframe/overflow.html": [ { "path": "css/iframe/overflow.html", diff --git a/tests/wpt/mozilla/tests/css/iframe/navigation.html b/tests/wpt/mozilla/tests/css/iframe/navigation.html new file mode 100644 index 000000000000..183bf26964c3 --- /dev/null +++ b/tests/wpt/mozilla/tests/css/iframe/navigation.html @@ -0,0 +1,36 @@ + + + + + + + + + diff --git a/tests/wpt/mozilla/tests/css/iframe/navigation_ref.html b/tests/wpt/mozilla/tests/css/iframe/navigation_ref.html new file mode 100644 index 000000000000..0207d0908e16 --- /dev/null +++ b/tests/wpt/mozilla/tests/css/iframe/navigation_ref.html @@ -0,0 +1,17 @@ + + + + + + + +