diff --git a/moli-renderer-v8/src/runtime/owner_local_store/entry.rs b/moli-renderer-v8/src/runtime/owner_local_store/entry.rs index 9b49afb7ed..2cf54712d5 100644 --- a/moli-renderer-v8/src/runtime/owner_local_store/entry.rs +++ b/moli-renderer-v8/src/runtime/owner_local_store/entry.rs @@ -357,13 +357,20 @@ impl LivePageEntry { &mut self, prepared: PageVmPreparedFollowedNavigationCommit, ) -> Result { - assert!( - self.pending_phase_one_navigation.is_none(), - "a source navigation commit cannot coexist with pending phase one" - ); + // Commit validation and WindowProxy detachment can fail. Keep the + // complete source residence live until they succeed; every operation + // after that boundary is synchronous and infallible. let navigation = self .page_vm_mut() .commit_prepared_followed_location_navigation(prepared)?; + if let Some(pending) = self.pending_phase_one_navigation.take() { + let (residence, mut metadata) = pending.into_parts(); + let mut page_vm = residence.into_navigation_triggered_page_vm(); + metadata.complete_service_worker_follow(&mut page_vm); + self.install_resumed_phase_one_page_vm(page_vm); + } + self.retire_document_lifecycle_turn(); + self.page_vm_mut().retire_committed_main_script_vm(); Ok(navigation) } diff --git a/moli-renderer-v8/src/runtime/page_vm/followed_navigation.rs b/moli-renderer-v8/src/runtime/page_vm/followed_navigation.rs index eea5846928..e4ef9400c4 100644 --- a/moli-renderer-v8/src/runtime/page_vm/followed_navigation.rs +++ b/moli-renderer-v8/src/runtime/page_vm/followed_navigation.rs @@ -17,7 +17,6 @@ use crate::runtime::{ ExternalRawDocumentBodyStream, PageId, PageVmFollowNavigationTurnOutcome, PageVmFollowedNavigationBuildOutcome, PageVmFollowedNavigationMetadata, PageVmInitStage, PageVmPendingPhaseOneNavigation, PendingDocumentLifecycleTurn, RendererBrowserContextRuntime, - RendererDocumentLifecycleTransition, RendererDocumentTerminationReason, RendererLifecycleStartReason, RendererPendingDownloadActivation, RendererPendingDownloadResponse, }; @@ -789,21 +788,6 @@ impl PageVm { | LoadedFollowedLocationNavigation::ExternalDocument { .. }) => loaded, }; - let termination = self.document_lifecycle.request_termination( - self.document_lifecycle.identity(), - RendererDocumentTerminationReason::SupersededByCrossDocumentNavigation, - ); - debug_assert!( - matches!( - termination, - RendererDocumentLifecycleTransition::Recorded(_) - | RendererDocumentLifecycleTransition::Deferred - | RendererDocumentLifecycleTransition::Duplicate - ), - "cross-document navigation should terminate the active renderer document: {termination:?}" - ); - *pending_document_lifecycle_turn = None; - Ok(PageVmDocumentCommitPreparation::Prepared(Box::new( PageVmPreparedFollowedNavigationCommit { initiator_url, @@ -943,16 +927,33 @@ impl PageVm { } = prepared; let env = self.followed_location_navigation_env(); let runtime_hooks = self.runtime_hooks.clone().for_cross_document_commit(); + #[cfg(test)] + let runtime_hooks = { + let mut runtime_hooks = runtime_hooks; + if loaded.has_header_for_test("x-moli-test-missing-navigation-script-environment") { + runtime_hooks.renderer_page_script_environment = None; + } + runtime_hooks + }; let browser_context_runtime = runtime_hooks.browser_context_runtime.clone(); let local_executor = self.local_executor.clone(); let request_client = self.request_client.clone(); let page_id = self.page_id; + #[cfg(test)] + let commit_page_id = + if loaded.has_header_for_test("x-moli-test-mismatched-navigation-page-id") { + PageId::new_for_testing(page_id.as_u64() + 1) + } else { + page_id + }; + #[cfg(not(test))] + let commit_page_id = page_id; let commit_result = (|| { ensure!( runtime_hooks.has_renderer_page_script_environment(), "owner-managed navigation commit requires a renderer Page script environment" ); - self.commit_main_window_proxy_navigation() + self.commit_main_window_proxy_navigation(commit_page_id) })(); if let Err(error) = commit_result { self.reject_failed_followed_location_navigation( @@ -1235,7 +1236,10 @@ impl PageVm { let env = self.followed_location_navigation_env(); let runtime_hooks = self.runtime_hooks.clone().for_cross_document_commit(); if runtime_hooks.has_renderer_page_script_environment() { - self.commit_main_window_proxy_navigation()?; + self.commit_main_window_proxy_navigation(self.page_id)?; + self.retire_committed_main_script_vm(); + } else { + self.record_main_navigation_commit(); } bootstrap_committed_followed_location_navigation( self.page_id, diff --git a/moli-renderer-v8/src/runtime/page_vm/mod.rs b/moli-renderer-v8/src/runtime/page_vm/mod.rs index c80f4109cf..da4aa87156 100644 --- a/moli-renderer-v8/src/runtime/page_vm/mod.rs +++ b/moli-renderer-v8/src/runtime/page_vm/mod.rs @@ -2191,28 +2191,46 @@ impl PageVm { .page_task_producer_routes_match(sources) } - fn commit_main_window_proxy_navigation(&mut self) -> Result<()> { - let Some(mut vm) = self.vm.take() else { - return Err(anyhow::anyhow!( - "main navigation attempted to commit an already retired PageVm" - )); - }; + fn commit_main_window_proxy_navigation(&mut self, page_id: PageId) -> Result<()> { + let vm = self.vm.as_mut().ok_or_else(|| { + anyhow::anyhow!("main navigation attempted to commit an already retired PageVm") + })?; + // ScriptVm validates ownership and proxy identity before detach_global. + // An Err therefore leaves the inspector, context resources and parser + // intact. Keep the ScriptVm until its parser residence has been retired. + vm.detach_main_window_proxy_for_navigation_commit(page_id.as_u64())?; + self.record_main_navigation_commit(); + tracing::debug!( + page_id = self.page_id.as_u64(), + "committed main navigation before replacement realm bootstrap" + ); + Ok(()) + } + + fn record_main_navigation_commit(&mut self) { + let termination = self.document_lifecycle.request_termination( + self.document_lifecycle.identity(), + RendererDocumentTerminationReason::SupersededByCrossDocumentNavigation, + ); + debug_assert!( + matches!( + termination, + RendererDocumentLifecycleTransition::Recorded(_) + | RendererDocumentLifecycleTransition::Deferred + | RendererDocumentLifecycleTransition::Duplicate + ), + "cross-document navigation should terminate the active renderer document: {termination:?}" + ); + } + + pub(in crate::runtime) fn retire_committed_main_script_vm(&mut self) { + let mut vm = self + .vm + .take() + .expect("committed main navigation must retain its source ScriptVm until retirement"); vm.detach_default_inspector_context_for_context_teardown(); vm.close_page_context_resources_for_context_teardown(); - match vm.detach_main_window_proxy_for_navigation_commit(self.page_id.as_u64()) { - Ok(()) => { - tracing::debug!( - page_id = self.page_id.as_u64(), - "committed main navigation before replacement realm bootstrap" - ); - drop(vm); - Ok(()) - } - Err(error) => { - self.vm = Some(vm); - Err(error) - } - } + drop(vm); } // Fresh PageVm bootstrap is a special V8-entry boundary. diff --git a/moli-renderer-v8/src/runtime/phase_one/pending_residence.rs b/moli-renderer-v8/src/runtime/phase_one/pending_residence.rs index d72eb80bb6..b2e78b3e31 100644 --- a/moli-renderer-v8/src/runtime/phase_one/pending_residence.rs +++ b/moli-renderer-v8/src/runtime/phase_one/pending_residence.rs @@ -133,6 +133,18 @@ impl PendingPhaseOneResidence { } } + /// Retire the source parser while keeping its PageVm live for a prepared + /// cross-document navigation commit. This never waits for source input. + pub(in crate::runtime) fn into_navigation_triggered_page_vm(self) -> PageVm { + match self { + Self::ParserBlockingSourceLoad { runtime, .. } + | Self::ClosedInputPageWork { runtime, .. } => { + (*runtime).into_navigation_triggered_page_vm() + } + Self::OpenStreaming(continuation) => continuation.into_navigation_triggered_page_vm(), + } + } + pub(in crate::runtime) async fn resume(self) -> Result { match self { Self::ParserBlockingSourceLoad { runtime, started } diff --git a/moli-renderer-v8/src/runtime/phase_one/streaming_residence.rs b/moli-renderer-v8/src/runtime/phase_one/streaming_residence.rs index 9404fde33c..a1c3416056 100644 --- a/moli-renderer-v8/src/runtime/phase_one/streaming_residence.rs +++ b/moli-renderer-v8/src/runtime/phase_one/streaming_residence.rs @@ -74,6 +74,14 @@ impl PendingStreamingPhaseOneContinuation { self.input.has_ready_input() } + pub(super) fn into_navigation_triggered_page_vm(self) -> PageVm { + let Self { runtime, input, .. } = self; + // Dropping the input receiver cancels the old body bridge without + // waiting for its response to finish. + drop(input); + (*runtime).into_navigation_triggered_page_vm() + } + pub(in crate::runtime) async fn resume(self) -> Result { self.runtime.publish_processing_main_document_phase(); let Self { diff --git a/moli-renderer-v8/src/runtime/tests/open_streaming.rs b/moli-renderer-v8/src/runtime/tests/open_streaming.rs index da5d6200f0..302bbe189a 100644 --- a/moli-renderer-v8/src/runtime/tests/open_streaming.rs +++ b/moli-renderer-v8/src/runtime/tests/open_streaming.rs @@ -21,6 +21,267 @@ struct OpenStreamingPage { activity_wake_rx: RendererExternalActivityTestReceiver, } +const SELF_REPLACE_WHILE_PARSING_HTML: &str = r#""#; + +#[tokio::test(flavor = "multi_thread")] +async fn location_replace_commits_while_source_phase_one_stream_remains_open() { + let (base_url, server) = spawn_owner_wake_server_with_content_type( + "/page", + "
replacement document
", + "text/html", + Duration::ZERO, + ) + .await; + // The timer runs as Page work after the parser has parked waiting for + // more body input. Keep the source stream open throughout the commit. + let mut source = OpenStreamingPage::create(&base_url, SELF_REPLACE_WHILE_PARSING_HTML).await; + + tokio::time::timeout( + Duration::from_secs(5), + recv_page_lifecycle_until( + &mut source.activity_wake_rx, + &source.page, + RendererDocumentLifecycleMilestone::Load, + ), + ) + .await + .expect("self-replacement must reach Load before the source stream reaches EOF"); + tokio::time::timeout(Duration::from_secs(5), source.body_tx.closed()) + .await + .expect("committing the replacement must release the source body receiver"); + assert!( + source.completion_tx.send(Ok(())).is_err(), + "the discarded phase-one bridge must release its completion receiver" + ); + let (reply, _) = source + .page + .run_async_command(RendererPageCommand::EvaluateExpression { + expression: "JSON.stringify([!!document.getElementById('replacement'), document.readyState, history.length, typeof __sourceDocument])".to_owned(), + await_promise: false, + }) + .await + .expect("replacement Document should remain usable after phase-one settlement"); + assert_eq!( + renderer_json_value(reply), + Some(serde_json::json!("[true,\"complete\",1,\"undefined\"]")) + ); + let state = RendererPageTestingHandle::new_for_testing(&source.page) + .current_page_state_async() + .await + .expect("replacement state should be published"); + assert_eq!(state.final_url().as_str(), format!("{base_url}/page")); + assert_eq!( + has_pending_location_navigation_for_test(&source.page).await, + Some(false) + ); + + source + .page + .close_async() + .await + .expect("replacement Page should close"); + server.await.expect("self-replacement server should finish"); +} + +#[tokio::test(flavor = "multi_thread")] +async fn location_replace_without_document_preserves_pending_phase_one_stream() { + let listener = TcpListener::bind("127.0.0.1:0") + .await + .expect("bind no-Document navigation server"); + let address = listener.local_addr().expect("navigation server address"); + let server = tokio::spawn(async move { + let (mut stream, _) = listener + .accept() + .await + .expect("accept self-replacement request"); + let request = read_owner_wake_http_request_head(&mut stream).await; + assert_eq!(request.lines().next().unwrap(), "GET /page HTTP/1.1"); + stream + .write_all(b"HTTP/1.1 204 No Content\r\nContent-Length: 0\r\nConnection: close\r\n\r\n") + .await + .expect("write no-Document navigation response"); + }); + let mut source = OpenStreamingPage::create( + &format!("http://{address}"), + SELF_REPLACE_WHILE_PARSING_HTML, + ) + .await; + tokio::time::timeout(Duration::from_secs(5), server) + .await + .expect("self-replacement request should reach the server") + .expect("no-Document navigation server should finish"); + // This command is ordered after the checked-out navigation task, so the + // 204 has been settled before the remaining source bytes are delivered. + let (reply, _) = source + .page + .run_async_command(RendererPageCommand::EvaluateExpression { + expression: "__sourceDocument".to_owned(), + await_promise: false, + }) + .await + .expect("the source Document must remain active after a 204 navigation"); + assert_eq!(renderer_json_value(reply), Some(serde_json::json!(true))); + source + .body_tx + .send(b"".to_vec()) + .await + .expect("the pending parser must retain its source body receiver"); + drop(source.body_tx); + source + .completion_tx + .send(Ok(())) + .expect("source body should finish"); + tokio::time::timeout( + Duration::from_secs(5), + recv_page_lifecycle_until( + &mut source.activity_wake_rx, + &source.page, + RendererDocumentLifecycleMilestone::Load, + ), + ) + .await + .expect("the source parser should resume and reach Load after the 204"); + let (reply, _) = source + .page + .run_async_command(RendererPageCommand::EvaluateExpression { + expression: + "JSON.stringify([__sourceDocument, __sourceTailParsed, document.readyState])" + .to_owned(), + await_promise: false, + }) + .await + .expect("source parser tail should execute in the original Document"); + assert_eq!( + renderer_json_value(reply), + Some(serde_json::json!("[true,true,\"complete\"]")) + ); + source + .page + .close_async() + .await + .expect("source Page should close"); +} + +#[tokio::test(flavor = "multi_thread")] +async fn location_replace_commit_precondition_failure_preserves_open_source_stream() { + assert_failed_prepared_navigation_preserves_open_source_stream( + "x-moli-test-missing-navigation-script-environment", + ) + .await; +} + +#[tokio::test(flavor = "multi_thread")] +async fn location_replace_window_proxy_validation_failure_preserves_open_source_stream() { + assert_failed_prepared_navigation_preserves_open_source_stream( + "x-moli-test-mismatched-navigation-page-id", + ) + .await; +} + +async fn assert_failed_prepared_navigation_preserves_open_source_stream( + injection_header: &'static str, +) { + let (base_url, server) = spawn_owner_wake_server_with_content_type_and_headers( + "/page", + "
replacement document
", + "text/html", + vec![(injection_header, "1")], + Duration::ZERO, + ) + .await; + let mut source = OpenStreamingPage::create( + &base_url, + r#""#, + ) + .await; + tokio::time::timeout(Duration::from_secs(5), server) + .await + .expect("the replacement response must exist before testing commit failure") + .expect("prepared navigation failure server should finish"); + + // Owner commands are ordered after the checked-out navigation task. The + // response reached Prepared, but its fallible commit must return a Live + // entry with the original context, inspector and parser still resident. + let messages = dispatch_runtime_protocol_for_test( + &source.page, + serde_json::json!({ + "id": 1, + "method": "Runtime.evaluate", + "params": { + "expression": "window === __sourceWindow && document === __sourceDocument && !document.getElementById('replacement')", + "returnByValue": true + } + }), + ) + .await + .expect("commit validation failure must preserve the source Inspector context"); + let response = runtime_protocol_response_by_id(&messages, 1) + .expect("source Inspector evaluation should reply"); + assert_eq!(response["result"]["result"]["value"], true); + assert_eq!( + has_pending_location_navigation_for_test(&source.page).await, + Some(false) + ); + assert!( + !source.body_tx.is_closed(), + "a failed Prepared commit must retain the source body receiver" + ); + source + .body_tx + .send(b"".to_vec()) + .await + .expect("the retained source parser must accept later body bytes"); + drop(source.body_tx); + source + .completion_tx + .send(Ok(())) + .expect("the original source stream must retain its completion receiver"); + let events = tokio::time::timeout( + Duration::from_secs(5), + recv_page_lifecycle_until( + &mut source.activity_wake_rx, + &source.page, + RendererDocumentLifecycleMilestone::Load, + ), + ) + .await + .expect("the original source parser must resume and reach Load after commit failure"); + assert!( + !events.iter().any(|event| matches!( + event.kind, + RendererDocumentLifecycleEventKind::Terminated { + reason: super::super::RendererDocumentTerminationReason::SupersededByCrossDocumentNavigation, + .. + } + )), + "a failed Prepared commit must not terminate the source lifecycle" + ); + let (reply, _) = source + .page + .run_async_command(RendererPageCommand::EvaluateExpression { + expression: "JSON.stringify([window === __sourceWindow, document === __sourceDocument, __sourceTailParsed, document.readyState])".to_owned(), + await_promise: false, + }) + .await + .expect("the source Document must remain usable after its parser reaches EOF"); + assert_eq!( + renderer_json_value(reply), + Some(serde_json::json!("[true,true,true,\"complete\"]")) + ); + source + .page + .close_async() + .await + .expect("source Page should close after a failed Prepared commit"); +} + #[tokio::test(flavor = "multi_thread")] async fn owner_loop_executes_async_script_while_main_document_stream_remains_open() { assert_open_stream_work_executes_before_eof(ASYNC_SCRIPT_HTML, "async script").await;