webdriver: Refactor webdriver session and improve window handles (#38745)

This PR contains 2 parts:

1. Refactor webdriver session.
2. Improve webdriver window handles:
- Webdriver always get window handles from script thread by default.
- If script thread is blocked by user prompt, embedder stores the window
handle before user prompt appears, then webdriver can get window handle
from embedder.

Testing: Clear timeout cause by user prompt blocking script thread:
https://github.com/longvatrong111/servo/actions/runs/17033900026

---------

Signed-off-by: batu_hoang <hoang.binh.trong@huawei.com>
Signed-off-by: batu_hoang <longvatrong111@gmail.com>
Co-authored-by: Euclid Ye <euclid.ye@huawei.com>

components/script/script_thread.rs

@@ -2436,9 +2436,6 @@ impl ScriptThread {
             WebDriverScriptCommand::RemoveLoadStatusSender(_) => {
                 webdriver_handlers::handle_remove_load_status_sender(&documents, pipeline_id)
             },
-            WebDriverScriptCommand::GetWindowHandle(reply) => {
-                webdriver_handlers::handle_get_window_handle(pipeline_id, reply)
-            },
             _ => (),
         }
     }

components/script/webdriver_handlers.rs

@@ -460,9 +460,7 @@ unsafe fn jsval_to_webdriver_inner(
             } else {
                 let pipeline = window.pipeline_id();
                 if window_proxy.browsing_context_id() == window_proxy.webview_id() {
-                    Ok(JSValue::Window(
+                    Ok(JSValue::Window(window.webview_id().to_string()))
-                        window.Document().upcast::<Node>().unique_id(pipeline),
-                    ))
                 } else {
                     Ok(JSValue::Frame(
                         window.Document().upcast::<Node>().unique_id(pipeline),
@@ -2076,16 +2074,3 @@ pub(crate) fn handle_remove_load_status_sender(
         window.set_webdriver_load_status_sender(None);
     }
 }
-
-pub(crate) fn handle_get_window_handle(
-    pipeline_id: PipelineId,
-    reply: IpcSender<Result<String, ErrorStatus>>,
-) {
-    if let Some(res) = ScriptThread::find_document(pipeline_id)
-        .map(|document| document.upcast::<Node>().unique_id(pipeline_id))
-    {
-        reply.send(Ok(res)).ok();
-    } else {
-        reply.send(Err(ErrorStatus::NoSuchWindow)).ok();
-    }
-}

components/shared/embedder/webdriver.rs

@@ -156,8 +156,8 @@ pub enum WebDriverCommandMsg {
     FocusWebView(WebViewId, IpcSender<bool>),
     /// Get focused webview. For now, this is only used when start new session.
     GetFocusedWebView(IpcSender<Option<WebViewId>>),
-    /// Get all webviews
+    /// Get webviews state
-    GetAllWebViews(IpcSender<Result<Vec<WebViewId>, ErrorStatus>>),
+    GetAllWebViews(IpcSender<Vec<WebViewId>>),
     /// Check whether top-level browsing context is open.
     IsWebViewOpen(WebViewId, IpcSender<bool>),
     /// Check whether browsing context is open.
@@ -246,7 +246,6 @@ pub enum WebDriverScriptCommand {
     WillSendKeys(String, String, bool, IpcSender<Result<bool, ErrorStatus>>),
     AddLoadStatusSender(WebViewId, IpcSender<WebDriverLoadStatus>),
     RemoveLoadStatusSender(WebViewId),
-    GetWindowHandle(IpcSender<Result<String, ErrorStatus>>),
 }
 
 #[derive(Debug, Deserialize, Serialize)]

components/webdriver_server/actions.rs

@@ -216,6 +216,8 @@ impl Handler {
         tick_actions: &TickActions,
         tick_duration: u64,
     ) -> Result<(), ErrorStatus> {
+        let session = self.session().unwrap();
+
         // Step 1. For each action object in tick actions:
         // Step 1.1. Let input_id be the value of the id property of action object.
         for (input_id, action) in tick_actions.iter() {
@@ -233,16 +235,12 @@ impl Handler {
                     // Step 9. If subtype is "keyDown", append a copy of action
                     // object with the subtype property changed to "keyUp" to
                     // input state's input cancel list.
-                    self.session()
+                    session.input_cancel_list_mut().push((
-                        .unwrap()
+                        input_id.clone(),
-                        .input_cancel_list
+                        ActionItem::Key(KeyActionItem::Key(KeyAction::Up(KeyUpAction {
-                        .borrow_mut()
+                            value: keydown_action.value.clone(),
-                        .push((
+                        }))),
-                            input_id.clone(),
+                    ));
-                            ActionItem::Key(KeyActionItem::Key(KeyAction::Up(KeyUpAction {
-                                value: keydown_action.value.clone(),
-                            }))),
-                        ));
                 },
                 ActionItem::Key(KeyActionItem::Key(KeyAction::Up(keyup_action))) => {
                     self.dispatch_keyup_action(input_id, keyup_action);
@@ -257,19 +255,15 @@ impl Handler {
                     // Step 10. If subtype is "pointerDown", append a copy of action
                     // object with the subtype property changed to "pointerUp" to
                     // input state's input cancel list.
-                    self.session()
+                    session.input_cancel_list_mut().push((
-                        .unwrap()
+                        input_id.clone(),
-                        .input_cancel_list
+                        ActionItem::Pointer(PointerActionItem::Pointer(PointerAction::Up(
-                        .borrow_mut()
+                            PointerUpAction {
-                        .push((
+                                button: pointer_down_action.button,
-                            input_id.clone(),
+                                ..Default::default()
-                            ActionItem::Pointer(PointerActionItem::Pointer(PointerAction::Up(
+                            },
-                                PointerUpAction {
+                        ))),
-                                    button: pointer_down_action.button,
+                    ));
-                                    ..Default::default()
-                                },
-                            ))),
-                        ));
                 },
                 ActionItem::Pointer(PointerActionItem::Pointer(PointerAction::Move(
                     pointer_move_action,
@@ -298,8 +292,7 @@ impl Handler {
     fn dispatch_general_action(&self, source_id: &str) {
         self.session()
             .unwrap()
-            .input_state_table
+            .input_state_table_mut()
-            .borrow_mut()
             .entry(source_id.to_string())
             .or_insert(InputSourceState::Null);
     }
@@ -307,9 +300,9 @@ impl Handler {
     /// <https://w3c.github.io/webdriver/#dfn-dispatch-a-keydown-action>
     fn dispatch_keydown_action(&self, source_id: &str, action: &KeyDownAction) {
         let session = self.session().unwrap();
+        let mut input_state_table = session.input_state_table_mut();
 
         let raw_key = action.value.chars().next().unwrap();
-        let mut input_state_table = session.input_state_table.borrow_mut();
         let key_input_state = match input_state_table.get_mut(source_id).unwrap() {
             InputSourceState::Key(key_input_state) => key_input_state,
             _ => unreachable!(),
@@ -321,7 +314,7 @@ impl Handler {
         self.increment_num_pending_actions();
         let msg_id = self.current_action_id.get();
         let cmd_msg =
-            WebDriverCommandMsg::KeyboardAction(session.webview_id, keyboard_event, msg_id);
+            WebDriverCommandMsg::KeyboardAction(self.verified_webview_id(), keyboard_event, msg_id);
         let _ = self.send_message_to_embedder(cmd_msg);
     }
 
@@ -333,7 +326,7 @@ impl Handler {
         // See https://github.com/w3c/webdriver/issues/1905 &&
         // https://github.com/servo/servo/issues/37579#issuecomment-2990762713
         {
-            let mut input_cancel_list = session.input_cancel_list.borrow_mut();
+            let mut input_cancel_list = session.input_cancel_list_mut();
             if let Some(pos) = input_cancel_list.iter().rposition(|(id, item)| {
                 id == source_id &&
                     matches!(item,
@@ -346,7 +339,7 @@ impl Handler {
         }
 
         let raw_key = action.value.chars().next().unwrap();
-        let mut input_state_table = session.input_state_table.borrow_mut();
+        let mut input_state_table = session.input_state_table_mut();
         let key_input_state = match input_state_table.get_mut(source_id).unwrap() {
             InputSourceState::Key(key_input_state) => key_input_state,
             _ => unreachable!(),
@@ -356,8 +349,11 @@ impl Handler {
             // Step 12
             self.increment_num_pending_actions();
             let msg_id = self.current_action_id.get();
-            let cmd_msg =
+            let cmd_msg = WebDriverCommandMsg::KeyboardAction(
-                WebDriverCommandMsg::KeyboardAction(session.webview_id, keyboard_event, msg_id);
+                self.verified_webview_id(),
+                keyboard_event,
+                msg_id,
+            );
             let _ = self.send_message_to_embedder(cmd_msg);
         }
     }
@@ -366,7 +362,7 @@ impl Handler {
     pub(crate) fn dispatch_pointerdown_action(&self, source_id: &str, action: &PointerDownAction) {
         let session = self.session().unwrap();
 
-        let mut input_state_table = session.input_state_table.borrow_mut();
+        let mut input_state_table = session.input_state_table_mut();
         let pointer_input_state = match input_state_table.get_mut(source_id).unwrap() {
             InputSourceState::Pointer(pointer_input_state) => pointer_input_state,
             _ => unreachable!(),
@@ -380,7 +376,7 @@ impl Handler {
         self.increment_num_pending_actions();
         let msg_id = self.current_action_id.get();
         let cmd_msg = WebDriverCommandMsg::MouseButtonAction(
-            session.webview_id,
+            self.verified_webview_id(),
             MouseButtonAction::Down,
             action.button.into(),
             pointer_input_state.x as f32,
@@ -394,7 +390,7 @@ impl Handler {
     pub(crate) fn dispatch_pointerup_action(&self, source_id: &str, action: &PointerUpAction) {
         let session = self.session().unwrap();
 
-        let mut input_state_table = session.input_state_table.borrow_mut();
+        let mut input_state_table = session.input_state_table_mut();
         let pointer_input_state = match input_state_table.get_mut(source_id).unwrap() {
             InputSourceState::Pointer(pointer_input_state) => pointer_input_state,
             _ => unreachable!(),
@@ -409,7 +405,7 @@ impl Handler {
         // See https://github.com/w3c/webdriver/issues/1905 &&
         // https://github.com/servo/servo/issues/37579#issuecomment-2990762713
         {
-            let mut input_cancel_list = session.input_cancel_list.borrow_mut();
+            let mut input_cancel_list = session.input_cancel_list_mut();
             if let Some(pos) = input_cancel_list.iter().position(|(id, item)| {
                 id == source_id &&
                     matches!(item, ActionItem::Pointer(PointerActionItem::Pointer(PointerAction::Up(
@@ -426,7 +422,7 @@ impl Handler {
         self.increment_num_pending_actions();
         let msg_id = self.current_action_id.get();
         let cmd_msg = WebDriverCommandMsg::MouseButtonAction(
-            session.webview_id,
+            self.verified_webview_id(),
             MouseButtonAction::Up,
             action.button.into(),
             pointer_input_state.x as f32,
@@ -452,8 +448,7 @@ impl Handler {
                 let (start_x, start_y) = match self
                     .session()
                     .unwrap()
-                    .input_state_table
+                    .input_state_table()
-                    .borrow()
                     .get(source_id)
                     .unwrap()
                 {
@@ -517,8 +512,7 @@ impl Handler {
         let (start_x, start_y) = match self
             .session()
             .unwrap()
-            .input_state_table
+            .input_state_table()
-            .borrow()
             .get(source_id)
             .unwrap()
         {
@@ -548,7 +542,7 @@ impl Handler {
         tick_start: Instant,
     ) {
         let session = self.session().unwrap();
-        let mut input_state_table = session.input_state_table.borrow_mut();
+        let mut input_state_table = session.input_state_table_mut();
         let pointer_input_state = match input_state_table.get_mut(source_id).unwrap() {
             InputSourceState::Pointer(pointer_input_state) => pointer_input_state,
             _ => unreachable!(),
@@ -594,7 +588,7 @@ impl Handler {
                     None
                 };
                 let cmd_msg = WebDriverCommandMsg::MouseMoveAction(
-                    session.webview_id,
+                    self.verified_webview_id(),
                     x as f32,
                     y as f32,
                     msg_id,
@@ -710,8 +704,6 @@ impl Handler {
         mut curr_delta_y: f64,
         tick_start: Instant,
     ) {
-        let session = self.session().unwrap();
-
         // Step 1. Let time delta be the time since the beginning of the current tick,
         // measured in milliseconds on a monotonic clock.
         let time_delta = tick_start.elapsed().as_millis();
@@ -751,7 +743,7 @@ impl Handler {
                 None
             };
             let cmd_msg = WebDriverCommandMsg::WheelScrollAction(
-                session.webview_id,
+                self.verified_webview_id(),
                 x,
                 y,
                 delta_x,
@@ -795,8 +787,7 @@ impl Handler {
             return Err(ErrorStatus::MoveTargetOutOfBounds);
         }
         let (sender, receiver) = ipc::channel().unwrap();
-        let cmd_msg =
+        let cmd_msg = WebDriverCommandMsg::GetViewportSize(self.verified_webview_id(), sender);
-            WebDriverCommandMsg::GetViewportSize(self.session.as_ref().unwrap().webview_id, sender);
         self.send_message_to_embedder(cmd_msg)
             .map_err(|_| ErrorStatus::UnknownError)?;
 
@@ -878,7 +869,7 @@ impl Handler {
         // Step 2. Let id be the value of the id property of action sequence.
         let id = action_sequence.id.clone();
 
-        let mut input_state_table = self.session().unwrap().input_state_table.borrow_mut();
+        let mut input_state_table = self.session().unwrap().input_state_table_mut();
 
         match action_sequence.actions {
             ActionsType::Null {

components/webdriver_server/session.rs

@@ -2,19 +2,25 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
+use std::cell::{Ref, RefCell, RefMut};
+use std::collections::HashMap;
+
 use base::id::{BrowsingContextId, WebViewId};
 use serde_json::{Map, Value, json};
 use uuid::Uuid;
 use webdriver::error::WebDriverResult;
 
+use crate::Handler;
+use crate::actions::{ActionItem, InputSourceState};
 use crate::capabilities::ServoCapabilities;
-use crate::timeout::{deserialize_as_timeouts_configuration, serialize_timeouts_configuration};
+use crate::timeout::{
-use crate::user_prompt::{
+    TimeoutsConfiguration, deserialize_as_timeouts_configuration, serialize_timeouts_configuration,
-    default_unhandled_prompt_behavior, deserialize_unhandled_prompt_behaviour,
+};
+use crate::user_prompt::{
+    UserPromptHandler, default_unhandled_prompt_behavior, deserialize_unhandled_prompt_behaviour,
 };
-use crate::{Handler, WebDriverSession};
 
-#[derive(Debug, PartialEq, serde::Serialize)]
+#[derive(Debug, Clone, PartialEq, serde::Serialize)]
 pub enum PageLoadStrategy {
     None,
     Eager,
@@ -33,17 +39,109 @@ impl ToString for PageLoadStrategy {
     }
 }
 
+/// Represents the current WebDriver session and holds relevant session state.
+/// Currently, only 1 webview is supported per session.
+/// So only there is only 1 InputState.
+pub struct WebDriverSession {
+    /// <https://www.w3.org/TR/webdriver2/#dfn-session-id>
+    id: Uuid,
+
+    /// <https://www.w3.org/TR/webdriver2/#dfn-current-top-level-browsing-context>
+    /// The id of the current top-level browsing context
+    webview_id: Option<WebViewId>,
+
+    /// <https://www.w3.org/TR/webdriver2/#dfn-current-browsing-context>
+    /// The id of the current browsing context
+    browsing_context_id: Option<BrowsingContextId>,
+
+    timeouts: TimeoutsConfiguration,
+
+    page_loading_strategy: PageLoadStrategy,
+
+    strict_file_interactability: bool,
+
+    user_prompt_handler: UserPromptHandler,
+
+    /// <https://w3c.github.io/webdriver/#dfn-input-state-map>
+    input_state_table: RefCell<HashMap<String, InputSourceState>>,
+
+    /// <https://w3c.github.io/webdriver/#dfn-input-cancel-list>
+    input_cancel_list: RefCell<Vec<(String, ActionItem)>>,
+}
+
+impl WebDriverSession {
+    pub fn new() -> WebDriverSession {
+        WebDriverSession {
+            id: Uuid::new_v4(),
+            webview_id: None,
+            browsing_context_id: None,
+            timeouts: TimeoutsConfiguration::default(),
+            page_loading_strategy: PageLoadStrategy::Normal,
+            strict_file_interactability: false,
+            user_prompt_handler: UserPromptHandler::new(),
+            input_state_table: RefCell::new(HashMap::new()),
+            input_cancel_list: RefCell::new(Vec::new()),
+        }
+    }
+
+    pub fn set_webview_id(&mut self, webview_id: Option<WebViewId>) {
+        self.webview_id = webview_id;
+    }
+
+    pub fn set_browsing_context_id(&mut self, browsing_context_id: Option<BrowsingContextId>) {
+        self.browsing_context_id = browsing_context_id;
+    }
+
+    pub fn current_webview_id(&self) -> Option<WebViewId> {
+        self.webview_id
+    }
+
+    pub fn current_browsing_context_id(&self) -> Option<BrowsingContextId> {
+        self.browsing_context_id
+    }
+
+    pub fn session_timeouts(&self) -> &TimeoutsConfiguration {
+        &self.timeouts
+    }
+
+    pub fn session_timeouts_mut(&mut self) -> &mut TimeoutsConfiguration {
+        &mut self.timeouts
+    }
+
+    pub fn page_loading_strategy(&self) -> PageLoadStrategy {
+        self.page_loading_strategy.clone()
+    }
+
+    pub fn strict_file_interactability(&self) -> bool {
+        self.strict_file_interactability
+    }
+
+    pub fn user_prompt_handler(&self) -> &UserPromptHandler {
+        &self.user_prompt_handler
+    }
+
+    pub fn input_state_table(&self) -> Ref<'_, HashMap<String, InputSourceState>> {
+        self.input_state_table.borrow()
+    }
+
+    pub fn input_state_table_mut(&self) -> RefMut<'_, HashMap<String, InputSourceState>> {
+        self.input_state_table.borrow_mut()
+    }
+
+    pub fn input_cancel_list_mut(&self) -> RefMut<'_, Vec<(String, ActionItem)>> {
+        self.input_cancel_list.borrow_mut()
+    }
+}
+
 impl Handler {
     /// <https://w3c.github.io/webdriver/#dfn-create-a-session>
     pub(crate) fn create_session(
         &mut self,
         capabilities: &mut Map<String, Value>,
         servo_capabilities: &ServoCapabilities,
-        webview_id: WebViewId,
-        browsing_context_id: BrowsingContextId,
     ) -> WebDriverResult<Uuid> {
         // Step 2. Let session be a new session
-        let mut session = WebDriverSession::new(browsing_context_id, webview_id);
+        let mut session = WebDriverSession::new();
 
         // Step 3. Let proxy be the result of getting property "proxy" from capabilities
         match capabilities.get("proxy") {

components/webdriver_server/user_prompt.rs

@@ -178,7 +178,7 @@ impl Handler {
     pub(crate) fn handle_dismiss_alert(&self) -> WebDriverResult<WebDriverResponse> {
         // Step 1. If session's current top-level browsing context is no longer open,
         // return error with error code no such window.
-        self.verify_top_level_browsing_context_is_open(self.session()?.webview_id)?;
+        self.verify_top_level_browsing_context_is_open(self.webview_id()?)?;
 
         // Step 3. Dismiss the current user prompt.
         let (sender, receiver) = ipc::channel().unwrap();
@@ -203,7 +203,7 @@ impl Handler {
     pub(crate) fn handle_accept_alert(&self) -> WebDriverResult<WebDriverResponse> {
         // Step 1. If session's current top-level browsing context is no longer open,
         // return error with error code no such window.
-        self.verify_top_level_browsing_context_is_open(self.session()?.webview_id)?;
+        self.verify_top_level_browsing_context_is_open(self.webview_id()?)?;
 
         // Step 3. Accept the current user prompt.
         let (sender, receiver) = ipc::channel().unwrap();
@@ -228,7 +228,7 @@ impl Handler {
     pub(crate) fn handle_get_alert_text(&self) -> WebDriverResult<WebDriverResponse> {
         // Step 1. If session's current top-level browsing context is no longer open,
         // return error with error code no such window.
-        self.verify_top_level_browsing_context_is_open(self.session()?.webview_id)?;
+        self.verify_top_level_browsing_context_is_open(self.webview_id()?)?;
 
         let (sender, receiver) = ipc::channel().unwrap();
         self.send_message_to_embedder(WebDriverCommandMsg::GetAlertText(
@@ -261,7 +261,7 @@ impl Handler {
         &self,
         text: String,
     ) -> WebDriverResult<WebDriverResponse> {
-        let webview_id = self.session()?.webview_id;
+        let webview_id = self.webview_id()?;
 
         // Step 3. If session's current top-level browsing context is no longer open,
         // return error with error code no such window.
@@ -321,7 +321,7 @@ impl Handler {
             Some(prompt_type) => {
                 // Step 2 - 4. Get user prompt handler for the prompt type.
                 let handler =
-                    get_user_prompt_handler(&self.session()?.user_prompt_handler, prompt_type);
+                    get_user_prompt_handler(self.session()?.user_prompt_handler(), prompt_type);
 
                 // Step 5. Perform the substeps based on handler's handler
                 let (sender, receiver) = ipc::channel().unwrap();

ports/servoshell/desktop/app.rs

@@ -382,13 +382,9 @@ impl App {
                     }
                 },
                 WebDriverCommandMsg::GetAllWebViews(response_sender) => {
-                    let webviews = running_state
+                    let webviews = running_state.webviews().iter().map(|(id, _)| *id).collect();
-                        .webviews()
-                        .iter()
-                        .map(|(id, _)| *id)
-                        .collect::<Vec<_>>();
 
-                    if let Err(error) = response_sender.send(Ok(webviews)) {
+                    if let Err(error) = response_sender.send(webviews) {
                         warn!("Failed to send response of GetAllWebViews: {error}");
                     }
                 },

tests/wpt/meta/webdriver/tests/classic/accept_alert/accept.py.ini

@@ -1,3 +0,0 @@
-[accept.py]
-  [test_accept_in_popup_window]
-    expected: FAIL

tests/wpt/meta/webdriver/tests/classic/back/back.py.ini

@@ -7,3 +7,6 @@
 
   [test_seen_nodes[https coop\]]
     expected: FAIL
+
+  [test_history_pushstate]
+    expected: FAIL

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

@@ -1,3 +0,0 @@
-[dismiss.py]
-  [test_dismiss_in_popup_window]
-    expected: FAIL

tests/wpt/meta/webdriver/tests/classic/execute_async_script/arguments.py.ini

@@ -37,3 +37,6 @@
 
   [test_element_reference[shadow-root\]]
     expected: FAIL
+
+  [test_element_reference[window\]]
+    expected: FAIL

tests/wpt/meta/webdriver/tests/classic/execute_script/arguments.py.ini

@@ -37,3 +37,6 @@
 
   [test_element_reference[shadow-root\]]
     expected: FAIL
+
+  [test_element_reference[window\]]
+    expected: FAIL

tests/wpt/meta/webdriver/tests/classic/execute_script/window.py.ini

@@ -1,3 +0,0 @@
-[window.py]
-  [test_same_id_after_cross_origin_navigation]
-    expected: FAIL

tests/wpt/meta/webdriver/tests/classic/forward/forward.py.ini

@@ -13,3 +13,6 @@
 
   [test_removed_iframe]
     expected: FAIL
+
+  [test_history_pushstate]
+    expected: FAIL

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

@@ -1,6 +0,0 @@
-[get.py]
-  [test_get_confirm_text]
-    expected: FAIL
-
-  [test_get_prompt_text]
-    expected: FAIL

tests/wpt/meta/webdriver/tests/classic/perform_actions/navigation.py.ini

@@ -1,4 +0,0 @@
-[navigation.py]
-  expected: TIMEOUT
-  [test_pointer]
-    expected: FAIL

tests/wpt/meta/webdriver/tests/classic/send_alert_text/send.py.ini

@@ -13,3 +13,6 @@
 
   [test_send_alert_text[Fed\\terer\]]
     expected: FAIL
+
+  [test_chained_alert_element_not_interactable[confirm\]]
+    expected: FAIL