diff --git a/moli-cdp-smoke/moli_cdp_smoke/groups/multi_context.py b/moli-cdp-smoke/moli_cdp_smoke/groups/multi_context.py index c31b57c4f..eca03ae09 100644 --- a/moli-cdp-smoke/moli_cdp_smoke/groups/multi_context.py +++ b/moli-cdp-smoke/moli_cdp_smoke/groups/multi_context.py @@ -5,6 +5,9 @@ import os import sys from typing import Any, Awaitable +from playwright.async_api import Error as PlaywrightError +from playwright.async_api import TimeoutError as PlaywrightTimeoutError + from ..assertions import SmokeError, assert_equal, record, wait_until from ..helpers import attach_cdp_event_collector @@ -1453,8 +1456,52 @@ async def _enable_response_stage_fetch(cdp: Any) -> None: async def _open_popup(page: Any, url: str) -> Any: - async with page.expect_popup(timeout=5_000) as popup_info: - await page.evaluate("(url) => window.open(url, '_blank')", url) + opened: bool | None = None + try: + async with page.expect_popup(timeout=5_000) as popup_info: + opened = await page.evaluate( + "(url) => Boolean(window.open(url, '_blank'))", url + ) + except PlaywrightTimeoutError as error: + observed = [] + for candidate in page.context.pages: + try: + opener = await candidate.opener() + observed.append( + { + "url": candidate.url, + "opener": opener.url if opener is not None else None, + "isExpectedOpener": opener is page, + } + ) + except PlaywrightError as diagnostic_error: + observed.append( + { + "url": candidate.url, + "diagnosticError": repr(diagnostic_error), + } + ) + target_infos: Any = None + browser_session: Any | None = None + try: + browser = page.context.browser + if browser is not None: + browser_session = await browser.new_browser_cdp_session() + target_infos = await browser_session.send("Target.getTargets") + except PlaywrightError as diagnostic_error: + target_infos = {"diagnosticError": repr(diagnostic_error)} + finally: + if browser_session is not None: + try: + await browser_session.detach() + except PlaywrightError: + pass + raise SmokeError( + f"popup event failed for {url}: {error}; " + f"windowOpenReturned={opened!r}; pages={observed!r}; " + f"targets={target_infos!r}" + ) from error + assert_equal(opened, True, "window.open returned a WindowProxy") popup = await popup_info.value await wait_until(lambda: popup.url == url, "popup URL") await popup.wait_for_load_state("load", timeout=10_000) diff --git a/moli-cdp-smoke/moli_cdp_smoke/groups/multi_page.py b/moli-cdp-smoke/moli_cdp_smoke/groups/multi_page.py index e1f31717b..9d7540672 100644 --- a/moli-cdp-smoke/moli_cdp_smoke/groups/multi_page.py +++ b/moli-cdp-smoke/moli_cdp_smoke/groups/multi_page.py @@ -6,6 +6,9 @@ from contextlib import suppress from pathlib import Path from typing import Any +from playwright.async_api import Error as PlaywrightError +from playwright.async_api import TimeoutError as PlaywrightTimeoutError + from ..assertions import SmokeError, assert_equal, record, wait_until from ..helpers import attach_cdp_event_collector, run_worker_command from .multi_page_contracts import run_multi_page_contracts @@ -1026,11 +1029,37 @@ async def _context_teardown_with_pending_commands( async def _open_popup(page: Any, url: str, name: str) -> Any: - async with page.expect_popup(timeout=5_000) as popup_info: - opened = await page.evaluate( - "({url, name}) => Boolean(window.open(url, name))", - {"url": url, "name": name}, - ) + opened: bool | None = None + try: + async with page.expect_popup(timeout=5_000) as popup_info: + opened = await page.evaluate( + "({url, name}) => Boolean(window.open(url, name))", + {"url": url, "name": name}, + ) + except PlaywrightTimeoutError as error: + pages = page.context.pages + observed = [] + for candidate in pages: + try: + opener = await candidate.opener() + observed.append( + { + "url": candidate.url, + "opener": opener.url if opener is not None else None, + "isExpectedOpener": opener is page, + } + ) + except PlaywrightError as diagnostic_error: + observed.append( + { + "url": candidate.url, + "diagnosticError": repr(diagnostic_error), + } + ) + raise SmokeError( + f"popup event for {name} failed: {error}; " + f"windowOpenReturned={opened!r}; pages={observed!r}" + ) from error assert_equal(opened, True, f"window.open returned a WindowProxy for {name}") popup = await popup_info.value await popup.wait_for_load_state("load", timeout=10_000) diff --git a/moli-cdp-smoke/moli_cdp_smoke/runner.py b/moli-cdp-smoke/moli_cdp_smoke/runner.py index 596d41cc0..ce29909d6 100644 --- a/moli-cdp-smoke/moli_cdp_smoke/runner.py +++ b/moli-cdp-smoke/moli_cdp_smoke/runner.py @@ -684,6 +684,8 @@ async def async_main(argv: list[str] | None = None) -> int: } if failures: payload["error"] = "\n".join(failures) + if serve is not None and serve.logs: + payload["moliLogTail"] = serve.logs[-5_000:] _emit_worker_payload(payload, args.result, failed=not ok) return 0 if ok else 1 diff --git a/moli-cdp-smoke/moli_cdp_smoke/serve.py b/moli-cdp-smoke/moli_cdp_smoke/serve.py index cc8fa3ad4..d744a7c12 100644 --- a/moli-cdp-smoke/moli_cdp_smoke/serve.py +++ b/moli-cdp-smoke/moli_cdp_smoke/serve.py @@ -91,7 +91,7 @@ async def start_moli_serve( str(port), "--resource", "--log-level", - "info", + os.environ.get("MOLI_SMOKE_LOG_LEVEL", "info"), ] if layout: command.append("--layout") diff --git a/moli-core/src/page/command_dispatch.rs b/moli-core/src/page/command_dispatch.rs index 245cd57df..f29102a98 100644 --- a/moli-core/src/page/command_dispatch.rs +++ b/moli-core/src/page/command_dispatch.rs @@ -12,6 +12,12 @@ pub struct PendingPageCommand { renderer_agent_attachment_id: Option, } +impl PendingPageCommand { + pub fn renderer_agent_attachment_id(&self) -> Option { + self.renderer_agent_attachment_id + } +} + pub struct PendingRuntimeInspectorCommandDispatch { kind: PendingRuntimeInspectorCommandDispatchKind, } diff --git a/moli-core/src/page/mod.rs b/moli-core/src/page/mod.rs index f11943d51..dc1e7227d 100644 --- a/moli-core/src/page/mod.rs +++ b/moli-core/src/page/mod.rs @@ -188,6 +188,11 @@ pub use crate::renderer::{ pub struct Page { page_state: PageStateCache, + // Chromium owns this on RenderFrameHostImpl::IdleManager. Keep it on the + // current document handle instead of deriving browser-side navigation + // policy from whichever renderer snapshot the protocol actor consumed + // most recently. + idle_override: Option, handle: RendererPageHandle, renderer_agent_attachment_id: Option, renderer_devtools_command_session_id: Option, @@ -211,8 +216,10 @@ impl Page { handle: RendererPageHandle, page_state: Arc, ) -> Self { + let idle_override = page_state.idle_override(); Self { page_state: PageStateCache::new(page_state), + idle_override, handle, renderer_agent_attachment_id: None, renderer_devtools_command_session_id: None, @@ -225,8 +232,10 @@ impl Page { page_state: Arc, page_creation_artifacts: RendererPageCreationArtifacts, ) -> Self { + let idle_override = page_state.idle_override(); Self { page_state: PageStateCache::new(page_state), + idle_override, handle, renderer_agent_attachment_id: None, renderer_devtools_command_session_id: None, diff --git a/moli-core/src/page/renderer_command_support.rs b/moli-core/src/page/renderer_command_support.rs index 15824cd36..e2057f303 100644 --- a/moli-core/src/page/renderer_command_support.rs +++ b/moli-core/src/page/renderer_command_support.rs @@ -175,7 +175,7 @@ impl Page { } pub fn idle_override(&self) -> Option { - self.page_state.state().idle_override() + self.idle_override } pub fn status(&self) -> u16 { diff --git a/moli-core/src/page/settings_support.rs b/moli-core/src/page/settings_support.rs index 512693560..9d26592bf 100644 --- a/moli-core/src/page/settings_support.rs +++ b/moli-core/src/page/settings_support.rs @@ -68,7 +68,10 @@ impl Page { resource_runtime: &BrowserResourceRuntime, ) -> Result<()> { self.dispatch_unit_page_command_async( - RendererPageCommand::ReplaceBrowserResourceRuntime(resource_runtime.clone()), + RendererPageCommand::ReplaceBrowserResourceRuntime { + resource_runtime: resource_runtime.clone(), + navigator_identity: resource_runtime.browser_identity().clone(), + }, "replace browser resource runtime", ) .await @@ -78,9 +81,21 @@ impl Page { &self, resource_runtime: &BrowserResourceRuntime, ) -> Result { - self.start_page_command(RendererPageCommand::ReplaceBrowserResourceRuntime( - resource_runtime.clone(), - )) + self.start_replace_browser_resource_runtime_with_navigator_identity( + resource_runtime, + resource_runtime.browser_identity().clone(), + ) + } + + pub fn start_replace_browser_resource_runtime_with_navigator_identity( + &self, + resource_runtime: &BrowserResourceRuntime, + navigator_identity: moli_browser_profile::BrowserIdentityProfile, + ) -> Result { + self.start_page_command(RendererPageCommand::ReplaceBrowserResourceRuntime { + resource_runtime: resource_runtime.clone(), + navigator_identity, + }) } pub fn finish_replace_browser_resource_runtime( @@ -204,10 +219,17 @@ impl Page { } pub fn start_set_idle_override( - &self, + &mut self, idle_override: Option, ) -> Result { - self.start_page_command(RendererPageCommand::SetIdleOverride(idle_override)) + let pending = + self.start_page_command(RendererPageCommand::SetIdleOverride(idle_override))?; + // SetIdleOverride is synchronous browser-side state in Chromium. Make + // it visible at command admission so a navigation from another CDP + // session cannot observe an older protocol snapshot after the renderer + // has already accepted the command. + self.idle_override = idle_override; + Ok(pending) } pub fn finish_set_idle_override(&mut self, completion: CompletedPageCommand) -> Result<()> { diff --git a/moli-core/src/runtime/navigation_engine.rs b/moli-core/src/runtime/navigation_engine.rs index 88e8743cf..d1820fff3 100644 --- a/moli-core/src/runtime/navigation_engine.rs +++ b/moli-core/src/runtime/navigation_engine.rs @@ -210,7 +210,11 @@ pub struct PreparedDocumentPageCommitConfiguration { pub emulated_media: EmulatedMediaOverrides, pub idle_override: Option, pub viewport_surface: Option, + pub browser_resource_runtime: BrowserResourceRuntime, + pub navigator_identity: moli_browser_profile::BrowserIdentityProfile, pub network_offline: bool, + pub bypass_service_worker: bool, + pub cache_disabled: bool, pub blocked_url_patterns: Vec, pub fetch_subresource_interception: (bool, Option), } @@ -268,7 +272,11 @@ impl PreparedDocumentPage { emulated_media: configuration.emulated_media, idle_override: configuration.idle_override, viewport_surface: configuration.viewport_surface, + 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 diff --git a/moli-protocol-server/src/cdp_scheduler.rs b/moli-protocol-server/src/cdp_scheduler.rs index 1895e583b..7bdccc1ce 100644 --- a/moli-protocol-server/src/cdp_scheduler.rs +++ b/moli-protocol-server/src/cdp_scheduler.rs @@ -44,9 +44,7 @@ pub(crate) use actor::spawn_cdp_scheduler_actor; pub(crate) use adapter_scheduler::{ ProtocolAdapterScheduler, ProtocolAdapterSchedulerAdvance, ProtocolAdapterSchedulerInput, }; -pub(crate) use command_dispatch::{ - CommandDispatchState, CommandDispatchStepOutput, CommandTurnOutput, -}; +pub(crate) use command_dispatch::{CommandDispatchState, CommandTurnOutput}; pub(crate) use frontend_control::{CdpCookieSnapshot, CdpOwnerActorLifecycle}; use protocol_residence::{ ClientTurnPredecessor, ProtocolSchedulerResidence, ProtocolSchedulerStep, SchedulerQueues, @@ -2422,12 +2420,20 @@ impl CdpScheduler { .await } - pub(crate) async fn ingest_renderer_publication_after_load( + pub(crate) async fn ingest_renderer_publication_after_loads( &mut self, publication: RendererOutputTransportMessage, - observation_id: DeferredMainDocumentLoadObservationId, + observation_ids: Vec, ) -> ProtocolOutputSequence { - self.ingest_renderer_publication(publication, vec![observation_id], None) + let future_load_predecessor = observation_ids + .is_empty() + .then(|| { + DeferredMainDocumentLoadPredecessorCandidate::from_renderer_publication( + &publication, + ) + }) + .flatten(); + self.ingest_renderer_publication(publication, observation_ids, future_load_predecessor) .await } diff --git a/moli-protocol-server/src/cdp_scheduler/actor.rs b/moli-protocol-server/src/cdp_scheduler/actor.rs index e99d34a2e..efc067813 100644 --- a/moli-protocol-server/src/cdp_scheduler/actor.rs +++ b/moli-protocol-server/src/cdp_scheduler/actor.rs @@ -24,10 +24,9 @@ use super::frontend_control::CdpFrontendControlState; use super::{ CdpBackgroundEventReceiver, CdpBackgroundNavigationCompletionReceiver, CdpCookieSnapshot, CdpOwnerActorLifecycle, CdpRendererPublicationReceiver, CdpScheduler, - CdpSchedulerEventReceivers, CommandDispatchState, CommandDispatchStepOutput, - CommandOutputReleasePermit, CommandStartAction, CommandTaskStep, CommandTurnOutput, - ProtocolAdapterScheduler, ProtocolAdapterSchedulerAdvance, ProtocolAdapterSchedulerInput, - ProtocolOutputSequence, + CdpSchedulerEventReceivers, CommandDispatchState, CommandOutputReleasePermit, + CommandStartAction, CommandTaskStep, CommandTurnOutput, ProtocolAdapterScheduler, + ProtocolAdapterSchedulerAdvance, ProtocolAdapterSchedulerInput, ProtocolOutputSequence, }; struct PendingRuntimeDeferredReplyState { @@ -157,7 +156,7 @@ impl SchedulerInputReceivers { &mut self, deferred_runtime_response_rx: &mut mpsc::UnboundedReceiver, has_pending_runtime_deferred_reply: bool, - adapter_scheduler: &mut ProtocolAdapterScheduler, + adapter_scheduler: &mut ProtocolAdapterScheduler, page_javascript_blocked: bool, ) -> Option { if let Some(input) = self @@ -242,7 +241,7 @@ async fn run_cdp_scheduler_actor( scheduler .conn .set_runtime_inspector_response_ready_sender(deferred_runtime_response_tx.clone()); - let mut adapter_scheduler = ProtocolAdapterScheduler::::default(); + let mut adapter_scheduler = ProtocolAdapterScheduler::default(); let mut pending_runtime_deferred_replies: VecDeque = VecDeque::new(); let (pending_command_completion_tx, mut pending_command_completion_rx) = @@ -314,11 +313,7 @@ async fn run_cdp_scheduler_actor( generation, visual_state, } = frame; - let output = route_top_level_background_event( - &mut adapter_scheduler, - &mut scheduler, - event, - ); + let output = scheduler.route_background_event_around_inflight_navigation(event); if !flush_protocol_output_with_runtime_deferred_reply_routing( &frontend_router, &mut scheduler, @@ -430,7 +425,7 @@ async fn handle_scheduler_input( scheduler_input_rx: &mut SchedulerInputReceivers, pending_runtime_deferred_replies: &mut VecDeque, deferred_runtime_response_tx: &mpsc::UnboundedSender, - adapter_scheduler: &mut ProtocolAdapterScheduler, + adapter_scheduler: &mut ProtocolAdapterScheduler, pending_command_completion_tx: &mpsc::UnboundedSender, in_flight_commands: &mut InFlightCommands, blocked_commands: &mut VecDeque, @@ -475,7 +470,7 @@ async fn handle_scheduler_input( .await } SchedulerInput::BackgroundEvent(event) => { - let output = route_top_level_background_event(adapter_scheduler, scheduler, event); + let output = scheduler.route_background_event_around_inflight_navigation(event); flush_protocol_output_with_runtime_deferred_reply_routing( frontend_router, scheduler, @@ -545,7 +540,7 @@ async fn flush_renderer_publication_predecessor( scheduler: &mut CdpScheduler, scheduler_input_rx: &mut SchedulerInputReceivers, pending_runtime_deferred_replies: &mut VecDeque, - adapter_scheduler: &mut ProtocolAdapterScheduler, + adapter_scheduler: &mut ProtocolAdapterScheduler, predecessor: Option<&RendererOutputFence>, ) -> bool { let Some(predecessor) = predecessor else { @@ -577,7 +572,7 @@ async fn ingest_and_flush_renderer_publication( frontend_router: &CdpFrontendRouter, scheduler: &mut CdpScheduler, pending_runtime_deferred_replies: &mut VecDeque, - adapter_scheduler: &mut ProtocolAdapterScheduler, + adapter_scheduler: &mut ProtocolAdapterScheduler, publication: RendererOutputTransportMessage, ) -> bool { let output = adapter_scheduler @@ -597,7 +592,7 @@ async fn flush_background_completion_input( scheduler: &mut CdpScheduler, scheduler_input_rx: &mut SchedulerInputReceivers, pending_runtime_deferred_replies: &mut VecDeque, - adapter_scheduler: &mut ProtocolAdapterScheduler, + adapter_scheduler: &mut ProtocolAdapterScheduler, input: SchedulerInput, ) -> bool { let (prefix_output, mut completion_output, renderer_output_predecessor) = match input { @@ -1143,14 +1138,12 @@ fn runtime_deferred_reply_unexpected_pending_output( async fn handle_adapter_scheduler_input( frontend_router: &CdpFrontendRouter, scheduler: &mut CdpScheduler, - adapter_scheduler: &mut ProtocolAdapterScheduler, + adapter_scheduler: &mut ProtocolAdapterScheduler, pending_runtime_deferred_replies: &mut VecDeque, input: ProtocolAdapterSchedulerInput, ) -> bool { let trace_started = moli_trace::cdp_runtime_trace_enabled().then(Instant::now); - let advance = adapter_scheduler - .advance_input(scheduler, input, CommandDispatchState::pending_command) - .await; + let advance = adapter_scheduler.advance_input(scheduler, input).await; let (kind, observation_id, ok) = match advance { ProtocolAdapterSchedulerAdvance::Idle => ("idle", None, true), ProtocolAdapterSchedulerAdvance::ClientTurnYielded => ("client_turn_yielded", None, true), @@ -1169,10 +1162,8 @@ async fn handle_adapter_scheduler_input( } ProtocolAdapterSchedulerAdvance::DeferredLoadCompleted { observation_id, - attachment, output, } => { - let output = attachment.complete_protocol_output(output); let ok = flush_protocol_output_with_runtime_deferred_reply_routing( frontend_router, scheduler, @@ -1203,27 +1194,6 @@ async fn handle_adapter_scheduler_input( ok } -fn route_top_level_background_event( - adapter_scheduler: &mut ProtocolAdapterScheduler, - scheduler: &mut CdpScheduler, - event: BackgroundProtocolEvent, -) -> ProtocolOutputSequence { - let output = scheduler.route_background_event_around_inflight_navigation(event); - if output.is_empty() { - return output; - } - let mut events = output.into_background_events(); - let Some(event) = events.pop() else { - return ProtocolOutputSequence::empty(); - }; - let Some(dispatch) = adapter_scheduler.pending_load_attachment_mut() else { - return ProtocolOutputSequence::from_background_event(event); - }; - match dispatch.route_pending_background_event(event) { - CommandDispatchStepOutput::Emit(output) => output, - } -} - fn enqueue_pending_runtime_deferred_reply_state( pending_runtime_deferred_replies: &mut VecDeque, mut pending: PendingRuntimeDeferredReplyState, @@ -1392,7 +1362,7 @@ async fn handle_frontend_command( scheduler_input_rx: &mut SchedulerInputReceivers, pending_runtime_deferred_replies: &mut VecDeque, deferred_runtime_response_tx: &mpsc::UnboundedSender, - adapter_scheduler: &mut ProtocolAdapterScheduler, + adapter_scheduler: &mut ProtocolAdapterScheduler, pending_command_completion_tx: &mpsc::UnboundedSender, in_flight_commands: &mut InFlightCommands, blocked_commands: &mut VecDeque, @@ -1436,7 +1406,7 @@ async fn handle_client_command_with_interleaved_output( command: ParsedCdpCommand, pending_runtime_deferred_replies: &mut VecDeque, deferred_runtime_response_tx: &mpsc::UnboundedSender, - adapter_scheduler: &mut ProtocolAdapterScheduler, + adapter_scheduler: &mut ProtocolAdapterScheduler, pending_command_completion_tx: &mpsc::UnboundedSender, in_flight_commands: &mut InFlightCommands, blocked_commands: &mut VecDeque, @@ -1519,7 +1489,7 @@ async fn start_ready_command_dispatch( mut command_context: CommandDispatchContext, pending_runtime_deferred_replies: &mut VecDeque, deferred_runtime_response_tx: &mpsc::UnboundedSender, - adapter_scheduler: &mut ProtocolAdapterScheduler, + adapter_scheduler: &mut ProtocolAdapterScheduler, pending_command_completion_tx: &mpsc::UnboundedSender, in_flight_commands: &mut InFlightCommands, next_in_flight_command_token: &mut u64, @@ -1615,7 +1585,7 @@ async fn drain_blocked_commands_after_navigation_gate( scheduler_input_rx: &mut SchedulerInputReceivers, pending_runtime_deferred_replies: &mut VecDeque, deferred_runtime_response_tx: &mpsc::UnboundedSender, - adapter_scheduler: &mut ProtocolAdapterScheduler, + adapter_scheduler: &mut ProtocolAdapterScheduler, pending_command_completion_tx: &mpsc::UnboundedSender, in_flight_commands: &mut InFlightCommands, blocked_commands: &mut VecDeque, @@ -1683,7 +1653,7 @@ async fn flush_completed_command_output( scheduler: &mut CdpScheduler, scheduler_input_rx: &mut SchedulerInputReceivers, pending_runtime_deferred_replies: &mut VecDeque, - adapter_scheduler: &mut ProtocolAdapterScheduler, + adapter_scheduler: &mut ProtocolAdapterScheduler, metadata: &InFlightCommandMetadata, mut output: CommandTurnOutput, ) -> bool { @@ -1850,7 +1820,7 @@ async fn handle_pending_command_completion( scheduler_input_rx: &mut SchedulerInputReceivers, pending_runtime_deferred_replies: &mut VecDeque, deferred_runtime_response_tx: &mpsc::UnboundedSender, - adapter_scheduler: &mut ProtocolAdapterScheduler, + adapter_scheduler: &mut ProtocolAdapterScheduler, pending_command_completion_tx: &mpsc::UnboundedSender, in_flight_commands: &mut InFlightCommands, completion: PendingCommandCompletion, diff --git a/moli-protocol-server/src/cdp_scheduler/adapter_scheduler.rs b/moli-protocol-server/src/cdp_scheduler/adapter_scheduler.rs index 39174758c..d34b7a2f3 100644 --- a/moli-protocol-server/src/cdp_scheduler/adapter_scheduler.rs +++ b/moli-protocol-server/src/cdp_scheduler/adapter_scheduler.rs @@ -15,26 +15,24 @@ use super::{CdpScheduler, ProtocolOutputSequence, protocol_residence::ProtocolSc /// work in a later client turn: /// /// - one coalesced self-turn signal; -/// - at most one exact main-document load observation; -/// - an adapter-specific attachment that remains inseparable from that exact -/// observation until its terminal is consumed. +/// - every exact main-document load observation currently awaiting its +/// target-local terminal. /// /// It never stores a renderer publication, Page task capability or protocol -/// transport route. Switching a Classic connection to BiDi therefore keeps -/// this value alive instead of recreating scheduler or load-observer state. -pub(crate) struct ProtocolAdapterScheduler { +/// transport route. Independent Pages may therefore wait for load in parallel, +/// and switching a Classic connection to BiDi keeps the observations alive. +pub(crate) struct ProtocolAdapterScheduler { turn_tx: mpsc::UnboundedSender<()>, turn_rx: mpsc::UnboundedReceiver<()>, turn_scheduled: bool, load_completion_tx: mpsc::UnboundedSender, load_completion_rx: mpsc::UnboundedReceiver, - pending_load: Option>, + pending_loads: Vec, } -struct PendingAdapterLoadObservation { +struct PendingAdapterLoadObservation { observation_id: DeferredMainDocumentLoadObservationId, output_interest: DeferredMainDocumentLoadCompletionOutputInterest, - attachment: A, } pub(crate) enum ProtocolAdapterSchedulerInput { @@ -45,9 +43,9 @@ pub(crate) enum ProtocolAdapterSchedulerInput { /// Result of consuming one shared adapter-scheduler input. /// /// `DeferredLoadStarted` deliberately does not expose the pending observer or -/// its wake interest. The exact identity and adapter attachment remain owned -/// by `ProtocolAdapterScheduler` until `DeferredLoadCompleted`. -pub(crate) enum ProtocolAdapterSchedulerAdvance { +/// its wake interest. The exact identity remains owned by +/// `ProtocolAdapterScheduler` until `DeferredLoadCompleted`. +pub(crate) enum ProtocolAdapterSchedulerAdvance { Idle, ClientTurnYielded, DeferredLoadStarted { @@ -56,7 +54,6 @@ pub(crate) enum ProtocolAdapterSchedulerAdvance { ProtocolResidenceCompleted(ProtocolOutputSequence), DeferredLoadCompleted { observation_id: DeferredMainDocumentLoadObservationId, - attachment: A, output: ProtocolOutputSequence, }, StaleDeferredLoadCompletion { @@ -64,7 +61,7 @@ pub(crate) enum ProtocolAdapterSchedulerAdvance { }, } -impl Default for ProtocolAdapterScheduler { +impl Default for ProtocolAdapterScheduler { fn default() -> Self { let (turn_tx, turn_rx) = mpsc::unbounded_channel(); let (load_completion_tx, load_completion_rx) = mpsc::unbounded_channel(); @@ -74,20 +71,14 @@ impl Default for ProtocolAdapterScheduler { turn_scheduled: false, load_completion_tx, load_completion_rx, - pending_load: None, + pending_loads: Vec::new(), } } } -impl ProtocolAdapterScheduler { - pub(crate) fn has_pending_load(&self) -> bool { - self.pending_load.is_some() - } - - pub(crate) fn pending_load_attachment_mut(&mut self) -> Option<&mut A> { - self.pending_load - .as_mut() - .map(|pending| &mut pending.attachment) +impl ProtocolAdapterScheduler { + pub(crate) fn has_pending_loads(&self) -> bool { + !self.pending_loads.is_empty() } /// Coalesces scheduler readiness into one later adapter turn. @@ -128,7 +119,7 @@ impl ProtocolAdapterScheduler { pub(crate) async fn recv_input(&mut self) -> ProtocolAdapterSchedulerInput { tokio::select! { biased; - completion = self.load_completion_rx.recv(), if self.has_pending_load() => { + completion = self.load_completion_rx.recv(), if self.has_pending_loads() => { ProtocolAdapterSchedulerInput::DeferredLoadCompletion(Box::new( completion.expect("shared adapter load-completion channel must remain open"), )) @@ -145,51 +136,46 @@ impl ProtocolAdapterScheduler { } } - /// Ingests one concrete renderer publication using the exact load - /// observation, if any, currently owned by this connection-local driver. + /// Ingests one concrete renderer publication behind every matching exact + /// load observation currently owned by this connection-local driver. pub(crate) async fn ingest_renderer_publication( &mut self, scheduler: &mut CdpScheduler, publication: RendererOutputTransportMessage, ) -> ProtocolOutputSequence { - let Some(pending) = self.pending_load.as_ref() else { + if self.pending_loads.is_empty() { return scheduler .ingest_renderer_publication_for_scheduler(publication) .await; - }; - let observation_id = pending.observation_id; - match scheduler.route_renderer_output_for_deferred_load_completion( - &publication, - &pending.output_interest, - ) { - DeferredMainDocumentLoadCompletionOutputAction::ProcessNow => { - scheduler.ingest_renderer_publication_now(publication).await - } - DeferredMainDocumentLoadCompletionOutputAction::Queue => { - scheduler - .ingest_renderer_publication_after_load(publication, observation_id) - .await - } } + let observation_ids = self + .pending_loads + .iter() + .filter_map(|pending| { + (scheduler.route_renderer_output_for_deferred_load_completion( + &publication, + &pending.output_interest, + ) == DeferredMainDocumentLoadCompletionOutputAction::Queue) + .then_some(pending.observation_id) + }) + .collect(); + scheduler + .ingest_renderer_publication_after_loads(publication, observation_ids) + .await } /// Consumes one input and advances at most one concrete scheduler /// residence. /// - /// `make_load_attachment` is called only when this turn starts a new exact - /// load observation. Keeping that attachment inside the shared driver - /// prevents adapters from maintaining a parallel observation id or - /// generation solely to associate output-routing state with the terminal. pub(crate) async fn advance_input( &mut self, scheduler: &mut CdpScheduler, input: ProtocolAdapterSchedulerInput, - make_load_attachment: impl FnOnce() -> A, - ) -> ProtocolAdapterSchedulerAdvance { + ) -> ProtocolAdapterSchedulerAdvance { match input { ProtocolAdapterSchedulerInput::Turn => { self.turn_scheduled = false; - self.advance_turn(scheduler, make_load_attachment).await + self.advance_turn(scheduler).await } ProtocolAdapterSchedulerInput::DeferredLoadCompletion(completion) => { self.complete_load(scheduler, *completion).await @@ -200,8 +186,7 @@ impl ProtocolAdapterScheduler { async fn advance_turn( &mut self, scheduler: &mut CdpScheduler, - make_load_attachment: impl FnOnce() -> A, - ) -> ProtocolAdapterSchedulerAdvance { + ) -> ProtocolAdapterSchedulerAdvance { match self.next_scheduler_step(scheduler) { ProtocolSchedulerStep::SatisfyClientTurnPredecessor => { scheduler.satisfy_front_protocol_residence_client_turn_predecessor(); @@ -210,19 +195,21 @@ impl ProtocolAdapterScheduler { ProtocolSchedulerStep::CompleteReadyResidence if scheduler.next_ready_protocol_residence_is_main_document_load_action() => { - assert!( - self.pending_load.is_none(), - "one adapter scheduler cannot start a second load observation" - ); let pending = scheduler .start_next_deferred_load_completion() .expect("ready load residence must produce an exact pending observation"); let observation_id = pending.observation_id(); let output_interest = pending.output_interest(); - self.pending_load = Some(PendingAdapterLoadObservation { + assert!( + !self + .pending_loads + .iter() + .any(|pending| pending.observation_id == observation_id), + "one exact load observation cannot be started twice" + ); + self.pending_loads.push(PendingAdapterLoadObservation { observation_id, output_interest, - attachment: make_load_attachment(), }); self.spawn_load_wait(pending); ProtocolAdapterSchedulerAdvance::DeferredLoadStarted { observation_id } @@ -237,33 +224,10 @@ impl ProtocolAdapterScheduler { } /// Returns the next concrete-residence transition this adapter may drive. - /// - /// An in-flight exact load observation reserves this driver's one load - /// attachment slot; it does not precede unrelated scheduler work. - /// `CdpScheduler` already makes exact `load_predecessors` authoritative. - /// Keeping no-predecessor owner actions runnable is required because a - /// replacement or termination action may itself settle the observation. + /// Exact `load_predecessors` in `CdpScheduler` provide the target-local + /// ordering; the adapter must not add a connection-wide capacity gate. fn next_scheduler_step(&self, scheduler: &CdpScheduler) -> ProtocolSchedulerStep { - let step = scheduler.next_protocol_scheduler_step(); - self.enforce_load_attachment_capacity( - step, - scheduler.next_ready_protocol_residence_is_main_document_load_action(), - ) - } - - fn enforce_load_attachment_capacity( - &self, - step: ProtocolSchedulerStep, - next_ready_residence_is_main_document_load_action: bool, - ) -> ProtocolSchedulerStep { - if self.has_pending_load() - && step == ProtocolSchedulerStep::CompleteReadyResidence - && next_ready_residence_is_main_document_load_action - { - ProtocolSchedulerStep::Wait - } else { - step - } + scheduler.next_protocol_scheduler_step() } fn spawn_load_wait(&self, pending: PendingDeferredMainDocumentLoadCompletion) { @@ -285,40 +249,32 @@ impl ProtocolAdapterScheduler { &mut self, scheduler: &mut CdpScheduler, completion: CompletedDeferredMainDocumentLoadCompletion, - ) -> ProtocolAdapterSchedulerAdvance { + ) -> ProtocolAdapterSchedulerAdvance { let observation_id = completion.observation_id(); - let Ok(pending) = self.take_pending_load(observation_id) else { + if self.take_pending_load(observation_id).is_err() { return ProtocolAdapterSchedulerAdvance::StaleDeferredLoadCompletion { observation_id }; - }; + } let output = scheduler .complete_deferred_load_completion(completion) .await; ProtocolAdapterSchedulerAdvance::DeferredLoadCompleted { observation_id, - attachment: pending.attachment, output, } } - /// Claims the attachment only for the exact observation that produced a - /// terminal. - /// - /// A delayed terminal from an already-retired observation must not detach - /// the current adapter mode or command-routing state. Restoring the - /// nonmatching value here keeps that invariant independent of how each - /// adapter reacts to `StaleDeferredLoadCompletion`. + /// Claims only the exact observation that produced a terminal. A delayed + /// or duplicate terminal must not retire another Page's load wait. fn take_pending_load( &mut self, observation_id: DeferredMainDocumentLoadObservationId, - ) -> Result, ()> { - let Some(pending) = self.pending_load.take() else { - return Err(()); - }; - if pending.observation_id != observation_id { - self.pending_load = Some(pending); - return Err(()); - } - Ok(pending) + ) -> Result { + let index = self + .pending_loads + .iter() + .position(|pending| pending.observation_id == observation_id) + .ok_or(())?; + Ok(self.pending_loads.remove(index)) } } @@ -363,15 +319,14 @@ mod tests { scheduler.apply_scheduler_events(vec![CdpSchedulerEvent::ProtocolWorkPublished { work: protocol_observation(1), }]); - let adapter = ProtocolAdapterScheduler::<()> { - pending_load: Some(PendingAdapterLoadObservation { + let adapter = ProtocolAdapterScheduler { + pending_loads: vec![PendingAdapterLoadObservation { observation_id, output_interest: deferred_main_document_load_output_interest( page_residence(), None, ), - attachment: (), - }), + }], ..Default::default() }; @@ -387,47 +342,65 @@ mod tests { "independent protocol work must remain runnable while the exact observation waits" ); assert!( - adapter.has_pending_load(), - "running independent work must not release the exact load attachment" + adapter.has_pending_loads(), + "running independent work must not release the exact load observation" ); } #[test] - fn pending_exact_load_observation_blocks_second_load_attachment() { - let observation_id = deferred_main_document_load_observation_id(1); - let adapter = ProtocolAdapterScheduler::<()> { - pending_load: Some(PendingAdapterLoadObservation { - observation_id, - output_interest: deferred_main_document_load_output_interest( - page_residence(), - None, - ), - attachment: (), - }), + fn multiple_exact_load_observations_retire_independently() { + let first = deferred_main_document_load_observation_id(1); + let second = deferred_main_document_load_observation_id(2); + let stale = deferred_main_document_load_observation_id(3); + let mut adapter = ProtocolAdapterScheduler { + pending_loads: vec![ + PendingAdapterLoadObservation { + observation_id: first, + output_interest: deferred_main_document_load_output_interest( + page_residence(), + None, + ), + }, + PendingAdapterLoadObservation { + observation_id: second, + output_interest: deferred_main_document_load_output_interest( + RendererOutputResidenceIdentity::Page { + owner_local_host_id: RendererOwnerLocalHostId::new_for_testing(1), + page_id: PageId::new_for_testing(8), + }, + None, + ), + }, + ], ..Default::default() }; + assert_eq!(adapter.pending_loads.len(), 2); + adapter + .take_pending_load(second) + .expect("the second Page terminal must claim only its observation"); assert_eq!( - adapter.enforce_load_attachment_capacity( - ProtocolSchedulerStep::CompleteReadyResidence, - true, - ), - ProtocolSchedulerStep::Wait, - "one adapter cannot start a second exact load observation" + adapter + .pending_loads + .iter() + .map(|pending| pending.observation_id) + .collect::>(), + [first], + "an out-of-order terminal must preserve the first Page wait" ); - assert_eq!( - adapter.enforce_load_attachment_capacity( - ProtocolSchedulerStep::CompleteReadyResidence, - false, - ), - ProtocolSchedulerStep::CompleteReadyResidence, - "load attachment capacity must not become a global execution lock" + assert!( + adapter.take_pending_load(stale).is_err(), + "an unknown terminal must not retire another Page wait" ); + adapter + .take_pending_load(first) + .expect("the first Page terminal should remain claimable"); + assert!(!adapter.has_pending_loads()); } #[tokio::test] async fn idle_adapter_input_remains_pending() { - let mut adapter = ProtocolAdapterScheduler::<()>::default(); + let mut adapter = ProtocolAdapterScheduler::default(); tokio::select! { biased; _ = adapter.recv_input() => { @@ -445,18 +418,14 @@ mod tests { scheduler.apply_scheduler_events(vec![CdpSchedulerEvent::ProtocolWorkPublished { work: protocol_observation(1), }]); - let mut adapter = ProtocolAdapterScheduler::<()>::default(); + let mut adapter = ProtocolAdapterScheduler::default(); adapter.schedule_turn_if_needed(&scheduler, false); adapter.schedule_turn_if_needed(&scheduler, false); let first = adapter.recv_input().await; assert!(matches!(first, ProtocolAdapterSchedulerInput::Turn)); assert!(matches!( - adapter - .advance_input(&mut scheduler, first, || { - panic!("ordinary protocol work cannot create a load attachment") - }) - .await, + adapter.advance_input(&mut scheduler, first).await, ProtocolAdapterSchedulerAdvance::ClientTurnYielded )); assert!( @@ -468,66 +437,10 @@ mod tests { let second = adapter.recv_input().await; assert!(matches!(second, ProtocolAdapterSchedulerInput::Turn)); assert!(matches!( - adapter - .advance_input(&mut scheduler, second, || { - panic!("ordinary protocol work cannot create a load attachment") - }) - .await, + adapter.advance_input(&mut scheduler, second).await, ProtocolAdapterSchedulerAdvance::ProtocolResidenceCompleted(_) )); }) .await; } - - #[derive(Debug, PartialEq, Eq)] - enum AdapterMode { - Classic, - Bidi { session_id: &'static str }, - } - - #[test] - fn exact_load_attachment_survives_adapter_switch_and_stale_terminal() { - let current = deferred_main_document_load_observation_id(2); - let stale = deferred_main_document_load_observation_id(1); - let mut adapter = ProtocolAdapterScheduler:: { - pending_load: Some(PendingAdapterLoadObservation { - observation_id: current, - output_interest: deferred_main_document_load_output_interest( - page_residence(), - None, - ), - attachment: AdapterMode::Classic, - }), - ..Default::default() - }; - - *adapter - .pending_load_attachment_mut() - .expect("exact load attachment should remain resident") = AdapterMode::Bidi { - session_id: "SID-upgraded", - }; - - assert!( - adapter.take_pending_load(stale).is_err(), - "a delayed terminal must not claim the current load attachment" - ); - assert_eq!( - adapter - .pending_load_attachment_mut() - .expect("stale terminal must preserve current attachment"), - &AdapterMode::Bidi { - session_id: "SID-upgraded", - } - ); - let claimed = adapter - .take_pending_load(current) - .expect("the exact terminal should claim its attachment"); - assert_eq!( - claimed.attachment, - AdapterMode::Bidi { - session_id: "SID-upgraded", - } - ); - assert!(!adapter.has_pending_load()); - } } diff --git a/moli-protocol-server/src/cdp_scheduler/command_dispatch.rs b/moli-protocol-server/src/cdp_scheduler/command_dispatch.rs index c71fdbd3b..85cf3a5e5 100644 --- a/moli-protocol-server/src/cdp_scheduler/command_dispatch.rs +++ b/moli-protocol-server/src/cdp_scheduler/command_dispatch.rs @@ -16,10 +16,6 @@ pub(crate) struct CommandTurnOutput { output_release_permit: Option, } -pub(crate) enum CommandDispatchStepOutput { - Emit(ProtocolOutputSequence), -} - impl CommandDispatchState { pub(crate) fn pending_command() -> Self { Self { @@ -27,13 +23,6 @@ impl CommandDispatchState { } } - pub(crate) fn route_pending_background_event( - &mut self, - event: BackgroundProtocolEvent, - ) -> CommandDispatchStepOutput { - CommandDispatchStepOutput::Emit(ProtocolOutputSequence::from_background_event(event)) - } - pub(crate) fn complete_with_turn_output( mut self, turn_output: CommandTurnOutput, @@ -57,14 +46,6 @@ impl CommandDispatchState { .with_renderer_output_boundary(renderer_output_boundary, post_renderer_output) .with_renderer_output_predecessor(renderer_output_predecessor) } - - pub(crate) fn complete_protocol_output( - mut self, - output: ProtocolOutputSequence, - ) -> ProtocolOutputSequence { - self.replies = output; - self.replies - } } impl CommandTurnOutput { diff --git a/moli-protocol-server/src/protocol_server/webdriver_bidi.rs b/moli-protocol-server/src/protocol_server/webdriver_bidi.rs index aad4a1b7f..913df2b31 100644 --- a/moli-protocol-server/src/protocol_server/webdriver_bidi.rs +++ b/moli-protocol-server/src/protocol_server/webdriver_bidi.rs @@ -176,7 +176,7 @@ async fn handle_bidi_session_socket_local( navigation_runtime_config, ); actor.install_runtime_response_ready_sender(&mut scheduler); - let mut adapter_scheduler = ProtocolAdapterScheduler::<()>::default(); + let mut adapter_scheduler = ProtocolAdapterScheduler::default(); loop { let page_javascript_blocked = scheduler.has_pending_javascript_dialog(); adapter_scheduler.schedule_turn_if_needed(&scheduler, page_javascript_blocked); @@ -362,7 +362,7 @@ impl BidiSocketActor { /// contract. pub(in crate::protocol_server) async fn recv_attached_input( &mut self, - adapter_scheduler: &mut ProtocolAdapterScheduler<()>, + adapter_scheduler: &mut ProtocolAdapterScheduler, page_javascript_blocked: bool, ) -> BidiSocketActorInput { tokio::select! { @@ -425,7 +425,7 @@ impl BidiSocketActor { pub(in crate::protocol_server) async fn handle_renderer_publication( &mut self, - adapter_scheduler: &mut ProtocolAdapterScheduler<()>, + adapter_scheduler: &mut ProtocolAdapterScheduler, scheduler: &mut CdpScheduler, receivers: &mut CdpSchedulerEventReceivers, publication: RendererOutputTransportMessage, @@ -465,15 +465,12 @@ impl BidiSocketActor { pub(in crate::protocol_server) async fn handle_adapter_scheduler_input( &mut self, - adapter_scheduler: &mut ProtocolAdapterScheduler<()>, + adapter_scheduler: &mut ProtocolAdapterScheduler, scheduler: &mut CdpScheduler, receivers: &mut CdpSchedulerEventReceivers, input: ProtocolAdapterSchedulerInput, ) -> bool { - let output = match adapter_scheduler - .advance_input(scheduler, input, || ()) - .await - { + let output = match adapter_scheduler.advance_input(scheduler, input).await { ProtocolAdapterSchedulerAdvance::ProtocolResidenceCompleted(output) | ProtocolAdapterSchedulerAdvance::DeferredLoadCompleted { output, .. } => output, ProtocolAdapterSchedulerAdvance::Idle diff --git a/moli-protocol-server/src/protocol_server/webdriver_classic/state.rs b/moli-protocol-server/src/protocol_server/webdriver_classic/state.rs index 274c22718..192c02efd 100644 --- a/moli-protocol-server/src/protocol_server/webdriver_classic/state.rs +++ b/moli-protocol-server/src/protocol_server/webdriver_classic/state.rs @@ -1348,7 +1348,7 @@ async fn classic_session_runtime_loop( navigation_runtime_config, ); let mut attached_bidi: Option = None; - let mut adapter_scheduler = ProtocolAdapterScheduler::<()>::default(); + let mut adapter_scheduler = ProtocolAdapterScheduler::default(); loop { if receivers.renderer_publication_rx.is_closed() { break; @@ -1532,7 +1532,7 @@ async fn classic_session_runtime_loop( } input = adapter_scheduler.recv_input(), if !page_javascript_blocked => { let _ = adapter_scheduler - .advance_input(&mut scheduler, input, || ()) + .advance_input(&mut scheduler, input) .await; } request = rx.recv() => { @@ -1874,7 +1874,7 @@ impl ValueExt for serde_json::Value { } async fn classic_session_ingest_ready_renderer_publications( - adapter_scheduler: &mut ProtocolAdapterScheduler<()>, + adapter_scheduler: &mut ProtocolAdapterScheduler, scheduler: &mut CdpScheduler, receivers: &mut CdpSchedulerEventReceivers, ) { diff --git a/moli-protocol/src/conn.rs b/moli-protocol/src/conn.rs index 38279e81c..dfff9ac2e 100644 --- a/moli-protocol/src/conn.rs +++ b/moli-protocol/src/conn.rs @@ -8,6 +8,7 @@ use std::{ }, }; +use indexmap::IndexMap; use moli_cookie_jar::{StoredCookie, StoredCookieQueryReport}; use moli_fetch::FetchConfig; use parking_lot::Mutex; @@ -1163,7 +1164,11 @@ pub struct CdpConnection { /// Chromium/Playwright auto-attach can ask new targets to wait until /// Runtime.runIfWaitingForDebugger before their initial document proceeds. pub auto_attach_wait_for_debugger_on_start: bool, - auto_attach_owner_sessions: HashMap, AutoAttachOwnerPolicy>, + // Insertion order is protocol state: the first matching owner supplies + // the primary auto-attached Page session, while later owners attach as + // auxiliary sessions. A randomized HashMap iteration order made that + // choice vary between otherwise identical processes. + auto_attach_owner_sessions: IndexMap, AutoAttachOwnerPolicy>, target_control: TargetControlPlane, default_target_lifecycle: DefaultTargetLifecycle, service_worker_auto_attach_related_owners: Vec, @@ -1475,7 +1480,7 @@ impl CdpConnection { target_info_change_events_enabled: false, target_discovery_filter: None, auto_attach_wait_for_debugger_on_start: false, - auto_attach_owner_sessions: HashMap::new(), + auto_attach_owner_sessions: IndexMap::new(), target_control: TargetControlPlane::default(), default_target_lifecycle: DefaultTargetLifecycle::default(), service_worker_auto_attach_related_owners: Vec::new(), @@ -1881,6 +1886,37 @@ impl CdpConnection { self.loaded_page_mut_for_interruptible_protocol_access_for_route(session_id, owner_route) } + /// Returns the Page that currently carries target-scoped configuration. + /// + /// A cross-Document navigation keeps its outgoing Page attached until the + /// replacement commits. Network and Emulation settings belong to the + /// stable target/session, so they must remain writable during that window: + /// the outgoing Page needs the update if navigation fails, while commit + /// configuration replays the same target state into the replacement Page. + /// Document-reading commands must continue to use + /// [`Self::loaded_page_mut_for_protocol_access`] and observe the navigation + /// gate instead. + pub(crate) fn loaded_page_mut_for_target_configuration( + &mut self, + session_id: Option<&str>, + ) -> Result<&mut Page, String> { + let none_session_owner_route = self.none_session_owner_route_override(); + self.loaded_page_mut_for_target_configuration_for_route( + session_id, + none_session_owner_route.as_ref(), + ) + } + + pub(crate) fn loaded_page_mut_for_target_configuration_for_route( + &mut self, + session_id: Option<&str>, + owner_route: Option<&CdpSessionRoute>, + ) -> Result<&mut Page, String> { + self.runtime_session_owner_slot_mut_for_route(session_id, owner_route)? + .loaded_page_mut() + .ok_or_else(|| "NoDocumentLoaded".to_owned()) + } + /// Returns the exact Page that remains attached while a cross-Document /// navigation is suspended. /// @@ -2259,12 +2295,12 @@ impl CdpConnection { Some(route), )); } - Some(CommandOwnerScope::from_session_and_owner_route( + Some(CommandOwnerScope::capture( + self, context .session_id .as_ref() .map(|session_id| session_id.as_str()), - None, )) } diff --git a/moli-protocol/src/conn/browser_context/fetch_owner.rs b/moli-protocol/src/conn/browser_context/fetch_owner.rs index 3b9076fe2..e5bcbc9dc 100644 --- a/moli-protocol/src/conn/browser_context/fetch_owner.rs +++ b/moli-protocol/src/conn/browser_context/fetch_owner.rs @@ -414,7 +414,22 @@ impl CdpConnection { session_id: Option<&str>, body: CapturedBody, ) -> Result { - let Some(mut owner) = self.target_session_owner_mut(session_id) else { + let none_session_owner_route = self.none_session_owner_route_override(); + self.open_io_stream_body_source_for_route( + session_id, + none_session_owner_route.as_ref(), + body, + ) + } + + pub(crate) fn open_io_stream_body_source_for_route( + &mut self, + session_id: Option<&str>, + owner_route: Option<&CdpSessionRoute>, + body: CapturedBody, + ) -> Result { + let Some(mut owner) = self.target_session_owner_mut_for_route(session_id, owner_route) + else { return Err("NoDocumentLoaded".to_owned()); }; owner.open_scoped_io_stream_body_source(body) diff --git a/moli-protocol/src/conn/browser_context/network_owner.rs b/moli-protocol/src/conn/browser_context/network_owner.rs index dab857273..0238642cf 100644 --- a/moli-protocol/src/conn/browser_context/network_owner.rs +++ b/moli-protocol/src/conn/browser_context/network_owner.rs @@ -1053,9 +1053,8 @@ impl CdpConnection { }; if refresh_active_engine { self.apply_active_engine_fetch_overrides(); - return self.start_rebuild_resource_runtime_for_session_owner(session_id); } - Ok(None) + self.start_rebuild_resource_runtime_for_session_owner(session_id) } fn start_set_non_page_browser_identity_override( @@ -1116,9 +1115,8 @@ impl CdpConnection { }; if refresh_active_engine { self.apply_active_engine_fetch_overrides(); - return self.start_rebuild_resource_runtime_for_session_owner(session_id); } - Ok(None) + self.start_rebuild_resource_runtime_for_session_owner(session_id) } pub(crate) fn start_set_tls_verify_host_for_session_owner( diff --git a/moli-protocol/src/conn/browser_context/target_session_owner.rs b/moli-protocol/src/conn/browser_context/target_session_owner.rs index b5b23b29a..d313da0d1 100644 --- a/moli-protocol/src/conn/browser_context/target_session_owner.rs +++ b/moli-protocol/src/conn/browser_context/target_session_owner.rs @@ -173,7 +173,10 @@ pub(crate) struct TargetNavigationLoadInputs { storage_handles: TargetNavigationStorageHandles, pub(crate) root_frame_id: Option, pub(crate) renderer_runtime: RendererBrowserContextRuntimeOwnerAccess, + /// Browser-side identity used for navigation request headers. pub(crate) browser_identity_override: Option, + /// Renderer-agent identity exposed through the committed Document's Navigator. + pub(crate) navigator_identity_override: Option, pub(crate) http_proxy_override: Option, pub(crate) http_no_proxy_override: Option, pub(crate) tls_verify_host_override: Option, @@ -270,6 +273,8 @@ impl TargetNavigationLoadInputs { renderer_runtime: browser_context.renderer_runtime_owner_access(), browser_identity_override: browser_context .effective_active_browser_identity_override_owned(), + navigator_identity_override: browser_context + .effective_active_renderer_browser_identity_override_owned(), http_proxy_override: browser_context.effective_active_http_proxy_override_owned(), http_no_proxy_override: browser_context.effective_active_http_no_proxy_override_owned(), tls_verify_host_override: browser_context.effective_active_tls_verify_host_override(), @@ -324,6 +329,8 @@ impl TargetNavigationLoadInputs { inputs.browser_context_id = Some(browser_context.id.clone()); inputs.browser_identity_override = browser_context.effective_active_browser_identity_override_owned(); + inputs.navigator_identity_override = + browser_context.effective_active_renderer_browser_identity_override_owned(); inputs.http_proxy_override = browser_context.effective_active_http_proxy_override_owned(); inputs.http_no_proxy_override = browser_context.effective_active_http_no_proxy_override_owned(); @@ -351,6 +358,7 @@ impl TargetNavigationLoadInputs { root_frame_id: None, renderer_runtime, browser_identity_override: None, + navigator_identity_override: None, http_proxy_override: None, http_no_proxy_override: None, tls_verify_host_override: None, @@ -805,6 +813,9 @@ impl<'a> TargetSessionOwnerRef<'a> { .network_policy .browser_identity_override_owned() .or_else(|| browser_context.default_browser_identity_override_owned()), + navigator_identity_override: page_state + .effective_renderer_browser_identity_override_owned() + .or_else(|| browser_context.default_browser_identity_override_owned()), http_proxy_override: page_state .http_proxy_override .clone() diff --git a/moli-protocol/src/conn/command_owner_scope.rs b/moli-protocol/src/conn/command_owner_scope.rs index 7eaf42342..1d6973cb6 100644 --- a/moli-protocol/src/conn/command_owner_scope.rs +++ b/moli-protocol/src/conn/command_owner_scope.rs @@ -10,7 +10,16 @@ impl CommandOwnerScope { pub(crate) fn capture(conn: &CdpConnection, session_id: Option<&str>) -> Self { let none_session_owner_route = session_id .is_none() - .then(|| conn.none_session_owner_route_override()) + .then(|| { + conn.none_session_owner_route_override().or_else(|| { + let browser_context = conn.browser_context.as_ref()?; + let target_id = browser_context.active_target_id_owned()?; + Some(CdpSessionRoute::ActiveTarget { + browser_context_id: browser_context.id.clone(), + target_id: Some(target_id), + }) + }) + }) .flatten(); Self { session_id: session_id.map(str::to_owned), @@ -35,8 +44,9 @@ impl CommandOwnerScope { /// Returns the exact route captured for an implicit-session command. /// /// A concrete CDP session remains authoritative through `session_id`; the - /// route is only needed for protocol-neutral and deferred work which uses - /// Chromium's implicit primary Page attachment. + /// route freezes Chromium's implicit primary Page attachment at command + /// admission, so deferred completion cannot follow a later foreground + /// selection. pub(crate) fn session_owner_route(&self) -> Option<&CdpSessionRoute> { self.session_owner_route.as_ref() } @@ -48,3 +58,28 @@ impl CommandOwnerScope { conn.scoped_optional_none_session_owner_route_override(self.session_owner_route.clone()) } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::conn::BrowserContext; + + #[test] + fn implicit_scope_freezes_the_concrete_active_target() { + let mut conn = CdpConnection::default(); + let mut browser_context = BrowserContext::new("BID-scope".to_owned()); + browser_context.set_active_target_id("TID-original"); + conn.browser_context = Some(browser_context); + + let scope = CommandOwnerScope::capture(&conn, None); + conn.browser_context + .as_mut() + .expect("browser context") + .set_active_target_id("TID-replacement"); + + assert_eq!( + conn.target_owner_identity_for_route(scope.session_id(), scope.session_owner_route(),), + Some(("BID-scope".to_owned(), Some("TID-original".to_owned()))) + ); + } +} diff --git a/moli-protocol/src/conn/page_state/parked.rs b/moli-protocol/src/conn/page_state/parked.rs index 372034068..583d42e8f 100644 --- a/moli-protocol/src/conn/page_state/parked.rs +++ b/moli-protocol/src/conn/page_state/parked.rs @@ -374,6 +374,8 @@ impl BrowserContext { let previous_bypass = state.network_policy.bypass_service_worker(); let previous_cache_disabled = state.network_policy.cache_disabled(); let previous_browser_identity = state.network_policy.browser_identity_override_owned(); + let previous_renderer_browser_identity = + state.effective_renderer_browser_identity_override_owned(); let previous_locale = state.locale_override.clone(); let previous_timezone = state.timezone_override.clone(); let routed_session_id = is_auxiliary.then_some(session_id); @@ -386,6 +388,10 @@ impl BrowserContext { previous_cache_disabled != state.network_policy.cache_disabled(); let browser_identity_changed = previous_browser_identity != state.network_policy.browser_identity_override_owned(); + let renderer_browser_identity_changed = previous_renderer_browser_identity + != state.effective_renderer_browser_identity_override_owned(); + let any_browser_identity_changed = + browser_identity_changed || renderer_browser_identity_changed; let locale_changed = previous_locale != state.locale_override; let timezone_changed = previous_timezone != state.timezone_override; @@ -395,7 +401,7 @@ impl BrowserContext { && !locale_changed && !timezone_changed { - return Ok(browser_identity_changed); + return Ok(any_browser_identity_changed); } let ( @@ -433,7 +439,7 @@ impl BrowserContext { .and_then(|target| target.runtime_slot.loaded_page_mut()) }; let Some(page) = page else { - return Ok(browser_identity_changed); + return Ok(any_browser_identity_changed); }; if extra_headers_changed || bypass_service_worker_changed || cache_disabled_changed { page.set_network_request_policy_async( @@ -456,7 +462,7 @@ impl BrowserContext { .await .map_err(|error| format!("failed to restore detached session timezone: {error}"))?; } - Ok(browser_identity_changed) + Ok(any_browser_identity_changed) } pub(crate) fn remove_auxiliary_sessions_for_target(&mut self, target_id: &str) -> Vec { diff --git a/moli-protocol/src/conn/resource_runtime_support.rs b/moli-protocol/src/conn/resource_runtime_support.rs index dc3763012..b086e136d 100644 --- a/moli-protocol/src/conn/resource_runtime_support.rs +++ b/moli-protocol/src/conn/resource_runtime_support.rs @@ -228,6 +228,11 @@ impl CdpConnection { session_id: Option<&str>, ) -> Result, String> { let load_inputs = self.navigation_load_inputs_for_session_owner(session_id); + let navigator_identity = load_inputs + .navigator_identity_override + .clone() + .or_else(|| self.global_browser_identity_override.clone()) + .unwrap_or_else(|| self.base_browser_identity.clone()); let storage = load_inputs.resource_storage_handles(); let request_client = self .configured_navigation_engine_for_load_inputs_mut(&load_inputs) @@ -239,9 +244,12 @@ impl CdpConnection { let Some(page) = self.resource_runtime_apply_page_for_session_owner(session_id) else { return Ok(None); }; - page.start_replace_browser_resource_runtime(&request_client.browser_resource_runtime()) - .map(Some) - .map_err(|error| format!("failed to update page resource runtime: {error}")) + page.start_replace_browser_resource_runtime_with_navigator_identity( + &request_client.browser_resource_runtime(), + navigator_identity, + ) + .map(Some) + .map_err(|error| format!("failed to update page resource runtime: {error}")) } pub(crate) fn finish_rebuild_resource_runtime_for_session_owner( @@ -249,11 +257,10 @@ impl CdpConnection { session_id: Option<&str>, completion: CompletedPageCommand, ) -> Result<(), String> { - let Some(page) = self.resource_runtime_apply_page_for_session_owner(session_id) else { - return Ok(()); - }; - page.finish_replace_browser_resource_runtime(completion) - .map_err(|error| format!("failed to update page resource runtime: {error}")) + finish_resource_runtime_update_on_current_attachment( + self.resource_runtime_apply_page_for_session_owner(session_id), + completion, + ) } pub(crate) fn finish_rebuild_resource_runtime_for_route( @@ -262,11 +269,10 @@ impl CdpConnection { owner_route: Option<&super::CdpSessionRoute>, completion: CompletedPageCommand, ) -> Result<(), String> { - let Some(page) = self.resource_runtime_apply_page_for_route(session_id, owner_route) else { - return Ok(()); - }; - page.finish_replace_browser_resource_runtime(completion) - .map_err(|error| format!("failed to update page resource runtime: {error}")) + finish_resource_runtime_update_on_current_attachment( + self.resource_runtime_apply_page_for_route(session_id, owner_route), + completion, + ) } fn resource_runtime_apply_page_for_session_owner( @@ -282,7 +288,8 @@ impl CdpConnection { .as_mut() .and_then(|bc| bc.active_target.runtime_slot.loaded_page_mut()); } - self.loaded_page_mut_for_protocol_access(session_id).ok() + self.loaded_page_mut_for_target_configuration(session_id) + .ok() } fn resource_runtime_apply_page_for_route( @@ -299,7 +306,28 @@ impl CdpConnection { .as_mut() .and_then(|bc| bc.active_target.runtime_slot.loaded_page_mut()); } - self.loaded_page_mut_for_protocol_access_for_route(session_id, owner_route) + self.loaded_page_mut_for_target_configuration_for_route(session_id, owner_route) .ok() } } + +fn finish_resource_runtime_update_on_current_attachment( + page: Option<&mut moli_core::page::Page>, + completion: CompletedPageCommand, +) -> Result<(), String> { + let completion_attachment = completion.renderer_agent_attachment_id(); + if let Some(page) = page + && page.renderer_agent_attachment_id() == completion_attachment + { + return page + .finish_replace_browser_resource_runtime(completion) + .map_err(|error| format!("failed to update page resource runtime: {error}")); + } + + completion + .into_unit_page_command_turn() + .map(drop) + .map_err(|error| { + format!("stale resource-runtime update returned an unexpected reply: {error}") + }) +} diff --git a/moli-protocol/src/conn/runtime_eval.rs b/moli-protocol/src/conn/runtime_eval.rs index 55a1b0072..78344c83d 100644 --- a/moli-protocol/src/conn/runtime_eval.rs +++ b/moli-protocol/src/conn/runtime_eval.rs @@ -3082,6 +3082,44 @@ impl CdpConnection { session_id: Option<&str>, dispatched_attachment_id: RendererAgentAttachmentId, messages: &mut Vec, + ) { + self.restore_frontend_command_ids_in_devtools_session_output_with_projection( + session_id, + dispatched_attachment_id, + messages, + true, + ); + } + + /// Resolves a terminal response which won its renderer response lease + /// before navigation, but whose old Page journal reached protocol ingress + /// after the replacement attachment committed. + /// + /// The exact `(session, renderer call, attachment)` correlation remains + /// the terminal authority in this race. Notifications and V8 state from + /// the retired attachment are discarded by the caller, and remote-object + /// ownership is intentionally not projected onto the replacement + /// document. + pub(crate) fn restore_frontend_command_ids_in_retired_devtools_session_output( + &mut self, + session_id: Option<&str>, + dispatched_attachment_id: RendererAgentAttachmentId, + messages: &mut Vec, + ) { + self.restore_frontend_command_ids_in_devtools_session_output_with_projection( + session_id, + dispatched_attachment_id, + messages, + false, + ); + } + + fn restore_frontend_command_ids_in_devtools_session_output_with_projection( + &mut self, + session_id: Option<&str>, + dispatched_attachment_id: RendererAgentAttachmentId, + messages: &mut Vec, + project_runtime_object_ownership: bool, ) { messages.retain_mut(|message| { let RendererRuntimeInspectorMessage::Protocol(message) = message else { @@ -3117,7 +3155,7 @@ impl CdpConnection { return false; }; message.value_mut()["id"] = json!(correlation.frontend_command_id().get()); - if message.value().get("result").is_some() { + if project_runtime_object_ownership && message.value().get("result").is_some() { if let Some(object_group) = result_object_group.as_deref() { self.register_runtime_remote_object_ids_from_value_for_session_owner_with_group( session_id, diff --git a/moli-protocol/src/conn/runtime_load.rs b/moli-protocol/src/conn/runtime_load.rs index 91e27c6c6..19d3bb201 100644 --- a/moli-protocol/src/conn/runtime_load.rs +++ b/moli-protocol/src/conn/runtime_load.rs @@ -1560,14 +1560,27 @@ impl CdpConnection { &mut self, session_id: Option<&str>, final_url: &Url, - ) -> PreparedDocumentPageCommitConfiguration { + ) -> Result { let idle_override = self.idle_override_for_navigation(session_id, final_url); let load_inputs = self.navigation_load_inputs_for_session_owner(session_id); + // The renderer runtime is shared by the BrowserContext, but each Page + // target owns its NavigationEngine and may have a different transport + // identity. Resolve through that target's engine at the commit + // boundary instead of copying whichever runtime another target most + // recently registered on the shared renderer context. + let browser_resource_runtime = self + .ensure_resource_request_client_for_navigation_load_inputs(&load_inputs)? + .browser_resource_runtime(); + let navigator_identity = load_inputs + .navigator_identity_override + .clone() + .or_else(|| self.global_browser_identity_override.clone()) + .unwrap_or_else(|| self.base_browser_identity.clone()); let runtime_isolated_worlds = self .prepare_loaded_navigation_commit_for_session_owner(session_id) .map(|commit_state| commit_state.isolated_worlds) .unwrap_or_default(); - PreparedDocumentPageCommitConfiguration { + Ok(PreparedDocumentPageCommitConfiguration { document_start_scripts: load_inputs.document_start_scripts, runtime_bindings: load_inputs.runtime_bindings, runtime_inspector_session_restore_snapshots: load_inputs @@ -1583,10 +1596,14 @@ impl CdpConnection { emulated_media: load_inputs.emulated_media, idle_override, viewport_surface: load_inputs.viewport_surface, + browser_resource_runtime, + navigator_identity, network_offline: load_inputs.network_offline, + bypass_service_worker: load_inputs.bypass_service_worker, + cache_disabled: load_inputs.cache_disabled, blocked_url_patterns: load_inputs.blocked_url_patterns, fetch_subresource_interception: load_inputs.fetch_subresource_interception, - } + }) } fn idle_override_for_navigation( @@ -1594,11 +1611,10 @@ impl CdpConnection { session_id: Option<&str>, final_url: &Url, ) -> Option { - // Commit configuration is assembled after the document navigation has - // entered its pending state, where ordinary protocol access to the old - // document is intentionally blocked. The outgoing page remains the - // owner of frame-host state until commit and is the source Chromium - // preserves when a same-site navigation reuses that frame host. + // Chromium stores this override on RenderFrameHostImpl's IdleManager, + // not on the DevTools target. Preserve it only while a same-site + // navigation can retain that frame-host state; a cross-site renderer + // replacement must start with the actual idle state. let page = self .runtime_session_owner_slot(session_id) .ok()? @@ -1801,7 +1817,13 @@ impl CdpConnection { .is_some_and(moli_url::is_about_blank) } - pub(crate) fn runtime_session_owner_should_start_initial_document_navigation( + /// Returns whether the materialized initial `about:blank` still needs to + /// be replaced by the target URL. + /// + /// This is a structural lifecycle query. It deliberately does not look + /// at `waitForDebuggerOnStart`: the explicit debugger-resume path uses it + /// after the paused session has been released. + pub(crate) fn runtime_session_owner_needs_initial_document_navigation( &self, session_id: Option<&str>, ) -> bool { @@ -1816,6 +1838,22 @@ impl CdpConnection { true } + /// Returns whether an ordinary Page/Target command may opportunistically + /// start the initial target-URL navigation. + /// + /// A target created with `waitForDebuggerOnStart` must remain on its + /// initial document until `Runtime.runIfWaitingForDebugger` has published + /// its terminal response. Keeping that admission rule here prevents + /// commands such as `Page.enable` and `Page.createIsolatedWorld` from + /// racing each other into replacing the paused renderer attachment. + pub(crate) fn runtime_session_owner_can_start_initial_document_navigation( + &self, + session_id: Option<&str>, + ) -> bool { + !self.session_owner_target_has_waiting_for_debugger_session(session_id) + && self.runtime_session_owner_needs_initial_document_navigation(session_id) + } + pub(crate) fn runtime_session_owner_initial_empty_document_has_replacement_url( &self, session_id: Option<&str>, @@ -2182,7 +2220,7 @@ impl CdpConnection { let configuration = self.prepared_document_commit_configuration_for_session_owner( session_id, navigation.final_url(), - ); + )?; navigation .update_commit_configuration(configuration) .await?; diff --git a/moli-protocol/src/conn/state/browser_context.rs b/moli-protocol/src/conn/state/browser_context.rs index 252c226f9..866770267 100644 --- a/moli-protocol/src/conn/state/browser_context.rs +++ b/moli-protocol/src/conn/state/browser_context.rs @@ -1074,6 +1074,24 @@ impl BrowserContext { .is_none_or(TargetOwnerState::can_install_current_initial_empty_document_page) } + /// Reports whether one exact Page target is still on its materialized + /// initial empty Document and has a non-empty target URL left to load. + /// + /// This target-addressed query is used when the last debugger barrier is + /// released by detaching its session, after that session can no longer be + /// used as a routing key. + pub(crate) fn target_needs_initial_document_navigation(&self, target_id: &str) -> bool { + let Some(target) = self.page_target(target_id) else { + return false; + }; + let owner_state = &target.owner_state; + let Some(initial_url) = owner_state.initial_empty_document_url_if_current() else { + return false; + }; + target.target_url() != initial_url + && !owner_state.initial_empty_document_pending_cross_document_navigation() + } + pub(crate) fn loaded_document_renderer_owner_ids_for_diagnostics(&self) -> HashSet { let mut owner_ids = HashSet::new(); if let Some(page) = self.loaded_page() { @@ -1634,6 +1652,15 @@ impl BrowserContext { self.effective_active_browser_identity_override().cloned() } + pub(crate) fn effective_active_renderer_browser_identity_override_owned( + &self, + ) -> Option { + self.page_targets + .active() + .and_then(|host| host.effective_renderer_browser_identity_override_owned()) + .or_else(|| self.default_browser_identity.profile_owned()) + } + pub(crate) fn default_browser_identity_override( &self, ) -> Option<&moli_browser_profile::BrowserIdentityProfile> { diff --git a/moli-protocol/src/conn/state/devtools_session.rs b/moli-protocol/src/conn/state/devtools_session.rs index 32c8ddab5..5bb8c4469 100644 --- a/moli-protocol/src/conn/state/devtools_session.rs +++ b/moli-protocol/src/conn/state/devtools_session.rs @@ -62,6 +62,7 @@ pub(crate) struct DevToolsSessionState { pub(crate) struct DevToolsSessionRegistry { states: BTreeMap, attached_order: Vec, + browser_identity_activation_order: Vec, } impl Default for DevToolsSessionRegistry { @@ -72,6 +73,7 @@ impl Default for DevToolsSessionRegistry { DevToolsSessionState::default(), )]), attached_order: Vec::new(), + browser_identity_activation_order: Vec::new(), } } } @@ -139,6 +141,8 @@ impl DevToolsSessionRegistry { if removed.is_some() { self.attached_order .retain(|attached| attached != session_id); + self.browser_identity_activation_order + .retain(|candidate| candidate != &key); } removed } @@ -147,6 +151,8 @@ impl DevToolsSessionRegistry { self.states .retain(|key, _state| matches!(key, DevToolsSessionKey::Primary)); self.attached_order.clear(); + self.browser_identity_activation_order + .retain(|key| matches!(key, DevToolsSessionKey::Primary)); } pub(crate) fn attached_len(&self) -> usize { @@ -207,19 +213,49 @@ impl DevToolsSessionRegistry { aggregate } - pub(crate) fn effective_browser_identity_override( + pub(crate) fn effective_network_browser_identity_override( &self, + ) -> Option { + Self::aggregate_browser_identity_overrides(self.states_in_attachment_order().filter_map( + |state| { + state + .emulation_session_state + .browser_identity_override + .as_ref() + }, + )) + } + + /// Resolves the identity exposed by the live renderer Document. + /// + /// Chromium's renderer agents enter the instrumenting-agent list when a + /// session first enables a non-empty UA override. Updating that session + /// does not move it in the list, so this order intentionally differs from + /// the browser-side attachment order used for navigation request headers. + pub(crate) fn effective_renderer_browser_identity_override( + &self, + ) -> Option { + Self::aggregate_browser_identity_overrides( + self.browser_identity_activation_order + .iter() + .filter_map(|key| self.states.get(key)) + .filter_map(|state| { + state + .emulation_session_state + .browser_identity_override + .as_ref() + }), + ) + } + + fn aggregate_browser_identity_overrides<'a>( + contributions: impl Iterator, ) -> Option { let mut identity_base = None; let mut user_agent = None; let mut accept_language = None; let mut navigator_platform = None; - for contribution in self.states_in_attachment_order().filter_map(|state| { - state - .emulation_session_state - .browser_identity_override - .as_ref() - }) { + for contribution in contributions { identity_base = Some(&contribution.base); if contribution.user_agent.is_some() { user_agent = Some(contribution); @@ -257,6 +293,12 @@ impl DevToolsSessionRegistry { session_id: Option<&str>, browser_identity_override: Option, ) { + let key = Self::routed_key(is_attached_session, session_id); + if browser_identity_override.is_some() + && !self.browser_identity_activation_order.contains(&key) + { + self.browser_identity_activation_order.push(key); + } let state = self.routed_mut_or_insert(is_attached_session, session_id); state.emulation_session_state.browser_identity_override = browser_identity_override; } @@ -342,6 +384,8 @@ impl DevToolsSessionRegistry { if let Some(state) = self.states.get_mut(&key) { state.emulation_session_state = DevToolsEmulationSessionState::default(); } + self.browser_identity_activation_order + .retain(|candidate| candidate != &key); } pub(crate) fn states_mut(&mut self) -> impl Iterator { @@ -350,6 +394,8 @@ impl DevToolsSessionRegistry { pub(crate) fn reset(&mut self, preserve_attached_sessions: bool) { *self.primary_mut() = DevToolsSessionState::default(); + self.browser_identity_activation_order + .retain(|key| !matches!(key, DevToolsSessionKey::Primary)); if !preserve_attached_sessions { self.clear_attached(); } @@ -1128,10 +1174,18 @@ mod tests { sessions: &DevToolsSessionRegistry, ) -> moli_browser_profile::BrowserIdentityProfile { sessions - .effective_browser_identity_override() + .effective_network_browser_identity_override() .expect("browser identity contribution should be effective") } + fn effective_renderer_identity( + sessions: &DevToolsSessionRegistry, + ) -> moli_browser_profile::BrowserIdentityProfile { + sessions + .effective_renderer_browser_identity_override() + .expect("renderer browser identity contribution should be effective") + } + #[test] fn registry_owns_primary_and_attached_sessions_in_stable_order() { let mut sessions = DevToolsSessionRegistry::default(); @@ -1203,7 +1257,7 @@ mod tests { } #[test] - fn browser_identity_override_uses_attachment_order_not_setter_order() { + fn browser_identity_uses_distinct_network_and_renderer_agent_order() { let mut sessions = DevToolsSessionRegistry::default(); sessions.ensure_attached("SID-later"); sessions.set_browser_identity_override( @@ -1217,41 +1271,63 @@ mod tests { identity_override("Moli/Primary-1", None, None), ); assert_eq!(effective_identity(&sessions).user_agent(), "Moli/Later-1"); - - sessions.set_browser_identity_override( - false, - None, - identity_override("Moli/Primary-2", None, None), - ); assert_eq!( - effective_identity(&sessions).user_agent(), - "Moli/Later-1", - "a later setter must not outrank a later-attached session" + effective_renderer_identity(&sessions).user_agent(), + "Moli/Primary-1", + "the renderer follows agent activation order, not attachment order" ); - sessions.set_browser_identity_override(true, Some("SID-later"), None); - assert_eq!(effective_identity(&sessions).user_agent(), "Moli/Primary-2"); - sessions.set_browser_identity_override( true, Some("SID-later"), identity_override("Moli/Later-2", None, None), ); + assert_eq!( + effective_identity(&sessions).user_agent(), + "Moli/Later-2", + "the browser-side winner remains the later-attached session" + ); + assert_eq!( + effective_renderer_identity(&sessions).user_agent(), + "Moli/Primary-1", + "updating an enabled renderer agent must not reorder it" + ); + + sessions.set_browser_identity_override( + false, + None, + identity_override("Moli/Primary-2", None, None), + ); assert_eq!(effective_identity(&sessions).user_agent(), "Moli/Later-2"); + assert_eq!( + effective_renderer_identity(&sessions).user_agent(), + "Moli/Primary-2" + ); + + sessions.set_browser_identity_override(false, None, None); + assert_eq!(effective_identity(&sessions).user_agent(), "Moli/Later-2"); + assert_eq!( + effective_renderer_identity(&sessions).user_agent(), + "Moli/Later-2" + ); + + sessions.set_browser_identity_override( + false, + None, + identity_override("Moli/Primary-3", Some("fr-FR"), Some("PrimaryPlatform")), + ); + assert_eq!(effective_identity(&sessions).user_agent(), "Moli/Later-2"); + let renderer_identity = effective_renderer_identity(&sessions); + assert_eq!(renderer_identity.user_agent(), "Moli/Primary-3"); + assert_eq!(renderer_identity.accept_language(), "fr-FR"); + assert_eq!(renderer_identity.navigator_platform(), "PrimaryPlatform"); sessions.remove_attached("SID-later"); - assert_eq!(effective_identity(&sessions).user_agent(), "Moli/Primary-2"); - - sessions.ensure_attached("SID-later"); - sessions.set_browser_identity_override( - true, - Some("SID-later"), - identity_override("", Some("fr-FR"), Some("AuxPlatform")), + assert_eq!(effective_identity(&sessions).user_agent(), "Moli/Primary-3"); + assert_eq!( + effective_renderer_identity(&sessions).user_agent(), + "Moli/Primary-3" ); - let identity = effective_identity(&sessions); - assert_eq!(identity.user_agent(), "Moli/Primary-2"); - assert_eq!(identity.accept_language(), "fr-FR"); - assert_eq!(identity.navigator_platform(), "AuxPlatform"); } #[test] diff --git a/moli-protocol/src/conn/state/session.rs b/moli-protocol/src/conn/state/session.rs index 793bf55fe..a84b77feb 100644 --- a/moli-protocol/src/conn/state/session.rs +++ b/moli-protocol/src/conn/state/session.rs @@ -149,6 +149,14 @@ impl Default for TargetPageState { } impl TargetPageState { + pub(crate) fn effective_renderer_browser_identity_override_owned( + &self, + ) -> Option { + self.devtools_sessions + .effective_renderer_browser_identity_override() + .or_else(|| self.network_policy.base_browser_identity_override_owned()) + } + pub(crate) fn mutate_devtools_network_session_state( &mut self, is_attached_session: bool, @@ -225,8 +233,9 @@ impl TargetPageState { } pub(crate) fn refresh_devtools_emulation_policy(&mut self) { - self.network_policy.devtools_browser_identity_override = - self.devtools_sessions.effective_browser_identity_override(); + self.network_policy.devtools_browser_identity_override = self + .devtools_sessions + .effective_network_browser_identity_override(); self.locale_override = self .devtools_sessions .effective_locale_override() @@ -649,6 +658,12 @@ impl Default for TargetNetworkPolicyState { } impl TargetNetworkPolicyState { + pub(crate) fn base_browser_identity_override_owned( + &self, + ) -> Option { + self.base_browser_identity.profile_owned() + } + pub(crate) fn cache_disabled(&self) -> bool { self.base_cache_disabled || self.devtools_cache_disabled } diff --git a/moli-protocol/src/conn/target/auto_attach_owner.rs b/moli-protocol/src/conn/target/auto_attach_owner.rs index 21dd16624..437ffbbc9 100644 --- a/moli-protocol/src/conn/target/auto_attach_owner.rs +++ b/moli-protocol/src/conn/target/auto_attach_owner.rs @@ -80,7 +80,7 @@ impl CdpConnection { }, ); } else { - self.auto_attach_owner_sessions.remove(&key); + self.auto_attach_owner_sessions.shift_remove(&key); } self.sync_auto_attach_flags_from_owners(); } @@ -88,7 +88,47 @@ impl CdpConnection { pub(crate) fn clear_auto_attach_owner(&mut self, session_id: Option<&str>) { self.clear_service_worker_auto_attach_related_owner(session_id); let key = session_id.map(str::to_owned); - self.auto_attach_owner_sessions.remove(&key); + self.auto_attach_owner_sessions.shift_remove(&key); self.sync_auto_attach_flags_from_owners(); } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::testing::TestContext; + + #[test] + fn auto_attach_owners_keep_protocol_attachment_order() { + let mut ctx = TestContext::new(); + let page_filter = CdpTargetFilter::default_auto_attach(); + ctx.conn + .set_auto_attach_owner(None, true, false, page_filter.clone()); + ctx.conn + .set_auto_attach_owner(Some("SID-browser"), true, true, page_filter.clone()); + + assert_eq!( + ctx.conn.auto_attach_owner_sessions_for_target_type("page"), + vec![None, Some("SID-browser".to_owned())] + ); + + // Updating an existing owner's policy is not a new attachment and + // must not move it behind newer owners. + ctx.conn + .set_auto_attach_owner(None, true, true, page_filter.clone()); + assert_eq!( + ctx.conn.auto_attach_owner_sessions_for_target_type("page"), + vec![None, Some("SID-browser".to_owned())] + ); + + // Removing and attaching again is a new protocol attachment. + ctx.conn + .set_auto_attach_owner(None, false, false, page_filter.clone()); + ctx.conn + .set_auto_attach_owner(None, true, false, page_filter); + assert_eq!( + ctx.conn.auto_attach_owner_sessions_for_target_type("page"), + vec![Some("SID-browser".to_owned()), None] + ); + } +} diff --git a/moli-protocol/src/conn/target/control.rs b/moli-protocol/src/conn/target/control.rs index a437e4925..e7cf6220e 100644 --- a/moli-protocol/src/conn/target/control.rs +++ b/moli-protocol/src/conn/target/control.rs @@ -482,6 +482,11 @@ impl TargetControlPlane { .target_has_waiting_for_debugger_session(target_id) } + pub(crate) fn release_waiting_for_debugger_session(&mut self, session_id: &str) -> bool { + self.sessions + .release_waiting_for_debugger_session(session_id) + } + pub(crate) fn auto_attached_sessions_for_owner( &self, owner_session_id: Option<&str>, @@ -714,7 +719,7 @@ mod tests { assert_eq!(detached.target_id(), "TID-page"); assert_eq!(detached.route(), Some(&route)); assert!(!detached.auto_attached()); - assert!(!detached.waiting_for_debugger()); + assert!(!detached.was_waiting_for_debugger()); let events = plan.into_background_events(); assert_eq!(events.len(), 1); diff --git a/moli-protocol/src/conn/target/session.rs b/moli-protocol/src/conn/target/session.rs index e6df6a11a..d0bbaa336 100644 --- a/moli-protocol/src/conn/target/session.rs +++ b/moli-protocol/src/conn/target/session.rs @@ -87,8 +87,7 @@ impl DetachedTargetSession { self.auto_attached } - #[cfg(test)] - pub(crate) fn waiting_for_debugger(&self) -> bool { + pub(crate) fn was_waiting_for_debugger(&self) -> bool { self.waiting_for_debugger } } @@ -367,6 +366,21 @@ impl TargetSessionRegistry { }) } + /// Releases the debugger-on-start barrier contributed by one attached + /// session. + /// + /// V8 keeps one barrier per inspector session and resumes the target only + /// after every waiting session has run `Runtime.runIfWaitingForDebugger` + /// (or detached). Keep that per-session transition in the attachment + /// registry instead of treating `waitingForDebugger` as immutable event + /// metadata. + pub(crate) fn release_waiting_for_debugger_session(&mut self, session_id: &str) -> bool { + let Some(session) = self.attached_sessions.get_mut(session_id) else { + return false; + }; + std::mem::take(&mut session.waiting_for_debugger) + } + pub(crate) fn attached_session_owner_session_id(&self, session_id: &str) -> Option<&str> { self.attached_sessions .get(session_id)? @@ -773,6 +787,29 @@ mod tests { assert!(!registry.target_has_waiting_for_debugger_session("TID-unattached")); } + #[test] + fn target_session_registry_releases_each_debugger_barrier_exactly_once() { + let mut registry = TargetSessionRegistry::default(); + for session_id in ["SID-first", "SID-second"] { + registry.commit_attached_session(PreparedAttachSession::new( + session_id.to_owned(), + Some("SID-owner"), + "TID-page", + None, + true, + true, + )); + } + + assert!(registry.target_has_waiting_for_debugger_session("TID-page")); + assert!(registry.release_waiting_for_debugger_session("SID-first")); + assert!(registry.target_has_waiting_for_debugger_session("TID-page")); + assert!(!registry.release_waiting_for_debugger_session("SID-first")); + assert!(registry.release_waiting_for_debugger_session("SID-second")); + assert!(!registry.target_has_waiting_for_debugger_session("TID-page")); + assert!(!registry.release_waiting_for_debugger_session("SID-missing")); + } + #[test] fn target_session_registry_clears_attached_session_and_auto_attached_indexes() { let mut registry = TargetSessionRegistry::default(); diff --git a/moli-protocol/src/conn/target/session_binding.rs b/moli-protocol/src/conn/target/session_binding.rs index 0fe5fc6c8..68f5e4976 100644 --- a/moli-protocol/src/conn/target/session_binding.rs +++ b/moli-protocol/src/conn/target/session_binding.rs @@ -545,10 +545,20 @@ impl CdpConnection { reason, parent_session_id, ); + let released_debugger_barrier = plan + .detached_sessions() + .iter() + .any(|session| session.target_id() == target_id && session.was_waiting_for_debugger()); self.clear_detached_target_session_owner_state(session_id); if let Some(attached_state_delta_plan) = attached_state_delta_plan { plan.extend(attached_state_delta_plan); } + if released_debugger_barrier && !self.target_has_waiting_for_debugger_session(target_id) { + crate::domains::target::schedule_initial_document_target_url_navigation_after_debugger_barrier_release_for_target( + self, + target_id, + ); + } plan } @@ -1076,6 +1086,16 @@ impl CdpConnection { .target_has_waiting_for_debugger_session(target_id) } + pub(crate) fn release_waiting_for_debugger_session( + &mut self, + session_id: Option<&str>, + ) -> bool { + session_id.is_some_and(|session_id| { + self.target_control + .release_waiting_for_debugger_session(session_id) + }) + } + pub(crate) fn auto_attached_sessions_for_owner( &self, owner_session_id: Option<&str>, diff --git a/moli-protocol/src/conn/tests/resource_runtime.rs b/moli-protocol/src/conn/tests/resource_runtime.rs index 51615bcd6..bd0754f06 100644 --- a/moli-protocol/src/conn/tests/resource_runtime.rs +++ b/moli-protocol/src/conn/tests/resource_runtime.rs @@ -48,10 +48,12 @@ async fn commit_navigation_outcome_for_session_test( match outcome { NavigationLoadOutcome::ResponseCommitReady(navigation) => { let navigation = *navigation; - let configuration = conn.prepared_document_commit_configuration_for_session_owner( - session_id, - navigation.final_url(), - ); + let configuration = conn + .prepared_document_commit_configuration_for_session_owner( + session_id, + navigation.final_url(), + ) + .expect("test navigation commit configuration should resolve"); navigation .update_commit_configuration(configuration) .await diff --git a/moli-protocol/src/domains/activity/output_ingress/prepared_outputs.rs b/moli-protocol/src/domains/activity/output_ingress/prepared_outputs.rs index e14f98fef..4445cdc5e 100644 --- a/moli-protocol/src/domains/activity/output_ingress/prepared_outputs.rs +++ b/moli-protocol/src/domains/activity/output_ingress/prepared_outputs.rs @@ -111,10 +111,21 @@ impl PreparedProtocolOutputs { .append_to_output_sink(&mut prepared); } RendererProtocolObservation::RuntimeInspector(batch) => { + let source_batches = vec![batch.clone()]; let mut batches = conn.route_current_renderer_inspector_output_for_session_owner( session_id, - vec![batch.clone()], + source_batches.clone(), ); + if batches.is_empty() { + crate::domains::runtime::RuntimePreparedOutputs:: + from_retired_renderer_runtime_inspector_session_responses( + conn, + session_id, + &source_batches, + ) + .append_to_output_sink(&mut prepared); + return prepared; + } for batch in &mut batches { let Some(attachment) = conn .target_page_protocol_attachment_identity_for_renderer_inspector_route( diff --git a/moli-protocol/src/domains/emulation.rs b/moli-protocol/src/domains/emulation.rs index 3450a5e38..a96cbdc86 100644 --- a/moli-protocol/src/domains/emulation.rs +++ b/moli-protocol/src/domains/emulation.rs @@ -72,6 +72,7 @@ struct PendingEmulationPageCommand { struct CompletedEmulationPageCommand { target: PendingEmulationPageTarget, operation: PendingEmulationPageOperation, + dispatched_attachment_id: Option, completed: Result, } @@ -80,10 +81,7 @@ enum PendingEmulationPageTarget { SessionOwner { owner_scope: CommandOwnerScope, }, - BrowserContextActive { - browser_context_id: String, - }, - BrowserContextBackground { + BrowserContextTarget { browser_context_id: String, target_id: String, }, @@ -108,6 +106,27 @@ enum PendingEmulationPageOperation { RuntimeProtocolMessage, } +impl PendingEmulationPageOperation { + fn has_authoritative_replay_state(&self) -> bool { + match self { + Self::SetExtraHttpHeaders + | Self::SetLocaleOverride + | Self::SetNetworkConditions + | Self::SetCpuThrottlingRate + | Self::SetTimezoneOverride + | Self::SetEmulatedMedia + | Self::SetViewportSurface + | Self::SetUserAgentLoader + | Self::ReplaceBrowserResourceRuntime + | Self::RuntimeProtocolMessage => true, + // Chromium owns this state on RenderFrameHostImpl::IdleManager. + // It can survive same-site RFH reuse, but it is not target policy + // that may be replayed after an arbitrary attachment replacement. + Self::SetIdleOverride => false, + } + } +} + impl PendingEmulationCommandDispatch { pub(crate) async fn wait(self) -> CompletedEmulationCommandDispatch { let completed = match self.pending { @@ -120,6 +139,7 @@ impl PendingEmulationCommandDispatch { pending, runtime_response_rx, } = pending; + let dispatched_attachment_id = pending.renderer_agent_attachment_id(); let completed_page = pending.wait().await.map_err(|error| error.to_string()); if completed_page.is_ok() && let Some(response_rx) = runtime_response_rx @@ -129,6 +149,7 @@ impl PendingEmulationCommandDispatch { completed.push(CompletedEmulationPageCommand { target, operation, + dispatched_attachment_id, completed: completed_page, }); } @@ -296,7 +317,7 @@ fn start_cpu_throttling_rate_command( )); } let owner_scope = CommandOwnerScope::capture(conn, cmd.session_id); - let Some(page) = loaded_page_mut_for_session(conn, cmd.session_id) else { + let Some(page) = loaded_page_mut_for_target_configuration(conn, cmd.session_id) else { return EmulationCommandTaskStep::Complete(CommandOutputPlan::result(json!({}))); }; match page.start_set_cpu_throttling_rate(params.rate) { @@ -361,7 +382,7 @@ fn start_script_execution_disabled_command( "BrowserContextNotLoaded", )); } - let Some(attachment_id) = loaded_page_mut_for_session(conn, cmd.session_id) + let Some(attachment_id) = loaded_page_mut_for_target_configuration(conn, cmd.session_id) .and_then(|page| page.renderer_agent_attachment_id()) else { return EmulationCommandTaskStep::Complete(CommandOutputPlan::result(json!({}))); @@ -374,7 +395,7 @@ fn start_script_execution_disabled_command( if cmd.id.is_none() || response_delivery == moli_page_types::RendererInspectorResponseDelivery::CommandReply { - let page = loaded_page_mut_for_session(conn, cmd.session_id) + let page = loaded_page_mut_for_target_configuration(conn, cmd.session_id) .expect("the captured Emulation Page must remain loaded synchronously"); let pending = page.start_set_script_execution_disabled_from_io(params.value); return EmulationCommandTaskStep::Pending(PendingEmulationCommandDispatch { @@ -408,7 +429,7 @@ fn start_script_execution_disabled_command( response_rx.is_none(), "Emulation session output must not allocate a command-reply receiver", ); - let pending = loaded_page_mut_for_session(conn, cmd.session_id) + let pending = loaded_page_mut_for_target_configuration(conn, cmd.session_id) .filter(|page| page.renderer_agent_attachment_id() == Some(attachment_id)) .ok_or_else(|| "Emulation renderer attachment changed before IO dispatch".to_owned()) .and_then(|page| { @@ -537,7 +558,7 @@ fn start_update_idle_override_command( return EmulationCommandTaskStep::Complete(CommandOutputPlan::result(json!({}))); } let owner_scope = CommandOwnerScope::capture(conn, cmd.session_id); - let Some(page) = loaded_page_mut_for_session(conn, cmd.session_id) else { + let Some(page) = loaded_page_mut_for_target_configuration(conn, cmd.session_id) else { return EmulationCommandTaskStep::Complete(CommandOutputPlan::result(json!({}))); }; match page.start_set_idle_override(idle_override) { @@ -582,7 +603,7 @@ fn start_timezone_override_command( return EmulationCommandTaskStep::Complete(CommandOutputPlan::error(code, message)); } let owner_scope = CommandOwnerScope::capture(conn, cmd.session_id); - let Some(page) = loaded_page_mut_for_session(conn, cmd.session_id) else { + let Some(page) = loaded_page_mut_for_target_configuration(conn, cmd.session_id) else { return EmulationCommandTaskStep::Complete(CommandOutputPlan::result(json!({}))); }; match page.start_set_timezone_override(timezone_override.as_deref()) { @@ -712,7 +733,7 @@ fn start_emulated_media_command( } } else { let owner_scope = CommandOwnerScope::capture(conn, cmd.session_id); - let Some(page) = loaded_page_mut_for_session(conn, cmd.session_id) else { + let Some(page) = loaded_page_mut_for_target_configuration(conn, cmd.session_id) else { return EmulationCommandTaskStep::Complete(CommandOutputPlan::result(json!({}))); }; match page.start_set_emulated_media(&page_overrides) { @@ -868,7 +889,7 @@ fn start_clear_device_metrics_override_command( } let owner_scope = CommandOwnerScope::capture(conn, cmd.session_id); let runtime_call_id = conn.next_internal_runtime_command_id(); - let Some(page) = loaded_page_mut_for_session(conn, cmd.session_id) else { + let Some(page) = loaded_page_mut_for_target_configuration(conn, cmd.session_id) else { return EmulationCommandTaskStep::Complete(CommandOutputPlan::result(json!({}))); }; let pending_viewport = match page.start_set_viewport_surface(None) { @@ -939,7 +960,7 @@ fn start_devtools_set_viewport_command( } let owner_scope = CommandOwnerScope::capture(conn, session_id); let runtime_call_id = conn.next_internal_runtime_command_id(); - let Some(page) = loaded_page_mut_for_session(conn, session_id) else { + let Some(page) = loaded_page_mut_for_target_configuration(conn, session_id) else { return Ok(None); }; let session_id = session_id.map(str::to_owned); @@ -1225,7 +1246,7 @@ async fn execute_geolocation_surface_updates_for_routes( ) -> Result { let mut pending = Vec::new(); for route in routes { - let target = pending_emulation_target_for_route(&route)?; + let target = pending_emulation_target_for_route(conn, &route)?; let result = { let mut route_scope = conn.scoped_none_session_owner_route_override(route); start_surface_override_for_route(route_scope.conn_mut(), target) @@ -1251,7 +1272,7 @@ fn start_geolocation_override_for_current_route( "BrowserContextNotLoaded".to_owned(), )); } - let target = pending_emulation_target_for_route(route)?; + let target = pending_emulation_target_for_route(conn, route)?; start_surface_override_for_route(conn, target) .map_err(|error| DevToolsError::new(DevToolsErrorKind::Internal, error)) } @@ -1353,18 +1374,19 @@ fn start_network_conditions_update_for_current_route( conn: &mut CdpConnection, route: &CdpSessionRoute, ) -> Result, DevToolsError> { - let target = pending_emulation_target_for_route(route)?; + let target = pending_emulation_target_for_route(conn, route)?; let effective_offline = match &target { - PendingEmulationPageTarget::BrowserContextActive { browser_context_id } => conn - .browser_context_by_id(browser_context_id) - .is_some_and(|browser_context| browser_context.effective_active_network_offline()), - PendingEmulationPageTarget::BrowserContextBackground { + PendingEmulationPageTarget::BrowserContextTarget { browser_context_id, target_id, } => conn .browser_context_by_id(browser_context_id) .is_some_and(|browser_context| { - browser_context.effective_parked_network_offline(target_id) + if browser_context.is_active_target(target_id) { + browser_context.effective_active_network_offline() + } else { + browser_context.effective_parked_network_offline(target_id) + } }), PendingEmulationPageTarget::SessionOwner { .. } => false, }; @@ -1399,7 +1421,7 @@ fn start_extra_headers_for_current_route( route: &CdpSessionRoute, headers: Vec<(String, String)>, ) -> Result, DevToolsError> { - let target = pending_emulation_target_for_route(route)?; + let target = pending_emulation_target_for_route(conn, route)?; let pending = conn .start_set_target_extra_http_headers_for_session_owner(None, headers) .map_err(devtools_emulation_owner_error)?; @@ -1431,17 +1453,20 @@ fn start_extra_headers_update_for_route( conn: &mut CdpConnection, route: &CdpSessionRoute, ) -> Result, DevToolsError> { - let target = pending_emulation_target_for_route(route)?; + let target = pending_emulation_target_for_route(conn, route)?; let headers = match &target { - PendingEmulationPageTarget::BrowserContextActive { browser_context_id } => conn - .browser_context_by_id(browser_context_id) - .map(|browser_context| browser_context.effective_extra_headers()), - PendingEmulationPageTarget::BrowserContextBackground { + PendingEmulationPageTarget::BrowserContextTarget { browser_context_id, target_id, } => conn .browser_context_by_id(browser_context_id) - .map(|browser_context| browser_context.effective_parked_extra_headers(target_id)), + .map(|browser_context| { + if browser_context.is_active_target(target_id) { + browser_context.effective_extra_headers() + } else { + browser_context.effective_parked_extra_headers(target_id) + } + }), PendingEmulationPageTarget::SessionOwner { .. } => None, }; let Some(headers) = headers else { @@ -1466,23 +1491,21 @@ fn loaded_page_mut_for_pending_emulation_target<'a>( target: &PendingEmulationPageTarget, ) -> Option<&'a moli_core::page::Page> { match target { - PendingEmulationPageTarget::BrowserContextActive { browser_context_id } => conn - .browser_context_by_id_mut(browser_context_id) - .and_then(|browser_context| { - browser_context.active_target.runtime_slot.loaded_page_mut() - }) - .map(|page| &*page), - PendingEmulationPageTarget::BrowserContextBackground { + PendingEmulationPageTarget::BrowserContextTarget { browser_context_id, target_id, } => conn .browser_context_by_id_mut(browser_context_id) - .and_then(|browser_context| browser_context.background_target_mut(target_id)) + .and_then(|browser_context| browser_context.page_target_mut(target_id)) .and_then(|target| target.loaded_page_mut()) .map(|page| &*page), - PendingEmulationPageTarget::SessionOwner { owner_scope } => { - loaded_page_mut_for_session(conn, owner_scope.session_id()).map(|page| &*page) - } + PendingEmulationPageTarget::SessionOwner { owner_scope } => conn + .loaded_page_mut_for_target_configuration_for_route( + owner_scope.session_id(), + owner_scope.session_owner_route(), + ) + .ok() + .map(|page| &*page), } } @@ -1651,7 +1674,7 @@ fn start_user_agent_override_for_current_route( route: &CdpSessionRoute, user_agent: Option, ) -> Result, DevToolsError> { - let target = pending_emulation_target_for_route(route)?; + let target = pending_emulation_target_for_route(conn, route)?; let pending = conn .start_set_base_user_agent_override_for_session_owner(None, user_agent) .map_err(devtools_emulation_owner_error)?; @@ -1670,12 +1693,12 @@ fn start_user_agent_loader_update_for_current_route( conn: &mut CdpConnection, route: &CdpSessionRoute, ) -> Result, DevToolsError> { - let target = pending_emulation_target_for_route(route)?; + let target = pending_emulation_target_for_route(conn, route)?; let load_inputs = conn.navigation_load_inputs_for_session_owner(None); let resource_runtime = conn .build_registered_browser_resource_runtime_for_navigation_load_inputs(&load_inputs) .map_err(|error| DevToolsError::new(DevToolsErrorKind::Internal, error))?; - let Some(page) = loaded_page_mut_for_session(conn, None) else { + let Some(page) = loaded_page_mut_for_target_configuration(conn, None) else { return Ok(None); }; let pending = page @@ -1784,11 +1807,11 @@ fn start_locale_update_for_current_route( if let Some(identity_update) = start_user_agent_loader_update_for_current_route(conn, route)? { pending.push(identity_update); } - let target = pending_emulation_target_for_route(route)?; + let target = pending_emulation_target_for_route(conn, route)?; let Some(locale_override) = locale_override_for_session(conn, None) else { return Ok(pending); }; - let Some(page) = loaded_page_mut_for_session(conn, None) else { + let Some(page) = loaded_page_mut_for_target_configuration(conn, None) else { return Ok(pending); }; pending.extend( @@ -1892,9 +1915,9 @@ fn start_timezone_update_for_current_route( conn: &mut CdpConnection, route: &CdpSessionRoute, ) -> Result, DevToolsError> { - let target = pending_emulation_target_for_route(route)?; + let target = pending_emulation_target_for_route(conn, route)?; let load_inputs = conn.navigation_load_inputs_for_session_owner(None); - let Some(page) = loaded_page_mut_for_session(conn, None) else { + let Some(page) = loaded_page_mut_for_target_configuration(conn, None) else { return Ok(None); }; let pending = page @@ -1909,18 +1932,32 @@ fn start_timezone_update_for_current_route( } fn pending_emulation_target_for_route( + conn: &CdpConnection, route: &CdpSessionRoute, ) -> Result { match route { CdpSessionRoute::ActiveTarget { - browser_context_id, .. - } => Ok(PendingEmulationPageTarget::BrowserContextActive { - browser_context_id: browser_context_id.clone(), - }), + browser_context_id, + target_id, + } => { + let target_id = target_id + .clone() + .or_else(|| { + conn.browser_context_by_id(browser_context_id) + .and_then(BrowserContext::active_target_id_owned) + }) + .ok_or_else(|| { + DevToolsError::new(DevToolsErrorKind::NoSuchTarget, "NoSuchTarget") + })?; + Ok(PendingEmulationPageTarget::BrowserContextTarget { + browser_context_id: browser_context_id.clone(), + target_id, + }) + } CdpSessionRoute::PageTargetHost { browser_context_id, target_id, - } => Ok(PendingEmulationPageTarget::BrowserContextBackground { + } => Ok(PendingEmulationPageTarget::BrowserContextTarget { browser_context_id: browser_context_id.clone(), target_id: target_id.clone(), }), @@ -2296,15 +2333,18 @@ fn start_browser_context_default_device_metrics_page_commands( runtime_call_ids: &mut Vec, ) -> Result, DevToolsError> { let browser_context_id = browser_context.id.clone(); + let active_target_id = browser_context.active_target_id_owned(); let mut pending = Vec::new(); let viewport_surface = Some(metrics.viewport_surface().to_page_viewport_surface()); - if let Some(active_target) = browser_context.page_targets.active_mut() + if let Some(active_target_id) = active_target_id + && let Some(active_target) = browser_context.page_targets.active_mut() && active_target.emulated_device_metrics.is_none() && let Some(page) = active_target.runtime_slot.loaded_page_mut() { pending.push(PendingEmulationPageCommand { - target: PendingEmulationPageTarget::BrowserContextActive { + target: PendingEmulationPageTarget::BrowserContextTarget { browser_context_id: browser_context_id.clone(), + target_id: active_target_id.clone(), }, operation: PendingEmulationPageOperation::SetViewportSurface, pending: page @@ -2323,8 +2363,9 @@ fn start_browser_context_default_device_metrics_page_commands( ) .map_err(|error| DevToolsError::new(DevToolsErrorKind::Internal, error))?; pending.push(PendingEmulationPageCommand { - target: PendingEmulationPageTarget::BrowserContextActive { + target: PendingEmulationPageTarget::BrowserContextTarget { browser_context_id: browser_context_id.clone(), + target_id: active_target_id, }, operation: PendingEmulationPageOperation::RuntimeProtocolMessage, pending: pending_runtime, @@ -2350,7 +2391,7 @@ fn start_browser_context_default_device_metrics_page_commands( continue; }; pending.push(PendingEmulationPageCommand { - target: PendingEmulationPageTarget::BrowserContextBackground { + target: PendingEmulationPageTarget::BrowserContextTarget { browser_context_id: browser_context_id.clone(), target_id: target_id.clone(), }, @@ -2371,7 +2412,7 @@ fn start_browser_context_default_device_metrics_page_commands( ) .map_err(|error| DevToolsError::new(DevToolsErrorKind::Internal, error))?; pending.push(PendingEmulationPageCommand { - target: PendingEmulationPageTarget::BrowserContextBackground { + target: PendingEmulationPageTarget::BrowserContextTarget { browser_context_id: browser_context_id.clone(), target_id, }, @@ -2394,16 +2435,30 @@ fn complete_pending_devtools_emulation_command( )); }; for completed_page in completed_pages { - let completion = completed_page - .completed + let CompletedEmulationPageCommand { + target, + operation, + dispatched_attachment_id, + completed, + } = completed_page; + let completion = match completed { + Ok(completion) => completion, + Err(_) + if pending_emulation_page_configuration_will_be_replayed( + conn, + &target, + &operation, + dispatched_attachment_id, + ) => + { + continue; + } + Err(error) => { + return Err(DevToolsError::new(DevToolsErrorKind::Internal, error)); + } + }; + finish_pending_emulation_page_command(conn, operation, target, completion) .map_err(|error| DevToolsError::new(DevToolsErrorKind::Internal, error))?; - finish_pending_emulation_page_command( - conn, - completed_page.operation, - completed_page.target, - completion, - ) - .map_err(|error| DevToolsError::new(DevToolsErrorKind::Internal, error))?; } Ok(DevToolsCommandResult::Empty) } @@ -2494,16 +2549,27 @@ pub(crate) fn complete_pending_emulation_command( } }; for completed_page in completed_pages { - let completion = match completed_page.completed { + let CompletedEmulationPageCommand { + target, + operation, + dispatched_attachment_id, + completed, + } = completed_page; + let completion = match completed { Ok(completion) => completion, + Err(_) + if pending_emulation_page_configuration_will_be_replayed( + conn, + &target, + &operation, + dispatched_attachment_id, + ) => + { + continue; + } Err(error) => return CommandOutputPlan::error(-32000, error), }; - let result = finish_pending_emulation_page_command( - conn, - completed_page.operation, - completed_page.target, - completion, - ); + let result = finish_pending_emulation_page_command(conn, operation, target, completion); if let Err(error) = result { return CommandOutputPlan::error(-32000, error); } @@ -2511,11 +2577,70 @@ pub(crate) fn complete_pending_emulation_command( CommandOutputPlan::result(json!({})) } -fn loaded_page_mut_for_session<'a>( +fn pending_emulation_page_configuration_will_be_replayed( + conn: &CdpConnection, + target: &PendingEmulationPageTarget, + operation: &PendingEmulationPageOperation, + dispatched_attachment_id: Option, +) -> bool { + if !operation.has_authoritative_replay_state() { + return false; + } + let Some(dispatched_attachment_id) = dispatched_attachment_id else { + return false; + }; + let current_attachment_id = match target { + PendingEmulationPageTarget::SessionOwner { owner_scope } => { + let Some((browser_context_id, target_id)) = conn.target_owner_identity_for_route( + owner_scope.session_id(), + owner_scope.session_owner_route(), + ) else { + return false; + }; + let Some(browser_context) = conn.browser_context_by_id(&browser_context_id) else { + return false; + }; + let target = match target_id.as_deref() { + Some(target_id) => browser_context.page_target(target_id), + None => browser_context.page_targets.active(), + }; + let Some(target) = target else { + return false; + }; + target + .loaded_page() + .and_then(moli_core::page::Page::renderer_agent_attachment_id) + } + PendingEmulationPageTarget::BrowserContextTarget { + browser_context_id, + target_id, + } => { + let Some(target) = conn + .browser_context_by_id(browser_context_id) + .and_then(|browser_context| browser_context.page_target(target_id)) + else { + return false; + }; + target + .loaded_page() + .and_then(moli_core::page::Page::renderer_agent_attachment_id) + } + }; + + // Target-configuration operations store their authoritative state before + // dispatch. If that exact target has moved away from the dispatched Page, + // commit configuration either replayed it into the replacement or will do + // so when the in-flight navigation commits. A cancellation from that + // retired renderer is therefore not a protocol failure. + current_attachment_id != Some(dispatched_attachment_id) +} + +fn loaded_page_mut_for_target_configuration<'a>( conn: &'a mut CdpConnection, session_id: Option<&str>, ) -> Option<&'a mut moli_core::page::Page> { - conn.loaded_page_mut_for_protocol_access(session_id).ok() + conn.loaded_page_mut_for_target_configuration(session_id) + .ok() } pub(crate) async fn clear_emulated_media_for_detached_session_async( @@ -2540,7 +2665,7 @@ pub(crate) async fn clear_emulated_media_for_detached_session_async( } let page_overrides: moli_core::page::EmulatedMediaOverrides = (&overrides).into(); - let Some(page) = loaded_page_mut_for_session(conn, Some(session_id)) else { + let Some(page) = loaded_page_mut_for_target_configuration(conn, Some(session_id)) else { return Ok(()); }; page.set_emulated_media_async(&page_overrides) @@ -2586,11 +2711,15 @@ fn start_context_emulated_media_page_commands( return Ok(Vec::new()); }; let browser_context_id = browser_context.id.clone(); + let active_target_id = browser_context.active_target_id_owned(); let mut pending = Vec::new(); - if let Some(page) = browser_context.active_target.runtime_slot.loaded_page_mut() { + if let Some(active_target_id) = active_target_id + && let Some(page) = browser_context.active_target.runtime_slot.loaded_page_mut() + { pending.push(PendingEmulationPageCommand { - target: PendingEmulationPageTarget::BrowserContextActive { + target: PendingEmulationPageTarget::BrowserContextTarget { browser_context_id: browser_context_id.clone(), + target_id: active_target_id, }, operation: PendingEmulationPageOperation::SetEmulatedMedia, pending: page @@ -2605,7 +2734,7 @@ fn start_context_emulated_media_page_commands( continue; }; pending.push(PendingEmulationPageCommand { - target: PendingEmulationPageTarget::BrowserContextBackground { + target: PendingEmulationPageTarget::BrowserContextTarget { browser_context_id: browser_context_id.clone(), target_id, }, @@ -2627,7 +2756,7 @@ fn start_session_locale_override_page_commands( return Ok(Vec::new()); }; let owner_scope = CommandOwnerScope::capture(conn, session_id); - let Some(page) = loaded_page_mut_for_session(conn, session_id) else { + let Some(page) = loaded_page_mut_for_target_configuration(conn, session_id) else { return Ok(Vec::new()); }; start_locale_override_page_command( @@ -2647,11 +2776,15 @@ fn start_context_locale_override_page_commands( .chain(conn.inactive_browser_contexts.iter_mut()) { let browser_context_id = browser_context.id.clone(); + let active_target_id = browser_context.active_target_id_owned(); let active_locale = browser_context.effective_active_locale_override_owned(); - if let Some(page) = browser_context.active_target.runtime_slot.loaded_page_mut() { + if let Some(active_target_id) = active_target_id + && let Some(page) = browser_context.active_target.runtime_slot.loaded_page_mut() + { pending.extend(start_locale_override_page_command( - PendingEmulationPageTarget::BrowserContextActive { + PendingEmulationPageTarget::BrowserContextTarget { browser_context_id: browser_context_id.clone(), + target_id: active_target_id, }, page, active_locale.as_deref(), @@ -2670,7 +2803,7 @@ fn start_context_locale_override_page_commands( continue; }; pending.extend(start_locale_override_page_command( - PendingEmulationPageTarget::BrowserContextBackground { + PendingEmulationPageTarget::BrowserContextTarget { browser_context_id: browser_context_id.clone(), target_id, }, @@ -2697,11 +2830,17 @@ fn start_geolocation_surface_override_page_commands( return Ok(Vec::new()); }; let browser_context_id = browser_context.id.clone(); + let Some(target_id) = browser_context.active_target_id_owned() else { + return Ok(Vec::new()); + }; let Some(page) = browser_context.active_target.runtime_slot.loaded_page_mut() else { return Ok(Vec::new()); }; start_surface_override_page_command( - PendingEmulationPageTarget::BrowserContextActive { browser_context_id }, + PendingEmulationPageTarget::BrowserContextTarget { + browser_context_id, + target_id, + }, page, script, runtime_call_id, @@ -2735,7 +2874,7 @@ fn start_session_surface_override_page_command( }; let owner_scope = CommandOwnerScope::capture(conn, session_id); let runtime_call_id = conn.next_internal_runtime_command_id(); - let Some(page) = loaded_page_mut_for_session(conn, session_id) else { + let Some(page) = loaded_page_mut_for_target_configuration(conn, session_id) else { return Ok(Vec::new()); }; start_surface_override_page_command( @@ -2752,20 +2891,18 @@ fn start_surface_override_for_route( target: PendingEmulationPageTarget, ) -> Result, String> { let script = match &target { - PendingEmulationPageTarget::BrowserContextActive { browser_context_id } => { - let Some(browser_context) = conn.browser_context_by_id(browser_context_id) else { - return Err("BrowserContextNotLoaded".to_owned()); - }; - browser_context.generated_surface_override_script_for_active_target() - } - PendingEmulationPageTarget::BrowserContextBackground { + PendingEmulationPageTarget::BrowserContextTarget { browser_context_id, target_id, } => { let Some(browser_context) = conn.browser_context_by_id(browser_context_id) else { return Err("BrowserContextNotLoaded".to_owned()); }; - browser_context.generated_surface_override_script_for_parked_target(target_id) + if browser_context.is_active_target(target_id) { + browser_context.generated_surface_override_script_for_active_target() + } else { + browser_context.generated_surface_override_script_for_parked_target(target_id) + } } PendingEmulationPageTarget::SessionOwner { owner_scope } => { return start_session_surface_override_page_command(conn, owner_scope.session_id()); @@ -2775,7 +2912,7 @@ fn start_surface_override_for_route( return Ok(Vec::new()); }; let runtime_call_id = conn.next_internal_runtime_command_id(); - let Some(page) = loaded_page_mut_for_session(conn, None) else { + let Some(page) = loaded_page_mut_for_target_configuration(conn, None) else { return Ok(Vec::new()); }; start_surface_override_page_command(target, page, script, runtime_call_id) @@ -2844,28 +2981,20 @@ fn finish_pending_emulation_page_command( ); } let page = conn - .loaded_page_mut_for_interruptible_protocol_access_for_route( + .loaded_page_mut_for_target_configuration_for_route( owner_scope.session_id(), owner_scope.session_owner_route(), ) .ok(); finish_emulation_page_operation_on_current_attachment(page, operation, completion) } - PendingEmulationPageTarget::BrowserContextActive { browser_context_id } => { - let page = conn - .browser_context_by_id_mut(&browser_context_id) - .and_then(|browser_context| { - browser_context.active_target.runtime_slot.loaded_page_mut() - }); - finish_emulation_page_operation_on_current_attachment(page, operation, completion) - } - PendingEmulationPageTarget::BrowserContextBackground { + PendingEmulationPageTarget::BrowserContextTarget { browser_context_id, target_id, } => { let page = conn .browser_context_by_id_mut(&browser_context_id) - .and_then(|browser_context| browser_context.background_target_mut(&target_id)) + .and_then(|browser_context| browser_context.page_target_mut(&target_id)) .and_then(|target| target.loaded_page_mut()); finish_emulation_page_operation_on_current_attachment(page, operation, completion) } @@ -2886,10 +3015,9 @@ fn finish_emulation_page_operation_on_current_attachment( // The renderer command has already settled successfully. A cross-Document // navigation may replace its Page before the protocol actor decodes that - // frozen completion; do not turn that success into NoDocumentLoaded or - // apply the old PageState snapshot to the replacement attachment. The - // authoritative emulation state was stored before dispatch and is replayed - // while the replacement Page is installed. + // frozen completion; decode the terminal reply, but never apply the old + // PageState snapshot to the replacement attachment. Whether state carries + // across the navigation is decided separately at the commit boundary. let output = match operation { PendingEmulationPageOperation::RuntimeProtocolMessage => { completion.into_runtime_protocol_message_command_turn() diff --git a/moli-protocol/src/domains/emulation/tests.rs b/moli-protocol/src/domains/emulation/tests.rs index 161da1dfd..911ab7a22 100644 --- a/moli-protocol/src/domains/emulation/tests.rs +++ b/moli-protocol/src/domains/emulation/tests.rs @@ -361,6 +361,209 @@ async fn emulated_media_can_complete_through_pending_command_dispatch() { assert_eq!(media.color_scheme.as_deref(), Some("dark")); } +#[tokio::test(flavor = "multi_thread")] +async fn pending_emulation_completion_follows_the_exact_target_across_activation_and_navigation() { + let mut ctx = TestContext::new(); + load_session_page_for_pending_emulation_test(&mut ctx).await; + let dispatched_attachment_id = ctx + .conn + .browser_context + .as_ref() + .and_then(|browser_context| browser_context.page_target("TID-1")) + .and_then(PageTargetHost::loaded_page) + .and_then(moli_core::page::Page::renderer_agent_attachment_id) + .expect("the original target should have a renderer attachment"); + let target = super::PendingEmulationPageTarget::BrowserContextTarget { + browser_context_id: "BID-1".to_owned(), + target_id: "TID-1".to_owned(), + }; + + let browser_context = ctx.conn.browser_context.as_mut().unwrap(); + assert!( + browser_context.insert_page_target_host(PageTargetHost::with_url( + "TID-2".to_owned(), + None, + "about:blank".to_owned(), + )) + ); + browser_context.set_active_target_id("TID-2"); + assert!( + !super::pending_emulation_page_configuration_will_be_replayed( + &ctx.conn, + &target, + &super::PendingEmulationPageOperation::SetTimezoneOverride, + Some(dispatched_attachment_id), + ), + "changing foreground selection must not make an error from the same Page look stale" + ); + + ctx.process_async(json!({ + "id": 9_104, + "sessionId": "SID-1", + "method": "Page.navigate", + "params": { "url": "data:text/html,replacement" } + })) + .await; + assert!(ctx.take_response_by_id(9_104)["result"]["loaderId"].is_string()); + assert!( + super::pending_emulation_page_configuration_will_be_replayed( + &ctx.conn, + &target, + &super::PendingEmulationPageOperation::SetTimezoneOverride, + Some(dispatched_attachment_id), + ), + "only replacement of the exact target attachment may retire its renderer error" + ); + assert!( + !super::pending_emulation_page_configuration_will_be_replayed( + &ctx.conn, + &target, + &super::PendingEmulationPageOperation::SetIdleOverride, + Some(dispatched_attachment_id), + ), + "frame-host idle state must not use the target-policy replay path", + ); + + let result = super::complete_pending_devtools_emulation_command( + &mut ctx.conn, + super::CompletedEmulationCommandDispatch { + command_id: None, + session_id: Some("SID-1".to_owned()), + completed: super::CompletedEmulationRendererDispatch::Pages(vec![ + super::CompletedEmulationPageCommand { + target, + operation: super::PendingEmulationPageOperation::SetTimezoneOverride, + dispatched_attachment_id: Some(dispatched_attachment_id), + completed: Err("renderer attachment retired".to_owned()), + }, + ]), + }, + ) + .expect("the stored target policy should be replayed into the replacement document"); + assert!(matches!(result, DevToolsCommandResult::Empty)); +} + +#[tokio::test(flavor = "multi_thread")] +async fn pending_idle_override_response_does_not_replay_into_replacement_page() { + let mut ctx = TestContext::new(); + load_session_page_for_pending_emulation_test(&mut ctx).await; + + let raw = json!({ + "id": 9_105, + "sessionId": "SID-1", + "method": "Emulation.setIdleOverride", + "params": { "isUserActive": false, "isScreenUnlocked": false } + }) + .to_string(); + let CdpCommandTaskStep::Pending(pending) = ctx.conn.start_command_dispatch(&raw) else { + panic!("the loaded Page should receive the idle override command"); + }; + let completed = pending.wait().await; + + ctx.process_async(json!({ + "id": 9_106, + "sessionId": "SID-1", + "method": "Page.navigate", + "params": { "url": "data:text/html,replacement" } + })) + .await; + assert!(ctx.take_response_by_id(9_106)["result"]["loaderId"].is_string()); + + let CdpCommandTaskStep::Complete(outcome) = + ctx.conn.complete_pending_command_dispatch(completed).await + else { + panic!("the retired idle override should settle in one protocol phase"); + }; + assert!(outcome.into_parts().0.iter().any(|message| { + message["id"] == json!(9_105) + && message["sessionId"] == json!("SID-1") + && message["result"] == json!({}) + })); + let page = ctx + .conn + .browser_context + .as_ref() + .and_then(|context| context.page_target("TID-1")) + .and_then(PageTargetHost::loaded_page) + .expect("replacement Page"); + assert_eq!( + page.idle_override(), + None, + "a settled command on the retired frame host must not become target-level policy", + ); +} + +#[tokio::test(flavor = "multi_thread")] +async fn admitted_idle_override_is_visible_to_concurrent_same_site_navigation() { + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let address = listener.local_addr().unwrap(); + let server = tokio::spawn(async move { + axum::serve( + listener, + Router::new().route( + "/", + get(|| async { "idle navigation" }), + ), + ) + .await + .unwrap(); + }); + let mut ctx = TestContext::new(); + load_session_page_for_pending_emulation_test_at_url( + &mut ctx, + &format!("http://{address}/?initial"), + ) + .await; + + let raw = json!({ + "id": 9_107, + "sessionId": "SID-1", + "method": "Emulation.setIdleOverride", + "params": { "isUserActive": false, "isScreenUnlocked": false } + }) + .to_string(); + let CdpCommandTaskStep::Pending(pending) = ctx.conn.start_command_dispatch(&raw) else { + panic!("the loaded Page should receive the idle override command"); + }; + let completed = pending.wait().await; + + ctx.process_async(json!({ + "id": 9_108, + "sessionId": "SID-1", + "method": "Page.navigate", + "params": { "url": format!("http://{address}/?replacement") } + })) + .await; + assert!(ctx.take_response_by_id(9_108)["result"]["loaderId"].is_string()); + + let CdpCommandTaskStep::Complete(outcome) = + ctx.conn.complete_pending_command_dispatch(completed).await + else { + panic!("the retired idle override should settle in one protocol phase"); + }; + assert!(outcome.into_parts().0.iter().any(|message| { + message["id"] == json!(9_107) + && message["sessionId"] == json!("SID-1") + && message["result"] == json!({}) + })); + let page = ctx + .conn + .browser_context + .as_ref() + .and_then(|context| context.page_target("TID-1")) + .and_then(PageTargetHost::loaded_page) + .expect("replacement Page"); + assert_eq!( + page.idle_override(), + Some(moli_core::page::EmulatedIdleOverride { + is_user_active: false, + is_screen_unlocked: false, + }), + "same-site commit must read admitted state from the outgoing document handle", + ); + server.abort(); +} + #[tokio::test(flavor = "multi_thread")] async fn idle_override_updates_idle_detector_and_clear_restores_actual_state() { let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); @@ -463,7 +666,6 @@ async fn idle_override_updates_idle_detector_and_clear_restores_actual_state() { && message["sessionId"] == json!("SID-1") && message["result"] == json!({}) })); - let page = ctx .conn .browser_context @@ -501,14 +703,18 @@ async fn idle_override_updates_idle_detector_and_clear_restores_actual_state() { })); ctx.conn - .start_document_navigation_for_session_owner(Some("SID-1"), "LID-idle-same-site".to_owned()) - .expect("same-site navigation should enter the pending state"); + .start_document_navigation_for_session_owner( + Some("SID-1"), + "LID-idle-cross-document".to_owned(), + ) + .expect("cross-Document navigation should enter the pending state"); let configuration = ctx .conn .prepared_document_commit_configuration_for_session_owner( Some("SID-1"), &url::Url::parse("http://127.0.0.1:65530/same-site-different-origin").unwrap(), - ); + ) + .expect("commit configuration should resolve the target resource runtime"); assert_eq!( configuration.idle_override, Some(moli_core::page::EmulatedIdleOverride { @@ -516,6 +722,17 @@ async fn idle_override_updates_idle_detector_and_clear_restores_actual_state() { is_screen_unlocked: false, }) ); + let cross_site_configuration = ctx + .conn + .prepared_document_commit_configuration_for_session_owner( + Some("SID-1"), + &url::Url::parse("http://idle-override-cross-site.test/").unwrap(), + ) + .expect("cross-site commit configuration should resolve the target resource runtime"); + assert_eq!( + cross_site_configuration.idle_override, None, + "a cross-site renderer replacement must not inherit frame-host idle state", + ); server.abort(); } @@ -909,6 +1126,18 @@ async fn multi_session_browser_identity_uses_attachment_order_and_field_contribu .user_agent_override(), Some("Moli/Aux-1") ); + assert_eq!( + ctx.conn + .browser_context + .as_ref() + .expect("browser context") + .active_page_state() + .effective_renderer_browser_identity_override_owned() + .expect("renderer identity") + .user_agent(), + "Moli/Primary-2", + "renderer agents use first-enable order rather than browser attachment order" + ); expect_session_command_result( &mut ctx, diff --git a/moli-protocol/src/domains/fetch.rs b/moli-protocol/src/domains/fetch.rs index c1ae5a9af..9af70453d 100644 --- a/moli-protocol/src/domains/fetch.rs +++ b/moli-protocol/src/domains/fetch.rs @@ -248,7 +248,9 @@ impl PendingFetchCommandDispatch { | CdpSessionRoute::DedicatedWorkerTarget { .. } | CdpSessionRoute::ServiceWorkerTarget { .. }, ) => None, - None if session_id.is_none() => conn.none_session_owner_route_override(), + None if session_id.is_none() => CommandOwnerScope::capture(conn, None) + .session_owner_route() + .cloned(), None => None, }; Self { diff --git a/moli-protocol/src/domains/fetch/commands.rs b/moli-protocol/src/domains/fetch/commands.rs index 7d0fe76c1..95f51008d 100644 --- a/moli-protocol/src/domains/fetch/commands.rs +++ b/moli-protocol/src/domains/fetch/commands.rs @@ -1892,10 +1892,10 @@ fn continue_streaming_document_response_in_background( prepared_document, } = pending; let session_id = navigation.navigate_session_id.clone(); - let none_session_owner_route = session_id - .is_none() - .then(|| conn.none_session_owner_route_override()) - .flatten(); + let none_session_owner_route = + crate::conn::CommandOwnerScope::capture(conn, session_id.as_deref()) + .session_owner_route() + .cloned(); let cancellation = response.cancellation_handle(); if response_code.is_none() && response_headers.is_empty() diff --git a/moli-protocol/src/domains/fetch/tests/command_correlation.rs b/moli-protocol/src/domains/fetch/tests/command_correlation.rs index 33c5ad300..93a088f39 100644 --- a/moli-protocol/src/domains/fetch/tests/command_correlation.rs +++ b/moli-protocol/src/domains/fetch/tests/command_correlation.rs @@ -182,6 +182,44 @@ async fn deferred_fetch_command_keeps_its_exact_page_for_implicit_work() { ); } +#[tokio::test(flavor = "multi_thread")] +async fn deferred_sessionless_fetch_command_freezes_the_active_page_at_admission() { + let mut ctx = TestContext::new(); + let mut browser_context = BrowserContext::new("BID-sessionless".to_owned()); + browser_context.set_active_target_id("TID-original".to_owned()); + browser_context.insert_page_target_host(PageTargetHost::with_url( + "TID-next".to_owned(), + None, + "https://example.test/next".to_owned(), + )); + ctx.conn.browser_context = Some(browser_context); + + let completed = PendingFetchCommandDispatch::new( + &ctx.conn, + Some(71), + None, + PendingFetchCommandKind::GetResponseBody, + PendingFetchCommandOperation::Ready, + ) + .wait() + .await; + ctx.conn + .browser_context + .as_mut() + .expect("browser context") + .set_active_target_id("TID-next"); + let mut scope = completed.owner_scope.enter(&mut ctx.conn); + + assert_eq!( + scope.conn_mut().target_owner_identity_for_session(None), + Some(( + "BID-sessionless".to_owned(), + Some("TID-original".to_owned()) + )), + "a deferred root Fetch command must not follow a later foreground selection" + ); +} + #[tokio::test(flavor = "multi_thread")] async fn continue_request_registers_correlation_before_renderer_completion() { let mut ctx = context_with_loaded_fetch_page().await; diff --git a/moli-protocol/src/domains/fetch/tests/runtime_auth_response.rs b/moli-protocol/src/domains/fetch/tests/runtime_auth_response.rs index 007e5ce69..8f70d1758 100644 --- a/moli-protocol/src/domains/fetch/tests/runtime_auth_response.rs +++ b/moli-protocol/src/domains/fetch/tests/runtime_auth_response.rs @@ -5025,7 +5025,8 @@ async fn stop_loading_aborts_paused_response_stage_runtime_xhr_subresource() { .cloned() .expect("network loadingFailed event"); assert_eq!(failed["params"]["type"], "XHR"); - assert_eq!(failed["params"]["errorText"], "Navigation stopped"); + assert_eq!(failed["params"]["errorText"], "net::ERR_ABORTED"); + assert_eq!(failed["params"]["canceled"], true); ctx.process_async(json!({ "id": 792, @@ -5437,7 +5438,8 @@ async fn stop_loading_aborts_paused_runtime_xhr_auth_subresource() { .cloned() .expect("network loadingFailed event"); assert_eq!(failed["params"]["type"], "XHR"); - assert_eq!(failed["params"]["errorText"], "Navigation stopped"); + assert_eq!(failed["params"]["errorText"], "net::ERR_ABORTED"); + assert_eq!(failed["params"]["canceled"], true); ctx.process_async(json!({ "id": 7937, diff --git a/moli-protocol/src/domains/fetch/tests/runtime_fetch.rs b/moli-protocol/src/domains/fetch/tests/runtime_fetch.rs index 596b3ecf2..4b867183d 100644 --- a/moli-protocol/src/domains/fetch/tests/runtime_fetch.rs +++ b/moli-protocol/src/domains/fetch/tests/runtime_fetch.rs @@ -7687,7 +7687,8 @@ async fn stop_loading_aborts_paused_runtime_fetch_subresource() { .cloned() .expect("network loadingFailed event"); assert_eq!(failed["params"]["type"], "Fetch"); - assert_eq!(failed["params"]["errorText"], "Navigation stopped"); + assert_eq!(failed["params"]["errorText"], "net::ERR_ABORTED"); + assert_eq!(failed["params"]["canceled"], true); ctx.process_async(json!({ "id": 785, diff --git a/moli-protocol/src/domains/network.rs b/moli-protocol/src/domains/network.rs index aca37a6a0..690ba9d26 100644 --- a/moli-protocol/src/domains/network.rs +++ b/moli-protocol/src/domains/network.rs @@ -1,4 +1,4 @@ -use crate::conn::{CdpConnection, Cmd}; +use crate::conn::{CdpConnection, Cmd, CommandOwnerScope}; use crate::devtools_runtime::{ DevToolsAddNetworkDataCollectorCommand, DevToolsBrowserContextId, DevToolsCommand, DevToolsCommandResult, DevToolsError, DevToolsErrorKind, @@ -122,28 +122,43 @@ use settings::clear_browser_cache_command_output_plan; pub(crate) struct PendingNetworkCommandDispatch { command_id: Option, - session_id: Option, + owner_scope: CommandOwnerScope, kind: PendingNetworkCommandKind, pending: PendingNetworkCommandWork, } pub(crate) struct CompletedNetworkCommandDispatch { command_id: Option, - session_id: Option, + owner_scope: CommandOwnerScope, kind: PendingNetworkCommandKind, completed: CompletedNetworkCommandWork, } enum PendingNetworkCommandWork { - Page(moli_core::page::PendingPageCommand), + Page { + attachment_id: Option, + pending: moli_core::page::PendingPageCommand, + }, Resource(Box), } enum CompletedNetworkCommandWork { - Page(Result, String>), + Page { + attachment_id: Option, + completed: Result, String>, + }, Resource(moli_core::page::RendererNetworkResourceLoadOutcome), } +impl PendingNetworkCommandWork { + fn page(pending: moli_core::page::PendingPageCommand) -> Self { + Self::Page { + attachment_id: pending.renderer_agent_attachment_id(), + pending, + } + } +} + pub(crate) enum NetworkCommandTaskStep { Pending(PendingNetworkCommandDispatch), Complete(CommandOutputPlan), @@ -164,25 +179,29 @@ enum PendingNetworkCommandKind { impl PendingNetworkCommandDispatch { pub(crate) fn session_id(&self) -> Option<&str> { - self.session_id.as_deref() + self.owner_scope.session_id() } pub(crate) async fn wait(self) -> CompletedNetworkCommandDispatch { let completed = match self.pending { - PendingNetworkCommandWork::Page(pending) => CompletedNetworkCommandWork::Page( - pending + PendingNetworkCommandWork::Page { + attachment_id, + pending, + } => CompletedNetworkCommandWork::Page { + attachment_id, + completed: pending .wait() .await .map(Box::new) .map_err(|error| error.to_string()), - ), + }, PendingNetworkCommandWork::Resource(pending) => { CompletedNetworkCommandWork::Resource((*pending).execute().await) } }; CompletedNetworkCommandDispatch { command_id: self.command_id, - session_id: self.session_id, + owner_scope: self.owner_scope, kind: self.kind, completed, } @@ -195,7 +214,7 @@ impl CompletedNetworkCommandDispatch { } pub(crate) fn session_id(&self) -> Option<&str> { - self.session_id.as_deref() + self.owner_scope.session_id() } } @@ -449,17 +468,22 @@ fn validate_top_level_target_ids( } fn pending_network_page_command_step( + conn: &mut CdpConnection, command_id: Option, session_id: Option<&str>, kind: PendingNetworkCommandKind, - result: Result, String>, + start: impl FnOnce( + &mut CdpConnection, + ) -> Result, String>, ) -> NetworkCommandTaskStep { + let owner_scope = CommandOwnerScope::capture(conn, session_id); + let result = start(conn); match result { Ok(Some(pending)) => NetworkCommandTaskStep::Pending(PendingNetworkCommandDispatch { command_id, - session_id: session_id.map(str::to_owned), + owner_scope, kind, - pending: PendingNetworkCommandWork::Page(pending), + pending: PendingNetworkCommandWork::page(pending), }), Ok(None) => NetworkCommandTaskStep::Complete(CommandOutputPlan::success()), Err(message) if message == "BrowserContextNotLoaded" => NetworkCommandTaskStep::Complete( @@ -491,12 +515,13 @@ fn start_set_network_domain_enabled_command( } else { PendingNetworkCommandKind::Disable }; + let owner_scope = CommandOwnerScope::capture(conn, cmd.session_id); match conn.start_replay_effective_network_request_policy_for_session_owner(cmd.session_id) { Ok(Some(pending)) => NetworkCommandTaskStep::Pending(PendingNetworkCommandDispatch { command_id: cmd.id, - session_id: cmd.session_id.map(str::to_owned), + owner_scope, kind, - pending: PendingNetworkCommandWork::Page(pending), + pending: PendingNetworkCommandWork::page(pending), }), Ok(None) => NetworkCommandTaskStep::Complete(if enabled { settings::enabled_command_output_plan(conn, cmd.session_id) @@ -516,10 +541,11 @@ fn start_set_extra_http_headers_command( Err(plan) => return NetworkCommandTaskStep::Complete(plan), }; pending_network_page_command_step( + conn, cmd.id, cmd.session_id, PendingNetworkCommandKind::SetExtraHttpHeaders, - conn.start_set_extra_http_headers_for_session_owner(cmd.session_id, headers), + |conn| conn.start_set_extra_http_headers_for_session_owner(cmd.session_id, headers), ) } @@ -532,10 +558,11 @@ fn start_set_cache_disabled_command( Err(plan) => return NetworkCommandTaskStep::Complete(plan), }; pending_network_page_command_step( + conn, cmd.id, cmd.session_id, PendingNetworkCommandKind::SetCacheDisabled, - conn.start_set_cache_disabled_for_session_owner(cmd.session_id, cache_disabled), + |conn| conn.start_set_cache_disabled_for_session_owner(cmd.session_id, cache_disabled), ) } @@ -548,10 +575,11 @@ fn start_set_blocked_urls_command( Err(plan) => return NetworkCommandTaskStep::Complete(plan), }; pending_network_page_command_step( + conn, cmd.id, cmd.session_id, PendingNetworkCommandKind::SetBlockedUrls, - conn.start_set_blocked_url_patterns_for_session_owner(cmd.session_id, patterns), + |conn| conn.start_set_blocked_url_patterns_for_session_owner(cmd.session_id, patterns), ) } @@ -564,10 +592,11 @@ fn start_set_bypass_service_worker_command( Err(plan) => return NetworkCommandTaskStep::Complete(plan), }; pending_network_page_command_step( + conn, cmd.id, cmd.session_id, PendingNetworkCommandKind::SetBypassServiceWorker, - conn.start_set_bypass_service_worker_for_session_owner(cmd.session_id, bypass), + |conn| conn.start_set_bypass_service_worker_for_session_owner(cmd.session_id, bypass), ) } @@ -580,17 +609,20 @@ fn start_emulate_network_conditions_command( Err(plan) => return NetworkCommandTaskStep::Complete(plan), }; pending_network_page_command_step( + conn, cmd.id, cmd.session_id, PendingNetworkCommandKind::EmulateNetworkConditions, - conn.start_set_emulated_network_conditions_for_session_owner( - cmd.session_id, - conditions.offline, - conditions.latency, - conditions.download_throughput, - conditions.upload_throughput, - conditions.connection_type, - ), + |conn| { + conn.start_set_emulated_network_conditions_for_session_owner( + cmd.session_id, + conditions.offline, + conditions.latency, + conditions.download_throughput, + conditions.upload_throughput, + conditions.connection_type, + ) + }, ) } @@ -604,13 +636,16 @@ fn start_set_user_agent_override_command( Err(plan) => return NetworkCommandTaskStep::Complete(plan), }; pending_network_page_command_step( + conn, cmd.id, cmd.session_id, PendingNetworkCommandKind::SetUserAgentOverride, - conn.start_set_devtools_browser_identity_override_for_session_owner( - cmd.session_id, - browser_identity, - ), + |conn| { + conn.start_set_devtools_browser_identity_override_for_session_owner( + cmd.session_id, + browser_identity, + ) + }, ) } @@ -670,6 +705,7 @@ pub(crate) fn complete_pending_network_command( #[derive(Clone, Copy)] enum NetworkPageCommandFinish { + RequestPolicy, ExtraHttpHeaders, BlockedUrls, BypassServiceWorker, @@ -681,31 +717,43 @@ fn complete_network_policy_refresh( completed: CompletedNetworkCommandDispatch, enabled: bool, ) -> CommandOutputPlan { + let owner_scope = completed.owner_scope.clone(); + let session_id = owner_scope.session_id().map(str::to_owned); let completion = match completed.completed { - CompletedNetworkCommandWork::Page(Ok(completion)) => *completion, - CompletedNetworkCommandWork::Page(Err(error)) => { + CompletedNetworkCommandWork::Page { + completed: Ok(completion), + .. + } => *completion, + CompletedNetworkCommandWork::Page { + attachment_id, + completed: Err(error), + } => { + if network_page_configuration_will_be_replayed(conn, &owner_scope, attachment_id) { + return if enabled { + settings::enabled_command_output_plan(conn, session_id.as_deref()) + } else { + CommandOutputPlan::success() + }; + } return CommandOutputPlan::error(-32000, error); } CompletedNetworkCommandWork::Resource(_) => { return CommandOutputPlan::error(-32000, "InvalidNetworkCommandCompletion"); } }; - let page = match conn.loaded_page_mut_for_protocol_access(completed.session_id.as_deref()) { - Ok(page) => page, - Err(message) if message == "NoDocumentLoaded" => { - return if enabled { - settings::enabled_command_output_plan(conn, completed.session_id.as_deref()) - } else { - CommandOutputPlan::success() - }; - } - Err(message) => return CommandOutputPlan::error(-32000, message), - }; - if let Err(error) = page.finish_set_network_request_policy(completion) { - return CommandOutputPlan::error(-32000, error.to_string()); + if let Err(error) = finish_network_page_operation_on_current_attachment( + conn.loaded_page_mut_for_target_configuration_for_route( + owner_scope.session_id(), + owner_scope.session_owner_route(), + ) + .ok(), + NetworkPageCommandFinish::RequestPolicy, + completion, + ) { + return CommandOutputPlan::error(-32000, error); } if enabled { - settings::enabled_command_output_plan(conn, completed.session_id.as_deref()) + settings::enabled_command_output_plan(conn, owner_scope.session_id()) } else { CommandOutputPlan::success() } @@ -716,53 +764,103 @@ fn complete_unit_page_network_command( completed: CompletedNetworkCommandDispatch, finish: NetworkPageCommandFinish, ) -> CommandOutputPlan { + let owner_scope = completed.owner_scope.clone(); let completion = match completed.completed { - CompletedNetworkCommandWork::Page(Ok(completion)) => *completion, - CompletedNetworkCommandWork::Page(Err(error)) => { + CompletedNetworkCommandWork::Page { + completed: Ok(completion), + .. + } => *completion, + CompletedNetworkCommandWork::Page { + attachment_id, + completed: Err(error), + } => { + if network_page_configuration_will_be_replayed(conn, &owner_scope, attachment_id) { + return CommandOutputPlan::success(); + } return CommandOutputPlan::error(-32000, error); } CompletedNetworkCommandWork::Resource(_) => { return CommandOutputPlan::error(-32000, "InvalidNetworkCommandCompletion"); } }; - let page = match conn.loaded_page_mut_for_protocol_access(completed.session_id.as_deref()) { - Ok(page) => page, - Err(message) if message == "NoDocumentLoaded" => { - return CommandOutputPlan::success(); - } - Err(message) => return CommandOutputPlan::error(-32000, message), - }; - let result = match finish { - NetworkPageCommandFinish::ExtraHttpHeaders => { - page.finish_set_extra_http_headers(completion) - } - NetworkPageCommandFinish::BlockedUrls => page.finish_set_blocked_url_patterns(completion), - NetworkPageCommandFinish::BypassServiceWorker => { - page.finish_set_bypass_service_worker(completion) - } - NetworkPageCommandFinish::NetworkOffline => page.finish_set_network_offline(completion), - }; - match result { + match finish_network_page_operation_on_current_attachment( + conn.loaded_page_mut_for_target_configuration_for_route( + owner_scope.session_id(), + owner_scope.session_owner_route(), + ) + .ok(), + finish, + completion, + ) { Ok(()) => CommandOutputPlan::success(), - Err(error) => CommandOutputPlan::error(-32000, error.to_string()), + Err(error) => CommandOutputPlan::error(-32000, error), } } +fn finish_network_page_operation_on_current_attachment( + page: Option<&mut moli_core::page::Page>, + finish: NetworkPageCommandFinish, + completion: moli_core::page::CompletedPageCommand, +) -> Result<(), String> { + let completion_attachment = completion.renderer_agent_attachment_id(); + if let Some(page) = page + && page.renderer_agent_attachment_id() == completion_attachment + { + let result = match finish { + NetworkPageCommandFinish::RequestPolicy => { + page.finish_set_network_request_policy(completion) + } + NetworkPageCommandFinish::ExtraHttpHeaders => { + page.finish_set_extra_http_headers(completion) + } + NetworkPageCommandFinish::BlockedUrls => { + page.finish_set_blocked_url_patterns(completion) + } + NetworkPageCommandFinish::BypassServiceWorker => { + page.finish_set_bypass_service_worker(completion) + } + NetworkPageCommandFinish::NetworkOffline => page.finish_set_network_offline(completion), + }; + return result.map_err(|error| error.to_string()); + } + + // Target/session policy was committed before renderer dispatch. If a + // navigation installs another attachment before this frozen unit reply is + // decoded, consume the old turn without applying its PageState snapshot + // to the replacement. Prepared-document commit configuration carries the + // authoritative policy into that replacement. + completion + .into_unit_page_command_turn() + .map(drop) + .map_err(|error| format!("stale Network command returned an unexpected reply: {error}")) +} + fn complete_rebuild_loader_network_command( conn: &mut CdpConnection, completed: CompletedNetworkCommandDispatch, ) -> CommandOutputPlan { + let owner_scope = completed.owner_scope.clone(); let completion = match completed.completed { - CompletedNetworkCommandWork::Page(Ok(completion)) => *completion, - CompletedNetworkCommandWork::Page(Err(error)) => { + CompletedNetworkCommandWork::Page { + completed: Ok(completion), + .. + } => *completion, + CompletedNetworkCommandWork::Page { + attachment_id, + completed: Err(error), + } => { + if network_page_configuration_will_be_replayed(conn, &owner_scope, attachment_id) { + return CommandOutputPlan::success(); + } return CommandOutputPlan::error(-32000, error); } CompletedNetworkCommandWork::Resource(_) => { return CommandOutputPlan::error(-32000, "InvalidNetworkCommandCompletion"); } }; - match conn.finish_rebuild_resource_runtime_for_session_owner( - completed.session_id.as_deref(), + match conn.finish_rebuild_resource_runtime_for_route( + owner_scope.session_id(), + owner_scope.session_owner_route(), completion, ) { Ok(()) => CommandOutputPlan::success(), @@ -770,6 +868,30 @@ fn complete_rebuild_loader_network_command( } } +fn network_page_configuration_will_be_replayed( + conn: &mut CdpConnection, + owner_scope: &CommandOwnerScope, + dispatched_attachment: Option, +) -> bool { + let Some(dispatched_attachment) = dispatched_attachment else { + return false; + }; + let owner_still_exists = conn + .target_owner_identity_for_route( + owner_scope.session_id(), + owner_scope.session_owner_route(), + ) + .is_some(); + let current_attachment = conn + .loaded_page_mut_for_target_configuration_for_route( + owner_scope.session_id(), + owner_scope.session_owner_route(), + ) + .ok() + .and_then(|page| page.renderer_agent_attachment_id()); + owner_still_exists && current_attachment != Some(dispatched_attachment) +} + #[cfg(test)] mod status_text_tests { use super::http_status_text; diff --git a/moli-protocol/src/domains/network/load_resource.rs b/moli-protocol/src/domains/network/load_resource.rs index cef6154e7..369edcd48 100644 --- a/moli-protocol/src/domains/network/load_resource.rs +++ b/moli-protocol/src/domains/network/load_resource.rs @@ -4,7 +4,7 @@ use serde_json::{Map, Value, json}; use url::Url; use crate::{ - conn::{CapturedBody, CdpConnection, Cmd}, + conn::{CapturedBody, CdpConnection, Cmd, CommandOwnerScope}, domains::command_output::CommandOutputPlan, }; @@ -51,6 +51,7 @@ pub(super) fn start_load_network_resource_command( "Parameter frameId must be provided for frame targets", )); }; + let owner_scope = CommandOwnerScope::capture(conn, cmd.session_id); let pending = match conn.loaded_page_mut_for_protocol_access(cmd.session_id) { Ok(page) => page.start_prepare_network_resource_load( frame_id, @@ -71,9 +72,9 @@ pub(super) fn start_load_network_resource_command( match pending { Ok(pending) => NetworkCommandTaskStep::Pending(PendingNetworkCommandDispatch { command_id: cmd.id, - session_id: cmd.session_id.map(str::to_owned), + owner_scope, kind: PendingNetworkCommandKind::PrepareNetworkResourceLoad, - pending: PendingNetworkCommandWork::Page(pending), + pending: PendingNetworkCommandWork::page(pending), }), Err(error) => { NetworkCommandTaskStep::Complete(CommandOutputPlan::error(-32000, error.to_string())) @@ -85,9 +86,16 @@ pub(super) fn complete_network_resource_preparation( conn: &mut CdpConnection, completed: CompletedNetworkCommandDispatch, ) -> NetworkCommandTaskStep { + let owner_scope = completed.owner_scope.clone(); let completion = match completed.completed { - CompletedNetworkCommandWork::Page(Ok(completion)) => *completion, - CompletedNetworkCommandWork::Page(Err(error)) => { + CompletedNetworkCommandWork::Page { + completed: Ok(completion), + .. + } => *completion, + CompletedNetworkCommandWork::Page { + completed: Err(error), + .. + } => { return NetworkCommandTaskStep::Complete(CommandOutputPlan::error(-32000, error)); } CompletedNetworkCommandWork::Resource(_) => { @@ -95,8 +103,14 @@ pub(super) fn complete_network_resource_preparation( } }; let preparation = match conn - .loaded_page_mut_for_protocol_access(completed.session_id.as_deref()) + .loaded_page_mut_for_protocol_access_for_route( + owner_scope.session_id(), + owner_scope.session_owner_route(), + ) .and_then(|page| { + if page.renderer_agent_attachment_id() != completion.renderer_agent_attachment_id() { + return Err("Document changed while preparing the network resource load".to_owned()); + } page.finish_prepare_network_resource_load(completion) .map_err(|error| error.to_string()) }) { @@ -109,7 +123,7 @@ pub(super) fn complete_network_resource_preparation( moli_core::page::RendererNetworkResourceLoadPreparation::Ready(pending) => { NetworkCommandTaskStep::Pending(PendingNetworkCommandDispatch { command_id: completed.command_id, - session_id: completed.session_id, + owner_scope, kind: PendingNetworkCommandKind::FetchNetworkResource, pending: PendingNetworkCommandWork::Resource(pending), }) @@ -133,9 +147,10 @@ pub(super) fn complete_network_resource_fetch( conn: &mut CdpConnection, completed: CompletedNetworkCommandDispatch, ) -> CommandOutputPlan { + let owner_scope = completed.owner_scope.clone(); let outcome = match completed.completed { CompletedNetworkCommandWork::Resource(outcome) => outcome, - CompletedNetworkCommandWork::Page(_) => { + CompletedNetworkCommandWork::Page { .. } => { return invalid_completion_plan(); } }; @@ -162,8 +177,9 @@ pub(super) fn complete_network_resource_fetch( Some(headers), ); } - let stream = match conn.open_io_stream_body_source_for_session_owner( - completed.session_id.as_deref(), + let stream = match conn.open_io_stream_body_source_for_route( + owner_scope.session_id(), + owner_scope.session_owner_route(), CapturedBody::from_bytes_spooled(response.body), ) { Ok(stream) => stream, diff --git a/moli-protocol/src/domains/network/tests/session.rs b/moli-protocol/src/domains/network/tests/session.rs index f44a505e7..605cfefdf 100644 --- a/moli-protocol/src/domains/network/tests/session.rs +++ b/moli-protocol/src/domains/network/tests/session.rs @@ -1,5 +1,24 @@ use super::*; +async fn install_network_session_page(ctx: &mut TestContext, url: &str) { + let mut browser_context = BrowserContext::new("BID-navigation".into()); + browser_context.set_active_target_id("TID-navigation"); + browser_context.attach_active_session("SID-navigation"); + ctx.conn.browser_context = Some(browser_context); + let page = ctx + .conn + .load_page_via_runtime_async(url) + .await + .expect("the target should have a committed document"); + ctx.conn + .browser_context + .as_mut() + .unwrap() + .active_target + .runtime_slot + .set_loaded_page_for_test(page); +} + /// Network.enable without a browser context fails. #[tokio::test(flavor = "multi_thread")] async fn enable_no_bc_error() { @@ -26,6 +45,215 @@ async fn enable_with_bc_succeeds() { .primary_network_events_enabled() ); } + +#[tokio::test(flavor = "multi_thread")] +async fn network_configuration_commands_succeed_while_the_target_is_changing_documents() { + let mut ctx = TestContext::new(); + install_network_session_page(&mut ctx, "data:text/html,committed").await; + ctx.conn + .start_document_navigation_for_session_owner( + Some("SID-navigation"), + "LID-pending".to_owned(), + ) + .expect("the replacement navigation should start"); + + ctx.process_async(json!({ + "id": 2, + "method": "Network.enable", + "sessionId": "SID-navigation", + })) + .await; + + ctx.expect_result(2, json!({}), Some("SID-navigation")); + + for (id, method, params) in [ + ( + 3, + "Network.setCacheDisabled", + json!({ "cacheDisabled": true }), + ), + ( + 4, + "Network.setBypassServiceWorker", + json!({ "bypass": true }), + ), + ( + 5, + "Network.setExtraHTTPHeaders", + json!({ "headers": { "X-During-Navigation": "current" } }), + ), + ( + 6, + "Network.setBlockedURLs", + json!({ "urls": ["*://blocked.example/*"] }), + ), + ( + 7, + "Network.emulateNetworkConditions", + json!({ + "offline": true, + "latency": 0, + "downloadThroughput": -1, + "uploadThroughput": -1, + }), + ), + ( + 8, + "Network.setUserAgentOverride", + json!({ "userAgent": "Moli/During-Navigation" }), + ), + ] { + ctx.process_async(json!({ + "id": id, + "method": method, + "sessionId": "SID-navigation", + "params": params, + })) + .await; + ctx.expect_result(id, json!({}), Some("SID-navigation")); + } + + assert!( + ctx.conn + .browser_context + .as_ref() + .unwrap() + .active_target + .runtime_slot + .primary_network_events_enabled() + ); + let configuration = ctx + .conn + .prepared_document_commit_configuration_for_session_owner( + Some("SID-navigation"), + &url::Url::parse("data:text/html,committed").unwrap(), + ) + .expect("commit configuration should resolve the target resource runtime"); + assert!(configuration.cache_disabled); + assert!(configuration.bypass_service_worker); + assert!(configuration.network_offline); + assert_eq!( + configuration.extra_http_headers, + [("X-During-Navigation".to_owned(), "current".to_owned())] + ); + assert_eq!( + configuration.blocked_url_patterns, + ["*://blocked.example/*".to_owned()] + ); + assert_eq!( + moli_core::network::ResourceRequestClient::from_browser_resource_runtime( + configuration.browser_resource_runtime, + ) + .user_agent(), + "Moli/During-Navigation", + ); +} + +#[tokio::test(flavor = "multi_thread")] +async fn network_configuration_completion_does_not_restore_a_replaced_document() { + let mut ctx = TestContext::new(); + install_network_session_page(&mut ctx, "data:text/html,outgoing") + .await; + + let raw = json!({ + "id": 20, + "method": "Network.setExtraHTTPHeaders", + "sessionId": "SID-navigation", + "params": { "headers": { "X-Replacement-Race": "configured" } }, + }) + .to_string(); + let crate::conn::CdpCommandTaskStep::Pending(pending) = ctx.conn.start_command_dispatch(&raw) + else { + panic!("the loaded Page should receive the Network configuration command"); + }; + let completed = pending.wait().await; + + ctx.process_async(json!({ + "id": 21, + "method": "Page.navigate", + "sessionId": "SID-navigation", + "params": { + "url": "data:text/html,replacement" + }, + })) + .await; + let navigate = ctx.take_response_by_id(21); + assert!(navigate["result"]["loaderId"].is_string()); + + let crate::conn::CdpCommandTaskStep::Complete(outcome) = + ctx.conn.complete_pending_command_dispatch(completed).await + else { + panic!("the settled Network command should complete in one protocol phase"); + }; + assert!(outcome.into_parts().0.iter().any(|message| { + message["id"] == json!(20) + && message["sessionId"] == json!("SID-navigation") + && message["result"] == json!({}) + })); + + let html = ctx + .conn + .browser_context + .as_mut() + .unwrap() + .active_target + .runtime_slot + .loaded_page_mut() + .expect("the replacement Page should remain installed") + .serialize_html_async() + .await + .expect("the replacement Page should remain usable"); + assert!(html.contains("id=\"replacement\"")); + assert!(!html.contains("id=\"outgoing\"")); +} + +#[tokio::test(flavor = "multi_thread")] +async fn commit_configuration_resolves_the_exact_target_network_runtime() { + let mut ctx = TestContext::new(); + let mut browser_context = BrowserContext::new("BID-runtime".into()); + browser_context.set_active_target_id("TID-a"); + browser_context.attach_active_session("SID-a"); + browser_context.insert_page_target_host(PageTargetHost::with_url( + "TID-b".to_owned(), + Some("SID-b".to_owned()), + "about:blank".to_owned(), + )); + ctx.conn.browser_context = Some(browser_context); + + for (id, session_id, user_agent) in [ + (30, "SID-a", "Moli/Target-A"), + (31, "SID-b", "Moli/Target-B"), + ] { + ctx.process_async(json!({ + "id": id, + "method": "Network.setUserAgentOverride", + "sessionId": session_id, + "params": { "userAgent": user_agent }, + })) + .await; + ctx.expect_result(id, json!({}), Some(session_id)); + } + + for (session_id, expected_user_agent) in [ + ("SID-b", "Moli/Target-B"), + ("SID-a", "Moli/Target-A"), + ("SID-b", "Moli/Target-B"), + ] { + let configuration = ctx + .conn + .prepared_document_commit_configuration_for_session_owner( + Some(session_id), + &url::Url::parse("about:blank").unwrap(), + ) + .expect("the target-specific resource runtime should resolve"); + let request_client = + moli_core::network::ResourceRequestClient::from_browser_resource_runtime( + configuration.browser_resource_runtime, + ); + assert_eq!(request_client.user_agent(), expected_user_agent); + } +} + #[tokio::test(flavor = "multi_thread")] async fn auxiliary_network_enable_does_not_enable_primary_session() { let mut ctx = TestContext::new(); diff --git a/moli-protocol/src/domains/page.rs b/moli-protocol/src/domains/page.rs index 046ed7831..a2363d75f 100644 --- a/moli-protocol/src/domains/page.rs +++ b/moli-protocol/src/domains/page.rs @@ -6510,13 +6510,10 @@ fn try_start_page_enable_command( "BrowserContextNotLoaded", ))); } - if conn.session_owner_target_has_waiting_for_debugger_session(cmd.session_id) { - return Some(PageCommandTaskStep::Complete(CommandOutputPlan::success())); - } match conn.runtime_session_owner_slot(cmd.session_id) { Ok(slot) if slot.has_loaded_page() - && conn.runtime_session_owner_should_start_initial_document_navigation( + && conn.runtime_session_owner_can_start_initial_document_navigation( cmd.session_id, ) => { diff --git a/moli-protocol/src/domains/page/navigation.rs b/moli-protocol/src/domains/page/navigation.rs index f38a53458..dc41cc81f 100644 --- a/moli-protocol/src/domains/page/navigation.rs +++ b/moli-protocol/src/domains/page/navigation.rs @@ -3077,11 +3077,12 @@ fn start_navigate_to_url_command_with_background_policy_and_request( "NavigationRequestNotCurrent", )); }; - let none_session_owner_route = completion_state - .navigate_session_id - .is_none() - .then(|| conn.none_session_owner_route_override()) - .flatten(); + let none_session_owner_route = crate::conn::CommandOwnerScope::capture( + conn, + completion_state.navigate_session_id.as_deref(), + ) + .session_owner_route() + .cloned(); tokio::task::spawn_local(async move { let body_completion_sink = crate::conn::BackgroundNavigationBodyCompletionSink::new( sender.clone(), @@ -3382,11 +3383,14 @@ pub(crate) async fn complete_materialized_navigation_into_buffer_async( match navigation { network::MaterializedNavigationLoadOutcome::ResponseCommitReady(navigation) => { let navigation = *navigation; - let configuration = conn.prepared_document_commit_configuration_for_session_owner( + let update_result = match conn.prepared_document_commit_configuration_for_session_owner( state.navigate_session_id.as_deref(), navigation.final_url(), - ); - if let Err(error) = navigation.update_commit_configuration(configuration).await { + ) { + Ok(configuration) => 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(); diff --git a/moli-protocol/src/domains/page/preload.rs b/moli-protocol/src/domains/page/preload.rs index cec8cb33b..b4df9a416 100644 --- a/moli-protocol/src/domains/page/preload.rs +++ b/moli-protocol/src/domains/page/preload.rs @@ -1283,7 +1283,7 @@ fn start_create_isolated_world_initial_navigation_or_renderer_phase( mut task: CreateIsolatedWorldCommandTask, ) -> PageCommandTaskStep { let should_start_target_url_navigation = - conn.runtime_session_owner_should_start_initial_document_navigation(session_id); + conn.runtime_session_owner_can_start_initial_document_navigation(session_id); if !should_start_target_url_navigation { return start_create_isolated_world_frame_or_world_phase( conn, command_id, session_id, task, diff --git a/moli-protocol/src/domains/page/termination.rs b/moli-protocol/src/domains/page/termination.rs index 47853353c..a3eae516f 100644 --- a/moli-protocol/src/domains/page/termination.rs +++ b/moli-protocol/src/domains/page/termination.rs @@ -135,7 +135,8 @@ pub(crate) async fn fail_pending_fetch_state_background_events_async( conn: &mut CdpConnection, out: &mut Vec, session_id: Option<&str>, - error_text: &str, + navigation_error_text: &str, + subresource_error_text: &str, pending_navigations: Vec, pending_auth_navigations: Vec, pending_response_navigations: Vec, @@ -146,6 +147,10 @@ pub(crate) async fn fail_pending_fetch_state_background_events_async( crate::conn::PendingSubresourceFetchResponseRequest, )>, ) -> Option { + // A protocol navigation waiter may expose an operation-specific failure, + // while Network.loadingFailed must retain the underlying net error. For + // Page.stopLoading Chromium reports ERR_ABORTED/canceled=true even though + // Moli's pending navigation reply remains "Navigation stopped". let mut renderer_output_predecessor = None; for pending in pending_navigations { let token = pending.document_navigation_token; @@ -153,7 +158,7 @@ pub(crate) async fn fail_pending_fetch_state_background_events_async( let navigation = network::materialize_navigation_failure_preserving_committed_document( conn, &navigation_state, - error_text.to_owned(), + navigation_error_text.to_owned(), ); let predecessor = complete_tokened_materialized_navigation_background_events_async( conn, @@ -171,7 +176,7 @@ pub(crate) async fn fail_pending_fetch_state_background_events_async( let navigation = network::materialize_navigation_failure_preserving_committed_document( conn, &navigation_state, - error_text.to_owned(), + navigation_error_text.to_owned(), ); let predecessor = complete_tokened_materialized_navigation_background_events_async( conn, @@ -184,11 +189,11 @@ pub(crate) async fn fail_pending_fetch_state_background_events_async( merge_renderer_output_predecessor(&mut renderer_output_predecessor, predecessor); } for pending in pending_response_navigations { - let (token, navigation, _) = pending.fail(error_text.to_owned()); + let (token, navigation, _) = pending.fail(navigation_error_text.to_owned()); let result = network::materialize_navigation_failure_preserving_committed_document( conn, &navigation, - error_text.to_owned(), + navigation_error_text.to_owned(), ); let predecessor = complete_tokened_materialized_navigation_background_events_async( conn, out, token, navigation, result, @@ -204,7 +209,7 @@ pub(crate) async fn fail_pending_fetch_state_background_events_async( .fail_pending_subresource_fetch_for_session_owner_async( session_id, pending.internal_id, - error_text.to_owned(), + subresource_error_text.to_owned(), ) .await { @@ -229,7 +234,7 @@ pub(crate) async fn fail_pending_fetch_state_background_events_async( .fail_pending_subresource_auth_for_session_owner_async( session_id, pending.internal_id, - error_text.to_owned(), + subresource_error_text.to_owned(), ) .await { @@ -254,7 +259,7 @@ pub(crate) async fn fail_pending_fetch_state_background_events_async( .fail_pending_subresource_response_for_session_owner_async( session_id, pending.internal_id, - error_text.to_owned(), + subresource_error_text.to_owned(), ) .await { @@ -387,6 +392,7 @@ pub(super) async fn complete_stop_loading_command_dispatch( &mut out, session_id, "Navigation stopped", + moli_fetch::NET_ERR_ABORTED_ERROR_TEXT, pending_navigations, pending_auth_navigations, pending_response_navigations, @@ -504,6 +510,7 @@ pub(super) async fn complete_crash_command_dispatch( &mut out, fail_session_id, "Page crashed", + "Page crashed", pending_navigations, pending_auth_navigations, pending_response_navigations, @@ -617,6 +624,7 @@ pub(super) async fn complete_close_command_dispatch( &mut out, fail_session_id, "Page closed", + "Page closed", pending_navigations, pending_auth_navigations, pending_response_navigations, diff --git a/moli-protocol/src/domains/runtime/activity.rs b/moli-protocol/src/domains/runtime/activity.rs index 990e63825..cfaff47e6 100644 --- a/moli-protocol/src/domains/runtime/activity.rs +++ b/moli-protocol/src/domains/runtime/activity.rs @@ -44,15 +44,7 @@ struct RuntimeBindingCallBatch { #[derive(Clone, Debug, PartialEq)] struct RuntimeInspectorMessageBatch { - /// Exact Page and protocol attachment that owned these messages when the - /// renderer snapshot was captured. - /// - /// `RuntimeInspectorMessageBatch` may contain command responses as well as - /// notifications. A projection-time session fallback would therefore be - /// able to deliver an old response to a replacement Page. The attachment - /// is captured before the output can cross an async or scheduler boundary - /// and is revalidated immediately before delivery. - attachment: crate::conn::TargetPageProtocolAttachmentIdentity, + authority: RuntimeInspectorMessageAuthority, messages: Vec, /// Contexts created by this exact Inspector batch. /// @@ -63,6 +55,55 @@ struct RuntimeInspectorMessageBatch { created_execution_context_ids: Vec, } +/// Projection authority for one prepared Inspector batch. +/// +/// Ordinary observations remain tied to the exact Page attachment that +/// produced them. A retired terminal response is a distinct state rather than +/// an absent attachment: its exact renderer correlation was already consumed +/// at ingress, and no notification or renderer state can inhabit this variant. +#[derive(Clone, Debug, PartialEq)] +enum RuntimeInspectorMessageAuthority { + CurrentPage(crate::conn::TargetPageProtocolAttachmentIdentity), + AuthorizedRetiredTerminal { + browser_context_id: String, + target_id: Option, + session_id: Option, + }, +} + +impl RuntimeInspectorMessageAuthority { + fn session_id(&self) -> Option<&str> { + match self { + Self::CurrentPage(attachment) => attachment.session_id(), + Self::AuthorizedRetiredTerminal { session_id, .. } => session_id.as_deref(), + } + } + + fn permits_projection(&self, conn: &CdpConnection) -> bool { + match self { + Self::CurrentPage(attachment) => { + conn.target_page_protocol_attachment_identity_is_current(attachment) + } + Self::AuthorizedRetiredTerminal { + browser_context_id, + target_id, + session_id, + } => { + let Some((current_browser_context_id, current_target_id)) = + conn.target_owner_identity_for_session(session_id.as_deref()) + else { + return false; + }; + let current_target_id = current_target_id.or_else(|| { + conn.browser_context_by_id(¤t_browser_context_id) + .and_then(|context| context.active_target_id_owned()) + }); + current_browser_context_id == *browser_context_id && current_target_id == *target_id + } + } + } +} + #[derive(Clone, Debug, PartialEq)] enum RuntimeInspectorMessage { Context(RuntimeContextProtocolEvent), @@ -150,12 +191,10 @@ impl RuntimeOutputProjectionStep { }) { for batch in batches { - if !conn - .target_page_protocol_attachment_identity_is_current(&batch.attachment) - { + if !batch.authority.permits_projection(conn) { continue; } - let session_id = batch.attachment.session_id().map(str::to_owned); + let session_id = batch.authority.session_id().map(str::to_owned); push_runtime_inspector_messages_for_session( conn, context.command.protocol_events_mut(), @@ -278,7 +317,7 @@ impl RuntimePreparedOutputs { } } let prepared = RuntimeInspectorMessageBatch { - attachment, + authority: RuntimeInspectorMessageAuthority::CurrentPage(attachment), messages, created_execution_context_ids, }; @@ -294,6 +333,92 @@ impl RuntimePreparedOutputs { outputs } + /// Preserves only terminal session responses from an attachment that was + /// current when the response lease was claimed but retired before its Page + /// journal crossed protocol ingress. + /// + /// The exact renderer correlation is consumed here. This makes the + /// resulting response an already-authorized session fact, while all + /// notifications, context events, V8 state, and remote-object projection + /// from the retired document are discarded. + pub(crate) fn from_retired_renderer_runtime_inspector_session_responses( + conn: &mut CdpConnection, + source_session_id: Option<&str>, + batches: &[RendererRuntimeInspectorMessageBatch], + ) -> Self { + let mut outputs = Self::default(); + for batch in batches { + let Some(renderer_agent_attachment_id) = batch.renderer_agent_attachment_id() else { + continue; + }; + let Some(protocol_attachment) = conn + .target_page_protocol_attachment_identity_for_renderer_inspector_route( + source_session_id, + batch.session.wire_session_id(), + ) + else { + continue; + }; + let session_id = protocol_attachment.session_id().map(str::to_owned); + if conn.renderer_agent_attachment_is_current_for_session_owner( + session_id.as_deref(), + renderer_agent_attachment_id, + ) { + continue; + } + + let mut responses = batch + .messages + .iter() + .filter(|message| { + matches!( + message, + RendererRuntimeInspectorMessage::Protocol(message) + if message.renderer_call_id().is_some() + ) + }) + .cloned() + .collect::>(); + conn.restore_frontend_command_ids_in_retired_devtools_session_output( + session_id.as_deref(), + renderer_agent_attachment_id, + &mut responses, + ); + if responses.is_empty() { + continue; + } + let target_id = protocol_attachment.page_owner().target_id(); + let messages = responses + .into_iter() + .map(|message| RuntimeInspectorMessage::from_renderer_message(message, target_id)) + .collect(); + let prepared = RuntimeInspectorMessageBatch { + authority: RuntimeInspectorMessageAuthority::AuthorizedRetiredTerminal { + browser_context_id: protocol_attachment + .page_owner() + .browser_context_id() + .to_owned(), + target_id: protocol_attachment + .page_owner() + .target_id() + .map(str::to_owned), + session_id, + }, + messages, + created_execution_context_ids: Vec::new(), + }; + match batch.command_response_order() { + RendererRuntimeInspectorMessageResponseOrder::BeforeCommandResponse => { + outputs.inspector_message_batches.push(prepared); + } + RendererRuntimeInspectorMessageResponseOrder::AfterCommandResponse => outputs + .post_response_inspector_message_batches + .push(prepared), + } + } + outputs + } + pub(crate) fn extend(&mut self, other: Self) { self.binding_call_batches.extend(other.binding_call_batches); self.inspector_message_batches @@ -406,15 +531,15 @@ pub(in crate::domains) fn push_routed_renderer_runtime_inspector_message_batch_b .into_iter() .chain(prepared.post_response_inspector_message_batches) { - if !conn.target_page_protocol_attachment_identity_is_current(&batch.attachment) { + if !batch.authority.permits_projection(conn) { continue; } - let effective_session_id = batch.attachment.session_id().map(str::to_owned); + let session_id = batch.authority.session_id().map(str::to_owned); push_runtime_inspector_messages_for_session( conn, out, batch.messages, - effective_session_id.as_deref(), + session_id.as_deref(), ); } } @@ -422,9 +547,10 @@ pub(in crate::domains) fn push_routed_renderer_runtime_inspector_message_batch_b #[cfg(test)] mod tests { use moli_core::page::{ - DevToolsSessionKey, RendererDevToolsAgentToken, RendererRuntimeInspectorMessage, - RendererRuntimeInspectorMessageBatch, + DevToolsSessionKey, RendererAgentAttachmentId, RendererDevToolsAgentToken, + RendererRuntimeInspectorMessage, RendererRuntimeInspectorMessageBatch, }; + use moli_page_types::RendererInspectorResponseDelivery; use serde_json::Value; use serde_json::json; @@ -610,6 +736,103 @@ mod tests { command_context.take_protocol_events() } + #[tokio::test(flavor = "multi_thread")] + async fn retired_attachment_keeps_only_the_exact_terminal_session_response() { + let mut ctx = TestContext::new(); + load_document(&mut ctx, "
replacement
").await; + let current = ctx + .conn + .runtime_session_owner_slot(Some("SID-1")) + .expect("loaded target runtime") + .current_renderer_attachment() + .expect("current renderer attachment"); + let retired_attachment = RendererAgentAttachmentId::allocate(); + assert_ne!(retired_attachment, current.id()); + + let frontend = crate::conn::ParsedCdpCommand::parse_str( + r#"{"id":901001,"method":"Runtime.evaluate","sessionId":"SID-1","params":{"expression":"({ stale: true })"}}"#, + ) + .expect("frontend Runtime command"); + let prepared = ctx + .conn + .try_register_renderer_call_for_session_owner( + Some("SID-1"), + 901_001, + Some(retired_attachment), + crate::conn::RendererCommandDescriptor::from_frontend_policy( + frontend.json().to_owned(), + frontend.renderer_policy(), + RendererInspectorResponseDelivery::DevToolsSession, + ), + ) + .expect("retired renderer call correlation"); + let correlation = prepared.correlation(); + drop(prepared); + + let mut batch = RendererRuntimeInspectorMessageBatch::new( + current.agent_token(), + DevToolsSessionKey::Primary, + renderer_messages(vec![ + json!({ + "method": "Runtime.consoleAPICalled", + "params": {"type": "log", "args": [], "executionContextId": 1}, + }), + json!({ + "id": correlation.renderer_call_id().get(), + "result": { + "result": {"type": "object", "objectId": "retired-object"} + }, + }), + ]), + ); + batch.bind_renderer_agent_attachment(retired_attachment); + + let outputs = + RuntimePreparedOutputs::from_retired_renderer_runtime_inspector_session_responses( + &mut ctx.conn, + Some("SID-1"), + &[batch], + ); + let outputs_after_detach = outputs.clone(); + let events = drain_runtime_inspector_outputs(&mut ctx.conn, outputs, None) + .await + .into_iter() + .map(crate::conn::BackgroundProtocolEvent::into_protocol_message) + .collect::>(); + + assert_eq!(events.len(), 1, "retired notifications must be discarded"); + assert_eq!(events[0]["id"], json!(901_001)); + assert_eq!(events[0]["sessionId"], json!("SID-1")); + assert!( + ctx.conn + .renderer_runtime_command_cause_for_frontend(Some("SID-1"), 901_001) + .is_none(), + "the accepted terminal response must consume its exact correlation" + ); + assert_eq!( + ctx.conn + .runtime_remote_object_group_for_session_owner(Some("SID-1"), "retired-object",), + None, + "a retired document response must not register objects on the replacement document" + ); + + assert_eq!( + ctx.conn + .browser_context + .as_mut() + .expect("browser context") + .detach_active_session() + .as_deref(), + Some("SID-1"), + ); + assert!( + drain_runtime_inspector_outputs(&mut ctx.conn, outputs_after_detach, None) + .await + .is_empty(), + "terminal authority may outlive its Page, but not its protocol session binding", + ); + } + #[tokio::test(flavor = "multi_thread")] async fn runtime_backlog_batch_ignores_prequeued_inspector_registry() { let mut ctx = TestContext::new(); diff --git a/moli-protocol/src/domains/runtime/dispatcher.rs b/moli-protocol/src/domains/runtime/dispatcher.rs index 96fc0e727..439b05353 100644 --- a/moli-protocol/src/domains/runtime/dispatcher.rs +++ b/moli-protocol/src/domains/runtime/dispatcher.rs @@ -1006,11 +1006,7 @@ fn start_main_runtime_inspector_command( } else { None }; - let session_owner_route = if await_promise { - conn.runtime_await_owner_route_for_session(cmd.session_id) - } else { - None - }; + let owner_scope = CommandOwnerScope::capture(conn, cmd.session_id); let pre_registered_await = match pre_register_runtime_await_if_needed( conn, await_promise, @@ -1046,10 +1042,7 @@ fn start_main_runtime_inspector_command( RuntimeCommandTaskStep::Pending(Box::new(PendingRuntimeCommandDispatch { command_id: cmd.id, action: action_label, - owner_scope: CommandOwnerScope::from_session_and_owner_route( - cmd.session_id, - session_owner_route, - ), + owner_scope, object_group, release_object_ids, release_object_group, @@ -1632,11 +1625,7 @@ fn try_start_pending_runtime_enable_command( conn: &mut CdpConnection, cmd: &Cmd<'_>, ) -> Option { - let session_owner_route = if cmd.session_id.is_none() { - conn.none_session_owner_route_override() - } else { - None - }; + let owner_scope = CommandOwnerScope::capture(conn, cmd.session_id); let has_loaded_page = match conn.runtime_session_owner_slot(cmd.session_id) { Ok(slot) => slot.has_loaded_page(), Err(_) if cmd.session_id.is_some() => { @@ -1684,7 +1673,7 @@ fn try_start_pending_runtime_enable_command( conn, cmd.id, cmd.session_id, - session_owner_route, + owner_scope, )) } @@ -1692,16 +1681,13 @@ fn start_pending_runtime_enable_events_phase( conn: &mut CdpConnection, command_id: Option, session_id: Option<&str>, - session_owner_route: Option, + owner_scope: CommandOwnerScope, ) -> RuntimeCommandTaskStep { match conn.start_runtime_enable_events_for_session_owner(session_id) { Ok(pending) => RuntimeCommandTaskStep::Pending(Box::new(PendingRuntimeCommandDispatch { command_id, action: "enable", - owner_scope: CommandOwnerScope::from_session_and_owner_route( - session_id, - session_owner_route, - ), + owner_scope, object_group: None, release_object_ids: Vec::new(), release_object_group: None, @@ -1729,11 +1715,7 @@ fn try_start_pending_runtime_binding_command( cmd: &Cmd<'_>, action: RuntimeBindingCommand, ) -> Option { - let session_owner_route = if cmd.session_id.is_none() { - conn.none_session_owner_route_override() - } else { - None - }; + let owner_scope = CommandOwnerScope::capture(conn, cmd.session_id); let (name, execution_context_name, execution_context_id) = match action { RuntimeBindingCommand::Add => { let params = match cmd.get_params::() { @@ -1805,10 +1787,7 @@ fn try_start_pending_runtime_binding_command( let meta = RuntimeCommandCompletionMeta { command_id: cmd.id, action: task.action.label(), - owner_scope: CommandOwnerScope::from_session_and_owner_route( - cmd.session_id, - session_owner_route.clone(), - ), + owner_scope: owner_scope.clone(), object_group: None, release_object_ids: Vec::new(), release_object_group: None, @@ -1824,7 +1803,7 @@ fn try_start_pending_runtime_binding_command( cmd.session_id, task, execution_context_id, - session_owner_route, + owner_scope, ); } if matches!(task.action, RuntimeBindingCommand::Add) @@ -1860,10 +1839,7 @@ fn try_start_pending_runtime_binding_command( PendingRuntimeCommandDispatch { command_id: cmd.id, action: action_label, - owner_scope: CommandOwnerScope::from_session_and_owner_route( - cmd.session_id, - session_owner_route, - ), + owner_scope, object_group: None, release_object_ids: Vec::new(), release_object_group: None, @@ -2122,11 +2098,7 @@ fn start_devtools_runtime_command( } }; let object_group = runtime_object_group_for_command_result(conn, cmd, action); - let session_owner_route = if await_promise { - conn.runtime_await_owner_route_for_session(cmd.session_id) - } else { - None - }; + let owner_scope = CommandOwnerScope::capture(conn, cmd.session_id); let pre_registered_await = match pre_register_runtime_await_if_needed( conn, await_promise, @@ -2159,10 +2131,7 @@ fn start_devtools_runtime_command( RuntimeCommandTaskStep::Pending(Box::new(PendingRuntimeCommandDispatch { command_id: cmd.id, action: action_label, - owner_scope: CommandOwnerScope::from_session_and_owner_route( - cmd.session_id, - session_owner_route, - ), + owner_scope, object_group, release_object_ids: Vec::new(), release_object_group: None, @@ -7651,7 +7620,7 @@ fn start_pending_runtime_binding_context_lookup_phase( session_id: Option<&str>, task: RuntimeBindingCommandTask, execution_context_id: i64, - session_owner_route: Option, + owner_scope: CommandOwnerScope, ) -> Option { let pending = conn .start_child_default_execution_context_lookup_for_session_owner( @@ -7663,10 +7632,7 @@ fn start_pending_runtime_binding_context_lookup_phase( PendingRuntimeCommandDispatch { command_id, action: task.action.label(), - owner_scope: CommandOwnerScope::from_session_and_owner_route( - session_id, - session_owner_route, - ), + owner_scope, object_group: None, release_object_ids: Vec::new(), release_object_group: None, @@ -8497,7 +8463,9 @@ async fn complete_pending_runtime_inspector_command( object_group, ); } - if completed.action == "runIfWaitingForDebugger" { + if completed.action == "runIfWaitingForDebugger" + && conn.release_waiting_for_debugger_session(completed.session_id()) + { crate::domains::target::schedule_initial_document_target_url_navigation_after_debugger_resume( conn, completed.session_id(), diff --git a/moli-protocol/src/domains/target.rs b/moli-protocol/src/domains/target.rs index 8b3f64baf..ba29381e1 100644 --- a/moli-protocol/src/domains/target.rs +++ b/moli-protocol/src/domains/target.rs @@ -35,6 +35,7 @@ pub(crate) use popup::{ complete_popup_target_navigation_owner_action_async, create_popup_target_from_renderer_output_background_events_async, emit_target_info_changed_for_session_owner_background_event, + schedule_initial_document_target_url_navigation_after_debugger_barrier_release_for_target, schedule_initial_document_target_url_navigation_after_debugger_resume, }; pub(crate) fn popup_activation_creates_new_target( @@ -254,6 +255,7 @@ async fn clear_detached_target_owner_fetch_state_background_events_async( out, session_id, "Target detached", + "Target detached", pending_navigations, pending_auth_navigations, pending_response_navigations, diff --git a/moli-protocol/src/domains/target/attachment.rs b/moli-protocol/src/domains/target/attachment.rs index 305305453..c4c7169c9 100644 --- a/moli-protocol/src/domains/target/attachment.rs +++ b/moli-protocol/src/domains/target/attachment.rs @@ -1026,6 +1026,7 @@ async fn detach_from_target_inner_async( out.background_events_mut(), Some(¤t_session_id), "Target detached", + "Target detached", pending_navigations, pending_auth_navigations, pending_response_navigations, diff --git a/moli-protocol/src/domains/target/browser_context_disposal.rs b/moli-protocol/src/domains/target/browser_context_disposal.rs index f176b04ce..4ed3e1bbb 100644 --- a/moli-protocol/src/domains/target/browser_context_disposal.rs +++ b/moli-protocol/src/domains/target/browser_context_disposal.rs @@ -234,6 +234,7 @@ async fn fail_pending_navigations_for_disposed_target_async( out, session_id, DISPOSE_REASON, + DISPOSE_REASON, pending_navigations, pending_auth_navigations, pending_response_navigations, diff --git a/moli-protocol/src/domains/target/events.rs b/moli-protocol/src/domains/target/events.rs index aed36a1da..647ddbe84 100644 --- a/moli-protocol/src/domains/target/events.rs +++ b/moli-protocol/src/domains/target/events.rs @@ -298,6 +298,7 @@ pub(super) async fn fail_pending_fetch_state_for_target_background_events_async( out, session_id, reason, + reason, pending_navigations, pending_auth_navigations, pending_response_navigations, diff --git a/moli-protocol/src/domains/target/popup.rs b/moli-protocol/src/domains/target/popup.rs index d0bea8c19..e408f4a30 100644 --- a/moli-protocol/src/domains/target/popup.rs +++ b/moli-protocol/src/domains/target/popup.rs @@ -436,7 +436,7 @@ pub(crate) async fn start_initial_document_target_url_navigation_if_needed_backg out: &mut Vec, session_id: Option<&str>, ) -> bool { - if !conn.runtime_session_owner_should_start_initial_document_navigation(session_id) { + if !conn.runtime_session_owner_can_start_initial_document_navigation(session_id) { return false; } let Some(target_url) = conn.runtime_session_owner_target_url(session_id) else { @@ -456,21 +456,51 @@ pub(crate) fn schedule_initial_document_target_url_navigation_after_debugger_res conn: &mut CdpConnection, session_id: Option<&str>, ) -> bool { - if !conn.runtime_session_owner_should_start_initial_document_navigation(session_id) { - return false; - } - let Some(target_url) = conn.runtime_session_owner_target_url(session_id) else { + let Some((_, Some(target_id))) = conn.target_owner_identity_for_session(session_id) else { return false; }; - let Some((browser_context_id, Some(target_id))) = - conn.target_owner_identity_for_session(session_id) + schedule_initial_document_target_url_navigation_after_debugger_barrier_release_for_target( + conn, &target_id, + ) +} + +pub(crate) fn schedule_initial_document_target_url_navigation_after_debugger_barrier_release_for_target( + conn: &mut CdpConnection, + target_id: &str, +) -> bool { + if conn.target_has_waiting_for_debugger_session(target_id) { + return false; + } + let Some(route) = conn.target_session_route_for_target_id(target_id) else { + return false; + }; + if !matches!( + &route, + crate::conn::CdpSessionRoute::ActiveTarget { .. } + | crate::conn::CdpSessionRoute::AuxiliaryTarget { .. } + | crate::conn::CdpSessionRoute::PageTargetHost { .. } + ) { + return false; + } + let Some(browser_context_id) = route.browser_context_id().map(str::to_owned) else { + return false; + }; + let Some(browser_context) = conn.browser_context_by_id(&browser_context_id) else { + return false; + }; + if !browser_context.target_needs_initial_document_navigation(target_id) { + return false; + } + let Some(target_url) = browser_context + .devtools_target_info(target_id) + .map(|target_info| target_info.url) else { return false; }; let Some(action) = PopupTargetNavigationOwnerAction::capture( conn, &browser_context_id, - &target_id, + target_id, target_url, PopupTargetNavigationKind::InitialDocumentAfterDebuggerResume, ) else { @@ -527,7 +557,11 @@ pub(crate) async fn complete_popup_target_navigation_owner_action_async( match kind { PopupTargetNavigationKind::InitialDocument | PopupTargetNavigationKind::InitialDocumentAfterDebuggerResume => { - if !conn.runtime_session_owner_should_start_initial_document_navigation(None) { + // Revalidate the barrier when the queued owner action actually + // runs. Another inspector session can attach after this action is + // scheduled; that new session must be able to pause the initial + // document before any target-URL request starts. + if !conn.runtime_session_owner_can_start_initial_document_navigation(None) { return crate::conn::CdpTurnOutcome::new_with_protocol_events( Vec::new(), conn.take_scheduler_events(), diff --git a/moli-protocol/src/domains/target/tests/tests_cdp_chromium_imports/p1_target_multipage.rs b/moli-protocol/src/domains/target/tests/tests_cdp_chromium_imports/p1_target_multipage.rs index f4ec33276..370633906 100644 --- a/moli-protocol/src/domains/target/tests/tests_cdp_chromium_imports/p1_target_multipage.rs +++ b/moli-protocol/src/domains/target/tests/tests_cdp_chromium_imports/p1_target_multipage.rs @@ -865,8 +865,46 @@ async fn run_waiting_popup_initial_document_after_resume( ctx.expect_result(260_218, json!({}), Some(popup_session_id)); ctx.sent.clear(); + // Playwright sends this command in the same initialization burst as + // Runtime.runIfWaitingForDebugger. It must operate on the materialized + // initial document without independently starting the target URL: only + // the debugger-resume response owns that transition. ctx.process_async(json!({ "id": 260_219, + "method": "Page.createIsolatedWorld", + "sessionId": popup_session_id, + "params": { + "frameId": popup_target_id, + "worldName": "__playwright_pre_resume_utility_world", + "grantUniveralAccess": true + } + })) + .await; + let initial_world = take_response_by_id(ctx, 260_219); + assert!( + initial_world["result"]["executionContextId"] + .as_i64() + .is_some(), + "createIsolatedWorld should resolve against the paused initial document: {initial_world:?}" + ); + assert!( + !ctx.conn + .has_pending_document_navigation_for_session_owner(Some(popup_session_id)), + "createIsolatedWorld must not claim the debugger-gated initial navigation" + ); + assert!( + !ctx.sent.iter().any(|message| { + message["method"] == json!("Fetch.requestPaused") + || (message["method"] == json!("Network.requestWillBeSent") + && message["params"]["request"]["url"] == json!(popup_url)) + }), + "createIsolatedWorld must leave the real popup URL gated: {:?}", + ctx.sent + ); + ctx.sent.clear(); + + ctx.process_async(json!({ + "id": 260_220, "method": "Runtime.runIfWaitingForDebugger", "sessionId": popup_session_id })) @@ -880,7 +918,7 @@ async fn run_waiting_popup_initial_document_after_resume( let response_index = ctx .sent .iter() - .position(|message| message["id"] == json!(260_219)) + .position(|message| message["id"] == json!(260_220)) .expect("runIfWaitingForDebugger terminal response"); let request_index = ctx .sent @@ -892,7 +930,7 @@ async fn run_waiting_popup_initial_document_after_resume( "the debugger-resume response must cross the frontend before its initial navigation can replace about:blank: {:?}", ctx.sent ); - take_response_by_id(ctx, 260_219); + take_response_by_id(ctx, 260_220); let paused = ctx .sent .iter() @@ -923,7 +961,7 @@ async fn run_waiting_popup_initial_document_after_resume( ctx.sent.clear(); ctx.process_async(json!({ - "id": 260_220, + "id": 260_221, "method": "Page.createIsolatedWorld", "sessionId": popup_session_id, "params": { @@ -933,7 +971,7 @@ async fn run_waiting_popup_initial_document_after_resume( } })) .await; - let isolated = take_response_by_id(ctx, 260_220); + let isolated = take_response_by_id(ctx, 260_221); assert!( isolated["result"]["executionContextId"].as_i64().is_some(), "createIsolatedWorld should resolve while popup initial document is paused: {isolated:?}" @@ -947,7 +985,7 @@ async fn run_waiting_popup_initial_document_after_resume( ); ctx.process_async(json!({ - "id": 260_221, + "id": 260_222, "method": "Fetch.fulfillRequest", "sessionId": popup_session_id, "params": { @@ -960,7 +998,7 @@ async fn run_waiting_popup_initial_document_after_resume( } })) .await; - ctx.expect_result(260_221, json!({}), Some(popup_session_id)); + ctx.expect_result(260_222, json!({}), Some(popup_session_id)); crate::testing::wait_until_scheduler_message(ctx, "resumed popup load lifecycle", |message| { message["method"] == json!("Page.loadEventFired") && message["sessionId"] == json!(popup_session_id) @@ -977,7 +1015,7 @@ async fn run_waiting_popup_initial_document_after_resume( ); ctx.process_async(json!({ - "id": 260_222, + "id": 260_223, "method": "Runtime.evaluate", "sessionId": popup_session_id, "params": { @@ -987,7 +1025,7 @@ async fn run_waiting_popup_initial_document_after_resume( })) .await; assert_eq!( - take_response_by_id(ctx, 260_222)["result"]["result"]["value"], + take_response_by_id(ctx, 260_223)["result"]["result"]["value"], "routed-popup" ); } diff --git a/moli-protocol/src/domains/target/tests/tests_cdp_smoke_playwright.rs b/moli-protocol/src/domains/target/tests/tests_cdp_smoke_playwright.rs index 3d2ec9ffb..dedef663d 100644 --- a/moli-protocol/src/domains/target/tests/tests_cdp_smoke_playwright.rs +++ b/moli-protocol/src/domains/target/tests/tests_cdp_smoke_playwright.rs @@ -83,7 +83,25 @@ async fn arm_popup_route( base: u64, popup_target_id: &str, popup_session_id: &str, + popup_url: &str, ) { + assert!( + ctx.conn + .target_has_waiting_for_debugger_session(popup_target_id), + "the auto-attached popup session must own the debugger gate", + ); + let initial_url = ctx + .conn + .browser_contexts() + .find_map(|browser_context| { + loaded_page_for_target(browser_context, popup_target_id) + .map(|page| page.final_url().to_string()) + }) + .expect("popup initial document"); + assert_eq!( + initial_url, "about:blank", + "the popup target URL must remain gated until debugger resume", + ); ctx.process_async(json!({ "id": base, "method": "Page.enable", @@ -114,7 +132,23 @@ async fn arm_popup_route( })) .await; ctx.expect_result(base + 2, json!({}), Some(popup_session_id)); - + let fetch_snapshot = ctx + .conn + .target_fetch_subresource_interception_snapshot_for_session_owner(Some(popup_session_id)) + .expect("popup target Fetch configuration"); + let matching_sessions = fetch_snapshot.matching_request_stage_pause_sessions( + Some(popup_session_id), + crate::devtools_runtime::DevToolsNetworkResourceType::Document, + &url::Url::parse(popup_url).expect("popup URL"), + ); + assert_eq!( + matching_sessions + .iter() + .map(|session| session.session_id.as_deref()) + .collect::>(), + [Some(popup_session_id)], + "Fetch.enable must commit the document pattern to the popup target before resume", + ); ctx.process_async(json!({ "id": base + 3, "method": "Runtime.runIfWaitingForDebugger", @@ -122,6 +156,11 @@ async fn arm_popup_route( })) .await; ctx.expect_result(base + 3, json!({}), Some(popup_session_id)); + assert!( + !ctx.conn + .target_has_waiting_for_debugger_session(popup_target_id), + "runIfWaitingForDebugger must release the popup session's debugger barrier", + ); ctx.process_async(json!({ "id": base + 4, @@ -149,6 +188,33 @@ async fn fulfill_popup_document_and_evaluate( popup_url: &str, expected_text: &str, ) { + let fetch_snapshot = ctx + .conn + .target_fetch_subresource_interception_snapshot_for_target(popup_target_id) + .expect("popup target Fetch configuration after debugger resume"); + let matching_sessions = fetch_snapshot.matching_request_stage_pause_sessions( + Some(popup_session_id), + crate::devtools_runtime::DevToolsNetworkResourceType::Document, + &url::Url::parse(popup_url).expect("popup URL"), + ); + assert_eq!( + matching_sessions + .iter() + .map(|session| session.session_id.as_deref()) + .collect::>(), + [Some(popup_session_id)], + "popup activation and debugger resume must preserve target-owned Fetch configuration", + ); + crate::testing::wait_until_scheduler_message( + ctx, + "debugger-resumed popup document request", + |message| { + message["method"] == json!("Fetch.requestPaused") + && message["sessionId"] == json!(popup_session_id) + && message["params"]["resourceType"] == json!("Document") + }, + ) + .await; let paused = ctx .sent .iter() @@ -524,6 +590,7 @@ async fn rust_cdp_playwright_auxiliary_session_network_event_contract() { async fn rust_cdp_playwright_multi_context_popup_route_and_evaluate_contract() { let fixture = SmokeFixtureServer::start().await; let mut ctx = TestContext::new(); + ctx.enable_background_navigation_scheduler_for_test(); let first = attached_smoke_session(&mut ctx, 87_000).await; let second = attached_smoke_session(&mut ctx, 87_100).await; assert_ne!(first.browser_context_id, second.browser_context_id); @@ -540,6 +607,7 @@ async fn rust_cdp_playwright_multi_context_popup_route_and_evaluate_contract() { 87_210, &first_popup_target_id, &first_popup_session_id, + &first_popup_url, ) .await; fulfill_popup_document_and_evaluate( @@ -562,6 +630,7 @@ async fn rust_cdp_playwright_multi_context_popup_route_and_evaluate_contract() { 87_310, &second_popup_target_id, &second_popup_session_id, + &second_popup_url, ) .await; fulfill_popup_document_and_evaluate( @@ -588,6 +657,7 @@ async fn rust_cdp_playwright_multi_context_popup_route_and_evaluate_contract() { async fn rust_cdp_playwright_concurrent_popup_routes_keep_their_navigation_owners() { let fixture = SmokeFixtureServer::start().await; let mut ctx = TestContext::new(); + ctx.enable_background_navigation_scheduler_for_test(); let opener = attached_smoke_session(&mut ctx, 88_000).await; set_auto_attach_waiting_for_debugger(&mut ctx, 88_100).await; @@ -602,6 +672,7 @@ async fn rust_cdp_playwright_concurrent_popup_routes_keep_their_navigation_owner 88_110, &first_popup_target_id, &first_popup_session_id, + &first_popup_url, ) .await; @@ -614,6 +685,7 @@ async fn rust_cdp_playwright_concurrent_popup_routes_keep_their_navigation_owner 88_210, &second_popup_target_id, &second_popup_session_id, + &second_popup_url, ) .await; @@ -636,3 +708,213 @@ async fn rust_cdp_playwright_concurrent_popup_routes_keep_their_navigation_owner ) .await; } + +#[tokio::test(flavor = "multi_thread")] +async fn rust_cdp_popup_waits_for_every_debugger_barrier_and_detach_releases_the_last() { + let fixture = SmokeFixtureServer::start().await; + let mut ctx = TestContext::new(); + ctx.enable_background_navigation_scheduler_for_test(); + let opener = attached_smoke_session(&mut ctx, 89_000).await; + + set_auto_attach_waiting_for_debugger(&mut ctx, 89_100).await; + ctx.process_async(json!({ + "id": 89_101, + "method": "Target.attachToBrowserTarget" + })) + .await; + let browser_attached = ctx.take_first_matching("browser target session", |message| { + message["method"] == json!("Target.attachedToTarget") + && message["params"]["targetInfo"]["type"] == json!("browser") + }); + let browser_session_id = browser_attached["params"]["sessionId"] + .as_str() + .expect("browser target session id") + .to_owned(); + ctx.expect_result(89_101, json!({ "sessionId": browser_session_id }), None); + ctx.process_async(json!({ + "id": 89_102, + "sessionId": browser_session_id, + "method": "Target.setAutoAttach", + "params": { + "autoAttach": true, + "waitForDebuggerOnStart": true, + "flatten": true + } + })) + .await; + ctx.expect_result(89_102, json!({}), Some(&browser_session_id)); + ctx.take_all(); + + let popup_url = fixture.url("/plain?popup=two-debugger-barriers"); + ctx.process_async(json!({ + "id": 89_103, + "method": "Runtime.evaluate", + "sessionId": opener.session_id, + "params": { + "expression": format!("window.open('{popup_url}', '_blank') !== null"), + "returnByValue": true + } + })) + .await; + let evaluated = take_response_by_id(&mut ctx, 89_103); + assert_eq!(evaluated["result"]["result"]["value"], true); + let created = ctx.take_first_matching("two-owner popup target", |message| { + message["method"] == json!("Target.targetCreated") + && message["params"]["targetInfo"]["url"] == json!(popup_url) + }); + let popup_target_id = created["params"]["targetInfo"]["targetId"] + .as_str() + .expect("popup target id") + .to_owned(); + let root_attached = ctx.take_first_matching("root popup attachment", |message| { + message.get("sessionId").is_none() + && message["method"] == json!("Target.attachedToTarget") + && message["params"]["targetInfo"]["targetId"] == json!(popup_target_id) + }); + let browser_owned_attached = + ctx.take_first_matching("browser-owned popup attachment", |message| { + message["sessionId"] == json!(browser_session_id) + && message["method"] == json!("Target.attachedToTarget") + && message["params"]["targetInfo"]["targetId"] == json!(popup_target_id) + }); + assert_eq!(root_attached["params"]["waitingForDebugger"], true); + assert_eq!(browser_owned_attached["params"]["waitingForDebugger"], true); + let root_popup_session_id = root_attached["params"]["sessionId"] + .as_str() + .expect("root popup session id") + .to_owned(); + let browser_popup_session_id = browser_owned_attached["params"]["sessionId"] + .as_str() + .expect("browser-owned popup session id") + .to_owned(); + + ctx.process_async(json!({ + "id": 89_104, + "method": "Fetch.enable", + "sessionId": root_popup_session_id, + "params": { + "patterns": [{ + "urlPattern": "*", + "resourceType": "Document", + "requestStage": "Request" + }] + } + })) + .await; + ctx.expect_result(89_104, json!({}), Some(&root_popup_session_id)); + + ctx.process_async(json!({ + "id": 89_105, + "method": "Runtime.runIfWaitingForDebugger", + "sessionId": root_popup_session_id + })) + .await; + ctx.expect_result(89_105, json!({}), Some(&root_popup_session_id)); + assert!( + ctx.conn + .target_has_waiting_for_debugger_session(&popup_target_id), + "the second inspector session must keep the target behind its debugger barrier" + ); + assert!( + !ctx.sent + .iter() + .any(|message| message["method"] == json!("Fetch.requestPaused")), + "one of two waiting sessions must not release the popup navigation: {:?}", + ctx.sent + ); + + ctx.process_async(json!({ + "id": 89_106, + "method": "Target.detachFromTarget", + "params": { "sessionId": browser_popup_session_id } + })) + .await; + ctx.expect_result(89_106, json!({}), None); + assert!( + !ctx.conn + .target_has_waiting_for_debugger_session(&popup_target_id), + "detaching the final waiting session must release the target barrier" + ); + + fulfill_popup_document_and_evaluate( + &mut ctx, + 89_110, + &popup_target_id, + &root_popup_session_id, + &popup_url, + "all-debugger-barriers-released", + ) + .await; +} + +#[tokio::test(flavor = "multi_thread")] +async fn queued_popup_navigation_rechecks_a_late_debugger_barrier() { + let fixture = SmokeFixtureServer::start().await; + let mut ctx = TestContext::new(); + let opener = attached_smoke_session(&mut ctx, 90_000).await; + set_auto_attach_waiting_for_debugger(&mut ctx, 90_100).await; + + let popup_url = fixture.url("/plain?popup=late-debugger-barrier"); + let (popup_target_id, popup_session_id, browser_context_id) = + open_popup_from_session(&mut ctx, 90_101, &opener.session_id, &popup_url).await; + let action = crate::conn::PopupTargetNavigationOwnerAction::capture( + &ctx.conn, + &browser_context_id, + &popup_target_id, + popup_url, + crate::conn::PopupTargetNavigationKind::InitialDocumentAfterDebuggerResume, + ) + .expect("the paused popup should have an exact navigation owner action"); + + assert!( + ctx.conn + .release_waiting_for_debugger_session(Some(&popup_session_id)) + ); + assert!( + !ctx.conn + .target_has_waiting_for_debugger_session(&popup_target_id) + ); + + let late_session_id = "SID-late-debugger".to_owned(); + assert!( + ctx.conn + .prepare_auto_attached_page_session_binding(&popup_target_id, late_session_id.clone(),) + ); + let prepared = ctx.conn.prepare_auto_attach_session_commit( + late_session_id, + Some(opener.session_id.clone()), + true, + ); + let target_info = ctx + .conn + .browser_context_by_id(&browser_context_id) + .and_then(|browser_context| browser_context.devtools_target_info(&popup_target_id)) + .expect("popup target info"); + let _ = ctx + .conn + .commit_prepared_attach_event_plan(crate::conn::PreparedTargetAttach::new( + &popup_target_id, + target_info, + [prepared], + )); + assert!( + ctx.conn + .target_has_waiting_for_debugger_session(&popup_target_id), + "the late session must install a new target barrier before queued work runs", + ); + + let outcome = complete_popup_target_navigation_owner_action_async(&mut ctx.conn, action).await; + assert!(outcome.into_parts().0.is_empty()); + assert!( + !ctx.conn + .has_pending_document_navigation_for_session_owner(Some(&popup_session_id)), + "queued work must not start the target URL through a newly paused target", + ); + let page_url = ctx + .conn + .browser_context_by_id(&browser_context_id) + .and_then(|browser_context| loaded_page_for_target(browser_context, &popup_target_id)) + .map(|page| page.final_url().as_str()) + .expect("popup initial Page"); + assert_eq!(page_url, "about:blank"); +} diff --git a/moli-protocol/src/testing.rs b/moli-protocol/src/testing.rs index 977e3a02b..c592fef87 100644 --- a/moli-protocol/src/testing.rs +++ b/moli-protocol/src/testing.rs @@ -864,11 +864,17 @@ impl TestContext { return false; } CdpCommandTaskStep::Pending(pending) => { - let completed = pending.wait().await; - step = self - .conn - .complete_pending_command_dispatch_with_context(completed, command_context) - .await; + // Keep the test scheduler's pending-command boundary shaped + // like production. Some domain completions carry a complete + // renderer Page build, so composing both futures inline can + // exceed Rust's default test-thread stack before the owner + // turn gets a chance to yield. + let completed = Box::pin(pending.wait()).await; + step = Box::pin(self.conn.complete_pending_command_dispatch_with_context( + completed, + command_context, + )) + .await; } } } diff --git a/moli-renderer-v8/src/frame_owner_model/records.rs b/moli-renderer-v8/src/frame_owner_model/records.rs index d84f61e5a..d9713e413 100644 --- a/moli-renderer-v8/src/frame_owner_model/records.rs +++ b/moli-renderer-v8/src/frame_owner_model/records.rs @@ -749,6 +749,30 @@ impl DocumentLifecycleRecord { true } + /// Abort the current Document's remaining loading algorithm without + /// retiring the Document itself. + /// + /// Blink's `Document::CancelParsing()` moves readiness to `complete` but + /// marks the load event as completed instead of dispatching it. Keep the + /// same distinction here: queued lifecycle work still names a current + /// owner, but none of its old transition tokens remain admissible. + pub(super) fn stop_loading_without_load_event(&mut self) -> Option { + let previous_readiness = self.readiness?; + let ready_state_changed = previous_readiness != DocumentReadinessState::Complete; + + self.blockers.clear_for_retirement(); + self.incomplete_child_frames.clear(); + self.parsing_delay_token = None; + self.interactive_transition_token = None; + self.domcontentloaded_transition_token = None; + self.complete_transition_token = None; + self.readiness = Some(DocumentReadinessState::Complete); + self.load = DocumentLoadEventProgress::Suppressed; + self.child_load_delivery_admission = None; + + Some(ready_state_changed) + } + pub(super) fn complete_transition_is_pending( &self, transition_token: DocumentLoadDelayTokenId, diff --git a/moli-renderer-v8/src/frame_owner_model/store.rs b/moli-renderer-v8/src/frame_owner_model/store.rs index e9f0ace6f..722d17e5c 100644 --- a/moli-renderer-v8/src/frame_owner_model/store.rs +++ b/moli-renderer-v8/src/frame_owner_model/store.rs @@ -1481,6 +1481,25 @@ impl FrameOwnerStore { }) } + /// Stop the exact current main Document while keeping its Window and + /// Document identities live. + /// + /// The returned boolean reports whether the observable ready state still + /// needs to transition to `complete`. `None` means the supplied owner is + /// stale or no longer owns lifecycle state. + pub(crate) fn stop_current_main_document_loading( + &mut self, + owner: FrameDocumentTaskOwner, + ) -> Option { + if !self.main_document_task_owner_is_current(owner) { + return None; + } + self.documents + .get_mut(&owner.document_id)? + .lifecycle_progress + .stop_loading_without_load_event() + } + pub(crate) fn apply_current_main_document_complete_transition( &mut self, action: MainDocumentCompleteLifecycleAction, diff --git a/moli-renderer-v8/src/native_bridge/context_host/main_document_lifecycle.rs b/moli-renderer-v8/src/native_bridge/context_host/main_document_lifecycle.rs index c31e2c4a3..8e47f78ff 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/main_document_lifecycle.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/main_document_lifecycle.rs @@ -336,6 +336,14 @@ impl JsContextHost { .current_main_document_complete_transition_is_ready(owner) } + pub(crate) fn stop_current_main_document_loading( + &mut self, + owner: FrameDocumentTaskOwner, + ) -> Option { + self.frame_owner_store + .stop_current_main_document_loading(owner) + } + pub(crate) fn apply_current_main_document_complete_transition( &mut self, action: MainDocumentCompleteLifecycleAction, diff --git a/moli-renderer-v8/src/network/backend/runtime.rs b/moli-renderer-v8/src/network/backend/runtime.rs index db8d6caf9..6e8eb96b6 100644 --- a/moli-renderer-v8/src/network/backend/runtime.rs +++ b/moli-renderer-v8/src/network/backend/runtime.rs @@ -187,6 +187,10 @@ impl BrowserResourceRuntime { self.inner.client.cookie_store() } + pub fn browser_identity(&self) -> &moli_browser_profile::BrowserIdentityProfile { + self.inner.client.browser_identity() + } + pub fn matches_fetch_config(&self, config: &FetchConfig) -> bool { self.inner.client.matches_config(config) } diff --git a/moli-renderer-v8/src/network/request_client.rs b/moli-renderer-v8/src/network/request_client.rs index d9d515c6f..97a4b900d 100644 --- a/moli-renderer-v8/src/network/request_client.rs +++ b/moli-renderer-v8/src/network/request_client.rs @@ -64,6 +64,13 @@ impl ResourceRequestClient { self.resource_runtime.clone() } + pub(crate) fn replace_browser_resource_runtime( + &mut self, + resource_runtime: BrowserResourceRuntime, + ) { + self.resource_runtime = resource_runtime; + } + pub fn shares_resource_runtime_with(&self, other: &Self) -> bool { self.resource_runtime .shares_state_with(&other.resource_runtime) diff --git a/moli-renderer-v8/src/runtime/owner.rs b/moli-renderer-v8/src/runtime/owner.rs index db3b44dae..e102dbea8 100644 --- a/moli-renderer-v8/src/runtime/owner.rs +++ b/moli-renderer-v8/src/runtime/owner.rs @@ -114,7 +114,11 @@ pub struct RendererPreparedDocumentCommitConfiguration { pub emulated_media: crate::protocol_types::EmulatedMediaOverrides, pub idle_override: Option, pub viewport_surface: Option, + pub browser_resource_runtime: crate::network::BrowserResourceRuntime, + pub navigator_identity: moli_browser_profile::BrowserIdentityProfile, pub network_offline: bool, + pub bypass_service_worker: bool, + pub cache_disabled: bool, pub blocked_url_patterns: Vec, pub fetch_subresource_interception_enabled: bool, pub fetch_subresource_interception_resource_type: Option, @@ -138,6 +142,7 @@ pub struct RendererCreateHtmlPageRequest { pub response_status: u16, pub response_headers: Vec<(String, String)>, pub loader: ResourceRequestClient, + pub navigator_identity: moli_browser_profile::BrowserIdentityProfile, pub web_storage: crate::RendererWebStorageHandles, pub final_url: Url, pub html: String, @@ -187,6 +192,7 @@ pub struct RendererCreateStreamingRawPageRequest { pub response_status: u16, pub response_headers: Vec<(String, String)>, pub loader: ResourceRequestClient, + pub navigator_identity: moli_browser_profile::BrowserIdentityProfile, pub web_storage: crate::RendererWebStorageHandles, pub raw_body: ExternalRawDocumentBodyStream, pub document_start_scripts: Vec, @@ -2134,6 +2140,7 @@ impl RendererOwnerHandle { response_status, response_headers, loader: loader.clone(), + navigator_identity: loader.browser_identity().clone(), web_storage, final_url, html, @@ -2210,6 +2217,7 @@ impl RendererOwnerHandle { response_status, response_headers, loader: loader.clone(), + navigator_identity: loader.browser_identity().clone(), web_storage, raw_body, document_start_scripts, @@ -6807,6 +6815,7 @@ impl RendererOwnerHandle { response_status, response_headers, loader, + navigator_identity, web_storage, final_url, html, @@ -6902,6 +6911,7 @@ impl RendererOwnerHandle { runtime_isolated_worlds, permission_overrides, extra_http_headers, + navigator_identity, document_policy_container, document_default_language, document_last_modified, @@ -7119,6 +7129,7 @@ impl RendererOwnerHandle { response_status, response_headers, loader, + navigator_identity, web_storage, raw_body, document_start_scripts, @@ -7201,6 +7212,7 @@ impl RendererOwnerHandle { runtime_isolated_worlds, permission_overrides, extra_http_headers, + navigator_identity, document_policy_container, document_default_language, document_last_modified, diff --git a/moli-renderer-v8/src/runtime/owner_local_store/mod.rs b/moli-renderer-v8/src/runtime/owner_local_store/mod.rs index a2692d86c..c0855e951 100644 --- a/moli-renderer-v8/src/runtime/owner_local_store/mod.rs +++ b/moli-renderer-v8/src/runtime/owner_local_store/mod.rs @@ -919,7 +919,17 @@ impl RendererOwnerLocalStore { request.emulated_media = configuration.emulated_media; request.idle_override = configuration.idle_override; request.viewport_surface = configuration.viewport_surface; + request + .loader + .replace_browser_resource_runtime(configuration.browser_resource_runtime); + request.navigator_identity = configuration.navigator_identity; request.network_offline = configuration.network_offline; + request + .loader + .set_bypass_service_worker(configuration.bypass_service_worker); + request + .loader + .set_cache_disabled(configuration.cache_disabled); request.blocked_url_patterns = configuration.blocked_url_patterns; request.fetch_subresource_interception_enabled = configuration.fetch_subresource_interception_enabled; diff --git a/moli-renderer-v8/src/runtime/page_commands.rs b/moli-renderer-v8/src/runtime/page_commands.rs index b3c3b46fc..6b47635ea 100644 --- a/moli-renderer-v8/src/runtime/page_commands.rs +++ b/moli-renderer-v8/src/runtime/page_commands.rs @@ -411,7 +411,7 @@ impl PageVm { ), ), RendererPageCommand::StopDocumentLifecycle => { - self.stop_document_lifecycle(); + self.stop_document_lifecycle()?; Ok(RendererPageReply::Unit) } RendererPageCommand::SearchTextByLines { @@ -1033,8 +1033,11 @@ impl PageVm { .set_javascript_dialog_handler_enabled(enabled); Ok(RendererPageReply::Unit) } - RendererPageCommand::ReplaceBrowserResourceRuntime(resource_runtime) => { - self.replace_browser_resource_runtime(&resource_runtime); + RendererPageCommand::ReplaceBrowserResourceRuntime { + resource_runtime, + navigator_identity, + } => { + self.replace_browser_resource_runtime(&resource_runtime, &navigator_identity); Ok(RendererPageReply::Unit) } RendererPageCommand::RetireDocumentResourceAuthorities => { diff --git a/moli-renderer-v8/src/runtime/page_network.rs b/moli-renderer-v8/src/runtime/page_network.rs index 6a6015e6d..50343f436 100644 --- a/moli-renderer-v8/src/runtime/page_network.rs +++ b/moli-renderer-v8/src/runtime/page_network.rs @@ -219,6 +219,7 @@ impl PageVm { pub(crate) fn replace_browser_resource_runtime( &mut self, resource_runtime: &crate::network::BrowserResourceRuntime, + navigator_identity: &moli_browser_profile::BrowserIdentityProfile, ) { // Replacing a browser/network backend must not replace the live // target's Page policy. Pair the new backend with the exact policy @@ -230,8 +231,12 @@ impl PageVm { ); let document_loader = self .vm_mut() - .replace_document_resource_runtime(&page_loader); + .replace_document_resource_runtime_with_navigator_identity( + &page_loader, + navigator_identity, + ); self.request_client = document_loader.request_client().clone(); + self.navigator_identity = navigator_identity.clone(); } pub(crate) fn retire_document_resource_authorities(&mut self) { diff --git a/moli-renderer-v8/src/runtime/page_surface.rs b/moli-renderer-v8/src/runtime/page_surface.rs index 62bfbdcd7..0ea62e9ea 100644 --- a/moli-renderer-v8/src/runtime/page_surface.rs +++ b/moli-renderer-v8/src/runtime/page_surface.rs @@ -5201,7 +5201,10 @@ pub enum RendererPageCommand { resource_type: Option, }, SetJavaScriptDialogHandlerEnabled(bool), - ReplaceBrowserResourceRuntime(crate::network::BrowserResourceRuntime), + ReplaceBrowserResourceRuntime { + resource_runtime: crate::network::BrowserResourceRuntime, + navigator_identity: moli_browser_profile::BrowserIdentityProfile, + }, RetireDocumentResourceAuthorities, ApplyDocumentCookieFacadeOverrides(moli_cookie_jar::BrowserCookieFacadeOverrides), ClearDocumentCookieFacadeOverrides, 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 6361313b4..44af06057 100644 --- a/moli-renderer-v8/src/runtime/page_vm/followed_navigation.rs +++ b/moli-renderer-v8/src/runtime/page_vm/followed_navigation.rs @@ -1182,6 +1182,7 @@ impl PageVm { runtime_isolated_worlds: self.runtime_isolated_worlds.clone(), permission_overrides: self.permission_overrides.clone(), extra_http_headers: self.extra_http_headers.clone(), + navigator_identity: self.navigator_identity.clone(), document_policy_container: crate::document_runtime::DocumentPolicyContainer { document_content_security_policies: self.vm().document_content_security_policies(), ..Default::default() diff --git a/moli-renderer-v8/src/runtime/page_vm/mod.rs b/moli-renderer-v8/src/runtime/page_vm/mod.rs index c6eda3047..01920fd9c 100644 --- a/moli-renderer-v8/src/runtime/page_vm/mod.rs +++ b/moli-renderer-v8/src/runtime/page_vm/mod.rs @@ -1073,6 +1073,7 @@ pub(crate) struct PageVmEnvConfig { pub(crate) runtime_isolated_worlds: Vec, pub(crate) permission_overrides: Vec, pub(crate) extra_http_headers: Vec<(String, String)>, + pub(crate) navigator_identity: moli_browser_profile::BrowserIdentityProfile, pub(crate) document_policy_container: crate::document_runtime::DocumentPolicyContainer, pub(crate) document_default_language: Option, pub(crate) document_last_modified: Option, @@ -1588,6 +1589,10 @@ pub(crate) struct PageVm { /// committed-Document loader lives only in `ScriptVm`'s exact-owner /// registry and may change across `document.open()` or navigation. pub(super) request_client: ResourceRequestClient, + /// Identity exposed by the current Document's Navigator. This is distinct + /// from the request transport identity when multiple DevTools sessions + /// contribute Emulation overrides. + pub(super) navigator_identity: moli_browser_profile::BrowserIdentityProfile, pub(super) runtime_isolated_worlds: Vec, pub(super) permission_overrides: Vec, pub(super) document_start_scripts: Vec, @@ -1851,11 +1856,12 @@ impl PageVm { self.document_lifecycle.drain_live_events() } - pub(super) fn stop_document_lifecycle(&self) { + pub(super) fn stop_document_lifecycle(&mut self) -> Result<()> { let _ = self.document_lifecycle.request_termination( self.document_lifecycle.identity(), RendererDocumentTerminationReason::Stopped, ); + self.vm_mut().stop_current_main_document_loading() } pub(crate) fn document_lifecycle_wait_outcome( @@ -4262,6 +4268,7 @@ impl PageVm { env.reserved_service_worker_client_id, )?; let mut vm = vm_bootstrap.finish()?; + vm.set_document_navigator_identity(&env.navigator_identity); vm.set_layout_policy(env.layout_policy); vm.install_page_task_capabilities(page_task_capabilities); vm.set_root_document_lifecycle(document_lifecycle.clone()); @@ -4290,6 +4297,7 @@ impl PageVm { next_module_script_evaluation_reaction_id: 0, target_stage: PageVmInitStage::Load, request_client: document_loader.request_client().clone(), + navigator_identity: env.navigator_identity.clone(), runtime_isolated_worlds: env.runtime_isolated_worlds.clone(), permission_overrides: env.permission_overrides.clone(), document_start_scripts: env.document_start_scripts.clone(), diff --git a/moli-renderer-v8/src/runtime/page_vm/test_support.rs b/moli-renderer-v8/src/runtime/page_vm/test_support.rs index e8ff61434..97f086914 100644 --- a/moli-renderer-v8/src/runtime/page_vm/test_support.rs +++ b/moli-renderer-v8/src/runtime/page_vm/test_support.rs @@ -662,6 +662,7 @@ fn minimal_test_page_vm_env_config() -> PageVmEnvConfig { runtime_isolated_worlds: Vec::new(), permission_overrides: Vec::new(), extra_http_headers: Vec::new(), + navigator_identity: Default::default(), document_policy_container: Default::default(), document_default_language: None, document_last_modified: None, diff --git a/moli-renderer-v8/src/runtime/page_vm/tests/lifecycle.rs b/moli-renderer-v8/src/runtime/page_vm/tests/lifecycle.rs index ae15760ac..b9b62ae3b 100644 --- a/moli-renderer-v8/src/runtime/page_vm/tests/lifecycle.rs +++ b/moli-renderer-v8/src/runtime/page_vm/tests/lifecycle.rs @@ -9,7 +9,8 @@ use super::*; use crate::{ RendererDocumentLifecycleEventKind, RendererDocumentLifecycleIdentity, RendererDocumentLifecycleMilestone, RendererDocumentLifecycleWaitOutcome, - RendererDocumentTerminationReason, RendererLifecycleStartReason, RendererPageState, + RendererDocumentTerminationReason, RendererLifecycleStartReason, RendererPageReply, + RendererPageState, page_task_queue::PostParseLifecycleWork, runtime::document_lifecycle_turn::DocumentLifecycleNavigationTiming, script_vm::{ @@ -1408,6 +1409,116 @@ fn main_document_lifecycle_coordinator_preserves_applied_callback_facts() { }); } +#[test] +fn stop_loading_completes_readiness_without_dispatching_window_load() { + run_page_vm_local_runtime_async_test("page-vm-stop-loading-lifecycle", || async move { + let mut page_vm = test_page_vm(); + let local_executor = page_vm.local_executor.clone(); + + local_executor + .run(async move { + let _initial = page_vm.take_page_creation_artifacts(); + page_vm.vm_mut().eval( + r#" +globalThis.__stopLoadingLifecycleEvents = []; +document.addEventListener("readystatechange", () => { + __stopLoadingLifecycleEvents.push(`readystatechange:${document.readyState}`); + queueMicrotask(() => { + __stopLoadingLifecycleEvents.push(`microtask:${document.readyState}`); + }); +}); +window.addEventListener("load", () => { + __stopLoadingLifecycleEvents.push("load"); +}); +window.addEventListener("pageshow", () => { + __stopLoadingLifecycleEvents.push("pageshow"); +}); +"installed" +"#, + )?; + + let owner = page_vm + .vm() + .current_main_document_task_owner() + .expect("stop-loading fixture requires a current Document owner"); + let interactive = page_vm + .vm_mut() + .finish_current_main_document_parsing(owner) + .expect("parser completion should prepare interactive work"); + execute_main_document_lifecycle_on_owner_local_task( + &mut page_vm, + MainDocumentLifecycleBody::Interactive(interactive), + ) + .await?; + execute_main_document_lifecycle_on_owner_local_task( + &mut page_vm, + MainDocumentLifecycleBody::DomContentLoaded { owner }, + ) + .await?; + page_vm + .vm_mut() + .eval("__stopLoadingLifecycleEvents.length = 0; 'cleared'")?; + + let reply = page_vm + .dispatch_renderer_page_command_async( + RendererPageCommand::StopDocumentLifecycle, + ) + .await?; + assert!(matches!(reply, RendererPageReply::Unit)); + assert_eq!( + page_vm.vm_mut().eval( + "`${document.readyState}|${__stopLoadingLifecycleEvents.join(',')}`", + )?, + "complete|readystatechange:complete,microtask:complete", + "stopLoading must synchronously complete readiness and its task checkpoint", + ); + + let snapshot = page_vm.document_lifecycle.current_snapshot(); + assert!(snapshot.dom_content_loaded.is_some()); + assert!(snapshot.load.is_none()); + assert!(matches!( + snapshot.terminated, + Some(crate::RendererLifecycleTerminationStamp { + reason: RendererDocumentTerminationReason::Stopped, + .. + }) + )); + assert_eq!( + page_vm.vm().current_main_document_task_owner(), + Some(owner), + "stopping loading must not replace the current Document owner", + ); + + let run = execute_main_document_lifecycle_on_owner_local_task( + &mut page_vm, + MainDocumentLifecycleBody::WindowLoad { owner }, + ) + .await?; + assert!(matches!( + run.completion.target(), + MainDocumentLifecycleTargetEffect::NotApplied { + reason: MainDocumentLifecycleTargetRejection::TransitionRejected, + current_owner: Some(current_owner), + } if current_owner == owner + )); + assert_eq!( + run.completion.callback(), + MainDocumentLifecycleCallbackEffect::NotEntered, + ); + assert_eq!( + page_vm + .vm_mut() + .eval("__stopLoadingLifecycleEvents.join(',')")?, + "readystatechange:complete,microtask:complete", + "retired lifecycle work must not dispatch load or pageshow", + ); + Ok::<_, anyhow::Error>(()) + }) + .await + .expect("stop-loading lifecycle should reconcile"); + }); +} + #[test] fn ordinary_main_lifecycle_body_leaves_listener_reaction_for_its_typed_checkpoint() { run_page_vm_local_runtime_async_test( diff --git a/moli-renderer-v8/src/runtime/page_vm/tests/mod.rs b/moli-renderer-v8/src/runtime/page_vm/tests/mod.rs index 020a561e8..6a5396f25 100644 --- a/moli-renderer-v8/src/runtime/page_vm/tests/mod.rs +++ b/moli-renderer-v8/src/runtime/page_vm/tests/mod.rs @@ -1795,6 +1795,7 @@ fn test_page_vm_with_loader_dom_host_hooks_and_response_referrer_policy( runtime_isolated_worlds: vec![], permission_overrides: vec![], extra_http_headers, + navigator_identity: loader.browser_identity().clone(), document_policy_container: crate::document_runtime::DocumentPolicyContainer { referrer_policy: response_referrer_policy, ..Default::default() @@ -3121,6 +3122,7 @@ fn default_runtime_hooks_reject_direct_no_owner_page_vm_construction() { runtime_isolated_worlds: vec![], permission_overrides: vec![], extra_http_headers: Vec::new(), + navigator_identity: loader.browser_identity().clone(), document_policy_container: Default::default(), document_default_language: None, document_last_modified: None, diff --git a/moli-renderer-v8/src/runtime/phase_one/mod.rs b/moli-renderer-v8/src/runtime/phase_one/mod.rs index fdb6ba4b4..868f23b92 100644 --- a/moli-renderer-v8/src/runtime/phase_one/mod.rs +++ b/moli-renderer-v8/src/runtime/phase_one/mod.rs @@ -2247,6 +2247,7 @@ document.body.setAttribute('data-error-state', [ runtime_isolated_worlds: vec![], permission_overrides: vec![], extra_http_headers: vec![], + navigator_identity: Default::default(), document_policy_container: Default::default(), document_default_language: None, document_last_modified: None, @@ -5979,6 +5980,7 @@ globalThis.__outerDocumentWriteScriptContinued = true; runtime_isolated_worlds: vec![], permission_overrides: vec![], extra_http_headers: vec![], + navigator_identity: Default::default(), document_policy_container: Default::default(), document_default_language: None, document_last_modified: None, @@ -14383,6 +14385,7 @@ document.body.setAttribute('data-result', [ runtime_isolated_worlds: vec![], permission_overrides: vec![], extra_http_headers: vec![], + navigator_identity: Default::default(), document_policy_container: Default::default(), document_default_language: None, document_last_modified: None, @@ -14563,6 +14566,7 @@ document.body.setAttribute('data-result', [ runtime_isolated_worlds: vec![], permission_overrides: vec![], extra_http_headers: vec![], + navigator_identity: Default::default(), document_policy_container: Default::default(), document_default_language: None, document_last_modified: None, @@ -14792,6 +14796,7 @@ document.body.setAttribute('data-result', [ runtime_isolated_worlds: vec![], permission_overrides: vec![], extra_http_headers: vec![], + navigator_identity: Default::default(), document_policy_container: Default::default(), document_default_language: None, document_last_modified: None, @@ -16790,6 +16795,7 @@ document.body.setAttribute("data-range", [ runtime_isolated_worlds: vec![], permission_overrides: vec![], extra_http_headers: vec![], + navigator_identity: Default::default(), document_policy_container: Default::default(), document_default_language: None, document_last_modified: None, @@ -16918,6 +16924,7 @@ document.body.setAttribute("data-range", [ runtime_isolated_worlds: vec![], permission_overrides: vec![], extra_http_headers: vec![], + navigator_identity: Default::default(), document_policy_container: Default::default(), document_default_language: None, document_last_modified: None, @@ -17056,6 +17063,7 @@ document.body.setAttribute("data-range", [ runtime_isolated_worlds: vec![], permission_overrides: vec![], extra_http_headers: vec![], + navigator_identity: Default::default(), document_policy_container: Default::default(), document_default_language: None, document_last_modified: None, @@ -17229,6 +17237,7 @@ document.body.setAttribute("data-range", [ runtime_isolated_worlds: vec![], permission_overrides: vec![], extra_http_headers: vec![], + navigator_identity: Default::default(), document_policy_container: Default::default(), document_default_language: None, document_last_modified: None, @@ -17552,6 +17561,7 @@ document.body.setAttribute("data-range", [ runtime_isolated_worlds: vec![], permission_overrides: vec![], extra_http_headers: vec![], + navigator_identity: Default::default(), document_policy_container: Default::default(), document_default_language: None, document_last_modified: None, @@ -17722,6 +17732,7 @@ document.body.setAttribute("data-range", [ runtime_isolated_worlds: vec![], permission_overrides: vec![], extra_http_headers: vec![], + navigator_identity: Default::default(), document_policy_container: Default::default(), document_default_language: None, document_last_modified: None, @@ -17810,6 +17821,7 @@ document.body.setAttribute("data-range", [ runtime_isolated_worlds: vec![], permission_overrides: vec![], extra_http_headers: vec![], + navigator_identity: Default::default(), document_policy_container: Default::default(), document_default_language: None, document_last_modified: None, diff --git a/moli-renderer-v8/src/runtime/phase_one/streaming.rs b/moli-renderer-v8/src/runtime/phase_one/streaming.rs index c7c7d3c21..35744a6aa 100644 --- a/moli-renderer-v8/src/runtime/phase_one/streaming.rs +++ b/moli-renderer-v8/src/runtime/phase_one/streaming.rs @@ -940,6 +940,7 @@ mod tests { runtime_isolated_worlds: vec![], permission_overrides: vec![], extra_http_headers: vec![], + navigator_identity: Default::default(), document_policy_container: Default::default(), document_default_language: None, document_last_modified: None, diff --git a/moli-renderer-v8/src/runtime/tests.rs b/moli-renderer-v8/src/runtime/tests.rs index d525fc441..690a47bc7 100644 --- a/moli-renderer-v8/src/runtime/tests.rs +++ b/moli-renderer-v8/src/runtime/tests.rs @@ -1510,7 +1510,11 @@ async fn streaming_unstyled_xml_converts_live_document_before_domcontentloaded() emulated_media: Default::default(), idle_override: None, viewport_surface: None, + 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, @@ -1643,7 +1647,11 @@ async fn prepared_streaming_xml_document_waits_for_permit_and_uses_latest_config emulated_media: Default::default(), idle_override: None, viewport_surface: None, + 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, @@ -2616,7 +2624,11 @@ globalThis.__preparedCommitObserved = JSON.stringify([ emulated_media: Default::default(), idle_override: None, viewport_surface: None, + 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, diff --git a/moli-renderer-v8/src/script_vm.rs b/moli-renderer-v8/src/script_vm.rs index 01b2e46d8..12411619d 100644 --- a/moli-renderer-v8/src/script_vm.rs +++ b/moli-renderer-v8/src/script_vm.rs @@ -4730,9 +4730,22 @@ impl ScriptVm { /// The Document authority is installed before realm bootstrap and remains /// stable here. Existing leases keep their captured request client; only /// subsequently registered loads observe this replacement transport. + #[cfg(test)] pub(super) fn replace_document_resource_runtime( &mut self, request_client: &ResourceRequestClient, + ) -> DocumentResourceLoader { + let navigator_identity = request_client.browser_identity().clone(); + self.replace_document_resource_runtime_with_navigator_identity( + request_client, + &navigator_identity, + ) + } + + pub(super) fn replace_document_resource_runtime_with_navigator_identity( + &mut self, + request_client: &ResourceRequestClient, + navigator_identity: &moli_browser_profile::BrowserIdentityProfile, ) -> DocumentResourceLoader { let current = self .current_main_document_resource_loader() @@ -4743,12 +4756,21 @@ impl ScriptVm { .replace_main_document_resource_transport(&document_loader); self.document_runtime .set_cookie_store(document_loader.request_client().cookie_store()); - let identity = document_loader.request_client().browser_identity().clone(); - let _ = self - .with_default_context_scope(|scope, _| set_window_navigator_identity(scope, &identity)); + let _ = self.with_default_context_scope(|scope, _| { + set_window_navigator_identity(scope, navigator_identity) + }); document_loader } + pub(super) fn set_document_navigator_identity( + &mut self, + navigator_identity: &moli_browser_profile::BrowserIdentityProfile, + ) { + let _ = self.with_default_context_scope(|scope, _| { + set_window_navigator_identity(scope, navigator_identity) + }); + } + pub(super) fn set_web_storage_handles(&mut self, handles: &crate::RendererWebStorageHandles) { self._context_host .borrow_mut() diff --git a/moli-renderer-v8/src/script_vm/main_document_lifecycle_body.rs b/moli-renderer-v8/src/script_vm/main_document_lifecycle_body.rs index 83f673974..f8cdfca0d 100644 --- a/moli-renderer-v8/src/script_vm/main_document_lifecycle_body.rs +++ b/moli-renderer-v8/src/script_vm/main_document_lifecycle_body.rs @@ -19,6 +19,31 @@ use crate::dom::native::DocumentReadyState; use crate::frame_owner_model::{FrameDocumentTaskOwner, MainDocumentInteractiveLifecycleAction}; impl ScriptVm { + /// Apply Blink's `CancelParsing()` readiness boundary for Page.stopLoading. + /// + /// This is deliberately separate from the ordinary Window-load body: the + /// Document remains current and receives `readystatechange`, while the + /// frame-owner lifecycle suppresses all later DCL/load delivery. + pub(crate) fn stop_current_main_document_loading(&mut self) -> anyhow::Result<()> { + let Some(owner) = self.current_main_document_task_owner() else { + return Ok(()); + }; + let ready_state_changed = self + ._context_host + .borrow_mut() + .stop_current_main_document_loading(owner) + .unwrap_or(false); + + if ready_state_changed { + self.set_document_ready_state(DocumentReadyState::Complete)?; + let _dispatch = + self.dispatch_document_lifecycle_event_body_best_effort("readystatechange"); + } + + let checkpoint = self.finish_main_document_lifecycle_checkpoint(); + self.finish_main_document_lifecycle_turn(checkpoint) + } + /// Start one lifecycle body without performing any task-end or internal /// lifecycle checkpoint. pub(crate) fn begin_main_document_lifecycle_body( diff --git a/moli-renderer-v8/src/script_vm/tests/lazy_window_surfaces/seeds.rs b/moli-renderer-v8/src/script_vm/tests/lazy_window_surfaces/seeds.rs index 32c82d864..0929a9978 100644 --- a/moli-renderer-v8/src/script_vm/tests/lazy_window_surfaces/seeds.rs +++ b/moli-renderer-v8/src/script_vm/tests/lazy_window_surfaces/seeds.rs @@ -26,6 +26,41 @@ async fn initial_navigator_materializes_from_the_committed_document_authority_se ); } +#[test] +fn replacing_document_transport_keeps_wire_and_navigator_identities_distinct() { + const WIRE_USER_AGENT: &str = "Moli-Wire-Identity/1.0"; + const NAVIGATOR_USER_AGENT: &str = "Moli-Navigator-Identity/1.0"; + + let mut vm = new_storage_test_vm("https://navigator-transport-split.test/"); + let mut fetch_config = moli_fetch::FetchConfig::default(); + fetch_config.set_user_agent(WIRE_USER_AGENT); + let loader = ResourceRequestClient::new(&fetch_config).expect("loader"); + let navigator_identity = + moli_browser_profile::BrowserIdentityProfile::new(NAVIGATOR_USER_AGENT, "fr-FR"); + + let document_loader = + vm.replace_document_resource_runtime_with_navigator_identity(&loader, &navigator_identity); + + assert_eq!( + document_loader + .request_client() + .browser_identity() + .user_agent(), + WIRE_USER_AGENT, + "the replacement transport must keep the browser-side request identity" + ); + assert_eq!( + vm.eval("navigator.userAgent") + .expect("Navigator should expose the renderer identity"), + NAVIGATOR_USER_AGENT + ); + assert_eq!( + vm.eval("navigator.language") + .expect("Navigator language should expose the renderer identity"), + "fr-FR" + ); +} + #[tokio::test(flavor = "current_thread")] async fn navigator_identity_seed_keeps_window_metadata_and_network_profile_coherent() { const USER_AGENT: &str = "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/146.1.2.3 Safari/537.36";