Allow running testharness/testdriver/reftests in servodriver (#34550)

* Make servodriver a thin wrapper over the base webdriver browser/executor classes.

Signed-off-by: Josh Matthews <josh@joshmatthews.net>

* Make ServoWebDriverRefTestExecutor a thin shell over the webdriver reftest executor.

Signed-off-by: Josh Matthews <josh@joshmatthews.net>

* Wait for the initial load to complete when opening a new tab via webdriver.

Signed-off-by: Josh Matthews <josh@joshmatthews.net>

* Remove assumption of a single tab from the webdriver server.

Signed-off-by: Josh Matthews <josh@joshmatthews.net>

* Serialize all keys of JS objects when converting to webdriver values.

Signed-off-by: Josh Matthews <josh@joshmatthews.net>

* Formatting.

Signed-off-by: Josh Matthews <josh@joshmatthews.net>

* Cleanup, docs, etc.

Signed-off-by: Josh Matthews <josh@joshmatthews.net>

* Use webview terminology more consistently.

Signed-off-by: Josh Matthews <josh@joshmatthews.net>

* Fix flake8 errors.

Signed-off-by: Josh Matthews <josh@joshmatthews.net>

---------

Signed-off-by: Josh Matthews <josh@joshmatthews.net>

components/constellation/constellation.rs

@@ -1461,7 +1461,11 @@ where
             // Create a new top level browsing context. Will use response_chan to return
             // the browsing context id.
             FromCompositorMsg::NewWebView(url, top_level_browsing_context_id) => {
-                self.handle_new_top_level_browsing_context(url, top_level_browsing_context_id);
+                self.handle_new_top_level_browsing_context(
+                    url,
+                    top_level_browsing_context_id,
+                    None,
+                );
             },
             // A top level browsing context is created and opened in both constellation and
             // compositor.
@@ -1485,14 +1489,7 @@ where
                 self.handle_panic(top_level_browsing_context_id, error, None);
             },
             FromCompositorMsg::FocusWebView(top_level_browsing_context_id) => {
-                if self.webviews.get(top_level_browsing_context_id).is_none() {
+                self.handle_focus_web_view(top_level_browsing_context_id);
-                    return warn!("{top_level_browsing_context_id}: FocusWebView on unknown top-level browsing context");
-                }
-                self.webviews.focus(top_level_browsing_context_id);
-                self.embedder_proxy.send((
-                    Some(top_level_browsing_context_id),
-                    EmbedderMsg::WebViewFocused(top_level_browsing_context_id),
-                ));
             },
             FromCompositorMsg::BlurWebView => {
                 self.webviews.unfocus();
@@ -2957,6 +2954,21 @@ where
         feature = "tracing",
         tracing::instrument(skip_all, fields(servo_profiling = true), level = "trace")
     )]
+    fn handle_focus_web_view(&mut self, top_level_browsing_context_id: TopLevelBrowsingContextId) {
+        if self.webviews.get(top_level_browsing_context_id).is_none() {
+            return warn!("{top_level_browsing_context_id}: FocusWebView on unknown top-level browsing context");
+        }
+        self.webviews.focus(top_level_browsing_context_id);
+        self.embedder_proxy.send((
+            Some(top_level_browsing_context_id),
+            EmbedderMsg::WebViewFocused(top_level_browsing_context_id),
+        ));
+    }
+
+    #[cfg_attr(
+        feature = "tracing",
+        tracing::instrument(skip_all, fields(servo_profiling = true))
+    )]
     fn handle_log_entry(
         &mut self,
         top_level_browsing_context_id: Option<TopLevelBrowsingContextId>,
@@ -3044,6 +3056,7 @@ where
         &mut self,
         url: ServoUrl,
         top_level_browsing_context_id: TopLevelBrowsingContextId,
+        response_sender: Option<IpcSender<webdriver_msg::LoadStatus>>,
     ) {
         let window_size = self.window_size.initial_viewport;
         let pipeline_id = PipelineId::new();
@@ -3104,6 +3117,10 @@ where
             }),
             window_size,
         });
+
+        if let Some(response_sender) = response_sender {
+            self.webdriver.load_channel = Some((pipeline_id, response_sender));
+        }
     }
 
     #[cfg_attr(
@@ -3759,7 +3776,7 @@ where
     ) {
         let mut webdriver_reset = false;
         if let Some((expected_pipeline_id, ref reply_chan)) = self.webdriver.load_channel {
-            debug!("Sending load to WebDriver");
+            debug!("Sending load for {:?} to WebDriver", expected_pipeline_id);
             if expected_pipeline_id == pipeline_id {
                 let _ = reply_chan.send(webdriver_msg::LoadStatus::LoadComplete);
                 webdriver_reset = true;
@@ -4606,6 +4623,21 @@ where
         // Find the script channel for the given parent pipeline,
         // and pass the event to that script thread.
         match msg {
+            WebDriverCommandMsg::CloseWebView(top_level_browsing_context_id) => {
+                self.handle_close_top_level_browsing_context(top_level_browsing_context_id);
+            },
+            WebDriverCommandMsg::NewWebView(sender, load_sender) => {
+                let top_level_browsing_context_id = TopLevelBrowsingContextId::new();
+                self.handle_new_top_level_browsing_context(
+                    ServoUrl::parse_with_base(None, "about:blank").expect("Infallible parse"),
+                    top_level_browsing_context_id,
+                    Some(load_sender),
+                );
+                let _ = sender.send(top_level_browsing_context_id);
+            },
+            WebDriverCommandMsg::FocusWebView(top_level_browsing_context_id) => {
+                self.handle_focus_web_view(top_level_browsing_context_id);
+            },
             WebDriverCommandMsg::GetWindowSize(_, response_sender) => {
                 let _ = response_sender.send(self.window_size);
             },
@@ -4888,13 +4920,20 @@ where
                 );
             },
         };
+
         if let Some(new_pipeline_id) = self.load_url(
             top_level_browsing_context_id,
             pipeline_id,
             load_data,
             replace,
         ) {
+            debug!(
+                "Setting up webdriver load notification for {:?}",
+                new_pipeline_id
+            );
             self.webdriver.load_channel = Some((new_pipeline_id, response_sender));
+        } else {
+            let _ = response_sender.send(webdriver_msg::LoadStatus::LoadCanceled);
         }
     }
 

components/script/webdriver_handlers.rs

@@ -11,10 +11,13 @@ use cookie::Cookie;
 use euclid::default::{Point2D, Rect, Size2D};
 use hyper_serde::Serde;
 use ipc_channel::ipc::{self, IpcSender};
-use js::jsapi::{HandleValueArray, JSAutoRealm, JSContext, JSType, JS_IsExceptionPending};
+use js::jsapi::{
+    self, GetPropertyKeys, HandleValueArray, JSAutoRealm, JSContext, JSType,
+    JS_GetOwnPropertyDescriptorById, JS_GetPropertyById, JS_IsExceptionPending, PropertyDescriptor,
+};
 use js::jsval::UndefinedValue;
 use js::rust::wrappers::{JS_CallFunctionName, JS_GetProperty, JS_HasOwnProperty, JS_TypeOfValue};
-use js::rust::{HandleObject, HandleValue};
+use js::rust::{HandleObject, HandleValue, IdVector};
 use net_traits::CookieSource::{NonHTTP, HTTP};
 use net_traits::CoreResourceMsg::{DeleteCookies, GetCookiesDataForUrl, SetCookieForUrl};
 use net_traits::IpcSend;
@@ -38,8 +41,8 @@ use crate::dom::bindings::codegen::Bindings::NodeBinding::{GetRootNodeOptions, N
 use crate::dom::bindings::codegen::Bindings::WindowBinding::WindowMethods;
 use crate::dom::bindings::codegen::Bindings::XMLSerializerBinding::XMLSerializerMethods;
 use crate::dom::bindings::conversions::{
-    get_property, get_property_jsval, is_array_like, root_from_object, ConversionBehavior,
+    get_property, get_property_jsval, is_array_like, jsid_to_string, root_from_object,
-    ConversionResult, FromJSValConvertible, StringificationBehavior,
+    ConversionBehavior, ConversionResult, FromJSValConvertible, StringificationBehavior,
 };
 use crate::dom::bindings::error::{throw_dom_exception, Error};
 use crate::dom::bindings::inheritance::Castable;
@@ -263,19 +266,50 @@ pub unsafe fn jsval_to_webdriver(
         } else {
             let mut result = HashMap::new();
 
-            let common_properties = ["x", "y", "width", "height", "key"];
+            let mut ids = IdVector::new(cx);
-            for property in common_properties.iter() {
+            if !GetPropertyKeys(
-                rooted!(in(cx) let mut item = UndefinedValue());
+                cx,
-                if get_property_jsval(cx, object.handle(), property, item.handle_mut()).is_ok() {
+                object.handle().into(),
-                    if !item.is_undefined() {
+                jsapi::JSITER_OWNONLY,
-                        if let Ok(value) = jsval_to_webdriver(cx, global_scope, item.handle()) {
+                ids.handle_mut(),
-                            result.insert(property.to_string(), value);
+            ) {
-                        }
+                return Err(WebDriverJSError::JSError);
-                    }
+            }
-                } else {
+            for id in ids.iter() {
-                    throw_dom_exception(SafeJSContext::from_ptr(cx), global_scope, Error::JSFailed);
+                rooted!(in(cx) let id = *id);
+                rooted!(in(cx) let mut desc = PropertyDescriptor::default());
+
+                let mut is_none = false;
+                if !JS_GetOwnPropertyDescriptorById(
+                    cx,
+                    object.handle().into(),
+                    id.handle().into(),
+                    desc.handle_mut().into(),
+                    &mut is_none,
+                ) {
                     return Err(WebDriverJSError::JSError);
                 }
+
+                rooted!(in(cx) let mut property = UndefinedValue());
+                if !JS_GetPropertyById(
+                    cx,
+                    object.handle().into(),
+                    id.handle().into(),
+                    property.handle_mut().into(),
+                ) {
+                    return Err(WebDriverJSError::JSError);
+                }
+                if !property.is_undefined() {
+                    let Some(name) = jsid_to_string(cx, id.handle()) else {
+                        return Err(WebDriverJSError::JSError);
+                    };
+
+                    if let Ok(value) = jsval_to_webdriver(cx, global_scope, property.handle()) {
+                        result.insert(name.into(), value);
+                    } else {
+                        return Err(WebDriverJSError::JSError);
+                    }
+                }
             }
 
             Ok(WebDriverJSValue::Object(result))

components/shared/script/lib.rs

@@ -808,6 +808,15 @@ pub enum WebDriverCommandMsg {
         Option<Rect<f32, CSSPixel>>,
         IpcSender<Option<Image>>,
     ),
+    /// Create a new webview that loads about:blank. The constellation will use
+    /// the provided channels to return the top level browsing context id
+    /// associated with the new webview, and a notification when the initial
+    /// load is complete.
+    NewWebView(IpcSender<TopLevelBrowsingContextId>, IpcSender<LoadStatus>),
+    /// Close the webview associated with the provided id.
+    CloseWebView(TopLevelBrowsingContextId),
+    /// Focus the webview associated with the provided id.
+    FocusWebView(TopLevelBrowsingContextId),
 }
 
 /// Resources required by workerglobalscopes

components/shared/script/webdriver_msg.rs

@@ -135,4 +135,5 @@ pub enum WebDriverFrameId {
 pub enum LoadStatus {
     LoadComplete,
     LoadTimeout,
+    LoadCanceled,
 }

components/webdriver_server/lib.rs

@@ -24,7 +24,7 @@ use cookie::{CookieBuilder, Expiration};
 use crossbeam_channel::{after, select, unbounded, Receiver, Sender};
 use euclid::{Rect, Size2D};
 use http::method::Method;
-use image::{DynamicImage, ImageFormat, RgbImage};
+use image::{DynamicImage, ImageFormat, RgbaImage};
 use ipc_channel::ipc::{self, IpcSender};
 use ipc_channel::router::ROUTER;
 use keyboard_types::webdriver::send_keys;
@@ -53,16 +53,16 @@ use webdriver::actions::{
 use webdriver::capabilities::{Capabilities, CapabilitiesMatching};
 use webdriver::command::{
     ActionsParameters, AddCookieParameters, GetParameters, JavascriptCommandParameters,
-    LocatorParameters, NewSessionParameters, SendKeysParameters, SwitchToFrameParameters,
+    LocatorParameters, NewSessionParameters, NewWindowParameters, SendKeysParameters,
-    SwitchToWindowParameters, TimeoutsParameters, WebDriverCommand, WebDriverExtensionCommand,
+    SwitchToFrameParameters, SwitchToWindowParameters, TimeoutsParameters, WebDriverCommand,
-    WebDriverMessage, WindowRectParameters,
+    WebDriverExtensionCommand, WebDriverMessage, WindowRectParameters,
 };
 use webdriver::common::{Cookie, Date, LocatorStrategy, Parameters, WebElement};
 use webdriver::error::{ErrorStatus, WebDriverError, WebDriverResult};
 use webdriver::httpapi::WebDriverExtensionRoute;
 use webdriver::response::{
-    CookieResponse, CookiesResponse, ElementRectResponse, NewSessionResponse, TimeoutsResponse,
+    CloseWindowResponse, CookieResponse, CookiesResponse, ElementRectResponse, NewSessionResponse,
-    ValueResponse, WebDriverResponse, WindowRectResponse,
+    NewWindowResponse, TimeoutsResponse, ValueResponse, WebDriverResponse, WindowRectResponse,
 };
 use webdriver::server::{self, Session, SessionTeardownKind, WebDriverHandler};
 
@@ -130,6 +130,8 @@ pub struct WebDriverSession {
     browsing_context_id: BrowsingContextId,
     top_level_browsing_context_id: TopLevelBrowsingContextId,
 
+    window_handles: HashMap<TopLevelBrowsingContextId, String>,
+
     /// Time to wait for injected scripts to run before interrupting them.  A [`None`] value
     /// specifies that the script should run indefinitely.
     script_timeout: Option<u64>,
@@ -158,11 +160,17 @@ impl WebDriverSession {
         browsing_context_id: BrowsingContextId,
         top_level_browsing_context_id: TopLevelBrowsingContextId,
     ) -> WebDriverSession {
+        let mut window_handles = HashMap::new();
+        let handle = Uuid::new_v4().to_string();
+        window_handles.insert(top_level_browsing_context_id, handle);
+
         WebDriverSession {
             id: Uuid::new_v4(),
             browsing_context_id,
             top_level_browsing_context_id,
 
+            window_handles,
+
             script_timeout: Some(30_000),
             load_timeout: 300_000,
             implicit_wait_timeout: 0,
@@ -418,6 +426,9 @@ impl Handler {
     }
 
     fn focus_top_level_browsing_context_id(&self) -> WebDriverResult<TopLevelBrowsingContextId> {
+        // FIXME(#34550): This is a hack for unexpected behaviour in the constellation.
+        thread::sleep(Duration::from_millis(1000));
+
         debug!("Getting focused context.");
         let interval = 20;
         let iterations = 30_000 / interval;
@@ -673,13 +684,16 @@ impl Handler {
     }
 
     fn wait_for_load(&self) -> WebDriverResult<WebDriverResponse> {
+        debug!("waiting for load");
         let timeout = self.session()?.load_timeout;
-        select! {
+        let result = select! {
             recv(self.load_status_receiver) -> _ => Ok(WebDriverResponse::Void),
             recv(after(Duration::from_millis(timeout))) -> _ => Err(
                 WebDriverError::new(ErrorStatus::Timeout, "Load timed out")
             ),
-        }
+        };
+        debug!("finished waiting for load with {:?}", result);
+        result
     }
 
     fn handle_current_url(&self) -> WebDriverResult<WebDriverResponse> {
@@ -718,6 +732,13 @@ impl Handler {
         params: &WindowRectParameters,
     ) -> WebDriverResult<WebDriverResponse> {
         let (sender, receiver) = ipc::channel().unwrap();
+
+        // We don't current allow modifying the window x/y positions, so we can just
+        // return the current window rectangle.
+        if params.width.is_none() || params.height.is_none() {
+            return self.handle_window_size();
+        }
+
         let width = params.width.unwrap_or(0);
         let height = params.height.unwrap_or(0);
         let size = Size2D::new(width as u32, height as u32);
@@ -829,20 +850,27 @@ impl Handler {
     }
 
     fn handle_window_handle(&self) -> WebDriverResult<WebDriverResponse> {
-        // For now we assume there's only one window so just use the session
+        let session = self.session.as_ref().unwrap();
-        // id as the window id
+        match session
-        let handle = self.session.as_ref().unwrap().id.to_string();
+            .window_handles
-        Ok(WebDriverResponse::Generic(ValueResponse(
+            .get(&session.top_level_browsing_context_id)
-            serde_json::to_value(handle)?,
+        {
-        )))
+            Some(handle) => Ok(WebDriverResponse::Generic(ValueResponse(
+                serde_json::to_value(handle)?,
+            ))),
+            None => Ok(WebDriverResponse::Void),
+        }
     }
 
     fn handle_window_handles(&self) -> WebDriverResult<WebDriverResponse> {
-        // For now we assume there's only one window so just use the session
+        let handles = self
-        // id as the window id
+            .session
-        let handles = vec![serde_json::to_value(
+            .as_ref()
-            self.session.as_ref().unwrap().id.to_string(),
+            .unwrap()
-        )?];
+            .window_handles
+            .values()
+            .map(serde_json::to_value)
+            .collect::<Result<Vec<_>, _>>()?;
         Ok(WebDriverResponse::Generic(ValueResponse(
             serde_json::to_value(handles)?,
         )))
@@ -891,6 +919,60 @@ impl Handler {
         }
     }
 
+    fn handle_close_window(&mut self) -> WebDriverResult<WebDriverResponse> {
+        {
+            let session = self.session_mut().unwrap();
+            session
+                .window_handles
+                .remove(&session.top_level_browsing_context_id);
+            let cmd_msg = WebDriverCommandMsg::CloseWebView(session.top_level_browsing_context_id);
+            self.constellation_chan
+                .send(ConstellationMsg::WebDriverCommand(cmd_msg))
+                .unwrap();
+        }
+
+        let top_level_browsing_context_id = self.focus_top_level_browsing_context_id()?;
+        let browsing_context_id = BrowsingContextId::from(top_level_browsing_context_id);
+        let session = self.session_mut().unwrap();
+        session.top_level_browsing_context_id = top_level_browsing_context_id;
+        session.browsing_context_id = browsing_context_id;
+
+        Ok(WebDriverResponse::CloseWindow(CloseWindowResponse(
+            session.window_handles.values().cloned().collect(),
+        )))
+    }
+
+    fn handle_new_window(
+        &mut self,
+        _parameters: &NewWindowParameters,
+    ) -> WebDriverResult<WebDriverResponse> {
+        let (sender, receiver) = ipc::channel().unwrap();
+
+        let cmd_msg = WebDriverCommandMsg::NewWebView(sender, self.load_status_sender.clone());
+        self.constellation_chan
+            .send(ConstellationMsg::WebDriverCommand(cmd_msg))
+            .unwrap();
+
+        if let Ok(new_top_level_browsing_context_id) = receiver.recv() {
+            let session = self.session_mut().unwrap();
+            session.top_level_browsing_context_id = new_top_level_browsing_context_id;
+            session.browsing_context_id =
+                BrowsingContextId::from(new_top_level_browsing_context_id);
+            let new_handle = Uuid::new_v4().to_string();
+            session
+                .window_handles
+                .insert(new_top_level_browsing_context_id, new_handle);
+        }
+
+        let _ = self.wait_for_load();
+
+        let handle = self.session.as_ref().unwrap().id.to_string();
+        Ok(WebDriverResponse::NewWindow(NewWindowResponse {
+            handle,
+            typ: "tab".to_string(),
+        }))
+    }
+
     fn handle_switch_to_frame(
         &mut self,
         parameters: &SwitchToFrameParameters,
@@ -919,9 +1001,21 @@ impl Handler {
         &mut self,
         parameters: &SwitchToWindowParameters,
     ) -> WebDriverResult<WebDriverResponse> {
-        // For now we assume there is only one window which has the current
+        let session = self.session_mut().unwrap();
-        // session's id as window id
+        if session.id.to_string() == parameters.handle {
-        if parameters.handle == self.session.as_ref().unwrap().id.to_string() {
+            // There's only one main window, so there's nothing to do here.
+            Ok(WebDriverResponse::Void)
+        } else if let Some((top_level_browsing_context_id, _)) = session
+            .window_handles
+            .iter()
+            .find(|(_k, v)| **v == parameters.handle)
+        {
+            let top_level_browsing_context_id = *top_level_browsing_context_id;
+            session.top_level_browsing_context_id = top_level_browsing_context_id;
+            session.browsing_context_id = BrowsingContextId::from(top_level_browsing_context_id);
+
+            let msg = ConstellationMsg::FocusWebView(top_level_browsing_context_id);
+            self.constellation_chan.send(msg).unwrap();
             Ok(WebDriverResponse::Void)
         } else {
             Err(WebDriverError::new(
@@ -1385,12 +1479,23 @@ impl Handler {
         parameters: &JavascriptCommandParameters,
     ) -> WebDriverResult<WebDriverResponse> {
         let func_body = &parameters.script;
-        let args_string = "";
+        let args_string: Vec<_> = parameters
+            .args
+            .as_deref()
+            .unwrap_or(&[])
+            .iter()
+            .map(webdriver_value_to_js_argument)
+            .collect();
 
         // 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() {{ {} }})({})", func_body, args_string);
+        let script = format!(
+            "(function() {{ {} }})({})",
+            func_body,
+            args_string.join(", ")
+        );
+        debug!("{}", script);
 
         let (sender, receiver) = ipc::channel().unwrap();
         let command = WebDriverScriptCommand::ExecuteScript(script, sender);
@@ -1404,7 +1509,14 @@ impl Handler {
         parameters: &JavascriptCommandParameters,
     ) -> WebDriverResult<WebDriverResponse> {
         let func_body = &parameters.script;
-        let args_string = "window.webdriverCallback";
+        let mut args_string: Vec<_> = parameters
+            .args
+            .as_deref()
+            .unwrap_or(&[])
+            .iter()
+            .map(webdriver_value_to_js_argument)
+            .collect();
+        args_string.push("window.webdriverCallback".to_string());
 
         let timeout_script = if let Some(script_timeout) = self.session()?.script_timeout {
             format!("setTimeout(webdriverTimeout, {});", script_timeout)
@@ -1412,9 +1524,12 @@ impl Handler {
             "".into()
         };
         let script = format!(
-            "{} (function(callback) {{ {} }})({})",
+            "{} (function() {{ {} }})({})",
-            timeout_script, func_body, args_string
+            timeout_script,
+            func_body,
+            args_string.join(", "),
         );
+        debug!("{}", script);
 
         let (sender, receiver) = ipc::channel().unwrap();
         let command = WebDriverScriptCommand::ExecuteAsyncScript(script, sender);
@@ -1587,16 +1702,16 @@ impl Handler {
             },
         };
 
-        // The compositor always sends RGB pixels.
+        // The compositor always sends RGBA pixels.
         assert_eq!(
             img.format,
-            PixelFormat::RGB8,
+            PixelFormat::RGBA8,
             "Unexpected screenshot pixel format"
         );
 
-        let rgb = RgbImage::from_raw(img.width, img.height, img.bytes.to_vec()).unwrap();
+        let rgb = RgbaImage::from_raw(img.width, img.height, img.bytes.to_vec()).unwrap();
         let mut png_data = Cursor::new(Vec::new());
-        DynamicImage::ImageRgb8(rgb)
+        DynamicImage::ImageRgba8(rgb)
             .write_to(&mut png_data, ImageFormat::Png)
             .unwrap();
 
@@ -1729,6 +1844,8 @@ impl WebDriverHandler<ServoExtensionRoute> for Handler {
             WebDriverCommand::GetTitle => self.handle_title(),
             WebDriverCommand::GetWindowHandle => self.handle_window_handle(),
             WebDriverCommand::GetWindowHandles => self.handle_window_handles(),
+            WebDriverCommand::NewWindow(ref parameters) => self.handle_new_window(parameters),
+            WebDriverCommand::CloseWindow => self.handle_close_window(),
             WebDriverCommand::SwitchToFrame(ref parameters) => {
                 self.handle_switch_to_frame(parameters)
             },
@@ -1794,3 +1911,26 @@ impl WebDriverHandler<ServoExtensionRoute> for Handler {
         self.session = None;
     }
 }
+
+fn webdriver_value_to_js_argument(v: &Value) -> String {
+    match v {
+        Value::String(ref s) => format!("\"{}\"", s),
+        Value::Null => "null".to_string(),
+        Value::Bool(b) => b.to_string(),
+        Value::Number(n) => n.to_string(),
+        Value::Array(list) => {
+            let elems = list
+                .iter()
+                .map(|v| webdriver_value_to_js_argument(v).to_string())
+                .collect::<Vec<_>>();
+            format!("[{}]", elems.join(", "))
+        },
+        Value::Object(map) => {
+            let elems = map
+                .iter()
+                .map(|(k, v)| format!("{}: {}", k, webdriver_value_to_js_argument(v)))
+                .collect::<Vec<_>>();
+            format!("{{{}}}", elems.join(", "))
+        },
+    }
+}

tests/wpt/meta/MANIFEST.json

@@ -499057,7 +499057,7 @@
         []
        ],
        "servodriver.py": [
-        "2cb638be1569c91a963bf2cb3824f2c07788f8cc",
+        "1e9a2f3090ef1ff826dac4ca3a88a958242ffae6",
         []
        ],
        "webkit.py": [
@@ -499123,7 +499123,7 @@
         []
        ],
        "executorservodriver.py": [
-        "5d7d55f30b551f59bc0b16aacc8641c0fc24e39c",
+        "41b8ed9ac1891edb7ec332c61923314f8b1e5f19",
         []
        ],
        "executorwebdriver.py": [
@@ -499269,10 +499269,6 @@
        "d6616739e6ed63a79e8b2f5d8aee1d5b2ced7f49",
        []
       ],
-      "testharnessreport-servodriver.js": [
-       "7819538dbb8f4a807d5db2649c2540854996c865",
-       []
-      ],
       "testharnessreport-wktr.js": [
        "b7d350a4262cb6f0d38337b17311fea7bd73eb70",
        []

tests/wpt/tests/tools/wptrunner/wptrunner/browsers/servodriver.py

@@ -1,19 +1,13 @@
 # mypy: allow-untyped-defs
 
 import os
-import subprocess
 import tempfile
 
-from mozprocess import ProcessHandler
-
 from tools.serve.serve import make_hosts_file
 
-from .base import (Browser,
+from .base import (WebDriverBrowser,
-                   ExecutorBrowser,
-                   OutputHandler,
                    require_arg,
-                   get_free_port,
+                   get_free_port)
-                   browser_command)
 from .base import get_timeout_multiplier   # noqa: F401
 from ..executors import executor_kwargs as base_executor_kwargs
 from ..executors.executorservodriver import (ServoWebDriverTestharnessExecutor,  # noqa: F401
@@ -64,8 +58,7 @@ def env_extras(**kwargs):
 
 def env_options():
     return {"server_host": "127.0.0.1",
-            "testharnessreport": "testharnessreport-servodriver.js",
+            "supports_debugger": False}
-            "supports_debugger": True}
 
 
 def update_properties():
@@ -79,107 +72,40 @@ def write_hosts_file(config):
     return hosts_path
 
 
-class ServoWebDriverBrowser(Browser):
+class ServoWebDriverBrowser(WebDriverBrowser):
     init_timeout = 300  # Large timeout for cases where we're booting an Android emulator
 
     def __init__(self, logger, binary, debug_info=None, webdriver_host="127.0.0.1",
                  server_config=None, binary_args=None,
                  user_stylesheets=None, headless=None, **kwargs):
-        Browser.__init__(self, logger)
+        hosts_path = write_hosts_file(server_config)
-        self.binary = binary
+        port = get_free_port()
-        self.binary_args = binary_args or []
-        self.webdriver_host = webdriver_host
-        self.webdriver_port = None
-        self.proc = None
-        self.debug_info = debug_info
-        self.hosts_path = write_hosts_file(server_config)
-        self.server_ports = server_config.ports if server_config else {}
-        self.command = None
-        self.user_stylesheets = user_stylesheets if user_stylesheets else []
-        self.headless = headless if headless else False
-        self.ca_certificate_path = server_config.ssl_config["ca_cert_path"]
-        self.output_handler = None
-
-    def start(self, **kwargs):
-        self.webdriver_port = get_free_port()
-
         env = os.environ.copy()
-        env["HOST_FILE"] = self.hosts_path
+        env["HOST_FILE"] = hosts_path
         env["RUST_BACKTRACE"] = "1"
-        env["EMULATOR_REVERSE_FORWARD_PORTS"] = ",".join(
-            str(port)
-            for _protocol, ports in self.server_ports.items()
-            for port in ports
-            if port
-        )
 
-        debug_args, command = browser_command(
+        args = [
-            self.binary,
+            "--hard-fail",
-            self.binary_args + [
+            "--webdriver=%s" % port,
-                "--hard-fail",
+            "about:blank",
-                "--webdriver=%s" % self.webdriver_port,
+        ]
-                "about:blank",
-            ],
-            self.debug_info
-        )
 
-        if self.headless:
+        ca_cert_path = server_config.ssl_config["ca_cert_path"]
-            command += ["--headless"]
+        if ca_cert_path:
+            args += ["--certificate-path", ca_cert_path]
+        if binary_args:
+            args += binary_args
+        if user_stylesheets:
+            for stylesheet in user_stylesheets:
+                args += ["--user-stylesheet", stylesheet]
+        if headless:
+            args += ["--headless"]
 
-        if self.ca_certificate_path:
+        WebDriverBrowser.__init__(self, env=env, logger=logger, host=webdriver_host, port=port,
-            command += ["--certificate-path", self.ca_certificate_path]
+                                  supports_pac=False, webdriver_binary=binary, webdriver_args=args,
-
+                                  binary=binary)
-        for stylesheet in self.user_stylesheets:
+        self.hosts_path = hosts_path
-            command += ["--user-stylesheet", stylesheet]
-
-        self.command = command
-
-        self.command = debug_args + self.command
-
-        if not self.debug_info or not self.debug_info.interactive:
-            self.output_handler = OutputHandler(self.logger, self.command)
-            self.proc = ProcessHandler(self.command,
-                                       processOutputLine=[self.on_output],
-                                       env=env,
-                                       storeOutput=False)
-            self.proc.run()
-            self.output_handler.after_process_start(self.proc.pid)
-            self.output_handler.start()
-        else:
-            self.proc = subprocess.Popen(self.command, env=env)
-
-        self.logger.debug("Servo Started")
-
-    def stop(self, force=False):
-        self.logger.debug("Stopping browser")
-        if self.proc is not None:
-            try:
-                self.proc.kill()
-            except OSError:
-                # This can happen on Windows if the process is already dead
-                pass
-        if self.output_handler is not None:
-            self.output_handler.after_process_stop()
-
-    @property
-    def pid(self):
-        if self.proc is None:
-            return None
-
-        try:
-            return self.proc.pid
-        except AttributeError:
-            return None
-
-    def is_alive(self):
-        return self.proc.poll() is None
 
     def cleanup(self):
-        self.stop()
+        WebDriverBrowser.cleanup(self)
         os.remove(self.hosts_path)
-
-    def executor_browser(self):
-        assert self.webdriver_port is not None
-        return ExecutorBrowser, {"webdriver_host": self.webdriver_host,
-                                 "webdriver_port": self.webdriver_port,
-                                 "init_timeout": self.init_timeout}

tests/wpt/tests/tools/wptrunner/wptrunner/executors/executorservodriver.py

@@ -1,18 +1,8 @@
 # mypy: allow-untyped-defs
 
-import json
 import os
-import socket
-import traceback
 
-from .base import (Protocol,
+from .executorwebdriver import WebDriverProtocol, WebDriverTestharnessExecutor, WebDriverRefTestExecutor
-                   RefTestExecutor,
-                   RefTestImplementation,
-                   TestharnessExecutor,
-                   TimedRunner,
-                   strip_server)
-from .protocol import BaseProtocolPart
-from ..environment import wait_for_service
 
 webdriver = None
 ServoCommandExtensions = None
@@ -64,240 +54,57 @@ def parse_pref_value(value):
         return value
 
 
-class ServoBaseProtocolPart(BaseProtocolPart):
+class ServoWebDriverProtocol(WebDriverProtocol):
-    def execute_script(self, script, asynchronous=False):
-        pass
-
-    def set_timeout(self, timeout):
-        pass
-
-    def wait(self):
-        return False
-
-    def set_window(self, handle):
-        pass
-
-    def window_handles(self):
-        return []
-
-    def load(self, url):
-        pass
-
-
-class ServoWebDriverProtocol(Protocol):
-    implements = [ServoBaseProtocolPart]
-
     def __init__(self, executor, browser, capabilities, **kwargs):
         do_delayed_imports()
-        Protocol.__init__(self, executor, browser)
+        WebDriverProtocol.__init__(self, executor, browser, capabilities, **kwargs)
-        self.capabilities = capabilities
-        self.host = browser.webdriver_host
-        self.port = browser.webdriver_port
-        self.init_timeout = browser.init_timeout
-        self.session = None
 
     def connect(self):
-        """Connect to browser via WebDriver."""
+        """Connect to browser via WebDriver and crete a WebDriver session."""
-        wait_for_service(self.logger, self.host, self.port, timeout=self.init_timeout)
+        self.logger.debug("Connecting to WebDriver on URL: %s" % self.url)
 
-        self.session = webdriver.Session(self.host, self.port, extension=ServoCommandExtensions)
+        host, port = self.url.split(":")[1].strip("/"), self.url.split(':')[-1].strip("/")
-        self.session.start()
 
-    def after_connect(self):
+        capabilities = {"alwaysMatch": self.capabilities}
-        pass
+        self.webdriver = webdriver.Session(host, port,
-
+                                           capabilities=capabilities,
-    def teardown(self):
+                                           enable_bidi=self.enable_bidi,
-        self.logger.debug("Hanging up on WebDriver session")
+                                           extension=ServoCommandExtensions)
-        try:
+        self.webdriver.start()
-            self.session.end()
-        except Exception:
-            pass
-
-    def is_alive(self):
-        try:
-            # Get a simple property over the connection
-            self.session.window_handle
-        # TODO what exception?
-        except Exception:
-            return False
-        return True
-
-    def wait(self):
-        while True:
-            try:
-                return self.session.execute_async_script("""let callback = arguments[arguments.length - 1];
-addEventListener("__test_restart", e => {e.preventDefault(); callback(true)})""")
-            except webdriver.TimeoutException:
-                pass
-            except (socket.timeout, OSError):
-                break
-            except Exception:
-                self.logger.error(traceback.format_exc())
-                break
-        return False
 
 
-class ServoWebDriverRun(TimedRunner):
+class ServoWebDriverTestharnessExecutor(WebDriverTestharnessExecutor):
-    def set_timeout(self):
-        pass
-
-    def run_func(self):
-        try:
-            self.result = True, self.func(self.protocol.session, self.url, self.timeout)
-        except webdriver.TimeoutException:
-            self.result = False, ("EXTERNAL-TIMEOUT", None)
-        except (socket.timeout, OSError):
-            self.result = False, ("CRASH", None)
-        except Exception as e:
-            message = getattr(e, "message", "")
-            if message:
-                message += "\n"
-            message += traceback.format_exc()
-            self.result = False, ("INTERNAL-ERROR", e)
-        finally:
-            self.result_flag.set()
-
-
-class ServoWebDriverTestharnessExecutor(TestharnessExecutor):
     supports_testdriver = True
+    protocol_cls = ServoWebDriverProtocol
 
     def __init__(self, logger, browser, server_config, timeout_multiplier=1,
-                 close_after_done=True, capabilities=None, debug_info=None,
+                 close_after_done=True, capabilities={}, debug_info=None,
                  **kwargs):
-        TestharnessExecutor.__init__(self, logger, browser, server_config, timeout_multiplier=1,
+        WebDriverTestharnessExecutor.__init__(self, logger, browser, server_config,
-                                     debug_info=None)
+                                              timeout_multiplier, capabilities=capabilities,
-        self.protocol = ServoWebDriverProtocol(self, browser, capabilities=capabilities)
+                                              debug_info=debug_info, close_after_done=close_after_done,
-        with open(os.path.join(here, "testharness_servodriver.js")) as f:
+                                              cleanup_after_test=False)
-            self.script = f.read()
-        self.timeout = None
-
-    def on_protocol_change(self, new_protocol):
-        pass
-
-    def is_alive(self):
-        return self.protocol.is_alive()
-
-    def do_test(self, test):
-        url = self.test_url(test)
-
-        timeout = test.timeout * self.timeout_multiplier + self.extra_timeout
-
-        if timeout != self.timeout:
-            try:
-                self.protocol.session.timeouts.script = timeout
-                self.timeout = timeout
-            except OSError:
-                msg = "Lost WebDriver connection"
-                self.logger.error(msg)
-                return ("INTERNAL-ERROR", msg)
-
-        success, data = ServoWebDriverRun(self.logger,
-                                          self.do_testharness,
-                                          self.protocol,
-                                          url,
-                                          timeout,
-                                          self.extra_timeout).run()
-
-        if success:
-            return self.convert_result(test, data)
-
-        return (test.make_result(*data), [])
-
-    def do_testharness(self, session, url, timeout):
-        session.url = url
-        result = json.loads(
-            session.execute_async_script(
-                self.script % {"abs_url": url,
-                               "url": strip_server(url),
-                               "timeout_multiplier": self.timeout_multiplier,
-                               "timeout": timeout * 1000}))
-        # Prevent leaking every page in history until Servo develops a more sane
-        # page cache
-        session.back()
-        return result
 
     def on_environment_change(self, new_environment):
-        self.protocol.session.extension.change_prefs(
+        self.protocol.webdriver.extension.change_prefs(
             self.last_environment.get("prefs", {}),
             new_environment.get("prefs", {})
         )
 
 
-class TimeoutError(Exception):
+class ServoWebDriverRefTestExecutor(WebDriverRefTestExecutor):
-    pass
+    protocol_cls = ServoWebDriverProtocol
 
-
-class ServoWebDriverRefTestExecutor(RefTestExecutor):
     def __init__(self, logger, browser, server_config, timeout_multiplier=1,
-                 screenshot_cache=None, capabilities=None, debug_info=None,
+                 screenshot_cache=None, capabilities={}, debug_info=None,
                  **kwargs):
-        """Selenium WebDriver-based executor for reftests"""
+        WebDriverRefTestExecutor.__init__(self, logger, browser, server_config,
-        RefTestExecutor.__init__(self,
+                                          timeout_multiplier, screenshot_cache,
-                                 logger,
+                                          capabilities=capabilities,
-                                 browser,
+                                          debug_info=debug_info)
-                                 server_config,
-                                 screenshot_cache=screenshot_cache,
-                                 timeout_multiplier=timeout_multiplier,
-                                 debug_info=debug_info)
-        self.protocol = ServoWebDriverProtocol(self, browser,
-                                               capabilities=capabilities)
-        self.implementation = RefTestImplementation(self)
-        self.timeout = None
-        with open(os.path.join(here, "test-wait.js")) as f:
-            self.wait_script = f.read() % {"classname": "reftest-wait"}
-
-    def reset(self):
-        self.implementation.reset()
-
-    def is_alive(self):
-        return self.protocol.is_alive()
-
-    def do_test(self, test):
-        try:
-            result = self.implementation.run_test(test)
-            return self.convert_result(test, result)
-        except OSError:
-            return test.make_result("CRASH", None), []
-        except TimeoutError:
-            return test.make_result("TIMEOUT", None), []
-        except Exception as e:
-            message = getattr(e, "message", "")
-            if message:
-                message += "\n"
-            message += traceback.format_exc()
-            return test.make_result("INTERNAL-ERROR", message), []
-
-    def screenshot(self, test, viewport_size, dpi, page_ranges):
-        # https://github.com/web-platform-tests/wpt/issues/7135
-        assert viewport_size is None
-        assert dpi is None
-
-        timeout = (test.timeout * self.timeout_multiplier + self.extra_timeout
-                   if self.debug_info is None else None)
-
-        if self.timeout != timeout:
-            try:
-                self.protocol.session.timeouts.script = timeout
-                self.timeout = timeout
-            except OSError:
-                msg = "Lost webdriver connection"
-                self.logger.error(msg)
-                return ("INTERNAL-ERROR", msg)
-
-        return ServoWebDriverRun(self.logger,
-                                 self._screenshot,
-                                 self.protocol,
-                                 self.test_url(test),
-                                 timeout,
-                                 self.extra_timeout).run()
-
-    def _screenshot(self, session, url, timeout):
-        session.url = url
-        session.execute_async_script(self.wait_script)
-        return session.screenshot()
 
     def on_environment_change(self, new_environment):
-        self.protocol.session.extension.change_prefs(
+        self.protocol.webdriver.extension.change_prefs(
             self.last_environment.get("prefs", {}),
             new_environment.get("prefs", {})
         )

tests/wpt/tests/tools/wptrunner/wptrunner/testharnessreport-servodriver.js

@@ -1,23 +0,0 @@
-setup({output:%(output)d, debug: %(debug)s});
-
-add_completion_callback(function() {
-    add_completion_callback(function (tests, status) {
-        var subtest_results = tests.map(function(x) {
-            return [x.name, x.status, x.message, x.stack]
-        });
-        var id = location.pathname + location.search + location.hash;
-        var results = JSON.stringify([id,
-                                      status.status,
-                                      status.message,
-                                      status.stack,
-                                      subtest_results]);
-        (function done() {
-            if (window.__wd_results_callback__) {
-                clearTimeout(__wd_results_timer__);
-                __wd_results_callback__(results)
-            } else {
-                setTimeout(done, 20);
-            }
-        })()
-    })
-});