From f47e9a70b48d105ca2bab4ea8b5f2ad801e82353 Mon Sep 17 00:00:00 2001 From: ldm0 Date: Sun, 18 Oct 2026 01:54:21 +0800 Subject: [PATCH] fix(html): start child meta refresh after owner load delivery Capture refresh candidates during parsing and load callbacks as document-owned pending work, then arm their timers only after the iframe owner load callback finishes. DOM readyState complete alone does not establish that load callbacks have run, so immediate mutation timers could change a fragment before the first iframe load observed the original URL. Use FrameOwnerStore's exact-document load progress to admit timer activation. Keep pending candidates when meta elements move, preserve active deadlines for identical candidates, and cancel both pending and armed work on retirement. Add a Browser integration test that reproduces the premature fragment change, and a Page task-executor ordering control. WPT same-document-refresh.html passes with bec88e11cf9 and fails with6faf8d3fed2 before this fix. Fixes: 6faf8d3fed2 (fix(html): schedule refresh mutations in loaded child documents) Validation: - 15 related child refresh and Browser integration tests pass - cargo fmt --all - cargo clippy --workspace --all-targets --all-features -- -D warnings - cargo nextest run --no-fail-fast --- moli-core/tests/meta_refresh_load_order.rs | 64 +++++++++++++++ .../src/frame_owner_model/records.rs | 13 +++ .../src/frame_owner_model/store.rs | 12 +++ .../native_bridge/context_host/host_loads.rs | 80 ++++++++++++++----- .../extracted/frame_navigation_and_history.rs | 51 ++++++++++++ 5 files changed, 200 insertions(+), 20 deletions(-) create mode 100644 moli-core/tests/meta_refresh_load_order.rs 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);