From d90ca0bc538d91bbd94b0d0768a432dc5389ceff Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Wed, 2 Oct 2024 21:46:21 -0400 Subject: Ensure iframe load event occurs if initial about:blank is recreated with document.open. --- components/constellation/constellation.rs | 10 +++-- components/constellation/tracing.rs | 3 +- components/script/dom/document.rs | 73 +++++++++++++++---------------- components/script/script_thread.rs | 9 ++-- components/shared/script/lib.rs | 2 + components/shared/script/script_msg.rs | 4 +- 6 files changed, 53 insertions(+), 48 deletions(-) diff --git a/components/constellation/constellation.rs b/components/constellation/constellation.rs index 56baf77543f..a3c06b1945b 100644 --- a/components/constellation/constellation.rs +++ b/components/constellation/constellation.rs @@ -1678,8 +1678,8 @@ where self.handle_abort_load_url_msg(source_pipeline_id); }, // A page loaded has completed all parsing, script, and reflow messages have been sent. - FromScriptMsg::LoadComplete => { - self.handle_load_complete_msg(source_top_ctx_id, source_pipeline_id) + FromScriptMsg::LoadComplete(is_initial_about_blank) => { + self.handle_load_complete_msg(source_top_ctx_id, source_pipeline_id, is_initial_about_blank) }, // Handle navigating to a fragment FromScriptMsg::NavigatedToFragment(new_url, replacement_enabled) => { @@ -3116,7 +3116,7 @@ where } #[tracing::instrument(skip(self), fields(servo_profiling = true))] - fn handle_subframe_loaded(&mut self, pipeline_id: PipelineId) { + fn handle_subframe_loaded(&mut self, pipeline_id: PipelineId, is_initial_about_blank: bool) { let browsing_context_id = match self.pipelines.get(&pipeline_id) { Some(pipeline) => pipeline.browsing_context_id, None => return warn!("{}: Subframe loaded after closure", pipeline_id), @@ -3141,6 +3141,7 @@ where target: browsing_context_id, parent: parent_pipeline_id, child: pipeline_id, + is_initial_about_blank, }; let result = match self.pipelines.get(&parent_pipeline_id) { Some(parent) => parent.event_loop.send(msg), @@ -3668,6 +3669,7 @@ where &mut self, top_level_browsing_context_id: TopLevelBrowsingContextId, pipeline_id: PipelineId, + is_initial_about_blank: bool, ) { let mut webdriver_reset = false; if let Some((expected_pipeline_id, ref reply_chan)) = self.webdriver.load_channel { @@ -3708,7 +3710,7 @@ where .send(CompositorMsg::LoadComplete(top_level_browsing_context_id)); } } else { - self.handle_subframe_loaded(pipeline_id); + self.handle_subframe_loaded(pipeline_id, is_initial_about_blank); } } diff --git a/components/constellation/tracing.rs b/components/constellation/tracing.rs index 46335012d03..60d78baca72 100644 --- a/components/constellation/tracing.rs +++ b/components/constellation/tracing.rs @@ -1,3 +1,4 @@ + /* This Source Code Form is subject to the terms of the Mozilla Public * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ @@ -156,7 +157,7 @@ mod from_script { Self::GetTopForBrowsingContext(..) => target!("GetTopForBrowsingContext"), Self::GetBrowsingContextInfo(..) => target!("GetBrowsingContextInfo"), Self::GetChildBrowsingContextId(..) => target!("GetChildBrowsingContextId"), - Self::LoadComplete => target!("LoadComplete"), + Self::LoadComplete(..) => target!("LoadComplete"), Self::LoadUrl(..) => target!("LoadUrl"), Self::AbortLoadUrl => target!("AbortLoadUrl"), Self::PostMessage { .. } => target!("PostMessage"), diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index f3c77c4169e..c4c9afcf2e4 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -486,8 +486,9 @@ pub struct Document { visibility_state: Cell, /// status_code: Option, - inhibit_load_and_pageshow: bool, + inhibit_load_and_pageshow: Cell, window_replaced: Cell, + is_initial_about_blank: Cell, } #[derive(JSTraceable, MallocSizeOf)] @@ -541,6 +542,10 @@ impl CollectionFilter for AnchorsFilter { #[allow(non_snake_case)] impl Document { + pub(crate) fn is_initial_about_blank(&self) -> bool { + self.is_initial_about_blank.get() + } + pub fn note_node_with_dirty_descendants(&self, node: &Node) { debug_assert!(*node.owner_doc() == *self); if !node.is_connected() { @@ -2364,17 +2369,20 @@ impl Document { // Step 7. debug!("Document loads are complete."); let document = Trusted::new(self); + let was_initial_about_blank = self.is_initial_about_blank.get(); self.window .task_manager() .dom_manipulation_task_source() .queue( - task!(fire_load_event: move || { + task!(prepare_for_post_load_tasks: move || { let document = document.root(); let window = document.window(); if !window.is_alive() { return; } + notify_constellation_load(&window, was_initial_about_blank); + // Step 7.1. document.set_ready_state(DocumentReadyState::Complete); @@ -2393,7 +2401,7 @@ impl Document { // http://w3c.github.io/navigation-timing/#widl-PerformanceNavigationTiming-loadEventStart update_with_current_instant(&document.load_event_start); - if !document.inhibit_load_and_pageshow { + if !document.inhibit_load_and_pageshow.get() { debug!("About to dispatch load for {:?}", document.url()); // FIXME(nox): Why are errors silenced here? let _ = window.dispatch_event_with_target_override( @@ -2404,28 +2412,8 @@ impl Document { // http://w3c.github.io/navigation-timing/#widl-PerformanceNavigationTiming-loadEventEnd update_with_current_instant(&document.load_event_end); - if let Some(fragment) = document.url().fragment() { - document.check_and_scroll_fragment(fragment); - } - }), - self.window.upcast(), - ) - .unwrap(); - - // Step 8. - let document = Trusted::new(self); - if document.root().browsing_context().is_some() && !self.inhibit_load_and_pageshow { - self.window - .task_manager() - .dom_manipulation_task_source() - .queue( - task!(fire_pageshow_event: move || { - let document = document.root(); - let window = document.window(); - if document.page_showing.get() || !window.is_alive() { - return; - } - + //assert!(!document.page_showing.get()); + if !document.page_showing.get() { document.page_showing.set(true); let event = PageTransitionEvent::new( @@ -2442,11 +2430,15 @@ impl Document { let _ = window.dispatch_event_with_target_override( event, ); - }), - self.window.upcast(), - ) - .unwrap(); - } + } + + if let Some(fragment) = document.url().fragment() { + document.check_and_scroll_fragment(fragment); + } + }), + self.window.upcast(), + ) + .unwrap(); // Step 9. // TODO: pending application cache download process tasks. @@ -2490,10 +2482,10 @@ impl Document { }), Duration::from_secs(*time), ); - } + }; // Note: this will, among others, result in the "iframe-load-event-steps" being run. // https://html.spec.whatwg.org/multipage/#iframe-load-event-steps - document.notify_constellation_load(); + //document.notify_constellation_load(); }), self.window.upcast(), ) @@ -2710,10 +2702,6 @@ impl Document { } } - pub fn notify_constellation_load(&self) { - self.window().send_to_constellation(ScriptMsg::LoadComplete); - } - pub fn set_current_parser(&self, script: Option<&ServoParser>) { self.current_parser.set(script); } @@ -3216,6 +3204,7 @@ impl Document { .and_then(|charset| Encoding::for_label(charset.as_str().as_bytes())) .unwrap_or(UTF_8); + let is_initial_about_blank = url.as_str() == "about:blank"; let has_browsing_context = has_browsing_context == HasBrowsingContext::Yes; Document { node: Node::new_document_node(), @@ -3337,8 +3326,9 @@ impl Document { fonts: Default::default(), visibility_state: Cell::new(DocumentVisibilityState::Hidden), status_code, - inhibit_load_and_pageshow, + inhibit_load_and_pageshow: Cell::new(inhibit_load_and_pageshow), window_replaced: Cell::new(false), + is_initial_about_blank: Cell::new(is_initial_about_blank), } } @@ -5303,12 +5293,16 @@ impl DocumentMethods for Document { self.set_url(new_url); } + self.inhibit_load_and_pageshow.set(false); + // Step 13 // TODO: https://github.com/servo/servo/issues/21938 // Step 14 self.set_quirks_mode(QuirksMode::NoQuirks); + self.is_initial_about_blank.set(false); + // Step 15 let resource_threads = self .window @@ -5678,3 +5672,8 @@ fn is_named_element_with_id_attribute(elem: &Element) -> bool { // behaviour is actually implemented elem.is::() && elem.get_name().is_some_and(|name| !name.is_empty()) } + +fn notify_constellation_load(window: &Window, is_initial_about_blank: bool) { + window.send_to_constellation(ScriptMsg::LoadComplete(is_initial_about_blank)); +} + diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index fedaf2fae9b..6f8fbfd602a 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -2194,9 +2194,8 @@ impl ScriptThread { TickAllAnimations(id, ..) => Some(id), WebFontLoaded(id, ..) => Some(id), DispatchIFrameLoadEvent { - target: _, parent: id, - child: _, + .. } => Some(id), DispatchStorageEvent(id, ..) => Some(id), ReportCSSError(id, ..) => Some(id), @@ -2401,7 +2400,8 @@ impl ScriptThread { target: browsing_context_id, parent: parent_id, child: child_id, - } => self.handle_iframe_load_event(parent_id, browsing_context_id, child_id, can_gc), + is_initial_about_blank, + } => self.handle_iframe_load_event(parent_id, browsing_context_id, child_id, is_initial_about_blank, can_gc), ConstellationControlMsg::DispatchStorageEvent( pipeline_id, storage, @@ -3467,6 +3467,7 @@ impl ScriptThread { parent_id: PipelineId, browsing_context_id: BrowsingContextId, child_id: PipelineId, + is_initial_about_blank: bool, can_gc: CanGc, ) { let iframe = self @@ -3474,7 +3475,7 @@ impl ScriptThread { .borrow() .find_iframe(parent_id, browsing_context_id); match iframe { - Some(iframe) => iframe.iframe_load_event_steps(child_id, can_gc, false), + Some(iframe) => iframe.iframe_load_event_steps(child_id, can_gc, !is_initial_about_blank), None => warn!("Message sent to closed pipeline {}.", parent_id), } } diff --git a/components/shared/script/lib.rs b/components/shared/script/lib.rs index 40325bd0b3d..1156f1c7a7f 100644 --- a/components/shared/script/lib.rs +++ b/components/shared/script/lib.rs @@ -370,6 +370,8 @@ pub enum ConstellationControlMsg { parent: PipelineId, /// The pipeline that has completed loading. child: PipelineId, + /// + is_initial_about_blank: bool, }, /// Cause a `storage` event to be dispatched at the appropriate window. /// The strings are key, old value and new value. diff --git a/components/shared/script/script_msg.rs b/components/shared/script/script_msg.rs index ee8e8da1386..6d46bafc91f 100644 --- a/components/shared/script/script_msg.rs +++ b/components/shared/script/script_msg.rs @@ -181,7 +181,7 @@ pub enum ScriptMsg { ), /// All pending loads are complete, and the `load` event for this pipeline /// has been dispatched. - LoadComplete, + LoadComplete(bool), /// A new load has been requested, with an option to replace the current entry once loaded /// instead of adding a new entry. LoadUrl(LoadData, HistoryEntryReplacement), @@ -293,7 +293,7 @@ impl fmt::Debug for ScriptMsg { GetBrowsingContextInfo(..) => "GetBrowsingContextInfo", GetTopForBrowsingContext(..) => "GetParentBrowsingContext", GetChildBrowsingContextId(..) => "GetChildBrowsingContextId", - LoadComplete => "LoadComplete", + LoadComplete(..) => "LoadComplete", LoadUrl(..) => "LoadUrl", AbortLoadUrl => "AbortLoadUrl", PostMessage { .. } => "PostMessage", -- cgit v1.2.3