diff --git a/moli-core/tests/meta_refresh_load_order.rs b/moli-core/tests/meta_refresh_load_order.rs new file mode 100644 index 0000000000..6d4e9ad384 --- /dev/null +++ b/moli-core/tests/meta_refresh_load_order.rs @@ -0,0 +1,64 @@ +use anyhow::Result; +use moli_core::runtime::{Browser, BrowserConfig, RenderedDomWaitUntil}; +use moli_test_support::FixtureServer; +use std::time::Duration; +use url::Url; + +#[tokio::test] +async fn zero_delay_child_meta_refresh_preserves_initial_owner_load_url() -> Result<()> { + let server = FixtureServer::spawn().await?; + let child_markup = r##" +

parent

section
#section +"##; + let mut child = Url::parse(&server.url("/compat/child-dynamic-markup-document"))?; + child.query_pairs_mut().append_pair("markup", child_markup); + let child_json = serde_json::to_string(child.as_str())?; + let parent_markup = format!( + r#""#, + ); + let mut parent = Url::parse(&server.url("/compat/child-dynamic-markup-document"))?; + parent + .query_pairs_mut() + .append_pair("markup", &parent_markup); + let browser = Browser::new(BrowserConfig::default())?; + let mut page = browser + .fetch_with_wait_until( + parent.as_str(), + RenderedDomWaitUntil::DomContentLoaded, + Duration::from_secs(3), + ) + .await?; + browser + .wait_for_script_truthy( + &mut page, + "messages.includes('hash:#section')", + Duration::from_secs(3), + ) + .await?; + let state = page + .evaluate_runtime_expression_async("JSON.stringify(messages)") + .await?; + assert_eq!( + state.get("value").and_then(serde_json::Value::as_str), + Some(r#"["window:","owner:","hash:#section"]"#), + "first iframe load must precede its zero-delay refresh: {state}", + ); + server.shutdown().await; + Ok(()) +} diff --git a/moli-renderer-v8/src/frame_owner_model/records.rs b/moli-renderer-v8/src/frame_owner_model/records.rs index 8edce1012d..d2ca65fe35 100644 --- a/moli-renderer-v8/src/frame_owner_model/records.rs +++ b/moli-renderer-v8/src/frame_owner_model/records.rs @@ -938,6 +938,19 @@ impl DocumentLifecycleRecord { ) } + pub(super) fn child_owner_load_is_complete(&self) -> bool { + self.load_delivery_kind == DocumentLoadDeliveryKind::Child + && matches!( + self.load, + DocumentLoadEventProgress::Suppressed + | DocumentLoadEventProgress::OwnerElementLoadDispatched + | DocumentLoadEventProgress::DispatchingPageShow + | DocumentLoadEventProgress::PageShowDispatched + | DocumentLoadEventProgress::DispatchingFrameFinish + | DocumentLoadEventProgress::Dispatched + ) + } + pub(super) fn reserve_child_load_delivery_task( &mut self, admission_id: FrameDocumentLoadDeliveryAdmissionId, diff --git a/moli-renderer-v8/src/frame_owner_model/store.rs b/moli-renderer-v8/src/frame_owner_model/store.rs index e55f465eed..be1456eeae 100644 --- a/moli-renderer-v8/src/frame_owner_model/store.rs +++ b/moli-renderer-v8/src/frame_owner_model/store.rs @@ -2085,6 +2085,18 @@ impl FrameOwnerStore { .is_some_and(|document| document.lifecycle_progress.load_delivery_is_ready()) } + pub(crate) fn current_child_document_owner_load_is_complete( + &self, + child_handle: DomHandle, + owner: FrameDocumentTaskOwner, + ) -> bool { + self.child_document_task_owner_is_current(child_handle, owner) + && self + .documents + .get(&owner.document_id) + .is_some_and(|document| document.lifecycle_progress.child_owner_load_is_complete()) + } + pub(crate) fn reserve_current_child_document_load_delivery_task( &mut self, task: FrameDocumentLoadDeliveryTask, diff --git a/moli-renderer-v8/src/native_bridge/context_host/host_loads.rs b/moli-renderer-v8/src/native_bridge/context_host/host_loads.rs index b0ef5112d9..9329298a09 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/host_loads.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/host_loads.rs @@ -5,7 +5,6 @@ use crate::{ meta_refresh_navigation_kind, navigate_location_object_with_child_navigate_event, }, document_runtime::{DocumentRuntime, DomHandle, EventTargetHandle, MetaRefreshNavigation}, - dom::native::DocumentReadyState, frame_owner_model::{ FrameDocumentLoadDeliveryAction, FrameDocumentLoadDeliveryPhase, FrameDocumentLoadDeliveryProgress, @@ -17,7 +16,7 @@ use serde::Serialize; use std::convert::TryFrom; pub(in crate::native_bridge::context_host) struct ChildMetaRefreshNavigationTask { - timer_id: u32, + timer_id: Option, owner: crate::frame_owner_model::FrameDocumentTaskOwner, target_url: url::Url, navigation_kind: LocationNavigationKind, @@ -383,6 +382,8 @@ impl JsContextHost { } Some(FrameDocumentLoadDeliveryProgress::Finished(finish)) => { let finished_child_handle = finish.child_handle; + let runtime = unsafe { &mut *self.runtime }; + self.arm_child_meta_refresh_navigation(scope, runtime, finished_child_handle); self.publish_child_frame_load_finish(finish); self.note_lightweight_popup_child_frame_load_finished(finished_child_handle); if moli_trace::window_message_trace_enabled() { @@ -460,6 +461,8 @@ impl JsContextHost { if !self.child_meta_refresh_navigations.contains_key(&handle) { self.queue_child_meta_refresh_navigation_if_needed(scope, handle); } + let runtime = unsafe { &mut *self.runtime }; + self.arm_child_meta_refresh_navigation(scope, runtime, handle); ChildFrameLoadDeliveryPhaseResult::after_callback(progress) } @@ -653,11 +656,6 @@ impl JsContextHost { document: DomHandle, navigation: MetaRefreshNavigation, ) { - if runtime.dom_host().document_ready_state_for_handle(document) - != Some(DocumentReadyState::Complete) - { - return; - } let Some(handle) = self.child_browsing_context_host_for_document_handle(document) else { return; }; @@ -698,8 +696,45 @@ impl JsContextHost { && task.delay_ms == navigation.delay_ms }) { + self.arm_child_meta_refresh_navigation(scope, runtime, handle); return; } + if let Some(previous) = self.child_meta_refresh_navigations.insert( + handle, + ChildMetaRefreshNavigationTask { + timer_id: None, + owner, + target_url: navigation.url, + navigation_kind, + delay_ms: navigation.delay_ms, + }, + ) && let Some(timer_id) = previous.timer_id + { + runtime.cancel_timer(timer_id); + } + self.arm_child_meta_refresh_navigation(scope, runtime, handle); + } + + fn arm_child_meta_refresh_navigation( + &mut self, + scope: &mut v8::PinScope<'_, '_>, + runtime: &mut DocumentRuntime, + handle: DomHandle, + ) { + let Some(task) = self.child_meta_refresh_navigations.get(&handle) else { + return; + }; + // readyState can become complete before load callbacks are delivered. + // Keep the captured candidate until the owner element's load finishes. + if task.timer_id.is_some() + || !self + .frame_owner_store + .current_child_document_owner_load_is_complete(handle, task.owner) + { + return; + } + let owner = task.owner; + let delay_ms = task.delay_ms; let Some(window) = self.child_window_proxy_records.live_window(scope, handle) else { return; }; @@ -717,24 +752,20 @@ impl JsContextHost { let timer_id = runtime.queue_timeout( scope, callback, - u64::from(navigation.delay_ms), + u64::from(delay_ms), HostTimerOwner::ChildWindow(handle), Vec::new(), ); if timer_id == 0 { return; } - if let Some(previous) = self.child_meta_refresh_navigations.insert( - handle, - ChildMetaRefreshNavigationTask { - timer_id, - owner, - target_url: navigation.url, - navigation_kind, - delay_ms: navigation.delay_ms, - }, - ) { - runtime.cancel_timer(previous.timer_id); + if let Some(task) = self.child_meta_refresh_navigations.get_mut(&handle) + && task.owner == owner + && task.timer_id.is_none() + { + task.timer_id = Some(timer_id); + } else { + runtime.cancel_timer(timer_id); } } @@ -745,7 +776,9 @@ impl JsContextHost { let Some(task) = self.child_meta_refresh_navigations.remove(&handle) else { return; }; - unsafe { &mut *self.runtime }.cancel_timer(task.timer_id); + if let Some(timer_id) = task.timer_id { + unsafe { &mut *self.runtime }.cancel_timer(timer_id); + } } pub(crate) fn run_child_browsing_context_host_load_task_work( @@ -854,6 +887,13 @@ fn child_meta_refresh_callback<'s>( return; }; let host = unsafe { &mut *host_ptr }; + if host + .child_meta_refresh_navigations + .get(&handle) + .is_none_or(|task| task.timer_id.is_none()) + { + return; + } let Some(task) = host.child_meta_refresh_navigations.remove(&handle) else { return; }; diff --git a/moli-renderer-v8/src/script_vm/tests/dom_elements/dom_surface/extracted/frame_navigation_and_history.rs b/moli-renderer-v8/src/script_vm/tests/dom_elements/dom_surface/extracted/frame_navigation_and_history.rs index 98ed91c5ad..9f38981414 100644 --- a/moli-renderer-v8/src/script_vm/tests/dom_elements/dom_surface/extracted/frame_navigation_and_history.rs +++ b/moli-renderer-v8/src/script_vm/tests/dom_elements/dom_surface/extracted/frame_navigation_and_history.rs @@ -1358,6 +1358,57 @@ async fn detached_child_navigation_error_exposes_committed_entry_during_dispatch "committed:true|error:true:true:AbortError|finished-rejected:AbortError||true" ); } +#[tokio::test(flavor = "current_thread")] +async fn child_zero_delay_meta_refresh_waits_for_owner_load_in_page_task_executor() { + let server = StaticHttpServer::spawn_with_responder( + 2, + "Content-Type: text/html; charset=utf-8\r\n", + |_, _| { + r##" +

source

"## + .to_owned() + }, + ) + .await; + let loader = static_http_loader([server.resolve_entry("child-refresh-order.test")]); + let parent = server.url_for_host("child-refresh-order.test", "/parent"); + let child = server.url_for_host("child-refresh-order.test", "/child"); + let mut vm = new_storage_page_task_executor_test_vm_with_loader(parent.as_str(), &loader); + vm.eval(&format!( + r#" +globalThis.messages = []; +const root = document.documentElement || document.appendChild(document.createElement('html')); +const frame = document.createElement('iframe'); +frame.onload = () => messages.push('owner:' + frame.contentWindow.location.hash); +frame.src = {child:?}; +root.appendChild(frame); +'queued' +"#, + child = child.as_str(), + )) + .unwrap(); + advance_page_task_executor_until_eval_equals( + &mut vm, + &loader, + "String(messages.includes('hash:#section'))", + "true", + "zero-delay refresh should finish through the production timer and navigation sources", + ) + .await; + assert_eq!( + vm.eval("JSON.stringify(messages)").unwrap(), + r#"["window:","owner:","hash:#section"]"#, + "the first iframe load must observe the original URL before meta refresh changes it", + ); +} + async fn assert_child_meta_refresh_can_be_inserted_during_load(in_window_load: bool) { let loader = ResourceRequestClient::new(&moli_fetch::FetchConfig::default()).expect("loader"); let mut vm = new_storage_test_vm_with_loader("https://child-meta-refresh.test/", &loader);