From ca9514fc4d96b824bba9bbd294a8fbb2cb79352e Mon Sep 17 00:00:00 2001 From: ldm0 Date: Mon, 26 Oct 2026 16:24:24 +0800 Subject: [PATCH] refactor(navigation): admit document configuration with its commit Capture target configuration inside the exact navigation admission and carry it in the renderer commit command. Apply it before consuming the prepared Document so a separate configuration acknowledgement cannot let an older snapshot overwrite an update accepted before admission. Use the same configured commit for buffered, error-document and compatibility navigation, and remove the protocol update-then-commit adapter. Cover additions and removals of queued preloads for new and replacement Pages, plus author-visible bindings, permissions and worlds. Validation: cargo fmt --all; full workspace Clippy with warnings denied; cargo nextest run --no-fail-fast (20919 passed, 19 skipped); 12 CLI CDP scenarios and Playwright routing smoke passed. --- moli-core/src/runtime/navigation_engine.rs | 69 +++++----- moli-protocol/src/conn/runtime_load.rs | 47 ++++--- .../conn/state_access/navigation_commit.rs | 36 +++--- .../src/conn/tests/navigation_commit.rs | 77 +++++++++++- .../src/conn/tests/navigation_error.rs | 6 +- .../src/conn/tests/resource_runtime.rs | 6 +- moli-protocol/src/domains/page/navigation.rs | 110 ++++++++-------- moli-renderer-v8/src/runtime/owner.rs | 2 + .../src/runtime/owner/owner_commands.rs | 30 ++++- moli-renderer-v8/src/runtime/page.rs | 10 +- .../tests/extracted/navigation_streaming.rs | 118 +++++++++--------- 11 files changed, 310 insertions(+), 201 deletions(-) diff --git a/moli-core/src/runtime/navigation_engine.rs b/moli-core/src/runtime/navigation_engine.rs index 393a8c4fd8..3802bd4dce 100644 --- a/moli-core/src/runtime/navigation_engine.rs +++ b/moli-core/src/runtime/navigation_engine.rs @@ -264,6 +264,32 @@ pub struct PreparedDocumentPageCommitConfiguration { pub fetch_subresource_interception: (bool, Option), } +impl PreparedDocumentPageCommitConfiguration { + fn into_renderer_configuration( + self, + ) -> moli_renderer_v8::RendererPreparedDocumentCommitConfiguration { + moli_renderer_v8::RendererPreparedDocumentCommitConfiguration { + web_mcp_invocation: self.web_mcp_invocation, + document_start_scripts: self.document_start_scripts, + runtime_bindings: self.runtime_bindings, + runtime_inspector_session_restore_snapshots: self + .runtime_inspector_session_restore_snapshots, + runtime_isolated_worlds: self.runtime_isolated_worlds, + permission_overrides: self.permission_overrides, + extra_http_headers: self.extra_http_headers, + document_settings: self.document_settings, + browser_resource_runtime: self.browser_resource_runtime, + navigator_identity: self.navigator_identity, + network_offline: self.network_offline, + bypass_service_worker: self.bypass_service_worker, + cache_disabled: self.cache_disabled, + blocked_url_patterns: self.blocked_url_patterns, + fetch_subresource_interception_enabled: self.fetch_subresource_interception.0, + fetch_subresource_interception_resource_type: self.fetch_subresource_interception.1, + } + } +} + impl std::fmt::Debug for PreparedDocumentPage { fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { formatter @@ -300,31 +326,7 @@ impl PreparedDocumentPage { configuration: PreparedDocumentPageCommitConfiguration, ) -> Result<()> { self.prepared - .update_commit_configuration( - moli_renderer_v8::RendererPreparedDocumentCommitConfiguration { - web_mcp_invocation: configuration.web_mcp_invocation, - document_start_scripts: configuration.document_start_scripts, - runtime_bindings: configuration.runtime_bindings, - runtime_inspector_session_restore_snapshots: configuration - .runtime_inspector_session_restore_snapshots, - runtime_isolated_worlds: configuration.runtime_isolated_worlds, - permission_overrides: configuration.permission_overrides, - extra_http_headers: configuration.extra_http_headers, - document_settings: configuration.document_settings, - browser_resource_runtime: configuration.browser_resource_runtime, - navigator_identity: configuration.navigator_identity, - network_offline: configuration.network_offline, - bypass_service_worker: configuration.bypass_service_worker, - cache_disabled: configuration.cache_disabled, - blocked_url_patterns: configuration.blocked_url_patterns, - fetch_subresource_interception_enabled: configuration - .fetch_subresource_interception - .0, - fetch_subresource_interception_resource_type: configuration - .fetch_subresource_interception - .1, - }, - ) + .update_commit_configuration(configuration.into_renderer_configuration()) .await } @@ -337,8 +339,12 @@ impl PreparedDocumentPage { pub fn start_commit( self, permit: PreparedDocumentPageCommitPermit, + configuration: Option, ) -> Result> + 'static> { - let pending = self.prepared.start_commit(permit.permit)?; + let pending = self.prepared.start_commit( + permit.permit, + configuration.map(PreparedDocumentPageCommitConfiguration::into_renderer_configuration), + )?; Ok(async move { let ( handle, @@ -362,7 +368,7 @@ impl PreparedDocumentPage { self, permit: PreparedDocumentPageCommitPermit, ) -> Result { - self.start_commit(permit)?.await + self.start_commit(permit, None)?.await } pub async fn cancel(self) -> Result<()> { @@ -373,6 +379,7 @@ impl PreparedDocumentPage { self, permit: PreparedDocumentPageCommitPermit, page: &Page, + configuration: Option, ) -> std::result::Result< PendingDocumentPageReplacement, moli_renderer_v8::RendererPageReplacementError, @@ -386,7 +393,11 @@ impl PreparedDocumentPage { ))); } Ok(PendingDocumentPageReplacement { - pending: self.prepared.start_commit_page_replacement(permit.permit)?, + pending: self.prepared.start_commit_page_replacement( + permit.permit, + configuration + .map(PreparedDocumentPageCommitConfiguration::into_renderer_configuration), + )?, }) } @@ -398,7 +409,7 @@ impl PreparedDocumentPage { CommittedDocumentPageReplacement, moli_renderer_v8::RendererPageReplacementError, > { - self.start_commit_page_replacement(permit, page)? + self.start_commit_page_replacement(permit, page, None)? .wait() .await? .adopt(page) diff --git a/moli-protocol/src/conn/runtime_load.rs b/moli-protocol/src/conn/runtime_load.rs index 13417ac5d6..737ba631ba 100644 --- a/moli-protocol/src/conn/runtime_load.rs +++ b/moli-protocol/src/conn/runtime_load.rs @@ -461,23 +461,6 @@ impl ResponseCommitReady { ) } - pub(crate) async fn update_commit_configuration( - &self, - configuration: PreparedDocumentPageCommitConfiguration, - ) -> anyhow::Result<()> { - self.prepared_page - .as_ref() - .expect("response commit-ready value must retain its prepared Page") - .update_commit_configuration(configuration) - .await - .with_context(|| { - format!( - "failed to attach commit-time target configuration for page `{}`", - self.requested_url - ) - }) - } - pub(crate) fn issue_commit_permit(&self) -> PreparedDocumentPageCommitPermit { self.prepared_page .as_ref() @@ -488,6 +471,7 @@ impl ResponseCommitReady { pub(crate) async fn commit( mut self, permit: PreparedDocumentPageCommitPermit, + configuration: PreparedDocumentPageCommitConfiguration, ) -> anyhow::Result { anyhow::ensure!( self.stable_page_target.is_none(), @@ -497,7 +481,11 @@ impl ResponseCommitReady { .prepared_page .take() .expect("response commit-ready value must retain its prepared Page"); - let built = match prepared_page.commit(permit).await { + let built = match prepared_page.start_commit(permit, Some(configuration)) { + Ok(pending) => pending.await, + Err(error) => Err(error), + }; + let built = match built { Ok(built) => built, Err(error) => { if let Some(body_capture) = self.body_capture.take() { @@ -522,6 +510,7 @@ impl ResponseCommitReady { mut self, permit: PreparedDocumentPageCommitPermit, page: &mut Page, + configuration: PreparedDocumentPageCommitConfiguration, ) -> Result, RendererPageReplacementError> { let target = self.stable_page_target.take().ok_or_else(|| { @@ -538,7 +527,11 @@ impl ResponseCommitReady { .prepared_page .take() .expect("prepared response retains its Document"); - let committed = prepared.commit_page_replacement(permit, page).await?; + let committed = prepared + .start_commit_page_replacement(permit, page, Some(configuration))? + .wait() + .await? + .adopt(page)?; self.finish_committed_document( target, committed.pending_download, @@ -553,6 +546,7 @@ impl ResponseCommitReady { &mut self, permit: PreparedDocumentPageCommitPermit, page: &Page, + configuration: PreparedDocumentPageCommitConfiguration, ) -> Result { let target = self.stable_page_target.as_ref().ok_or_else(|| { @@ -568,12 +562,13 @@ impl ResponseCommitReady { self.prepared_page .take() .expect("prepared response retains its Document") - .start_commit_page_replacement(permit, page) + .start_commit_page_replacement(permit, page, Some(configuration)) } pub(crate) fn start_new_document_commit( &mut self, permit: PreparedDocumentPageCommitPermit, + configuration: PreparedDocumentPageCommitConfiguration, ) -> anyhow::Result< impl std::future::Future> + 'static @@ -586,7 +581,7 @@ impl ResponseCommitReady { self.prepared_page .take() .expect("prepared response retains its Document") - .start_commit(permit) + .start_commit(permit, Some(configuration)) } pub(crate) async fn finish_committed_document( @@ -1695,6 +1690,7 @@ impl CdpConnection { expected: &StablePageNavigationCommitTarget, navigation: ResponseCommitReady, permit: PreparedDocumentPageCommitPermit, + configuration: PreparedDocumentPageCommitConfiguration, ) -> Result, RendererPageReplacementError> { if self @@ -1720,7 +1716,9 @@ impl CdpConnection { "navigation's Page was removed" )) })?; - navigation.commit_page_replacement(permit, page).await + navigation + .commit_page_replacement(permit, page, configuration) + .await } fn claim_background_navigation( @@ -2382,11 +2380,8 @@ impl CdpConnection { owner, navigation.final_url(), )?; - navigation - .update_commit_configuration(configuration) - .await?; let permit = navigation.issue_commit_permit(); - navigation.commit(permit).await + navigation.commit(permit, configuration).await } NavigationLoadOutcome::Loaded(navigation) => Ok(*navigation), NavigationLoadOutcome::Download(_) => { diff --git a/moli-protocol/src/conn/state_access/navigation_commit.rs b/moli-protocol/src/conn/state_access/navigation_commit.rs index 3af9e1fc88..0b97a94fa6 100644 --- a/moli-protocol/src/conn/state_access/navigation_commit.rs +++ b/moli-protocol/src/conn/state_access/navigation_commit.rs @@ -4,7 +4,6 @@ use moli_core::{ page::RendererPageReplacementError, runtime::{ BuiltDocumentPage, CompletedDocumentPageReplacement, PendingDocumentPageReplacement, - PreparedDocumentPageCommitConfiguration, }, }; @@ -46,6 +45,7 @@ impl ProtocolWorkflowState<'_> { owner: &CommandOwnerScope, token: &DocumentNavigationToken, mut navigation: ResponseCommitReady, + web_mcp_invocation: Option, transaction_slot: &AttachmentTransaction, ) -> Result<(ResponseCommitReady, NativeDocumentCommit), RendererPageReplacementError> { let conn = self.connection; @@ -54,6 +54,14 @@ impl ProtocolWorkflowState<'_> { anyhow::anyhow!("NavigationRequestNotCurrent"), )); } + // Configuration and native admission share this synchronous state + // operation. An update accepted before admission belongs to this + // Document; there is no configuration-ACK window to replay a stale + // snapshot over another session's later policy. + let mut configuration = conn + .prepared_document_commit_configuration_for_owner(owner, navigation.final_url()) + .map_err(RendererPageReplacementError::document_preserved)?; + configuration.web_mcp_invocation = web_mcp_invocation; let expected = navigation.stable_page_target().cloned(); let renderer_page = navigation.renderer_page_residence_identity(); let candidate = conn @@ -99,11 +107,15 @@ impl ProtocolWorkflowState<'_> { "navigation's Page was removed" )) })?; - NativeDocumentCommit::Replacement(navigation.start_page_replacement(permit, page)?) + NativeDocumentCommit::Replacement(navigation.start_page_replacement( + permit, + page, + configuration, + )?) } None => NativeDocumentCommit::New(Box::pin( navigation - .start_new_document_commit(permit) + .start_new_document_commit(permit, configuration) .map_err(RendererPageReplacementError::document_preserved)?, )), }; @@ -156,13 +168,9 @@ async fn commit_prepared_document( owner: CommandOwnerScope, token: DocumentNavigationToken, navigation: ResponseCommitReady, - configuration: PreparedDocumentPageCommitConfiguration, + web_mcp_invocation: Option, transaction: AttachmentTransaction, ) -> Result, RendererPageReplacementError> { - navigation - .update_commit_configuration(configuration) - .await - .map_err(RendererPageReplacementError::document_preserved)?; let admission_owner = owner.clone(); let admission_token = token.clone(); let admission_transaction = transaction.clone(); @@ -172,6 +180,7 @@ async fn commit_prepared_document( &admission_owner, &admission_token, navigation, + web_mcp_invocation, &admission_transaction, ) }) @@ -245,12 +254,7 @@ impl CdpConnection { let sender = self .background_navigation_completion_sender_for_owner(&state.owner) .expect("background navigation retains its completion channel"); - let configuration = self - .prepared_document_commit_configuration_for_owner(&state.owner, navigation.final_url()) - .map(|mut configuration| { - configuration.web_mcp_invocation = state.web_mcp_invocation.clone(); - configuration - }); + let web_mcp_invocation = state.web_mcp_invocation.clone(); let expected = navigation.stable_page_target().cloned(); let transaction = AttachmentTransaction::default(); let wait_transaction = transaction.clone(); @@ -260,14 +264,12 @@ impl CdpConnection { let task_owner = state.owner.clone(); let task = spawn_owned_protocol_wait( async move { - let configuration = - configuration.map_err(RendererPageReplacementError::document_preserved)?; commit_prepared_document( access, wait_owner, wait_token, navigation, - configuration, + web_mcp_invocation, wait_transaction, ) .await diff --git a/moli-protocol/src/conn/tests/navigation_commit.rs b/moli-protocol/src/conn/tests/navigation_commit.rs index a90ec9dad7..8f6f84bb25 100644 --- a/moli-protocol/src/conn/tests/navigation_commit.rs +++ b/moli-protocol/src/conn/tests/navigation_commit.rs @@ -77,6 +77,79 @@ async fn independent_command(ctx: &mut TestContext, id: u64) { assert!(ctx.take_response_by_id(id)["result"]["product"].is_string()); } +#[tokio::test] +async fn queued_commit_captures_preload_changes_at_native_admission() { + tokio::task::LocalSet::new() + .run_until(async { + for replace_existing in [false, true] { + for remove_preload in [false, true] { + let CommitFixture { + mut ctx, + token, + navigation, + prepared, + mut requests, + mut completions, + } = commit_fixture(replace_existing).await; + let mut preload_id = None; + if remove_preload { + ctx.process_async(json!({ + "id": 70, "sessionId": "SID-1", + "method": "Page.addScriptToEvaluateOnNewDocument", + "params": {"source": "globalThis.__commitConfiguration = 'removed'"} + })).await; + preload_id = Some(ctx.take_response_by_id(70)["result"]["identifier"].clone()); + } + ctx.conn.start_background_prepared_navigation_commit( + token, + navigation, + prepared, + ); + // Hold the real state request, not a timing assumption: + // the workflow is alive, but native admission is queued. + let admission = requests.recv().await.unwrap(); + if let Some(identifier) = preload_id { + assert!(identifier.is_string()); + ctx.process_async(json!({ + "id": 71, "sessionId": "SID-1", + "method": "Page.removeScriptToEvaluateOnNewDocument", + "params": {"identifier": identifier} + })).await; + ctx.expect_result(71, json!({}), Some("SID-1")); + } else { + ctx.process_async(json!({ + "id": 71, "sessionId": "SID-1", + "method": "Page.addScriptToEvaluateOnNewDocument", + "params": {"source": "globalThis.__commitConfiguration = 'latest'"} + })).await; + assert!(ctx.take_response_by_id(71)["result"]["identifier"].is_string()); + } + independent_command(&mut ctx, 72).await; + assert!(completions.try_recv().is_err()); + admission.apply(&mut ctx.conn); + if replace_existing { + requests.recv().await.unwrap().apply(&mut ctx.conn); + } + let completion = completions.recv().await.unwrap(); + assert!(matches!(&completion, BackgroundNavigationCompletion::PreparedDocument(result) if result.navigation.is_ok())); + let outcome = ctx.conn.drain_background_navigation_completion_turn_async(completion).await; + let (messages, _) = ctx.route_completed_command_outcome_for_test(outcome).await; + assert_eq!(messages.iter().filter(|message| message["id"] == json!(1)).count(), 1); + ctx.process_async(json!({ + "id": 73, "sessionId": "SID-1", "method": "Runtime.evaluate", + "params": {"expression": "String(globalThis.__commitConfiguration)", "returnByValue": true} + })).await; + assert_eq!( + ctx.take_response_by_id(73)["result"]["result"]["value"], + json!(if remove_preload { "undefined" } else { "latest" }), + "replace_existing={replace_existing}, remove_preload={remove_preload}" + ); + } + } + }) + .await; +} + #[tokio::test] async fn aborting_owned_commit_before_admission_preserves_page_and_reports_failure() { tokio::task::LocalSet::new() @@ -161,8 +234,8 @@ async fn prepared_navigation_commit_yields_between_configuration_commit_and_adop navigation.clone(), prepared, ); - // Configuration has completed, but no commit is admitted until - // the actor handles this synchronous operation. + // Configuration is captured together with native admission + // when the actor handles this synchronous operation. let admission = requests.recv().await.unwrap(); independent_command(&mut ctx, 90).await; assert!(completions.try_recv().is_err()); diff --git a/moli-protocol/src/conn/tests/navigation_error.rs b/moli-protocol/src/conn/tests/navigation_error.rs index 0c8b7e25cd..3c43ace58d 100644 --- a/moli-protocol/src/conn/tests/navigation_error.rs +++ b/moli-protocol/src/conn/tests/navigation_error.rs @@ -961,8 +961,12 @@ async fn navigation_error_document_uses_typed_failure_request_after_context() { panic!("network failure must prepare an error document commit"); }; let permit = prepared.issue_commit_permit(); + let configuration = ctx + .conn + .prepared_document_commit_configuration_for_owner(&navigation.owner, prepared.final_url()) + .unwrap(); let loaded = prepared - .commit(permit) + .commit(permit, configuration) .await .expect("error document should commit"); assert_eq!(loaded.requested_url, unreachable_url); diff --git a/moli-protocol/src/conn/tests/resource_runtime.rs b/moli-protocol/src/conn/tests/resource_runtime.rs index 16bced60cd..2e48e97e90 100644 --- a/moli-protocol/src/conn/tests/resource_runtime.rs +++ b/moli-protocol/src/conn/tests/resource_runtime.rs @@ -57,13 +57,9 @@ async fn commit_navigation_outcome_for_session_test( navigation.final_url(), ) .expect("test navigation commit configuration should resolve"); - navigation - .update_commit_configuration(configuration) - .await - .expect("test navigation commit configuration should apply"); let permit = navigation.issue_commit_permit(); navigation - .commit(permit) + .commit(permit, configuration) .await .expect("test navigation should commit") } diff --git a/moli-protocol/src/domains/page/navigation.rs b/moli-protocol/src/domains/page/navigation.rs index 444cc56173..c8b26c3b7b 100644 --- a/moli-protocol/src/domains/page/navigation.rs +++ b/moli-protocol/src/domains/page/navigation.rs @@ -3506,67 +3506,62 @@ async fn complete_materialized_navigation_into_buffer_inner_async( network::MaterializedNavigationLoadOutcome::ResponseCommitReady(navigation) => { let navigation = *navigation; let stable_page_target = navigation.stable_page_target().cloned(); - let update_result = match conn.prepared_document_commit_configuration_for_owner( + let configuration = conn.prepared_document_commit_configuration_for_owner( &state.owner, navigation.final_url(), - ) { + ); + match configuration { + Err(error) => push_navigation_commit_error(out, &state, error), Ok(mut configuration) => { configuration.web_mcp_invocation = state.web_mcp_invocation.clone(); - navigation.update_commit_configuration(configuration).await - } - Err(error) => Err(error), - }; - if let Err(error) = update_result { - push_navigation_commit_error(out, &state, error); - } else { - let renderer_page = navigation.renderer_page_residence_identity(); - let candidate = conn.prepare_renderer_agent_candidate_token_for_owner( - &state.owner, - &token, - navigation.renderer_devtools_agent_token(), - ); - match candidate.and_then(|candidate| match stable_page_target.as_ref() { - Some(expected) => conn.commit_renderer_agent_candidate_for_existing_page( + let renderer_page = navigation.renderer_page_residence_identity(); + let candidate = conn.prepare_renderer_agent_candidate_token_for_owner( &state.owner, - candidate, - expected, - ), - None => conn.commit_renderer_agent_candidate_for_owner( - &state.owner, - candidate, - renderer_page, - ), - }) { - Ok(transaction) => { - let permit = navigation.issue_commit_permit(); - let failed_url = navigation.final_url().clone(); - let committed = match stable_page_target.as_ref() { + &token, + navigation.renderer_devtools_agent_token(), + ); + match candidate.and_then(|candidate| match stable_page_target.as_ref() { + Some(expected) => conn.commit_renderer_agent_candidate_for_existing_page( + &state.owner, + candidate, + expected, + ), + None => conn.commit_renderer_agent_candidate_for_owner( + &state.owner, + candidate, + renderer_page, + ), + }) { + Ok(transaction) => { + let permit = navigation.issue_commit_permit(); + let failed_url = navigation.final_url().clone(); + let committed = match stable_page_target.as_ref() { Some(expected) => conn.commit_existing_page_navigation_document( - &state.owner, expected, navigation, permit, + &state.owner, expected, navigation, permit, configuration, ).await.map(|navigation| network::materialize_loaded_navigation_progress( conn, &state, navigation, )), - None => navigation.commit(permit).await + None => navigation.commit(permit, configuration).await .map(|navigation| network::materialize_loaded_navigation_progress( conn, &state, navigation, )) .map_err(moli_core::page::RendererPageReplacementError::document_preserved), }; - match committed { - Ok(navigation) => { - commit_loaded_navigation_async( - conn, - out, - Some(&token), - state, - navigation, - Some(transaction), - command_context, - ) - .await; - } - Err(error) => { - if error.disposition() == moli_core::page::RendererPageReplacementFailureDisposition::DocumentUnavailable { + match committed { + Ok(navigation) => { + commit_loaded_navigation_async( + conn, + out, + Some(&token), + state, + navigation, + Some(transaction), + command_context, + ) + .await; + } + Err(error) => { + if error.disposition() == moli_core::page::RendererPageReplacementFailureDisposition::DocumentUnavailable { if let Some(expected) = stable_page_target.as_ref() && let Some(page) = conn.take_loaded_page_after_failed_navigation_for_owner( &state.owner, &token, Some(expected), &failed_url, @@ -3586,18 +3581,19 @@ async fn complete_materialized_navigation_into_buffer_inner_async( "failed to roll back renderer channel after prepared document commit failure" ); } - push_navigation_commit_error(out, &state, error); + push_navigation_commit_error(out, &state, error); + } } } - } - Err(error) => { - tracing::debug!( - %error, - session_id = state.owner.session_id(), - loader_id = token.loader_id, - "dropping superseded response commit-ready navigation" - ); - push_navigation_commit_error(out, &state, error); + Err(error) => { + tracing::debug!( + %error, + session_id = state.owner.session_id(), + loader_id = token.loader_id, + "dropping superseded response commit-ready navigation" + ); + push_navigation_commit_error(out, &state, error); + } } } } diff --git a/moli-renderer-v8/src/runtime/owner.rs b/moli-renderer-v8/src/runtime/owner.rs index 1292426916..5171a8f352 100644 --- a/moli-renderer-v8/src/runtime/owner.rs +++ b/moli-renderer-v8/src/runtime/owner.rs @@ -240,9 +240,11 @@ pub enum RendererOwnerCommand { }, CommitPreparedRendererDocument { permit: RendererDocumentCommitPermit, + configuration: Option>, }, CommitPreparedPageReplacement { permit: RendererDocumentCommitPermit, + configuration: Option>, }, CancelPreparedRendererDocument { token: RendererPageReservationToken, diff --git a/moli-renderer-v8/src/runtime/owner/owner_commands.rs b/moli-renderer-v8/src/runtime/owner/owner_commands.rs index 096628c6a0..7af4193415 100644 --- a/moli-renderer-v8/src/runtime/owner/owner_commands.rs +++ b/moli-renderer-v8/src/runtime/owner/owner_commands.rs @@ -569,7 +569,10 @@ impl RendererOwnerHandle { Err(error) => Err(error).into(), } } - RendererOwnerCommand::CommitPreparedRendererDocument { permit } => { + RendererOwnerCommand::CommitPreparedRendererDocument { + permit, + configuration, + } => { let token = permit.prepared_document(); if token.replacement.is_some() { owner_local_store.cancel_prepared_document(token); @@ -586,7 +589,14 @@ impl RendererOwnerHandle { )) .into(); } - match owner_local_store.take_prepared_document(token) { + let prepared = (|| { + if let Some(configuration) = configuration { + owner_local_store + .update_prepared_document_commit_configuration(token, *configuration)?; + } + owner_local_store.take_prepared_document(token) + })(); + match prepared { Ok(residence) => { self.create_page_reply_from_prepared_document_on_owner_local_store( token.page_id(), @@ -598,7 +608,10 @@ impl RendererOwnerHandle { Err(error) => Err(error).into(), } } - RendererOwnerCommand::CommitPreparedPageReplacement { permit } => { + RendererOwnerCommand::CommitPreparedPageReplacement { + permit, + configuration, + } => { let reservation = permit.prepared_document(); if reservation.local_host_id() != self.state.owner_local_host_id || reservation.replacement.is_none() @@ -608,7 +621,16 @@ impl RendererOwnerHandle { )) .into(); } - match owner_local_store.take_prepared_document(reservation) { + let prepared = (|| { + if let Some(configuration) = configuration { + owner_local_store.update_prepared_document_commit_configuration( + reservation, + *configuration, + )?; + } + owner_local_store.take_prepared_document(reservation) + })(); + match prepared { Ok(residence) => { Box::pin(self.commit_prepared_page_replacement(reservation, residence)) .await diff --git a/moli-renderer-v8/src/runtime/page.rs b/moli-renderer-v8/src/runtime/page.rs index f9c29adaf4..1f9fd0d859 100644 --- a/moli-renderer-v8/src/runtime/page.rs +++ b/moli-renderer-v8/src/runtime/page.rs @@ -755,6 +755,7 @@ impl PreparedRendererDocument { pub fn start_commit( mut self, permit: RendererDocumentCommitPermit, + configuration: Option, ) -> Result< impl std::future::Future< Output = Result<( @@ -777,6 +778,7 @@ impl PreparedRendererDocument { .renderer_owner .enqueue_command_with_reply(RendererOwnerCommand::CommitPreparedRendererDocument { permit, + configuration: configuration.map(Box::new), })?; Ok(async move { let reply = reply_rx @@ -801,7 +803,7 @@ impl PreparedRendererDocument { RendererPageCreationArtifacts, Option, )> { - self.start_commit(permit)?.await + self.start_commit(permit, None)?.await } /// Commits a response into the Page that issued this replacement reservation. @@ -809,6 +811,7 @@ impl PreparedRendererDocument { pub fn start_commit_page_replacement( self, permit: RendererDocumentCommitPermit, + configuration: Option, ) -> std::result::Result { use super::RendererPageReplacementError; @@ -829,6 +832,7 @@ impl PreparedRendererDocument { .renderer_owner .enqueue_command_with_reply(RendererOwnerCommand::CommitPreparedPageReplacement { permit, + configuration: configuration.map(Box::new), }) .map_err(RendererPageReplacementError::from_dispatch_error)?; Ok(PendingRendererPageReplacement { @@ -844,7 +848,9 @@ impl PreparedRendererDocument { super::RendererPageReplacementCommit, super::RendererPageReplacementError, > { - self.start_commit_page_replacement(permit)?.wait().await + self.start_commit_page_replacement(permit, None)? + .wait() + .await } pub async fn cancel(mut self) -> Result<()> { diff --git a/moli-renderer-v8/src/runtime/tests/extracted/navigation_streaming.rs b/moli-renderer-v8/src/runtime/tests/extracted/navigation_streaming.rs index 24298c899d..2bed04961d 100644 --- a/moli-renderer-v8/src/runtime/tests/extracted/navigation_streaming.rs +++ b/moli-renderer-v8/src/runtime/tests/extracted/navigation_streaming.rs @@ -818,68 +818,68 @@ globalThis.__preparedCommitObserved = JSON.stringify([ producer .await .expect("prepared body producer should finish"); - prepared - .update_commit_configuration(RendererPreparedDocumentCommitConfiguration { - web_mcp_invocation: None, - document_start_scripts: vec![ - crate::DocumentStartScript { - registry_key: None, - devtools_session: None, - source: r#"globalThis.__latestPreload = "ready";"#.to_owned(), - world_name: None, - has_bidi_channel_argument: false, - bidi_channel_handoffs: Vec::new(), - }, - crate::DocumentStartScript { - registry_key: None, - devtools_session: None, - source: r#"globalThis.__latestWorld = "ready";"#.to_owned(), - world_name: Some("latest-world".to_owned()), - has_bidi_channel_argument: false, - bidi_channel_handoffs: Vec::new(), - }, - ], - runtime_bindings: vec![crate::protocol_types::RuntimeBindingRegistration { + let configuration = RendererPreparedDocumentCommitConfiguration { + web_mcp_invocation: None, + document_start_scripts: vec![ + crate::DocumentStartScript { + registry_key: None, devtools_session: None, - name: "latestBinding".to_owned(), - execution_context_name: None, - }], - runtime_inspector_session_restore_snapshots: vec![ - RendererInspectorSessionRestoreSnapshot { - protocol_configuration: RendererInspectorProtocolConfiguration { - runtime_frontend_enabled: true, - ..Default::default() - }, + source: r#"globalThis.__latestPreload = "ready";"#.to_owned(), + world_name: None, + has_bidi_channel_argument: false, + bidi_channel_handoffs: Vec::new(), + }, + crate::DocumentStartScript { + registry_key: None, + devtools_session: None, + source: r#"globalThis.__latestWorld = "ready";"#.to_owned(), + world_name: Some("latest-world".to_owned()), + has_bidi_channel_argument: false, + bidi_channel_handoffs: Vec::new(), + }, + ], + runtime_bindings: vec![crate::protocol_types::RuntimeBindingRegistration { + devtools_session: None, + name: "latestBinding".to_owned(), + execution_context_name: None, + }], + runtime_inspector_session_restore_snapshots: vec![ + RendererInspectorSessionRestoreSnapshot { + protocol_configuration: RendererInspectorProtocolConfiguration { + runtime_frontend_enabled: true, ..Default::default() }, - ], - runtime_isolated_worlds: vec![crate::protocol_types::RuntimeIsolatedWorldDefinition { - name: "latest-world".to_owned(), - grant_universal_access: false, - }], - permission_overrides: vec![crate::protocol_types::PermissionOverrideRegistration { - permission: serde_json::Value::String("notifications".to_owned()), - setting: "granted".to_owned(), - origin: None, - embedded_origin: None, - }], - extra_http_headers: Default::default(), - document_settings: Default::default(), - browser_resource_runtime: loader.browser_resource_runtime(), - navigator_identity: loader.browser_identity().clone(), - network_offline: false, - bypass_service_worker: false, - cache_disabled: false, - blocked_url_patterns: Vec::new(), - fetch_subresource_interception_enabled: false, - fetch_subresource_interception_resource_type: None, - }) - .await - .expect("prepared document should accept the latest commit configuration"); + ..Default::default() + }, + ], + runtime_isolated_worlds: vec![crate::protocol_types::RuntimeIsolatedWorldDefinition { + name: "latest-world".to_owned(), + grant_universal_access: false, + }], + permission_overrides: vec![crate::protocol_types::PermissionOverrideRegistration { + permission: serde_json::Value::String("notifications".to_owned()), + setting: "granted".to_owned(), + origin: None, + embedded_origin: None, + }], + extra_http_headers: Default::default(), + document_settings: Default::default(), + browser_resource_runtime: loader.browser_resource_runtime(), + navigator_identity: loader.browser_identity().clone(), + network_offline: false, + bypass_service_worker: false, + cache_disabled: false, + blocked_url_patterns: Vec::new(), + fetch_subresource_interception_enabled: false, + fetch_subresource_interception_resource_type: None, + }; let permit = prepared.issue_commit_permit(); - let (mut page, _, diagnostics, _, pending_download) = - prepared.commit(permit).await.expect("permit should commit"); + let (mut page, _, diagnostics, _, pending_download) = prepared + .start_commit(permit, Some(configuration)) + .expect("configured commit should be admitted synchronously") + .await + .expect("permit should commit with the latest configuration"); assert!(pending_download.is_none()); assert!( diagnostics.renderer_output_predecessor.is_some(), @@ -2779,7 +2779,9 @@ async fn live_page_replacement_is_enqueued_before_the_owned_wait_is_polled() { .await; let (entered, release) = runtime.install_owner_command_dispatch_gate_for_testing(); let permit = prepared.issue_commit_permit(); - let pending = prepared.start_commit_page_replacement(permit).unwrap(); + let pending = prepared + .start_commit_page_replacement(permit, None) + .unwrap(); // No task has polled pending.wait(). A renderer-side signal proves the // commit was already enqueued by synchronous protocol admission. tokio::task::spawn_blocking(move || entered.recv_timeout(Duration::from_secs(5)))