webdriver: Reuse JSValue as WebDriverJSValue (#38751)

After #38748, `WebDriverJSValue` is almost same as `JSValue`. Now we turn "potentially merge into one in the future" into reality. The only thing we should be cautious is to properly serialize `WebFrame`, `WebWindow`, `WebElement` for WebDriver. Testing: No regression. Some error is fixed previously by #38709 which didn't update test :) Binary size reduced by 134KB. --------- Signed-off-by: Euclid Ye <euclid.ye@huawei.com>
2025-08-24 14:48:21 +01:00 · 2025-08-19 01:38:48 +08:00 · 2025-08-19 01:38:48 +08:00 · ec5872992b
commit ec5872992b
parent 7471ad7730
7 changed files with 57 additions and 101 deletions
--- a/components/script/script_thread.rs
+++ b/components/script/script_thread.rs
@ -3979,16 +3979,14 @@ impl ScriptThread {
            return;
        };

-        let result = match jsval_to_webdriver(
+        let result = jsval_to_webdriver(
            context,
            global_scope,
            return_value.handle(),
            (&realm).into(),
            can_gc,
-        ) {
-            Ok(ref value) => Ok(value.into()),
-            Err(_) => Err(JavaScriptEvaluationError::SerializationError),
-        };
+        )
+        .map_err(|_| JavaScriptEvaluationError::SerializationError);

        let _ = self.senders.pipeline_to_constellation_sender.send((
            pipeline_id,
--- a/components/script/webdriver_handlers.rs
+++ b/components/script/webdriver_handlers.rs
@ -9,7 +9,7 @@ use std::ptr::NonNull;
 use base::id::{BrowsingContextId, PipelineId};
 use cookie::Cookie;
 use embedder_traits::{
-    WebDriverFrameId, WebDriverJSError, WebDriverJSResult, WebDriverJSValue, WebDriverLoadStatus,
+    JSValue, WebDriverFrameId, WebDriverJSError, WebDriverJSResult, WebDriverLoadStatus,
 };
 use euclid::default::{Point2D, Rect, Size2D};
 use hyper_serde::Serde;
@ -31,7 +31,6 @@ use script_bindings::codegen::GenericBindings::ShadowRootBinding::ShadowRootMeth
 use script_bindings::conversions::is_array_like;
 use script_bindings::num::Finite;
 use servo_url::ServoUrl;
-use webdriver::common::{WebElement, WebFrame, WebWindow};
 use webdriver::error::ErrorStatus;

 use crate::document_collection::DocumentCollection;
@ -364,13 +363,13 @@ unsafe fn jsval_to_webdriver_inner(
 ) -> WebDriverJSResult {
    let _ac = enter_realm(global_scope);
    if val.get().is_undefined() {
-        Ok(WebDriverJSValue::Undefined)
+        Ok(JSValue::Undefined)
    } else if val.get().is_null() {
-        Ok(WebDriverJSValue::Null)
+        Ok(JSValue::Null)
    } else if val.get().is_boolean() {
-        Ok(WebDriverJSValue::Boolean(val.get().to_boolean()))
+        Ok(JSValue::Boolean(val.get().to_boolean()))
    } else if val.get().is_number() {
-        Ok(WebDriverJSValue::Number(
+        Ok(JSValue::Number(
            match FromJSValConvertible::from_jsval(cx, val, ()).unwrap() {
                ConversionResult::Success(c) => c,
                _ => unreachable!(),
@ -385,7 +384,7 @@ unsafe fn jsval_to_webdriver_inner(
                ConversionResult::Success(c) => c,
                _ => unreachable!(),
            };
-        Ok(WebDriverJSValue::String(String::from(string)))
+        Ok(JSValue::String(String::from(string)))
    }
    // https://w3c.github.io/webdriver/#dfn-clone-an-object
    else if val.get().is_object() {
@ -406,7 +405,7 @@ unsafe fn jsval_to_webdriver_inner(
        let return_val = if is_array_like::<crate::DomTypeHolder>(cx, val) ||
            is_arguments_object(cx, val)
        {
-            let mut result: Vec<WebDriverJSValue> = Vec::new();
+            let mut result: Vec<JSValue> = Vec::new();

            let length = match get_property::<u32>(
                cx,
@ -449,12 +448,10 @@ unsafe fn jsval_to_webdriver_inner(
                    },
                }
            }
-            Ok(WebDriverJSValue::ArrayLike(result))
+            Ok(JSValue::Array(result))
        } else if let Ok(element) = root_from_object::<Element>(*object, cx) {
-            Ok(WebDriverJSValue::Element(WebElement(
-                element
-                    .upcast::<Node>()
-                    .unique_id(element.owner_document().window().pipeline_id()),
+            Ok(JSValue::Element(element.upcast::<Node>().unique_id(
+                element.owner_document().window().pipeline_id(),
            )))
        } else if let Ok(window) = root_from_object::<Window>(*object, cx) {
            let window_proxy = window.window_proxy();
@ -463,13 +460,13 @@ unsafe fn jsval_to_webdriver_inner(
            } else {
                let pipeline = window.pipeline_id();
                if window_proxy.browsing_context_id() == window_proxy.webview_id() {
-                    Ok(WebDriverJSValue::Window(WebWindow(
+                    Ok(JSValue::Window(
                        window.Document().upcast::<Node>().unique_id(pipeline),
-                    )))
+                    ))
                } else {
-                    Ok(WebDriverJSValue::Frame(WebFrame(
+                    Ok(JSValue::Frame(
                        window.Document().upcast::<Node>().unique_id(pipeline),
-                    )))
+                    ))
                }
            }
        } else if object_has_to_json_property(cx, global_scope, object.handle()) {
@ -547,7 +544,7 @@ unsafe fn jsval_to_webdriver_inner(
                    }
                }
            }
-            Ok(WebDriverJSValue::Object(result))
+            Ok(JSValue::Object(result))
        };
        // Step 5. Remove the last element of `seen`.
        seen.remove(&hashable);
@ -1648,7 +1645,7 @@ pub(crate) fn handle_get_property(
    pipeline: PipelineId,
    node_id: String,
    name: String,
-    reply: IpcSender<Result<WebDriverJSValue, ErrorStatus>>,
+    reply: IpcSender<Result<JSValue, ErrorStatus>>,
    can_gc: CanGc,
 ) {
    reply
@ -1676,12 +1673,12 @@ pub(crate) fn handle_get_property(
                            can_gc,
                        ) {
                            Ok(property) => property,
-                            Err(_) => WebDriverJSValue::Undefined,
+                            Err(_) => JSValue::Undefined,
                        }
                    },
                    Err(error) => {
                        throw_dom_exception(cx, &element.global(), error, can_gc);
-                        WebDriverJSValue::Undefined
+                        JSValue::Undefined
                    },
                }
            }),
--- a/components/shared/embedder/lib.rs
+++ b/components/shared/embedder/lib.rs
@ -1003,29 +1003,6 @@ pub enum JSValue {
    Object(HashMap<String, JSValue>),
 }

-impl From<&WebDriverJSValue> for JSValue {
-    fn from(value: &WebDriverJSValue) -> Self {
-        match value {
-            WebDriverJSValue::Undefined => Self::Undefined,
-            WebDriverJSValue::Null => Self::Null,
-            WebDriverJSValue::Boolean(value) => Self::Boolean(*value),
-            WebDriverJSValue::Number(value) => Self::Number(*value),
-            WebDriverJSValue::String(value) => Self::String(value.clone()),
-            WebDriverJSValue::Element(web_element) => Self::Element(web_element.0.clone()),
-            WebDriverJSValue::Frame(web_frame) => Self::Frame(web_frame.0.clone()),
-            WebDriverJSValue::Window(web_window) => Self::Window(web_window.0.clone()),
-            WebDriverJSValue::ArrayLike(vector) => {
-                Self::Array(vector.iter().map(Into::into).collect())
-            },
-            WebDriverJSValue::Object(map) => Self::Object(
-                map.iter()
-                    .map(|(key, value)| (key.clone(), value.into()))
-                    .collect(),
-            ),
-        }
-    }
-}
-
 #[derive(Clone, Debug, Deserialize, PartialEq, Serialize)]
 pub enum JavaScriptEvaluationError {
    /// The script could not be compiled
--- a/components/shared/embedder/webdriver.rs
+++ b/components/shared/embedder/webdriver.rs
@ -19,11 +19,10 @@ use serde::{Deserialize, Serialize};
 use servo_geometry::DeviceIndependentIntRect;
 use servo_url::ServoUrl;
 use style_traits::CSSPixel;
-use webdriver::common::{WebElement, WebFrame, WebWindow};
 use webdriver::error::ErrorStatus;
 use webrender_api::units::DevicePixel;

-use crate::{FocusId, MouseButton, MouseButtonAction, TraversalId};
+use crate::{FocusId, JSValue, MouseButton, MouseButtonAction, TraversalId};

 #[derive(Clone, Copy, Debug, Deserialize, PartialEq, Serialize)]
 pub struct WebDriverMessageId(pub usize);
@ -226,11 +225,7 @@ pub enum WebDriverScriptCommand {
        String,
        IpcSender<Result<Option<String>, ErrorStatus>>,
    ),
-    GetElementProperty(
-        String,
-        String,
-        IpcSender<Result<WebDriverJSValue, ErrorStatus>>,
-    ),
+    GetElementProperty(String, String, IpcSender<Result<JSValue, ErrorStatus>>),
    GetElementCSS(String, String, IpcSender<Result<String, ErrorStatus>>),
    GetElementRect(String, IpcSender<Result<UntypedRect<f64>, ErrorStatus>>),
    GetElementTagName(String, IpcSender<Result<String, ErrorStatus>>),
@ -254,33 +249,19 @@ pub enum WebDriverScriptCommand {
    GetWindowHandle(IpcSender<Result<String, ErrorStatus>>),
 }

-#[derive(Clone, Debug, Deserialize, Serialize)]
-pub enum WebDriverJSValue {
-    Undefined,
-    Null,
-    Boolean(bool),
-    Number(f64),
-    String(String),
-    Element(WebElement),
-    Frame(WebFrame),
-    Window(WebWindow),
-    ArrayLike(Vec<WebDriverJSValue>),
-    Object(HashMap<String, WebDriverJSValue>),
-}
-
 #[derive(Debug, Deserialize, Serialize)]
 pub enum WebDriverJSError {
    /// Occurs when handler received an event message for a layout channel that is not
    /// associated with the current script thread
    BrowsingContextNotFound,
-    JSException(WebDriverJSValue),
+    JSException(JSValue),
    JSError,
    StaleElementReference,
    Timeout,
    UnknownType,
 }

-pub type WebDriverJSResult = Result<WebDriverJSValue, WebDriverJSError>;
+pub type WebDriverJSResult = Result<JSValue, WebDriverJSError>;

 #[derive(Debug, Deserialize, Serialize)]
 pub enum WebDriverFrameId {
--- a/components/webdriver_server/lib.rs
+++ b/components/webdriver_server/lib.rs
@ -27,8 +27,8 @@ use capabilities::ServoCapabilities;
 use cookie::{CookieBuilder, Expiration, SameSite};
 use crossbeam_channel::{Receiver, Sender, after, select, unbounded};
 use embedder_traits::{
-    EventLoopWaker, MouseButton, WebDriverCommandMsg, WebDriverCommandResponse, WebDriverFrameId,
-    WebDriverJSError, WebDriverJSResult, WebDriverJSValue, WebDriverLoadStatus, WebDriverMessageId,
+    EventLoopWaker, JSValue, MouseButton, WebDriverCommandMsg, WebDriverCommandResponse,
+    WebDriverFrameId, WebDriverJSError, WebDriverJSResult, WebDriverLoadStatus, WebDriverMessageId,
    WebDriverScriptCommand,
 };
 use euclid::{Point2D, Rect, Size2D};
@ -60,7 +60,9 @@ use webdriver::command::{
    SwitchToFrameParameters, SwitchToWindowParameters, TimeoutsParameters, WebDriverCommand,
    WebDriverExtensionCommand, WebDriverMessage, WindowRectParameters,
 };
-use webdriver::common::{Cookie, Date, LocatorStrategy, Parameters, ShadowRoot, WebElement};
+use webdriver::common::{
+    Cookie, Date, LocatorStrategy, Parameters, ShadowRoot, WebElement, WebFrame, WebWindow,
+};
 use webdriver::error::{ErrorStatus, WebDriverError, WebDriverResult};
 use webdriver::httpapi::WebDriverExtensionRoute;
 use webdriver::response::{
@ -294,31 +296,31 @@ impl WebDriverExtensionCommand for ServoExtensionCommand {
 }

 #[derive(Clone)]
-struct SendableWebDriverJSValue(pub WebDriverJSValue);
+struct SendableJSValue(pub JSValue);

-impl Serialize for SendableWebDriverJSValue {
+impl Serialize for SendableJSValue {
    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
    where
        S: Serializer,
    {
        match self.0 {
-            WebDriverJSValue::Undefined => serializer.serialize_unit(),
-            WebDriverJSValue::Null => serializer.serialize_unit(),
-            WebDriverJSValue::Boolean(x) => serializer.serialize_bool(x),
-            WebDriverJSValue::Number(x) => serializer.serialize_f64(x),
-            WebDriverJSValue::String(ref x) => serializer.serialize_str(x),
-            WebDriverJSValue::Element(ref x) => x.serialize(serializer),
-            WebDriverJSValue::Frame(ref x) => x.serialize(serializer),
-            WebDriverJSValue::Window(ref x) => x.serialize(serializer),
-            WebDriverJSValue::ArrayLike(ref x) => x
+            JSValue::Undefined => serializer.serialize_unit(),
+            JSValue::Null => serializer.serialize_unit(),
+            JSValue::Boolean(x) => serializer.serialize_bool(x),
+            JSValue::Number(x) => serializer.serialize_f64(x),
+            JSValue::String(ref x) => serializer.serialize_str(x),
+            JSValue::Element(ref x) => WebElement(x.clone()).serialize(serializer),
+            JSValue::Frame(ref x) => WebFrame(x.clone()).serialize(serializer),
+            JSValue::Window(ref x) => WebWindow(x.clone()).serialize(serializer),
+            JSValue::Array(ref x) => x
                .iter()
-                .map(|element| SendableWebDriverJSValue(element.clone()))
-                .collect::<Vec<SendableWebDriverJSValue>>()
+                .map(|element| SendableJSValue(element.clone()))
+                .collect::<Vec<SendableJSValue>>()
                .serialize(serializer),
-            WebDriverJSValue::Object(ref x) => x
+            JSValue::Object(ref x) => x
                .iter()
-                .map(|(k, v)| (k.clone(), SendableWebDriverJSValue(v.clone())))
-                .collect::<HashMap<String, SendableWebDriverJSValue>>()
+                .map(|(k, v)| (k.clone(), SendableJSValue(v.clone())))
+                .collect::<HashMap<String, SendableJSValue>>()
                .serialize(serializer),
        }
    }
@ -1748,7 +1750,7 @@ impl Handler {

        match wait_for_ipc_response(receiver)? {
            Ok(value) => Ok(WebDriverResponse::Generic(ValueResponse(
-                serde_json::to_value(SendableWebDriverJSValue(value))?,
+                serde_json::to_value(SendableJSValue(value))?,
            ))),
            Err(error) => Err(WebDriverError::new(error, "")),
        }
@ -2100,7 +2102,7 @@ impl Handler {
    ) -> WebDriverResult<WebDriverResponse> {
        match result {
            Ok(value) => Ok(WebDriverResponse::Generic(ValueResponse(
-                serde_json::to_value(SendableWebDriverJSValue(value))?,
+                serde_json::to_value(SendableJSValue(value))?,
            ))),
            Err(WebDriverJSError::BrowsingContextNotFound) => Err(WebDriverError::new(
                ErrorStatus::NoSuchWindow,
--- a/ports/servoshell/desktop/app_state.rs
+++ b/ports/servoshell/desktop/app_state.rs
@ -20,10 +20,9 @@ use servo::webrender_api::ScrollLocation;
 use servo::webrender_api::units::{DeviceIntPoint, DeviceIntSize};
 use servo::{
    AllowOrDenyRequest, AuthenticationRequest, FilterPattern, FocusId, FormControl,
-    GamepadHapticEffectType, KeyboardEvent, LoadStatus, PermissionRequest, Servo, ServoDelegate,
-    ServoError, SimpleDialog, TraversalId, WebDriverCommandMsg, WebDriverJSResult,
-    WebDriverJSValue, WebDriverLoadStatus, WebDriverUserPrompt, WebView, WebViewBuilder,
-    WebViewDelegate,
+    GamepadHapticEffectType, JSValue, KeyboardEvent, LoadStatus, PermissionRequest, Servo,
+    ServoDelegate, ServoError, SimpleDialog, TraversalId, WebDriverCommandMsg, WebDriverJSResult,
+    WebDriverLoadStatus, WebDriverUserPrompt, WebView, WebViewBuilder, WebViewDelegate,
 };
 use url::Url;

@ -500,8 +499,10 @@ impl RunningAppState {
            .borrow()
            .script_evaluation_interrupt_sender
        {
-            sender.send(Ok(WebDriverJSValue::Null)).unwrap_or_else(|err| {
-                info!("Notify dialog appear failed. Maybe the channel to webdriver is closed: {err}");
+            sender.send(Ok(JSValue::Null)).unwrap_or_else(|err| {
+                info!(
+                    "Notify dialog appear failed. Maybe the channel to webdriver is closed: {err}"
+                );
            });
        }
    }
--- a/tests/wpt/meta/webdriver/tests/classic/execute_script/node.py.ini
+++ b/tests/wpt/meta/webdriver/tests/classic/execute_script/node.py.ini
@ -27,7 +27,7 @@
    expected: FAIL

  [test_not_supported_nodes[document\]]
-    expected: ERROR
+    expected: FAIL

  [test_not_supported_nodes[doctype\]]
-    expected: ERROR
+    expected: FAIL