From 28ac0abf6a1aeb8540d1b41526c0fccffa749ce9 Mon Sep 17 00:00:00 2001 From: James Graham Date: Tue, 28 Apr 2015 18:16:23 +0100 Subject: [PATCH 1/4] Make WebDriver Get() command wait on pages loading before returning. This makes using WebDriver significantly less racy. Also refactors the message structure a little --- components/compositing/constellation.rs | 61 ++++++++++++++++++------- components/msg/constellation_msg.rs | 11 +++-- components/script/script_task.rs | 6 +-- components/script/webdriver_handlers.rs | 2 +- components/script_traits/lib.rs | 2 +- components/webdriver_server/lib.rs | 40 ++++++++++------ components/webdriver_traits/lib.rs | 4 +- 7 files changed, 87 insertions(+), 39 deletions(-) diff --git a/components/compositing/constellation.rs b/components/compositing/constellation.rs index 0c344f1c758..af94cea02e9 100644 --- a/components/compositing/constellation.rs +++ b/components/compositing/constellation.rs @@ -29,6 +29,7 @@ use msg::constellation_msg::{IFrameSandboxState, MozBrowserEvent, NavigationDire use msg::constellation_msg::{Key, KeyState, KeyModifiers, LoadData}; use msg::constellation_msg::{SubpageId, WindowSizeData}; use msg::constellation_msg::{self, ConstellationChan, Failure}; +use msg::constellation_msg::{WebDriverCommandMsg}; use net_traits::{self, ResourceTask}; use net_traits::image_cache_task::ImageCacheTask; use net_traits::storage_task::{StorageTask, StorageTaskMsg}; @@ -49,7 +50,7 @@ use util::geometry::PagePx; use util::opts; use util::task::spawn_named; use clipboard::ClipboardContext; -use webdriver_traits::WebDriverScriptCommand; +use webdriver_traits; /// Maintains the pipelines and navigation context and grants permission to composite. /// @@ -122,6 +123,9 @@ pub struct Constellation { /// Means of accessing the clipboard clipboard_ctx: ClipboardContext, + + /// Bits of state used to interact with the webdriver implementation + webdriver: WebDriverData } /// Stores the navigation context for a single frame in the frame tree. @@ -185,6 +189,18 @@ pub struct SendableFrameTree { pub children: Vec, } +struct WebDriverData { + load_channel: Option> +} + +impl WebDriverData { + pub fn new() -> WebDriverData { + WebDriverData { + load_channel: None + } + } +} + #[derive(Clone, Copy)] enum ExitPipelineMode { Normal, @@ -233,6 +249,7 @@ impl Constellation { }, phantom: PhantomData, clipboard_ctx: ClipboardContext::new().unwrap(), + webdriver: WebDriverData::new() }; constellation.run(); }); @@ -376,7 +393,7 @@ impl Constellation { // script, and reflow messages have been sent. ConstellationMsg::LoadComplete => { debug!("constellation got load complete message"); - self.compositor_proxy.send(CompositorMsg::LoadComplete); + self.handle_load_complete_msg() } // Handle a forward or back request ConstellationMsg::Navigate(pipeline_info, direction) => { @@ -426,14 +443,9 @@ impl Constellation { }; sender.send(result).unwrap(); } - ConstellationMsg::CompositePng(reply) => { - self.compositor_proxy.send(CompositorMsg::CreatePng(reply)); - } - ConstellationMsg::WebDriverCommand(pipeline_id, - command) => { + ConstellationMsg::WebDriverCommand(command) => { debug!("constellation got webdriver command message"); - self.handle_webdriver_command_msg(pipeline_id, - command); + self.handle_webdriver_msg(command); } ConstellationMsg::ViewportConstrained(pipeline_id, constraints) => { debug!("constellation got viewport-constrained event message"); @@ -649,6 +661,14 @@ impl Constellation { } } + fn handle_load_complete_msg(&mut self) { + self.compositor_proxy.send(CompositorMsg::LoadComplete); + if let Some(ref reply_chan) = self.webdriver.load_channel { + reply_chan.send(webdriver_traits::LoadComplete).unwrap(); + } + self.webdriver.load_channel = None; + } + fn handle_navigate_msg(&mut self, pipeline_info: Option<(PipelineId, SubpageId)>, direction: constellation_msg::NavigationDirection) { @@ -814,15 +834,24 @@ impl Constellation { } } - fn handle_webdriver_command_msg(&mut self, - pipeline_id: PipelineId, - msg: WebDriverScriptCommand) { + fn handle_webdriver_msg(&mut self, msg: WebDriverCommandMsg) { // Find the script channel for the given parent pipeline, // and pass the event to that script task. - let pipeline = self.pipeline(pipeline_id); - let control_msg = ConstellationControlMsg::WebDriverCommand(pipeline_id, msg); - let ScriptControlChan(ref script_channel) = pipeline.script_chan; - script_channel.send(control_msg).unwrap(); + match msg { + WebDriverCommandMsg::LoadUrl(pipeline_id, load_data, reply) => { + self.handle_load_url_msg(pipeline_id, load_data); + self.webdriver.load_channel = Some(reply); + }, + WebDriverCommandMsg::ScriptCommand(pipeline_id, cmd) => { + let pipeline = self.pipeline(pipeline_id); + let control_msg = ConstellationControlMsg::WebDriverScriptCommand(pipeline_id, cmd); + let ScriptControlChan(ref script_channel) = pipeline.script_chan; + script_channel.send(control_msg).unwrap(); + }, + WebDriverCommandMsg::TakeScreenshot(reply) => { + self.compositor_proxy.send(CompositorMsg::CreatePng(reply)); + }, + } } fn add_or_replace_pipeline_in_frame_tree(&mut self, frame_change: FrameChange) { diff --git a/components/msg/constellation_msg.rs b/components/msg/constellation_msg.rs index a147842db25..ffe76ddb7f8 100644 --- a/components/msg/constellation_msg.rs +++ b/components/msg/constellation_msg.rs @@ -18,7 +18,7 @@ use util::geometry::{PagePx, ViewportPx}; use std::collections::HashMap; use std::sync::mpsc::{channel, Sender, Receiver}; use style::viewport::ViewportConstraints; -use webdriver_traits::WebDriverScriptCommand; +use webdriver_traits::{WebDriverScriptCommand, LoadComplete}; use url::Url; #[derive(Clone)] @@ -237,11 +237,9 @@ pub enum Msg { /// Requests that the constellation retrieve the current contents of the clipboard GetClipboardContents(Sender), /// Dispatch a webdriver command - WebDriverCommand(PipelineId, WebDriverScriptCommand), + WebDriverCommand(WebDriverCommandMsg), /// Notifies the constellation that the viewport has been constrained in some manner ViewportConstrained(PipelineId, ViewportConstraints), - /// Create a PNG of the window contents - CompositePng(Sender>), /// Query the constellation to see if the current compositor output is stable IsReadyToSaveImage(HashMap), /// Notification that this iframe should be removed. @@ -319,6 +317,11 @@ impl MozBrowserEvent { } } +pub enum WebDriverCommandMsg { + LoadUrl(PipelineId, LoadData, Sender), + ScriptCommand(PipelineId, WebDriverScriptCommand), + TakeScreenshot(Sender>) +} /// Similar to net::resource_task::LoadData /// can be passed to LoadUrl to load a page with GET/POST diff --git a/components/script/script_task.rs b/components/script/script_task.rs index c9d92497a04..df838f130e8 100644 --- a/components/script/script_task.rs +++ b/components/script/script_task.rs @@ -726,7 +726,7 @@ impl ScriptTask { self.handle_update_subpage_id(containing_pipeline_id, old_subpage_id, new_subpage_id), ConstellationControlMsg::FocusIFrame(containing_pipeline_id, subpage_id) => self.handle_focus_iframe_msg(containing_pipeline_id, subpage_id), - ConstellationControlMsg::WebDriverCommand(pipeline_id, msg) => + ConstellationControlMsg::WebDriverScriptCommand(pipeline_id, msg) => self.handle_webdriver_msg(pipeline_id, msg), ConstellationControlMsg::TickAllAnimations(pipeline_id) => self.handle_tick_all_animations(pipeline_id), @@ -801,8 +801,8 @@ impl ScriptTask { fn handle_webdriver_msg(&self, pipeline_id: PipelineId, msg: WebDriverScriptCommand) { let page = self.root_page(); match msg { - WebDriverScriptCommand::EvaluateJS(script, reply) => - webdriver_handlers::handle_evaluate_js(&page, pipeline_id, script, reply), + WebDriverScriptCommand::ExecuteScript(script, reply) => + webdriver_handlers::handle_execute_script(&page, pipeline_id, script, reply), WebDriverScriptCommand::FindElementCSS(selector, reply) => webdriver_handlers::handle_find_element_css(&page, pipeline_id, selector, reply), WebDriverScriptCommand::FindElementsCSS(selector, reply) => diff --git a/components/script/webdriver_handlers.rs b/components/script/webdriver_handlers.rs index 55525455e10..cdaea5f5fb3 100644 --- a/components/script/webdriver_handlers.rs +++ b/components/script/webdriver_handlers.rs @@ -35,7 +35,7 @@ fn find_node_by_unique_id(page: &Rc, pipeline: PipelineId, node_id: String None } -pub fn handle_evaluate_js(page: &Rc, pipeline: PipelineId, eval: String, reply: Sender>) { +pub fn handle_execute_script(page: &Rc, pipeline: PipelineId, eval: String, reply: Sender>) { let page = get_page(&*page, pipeline); let window = page.window().root(); let cx = window.r().get_cx(); diff --git a/components/script_traits/lib.rs b/components/script_traits/lib.rs index 4969c2bf9cb..188c5effe28 100644 --- a/components/script_traits/lib.rs +++ b/components/script_traits/lib.rs @@ -98,7 +98,7 @@ pub enum ConstellationControlMsg { /// Set an iframe to be focused. Used when an element in an iframe gains focus. FocusIFrame(PipelineId, SubpageId), // Passes a webdriver command to the script task for execution - WebDriverCommand(PipelineId, WebDriverScriptCommand), + WebDriverScriptCommand(PipelineId, WebDriverScriptCommand), /// Notifies script task that all animations are done TickAllAnimations(PipelineId), /// Notifies script that a stylesheet has finished loading. diff --git a/components/webdriver_server/lib.rs b/components/webdriver_server/lib.rs index f1711aff839..282e707cb89 100644 --- a/components/webdriver_server/lib.rs +++ b/components/webdriver_server/lib.rs @@ -19,7 +19,7 @@ extern crate rustc_serialize; extern crate uuid; extern crate webdriver_traits; -use msg::constellation_msg::{ConstellationChan, LoadData, PipelineId, NavigationDirection}; +use msg::constellation_msg::{ConstellationChan, LoadData, PipelineId, NavigationDirection, WebDriverCommandMsg}; use msg::constellation_msg::Msg as ConstellationMsg; use std::sync::mpsc::channel; use webdriver_traits::WebDriverScriptCommand; @@ -111,10 +111,16 @@ impl Handler { let pipeline_id = self.get_root_pipeline(); + let (sender, reciever) = channel(); + let load_data = LoadData::new(url); let ConstellationChan(ref const_chan) = self.constellation_chan; - const_chan.send(ConstellationMsg::LoadUrl(pipeline_id, load_data)).unwrap(); - //TODO: Now we ought to wait until we get a load event + let cmd_msg = WebDriverCommandMsg::LoadUrl(pipeline_id, load_data, sender); + const_chan.send(ConstellationMsg::WebDriverCommand(cmd_msg)).unwrap(); + + //Wait to get a load event + reciever.recv().unwrap(); + Ok(WebDriverResponse::Void) } @@ -135,8 +141,9 @@ impl Handler { let (sender, reciever) = channel(); let ConstellationChan(ref const_chan) = self.constellation_chan; - const_chan.send(ConstellationMsg::WebDriverCommand(pipeline_id, - WebDriverScriptCommand::GetTitle(sender))).unwrap(); + let cmd_msg = WebDriverCommandMsg::ScriptCommand(pipeline_id, + WebDriverScriptCommand::GetTitle(sender)); + const_chan.send(ConstellationMsg::WebDriverCommand(cmd_msg)).unwrap(); let value = reciever.recv().unwrap(); Ok(WebDriverResponse::Generic(ValueResponse::new(value.to_json()))) } @@ -166,7 +173,8 @@ impl Handler { let (sender, reciever) = channel(); let ConstellationChan(ref const_chan) = self.constellation_chan; let cmd = WebDriverScriptCommand::FindElementCSS(parameters.value.clone(), sender); - const_chan.send(ConstellationMsg::WebDriverCommand(pipeline_id, cmd)).unwrap(); + let cmd_msg = WebDriverCommandMsg::ScriptCommand(pipeline_id, cmd); + const_chan.send(ConstellationMsg::WebDriverCommand(cmd_msg)).unwrap(); match reciever.recv().unwrap() { Ok(value) => { Ok(WebDriverResponse::Generic(ValueResponse::new(value.map(|x| WebElement::new(x).to_json()).to_json()))) @@ -187,7 +195,8 @@ impl Handler { let (sender, reciever) = channel(); let ConstellationChan(ref const_chan) = self.constellation_chan; let cmd = WebDriverScriptCommand::FindElementsCSS(parameters.value.clone(), sender); - const_chan.send(ConstellationMsg::WebDriverCommand(pipeline_id, cmd)).unwrap(); + let cmd_msg = WebDriverCommandMsg::ScriptCommand(pipeline_id, cmd); + const_chan.send(ConstellationMsg::WebDriverCommand(cmd_msg)).unwrap(); match reciever.recv().unwrap() { Ok(value) => { let resp_value: Vec = value.into_iter().map( @@ -205,7 +214,8 @@ impl Handler { let (sender, reciever) = channel(); let ConstellationChan(ref const_chan) = self.constellation_chan; let cmd = WebDriverScriptCommand::GetElementText(element.id.clone(), sender); - const_chan.send(ConstellationMsg::WebDriverCommand(pipeline_id, cmd)).unwrap(); + let cmd_msg = WebDriverCommandMsg::ScriptCommand(pipeline_id, cmd); + const_chan.send(ConstellationMsg::WebDriverCommand(cmd_msg)).unwrap(); match reciever.recv().unwrap() { Ok(value) => Ok(WebDriverResponse::Generic(ValueResponse::new(value.to_json()))), Err(_) => Err(WebDriverError::new(ErrorStatus::StaleElementReference, @@ -219,7 +229,8 @@ impl Handler { let (sender, reciever) = channel(); let ConstellationChan(ref const_chan) = self.constellation_chan; let cmd = WebDriverScriptCommand::GetActiveElement(sender); - const_chan.send(ConstellationMsg::WebDriverCommand(pipeline_id, cmd)).unwrap(); + let cmd_msg = WebDriverCommandMsg::ScriptCommand(pipeline_id, cmd); + const_chan.send(ConstellationMsg::WebDriverCommand(cmd_msg)).unwrap(); let value = reciever.recv().unwrap().map(|x| WebElement::new(x).to_json()); Ok(WebDriverResponse::Generic(ValueResponse::new(value.to_json()))) } @@ -230,7 +241,8 @@ impl Handler { let (sender, reciever) = channel(); let ConstellationChan(ref const_chan) = self.constellation_chan; let cmd = WebDriverScriptCommand::GetElementTagName(element.id.clone(), sender); - const_chan.send(ConstellationMsg::WebDriverCommand(pipeline_id, cmd)).unwrap(); + let cmd_msg = WebDriverCommandMsg::ScriptCommand(pipeline_id, cmd); + const_chan.send(ConstellationMsg::WebDriverCommand(cmd_msg)).unwrap(); match reciever.recv().unwrap() { Ok(value) => Ok(WebDriverResponse::Generic(ValueResponse::new(value.to_json()))), Err(_) => Err(WebDriverError::new(ErrorStatus::StaleElementReference, @@ -253,8 +265,9 @@ impl Handler { let (sender, reciever) = channel(); let ConstellationChan(ref const_chan) = self.constellation_chan; - const_chan.send(ConstellationMsg::WebDriverCommand(pipeline_id, - WebDriverScriptCommand::EvaluateJS(script, sender))).unwrap(); + let cmd = WebDriverScriptCommand::ExecuteScript(script, sender); + let cmd_msg = WebDriverCommandMsg::ScriptCommand(pipeline_id, cmd); + const_chan.send(ConstellationMsg::WebDriverCommand(cmd_msg)).unwrap(); match reciever.recv().unwrap() { Ok(value) => Ok(WebDriverResponse::Generic(ValueResponse::new(value.to_json()))), @@ -273,7 +286,8 @@ impl Handler { for _ in 0..iterations { let (sender, reciever) = channel(); let ConstellationChan(ref const_chan) = self.constellation_chan; - const_chan.send(ConstellationMsg::CompositePng(sender)).unwrap(); + let cmd_msg = WebDriverCommandMsg::TakeScreenshot(sender); + const_chan.send(ConstellationMsg::WebDriverCommand(cmd_msg)).unwrap(); if let Some(x) = reciever.recv().unwrap() { img = Some(x); diff --git a/components/webdriver_traits/lib.rs b/components/webdriver_traits/lib.rs index ecc0b647501..162311de75a 100644 --- a/components/webdriver_traits/lib.rs +++ b/components/webdriver_traits/lib.rs @@ -11,7 +11,7 @@ use rustc_serialize::json::{Json, ToJson}; use std::sync::mpsc::Sender; pub enum WebDriverScriptCommand { - EvaluateJS(String, Sender>), + ExecuteScript(String, Sender>), FindElementCSS(String, Sender, ()>>), FindElementsCSS(String, Sender, ()>>), GetActiveElement(Sender>), @@ -40,3 +40,5 @@ impl ToJson for EvaluateJSReply { } } } + +pub struct LoadComplete; From 98cb65ca0a7d6c940e69f77275b6941499a434a3 Mon Sep 17 00:00:00 2001 From: James Graham Date: Wed, 29 Apr 2015 19:47:38 +0100 Subject: [PATCH 2/4] Wait for the root pipeline to become ready before running webdriver commands. --- components/webdriver_server/lib.rs | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/components/webdriver_server/lib.rs b/components/webdriver_server/lib.rs index 282e707cb89..818bd48fb97 100644 --- a/components/webdriver_server/lib.rs +++ b/components/webdriver_server/lib.rs @@ -76,12 +76,25 @@ impl Handler { } } - fn get_root_pipeline(&self) -> PipelineId { - let (sender, reciever) = channel(); - let ConstellationChan(ref const_chan) = self.constellation_chan; - const_chan.send(ConstellationMsg::GetRootPipeline(sender)).unwrap(); + fn get_root_pipeline(&self) -> WebDriverResult { + let interval = Duration::milliseconds(20); + let iterations = 30_000 / interval.num_milliseconds(); - reciever.recv().unwrap().unwrap() + for _ in 0..iterations { + let (sender, reciever) = channel(); + let ConstellationChan(ref const_chan) = self.constellation_chan; + const_chan.send(ConstellationMsg::GetRootPipeline(sender)).unwrap(); + + + if let Some(x) = reciever.recv().unwrap() { + return Ok(x); + }; + + sleep(interval) + }; + + Err(WebDriverError::new(ErrorStatus::Timeout, + "Failed to get root window handle")) } fn handle_new_session(&mut self) -> WebDriverResult { @@ -109,7 +122,7 @@ impl Handler { "Invalid URL")) }; - let pipeline_id = self.get_root_pipeline(); + let pipeline_id = try!(self.get_root_pipeline()); let (sender, reciever) = channel(); From 8d10fa1f2d2d26cfb5ca5bcbee0bd6d0f77b730a Mon Sep 17 00:00:00 2001 From: James Graham Date: Wed, 29 Apr 2015 19:49:38 +0100 Subject: [PATCH 3/4] Add basic support for executeAsyncScript. This relies on a global webdriverCallback function, which is visible to content. Obviously that's not a long term solution for a number of reasons, but it allows us to experiment for now --- components/script/dom/webidls/Window.webidl | 3 ++ components/script/dom/window.rs | 22 +++++++++ components/script/script_task.rs | 6 ++- components/script/webdriver_handlers.rs | 44 +++++++++++------ components/webdriver_server/lib.rs | 55 ++++++++++++++------- components/webdriver_traits/lib.rs | 1 + 6 files changed, 96 insertions(+), 35 deletions(-) diff --git a/components/script/dom/webidls/Window.webidl b/components/script/dom/webidls/Window.webidl index 08caf90004e..2af5786b061 100644 --- a/components/script/dom/webidls/Window.webidl +++ b/components/script/dom/webidls/Window.webidl @@ -56,6 +56,9 @@ //void postMessage(any message, DOMString targetOrigin, optional sequence transfer); + // Shouldn't be public, but just to make things work for now + void webdriverCallback(optional any result); + // also has obsolete members }; Window implements GlobalEventHandlers; diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index dd4a7ea67c1..1cf2de74691 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -35,8 +35,10 @@ use script_task::{TimerSource, ScriptChan, ScriptPort, NonWorkerScriptChan}; use script_task::ScriptMsg; use script_traits::ScriptControlChan; use timers::{IsInterval, TimerId, TimerManager, TimerCallback}; +use webdriver_handlers::jsval_to_webdriver; use devtools_traits::{DevtoolsControlChan, TimelineMarker, TimelineMarkerType, TracingMetadata}; +use webdriver_traits::EvaluateJSReply; use msg::compositor_msg::ScriptListener; use msg::constellation_msg::{LoadData, PipelineId, SubpageId, ConstellationChan, WindowSizeData, WorkerId}; use net_traits::ResourceTask; @@ -166,6 +168,9 @@ pub struct Window { /// A counter of the number of pending reflows for this window. pending_reflow_count: Cell, + + /// A channel for communicating results of async scripts back to the webdriver server + webdriver_script_chan: RefCell>>> } impl Window { @@ -483,6 +488,17 @@ impl<'a> WindowMethods for JSRef<'a, Window> { let doc = self.Document().root(); doc.r().cancel_animation_frame(ident); } + + fn WebdriverCallback(self, cx: *mut JSContext, val: JSVal) { + let rv = jsval_to_webdriver(cx, val); + { + let opt_chan = self.webdriver_script_chan.borrow(); + if let Some(ref chan) = *opt_chan { + chan.send(rv).unwrap(); + } + } + self.set_webdriver_script_chan(None); + } } pub trait WindowHelpers { @@ -523,6 +539,7 @@ pub trait WindowHelpers { fn emit_timeline_marker(self, marker: TimelineMarker); fn set_devtools_timeline_marker(self, marker: TimelineMarkerType, reply: Sender); fn drop_devtools_timeline_markers(self); + fn set_webdriver_script_chan(self, chan: Option>>); } pub trait ScriptHelpers { @@ -880,6 +897,10 @@ impl<'a> WindowHelpers for JSRef<'a, Window> { self.devtools_markers.borrow_mut().clear(); *self.devtools_marker_sender.borrow_mut() = None; } + + fn set_webdriver_script_chan(self, chan: Option>>) { + *self.webdriver_script_chan.borrow_mut() = chan; + } } impl Window { @@ -947,6 +968,7 @@ impl Window { devtools_marker_sender: RefCell::new(None), devtools_markers: RefCell::new(HashSet::new()), devtools_wants_updates: Cell::new(false), + webdriver_script_chan: RefCell::new(None), }; WindowBinding::Wrap(runtime.cx(), win) diff --git a/components/script/script_task.rs b/components/script/script_task.rs index df838f130e8..b114819c613 100644 --- a/components/script/script_task.rs +++ b/components/script/script_task.rs @@ -316,7 +316,7 @@ pub struct ScriptTask { /// The JavaScript runtime. js_runtime: Rc, - mouse_over_targets: DOMRefCell>> + mouse_over_targets: DOMRefCell>>, } /// In the event of task failure, all data on the stack runs its destructor. However, there @@ -814,7 +814,9 @@ impl ScriptTask { WebDriverScriptCommand::GetElementText(node_id, reply) => webdriver_handlers::handle_get_text(&page, pipeline_id, node_id, reply), WebDriverScriptCommand::GetTitle(reply) => - webdriver_handlers::handle_get_title(&page, pipeline_id, reply) + webdriver_handlers::handle_get_title(&page, pipeline_id, reply), + WebDriverScriptCommand::ExecuteAsyncScript(script, reply) => + webdriver_handlers::handle_execute_async_script(&page, pipeline_id, script, reply), } } diff --git a/components/script/webdriver_handlers.rs b/components/script/webdriver_handlers.rs index cdaea5f5fb3..5ab1aa50a21 100644 --- a/components/script/webdriver_handlers.rs +++ b/components/script/webdriver_handlers.rs @@ -12,8 +12,10 @@ use dom::bindings::codegen::Bindings::NodeBinding::NodeMethods; use dom::bindings::codegen::Bindings::NodeListBinding::NodeListMethods; use dom::bindings::js::{OptionalRootable, Rootable, Temporary}; use dom::node::{Node, NodeHelpers}; -use dom::window::ScriptHelpers; +use dom::window::{ScriptHelpers, WindowHelpers}; use dom::document::DocumentHelpers; +use js::jsapi::JSContext; +use js::jsval::JSVal; use page::Page; use msg::constellation_msg::PipelineId; use script_task::get_page; @@ -35,26 +37,38 @@ fn find_node_by_unique_id(page: &Rc, pipeline: PipelineId, node_id: String None } +pub fn jsval_to_webdriver(cx: *mut JSContext, val: JSVal) -> Result { + if val.is_undefined() { + Ok(EvaluateJSReply::VoidValue) + } else if val.is_boolean() { + Ok(EvaluateJSReply::BooleanValue(val.to_boolean())) + } else if val.is_double() { + Ok(EvaluateJSReply::NumberValue(FromJSValConvertible::from_jsval(cx, val, ()).unwrap())) + } else if val.is_string() { + //FIXME: use jsstring_to_str when jsval grows to_jsstring + Ok(EvaluateJSReply::StringValue(FromJSValConvertible::from_jsval(cx, val, StringificationBehavior::Default).unwrap())) + } else if val.is_null() { + Ok(EvaluateJSReply::NullValue) + } else { + Err(()) + } +} + pub fn handle_execute_script(page: &Rc, pipeline: PipelineId, eval: String, reply: Sender>) { let page = get_page(&*page, pipeline); let window = page.window().root(); let cx = window.r().get_cx(); let rval = window.r().evaluate_js_on_global_with_result(&eval); - reply.send(if rval.is_undefined() { - Ok(EvaluateJSReply::VoidValue) - } else if rval.is_boolean() { - Ok(EvaluateJSReply::BooleanValue(rval.to_boolean())) - } else if rval.is_double() { - Ok(EvaluateJSReply::NumberValue(FromJSValConvertible::from_jsval(cx, rval, ()).unwrap())) - } else if rval.is_string() { - //FIXME: use jsstring_to_str when jsval grows to_jsstring - Ok(EvaluateJSReply::StringValue(FromJSValConvertible::from_jsval(cx, rval, StringificationBehavior::Default).unwrap())) - } else if rval.is_null() { - Ok(EvaluateJSReply::NullValue) - } else { - Err(()) - }).unwrap(); + reply.send(jsval_to_webdriver(cx, rval)).unwrap(); +} + + +pub fn handle_execute_async_script(page: &Rc, pipeline: PipelineId, eval: String, reply: Sender>) { + let page = get_page(&*page, pipeline); + let window = page.window().root(); + window.r().set_webdriver_script_chan(Some(reply)); + window.r().evaluate_js_on_global_with_result(&eval); } pub fn handle_find_element_css(page: &Rc, _pipeline: PipelineId, selector: String, reply: Sender, ()>>) { diff --git a/components/webdriver_server/lib.rs b/components/webdriver_server/lib.rs index 818bd48fb97..57790548e27 100644 --- a/components/webdriver_server/lib.rs +++ b/components/webdriver_server/lib.rs @@ -21,8 +21,8 @@ extern crate webdriver_traits; use msg::constellation_msg::{ConstellationChan, LoadData, PipelineId, NavigationDirection, WebDriverCommandMsg}; use msg::constellation_msg::Msg as ConstellationMsg; -use std::sync::mpsc::channel; -use webdriver_traits::WebDriverScriptCommand; +use std::sync::mpsc::{channel, Receiver}; +use webdriver_traits::{WebDriverScriptCommand, EvaluateJSReply}; use url::Url; use webdriver::command::{WebDriverMessage, WebDriverCommand}; @@ -77,8 +77,8 @@ impl Handler { } fn get_root_pipeline(&self) -> WebDriverResult { - let interval = Duration::milliseconds(20); - let iterations = 30_000 / interval.num_milliseconds(); + let interval = 20; + let iterations = 30_000 / interval; for _ in 0..iterations { let (sender, reciever) = channel(); @@ -90,7 +90,7 @@ impl Handler { return Ok(x); }; - sleep(interval) + sleep_ms(interval) }; Err(WebDriverError::new(ErrorStatus::Timeout, @@ -150,7 +150,7 @@ impl Handler { } fn handle_get_title(&self) -> WebDriverResult { - let pipeline_id = self.get_root_pipeline(); + let pipeline_id = try!(self.get_root_pipeline()); let (sender, reciever) = channel(); let ConstellationChan(ref const_chan) = self.constellation_chan; @@ -176,7 +176,7 @@ impl Handler { } fn handle_find_element(&self, parameters: &LocatorParameters) -> WebDriverResult { - let pipeline_id = self.get_root_pipeline(); + let pipeline_id = try!(self.get_root_pipeline()); if parameters.using != LocatorStrategy::CSSSelector { return Err(WebDriverError::new(ErrorStatus::UnsupportedOperation, @@ -198,7 +198,7 @@ impl Handler { } fn handle_find_elements(&self, parameters: &LocatorParameters) -> WebDriverResult { - let pipeline_id = self.get_root_pipeline(); + let pipeline_id = try!(self.get_root_pipeline()); if parameters.using != LocatorStrategy::CSSSelector { return Err(WebDriverError::new(ErrorStatus::UnsupportedOperation, @@ -222,7 +222,7 @@ impl Handler { } fn handle_get_element_text(&self, element: &WebElement) -> WebDriverResult { - let pipeline_id = self.get_root_pipeline(); + let pipeline_id = try!(self.get_root_pipeline()); let (sender, reciever) = channel(); let ConstellationChan(ref const_chan) = self.constellation_chan; @@ -237,7 +237,7 @@ impl Handler { } fn handle_get_active_element(&self) -> WebDriverResult { - let pipeline_id = self.get_root_pipeline(); + let pipeline_id = try!(self.get_root_pipeline()); let (sender, reciever) = channel(); let ConstellationChan(ref const_chan) = self.constellation_chan; @@ -249,7 +249,7 @@ impl Handler { } fn handle_get_element_tag_name(&self, element: &WebElement) -> WebDriverResult { - let pipeline_id = self.get_root_pipeline(); + let pipeline_id = try!(self.get_root_pipeline()); let (sender, reciever) = channel(); let ConstellationChan(ref const_chan) = self.constellation_chan; @@ -263,11 +263,7 @@ impl Handler { } } - fn handle_execute_script(&self, parameters: &JavascriptCommandParameters) -> WebDriverResult { - // TODO: This isn't really right because it always runs the script in the - // root window - let pipeline_id = self.get_root_pipeline(); - + fn handle_execute_script(&self, parameters: &JavascriptCommandParameters) -> WebDriverResult { let func_body = ¶meters.script; let args_string = ""; @@ -277,9 +273,31 @@ impl Handler { let script = format!("(function() {{ {} }})({})", func_body, args_string); let (sender, reciever) = channel(); + let command = WebDriverScriptCommand::ExecuteScript(script, sender); + self.execute_script(command, reciever) + } + + fn handle_execute_async_script(&self, parameters: &JavascriptCommandParameters) -> WebDriverResult { + let func_body = ¶meters.script; + let args_string = "window.webdriverCallback"; + + // This is pretty ugly; we really want something that acts like + // new Function() and then takes the resulting function and executes + // it with a vec of arguments. + let script = format!("(function(callback) {{ {} }})({})", func_body, args_string); + + let (sender, reciever) = channel(); + let command = WebDriverScriptCommand::ExecuteAsyncScript(script, sender); + self.execute_script(command, reciever) + } + + fn execute_script(&self, command: WebDriverScriptCommand, reciever: Receiver>) -> WebDriverResult { + // TODO: This isn't really right because it always runs the script in the + // root window + let pipeline_id = try!(self.get_root_pipeline()); + let ConstellationChan(ref const_chan) = self.constellation_chan; - let cmd = WebDriverScriptCommand::ExecuteScript(script, sender); - let cmd_msg = WebDriverCommandMsg::ScriptCommand(pipeline_id, cmd); + let cmd_msg = WebDriverCommandMsg::ScriptCommand(pipeline_id, command); const_chan.send(ConstellationMsg::WebDriverCommand(cmd_msg)).unwrap(); match reciever.recv().unwrap() { @@ -348,6 +366,7 @@ impl WebDriverHandler for Handler { WebDriverCommand::GetElementText(ref element) => self.handle_get_element_text(element), WebDriverCommand::GetElementTagName(ref element) => self.handle_get_element_tag_name(element), WebDriverCommand::ExecuteScript(ref x) => self.handle_execute_script(x), + WebDriverCommand::ExecuteAsyncScript(ref x) => self.handle_execute_async_script(x), WebDriverCommand::TakeScreenshot => self.handle_take_screenshot(), _ => Err(WebDriverError::new(ErrorStatus::UnsupportedOperation, "Command not implemented")) diff --git a/components/webdriver_traits/lib.rs b/components/webdriver_traits/lib.rs index 162311de75a..db890f6d9d4 100644 --- a/components/webdriver_traits/lib.rs +++ b/components/webdriver_traits/lib.rs @@ -12,6 +12,7 @@ use std::sync::mpsc::Sender; pub enum WebDriverScriptCommand { ExecuteScript(String, Sender>), + ExecuteAsyncScript(String, Sender>), FindElementCSS(String, Sender, ()>>), FindElementsCSS(String, Sender, ()>>), GetActiveElement(Sender>), From 50f59c82550b6878f78d07ddad5c022febf1f017 Mon Sep 17 00:00:00 2001 From: James Graham Date: Thu, 30 Apr 2015 16:26:04 +0100 Subject: [PATCH 4/4] Add support for timing out scripts --- components/script/dom/webidls/Window.webidl | 10 +++-- components/script/dom/window.rs | 24 +++++++----- components/script/webdriver_handlers.rs | 21 +++++------ components/webdriver_server/lib.rs | 41 +++++++++++++++------ components/webdriver_traits/lib.rs | 35 +++++++++++------- 5 files changed, 82 insertions(+), 49 deletions(-) diff --git a/components/script/dom/webidls/Window.webidl b/components/script/dom/webidls/Window.webidl index 2af5786b061..ae8758e3c56 100644 --- a/components/script/dom/webidls/Window.webidl +++ b/components/script/dom/webidls/Window.webidl @@ -56,9 +56,6 @@ //void postMessage(any message, DOMString targetOrigin, optional sequence transfer); - // Shouldn't be public, but just to make things work for now - void webdriverCallback(optional any result); - // also has obsolete members }; Window implements GlobalEventHandlers; @@ -131,6 +128,13 @@ partial interface Window { }; Window implements OnErrorEventHandlerForWindow; +// WebDriver extensions +partial interface Window { + // Shouldn't be public, but just to make things work for now + void webdriverCallback(optional any result); + void webdriverTimeout(); +}; + // https://html.spec.whatwg.org/multipage/#dom-sessionstorage [NoInterfaceObject] interface WindowSessionStorage { diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index 1cf2de74691..2621a9498a3 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -38,7 +38,7 @@ use timers::{IsInterval, TimerId, TimerManager, TimerCallback}; use webdriver_handlers::jsval_to_webdriver; use devtools_traits::{DevtoolsControlChan, TimelineMarker, TimelineMarkerType, TracingMetadata}; -use webdriver_traits::EvaluateJSReply; +use webdriver_traits::{WebDriverJSError, WebDriverJSResult}; use msg::compositor_msg::ScriptListener; use msg::constellation_msg::{LoadData, PipelineId, SubpageId, ConstellationChan, WindowSizeData, WorkerId}; use net_traits::ResourceTask; @@ -170,7 +170,7 @@ pub struct Window { pending_reflow_count: Cell, /// A channel for communicating results of async scripts back to the webdriver server - webdriver_script_chan: RefCell>>> + webdriver_script_chan: RefCell>> } impl Window { @@ -491,13 +491,17 @@ impl<'a> WindowMethods for JSRef<'a, Window> { fn WebdriverCallback(self, cx: *mut JSContext, val: JSVal) { let rv = jsval_to_webdriver(cx, val); - { - let opt_chan = self.webdriver_script_chan.borrow(); - if let Some(ref chan) = *opt_chan { - chan.send(rv).unwrap(); - } + let opt_chan = self.webdriver_script_chan.borrow_mut().take(); + if let Some(chan) = opt_chan { + chan.send(rv).unwrap(); + } + } + + fn WebdriverTimeout(self) { + let opt_chan = self.webdriver_script_chan.borrow_mut().take(); + if let Some(chan) = opt_chan { + chan.send(Err(WebDriverJSError::Timeout)).unwrap(); } - self.set_webdriver_script_chan(None); } } @@ -539,7 +543,7 @@ pub trait WindowHelpers { fn emit_timeline_marker(self, marker: TimelineMarker); fn set_devtools_timeline_marker(self, marker: TimelineMarkerType, reply: Sender); fn drop_devtools_timeline_markers(self); - fn set_webdriver_script_chan(self, chan: Option>>); + fn set_webdriver_script_chan(self, chan: Option>); } pub trait ScriptHelpers { @@ -898,7 +902,7 @@ impl<'a> WindowHelpers for JSRef<'a, Window> { *self.devtools_marker_sender.borrow_mut() = None; } - fn set_webdriver_script_chan(self, chan: Option>>) { + fn set_webdriver_script_chan(self, chan: Option>) { *self.webdriver_script_chan.borrow_mut() = chan; } } diff --git a/components/script/webdriver_handlers.rs b/components/script/webdriver_handlers.rs index 5ab1aa50a21..dc04e2346a0 100644 --- a/components/script/webdriver_handlers.rs +++ b/components/script/webdriver_handlers.rs @@ -2,7 +2,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -use webdriver_traits::{EvaluateJSReply}; +use webdriver_traits::{WebDriverJSValue, WebDriverJSError, WebDriverJSResult}; use dom::bindings::conversions::FromJSValConvertible; use dom::bindings::conversions::StringificationBehavior; use dom::bindings::codegen::InheritTypes::{NodeCast, ElementCast}; @@ -37,24 +37,24 @@ fn find_node_by_unique_id(page: &Rc, pipeline: PipelineId, node_id: String None } -pub fn jsval_to_webdriver(cx: *mut JSContext, val: JSVal) -> Result { +pub fn jsval_to_webdriver(cx: *mut JSContext, val: JSVal) -> WebDriverJSResult { if val.is_undefined() { - Ok(EvaluateJSReply::VoidValue) + Ok(WebDriverJSValue::Undefined) } else if val.is_boolean() { - Ok(EvaluateJSReply::BooleanValue(val.to_boolean())) + Ok(WebDriverJSValue::Boolean(val.to_boolean())) } else if val.is_double() { - Ok(EvaluateJSReply::NumberValue(FromJSValConvertible::from_jsval(cx, val, ()).unwrap())) + Ok(WebDriverJSValue::Number(FromJSValConvertible::from_jsval(cx, val, ()).unwrap())) } else if val.is_string() { //FIXME: use jsstring_to_str when jsval grows to_jsstring - Ok(EvaluateJSReply::StringValue(FromJSValConvertible::from_jsval(cx, val, StringificationBehavior::Default).unwrap())) + Ok(WebDriverJSValue::String(FromJSValConvertible::from_jsval(cx, val, StringificationBehavior::Default).unwrap())) } else if val.is_null() { - Ok(EvaluateJSReply::NullValue) + Ok(WebDriverJSValue::Null) } else { - Err(()) + Err(WebDriverJSError::UnknownType) } } -pub fn handle_execute_script(page: &Rc, pipeline: PipelineId, eval: String, reply: Sender>) { +pub fn handle_execute_script(page: &Rc, pipeline: PipelineId, eval: String, reply: Sender) { let page = get_page(&*page, pipeline); let window = page.window().root(); let cx = window.r().get_cx(); @@ -63,8 +63,7 @@ pub fn handle_execute_script(page: &Rc, pipeline: PipelineId, eval: String reply.send(jsval_to_webdriver(cx, rval)).unwrap(); } - -pub fn handle_execute_async_script(page: &Rc, pipeline: PipelineId, eval: String, reply: Sender>) { +pub fn handle_execute_async_script(page: &Rc, pipeline: PipelineId, eval: String, reply: Sender) { let page = get_page(&*page, pipeline); let window = page.window().root(); window.r().set_webdriver_script_chan(Some(reply)); diff --git a/components/webdriver_server/lib.rs b/components/webdriver_server/lib.rs index 57790548e27..df32cd1e4ea 100644 --- a/components/webdriver_server/lib.rs +++ b/components/webdriver_server/lib.rs @@ -22,11 +22,11 @@ extern crate webdriver_traits; use msg::constellation_msg::{ConstellationChan, LoadData, PipelineId, NavigationDirection, WebDriverCommandMsg}; use msg::constellation_msg::Msg as ConstellationMsg; use std::sync::mpsc::{channel, Receiver}; -use webdriver_traits::{WebDriverScriptCommand, EvaluateJSReply}; +use webdriver_traits::{WebDriverScriptCommand, WebDriverJSError, WebDriverJSResult}; use url::Url; use webdriver::command::{WebDriverMessage, WebDriverCommand}; -use webdriver::command::{GetParameters, JavascriptCommandParameters, LocatorParameters}; +use webdriver::command::{GetParameters, JavascriptCommandParameters, LocatorParameters, TimeoutsParameters}; use webdriver::common::{LocatorStrategy, WebElement}; use webdriver::response::{ WebDriverResponse, NewSessionResponse, ValueResponse}; @@ -58,6 +58,9 @@ struct WebdriverSession { struct Handler { session: Option, constellation_chan: ConstellationChan, + script_timeout: u32, + load_timeout: u32, + implicit_wait_timeout: u32 } impl WebdriverSession { @@ -72,7 +75,10 @@ impl Handler { pub fn new(constellation_chan: ConstellationChan) -> Handler { Handler { session: None, - constellation_chan: constellation_chan + constellation_chan: constellation_chan, + script_timeout: 30_000, + load_timeout: 300_000, + implicit_wait_timeout: 0 } } @@ -263,6 +269,19 @@ impl Handler { } } + fn handle_set_timeouts(&mut self, parameters: &TimeoutsParameters) -> WebDriverResult { + //TODO: this conversion is crazy, spec should limit these to u32 and check upstream + let value = parameters.ms as u32; + match ¶meters.type_[..] { + "implicit" => self.implicit_wait_timeout = value, + "page load" => self.load_timeout = value, + "script" => self.script_timeout = value, + x @ _ => return Err(WebDriverError::new(ErrorStatus::InvalidSelector, + &format!("Unknown timeout type {}", x))) + } + Ok(WebDriverResponse::Void) + } + fn handle_execute_script(&self, parameters: &JavascriptCommandParameters) -> WebDriverResult { let func_body = ¶meters.script; let args_string = ""; @@ -281,17 +300,16 @@ impl Handler { let func_body = ¶meters.script; let args_string = "window.webdriverCallback"; - // This is pretty ugly; we really want something that acts like - // new Function() and then takes the resulting function and executes - // it with a vec of arguments. - let script = format!("(function(callback) {{ {} }})({})", func_body, args_string); + let script = format!( + "setTimeout(webdriverTimeout, {}); (function(callback) {{ {} }})({})", + self.script_timeout, func_body, args_string); let (sender, reciever) = channel(); let command = WebDriverScriptCommand::ExecuteAsyncScript(script, sender); self.execute_script(command, reciever) } - fn execute_script(&self, command: WebDriverScriptCommand, reciever: Receiver>) -> WebDriverResult { + fn execute_script(&self, command: WebDriverScriptCommand, reciever: Receiver) -> WebDriverResult { // TODO: This isn't really right because it always runs the script in the // root window let pipeline_id = try!(self.get_root_pipeline()); @@ -302,12 +320,12 @@ impl Handler { match reciever.recv().unwrap() { Ok(value) => Ok(WebDriverResponse::Generic(ValueResponse::new(value.to_json()))), - Err(_) => Err(WebDriverError::new(ErrorStatus::UnsupportedOperation, - "Unsupported return type")) + Err(WebDriverJSError::Timeout) => Err(WebDriverError::new(ErrorStatus::Timeout, "")), + Err(WebDriverJSError::UnknownType) => Err(WebDriverError::new( + ErrorStatus::UnsupportedOperation, "Unsupported return type")) } } - fn handle_take_screenshot(&self) -> WebDriverResult { let mut img = None; @@ -367,6 +385,7 @@ impl WebDriverHandler for Handler { WebDriverCommand::GetElementTagName(ref element) => self.handle_get_element_tag_name(element), WebDriverCommand::ExecuteScript(ref x) => self.handle_execute_script(x), WebDriverCommand::ExecuteAsyncScript(ref x) => self.handle_execute_async_script(x), + WebDriverCommand::SetTimeouts(ref x) => self.handle_set_timeouts(x), WebDriverCommand::TakeScreenshot => self.handle_take_screenshot(), _ => Err(WebDriverError::new(ErrorStatus::UnsupportedOperation, "Command not implemented")) diff --git a/components/webdriver_traits/lib.rs b/components/webdriver_traits/lib.rs index db890f6d9d4..9279e8e2dd9 100644 --- a/components/webdriver_traits/lib.rs +++ b/components/webdriver_traits/lib.rs @@ -11,8 +11,8 @@ use rustc_serialize::json::{Json, ToJson}; use std::sync::mpsc::Sender; pub enum WebDriverScriptCommand { - ExecuteScript(String, Sender>), - ExecuteAsyncScript(String, Sender>), + ExecuteScript(String, Sender), + ExecuteAsyncScript(String, Sender), FindElementCSS(String, Sender, ()>>), FindElementsCSS(String, Sender, ()>>), GetActiveElement(Sender>), @@ -21,23 +21,30 @@ pub enum WebDriverScriptCommand { GetTitle(Sender) } -pub enum EvaluateJSReply { - VoidValue, - NullValue, - BooleanValue(bool), - NumberValue(f64), - StringValue(String), +pub enum WebDriverJSValue { + Undefined, + Null, + Boolean(bool), + Number(f64), + String(String), // TODO: ObjectValue and WebElementValue } -impl ToJson for EvaluateJSReply { +pub enum WebDriverJSError { + Timeout, + UnknownType +} + +pub type WebDriverJSResult = Result; + +impl ToJson for WebDriverJSValue { fn to_json(&self) -> Json { match self { - &EvaluateJSReply::VoidValue => Json::Null, - &EvaluateJSReply::NullValue => Json::Null, - &EvaluateJSReply::BooleanValue(ref x) => x.to_json(), - &EvaluateJSReply::NumberValue(ref x) => x.to_json(), - &EvaluateJSReply::StringValue(ref x) => x.to_json() + &WebDriverJSValue::Undefined => Json::Null, + &WebDriverJSValue::Null => Json::Null, + &WebDriverJSValue::Boolean(ref x) => x.to_json(), + &WebDriverJSValue::Number(ref x) => x.to_json(), + &WebDriverJSValue::String(ref x) => x.to_json() } } }