Report exceptions for async script executions to webdriver (#27041)

Improving the accuracy of the async script execution for our webdriver
implementation allows us to run many more webdriver tests without
timeouts, which should help with
https://github.com/web-platform-tests/wpt/issues/24257.

Specification:
* https://w3c.github.io/webdriver/#dfn-clone-an-object
* https://w3c.github.io/webdriver/#execute-async-script

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #27036 and fix #27035 and fix #27031.
- [x] There are tests for these changes (but we don't run them in CI
yet)

---------

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

components/script/dom/window.rs

@@ -1216,15 +1216,26 @@ impl WindowMethods<crate::DomTypeHolder> for Window {
         }
     }
 
-    #[allow(unsafe_code)]
-    fn WebdriverCallback(&self, cx: JSContext, val: HandleValue) {
-        let rv = unsafe { jsval_to_webdriver(*cx, &self.globalscope, val) };
+    fn WebdriverCallback(&self, cx: JSContext, val: HandleValue, realm: InRealm, can_gc: CanGc) {
+        let rv = jsval_to_webdriver(cx, &self.globalscope, val, realm, can_gc);
         let opt_chan = self.webdriver_script_chan.borrow_mut().take();
         if let Some(chan) = opt_chan {
             chan.send(rv).unwrap();
         }
     }
 
+    fn WebdriverException(&self, cx: JSContext, val: HandleValue, realm: InRealm, can_gc: CanGc) {
+        let rv = jsval_to_webdriver(cx, &self.globalscope, val, realm, can_gc);
+        let opt_chan = self.webdriver_script_chan.borrow_mut().take();
+        if let Some(chan) = opt_chan {
+            if let Ok(rv) = rv {
+                chan.send(Err(WebDriverJSError::JSException(rv))).unwrap();
+            } else {
+                chan.send(rv).unwrap();
+            }
+        }
+    }
+
     fn WebdriverTimeout(&self) {
         let opt_chan = self.webdriver_script_chan.borrow_mut().take();
         if let Some(chan) = opt_chan {

components/script/script_thread.rs

@@ -2274,6 +2274,7 @@ impl ScriptThread {
                     node_id,
                     name,
                     reply,
+                    can_gc,
                 )
             },
             WebDriverScriptCommand::GetElementCSS(node_id, name, reply) => {

components/script/webdriver_handlers.rs

@@ -3,7 +3,7 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
 use std::cmp;
-use std::collections::HashMap;
+use std::collections::{HashMap, HashSet};
 use std::ffi::CString;
 use std::ptr::NonNull;
 
@@ -48,7 +48,7 @@ use crate::dom::bindings::conversions::{
     ConversionBehavior, ConversionResult, FromJSValConvertible, StringificationBehavior,
     get_property, get_property_jsval, jsid_to_string, jsstring_to_str, root_from_object,
 };
-use crate::dom::bindings::error::{Error, throw_dom_exception};
+use crate::dom::bindings::error::{Error, report_pending_exception, throw_dom_exception};
 use crate::dom::bindings::inheritance::Castable;
 use crate::dom::bindings::reflector::{DomGlobal, DomObject};
 use crate::dom::bindings::root::DomRoot;
@@ -67,7 +67,7 @@ use crate::dom::node::{Node, NodeTraits, ShadowIncluding};
 use crate::dom::nodelist::NodeList;
 use crate::dom::window::Window;
 use crate::dom::xmlserializer::XMLSerializer;
-use crate::realms::enter_realm;
+use crate::realms::{AlreadyInRealm, InRealm, enter_realm};
 use crate::script_module::ScriptFetchOptions;
 use crate::script_runtime::{CanGc, JSContext as SafeJSContext};
 use crate::script_thread::ScriptThread;
@@ -183,12 +183,44 @@ unsafe fn is_arguments_object(cx: *mut JSContext, value: HandleValue) -> bool {
     jsstring_to_str(cx, class_name) == "[object Arguments]"
 }
 
+#[derive(Eq, Hash, PartialEq)]
+struct HashableJSVal(u64);
+
+impl From<HandleValue<'_>> for HashableJSVal {
+    fn from(v: HandleValue<'_>) -> HashableJSVal {
+        HashableJSVal(v.get().asBits_)
+    }
+}
+
 #[allow(unsafe_code)]
-pub(crate) unsafe fn jsval_to_webdriver(
+pub(crate) fn jsval_to_webdriver(
+    cx: SafeJSContext,
+    global_scope: &GlobalScope,
+    val: HandleValue,
+    realm: InRealm,
+    can_gc: CanGc,
+) -> WebDriverJSResult {
+    let mut seen = HashSet::new();
+    let result = unsafe { jsval_to_webdriver_inner(*cx, global_scope, val, &mut seen) };
+    if result.is_err() {
+        report_pending_exception(cx, true, realm, can_gc);
+    }
+    result
+}
+
+#[allow(unsafe_code)]
+unsafe fn jsval_to_webdriver_inner(
     cx: *mut JSContext,
     global_scope: &GlobalScope,
     val: HandleValue,
+    seen: &mut HashSet<HashableJSVal>,
 ) -> WebDriverJSResult {
+    let hashable = val.into();
+    if seen.contains(&hashable) {
+        return Err(WebDriverJSError::JSError);
+    }
+    seen.insert(hashable);
+
     let _ac = enter_realm(global_scope);
     if val.get().is_undefined() {
         Ok(WebDriverJSValue::Undefined)
@@ -254,9 +286,11 @@ pub(crate) unsafe fn jsval_to_webdriver(
             for i in 0..length {
                 rooted!(in(cx) let mut item = UndefinedValue());
                 match get_property_jsval(cx, object.handle(), &i.to_string(), item.handle_mut()) {
-                    Ok(_) => match jsval_to_webdriver(cx, global_scope, item.handle()) {
-                        Ok(converted_item) => result.push(converted_item),
-                        err @ Err(_) => return err,
+                    Ok(_) => {
+                        match jsval_to_webdriver_inner(cx, global_scope, item.handle(), seen) {
+                            Ok(converted_item) => result.push(converted_item),
+                            err @ Err(_) => return err,
+                        }
                     },
                     Err(error) => {
                         throw_dom_exception(
@@ -298,7 +332,7 @@ pub(crate) unsafe fn jsval_to_webdriver(
                 &HandleValueArray::empty(),
                 value.handle_mut(),
             ) {
-                jsval_to_webdriver(cx, global_scope, value.handle())
+                jsval_to_webdriver_inner(cx, global_scope, value.handle(), seen)
             } else {
                 throw_dom_exception(
                     SafeJSContext::from_ptr(cx),
@@ -349,7 +383,9 @@ pub(crate) unsafe fn jsval_to_webdriver(
                         return Err(WebDriverJSError::JSError);
                     };
 
-                    if let Ok(value) = jsval_to_webdriver(cx, global_scope, property.handle()) {
+                    if let Ok(value) =
+                        jsval_to_webdriver_inner(cx, global_scope, property.handle(), seen)
+                    {
                         result.insert(name.into(), value);
                     } else {
                         return Err(WebDriverJSError::JSError);
@@ -373,18 +409,22 @@ pub(crate) fn handle_execute_script(
 ) {
     match window {
         Some(window) => {
-            let result = unsafe {
-                let cx = window.get_cx();
-                rooted!(in(*cx) let mut rval = UndefinedValue());
-                let global = window.as_global_scope();
-                global.evaluate_js_on_global_with_result(
-                    &eval,
-                    rval.handle_mut(),
-                    ScriptFetchOptions::default_classic_script(global),
-                    global.api_base_url(),
-                    can_gc,
-                );
-                jsval_to_webdriver(*cx, global, rval.handle())
+            let cx = window.get_cx();
+            let realm = AlreadyInRealm::assert_for_cx(cx);
+            let realm = InRealm::already(&realm);
+
+            rooted!(in(*cx) let mut rval = UndefinedValue());
+            let global = window.as_global_scope();
+            let result = if global.evaluate_js_on_global_with_result(
+                &eval,
+                rval.handle_mut(),
+                ScriptFetchOptions::default_classic_script(global),
+                global.api_base_url(),
+                can_gc,
+            ) {
+                jsval_to_webdriver(cx, global, rval.handle(), realm, can_gc)
+            } else {
+                Err(WebDriverJSError::JSError)
             };
 
             reply.send(result).unwrap();
@@ -406,17 +446,20 @@ pub(crate) fn handle_execute_async_script(
     match window {
         Some(window) => {
             let cx = window.get_cx();
+            let reply_sender = reply.clone();
             window.set_webdriver_script_chan(Some(reply));
             rooted!(in(*cx) let mut rval = UndefinedValue());
 
             let global_scope = window.as_global_scope();
-            global_scope.evaluate_js_on_global_with_result(
+            if !global_scope.evaluate_js_on_global_with_result(
                 &eval,
                 rval.handle_mut(),
                 ScriptFetchOptions::default_classic_script(global_scope),
                 global_scope.api_base_url(),
                 can_gc,
-            );
+            ) {
+                reply_sender.send(Err(WebDriverJSError::JSError)).unwrap();
+            }
         },
         None => {
             reply
@@ -1136,12 +1179,13 @@ pub(crate) fn handle_get_property(
     node_id: String,
     name: String,
     reply: IpcSender<Result<WebDriverJSValue, ErrorStatus>>,
+    can_gc: CanGc,
 ) {
     reply
         .send(
             find_node_by_unique_id(documents, pipeline, node_id).map(|node| {
                 let document = documents.find_document(pipeline).unwrap();
-                let _ac = enter_realm(&*document);
+                let realm = enter_realm(&*document);
                 let cx = document.window().get_cx();
 
                 rooted!(in(*cx) let mut property = UndefinedValue());
@@ -1154,14 +1198,19 @@ pub(crate) fn handle_get_property(
                     )
                 } {
                     Ok(_) => {
-                        match unsafe { jsval_to_webdriver(*cx, &node.global(), property.handle()) }
-                        {
+                        match jsval_to_webdriver(
+                            cx,
+                            &node.global(),
+                            property.handle(),
+                            InRealm::entered(&realm),
+                            can_gc,
+                        ) {
                             Ok(property) => property,
                             Err(_) => WebDriverJSValue::Undefined,
                         }
                     },
                     Err(error) => {
-                        throw_dom_exception(cx, &node.global(), error, CanGc::note());
+                        throw_dom_exception(cx, &node.global(), error, can_gc);
                         WebDriverJSValue::Undefined
                     },
                 }

components/script_bindings/codegen/Bindings.conf

@@ -642,8 +642,8 @@ DOMInterfaces = {
 },
 
 'Window': {
-    'canGc': ['Stop', 'Fetch', 'Scroll', 'Scroll_','ScrollBy', 'ScrollBy_', 'Stop', 'Fetch', 'Open', 'CreateImageBitmap', 'TrustedTypes'],
-    'inRealms': ['Fetch', 'GetOpener'],
+    'canGc': ['Stop', 'Fetch', 'Scroll', 'Scroll_','ScrollBy', 'ScrollBy_', 'Stop', 'Fetch', 'Open', 'CreateImageBitmap', 'TrustedTypes', 'WebdriverCallback', 'WebdriverException'],
+    'inRealms': ['Fetch', 'GetOpener', 'WebdriverCallback', 'WebdriverException'],
     'additionalTraits': ['crate::interfaces::WindowHelpers'],
 },
 

components/script_bindings/webidls/Window.webidl

@@ -148,6 +148,7 @@ partial interface Window {
 partial interface Window {
   // Shouldn't be public, but just to make things work for now
   undefined webdriverCallback(optional any result);
+  undefined webdriverException(optional any result);
   undefined webdriverTimeout();
 };
 

components/shared/embedder/webdriver.rs

@@ -170,6 +170,7 @@ 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),
     JSError,
     StaleElementReference,
     Timeout,

components/webdriver_server/lib.rs

@@ -1505,7 +1505,7 @@ impl Handler {
             .iter()
             .map(webdriver_value_to_js_argument)
             .collect();
-        args_string.push("window.webdriverCallback".to_string());
+        args_string.push("resolve".to_string());
 
         let timeout_script = if let Some(script_timeout) = self.session()?.script_timeout {
             format!("setTimeout(webdriverTimeout, {});", script_timeout)
@@ -1513,7 +1513,17 @@ impl Handler {
             "".into()
         };
         let script = format!(
-            "{} (function() {{ {}\n }})({})",
+            r#"(function() {{
+              let webdriverPromise = new Promise(function(resolve, reject) {{
+                  {}
+                  (async function() {{
+                    {}
+                  }})({})
+                    .then((v) => {{}}, (err) => reject(err))
+              }})
+              .then((v) => window.webdriverCallback(v), (r) => window.webdriverException(r))
+              .catch((r) => window.webdriverException(r));
+            }})();"#,
             timeout_script,
             func_body,
             args_string.join(", "),
@@ -1541,15 +1551,21 @@ impl Handler {
                 ErrorStatus::NoSuchWindow,
                 "Pipeline id not found in browsing context",
             )),
-            Err(WebDriverJSError::JSError) => Err(WebDriverError::new(
+            Err(WebDriverJSError::JSException(_e)) => Err(WebDriverError::new(
                 ErrorStatus::JavascriptError,
                 "JS evaluation raised an exception",
             )),
+            Err(WebDriverJSError::JSError) => Err(WebDriverError::new(
+                ErrorStatus::JavascriptError,
+                "JS evaluation raised an unknown exception",
+            )),
             Err(WebDriverJSError::StaleElementReference) => Err(WebDriverError::new(
                 ErrorStatus::StaleElementReference,
                 "Stale element",
             )),
-            Err(WebDriverJSError::Timeout) => Err(WebDriverError::new(ErrorStatus::Timeout, "")),
+            Err(WebDriverJSError::Timeout) => {
+                Err(WebDriverError::new(ErrorStatus::ScriptTimeout, ""))
+            },
             Err(WebDriverJSError::UnknownType) => Err(WebDriverError::new(
                 ErrorStatus::UnsupportedOperation,
                 "Unsupported return type",