diff --git a/moli-protocol-server/src/cdp_scheduler.rs b/moli-protocol-server/src/cdp_scheduler.rs index 75ce158bd3..7277895b28 100644 --- a/moli-protocol-server/src/cdp_scheduler.rs +++ b/moli-protocol-server/src/cdp_scheduler.rs @@ -1400,7 +1400,9 @@ impl CdpScheduler { // background command response remains the completion authority. if !observed_download && !observed_lifecycle_protocol_event - && !self.has_pending_javascript_dialog() + && !self + .conn + .has_pending_javascript_dialog_for_devtools_context(&navigation_context) { if let Some(key) = document_lifecycle_wait_key.as_ref() { let wait_state = self @@ -1480,7 +1482,9 @@ impl CdpScheduler { .conn .devtools_document_lifecycle_wait_state(context, key) == moli_protocol::DevToolsDocumentLifecycleWaitState::Pending - && !self.has_pending_javascript_dialog() + && !self + .conn + .has_pending_javascript_dialog_for_devtools_context(context) { let Some(input) = receivers.recv_interleaved_input().await else { return Err(RendererOutputTransportFailure::new( @@ -1786,7 +1790,10 @@ impl CdpScheduler { if !self.has_deferred_main_document_load_completion_for_devtools_context(context) { return Ok(out); } - if self.has_pending_javascript_dialog() { + if self + .conn + .has_pending_javascript_dialog_for_devtools_context(context) + { return Ok(out); } let Some(input) = receivers.recv_interleaved_input().await else { @@ -1813,9 +1820,15 @@ impl CdpScheduler { ) -> ProtocolOutputSequence { let mut out = ProtocolOutputSequence::empty(); let mut snapshot = self.queues.take_external_load_wait_snapshot(); + let mut retained = VecDeque::new(); + let mut blocked_target_ids = Vec::new(); while let Some(mut residence) = snapshot.pop_front() { self.queues .satisfy_checked_out_client_turn_predecessor(&mut residence); + let target_ids = self.protocol_residence_navigation_gate_target_ids(&residence); + let has_pending_target_predecessor = target_ids + .iter() + .any(|target_id| blocked_target_ids.contains(target_id)); let has_pending_scheduler_predecessor = !residence.is_ready_to_complete(); let pending_load_observation = matches!( &residence, @@ -1823,13 +1836,23 @@ impl CdpScheduler { if work.kind() == ProtocolSchedulerWorkKind::MainDocumentLoadOwnerAction && !work.is_ready() ); - if has_pending_scheduler_predecessor || pending_load_observation { - snapshot.push_front(residence); - self.queues.restore_snapshot_to_front(snapshot); - return out; + if has_pending_target_predecessor + || has_pending_scheduler_predecessor + || pending_load_observation + { + retained.push_back(residence); + if target_ids.is_empty() { + retained.append(&mut snapshot); + break; + } + append_unique_target_ids(&mut blocked_target_ids, target_ids); + continue; } out.append(self.complete_protocol_residence(residence).await); } + if !retained.is_empty() { + self.queues.restore_snapshot_to_front(retained); + } out } diff --git a/moli-protocol-server/src/protocol_server/tests/classic/extracted/navigation.rs b/moli-protocol-server/src/protocol_server/tests/classic/extracted/navigation.rs index 191aa0a474..64676ced7d 100644 --- a/moli-protocol-server/src/protocol_server/tests/classic/extracted/navigation.rs +++ b/moli-protocol-server/src/protocol_server/tests/classic/extracted/navigation.rs @@ -2158,10 +2158,184 @@ async fn webdriver_classic_sync_alert_during_eager_load_does_not_deadlock_sessio .await; classic_sync_alert_open_text(&app, &session_id).await; + let (status, accepted) = classic_sync_alert_request( + &app, + Method::POST, + &format!("/session/{session_id}/alert/accept"), + Some(json!({})), + ) + .await; + assert_eq!(status, StatusCode::OK, "{accepted:?}"); + assert_eq!(accepted, json!({ "value": null })); + let (status, resumed) = classic_sync_alert_request( + &app, + Method::POST, + &format!("/session/{session_id}/execute/sync"), + Some(json!({ + "script": "return [document.title, document.body.textContent.includes('after')];", + "args": [] + })), + ) + .await; + assert_eq!(status, StatusCode::OK, "{resumed:?}"); + assert_eq!(resumed, json!({ "value": ["Sync Alert", true] })); + classic_sync_alert_delete(&app, &session_id).await; fixture_server.abort(); } +async fn classic_navigation_waits_while_another_window_has_an_alert(strategy: &str) { + let resource_requested = Arc::new(tokio::sync::Notify::new()); + let release_resource = Arc::new(tokio::sync::Notify::new()); + let requested_for_route = Arc::clone(&resource_requested); + let release_for_route = Arc::clone(&release_resource); + let page = match strategy { + "normal" => { + "Blocked Load

loaded

" + } + "eager" => { + "Blocked DCL

loaded

" + } + _ => unreachable!("unsupported page load strategy"), + }; + let fixture_app = Router::new() + .route( + "/page", + get(move || async move { ([(header::CONTENT_TYPE, "text/html")], page) }), + ) + .route( + "/blocked.js", + get(move || { + let requested = Arc::clone(&requested_for_route); + let release = Arc::clone(&release_for_route); + async move { + requested.notify_one(); + release.notified().await; + ( + [(header::CONTENT_TYPE, "text/javascript")], + "window.blockedScriptRan = true;", + ) + } + }), + ); + let (fixture_addr, _fixture_server) = + spawn_dedicated_fixture_server(fixture_app, "classic-other-window-alert"); + let app = build_router(test_state()); + let session_id = + classic_sync_alert_session(&app, json!({ "pageLoadStrategy": strategy })).await; + let window_path = format!("/session/{session_id}/window"); + let (status, original) = + classic_sync_alert_request(&app, Method::GET, &window_path, None).await; + assert_eq!(status, StatusCode::OK, "{original:?}"); + let original_handle = original["value"].as_str().expect("original window handle"); + let (status, created) = classic_sync_alert_request( + &app, + Method::POST, + &format!("/session/{session_id}/window/new"), + Some(json!({ "type": "window" })), + ) + .await; + assert_eq!(status, StatusCode::OK, "{created:?}"); + let other_handle = created["value"]["handle"] + .as_str() + .expect("other window handle"); + + classic_sync_alert_navigate( + &app, + &session_id, + &classic_data_url("

after

"), + ) + .await; + classic_sync_alert_open_text(&app, &session_id).await; + let (status, switched) = classic_sync_alert_request( + &app, + Method::POST, + &window_path, + Some(json!({ "handle": other_handle })), + ) + .await; + assert_eq!(status, StatusCode::OK, "{switched:?}"); + assert_eq!(switched, json!({ "value": null })); + + let navigation_app = app.clone(); + let navigation_session = session_id.clone(); + let mut navigation = tokio::spawn(async move { + classic_sync_alert_request( + &navigation_app, + Method::POST, + &format!("/session/{navigation_session}/url"), + Some(json!({ "url": format!("http://{fixture_addr}/page") })), + ) + .await + }); + timeout(CLASSIC_SYNC_ALERT_DEADLINE, resource_requested.notified()) + .await + .expect("the other window should request its blocking resource"); + assert!( + timeout(Duration::from_millis(200), &mut navigation) + .await + .is_err(), + "{strategy} navigation must wait for its own lifecycle while another window has an alert" + ); + release_resource.notify_one(); + let (status, navigated) = navigation.await.expect("navigation task"); + assert_eq!(status, StatusCode::OK, "{navigated:?}"); + assert_eq!(navigated, json!({ "value": null })); + let (status, lifecycle) = classic_sync_alert_request( + &app, + Method::POST, + &format!("/session/{session_id}/execute/sync"), + Some(json!({ + "script": "return [window.dclReached, window.blockedScriptRan, document.readyState];", + "args": [] + })), + ) + .await; + assert_eq!(status, StatusCode::OK, "{lifecycle:?}"); + assert_eq!(lifecycle["value"][0], json!(true)); + assert_eq!(lifecycle["value"][1], json!(true)); + if strategy == "normal" { + assert_eq!(lifecycle["value"][2], json!("complete")); + } + + let (status, switched) = classic_sync_alert_request( + &app, + Method::POST, + &window_path, + Some(json!({ "handle": original_handle })), + ) + .await; + assert_eq!(status, StatusCode::OK, "{switched:?}"); + classic_sync_alert_open_text(&app, &session_id).await; + let (status, dismissed) = classic_sync_alert_request( + &app, + Method::POST, + &format!("/session/{session_id}/alert/dismiss"), + Some(json!({})), + ) + .await; + assert_eq!(status, StatusCode::OK, "{dismissed:?}"); + assert_eq!(dismissed, json!({ "value": null })); + classic_sync_alert_delete(&app, &session_id).await; +} + +#[tokio::test] +async fn webdriver_classic_normal_navigation_waits_while_another_window_has_an_alert() { + classic_navigation_waits_while_another_window_has_an_alert("normal").await; +} + +#[tokio::test] +async fn webdriver_classic_eager_navigation_waits_while_another_window_has_an_alert() { + classic_navigation_waits_while_another_window_has_an_alert("eager").await; +} + #[tokio::test] async fn webdriver_classic_sync_alert_control_page_without_alert_responds_in_time() { let app = build_router(test_state()); 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 1c787f19be..afe37f99a5 100644 --- a/moli-protocol/src/conn/browser_context/target_session_owner.rs +++ b/moli-protocol/src/conn/browser_context/target_session_owner.rs @@ -1,5 +1,7 @@ use super::*; -use crate::automation::{DevToolsNetworkInterceptId, DevToolsNetworkResourceType}; +use crate::automation::{ + AutomationContext, DevToolsNetworkInterceptId, DevToolsNetworkResourceType, +}; use crate::conn::state::{ BrowserContextPageStorageHandles, BrowserContextResourceStorageHandles, DevToolsSessionState, PageNavigationHistoryEntry, PageTargetHost, RendererMainDocumentCommitSeed, TargetFetchConfig, @@ -1608,6 +1610,16 @@ impl CdpConnection { .page_session_state() } + /// Includes dialogs from the addressed Page's child frames and DevTools sessions. + pub fn has_pending_javascript_dialog_for_devtools_context( + &self, + context: &AutomationContext, + ) -> bool { + self.command_owner_scope_for_devtools_context(context) + .and_then(|owner| self.target_session_owner_ref_for_owner(&owner)) + .is_some_and(|owner| owner.target().has_pending_javascript_dialog()) + } + pub(crate) fn target_devtools_session_state_for_session( &self, session_id: Option<&str>, diff --git a/moli-protocol/src/domains/page/tests/dialog.rs b/moli-protocol/src/domains/page/tests/dialog.rs index 5b825d58ca..94e8fe6d25 100644 --- a/moli-protocol/src/domains/page/tests/dialog.rs +++ b/moli-protocol/src/domains/page/tests/dialog.rs @@ -116,6 +116,13 @@ fn retiring_page_scope_and_clearing_dialog_state_dismisses_installed_dialog() { #[tokio::test(flavor = "multi_thread")] async fn pending_javascript_dialogs_are_preserved_per_background_target() { + let context = |target_id: &str| AutomationContext { + protocol: FrontendProtocol::WebDriverClassic, + session_id: None, + target_id: Some(DevToolsTargetId::from(target_id)), + browser_context_id: None, + }; + fn dialog(target_id: &str, message: &str) -> crate::conn::TargetJavaScriptDialog { target_dialog_for_test( crate::conn::TargetPageResidenceIdentity::new_for_test( @@ -164,6 +171,22 @@ async fn pending_javascript_dialogs_are_preserved_per_background_target() { .javascript_dialog_state .is_empty() ); + } + assert!(ctx.conn.has_pending_javascript_dialog()); + assert!( + ctx.conn + .has_pending_javascript_dialog_for_devtools_context(&context("TID-A")) + ); + assert!( + !ctx.conn + .has_pending_javascript_dialog_for_devtools_context(&context("TID-B")) + ); + assert!( + !ctx.conn + .has_pending_javascript_dialog_for_devtools_context(&context("TID-missing")) + ); + { + let browser_context = ctx.conn.browser_context.as_mut().unwrap(); browser_context.active_page_target_mut().devtools_sessions [moli_page_types::DevToolsSessionKey::Primary] .page_session_state @@ -572,6 +595,17 @@ async fn javascript_dialog_pending_state_is_session_local_for_active_attached_se .is_empty(), "primary session must not see attached pending dialog" ); + let context = AutomationContext { + protocol: FrontendProtocol::Cdp, + session_id: Some(crate::automation::DevToolsSessionId::from("SID-primary")), + target_id: Some(DevToolsTargetId::from("TID-dialog-attached")), + browser_context_id: None, + }; + assert!( + ctx.conn + .has_pending_javascript_dialog_for_devtools_context(&context), + "a dialog owned by another DevTools attachment still blocks the same Page" + ); ctx.process_async(json!({ "id": 36, @@ -611,6 +645,10 @@ async fn javascript_dialog_pending_state_is_session_local_for_active_attached_se .is_empty(), "attached handle should consume only its own dialog" ); + assert!( + !ctx.conn + .has_pending_javascript_dialog_for_devtools_context(&context) + ); } #[tokio::test(flavor = "multi_thread")]