WebDriver: Wait focus to complete when switching window (#38160)

Previously the webdriver do not wait for focus to complete, which can
cause some instability.

No matter interact as human or webdriver, the focus chain always goes
as: Embedder forwards -> Constellation (do some updates) -> Embedder (do
some updates).

---------

Signed-off-by: Euclid Ye <euclid.ye@huawei.com>

components/constellation/constellation.rs

@@ -1366,8 +1366,8 @@ where
                 }
                 self.handle_panic(webview_id, error, None);
             },
-            EmbedderToConstellationMessage::FocusWebView(webview_id) => {
-                self.handle_focus_web_view(webview_id);
+            EmbedderToConstellationMessage::FocusWebView(webview_id, response_sender) => {
+                self.handle_focus_web_view(webview_id, response_sender);
             },
             EmbedderToConstellationMessage::BlurWebView => {
                 self.webviews.unfocus();
@@ -2774,13 +2774,20 @@ where
     }
 
     #[servo_tracing::instrument(skip_all)]
-    fn handle_focus_web_view(&mut self, webview_id: WebViewId) {
+    fn handle_focus_web_view(
+        &mut self,
+        webview_id: WebViewId,
+        response_sender: Option<IpcSender<bool>>,
+    ) {
         if self.webviews.get(webview_id).is_none() {
+            if let Some(response_sender) = response_sender {
+                let _ = response_sender.send(false);
+            }
             return warn!("{webview_id}: FocusWebView on unknown top-level browsing context");
         }
         self.webviews.focus(webview_id);
         self.embedder_proxy
-            .send(EmbedderMsg::WebViewFocused(webview_id));
+            .send(EmbedderMsg::WebViewFocused(webview_id, response_sender));
     }
 
     #[servo_tracing::instrument(skip_all)]
@@ -4152,7 +4159,7 @@ where
         // Focus the top-level browsing context.
         self.webviews.focus(webview_id);
         self.embedder_proxy
-            .send(EmbedderMsg::WebViewFocused(webview_id));
+            .send(EmbedderMsg::WebViewFocused(webview_id, None));
 
         // If a container with a non-null nested browsing context is focused,
         // the nested browsing context's active document becomes the focused

components/servo/lib.rs

@@ -732,13 +732,16 @@ impl Servo {
                     webview.delegate().notify_closed(webview);
                 }
             },
-            EmbedderMsg::WebViewFocused(webview_id) => {
+            EmbedderMsg::WebViewFocused(webview_id, response_sender) => {
                 for id in self.webviews.borrow().keys() {
                     if let Some(webview) = self.get_webview_handle(*id) {
                         let focused = webview.id() == webview_id;
                         webview.set_focused(focused);
                     }
                 }
+                if let Some(response_sender) = response_sender {
+                    let _ = response_sender.send(true);
+                }
             },
             EmbedderMsg::WebViewBlurred => {
                 for id in self.webviews.borrow().keys() {

components/servo/webview.rs

@@ -25,7 +25,7 @@ use webrender_api::units::{DeviceIntPoint, DevicePixel, DeviceRect};
 use crate::clipboard_delegate::{ClipboardDelegate, DefaultClipboardDelegate};
 use crate::javascript_evaluator::JavaScriptEvaluator;
 use crate::webview_delegate::{DefaultWebViewDelegate, WebViewDelegate};
-use crate::{ConstellationProxy, Servo, WebRenderDebugOption};
+use crate::{ConstellationProxy, IpcSender, Servo, WebRenderDebugOption};
 
 /// A handle to a Servo webview. If you clone this handle, it does not create a new webview,
 /// but instead creates a new handle to the webview. Once the last handle is dropped, Servo
@@ -308,7 +308,19 @@ impl WebView {
     pub fn focus(&self) {
         self.inner()
             .constellation_proxy
-            .send(EmbedderToConstellationMessage::FocusWebView(self.id()));
+            .send(EmbedderToConstellationMessage::FocusWebView(
+                self.id(),
+                None,
+            ));
+    }
+
+    pub fn focus_from_webdriver(&self, response_sender: IpcSender<bool>) {
+        self.inner()
+            .constellation_proxy
+            .send(EmbedderToConstellationMessage::FocusWebView(
+                self.id(),
+                Some(response_sender),
+            ));
     }
 
     pub fn blur(&self) {

components/shared/constellation/lib.rs

@@ -68,8 +68,9 @@ pub enum EmbedderToConstellationMessage {
     CloseWebView(WebViewId),
     /// Panic a top level browsing context.
     SendError(Option<WebViewId>, String),
-    /// Make a webview focused.
-    FocusWebView(WebViewId),
+    /// Make a webview focused. If sender is provided, it will be used to send back a
+    /// bool indicating whether the focus was successfully set in EmbedderMsg::WebViewFocused.
+    FocusWebView(WebViewId, Option<IpcSender<bool>>),
     /// Make none of the webviews focused.
     BlurWebView,
     /// Forward an input event to an appropriate ScriptTask.

components/shared/embedder/lib.rs

@@ -373,7 +373,9 @@ pub enum EmbedderMsg {
     /// A webview was destroyed.
     WebViewClosed(WebViewId),
     /// A webview gained focus for keyboard events
-    WebViewFocused(WebViewId),
+    /// If sender is provided, it will be used to send back a
+    /// bool indicating whether the focus was successfully set.
+    WebViewFocused(WebViewId, Option<IpcSender<bool>>),
     /// All webviews lost focus for keyboard events.
     WebViewBlurred,
     /// Wether or not to unload a document

components/shared/embedder/webdriver.rs

@@ -152,7 +152,8 @@ pub enum WebDriverCommandMsg {
     /// Close the webview associated with the provided id.
     CloseWebView(WebViewId),
     /// Focus the webview associated with the provided id.
-    FocusWebView(WebViewId),
+    /// Sends back a bool indicating whether the focus was successfully set.
+    FocusWebView(WebViewId, IpcSender<bool>),
     /// Get focused webview.
     GetFocusedWebView(IpcSender<Option<WebViewId>>),
     /// Check whether top-level browsing context is open.

components/webdriver_server/lib.rs

@@ -1266,9 +1266,14 @@ impl Handler {
             let webview_id = *webview_id;
             session.webview_id = webview_id;
             session.browsing_context_id = BrowsingContextId::from(webview_id);
-
-            let msg = WebDriverCommandMsg::FocusWebView(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");
+            }
             Ok(WebDriverResponse::Void)
         } else {
             Err(WebDriverError::new(

ports/servoshell/desktop/app.rs

@@ -377,13 +377,10 @@ impl App {
                 WebDriverCommandMsg::CloseWebView(webview_id) => {
                     running_state.close_webview(webview_id);
                 },
-                WebDriverCommandMsg::FocusWebView(webview_id) => {
+                WebDriverCommandMsg::FocusWebView(webview_id, response_sender) => {
                     if let Some(webview) = running_state.webview_by_id(webview_id) {
-                        webview.focus();
+                        webview.focus_from_webdriver(response_sender);
                     }
-
-                    // TODO: send a response to the WebDriver
-                    // so it knows when the focus has finished.
                 },
                 WebDriverCommandMsg::GetWindowRect(_webview_id, response_sender) => {
                     let window = self

tests/wpt/meta/webdriver/tests/classic/dismiss_alert/dismiss.py.ini

@@ -3,7 +3,7 @@
     expected: ERROR
 
   [test_dismiss_confirm]
-    expected: ERROR
+    expected: FAIL
 
   [test_dismiss_prompt]
     expected: FAIL

tests/wpt/meta/webdriver/tests/classic/get_alert_text/get.py.ini

@@ -1,7 +1,4 @@
 [get.py]
-  [test_get_alert_text]
-    expected: FAIL
-
   [test_get_confirm_text]
     expected: FAIL