From 4e9993128b81b5a3757970786d47fb165ed3ebca Mon Sep 17 00:00:00 2001 From: Euclid Ye Date: Mon, 9 Jun 2025 14:23:31 +0800 Subject: [PATCH] [WebDriver] Unify Cookie related Error types (#37339) Remove `embedder/webdriver.rs::WebDriverCookieError` and use universal `ErrorStatus` from webdriver crate. This is needed as we might need to send back more universal error such as `NoSuchWindow`. Testing: No behaviour change. Signed-off-by: Euclid Ye --- components/script/webdriver_handlers.rs | 28 +++++++++++-------------- components/shared/embedder/webdriver.rs | 15 ++++++------- components/webdriver_server/lib.rs | 27 +++++++++++------------- 3 files changed, 30 insertions(+), 40 deletions(-) diff --git a/components/script/webdriver_handlers.rs b/components/script/webdriver_handlers.rs index 322839aa078..9c647ab5b8b 100644 --- a/components/script/webdriver_handlers.rs +++ b/components/script/webdriver_handlers.rs @@ -9,9 +9,7 @@ use std::ptr::NonNull; use base::id::{BrowsingContextId, PipelineId}; use cookie::Cookie; -use embedder_traits::{ - WebDriverCookieError, WebDriverFrameId, WebDriverJSError, WebDriverJSResult, WebDriverJSValue, -}; +use embedder_traits::{WebDriverFrameId, WebDriverJSError, WebDriverJSResult, WebDriverJSValue}; use euclid::default::{Point2D, Rect, Size2D}; use hyper_serde::Serde; use ipc_channel::ipc::{self, IpcSender}; @@ -994,7 +992,7 @@ pub(crate) fn handle_get_page_source( pub(crate) fn handle_get_cookies( documents: &DocumentCollection, pipeline: PipelineId, - reply: IpcSender>>>, + reply: IpcSender>>, ErrorStatus>>, ) { reply .send( @@ -1008,9 +1006,9 @@ pub(crate) fn handle_get_cookies( .as_global_scope() .resource_threads() .send(GetCookiesDataForUrl(url, sender, NonHTTP)); - receiver.recv().unwrap() + Ok(receiver.recv().unwrap()) }, - None => Vec::new(), + None => Ok(Vec::new()), }, ) .unwrap(); @@ -1021,7 +1019,7 @@ pub(crate) fn handle_get_cookie( documents: &DocumentCollection, pipeline: PipelineId, name: String, - reply: IpcSender>>>, + reply: IpcSender>>, ErrorStatus>>, ) { reply .send( @@ -1036,12 +1034,12 @@ pub(crate) fn handle_get_cookie( .resource_threads() .send(GetCookiesDataForUrl(url, sender, NonHTTP)); let cookies = receiver.recv().unwrap(); - cookies + Ok(cookies .into_iter() .filter(|cookie| cookie.name() == &*name) - .collect() + .collect()) }, - None => Vec::new(), + None => Ok(Vec::new()), }, ) .unwrap(); @@ -1052,15 +1050,13 @@ pub(crate) fn handle_add_cookie( documents: &DocumentCollection, pipeline: PipelineId, cookie: Cookie<'static>, - reply: IpcSender>, + reply: IpcSender>, ) { // TODO: Return a different error if the pipeline doesn't exist let document = match documents.find_document(pipeline) { Some(document) => document, None => { - return reply - .send(Err(WebDriverCookieError::UnableToSetCookie)) - .unwrap(); + return reply.send(Err(ErrorStatus::UnableToSetCookie)).unwrap(); }, }; let url = document.url(); @@ -1073,7 +1069,7 @@ pub(crate) fn handle_add_cookie( let domain = cookie.domain().map(ToOwned::to_owned); reply .send(match (document.is_cookie_averse(), domain) { - (true, _) => Err(WebDriverCookieError::InvalidDomain), + (true, _) => Err(ErrorStatus::InvalidCookieDomain), (false, Some(ref domain)) if url.host_str().map(|x| x == domain).unwrap_or(false) => { let _ = document .window() @@ -1090,7 +1086,7 @@ pub(crate) fn handle_add_cookie( .send(SetCookieForUrl(url, Serde(cookie), method)); Ok(()) }, - (_, _) => Err(WebDriverCookieError::UnableToSetCookie), + (_, _) => Err(ErrorStatus::UnableToSetCookie), }) .unwrap(); } diff --git a/components/shared/embedder/webdriver.rs b/components/shared/embedder/webdriver.rs index d28bb6fc6c3..4f58c10fb6f 100644 --- a/components/shared/embedder/webdriver.rs +++ b/components/shared/embedder/webdriver.rs @@ -94,7 +94,7 @@ pub enum WebDriverScriptCommand { serialize_with = "::hyper_serde::serialize" )] Cookie<'static>, - IpcSender>, + IpcSender>, ), DeleteCookies(IpcSender>), DeleteCookie(String, IpcSender>), @@ -134,8 +134,11 @@ pub enum WebDriverScriptCommand { ElementClick(String, IpcSender, ErrorStatus>>), GetActiveElement(IpcSender>), GetComputedRole(String, IpcSender, ErrorStatus>>), - GetCookie(String, IpcSender>>>), - GetCookies(IpcSender>>>), + GetCookie( + String, + IpcSender>>, ErrorStatus>>, + ), + GetCookies(IpcSender>>, ErrorStatus>>), GetElementAttribute( String, String, @@ -165,12 +168,6 @@ pub enum WebDriverScriptCommand { WillSendKeys(String, String, bool, IpcSender>), } -#[derive(Debug, Deserialize, Serialize)] -pub enum WebDriverCookieError { - InvalidDomain, - UnableToSetCookie, -} - #[derive(Clone, Debug, Deserialize, Serialize)] pub enum WebDriverJSValue { Undefined, diff --git a/components/webdriver_server/lib.rs b/components/webdriver_server/lib.rs index 5d5159f7232..0d92be01a3f 100644 --- a/components/webdriver_server/lib.rs +++ b/components/webdriver_server/lib.rs @@ -24,9 +24,9 @@ use constellation_traits::{EmbedderToConstellationMessage, TraversalDirection}; use cookie::{CookieBuilder, Expiration}; use crossbeam_channel::{Receiver, Sender, after, select, unbounded}; use embedder_traits::{ - MouseButton, WebDriverCommandMsg, WebDriverCommandResponse, WebDriverCookieError, - WebDriverFrameId, WebDriverJSError, WebDriverJSResult, WebDriverJSValue, WebDriverLoadStatus, - WebDriverMessageId, WebDriverScriptCommand, + MouseButton, WebDriverCommandMsg, WebDriverCommandResponse, WebDriverFrameId, WebDriverJSError, + WebDriverJSResult, WebDriverJSValue, WebDriverLoadStatus, WebDriverMessageId, + WebDriverScriptCommand, }; use euclid::{Rect, Size2D}; use http::method::Method; @@ -1365,7 +1365,10 @@ impl Handler { let (sender, receiver) = ipc::channel().unwrap(); let cmd = WebDriverScriptCommand::GetCookies(sender); self.browsing_context_script_command(cmd)?; - let cookies = wait_for_script_response(receiver)?; + let cookies = match wait_for_script_response(receiver)? { + Ok(cookies) => cookies, + Err(error) => return Err(WebDriverError::new(error, "")), + }; let response = cookies .into_iter() .map(|cookie| cookie_msg_to_cookie(cookie.into_inner())) @@ -1377,7 +1380,10 @@ impl Handler { let (sender, receiver) = ipc::channel().unwrap(); let cmd = WebDriverScriptCommand::GetCookie(name, sender); self.browsing_context_script_command(cmd)?; - let cookies = wait_for_script_response(receiver)?; + let cookies = match wait_for_script_response(receiver)? { + Ok(cookies) => cookies, + Err(error) => return Err(WebDriverError::new(error, "")), + }; let Some(response) = cookies .into_iter() .map(|cookie| cookie_msg_to_cookie(cookie.into_inner())) @@ -1410,16 +1416,7 @@ impl Handler { self.browsing_context_script_command(cmd)?; match wait_for_script_response(receiver)? { Ok(_) => Ok(WebDriverResponse::Void), - Err(response) => match response { - WebDriverCookieError::InvalidDomain => Err(WebDriverError::new( - ErrorStatus::InvalidCookieDomain, - "Invalid cookie domain", - )), - WebDriverCookieError::UnableToSetCookie => Err(WebDriverError::new( - ErrorStatus::UnableToSetCookie, - "Unable to set cookie", - )), - }, + Err(error) => Err(WebDriverError::new(error, "")), } }