From 802fdd9068ab7b3be732b3ad4d120786b441baa6 Mon Sep 17 00:00:00 2001 From: Euclid Ye Date: Tue, 2 Sep 2025 16:57:45 +0800 Subject: [PATCH] webdriver: Focus browsing context when switch frame (#39086) #38889 adds back the mechanism to focus the window when [switch to window](https://w3c.github.io/webdriver/#switch-to-window). After that, it causes many flaky TIMEOUT. Turns out the real reason is same as the phenomenon which I thought was unrelated: https://github.com/servo/servo/pull/38889#issuecomment-3217512339: we need to focus the browsing context as well according to spec. This is important for Servo because it relies on https://github.com/servo/servo/blob/f4dd2960b89bcc6bae3f9dae5be8964c64c1c2c7/components/constellation/constellation_webview.rs#L64 to determine which pipeline to send `InputEvent` to. Testing: Before, there are 9 ~ 13 flaky results. [Before 1](https://github.com/yezhizhen/servo/actions/runs/17379170889), [Before 2](https://github.com/yezhizhen/servo/actions/runs/17392219417), [Before 3](https://github.com/yezhizhen/servo/actions/runs/17379172612). Now there are 2 ~ 4 flaky results. [After 1](https://github.com/yezhizhen/servo/actions/runs/17394359570), [After 2](https://github.com/yezhizhen/servo/actions/runs/17394358218), [After 3](https://github.com/yezhizhen/servo/actions/runs/17394357400). Fixes: https://github.com/servo/servo/pull/38889#issuecomment-3218600566 Fixes: #38906, which is last blocking point to enable WebDriver CI. --------- Signed-off-by: Euclid Ye --- components/constellation/constellation.rs | 3 +++ components/shared/embedder/webdriver.rs | 1 + components/webdriver_server/lib.rs | 22 +++++++++++++++++++++- ports/servoshell/desktop/app.rs | 3 +++ 4 files changed, 28 insertions(+), 1 deletion(-) diff --git a/components/constellation/constellation.rs b/components/constellation/constellation.rs index 5dfb421ce21..15afd57a217 100644 --- a/components/constellation/constellation.rs +++ b/components/constellation/constellation.rs @@ -4524,6 +4524,9 @@ where let is_open = self.browsing_contexts.contains_key(&browsing_context_id); let _ = response_sender.send(is_open); }, + WebDriverCommandMsg::FocusBrowsingContext(browsing_context_id) => { + self.handle_focus_remote_document_msg(browsing_context_id); + }, // TODO: This should use the ScriptThreadMessage::EvaluateJavaScript command WebDriverCommandMsg::ScriptCommand(browsing_context_id, cmd) => { let pipeline_id = if let Some(browsing_context) = diff --git a/components/shared/embedder/webdriver.rs b/components/shared/embedder/webdriver.rs index da0f73f2e7b..9bb08a4e894 100644 --- a/components/shared/embedder/webdriver.rs +++ b/components/shared/embedder/webdriver.rs @@ -173,6 +173,7 @@ pub enum WebDriverCommandMsg { ), GetAlertText(WebViewId, IpcSender>), SendAlertText(WebViewId, String), + FocusBrowsingContext(BrowsingContextId), } #[derive(Debug, Deserialize, Serialize)] diff --git a/components/webdriver_server/lib.rs b/components/webdriver_server/lib.rs index d81c35d03cc..1f904dc2c8d 100644 --- a/components/webdriver_server/lib.rs +++ b/components/webdriver_server/lib.rs @@ -1214,8 +1214,14 @@ impl Handler { self.handle_any_user_prompts(webview_id)?; // Step 3. Set the current browsing context with session and // session's current top-level browsing context. + let browsing_context_id = BrowsingContextId::from(webview_id); self.session_mut()? - .set_browsing_context_id(BrowsingContextId::from(webview_id)); + .set_browsing_context_id(browsing_context_id); + + // Step 4. Update any implementation-specific state that would result from + // the user selecting session's current browsing context for interaction, + // without altering OS-level focus. + self.focus_browsing_context(browsing_context_id)?; return Ok(WebDriverResponse::Void); }, // id is a Number object @@ -1260,6 +1266,10 @@ impl Handler { Ok(browsing_context_id) => { self.session_mut()? .set_browsing_context_id(browsing_context_id); + // Step 5. Update any implementation-specific state that would result from + // the user selecting session's current browsing context for interaction, + // without altering OS-level focus. + self.focus_browsing_context(browsing_context_id)?; Ok(WebDriverResponse::Void) }, Err(error) => Err(WebDriverError::new(error, "")), @@ -1307,6 +1317,10 @@ impl Handler { Ok(browsing_context_id) => { self.session_mut()? .set_browsing_context_id(browsing_context_id); + // Step 4. Update any implementation-specific state that would result from + // the user selecting session's current browsing context for interaction, + // without altering OS-level focus. + self.focus_browsing_context(browsing_context_id)?; Ok(WebDriverResponse::Void) }, Err(error) => Err(WebDriverError::new(error, "")), @@ -2506,6 +2520,12 @@ impl Handler { } Ok(()) } + + fn focus_browsing_context(&self, browsing_cotext_id: BrowsingContextId) -> WebDriverResult<()> { + self.send_message_to_embedder(WebDriverCommandMsg::FocusBrowsingContext( + browsing_cotext_id, + )) + } } impl WebDriverHandler for Handler { diff --git a/ports/servoshell/desktop/app.rs b/ports/servoshell/desktop/app.rs index 4b0a14a4d99..06621f69143 100644 --- a/ports/servoshell/desktop/app.rs +++ b/ports/servoshell/desktop/app.rs @@ -380,6 +380,9 @@ impl App { running_state.set_pending_focus(focus_id, response_sender); } }, + WebDriverCommandMsg::FocusBrowsingContext(..) => { + running_state.servo().execute_webdriver_command(msg); + }, WebDriverCommandMsg::GetAllWebViews(response_sender) => { let webviews = running_state.webviews().iter().map(|(id, _)| *id).collect();