From f323e670245aa7854e61ca284ffc1d69944fd60d Mon Sep 17 00:00:00 2001 From: Euclid Ye Date: Sat, 23 Aug 2025 01:19:12 +0800 Subject: [PATCH] webdriver: Simplify webview related steps that are guaranteed to succeed (#38855) Testing: Semantically it is the same. But still run the test locally and in [try](https://github.com/yezhizhen/servo/actions/runs/17159323926) as I was worried about some unexpected side effect from 176e42d36d49bce2f9b759c3e93faa929e38fa39. But seems everything is good. --------- Signed-off-by: Euclid Ye --- components/webdriver_server/lib.rs | 60 +++++++++++--------------- components/webdriver_server/session.rs | 9 ++-- 2 files changed, 31 insertions(+), 38 deletions(-) diff --git a/components/webdriver_server/lib.rs b/components/webdriver_server/lib.rs index 1a6cb9493ef..7d7ad686a18 100644 --- a/components/webdriver_server/lib.rs +++ b/components/webdriver_server/lib.rs @@ -574,9 +574,9 @@ impl Handler { webview_id }, }; - self.session_mut()?.set_webview_id(Some(webview_id)); + self.session_mut()?.set_webview_id(webview_id); self.session_mut()? - .set_browsing_context_id(Some(BrowsingContextId::from(webview_id))); + .set_browsing_context_id(BrowsingContextId::from(webview_id)); // Step 9. Set the request queue to a new queue. // Skip here because the requests are handled in the external crate. @@ -674,7 +674,7 @@ impl Handler { // Step 8.3. Set current browsing context with session and current top browsing context self.session_mut()? - .set_browsing_context_id(Some(BrowsingContextId::from(webview_id))); + .set_browsing_context_id(BrowsingContextId::from(webview_id)); Ok(WebDriverResponse::Void) } @@ -1016,7 +1016,7 @@ impl Handler { // Step 5. Set current browsing context with session and current top browsing context. self.session_mut()? - .set_browsing_context_id(Some(BrowsingContextId::from(webview_id))); + .set_browsing_context_id(BrowsingContextId::from(webview_id)); Ok(WebDriverResponse::Void) } @@ -1066,7 +1066,7 @@ impl Handler { /// fn handle_window_handles(&mut self) -> WebDriverResult { - let mut handles = self.get_window_handles()?; + let mut handles = self.get_window_handles(); handles.sort(); Ok(WebDriverResponse::Generic(ValueResponse( @@ -1074,35 +1074,25 @@ impl Handler { ))) } - fn get_window_handle(&mut self, webview_id: WebViewId) -> WebDriverResult { - self.get_window_handles()? + fn get_window_handle(&mut self, webview_id: WebViewId) -> Option { + self.get_window_handles() .iter() .find(|id| id == &&webview_id.to_string()) .cloned() - .ok_or_else(|| { - WebDriverError::new( - ErrorStatus::UnknownError, - "No such window while getting window handle", - ) - }) } - fn get_window_handles(&self) -> WebDriverResult> { - let handles = self - .get_all_webview_ids()? + fn get_window_handles(&self) -> Vec { + self.get_all_webview_ids() .into_iter() .map(|id| id.to_string()) - .collect::>(); - - Ok(handles) + .collect() } - fn get_all_webview_ids(&self) -> WebDriverResult> { + fn get_all_webview_ids(&self) -> Vec { let (sender, receiver) = ipc::channel().unwrap(); - self.send_message_to_embedder(WebDriverCommandMsg::GetAllWebViews(sender))?; - let webviews = wait_for_ipc_response(receiver)?; - - Ok(webviews) + self.send_message_to_embedder(WebDriverCommandMsg::GetAllWebViews(sender)) + .unwrap(); + wait_for_ipc_response(receiver).unwrap_or_default() } /// @@ -1136,7 +1126,7 @@ impl Handler { wait_for_ipc_response(receiver)?; // Step 4. If there are no more open top-level browsing contexts, try to close the session. - let window_handles = self.get_window_handles()?; + let window_handles = self.get_window_handles(); if window_handles.is_empty() { self.session = None; @@ -1172,7 +1162,9 @@ impl Handler { if let Ok(webview_id) = receiver.recv() { let _ = self.wait_for_document_ready_state(); - let handle = self.get_window_handle(webview_id)?; + let handle = self + .get_window_handle(webview_id) + .expect("Failed to get window handle of an existing webview"); Ok(WebDriverResponse::NewWindow(NewWindowResponse { handle, @@ -1204,7 +1196,7 @@ impl Handler { // Step 3. Set the current browsing context with session and // session's current top-level browsing context. self.session_mut()? - .set_browsing_context_id(Some(BrowsingContextId::from(webview_id))); + .set_browsing_context_id(BrowsingContextId::from(webview_id)); return Ok(WebDriverResponse::Void); }, // id is a Number object @@ -1248,7 +1240,7 @@ impl Handler { match wait_for_ipc_response(receiver)? { Ok(browsing_context_id) => { self.session_mut()? - .set_browsing_context_id(Some(browsing_context_id)); + .set_browsing_context_id(browsing_context_id); Ok(WebDriverResponse::Void) }, Err(error) => Err(WebDriverError::new(error, "")), @@ -1260,9 +1252,9 @@ impl Handler { &mut self, parameters: &SwitchToWindowParameters, ) -> WebDriverResult { - let window_handles = self.get_all_webview_ids()?; - let Some(webview_id) = window_handles - .iter() + let Some(webview_id) = self + .get_all_webview_ids() + .into_iter() .find(|id| id.to_string() == parameters.handle) else { return Err(WebDriverError::new( @@ -1272,8 +1264,8 @@ impl Handler { }; let session = self.session_mut()?; - session.set_webview_id(Some(*webview_id)); - session.set_browsing_context_id(Some(BrowsingContextId::from(*webview_id))); + session.set_webview_id(webview_id); + session.set_browsing_context_id(BrowsingContextId::from(webview_id)); Ok(WebDriverResponse::Void) } @@ -1289,7 +1281,7 @@ impl Handler { match wait_for_ipc_response(receiver)? { Ok(browsing_context_id) => { self.session_mut()? - .set_browsing_context_id(Some(browsing_context_id)); + .set_browsing_context_id(browsing_context_id); Ok(WebDriverResponse::Void) }, Err(error) => Err(WebDriverError::new(error, "")), diff --git a/components/webdriver_server/session.rs b/components/webdriver_server/session.rs index 1e5cf06e598..2494f6578d0 100644 --- a/components/webdriver_server/session.rs +++ b/components/webdriver_server/session.rs @@ -60,6 +60,7 @@ pub struct WebDriverSession { strict_file_interactability: bool, + /// user_prompt_handler: UserPromptHandler, /// @@ -84,12 +85,12 @@ impl WebDriverSession { } } - pub fn set_webview_id(&mut self, webview_id: Option) { - self.webview_id = webview_id; + pub fn set_webview_id(&mut self, webview_id: WebViewId) { + self.webview_id = Some(webview_id); } - pub fn set_browsing_context_id(&mut self, browsing_context_id: Option) { - self.browsing_context_id = browsing_context_id; + pub fn set_browsing_context_id(&mut self, browsing_context_id: BrowsingContextId) { + self.browsing_context_id = Some(browsing_context_id); } pub fn current_webview_id(&self) -> Option {