From 4f8731d56205802091b5cf3c9e7b694cab4ce5c7 Mon Sep 17 00:00:00 2001 From: Rodion Borovyk Date: Sun, 10 Aug 2025 18:51:46 +0200 Subject: [PATCH] script: Return a Result from GlobalScope::evaluate_script_on_global_with_result (#38549) Make GlobalScope::evaluate_script_on_global_with_result return a Result instead of a boolean. This is the first step to resolve issue #37810. Testing: Should not break or fix any existing tests --------- Signed-off-by: Rodion Borovyk --- components/script/devtools.rs | 2 +- components/script/dom/debuggerglobalscope.rs | 8 ++++-- components/script/dom/globalscope.rs | 11 ++++---- components/script/dom/htmlscriptelement.rs | 2 +- components/script/dom/userscripts.rs | 2 +- components/script/dom/worklet.rs | 2 +- components/script/dom/workletglobalscope.rs | 7 ++++- components/script/script_thread.rs | 4 +-- components/script/timers.rs | 2 +- components/script/webdriver_handlers.rs | 29 +++++++++++--------- components/shared/embedder/lib.rs | 4 +++ 11 files changed, 45 insertions(+), 28 deletions(-) diff --git a/components/script/devtools.rs b/components/script/devtools.rs index 65b41476044..05a18a613e5 100644 --- a/components/script/devtools.rs +++ b/components/script/devtools.rs @@ -60,7 +60,7 @@ pub(crate) fn handle_evaluate_js( let source_code = SourceCode::Text(Rc::new(DOMString::from_string(eval))); // TODO: run code with SpiderMonkey Debugger API, like Firefox does // - global.evaluate_script_on_global_with_result( + _ = global.evaluate_script_on_global_with_result( &source_code, "", rval.handle_mut(), diff --git a/components/script/dom/debuggerglobalscope.rs b/components/script/dom/debuggerglobalscope.rs index 0ac4d0ef695..118539bfbdb 100644 --- a/components/script/dom/debuggerglobalscope.rs +++ b/components/script/dom/debuggerglobalscope.rs @@ -6,6 +6,7 @@ use base::id::PipelineId; use constellation_traits::ScriptToConstellationChan; use devtools_traits::{ScriptToDevtoolsControlMsg, WorkerId}; use dom_struct::dom_struct; +use embedder_traits::JavaScriptEvaluationError; use embedder_traits::resources::{self, Resource}; use ipc_channel::ipc::IpcSender; use js::jsval::UndefinedValue; @@ -104,7 +105,7 @@ impl DebuggerGlobalScope { GlobalScope::get_cx() } - fn evaluate_js(&self, script: &str, can_gc: CanGc) -> bool { + fn evaluate_js(&self, script: &str, can_gc: CanGc) -> Result<(), JavaScriptEvaluationError> { rooted!(in (*Self::get_cx()) let mut rval = UndefinedValue()); self.global_scope.evaluate_js_on_global_with_result( script, @@ -117,7 +118,10 @@ impl DebuggerGlobalScope { } pub(crate) fn execute(&self, can_gc: CanGc) { - if !self.evaluate_js(&resources::read_string(Resource::DebuggerJS), can_gc) { + if self + .evaluate_js(&resources::read_string(Resource::DebuggerJS), can_gc) + .is_err() + { let ar = enter_realm(self); report_pending_exception(Self::get_cx(), true, InRealm::Entered(&ar), can_gc); } diff --git a/components/script/dom/globalscope.rs b/components/script/dom/globalscope.rs index 98a4a472f7c..0107ef34658 100644 --- a/components/script/dom/globalscope.rs +++ b/components/script/dom/globalscope.rs @@ -26,7 +26,7 @@ use content_security_policy::CspList; use crossbeam_channel::Sender; use devtools_traits::{PageError, ScriptToDevtoolsControlMsg}; use dom_struct::dom_struct; -use embedder_traits::EmbedderMsg; +use embedder_traits::{EmbedderMsg, JavaScriptEvaluationError}; use ipc_channel::ipc::{self, IpcSender}; use ipc_channel::router::ROUTER; use js::glue::{IsWrapper, UnwrapObjectDynamic}; @@ -2762,7 +2762,7 @@ impl GlobalScope { script_base_url: ServoUrl, can_gc: CanGc, introduction_type: Option<&'static CStr>, - ) -> bool { + ) -> Result<(), JavaScriptEvaluationError> { let source_code = SourceCode::Text(Rc::new(DOMString::from_string((*code).to_string()))); self.evaluate_script_on_global_with_result( &source_code, @@ -2789,7 +2789,7 @@ impl GlobalScope { script_base_url: ServoUrl, can_gc: CanGc, introduction_type: Option<&'static CStr>, - ) -> bool { + ) -> Result<(), JavaScriptEvaluationError> { let cx = GlobalScope::get_cx(); let ar = enter_realm(self); @@ -2815,7 +2815,7 @@ impl GlobalScope { if compiled_script.is_null() { debug!("error compiling Dom string"); report_pending_exception(cx, true, InRealm::Entered(&ar), can_gc); - return false; + return Err(JavaScriptEvaluationError::CompilationFailure); } }, SourceCode::Compiled(pre_compiled_script) => { @@ -2865,10 +2865,11 @@ impl GlobalScope { if !result { debug!("error evaluating Dom string"); report_pending_exception(cx, true, InRealm::Entered(&ar), can_gc); + return Err(JavaScriptEvaluationError::EvaluationFailure); } maybe_resume_unwind(); - result + Ok(()) } } diff --git a/components/script/dom/htmlscriptelement.rs b/components/script/dom/htmlscriptelement.rs index 40cc26d48cc..1d12f9801d1 100644 --- a/components/script/dom/htmlscriptelement.rs +++ b/components/script/dom/htmlscriptelement.rs @@ -1160,7 +1160,7 @@ impl HTMLScriptElement { self.line_number as u32 }; rooted!(in(*GlobalScope::get_cx()) let mut rval = UndefinedValue()); - window + _ = window .as_global_scope() .evaluate_script_on_global_with_result( &script.code, diff --git a/components/script/dom/userscripts.rs b/components/script/dom/userscripts.rs index 4433bfe4d64..6e291b67e31 100644 --- a/components/script/dom/userscripts.rs +++ b/components/script/dom/userscripts.rs @@ -31,7 +31,7 @@ pub(crate) fn load_script(head: &HTMLHeadElement) { Rc::new(DOMString::from_string(user_script.script)) ); let global_scope = win.as_global_scope(); - global_scope.evaluate_script_on_global_with_result( + _ = global_scope.evaluate_script_on_global_with_result( &script_text, &user_script.source_file.map(|path| path.to_string_lossy().to_string()).unwrap_or_default(), rval.handle_mut(), diff --git a/components/script/dom/worklet.rs b/components/script/dom/worklet.rs index 35157e89a8f..2058e05599d 100644 --- a/components/script/dom/worklet.rs +++ b/components/script/dom/worklet.rs @@ -691,7 +691,7 @@ impl WorkletThread { // to the main script thread. // https://github.com/w3c/css-houdini-drafts/issues/407 let ok = script - .map(|script| global_scope.evaluate_js(&script, can_gc)) + .map(|s| global_scope.evaluate_js(&s, can_gc).is_ok()) .unwrap_or(false); if !ok { diff --git a/components/script/dom/workletglobalscope.rs b/components/script/dom/workletglobalscope.rs index bfb70ea1741..80767ac2e21 100644 --- a/components/script/dom/workletglobalscope.rs +++ b/components/script/dom/workletglobalscope.rs @@ -9,6 +9,7 @@ use constellation_traits::{ScriptToConstellationChan, ScriptToConstellationMessa use crossbeam_channel::Sender; use devtools_traits::ScriptToDevtoolsControlMsg; use dom_struct::dom_struct; +use embedder_traits::JavaScriptEvaluationError; use ipc_channel::ipc::IpcSender; use js::jsval::UndefinedValue; use js::rust::Runtime; @@ -124,7 +125,11 @@ impl WorkletGlobalScope { } /// Evaluate a JS script in this global. - pub(crate) fn evaluate_js(&self, script: &str, can_gc: CanGc) -> bool { + pub(crate) fn evaluate_js( + &self, + script: &str, + can_gc: CanGc, + ) -> Result<(), JavaScriptEvaluationError> { debug!("Evaluating Dom in a worklet."); rooted!(in (*GlobalScope::get_cx()) let mut rval = UndefinedValue()); self.globalscope.evaluate_js_on_global_with_result( diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index a35d96c3280..93adedce430 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -3738,7 +3738,7 @@ impl ScriptThread { // Script source is ready to be evaluated (11.) let _ac = enter_realm(global_scope); rooted!(in(*GlobalScope::get_cx()) let mut jsval = UndefinedValue()); - global_scope.evaluate_js_on_global_with_result( + _ = global_scope.evaluate_js_on_global_with_result( &script_source, jsval.handle_mut(), ScriptFetchOptions::default_classic_script(global_scope), @@ -4093,7 +4093,7 @@ impl ScriptThread { let context = window.get_cx(); rooted!(in(*context) let mut return_value = UndefinedValue()); - global_scope.evaluate_js_on_global_with_result( + _ = global_scope.evaluate_js_on_global_with_result( &script, return_value.handle_mut(), ScriptFetchOptions::default_classic_script(global_scope), diff --git a/components/script/timers.rs b/components/script/timers.rs index 0fbd07cd77a..23d63a53ab2 100644 --- a/components/script/timers.rs +++ b/components/script/timers.rs @@ -556,7 +556,7 @@ impl JsTimerTask { let cx = GlobalScope::get_cx(); rooted!(in(*cx) let mut rval = UndefinedValue()); // FIXME(cybai): Use base url properly by saving private reference for timers (#27260) - global.evaluate_js_on_global_with_result( + _ = global.evaluate_js_on_global_with_result( code_str, rval.handle_mut(), ScriptFetchOptions::default_classic_script(&global), diff --git a/components/script/webdriver_handlers.rs b/components/script/webdriver_handlers.rs index 44c20ae81e4..de35116ddb2 100644 --- a/components/script/webdriver_handlers.rs +++ b/components/script/webdriver_handlers.rs @@ -524,17 +524,17 @@ pub(crate) fn handle_execute_script( rooted!(in(*cx) let mut rval = UndefinedValue()); let global = window.as_global_scope(); - let result = if global.evaluate_js_on_global_with_result( + let evaluation_result = global.evaluate_js_on_global_with_result( &eval, rval.handle_mut(), ScriptFetchOptions::default_classic_script(global), global.api_base_url(), can_gc, None, // No known `introductionType` for JS code from WebDriver - ) { - jsval_to_webdriver(cx, global, rval.handle(), realm, can_gc) - } else { - Err(WebDriverJSError::JSError) + ); + let result = match evaluation_result { + Ok(_) => jsval_to_webdriver(cx, global, rval.handle(), realm, can_gc), + Err(_) => Err(WebDriverJSError::JSError), }; if reply.send(result).is_err() { @@ -566,14 +566,17 @@ pub(crate) fn handle_execute_async_script( rooted!(in(*cx) let mut rval = UndefinedValue()); let global_scope = window.as_global_scope(); - 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, - None, // No known `introductionType` for JS code from WebDriver - ) { + 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, + None, // No known `introductionType` for JS code from WebDriver + ) + .is_err() + { reply_sender.send(Err(WebDriverJSError::JSError)).unwrap(); } }, diff --git a/components/shared/embedder/lib.rs b/components/shared/embedder/lib.rs index 378ebf55ff3..506f7c7c0d7 100644 --- a/components/shared/embedder/lib.rs +++ b/components/shared/embedder/lib.rs @@ -1033,6 +1033,10 @@ impl From<&WebDriverJSValue> for JSValue { #[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] pub enum JavaScriptEvaluationError { + /// The script could not be compiled + CompilationFailure, + /// The script could not be evaluated + EvaluationFailure, /// An internal Servo error prevented the JavaSript evaluation from completing properly. /// This indicates a bug in Servo. InternalError,