From 8b3e7b1c6a649c5bbe4e71a0f73dd6fd59e48ef4 Mon Sep 17 00:00:00 2001 From: Euclid Ye Date: Wed, 30 Jul 2025 14:41:58 +0800 Subject: [PATCH] webdriver: Keep constellation alive and Open new top-level browsing context with new session request when none is open (#37410) Keep Constellation alive even when all browsing context closed in WebDriver mode. In this case, when creating a new session, we would open a new top-level browsing context. Fixes: #37408 Testing: `./mach test-wpt -r .\tests\wpt\tests\webdriver\tests\classic\close_window\close.py --product servodriver` --------- Signed-off-by: Euclid Ye Signed-off-by: Euclid Ye --- components/shared/embedder/webdriver.rs | 9 ++-- components/webdriver_server/lib.rs | 49 ++++++++++++------- ports/servoshell/desktop/app.rs | 7 ++- ports/servoshell/desktop/app_state.rs | 8 ++- .../tests/classic/accept_alert/accept.py.ini | 2 +- .../tests/classic/close_window/close.py.ini | 3 -- .../classic/dismiss_alert/dismiss.py.ini | 6 --- .../element_send_keys/file_upload.py.ini | 7 +-- 8 files changed, 50 insertions(+), 41 deletions(-) diff --git a/components/shared/embedder/webdriver.rs b/components/shared/embedder/webdriver.rs index bd952b03a7d..e329f55bd93 100644 --- a/components/shared/embedder/webdriver.rs +++ b/components/shared/embedder/webdriver.rs @@ -146,17 +146,16 @@ pub enum WebDriverCommandMsg { Option>, IpcSender>, ), - /// Create a new webview that loads about:blank. The constellation will use + /// Create a new webview that loads about:blank. The embedder will use /// the provided channels to return the top level browsing context id - /// associated with the new webview, and a notification when the initial - /// load is complete. - NewWebView(IpcSender, IpcSender), + /// associated with the new webview, and sets a "load status sender" if provided. + NewWebView(IpcSender, Option>), /// Close the webview associated with the provided id. CloseWebView(WebViewId), /// Focus the webview associated with the provided id. /// Sends back a bool indicating whether the focus was successfully set. FocusWebView(WebViewId, IpcSender), - /// Get focused webview. + /// Get focused webview. For now, this is only used when start new session. GetFocusedWebView(IpcSender>), /// Check whether top-level browsing context is open. IsWebViewOpen(WebViewId, IpcSender), diff --git a/components/webdriver_server/lib.rs b/components/webdriver_server/lib.rs index 30f3cdb55b0..a3b403483ba 100644 --- a/components/webdriver_server/lib.rs +++ b/components/webdriver_server/lib.rs @@ -504,17 +504,11 @@ impl Handler { self.session().unwrap().webview_id } - fn focus_webview_id(&self) -> WebDriverResult { + fn focused_webview_id(&self) -> WebDriverResult> { let (sender, receiver) = ipc::channel().unwrap(); self.send_message_to_embedder(WebDriverCommandMsg::GetFocusedWebView(sender.clone()))?; // Wait until the document is ready before returning the top-level browsing context id. - match wait_for_script_response(receiver)? { - Some(webview_id) => Ok(webview_id), - None => Err(WebDriverError::new( - ErrorStatus::NoSuchWindow, - "No focused webview found", - )), - } + wait_for_script_response(receiver) } fn session(&self) -> WebDriverResult<&WebDriverSession> { @@ -578,8 +572,22 @@ impl Handler { // Step 6. Create a session // Step 8. Set session' current top-level browsing context - let webview_id = self.focus_webview_id()?; + let webview_id = match self.focused_webview_id()? { + Some(webview_id) => webview_id, + None => { + // This happens when there is no open webview. + // We need to create a new one. See https://github.com/servo/servo/issues/37408 + let (sender, receiver) = ipc::channel().unwrap(); + self.send_message_to_embedder(WebDriverCommandMsg::NewWebView(sender, None))?; + let webview_id = receiver + .recv() + .expect("IPC failure when creating new webview for new session"); + self.focus_webview(webview_id)?; + webview_id + }, + }; let browsing_context_id = BrowsingContextId::from(webview_id); + // Create and append session to the handler let session_id = self.create_session( &mut capabilities, @@ -1085,7 +1093,8 @@ impl Handler { // Step 3. Handle any user prompt. self.handle_any_user_prompts(session.webview_id)?; - let cmd_msg = WebDriverCommandMsg::NewWebView(sender, self.load_status_sender.clone()); + let cmd_msg = + WebDriverCommandMsg::NewWebView(sender, Some(self.load_status_sender.clone())); // Step 5. Create a new top-level browsing context by running the window open steps. // This MUST be done without invoking the focusing steps. self.send_message_to_embedder(cmd_msg)?; @@ -1189,14 +1198,7 @@ impl Handler { let webview_id = *webview_id; session.webview_id = webview_id; session.browsing_context_id = BrowsingContextId::from(webview_id); - let (sender, receiver) = ipc::channel().unwrap(); - let msg = WebDriverCommandMsg::FocusWebView(webview_id, sender); - self.send_message_to_embedder(msg)?; - if wait_for_script_response(receiver)? { - debug!("Focus new webview successfully"); - } else { - debug!("Focus new webview failed, it may not exist anymore"); - } + self.focus_webview(webview_id)?; Ok(WebDriverResponse::Void) } else { Err(WebDriverError::new( @@ -2387,6 +2389,17 @@ impl Handler { Ok(()) } } + + fn focus_webview(&self, webview_id: WebViewId) -> Result<(), WebDriverError> { + let (sender, receiver) = ipc::channel().unwrap(); + self.send_message_to_embedder(WebDriverCommandMsg::FocusWebView(webview_id, sender))?; + if wait_for_script_response(receiver)? { + debug!("Focus new webview successfully"); + } else { + debug!("Focus new webview failed, it may not exist anymore"); + } + Ok(()) + } } impl WebDriverHandler for Handler { diff --git a/ports/servoshell/desktop/app.rs b/ports/servoshell/desktop/app.rs index 1c3df324194..c20e5b827f9 100644 --- a/ports/servoshell/desktop/app.rs +++ b/ports/servoshell/desktop/app.rs @@ -369,8 +369,9 @@ impl App { if let Err(error) = response_sender.send(new_webview.id()) { warn!("Failed to send response of NewWebview: {error}"); } - - running_state.set_load_status_sender(new_webview.id(), load_status_sender); + if let Some(load_status_sender) = load_status_sender { + running_state.set_load_status_sender(new_webview.id(), load_status_sender); + } }, WebDriverCommandMsg::CloseWebView(webview_id) => { running_state.close_webview(webview_id); @@ -446,8 +447,10 @@ impl App { warn!("Failed to send response of GetViewportSize: {error}"); } }, + // This is only received when start new session. WebDriverCommandMsg::GetFocusedWebView(sender) => { let focused_webview = running_state.focused_webview(); + if let Err(error) = sender.send(focused_webview.map(|w| w.id())) { warn!("Failed to send response of GetFocusedWebView: {error}"); }; diff --git a/ports/servoshell/desktop/app_state.rs b/ports/servoshell/desktop/app_state.rs index 2f2a94aa63b..038129c987f 100644 --- a/ports/servoshell/desktop/app_state.rs +++ b/ports/servoshell/desktop/app_state.rs @@ -272,7 +272,13 @@ impl RunningAppState { Some(last_created_webview) => { last_created_webview.focus(); }, - None => self.servo.start_shutting_down(), + None if self.servoshell_preferences.webdriver_port.is_none() => { + self.servo.start_shutting_down() + }, + None => { + // For WebDriver, don't shut down when last webview closed + // https://github.com/servo/servo/issues/37408 + }, } } diff --git a/tests/wpt/meta/webdriver/tests/classic/accept_alert/accept.py.ini b/tests/wpt/meta/webdriver/tests/classic/accept_alert/accept.py.ini index 1bd6fa792a2..04e5861754c 100644 --- a/tests/wpt/meta/webdriver/tests/classic/accept_alert/accept.py.ini +++ b/tests/wpt/meta/webdriver/tests/classic/accept_alert/accept.py.ini @@ -12,7 +12,7 @@ expected: FAIL [test_accept_prompt] - expected: ERROR + expected: FAIL [test_accept_in_popup_window] expected: FAIL diff --git a/tests/wpt/meta/webdriver/tests/classic/close_window/close.py.ini b/tests/wpt/meta/webdriver/tests/classic/close_window/close.py.ini index bca64443c8d..c29d93dc166 100644 --- a/tests/wpt/meta/webdriver/tests/classic/close_window/close.py.ini +++ b/tests/wpt/meta/webdriver/tests/classic/close_window/close.py.ini @@ -1,6 +1,3 @@ [close.py] [test_close_last_browsing_context] expected: FAIL - - [test_element_usage_after_closing_browsing_context] - expected: ERROR diff --git a/tests/wpt/meta/webdriver/tests/classic/dismiss_alert/dismiss.py.ini b/tests/wpt/meta/webdriver/tests/classic/dismiss_alert/dismiss.py.ini index 324fbc60cff..690ac65db28 100644 --- a/tests/wpt/meta/webdriver/tests/classic/dismiss_alert/dismiss.py.ini +++ b/tests/wpt/meta/webdriver/tests/classic/dismiss_alert/dismiss.py.ini @@ -2,12 +2,6 @@ [test_no_top_browsing_context] expected: ERROR - [test_no_browsing_context] - expected: ERROR - - [test_dismiss_alert] - expected: FAIL - [test_dismiss_confirm] expected: FAIL diff --git a/tests/wpt/meta/webdriver/tests/classic/element_send_keys/file_upload.py.ini b/tests/wpt/meta/webdriver/tests/classic/element_send_keys/file_upload.py.ini index e1f1291f1a1..26ee5011925 100644 --- a/tests/wpt/meta/webdriver/tests/classic/element_send_keys/file_upload.py.ini +++ b/tests/wpt/meta/webdriver/tests/classic/element_send_keys/file_upload.py.ini @@ -8,11 +8,8 @@ [test_single_file_appends_with_multiple_attribute] expected: FAIL - [test_focused] - expected: ERROR - [test_strict_hidden] - expected: ERROR + expected: FAIL [test_strict_display_none] - expected: ERROR + expected: FAIL