From d7b9fede99eaaf854e78a2e544595ea8439baecf Mon Sep 17 00:00:00 2001 From: George Roman Date: Mon, 12 Aug 2019 00:51:25 +0300 Subject: [PATCH] Return ErrorStatus from webdriver_handlers --- components/script/dom/node.rs | 8 +- components/script/script_thread.rs | 24 + components/script/webdriver_handlers.rs | 696 ++++++++++-------- components/script_traits/webdriver_msg.rs | 78 +- components/webdriver_server/lib.rs | 94 +-- .../tests/get_element_attribute/get.py.ini | 3 - .../tests/get_element_css_value/get.py.ini | 3 - .../tests/get_element_property/get.py.ini | 3 - .../tests/get_element_rect/get.py.ini | 3 - .../tests/get_element_tag_name/get.py.ini | 3 - .../tests/get_element_text/get.py.ini | 3 - .../tests/switch_to_frame/switch.py.ini | 6 - 12 files changed, 484 insertions(+), 440 deletions(-) diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs index 26bb6d537521..8746a30a7e02 100644 --- a/components/script/dom/node.rs +++ b/components/script/dom/node.rs @@ -1654,7 +1654,7 @@ impl Node { #[allow(unrooted_must_root)] fn new_(flags: NodeFlags, doc: Option<&Document>) -> Node { - Node { + let node = Node { eventtarget: EventTarget::new_inherited(), parent_node: Default::default(), @@ -1673,7 +1673,11 @@ impl Node { style_and_layout_data: Cell::new(None), unique_id: UniqueId::new(), - } + }; + + ScriptThread::save_node_id(node.unique_id()); + + node } // https://dom.spec.whatwg.org/#concept-node-adopt diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index 06442d3042af..88523e09672a 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -687,6 +687,9 @@ pub struct ScriptThread { /// A mechanism to force the compositor's event loop to process events. event_loop_waker: Option>, + + /// A set of all nodes ever created in this script thread + node_ids: DomRefCell>, } /// In the event of thread panic, all data on the stack runs its destructor. However, there @@ -1183,6 +1186,25 @@ impl ScriptThread { }) } + pub fn save_node_id(node_id: String) { + SCRIPT_THREAD_ROOT.with(|root| { + if let Some(script_thread) = root.get() { + let script_thread = unsafe { &*script_thread }; + script_thread.node_ids.borrow_mut().insert(node_id); + } + }) + } + + pub fn has_node_id(node_id: &str) -> bool { + SCRIPT_THREAD_ROOT.with(|root| match root.get() { + Some(script_thread) => { + let script_thread = unsafe { &*script_thread }; + script_thread.node_ids.borrow().contains(node_id) + }, + None => false, + }) + } + /// Creates a new script thread. pub fn new( state: InitialScriptState, @@ -1315,6 +1337,8 @@ impl ScriptThread { user_agent, player_context: state.player_context, event_loop_waker: state.event_loop_waker, + + node_ids: Default::default(), } } diff --git a/components/script/webdriver_handlers.rs b/components/script/webdriver_handlers.rs index 182f110c0c82..22d3e56fcaf4 100644 --- a/components/script/webdriver_handlers.rs +++ b/components/script/webdriver_handlers.rs @@ -33,7 +33,7 @@ use crate::dom::nodelist::NodeList; use crate::dom::window::Window; use crate::dom::xmlserializer::XMLSerializer; use crate::script_runtime::JSContext as SafeJSContext; -use crate::script_thread::Documents; +use crate::script_thread::{Documents, ScriptThread}; use cookie::Cookie; use euclid::default::{Point2D, Rect, Size2D}; use hyper_serde::Serde; @@ -52,18 +52,28 @@ use script_traits::webdriver_msg::{ }; use servo_url::ServoUrl; use webdriver::common::WebElement; +use webdriver::error::ErrorStatus; fn find_node_by_unique_id( documents: &Documents, pipeline: PipelineId, node_id: String, -) -> Option> { - documents.find_document(pipeline).and_then(|document| { +) -> Result, ErrorStatus> { + match documents.find_document(pipeline).and_then(|document| { document .upcast::() .traverse_preorder(ShadowIncluding::Yes) - .find(|candidate| candidate.unique_id() == node_id) - }) + .find(|node| node.unique_id() == node_id) + }) { + Some(node) => Ok(node), + None => { + if ScriptThread::has_node_id(&node_id) { + Err(ErrorStatus::StaleElementReference) + } else { + Err(ErrorStatus::NoSuchElement) + } + }, + } } fn matching_links<'a>( @@ -92,10 +102,10 @@ fn all_matching_links( root_node: &Node, link_text: String, partial: bool, -) -> Result, ()> { +) -> Result, ErrorStatus> { root_node .query_selector_all(DOMString::from("a")) - .map_err(|_| ()) + .map_err(|_| ErrorStatus::UnknownError) .map(|nodes| matching_links(&nodes, link_text, partial).collect()) } @@ -103,10 +113,10 @@ fn first_matching_link( root_node: &Node, link_text: String, partial: bool, -) -> Result, ()> { +) -> Result, ErrorStatus> { root_node .query_selector_all(DOMString::from("a")) - .map_err(|_| ()) + .map_err(|_| ErrorStatus::UnknownError) .map(|nodes| matching_links(&nodes, link_text, partial).take(1).next()) } @@ -243,45 +253,53 @@ pub fn handle_get_browsing_context_id( documents: &Documents, pipeline: PipelineId, webdriver_frame_id: WebDriverFrameId, - reply: IpcSender>, + reply: IpcSender>, ) { - let result = match webdriver_frame_id { - WebDriverFrameId::Short(_) => { - // This isn't supported yet - Err(()) - }, - WebDriverFrameId::Element(x) => find_node_by_unique_id(documents, pipeline, x) - .and_then(|node| { - node.downcast::() - .and_then(|elem| elem.browsing_context_id()) - }) - .ok_or(()), - WebDriverFrameId::Parent => documents - .find_window(pipeline) - .and_then(|window| { - window - .window_proxy() - .parent() - .map(|parent| parent.browsing_context_id()) - }) - .ok_or(()), - }; - - reply.send(result).unwrap() + reply + .send(match webdriver_frame_id { + WebDriverFrameId::Short(_) => { + // This isn't supported yet + Err(ErrorStatus::UnsupportedOperation) + }, + WebDriverFrameId::Element(element_id) => { + find_node_by_unique_id(documents, pipeline, element_id).and_then(|node| { + node.downcast::() + .and_then(|element| element.browsing_context_id()) + .ok_or(ErrorStatus::NoSuchFrame) + }) + }, + WebDriverFrameId::Parent => documents + .find_window(pipeline) + .and_then(|window| { + window + .window_proxy() + .parent() + .map(|parent| parent.browsing_context_id()) + }) + .ok_or(ErrorStatus::NoSuchFrame), + }) + .unwrap(); } pub fn handle_find_element_css( documents: &Documents, pipeline: PipelineId, selector: String, - reply: IpcSender, ()>>, + reply: IpcSender, ErrorStatus>>, ) { - let node_id = documents - .find_document(pipeline) - .ok_or(()) - .and_then(|doc| doc.QuerySelector(DOMString::from(selector)).map_err(|_| ())) - .map(|node| node.map(|x| x.upcast::().unique_id())); - reply.send(node_id).unwrap(); + reply + .send( + documents + .find_document(pipeline) + .ok_or(ErrorStatus::UnknownError) + .and_then(|document| { + document + .QuerySelector(DOMString::from(selector)) + .map_err(|_| ErrorStatus::InvalidSelector) + }) + .map(|node| node.map(|x| x.upcast::().unique_id())), + ) + .unwrap(); } pub fn handle_find_element_link_text( @@ -289,54 +307,66 @@ pub fn handle_find_element_link_text( pipeline: PipelineId, selector: String, partial: bool, - reply: IpcSender, ()>>, + reply: IpcSender, ErrorStatus>>, ) { - let node_id = documents - .find_document(pipeline) - .ok_or(()) - .and_then(|doc| first_matching_link(&doc.upcast::(), selector.clone(), partial)); - reply.send(node_id).unwrap(); + reply + .send( + documents + .find_document(pipeline) + .ok_or(ErrorStatus::UnknownError) + .and_then(|document| { + first_matching_link(&document.upcast::(), selector.clone(), partial) + }), + ) + .unwrap(); } pub fn handle_find_element_tag_name( documents: &Documents, pipeline: PipelineId, selector: String, - reply: IpcSender, ()>>, + reply: IpcSender, ErrorStatus>>, ) { - let node_id = documents - .find_document(pipeline) - .ok_or(()) - .and_then(|doc| { - Ok(doc - .GetElementsByTagName(DOMString::from(selector)) - .elements_iter() - .next()) - }) - .map(|node| node.map(|x| x.upcast::().unique_id())); - reply.send(node_id).unwrap(); + reply + .send( + documents + .find_document(pipeline) + .ok_or(ErrorStatus::UnknownError) + .and_then(|document| { + Ok(document + .GetElementsByTagName(DOMString::from(selector)) + .elements_iter() + .next()) + }) + .map(|node| node.map(|x| x.upcast::().unique_id())), + ) + .unwrap(); } pub fn handle_find_elements_css( documents: &Documents, pipeline: PipelineId, selector: String, - reply: IpcSender, ()>>, + reply: IpcSender, ErrorStatus>>, ) { - let node_ids = documents - .find_document(pipeline) - .ok_or(()) - .and_then(|doc| { - doc.QuerySelectorAll(DOMString::from(selector)) - .map_err(|_| ()) - }) - .map(|nodes| { - nodes - .iter() - .map(|x| x.upcast::().unique_id()) - .collect() - }); - reply.send(node_ids).unwrap(); + reply + .send( + documents + .find_document(pipeline) + .ok_or(ErrorStatus::UnknownError) + .and_then(|document| { + document + .QuerySelectorAll(DOMString::from(selector)) + .map_err(|_| ErrorStatus::InvalidSelector) + }) + .map(|nodes| { + nodes + .iter() + .map(|x| x.upcast::().unique_id()) + .collect() + }), + ) + .unwrap(); } pub fn handle_find_elements_link_text( @@ -344,32 +374,40 @@ pub fn handle_find_elements_link_text( pipeline: PipelineId, selector: String, partial: bool, - reply: IpcSender, ()>>, + reply: IpcSender, ErrorStatus>>, ) { - let node_ids = documents - .find_document(pipeline) - .ok_or(()) - .and_then(|doc| all_matching_links(&doc.upcast::(), selector.clone(), partial)); - reply.send(node_ids).unwrap(); + reply + .send( + documents + .find_document(pipeline) + .ok_or(ErrorStatus::UnknownError) + .and_then(|document| { + all_matching_links(&document.upcast::(), selector.clone(), partial) + }), + ) + .unwrap(); } pub fn handle_find_elements_tag_name( documents: &Documents, pipeline: PipelineId, selector: String, - reply: IpcSender, ()>>, + reply: IpcSender, ErrorStatus>>, ) { - let node_ids = documents - .find_document(pipeline) - .ok_or(()) - .and_then(|doc| Ok(doc.GetElementsByTagName(DOMString::from(selector)))) - .map(|nodes| { - nodes - .elements_iter() - .map(|x| x.upcast::().unique_id()) - .collect::>() - }); - reply.send(node_ids).unwrap(); + reply + .send( + documents + .find_document(pipeline) + .ok_or(ErrorStatus::UnknownError) + .and_then(|document| Ok(document.GetElementsByTagName(DOMString::from(selector)))) + .map(|nodes| { + nodes + .elements_iter() + .map(|x| x.upcast::().unique_id()) + .collect::>() + }), + ) + .unwrap(); } pub fn handle_find_element_element_css( @@ -377,16 +415,17 @@ pub fn handle_find_element_element_css( pipeline: PipelineId, element_id: String, selector: String, - reply: IpcSender, ()>>, + reply: IpcSender, ErrorStatus>>, ) { - let node_id = find_node_by_unique_id(documents, pipeline, element_id) - .ok_or(()) - .and_then(|node| { - node.query_selector(DOMString::from(selector)) - .map_err(|_| ()) - }) - .map(|node| node.map(|x| x.upcast::().unique_id())); - reply.send(node_id).unwrap(); + reply + .send( + find_node_by_unique_id(documents, pipeline, element_id).and_then(|node| { + node.query_selector(DOMString::from(selector)) + .map_err(|_| ErrorStatus::InvalidSelector) + .map(|node| node.map(|x| x.upcast::().unique_id())) + }), + ) + .unwrap(); } pub fn handle_find_element_element_link_text( @@ -395,12 +434,14 @@ pub fn handle_find_element_element_link_text( element_id: String, selector: String, partial: bool, - reply: IpcSender, ()>>, + reply: IpcSender, ErrorStatus>>, ) { - let node_id = find_node_by_unique_id(documents, pipeline, element_id) - .ok_or(()) - .and_then(|node| first_matching_link(&node, selector.clone(), partial)); - reply.send(node_id).unwrap(); + reply + .send( + find_node_by_unique_id(documents, pipeline, element_id) + .and_then(|node| first_matching_link(&node, selector.clone(), partial)), + ) + .unwrap(); } pub fn handle_find_element_element_tag_name( @@ -408,19 +449,22 @@ pub fn handle_find_element_element_tag_name( pipeline: PipelineId, element_id: String, selector: String, - reply: IpcSender, ()>>, + reply: IpcSender, ErrorStatus>>, ) { - let node_id = find_node_by_unique_id(documents, pipeline, element_id) - .ok_or(()) - .and_then(|node| match node.downcast::() { - Some(elem) => Ok(elem - .GetElementsByTagName(DOMString::from(selector)) - .elements_iter() - .next()), - None => Err(()), - }) - .map(|node| node.map(|x| x.upcast::().unique_id())); - reply.send(node_id).unwrap(); + reply + .send( + find_node_by_unique_id(documents, pipeline, element_id).and_then(|node| match node + .downcast::( + ) { + Some(element) => Ok(element + .GetElementsByTagName(DOMString::from(selector)) + .elements_iter() + .next() + .map(|x| x.upcast::().unique_id())), + None => Err(ErrorStatus::UnknownError), + }), + ) + .unwrap(); } pub fn handle_find_element_elements_css( @@ -428,21 +472,22 @@ pub fn handle_find_element_elements_css( pipeline: PipelineId, element_id: String, selector: String, - reply: IpcSender, ()>>, + reply: IpcSender, ErrorStatus>>, ) { - let node_ids = find_node_by_unique_id(documents, pipeline, element_id) - .ok_or(()) - .and_then(|node| { - node.query_selector_all(DOMString::from(selector)) - .map_err(|_| ()) - }) - .map(|nodes| { - nodes - .iter() - .map(|x| x.upcast::().unique_id()) - .collect() - }); - reply.send(node_ids).unwrap(); + reply + .send( + find_node_by_unique_id(documents, pipeline, element_id).and_then(|node| { + node.query_selector_all(DOMString::from(selector)) + .map_err(|_| ErrorStatus::InvalidSelector) + .map(|nodes| { + nodes + .iter() + .map(|x| x.upcast::().unique_id()) + .collect() + }) + }), + ) + .unwrap(); } pub fn handle_find_element_elements_link_text( @@ -451,12 +496,14 @@ pub fn handle_find_element_elements_link_text( element_id: String, selector: String, partial: bool, - reply: IpcSender, ()>>, + reply: IpcSender, ErrorStatus>>, ) { - let node_ids = find_node_by_unique_id(documents, pipeline, element_id) - .ok_or(()) - .and_then(|node| all_matching_links(&node, selector.clone(), partial)); - reply.send(node_ids).unwrap(); + reply + .send( + find_node_by_unique_id(documents, pipeline, element_id) + .and_then(|node| all_matching_links(&node, selector.clone(), partial)), + ) + .unwrap(); } pub fn handle_find_element_elements_tag_name( @@ -464,44 +511,42 @@ pub fn handle_find_element_elements_tag_name( pipeline: PipelineId, element_id: String, selector: String, - reply: IpcSender, ()>>, + reply: IpcSender, ErrorStatus>>, ) { - let node_ids = find_node_by_unique_id(documents, pipeline, element_id) - .ok_or(()) - .and_then(|node| match node.downcast::() { - Some(elem) => Ok(elem.GetElementsByTagName(DOMString::from(selector))), - None => Err(()), - }) - .map(|nodes| { - nodes - .elements_iter() - .map(|x| x.upcast::().unique_id()) - .collect::>() - }); - reply.send(node_ids).unwrap(); + reply + .send( + find_node_by_unique_id(documents, pipeline, element_id).and_then(|node| match node + .downcast::( + ) { + Some(element) => Ok(element + .GetElementsByTagName(DOMString::from(selector)) + .elements_iter() + .map(|x| x.upcast::().unique_id()) + .collect::>()), + None => Err(ErrorStatus::UnknownError), + }), + ) + .unwrap(); } pub fn handle_focus_element( documents: &Documents, pipeline: PipelineId, element_id: String, - reply: IpcSender>, + reply: IpcSender>, ) { reply .send( - match find_node_by_unique_id(documents, pipeline, element_id) { - Some(ref node) => { - match node.downcast::() { - Some(ref elem) => { - // Need a way to find if this actually succeeded - elem.Focus(); - Ok(()) - }, - None => Err(()), - } - }, - None => Err(()), - }, + find_node_by_unique_id(documents, pipeline, element_id).and_then(|node| { + match node.downcast::() { + Some(element) => { + // Need a way to find if this actually succeeded + element.Focus(); + Ok(()) + }, + None => Err(ErrorStatus::UnknownError), + } + }), ) .unwrap(); } @@ -515,8 +560,8 @@ pub fn handle_get_active_element( .send( documents .find_document(pipeline) - .and_then(|doc| doc.GetActiveElement()) - .map(|elem| elem.upcast::().unique_id()), + .and_then(|document| document.GetActiveElement()) + .map(|element| element.upcast::().unique_id()), ) .unwrap(); } @@ -524,25 +569,28 @@ pub fn handle_get_active_element( pub fn handle_get_page_source( documents: &Documents, pipeline: PipelineId, - reply: IpcSender>, + reply: IpcSender>, ) { reply - .send(documents.find_document(pipeline).ok_or(()).and_then(|doc| { - match doc.GetDocumentElement() { - Some(elem) => match elem.GetOuterHTML() { - Ok(source) => Ok(source.to_string()), - Err(_) => { - match XMLSerializer::new(doc.window()) - .SerializeToString(elem.upcast::()) - { - Ok(source) => Ok(source.to_string()), - Err(_) => Err(()), - } + .send( + documents + .find_document(pipeline) + .ok_or(ErrorStatus::UnknownError) + .and_then(|document| match document.GetDocumentElement() { + Some(element) => match element.GetOuterHTML() { + Ok(source) => Ok(source.to_string()), + Err(_) => { + match XMLSerializer::new(document.window()) + .SerializeToString(element.upcast::()) + { + Ok(source) => Ok(source.to_string()), + Err(_) => Err(ErrorStatus::UnknownError), + } + }, }, - }, - None => Err(()), - } - })) + None => Err(ErrorStatus::UnknownError), + }), + ) .unwrap(); } @@ -551,21 +599,24 @@ pub fn handle_get_cookies( pipeline: PipelineId, reply: IpcSender>>>, ) { - // TODO: Return an error if the pipeline doesn't exist? - let cookies = match documents.find_document(pipeline) { - None => Vec::new(), - Some(document) => { - let url = document.url(); - let (sender, receiver) = ipc::channel().unwrap(); - let _ = document - .window() - .upcast::() - .resource_threads() - .send(GetCookiesDataForUrl(url, sender, NonHTTP)); - receiver.recv().unwrap() - }, - }; - reply.send(cookies).unwrap(); + reply + .send( + // TODO: Return an error if the pipeline doesn't exist + match documents.find_document(pipeline) { + Some(document) => { + let url = document.url(); + let (sender, receiver) = ipc::channel().unwrap(); + let _ = document + .window() + .upcast::() + .resource_threads() + .send(GetCookiesDataForUrl(url, sender, NonHTTP)); + receiver.recv().unwrap() + }, + None => Vec::new(), + }, + ) + .unwrap(); } // https://w3c.github.io/webdriver/webdriver-spec.html#get-cookie @@ -575,22 +626,27 @@ pub fn handle_get_cookie( name: String, reply: IpcSender>>>, ) { - // TODO: Return an error if the pipeline doesn't exist? - let cookies = match documents.find_document(pipeline) { - None => Vec::new(), - Some(document) => { - let url = document.url(); - let (sender, receiver) = ipc::channel().unwrap(); - let _ = document - .window() - .upcast::() - .resource_threads() - .send(GetCookiesDataForUrl(url, sender, NonHTTP)); - receiver.recv().unwrap() - }, - }; reply - .send(cookies.into_iter().filter(|c| c.name() == &*name).collect()) + .send( + // TODO: Return an error if the pipeline doesn't exist + match documents.find_document(pipeline) { + Some(document) => { + let url = document.url(); + let (sender, receiver) = ipc::channel().unwrap(); + let _ = document + .window() + .upcast::() + .resource_threads() + .send(GetCookiesDataForUrl(url, sender, NonHTTP)); + let cookies = receiver.recv().unwrap(); + cookies + .into_iter() + .filter(|cookie| cookie.name() == &*name) + .collect() + }, + None => Vec::new(), + }, + ) .unwrap(); } @@ -601,7 +657,7 @@ pub fn handle_add_cookie( cookie: Cookie<'static>, reply: IpcSender>, ) { - // TODO: Return a different error if the pipeline doesn't exist? + // TODO: Return a different error if the pipeline doesn't exist let document = match documents.find_document(pipeline) { Some(document) => document, None => { @@ -645,12 +701,12 @@ pub fn handle_add_cookie( pub fn handle_delete_cookies( documents: &Documents, pipeline: PipelineId, - reply: IpcSender>, + reply: IpcSender>, ) { let document = match documents.find_document(pipeline) { Some(document) => document, None => { - return reply.send(Err(())).unwrap(); + return reply.send(Err(ErrorStatus::UnknownError)).unwrap(); }, }; let url = document.url(); @@ -660,62 +716,62 @@ pub fn handle_delete_cookies( .resource_threads() .send(DeleteCookies(url)) .unwrap(); - let _ = reply.send(Ok(())); + reply.send(Ok(())).unwrap(); } pub fn handle_get_title(documents: &Documents, pipeline: PipelineId, reply: IpcSender) { - // TODO: Return an error if the pipeline doesn't exist. - let title = documents - .find_document(pipeline) - .map(|doc| String::from(doc.Title())) - .unwrap_or_default(); - reply.send(title).unwrap(); + reply + .send( + // TODO: Return an error if the pipeline doesn't exist + documents + .find_document(pipeline) + .map(|document| String::from(document.Title())) + .unwrap_or_default(), + ) + .unwrap(); } pub fn handle_get_rect( documents: &Documents, pipeline: PipelineId, element_id: String, - reply: IpcSender, ()>>, + reply: IpcSender, ErrorStatus>>, ) { reply .send( - match find_node_by_unique_id(documents, pipeline, element_id) { - Some(elem) => { - // https://w3c.github.io/webdriver/webdriver-spec.html#dfn-calculate-the-absolute-position - match elem.downcast::() { - Some(html_elem) => { - // Step 1 - let mut x = 0; - let mut y = 0; - - let mut offset_parent = html_elem.GetOffsetParent(); - - // Step 2 - while let Some(element) = offset_parent { - offset_parent = match element.downcast::() { - Some(elem) => { - x += elem.OffsetLeft(); - y += elem.OffsetTop(); - elem.GetOffsetParent() - }, - None => None, - }; - } - // Step 3 - Ok(Rect::new( - Point2D::new(x as f64, y as f64), - Size2D::new( - html_elem.OffsetWidth() as f64, - html_elem.OffsetHeight() as f64, - ), - )) - }, - None => Err(()), - } - }, - None => Err(()), - }, + find_node_by_unique_id(documents, pipeline, element_id).and_then(|node| { + // https://w3c.github.io/webdriver/webdriver-spec.html#dfn-calculate-the-absolute-position + match node.downcast::() { + Some(html_element) => { + // Step 1 + let mut x = 0; + let mut y = 0; + + let mut offset_parent = html_element.GetOffsetParent(); + + // Step 2 + while let Some(element) = offset_parent { + offset_parent = match element.downcast::() { + Some(elem) => { + x += elem.OffsetLeft(); + y += elem.OffsetTop(); + elem.GetOffsetParent() + }, + None => None, + }; + } + // Step 3 + Ok(Rect::new( + Point2D::new(x as f64, y as f64), + Size2D::new( + html_element.OffsetWidth() as f64, + html_element.OffsetHeight() as f64, + ), + )) + }, + None => Err(ErrorStatus::UnknownError), + } + }), ) .unwrap(); } @@ -724,13 +780,13 @@ pub fn handle_get_text( documents: &Documents, pipeline: PipelineId, node_id: String, - reply: IpcSender>, + reply: IpcSender>, ) { reply - .send(match find_node_by_unique_id(documents, pipeline, node_id) { - Some(ref node) => Ok(node.GetTextContent().map_or("".to_owned(), String::from)), - None => Err(()), - }) + .send( + find_node_by_unique_id(documents, pipeline, node_id) + .and_then(|node| Ok(node.GetTextContent().map_or("".to_owned(), String::from))), + ) .unwrap(); } @@ -738,13 +794,13 @@ pub fn handle_get_name( documents: &Documents, pipeline: PipelineId, node_id: String, - reply: IpcSender>, + reply: IpcSender>, ) { reply - .send(match find_node_by_unique_id(documents, pipeline, node_id) { - Some(node) => Ok(String::from(node.downcast::().unwrap().TagName())), - None => Err(()), - }) + .send( + find_node_by_unique_id(documents, pipeline, node_id) + .and_then(|node| Ok(String::from(node.downcast::().unwrap().TagName()))), + ) .unwrap(); } @@ -753,17 +809,18 @@ pub fn handle_get_attribute( pipeline: PipelineId, node_id: String, name: String, - reply: IpcSender, ()>>, + reply: IpcSender, ErrorStatus>>, ) { reply - .send(match find_node_by_unique_id(documents, pipeline, node_id) { - Some(node) => Ok(node - .downcast::() - .unwrap() - .GetAttribute(DOMString::from(name)) - .map(String::from)), - None => Err(()), - }) + .send( + find_node_by_unique_id(documents, pipeline, node_id).and_then(|node| { + Ok(node + .downcast::() + .unwrap() + .GetAttribute(DOMString::from(name)) + .map(String::from)) + }), + ) .unwrap(); } @@ -773,11 +830,11 @@ pub fn handle_get_property( pipeline: PipelineId, node_id: String, name: String, - reply: IpcSender>, + reply: IpcSender>, ) { reply - .send(match find_node_by_unique_id(documents, pipeline, node_id) { - Some(node) => { + .send( + find_node_by_unique_id(documents, pipeline, node_id).and_then(|node| { let cx = documents.find_document(pipeline).unwrap().window().get_cx(); rooted!(in(*cx) let mut property = UndefinedValue()); @@ -800,9 +857,8 @@ pub fn handle_get_property( Ok(WebDriverJSValue::Undefined) }, } - }, - None => Err(()), - }) + }), + ) .unwrap(); } @@ -811,48 +867,49 @@ pub fn handle_get_css( pipeline: PipelineId, node_id: String, name: String, - reply: IpcSender>, + reply: IpcSender>, ) { reply - .send(match find_node_by_unique_id(documents, pipeline, node_id) { - Some(node) => { + .send( + find_node_by_unique_id(documents, pipeline, node_id).and_then(|node| { let window = window_from_node(&*node); - let elem = node.downcast::().unwrap(); + let element = node.downcast::().unwrap(); Ok(String::from( window - .GetComputedStyle(&elem, None) + .GetComputedStyle(&element, None) .GetPropertyValue(DOMString::from(name)), )) - }, - None => Err(()), - }) + }), + ) .unwrap(); } pub fn handle_get_url(documents: &Documents, pipeline: PipelineId, reply: IpcSender) { - // TODO: Return an error if the pipeline doesn't exist. - let url = documents - .find_document(pipeline) - .map(|document| document.url()) - .unwrap_or_else(|| ServoUrl::parse("about:blank").expect("infallible")); - reply.send(url).unwrap(); + reply + .send( + // TODO: Return an error if the pipeline doesn't exist. + documents + .find_document(pipeline) + .map(|document| document.url()) + .unwrap_or_else(|| ServoUrl::parse("about:blank").expect("infallible")), + ) + .unwrap(); } pub fn handle_is_enabled( documents: &Documents, pipeline: PipelineId, element_id: String, - reply: IpcSender>, + reply: IpcSender>, ) { reply .send( - match find_node_by_unique_id(&documents, pipeline, element_id) { - Some(ref node) => match node.downcast::() { - Some(elem) => Ok(elem.enabled_state()), - None => Err(()), - }, - None => Err(()), - }, + find_node_by_unique_id(&documents, pipeline, element_id).and_then(|node| match node + .downcast::( + ) { + Some(element) => Ok(element.enabled_state()), + None => Err(ErrorStatus::UnknownError), + }), ) .unwrap(); } @@ -861,24 +918,21 @@ pub fn handle_is_selected( documents: &Documents, pipeline: PipelineId, element_id: String, - reply: IpcSender>, + reply: IpcSender>, ) { reply .send( - match find_node_by_unique_id(documents, pipeline, element_id) { - Some(ref node) => { - if let Some(input_element) = node.downcast::() { - Ok(input_element.Checked()) - } else if let Some(option_element) = node.downcast::() { - Ok(option_element.Selected()) - } else if node.is::() { - Ok(false) // regular elements are not selectable - } else { - Err(()) - } - }, - None => Err(()), - }, + find_node_by_unique_id(documents, pipeline, element_id).and_then(|node| { + if let Some(input_element) = node.downcast::() { + Ok(input_element.Checked()) + } else if let Some(option_element) = node.downcast::() { + Ok(option_element.Selected()) + } else if node.is::() { + Ok(false) // regular elements are not selectable + } else { + Err(ErrorStatus::UnknownError) + } + }), ) .unwrap(); } diff --git a/components/script_traits/webdriver_msg.rs b/components/script_traits/webdriver_msg.rs index c33a5ae302d9..e7087d8825d3 100644 --- a/components/script_traits/webdriver_msg.rs +++ b/components/script_traits/webdriver_msg.rs @@ -11,6 +11,7 @@ use ipc_channel::ipc::IpcSender; use msg::constellation_msg::BrowsingContextId; use servo_url::ServoUrl; use webdriver::common::WebElement; +use webdriver::error::ErrorStatus; #[derive(Debug, Deserialize, Serialize)] pub enum WebDriverScriptCommand { @@ -22,36 +23,65 @@ pub enum WebDriverScriptCommand { Cookie<'static>, IpcSender>, ), - DeleteCookies(IpcSender>), + DeleteCookies(IpcSender>), ExecuteScript(String, IpcSender), ExecuteAsyncScript(String, IpcSender), - FindElementCSS(String, IpcSender, ()>>), - FindElementLinkText(String, bool, IpcSender, ()>>), - FindElementTagName(String, IpcSender, ()>>), - FindElementsCSS(String, IpcSender, ()>>), - FindElementsLinkText(String, bool, IpcSender, ()>>), - FindElementsTagName(String, IpcSender, ()>>), - FindElementElementCSS(String, String, IpcSender, ()>>), - FindElementElementLinkText(String, String, bool, IpcSender, ()>>), - FindElementElementTagName(String, String, IpcSender, ()>>), - FindElementElementsCSS(String, String, IpcSender, ()>>), - FindElementElementsLinkText(String, String, bool, IpcSender, ()>>), - FindElementElementsTagName(String, String, IpcSender, ()>>), - FocusElement(String, IpcSender>), + FindElementCSS(String, IpcSender, ErrorStatus>>), + FindElementLinkText(String, bool, IpcSender, ErrorStatus>>), + FindElementTagName(String, IpcSender, ErrorStatus>>), + FindElementsCSS(String, IpcSender, ErrorStatus>>), + FindElementsLinkText(String, bool, IpcSender, ErrorStatus>>), + FindElementsTagName(String, IpcSender, ErrorStatus>>), + FindElementElementCSS( + String, + String, + IpcSender, ErrorStatus>>, + ), + FindElementElementLinkText( + String, + String, + bool, + IpcSender, ErrorStatus>>, + ), + FindElementElementTagName( + String, + String, + IpcSender, ErrorStatus>>, + ), + FindElementElementsCSS(String, String, IpcSender, ErrorStatus>>), + FindElementElementsLinkText( + String, + String, + bool, + IpcSender, ErrorStatus>>, + ), + FindElementElementsTagName(String, String, IpcSender, ErrorStatus>>), + FocusElement(String, IpcSender>), GetActiveElement(IpcSender>), GetCookie(String, IpcSender>>>), GetCookies(IpcSender>>>), - GetElementAttribute(String, String, IpcSender, ()>>), - GetElementProperty(String, String, IpcSender>), - GetElementCSS(String, String, IpcSender>), - GetElementRect(String, IpcSender, ()>>), - GetElementTagName(String, IpcSender>), - GetElementText(String, IpcSender>), - GetBrowsingContextId(WebDriverFrameId, IpcSender>), + GetElementAttribute( + String, + String, + IpcSender, ErrorStatus>>, + ), + GetElementProperty( + String, + String, + IpcSender>, + ), + GetElementCSS(String, String, IpcSender>), + GetElementRect(String, IpcSender, ErrorStatus>>), + GetElementTagName(String, IpcSender>), + GetElementText(String, IpcSender>), + GetBrowsingContextId( + WebDriverFrameId, + IpcSender>, + ), GetUrl(IpcSender), - GetPageSource(IpcSender>), - IsEnabled(String, IpcSender>), - IsSelected(String, IpcSender>), + GetPageSource(IpcSender>), + IsEnabled(String, IpcSender>), + IsSelected(String, IpcSender>), GetTitle(IpcSender), } diff --git a/components/webdriver_server/lib.rs b/components/webdriver_server/lib.rs index a38d9bc37892..a40b766a66ae 100644 --- a/components/webdriver_server/lib.rs +++ b/components/webdriver_server/lib.rs @@ -735,10 +735,7 @@ impl Handler { Ok(is_enabled) => Ok(WebDriverResponse::Generic(ValueResponse( serde_json::to_value(is_enabled)?, ))), - Err(_) => Err(WebDriverError::new( - ErrorStatus::StaleElementReference, - "Element not found", - )), + Err(error) => Err(WebDriverError::new(error, "")), } } @@ -754,10 +751,7 @@ impl Handler { Ok(is_selected) => Ok(WebDriverResponse::Generic(ValueResponse( serde_json::to_value(is_selected)?, ))), - Err(_) => Err(WebDriverError::new( - ErrorStatus::StaleElementReference, - "Element not found", - )), + Err(error) => Err(WebDriverError::new(error, "")), } } @@ -860,10 +854,7 @@ impl Handler { )?; Ok(WebDriverResponse::Generic(ValueResponse(value_resp))) }, - Err(_) => Err(WebDriverError::new( - ErrorStatus::InvalidSelector, - "Invalid selector", - )), + Err(error) => Err(WebDriverError::new(error, "")), } } @@ -922,13 +913,13 @@ impl Handler { let cmd = WebDriverScriptCommand::GetBrowsingContextId(frame_id, sender); self.browsing_context_script_command(cmd)?; - let browsing_context_id = receiver.recv().unwrap().or(Err(WebDriverError::new( - ErrorStatus::NoSuchFrame, - "Frame does not exist", - )))?; - - self.session_mut()?.browsing_context_id = browsing_context_id; - Ok(WebDriverResponse::Void) + match receiver.recv().unwrap() { + Ok(browsing_context_id) => { + self.session_mut()?.browsing_context_id = browsing_context_id; + Ok(WebDriverResponse::Void) + }, + Err(error) => Err(WebDriverError::new(error, "")), + } } // https://w3c.github.io/webdriver/#find-elements @@ -974,10 +965,7 @@ impl Handler { serde_json::to_value(resp_value)?, ))) }, - Err(_) => Err(WebDriverError::new( - ErrorStatus::InvalidSelector, - "Invalid selector", - )), + Err(error) => Err(WebDriverError::new(error, "")), } } @@ -1030,10 +1018,7 @@ impl Handler { )?; Ok(WebDriverResponse::Generic(ValueResponse(value_resp))) }, - Err(_) => Err(WebDriverError::new( - ErrorStatus::InvalidSelector, - "Invalid selector", - )), + Err(error) => Err(WebDriverError::new(error, "")), } } @@ -1089,10 +1074,7 @@ impl Handler { serde_json::to_value(resp_value)?, ))) }, - Err(_) => Err(WebDriverError::new( - ErrorStatus::InvalidSelector, - "Invalid selector", - )), + Err(error) => Err(WebDriverError::new(error, "")), } } @@ -1111,10 +1093,7 @@ impl Handler { }; Ok(WebDriverResponse::ElementRect(response)) }, - Err(_) => Err(WebDriverError::new( - ErrorStatus::StaleElementReference, - "Unable to find element in document", - )), + Err(error) => Err(WebDriverError::new(error, "")), } } @@ -1126,10 +1105,7 @@ impl Handler { Ok(value) => Ok(WebDriverResponse::Generic(ValueResponse( serde_json::to_value(value)?, ))), - Err(_) => Err(WebDriverError::new( - ErrorStatus::StaleElementReference, - "Unable to find element in document", - )), + Err(error) => Err(WebDriverError::new(error, "")), } } @@ -1154,10 +1130,7 @@ impl Handler { Ok(value) => Ok(WebDriverResponse::Generic(ValueResponse( serde_json::to_value(value)?, ))), - Err(_) => Err(WebDriverError::new( - ErrorStatus::StaleElementReference, - "Unable to find element in document", - )), + Err(error) => Err(WebDriverError::new(error, "")), } } @@ -1177,10 +1150,7 @@ impl Handler { Ok(value) => Ok(WebDriverResponse::Generic(ValueResponse( serde_json::to_value(value)?, ))), - Err(_) => Err(WebDriverError::new( - ErrorStatus::StaleElementReference, - "Unable to find element in document", - )), + Err(error) => Err(WebDriverError::new(error, "")), } } @@ -1202,10 +1172,7 @@ impl Handler { Ok(value) => Ok(WebDriverResponse::Generic(ValueResponse( serde_json::to_value(SendableWebDriverJSValue(value))?, ))), - Err(_) => Err(WebDriverError::new( - ErrorStatus::StaleElementReference, - "Unable to find element in document", - )), + Err(error) => Err(WebDriverError::new(error, "")), } } @@ -1222,10 +1189,7 @@ impl Handler { Ok(value) => Ok(WebDriverResponse::Generic(ValueResponse( serde_json::to_value(value)?, ))), - Err(_) => Err(WebDriverError::new( - ErrorStatus::StaleElementReference, - "Unable to find element in document", - )), + Err(error) => Err(WebDriverError::new(error, "")), } } @@ -1295,10 +1259,7 @@ impl Handler { self.browsing_context_script_command(cmd)?; match receiver.recv().unwrap() { Ok(_) => Ok(WebDriverResponse::Void), - Err(_) => Err(WebDriverError::new( - ErrorStatus::NoSuchWindow, - "No such window found.", - )), + Err(error) => Err(WebDriverError::new(error, "")), } } @@ -1355,10 +1316,7 @@ impl Handler { Ok(source) => Ok(WebDriverResponse::Generic(ValueResponse( serde_json::to_value(source)?, ))), - Err(_) => Err(WebDriverError::new( - ErrorStatus::UnknownError, - "Unknown error", - )), + Err(error) => Err(WebDriverError::new(error, "")), } } @@ -1469,12 +1427,10 @@ impl Handler { .unwrap(); // TODO: distinguish the not found and not focusable cases - receiver.recv().unwrap().or_else(|_| { - Err(WebDriverError::new( - ErrorStatus::StaleElementReference, - "Element not found or not focusable", - )) - })?; + receiver + .recv() + .unwrap() + .or_else(|error| Err(WebDriverError::new(error, "")))?; let input_events = send_keys(&keys.text); diff --git a/tests/wpt/metadata/webdriver/tests/get_element_attribute/get.py.ini b/tests/wpt/metadata/webdriver/tests/get_element_attribute/get.py.ini index 21ecdc958e77..585c289213eb 100644 --- a/tests/wpt/metadata/webdriver/tests/get_element_attribute/get.py.ini +++ b/tests/wpt/metadata/webdriver/tests/get_element_attribute/get.py.ini @@ -2,9 +2,6 @@ [test_boolean_attribute[video-attrs17\]] expected: FAIL - [test_element_not_found] - expected: FAIL - [test_global_boolean_attributes] expected: FAIL diff --git a/tests/wpt/metadata/webdriver/tests/get_element_css_value/get.py.ini b/tests/wpt/metadata/webdriver/tests/get_element_css_value/get.py.ini index e047bc694372..6cf1e1da10a2 100644 --- a/tests/wpt/metadata/webdriver/tests/get_element_css_value/get.py.ini +++ b/tests/wpt/metadata/webdriver/tests/get_element_css_value/get.py.ini @@ -1,7 +1,4 @@ [get.py] - [test_element_not_found] - expected: FAIL - [test_no_browsing_context] expected: ERROR diff --git a/tests/wpt/metadata/webdriver/tests/get_element_property/get.py.ini b/tests/wpt/metadata/webdriver/tests/get_element_property/get.py.ini index 6ba107cdf754..d6b7c0acee80 100644 --- a/tests/wpt/metadata/webdriver/tests/get_element_property/get.py.ini +++ b/tests/wpt/metadata/webdriver/tests/get_element_property/get.py.ini @@ -5,9 +5,6 @@ [test_idl_attribute] expected: FAIL - [test_element_not_found] - expected: FAIL - [test_primitives_set_by_execute_script[42-42\]] expected: FAIL diff --git a/tests/wpt/metadata/webdriver/tests/get_element_rect/get.py.ini b/tests/wpt/metadata/webdriver/tests/get_element_rect/get.py.ini index e047bc694372..6cf1e1da10a2 100644 --- a/tests/wpt/metadata/webdriver/tests/get_element_rect/get.py.ini +++ b/tests/wpt/metadata/webdriver/tests/get_element_rect/get.py.ini @@ -1,7 +1,4 @@ [get.py] - [test_element_not_found] - expected: FAIL - [test_no_browsing_context] expected: ERROR diff --git a/tests/wpt/metadata/webdriver/tests/get_element_tag_name/get.py.ini b/tests/wpt/metadata/webdriver/tests/get_element_tag_name/get.py.ini index 624ec583ae8a..87de227b9660 100644 --- a/tests/wpt/metadata/webdriver/tests/get_element_tag_name/get.py.ini +++ b/tests/wpt/metadata/webdriver/tests/get_element_tag_name/get.py.ini @@ -1,7 +1,4 @@ [get.py] - [test_element_not_found] - expected: FAIL - [test_no_browsing_context] expected: ERROR diff --git a/tests/wpt/metadata/webdriver/tests/get_element_text/get.py.ini b/tests/wpt/metadata/webdriver/tests/get_element_text/get.py.ini index 91db3bd49f6d..6cf1e1da10a2 100644 --- a/tests/wpt/metadata/webdriver/tests/get_element_text/get.py.ini +++ b/tests/wpt/metadata/webdriver/tests/get_element_text/get.py.ini @@ -1,7 +1,4 @@ [get.py] - [test_getting_text_of_a_non_existant_element_is_an_error] - expected: FAIL - [test_no_browsing_context] expected: ERROR diff --git a/tests/wpt/metadata/webdriver/tests/switch_to_frame/switch.py.ini b/tests/wpt/metadata/webdriver/tests/switch_to_frame/switch.py.ini index 0f50b6c34de7..839bee104d55 100644 --- a/tests/wpt/metadata/webdriver/tests/switch_to_frame/switch.py.ini +++ b/tests/wpt/metadata/webdriver/tests/switch_to_frame/switch.py.ini @@ -8,18 +8,12 @@ [test_frame_id_webelement_frame[0-foo\]] expected: FAIL - [test_frame_id_webelement_no_element_reference] - expected: FAIL - [test_frame_id_number_index_out_of_bounds] expected: FAIL [test_frame_id_number_index[0-foo\]] expected: FAIL - [test_frame_id_webelement_stale_reference] - expected: FAIL - [test_no_browsing_context] expected: ERROR