From 7015e0fb5f1a63ed13a5b07b056036e6e7adbc16 Mon Sep 17 00:00:00 2001 From: tanishka <109246904+taniishkaaa@users.noreply.github.com> Date: Tue, 22 Oct 2024 15:38:55 +0530 Subject: [PATCH] CanGc fixes in `errorevent.rs` (#33960) * CanGc fixes in errorevent.rs Signed-off-by: taniishkaaa * Allow too_many_arguments to avoid lint error Signed-off-by: taniishkaaa --------- Signed-off-by: taniishkaaa --- components/script/devtools.rs | 8 +++++++- components/script/dom/bindings/callback.rs | 4 ++-- components/script/dom/bindings/error.rs | 11 ++++++++--- components/script/dom/create.rs | 2 +- components/script/dom/customelementregistry.rs | 2 +- .../script/dom/dedicatedworkerglobalscope.rs | 5 +++-- components/script/dom/document.rs | 4 ++-- components/script/dom/errorevent.rs | 14 +++----------- components/script/dom/eventtarget.rs | 4 ++-- components/script/dom/globalscope.rs | 11 ++++++++--- components/script/dom/htmliframeelement.rs | 2 +- components/script/dom/htmlscriptelement.rs | 13 +++++++------ components/script/dom/serviceworkerglobalscope.rs | 2 +- components/script/dom/userscripts.rs | 2 ++ components/script/dom/workerglobalscope.rs | 2 +- components/script/dom/worklet.rs | 10 ++++++---- components/script/dom/workletglobalscope.rs | 5 +++-- components/script/script_module.rs | 9 +++++++-- components/script/script_thread.rs | 15 +++++++++------ components/script/timers.rs | 1 + components/script/webdriver_handlers.rs | 4 ++++ 21 files changed, 79 insertions(+), 51 deletions(-) diff --git a/components/script/devtools.rs b/components/script/devtools.rs index e02d413caf1..cad26f03941 100644 --- a/components/script/devtools.rs +++ b/components/script/devtools.rs @@ -44,7 +44,12 @@ use crate::script_runtime::CanGc; use crate::script_thread::Documents; #[allow(unsafe_code)] -pub fn handle_evaluate_js(global: &GlobalScope, eval: String, reply: IpcSender) { +pub fn handle_evaluate_js( + global: &GlobalScope, + eval: String, + reply: IpcSender, + can_gc: CanGc, +) { // global.get_cx() returns a valid `JSContext` pointer, so this is safe. let result = unsafe { let cx = GlobalScope::get_cx(); @@ -58,6 +63,7 @@ pub fn handle_evaluate_js(global: &GlobalScope, eval: String, reply: IpcSender, element: &Elemen unsafe { let ar = enter_realm(&*global); throw_dom_exception(cx, &global, error); - report_pending_exception(*cx, true, InRealm::Entered(&ar)); + report_pending_exception(*cx, true, InRealm::Entered(&ar), can_gc); } return; diff --git a/components/script/dom/dedicatedworkerglobalscope.rs b/components/script/dom/dedicatedworkerglobalscope.rs index 1b99d8771f2..4ce3fd6d88a 100644 --- a/components/script/dom/dedicatedworkerglobalscope.rs +++ b/components/script/dom/dedicatedworkerglobalscope.rs @@ -544,7 +544,7 @@ impl DedicatedWorkerGlobalScope { match msg { MixedMessage::Devtools(msg) => match msg { DevtoolScriptControlMsg::EvaluateJS(_pipe_id, string, sender) => { - devtools::handle_evaluate_js(self.upcast(), string, sender) + devtools::handle_evaluate_js(self.upcast(), string, sender, can_gc) }, DevtoolScriptControlMsg::WantsLiveNotifications(_pipe_id, bool_val) => { devtools::handle_wants_live_notifications(self.upcast(), bool_val) @@ -583,13 +583,14 @@ impl DedicatedWorkerGlobalScope { error_info.lineno, error_info.column, HandleValue::null(), + CanGc::note(), ); let event_status = event.upcast::().fire(worker.upcast::()); // Step 2. if event_status == EventStatus::NotCanceled { - global.report_an_error(error_info, HandleValue::null()); + global.report_an_error(error_info, HandleValue::null(), CanGc::note()); } })); self.parent_sender diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index 602298ec9c5..746e55fb186 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -3097,13 +3097,13 @@ impl Document { } /// - pub(crate) fn deliver_resize_loop_error_notification(&self) { + pub(crate) fn deliver_resize_loop_error_notification(&self, can_gc: CanGc) { let global_scope = self.window.upcast::(); let error_info: ErrorInfo = crate::dom::bindings::error::ErrorInfo { message: "ResizeObserver loop completed with undelivered notifications.".to_string(), ..Default::default() }; - global_scope.report_an_error(error_info, HandleValue::null()); + global_scope.report_an_error(error_info, HandleValue::null(), can_gc); } pub(crate) fn status_code(&self) -> Option { diff --git a/components/script/dom/errorevent.rs b/components/script/dom/errorevent.rs index 3638d6bff5f..012a2cde343 100644 --- a/components/script/dom/errorevent.rs +++ b/components/script/dom/errorevent.rs @@ -66,19 +66,11 @@ impl ErrorEvent { lineno: u32, colno: u32, error: HandleValue, + can_gc: CanGc, ) -> DomRoot { Self::new_with_proto( - global, - None, - type_, - bubbles, - cancelable, - message, - filename, - lineno, - colno, - error, - CanGc::note(), + global, None, type_, bubbles, cancelable, message, filename, lineno, colno, error, + can_gc, ) } diff --git a/components/script/dom/eventtarget.rs b/components/script/dom/eventtarget.rs index 811938b5aa5..6515f2ba19f 100644 --- a/components/script/dom/eventtarget.rs +++ b/components/script/dom/eventtarget.rs @@ -486,7 +486,7 @@ impl EventTarget { &self, handler: InternalRawUncompiledHandler, ty: &Atom, - _can_gc: CanGc, + can_gc: CanGc, ) -> Option { // Step 3.1 let element = self.downcast::(); @@ -568,7 +568,7 @@ impl EventTarget { unsafe { let ar = enter_realm(self); // FIXME(#13152): dispatch error event. - report_pending_exception(*cx, false, InRealm::Entered(&ar)); + report_pending_exception(*cx, false, InRealm::Entered(&ar), can_gc); } return None; } diff --git a/components/script/dom/globalscope.rs b/components/script/dom/globalscope.rs index 81574c8b50a..a972c7d7eb1 100644 --- a/components/script/dom/globalscope.rs +++ b/components/script/dom/globalscope.rs @@ -2453,7 +2453,7 @@ impl GlobalScope { } /// - pub fn report_an_error(&self, error_info: ErrorInfo, value: HandleValue) { + pub fn report_an_error(&self, error_info: ErrorInfo, value: HandleValue, can_gc: CanGc) { // Step 1. if self.in_error_reporting_mode.get() { return; @@ -2474,6 +2474,7 @@ impl GlobalScope { error_info.lineno, error_info.column, value, + can_gc, ); // Step 7. @@ -2613,6 +2614,7 @@ impl GlobalScope { rval: MutableHandleValue, fetch_options: ScriptFetchOptions, script_base_url: ServoUrl, + can_gc: CanGc, ) -> bool { let source_code = SourceCode::Text(Rc::new(DOMString::from_string((*code).to_string()))); self.evaluate_script_on_global_with_result( @@ -2622,11 +2624,13 @@ impl GlobalScope { 1, fetch_options, script_base_url, + can_gc, ) } /// Evaluate a JS script on this global scope. #[allow(unsafe_code)] + #[allow(clippy::too_many_arguments)] pub fn evaluate_script_on_global_with_result( &self, code: &SourceCode, @@ -2635,6 +2639,7 @@ impl GlobalScope { line_number: u32, fetch_options: ScriptFetchOptions, script_base_url: ServoUrl, + can_gc: CanGc, ) -> bool { let metadata = profile_time::TimerMetadata { url: if filename.is_empty() { @@ -2671,7 +2676,7 @@ impl GlobalScope { if compiled_script.is_null() { debug!("error compiling Dom string"); - report_pending_exception(*cx, true, InRealm::Entered(&ar)); + report_pending_exception(*cx, true, InRealm::Entered(&ar), can_gc); return false; } }, @@ -2720,7 +2725,7 @@ impl GlobalScope { if !result { debug!("error evaluating Dom string"); - report_pending_exception(*cx, true, InRealm::Entered(&ar)); + report_pending_exception(*cx, true, InRealm::Entered(&ar), can_gc); } maybe_resume_unwind(); diff --git a/components/script/dom/htmliframeelement.rs b/components/script/dom/htmliframeelement.rs index c62e28aca71..d7f0f39274e 100644 --- a/components/script/dom/htmliframeelement.rs +++ b/components/script/dom/htmliframeelement.rs @@ -159,7 +159,7 @@ impl HTMLIFrameElement { // TODO: check according to https://w3c.github.io/webappsec-csp/#should-block-navigation-request if ScriptThread::check_load_origin(&load_data.load_origin, &document.url().origin()) { - ScriptThread::eval_js_url(&window_proxy.global(), &mut load_data); + ScriptThread::eval_js_url(&window_proxy.global(), &mut load_data, can_gc); } } } diff --git a/components/script/dom/htmlscriptelement.rs b/components/script/dom/htmlscriptelement.rs index edeca5317db..8631f4493eb 100644 --- a/components/script/dom/htmlscriptelement.rs +++ b/components/script/dom/htmlscriptelement.rs @@ -1016,12 +1016,12 @@ impl HTMLScriptElement { match script.type_ { ScriptType::Classic => { - self.run_a_classic_script(&script); + self.run_a_classic_script(&script, CanGc::note()); document.set_current_script(old_script.as_deref()); }, ScriptType::Module => { assert!(document.GetCurrentScript().is_none()); - self.run_a_module_script(&script, false); + self.run_a_module_script(&script, false, CanGc::note()); }, } @@ -1037,7 +1037,7 @@ impl HTMLScriptElement { } // https://html.spec.whatwg.org/multipage/#run-a-classic-script - pub fn run_a_classic_script(&self, script: &ScriptOrigin) { + pub fn run_a_classic_script(&self, script: &ScriptOrigin, can_gc: CanGc) { // TODO use a settings object rather than this element's document/window // Step 2 let document = document_from_node(self); @@ -1061,12 +1061,13 @@ impl HTMLScriptElement { line_number, script.fetch_options.clone(), script.url.clone(), + can_gc, ); } #[allow(unsafe_code)] /// - pub fn run_a_module_script(&self, script: &ScriptOrigin, _rethrow_errors: bool) { + pub fn run_a_module_script(&self, script: &ScriptOrigin, _rethrow_errors: bool, can_gc: CanGc) { // TODO use a settings object rather than this element's document/window // Step 2 let document = document_from_node(self); @@ -1095,7 +1096,7 @@ impl HTMLScriptElement { let module_error = module_tree.get_rethrow_error().borrow(); let network_error = module_tree.get_network_error().borrow(); if module_error.is_some() && network_error.is_none() { - module_tree.report_error(global); + module_tree.report_error(global, can_gc); return; } } @@ -1113,7 +1114,7 @@ impl HTMLScriptElement { if let Err(exception) = evaluated { module_tree.set_rethrow_error(exception); - module_tree.report_error(global); + module_tree.report_error(global, can_gc); } } } diff --git a/components/script/dom/serviceworkerglobalscope.rs b/components/script/dom/serviceworkerglobalscope.rs index 54c879eaa1e..98f800c0dd6 100644 --- a/components/script/dom/serviceworkerglobalscope.rs +++ b/components/script/dom/serviceworkerglobalscope.rs @@ -414,7 +414,7 @@ impl ServiceWorkerGlobalScope { match msg { MixedMessage::Devtools(msg) => match msg { DevtoolScriptControlMsg::EvaluateJS(_pipe_id, string, sender) => { - devtools::handle_evaluate_js(self.upcast(), string, sender) + devtools::handle_evaluate_js(self.upcast(), string, sender, can_gc) }, DevtoolScriptControlMsg::WantsLiveNotifications(_pipe_id, bool_val) => { devtools::handle_wants_live_notifications(self.upcast(), bool_val) diff --git a/components/script/dom/userscripts.rs b/components/script/dom/userscripts.rs index 10197dbcfb7..5ac6e5042e7 100644 --- a/components/script/dom/userscripts.rs +++ b/components/script/dom/userscripts.rs @@ -17,6 +17,7 @@ use crate::dom::htmlheadelement::HTMLHeadElement; use crate::dom::htmlscriptelement::SourceCode; use crate::dom::node::document_from_node; use crate::script_module::ScriptFetchOptions; +use crate::script_runtime::CanGc; pub fn load_script(head: &HTMLHeadElement) { let doc = document_from_node(head); @@ -54,6 +55,7 @@ pub fn load_script(head: &HTMLHeadElement) { 1, ScriptFetchOptions::default_classic_script(global), global.api_base_url(), + CanGc::note(), ); } })); diff --git a/components/script/dom/workerglobalscope.rs b/components/script/dom/workerglobalscope.rs index 47bd9232e2d..16cee5b7d9f 100644 --- a/components/script/dom/workerglobalscope.rs +++ b/components/script/dom/workerglobalscope.rs @@ -467,7 +467,7 @@ impl WorkerGlobalScope { println!("evaluate_script failed"); unsafe { let ar = enter_realm(self); - report_pending_exception(cx, true, InRealm::Entered(&ar)); + report_pending_exception(cx, true, InRealm::Entered(&ar), CanGc::note()); } } }, diff --git a/components/script/dom/worklet.rs b/components/script/dom/worklet.rs index 5f639d3214e..029f266dfd3 100644 --- a/components/script/dom/worklet.rs +++ b/components/script/dom/worklet.rs @@ -546,10 +546,10 @@ impl WorkletThread { // try to become the cold backup. if self.role.is_cold_backup { if let Some(control) = self.control_buffer.take() { - self.process_control(control); + self.process_control(control, CanGc::note()); } while let Ok(control) = self.control_receiver.try_recv() { - self.process_control(control); + self.process_control(control, CanGc::note()); } self.gc(); } else if self.control_buffer.is_none() { @@ -638,6 +638,7 @@ impl WorkletThread { credentials: RequestCredentials, pending_tasks_struct: PendingTasksStruct, promise: TrustedPromise, + can_gc: CanGc, ) { debug!("Fetching from {}.", script_url); // Step 1. @@ -672,7 +673,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)) + .map(|script| global_scope.evaluate_js(&script, can_gc)) .unwrap_or(false); if !ok { @@ -707,7 +708,7 @@ impl WorkletThread { } /// Process a control message. - fn process_control(&mut self, control: WorkletControl) { + fn process_control(&mut self, control: WorkletControl, can_gc: CanGc) { match control { WorkletControl::ExitWorklet(worklet_id) => { self.global_scopes.remove(&worklet_id); @@ -733,6 +734,7 @@ impl WorkletThread { credentials, pending_tasks_struct, promise, + can_gc, ) }, } diff --git a/components/script/dom/workletglobalscope.rs b/components/script/dom/workletglobalscope.rs index 35adf0ac6e9..307bcab46c8 100644 --- a/components/script/dom/workletglobalscope.rs +++ b/components/script/dom/workletglobalscope.rs @@ -27,7 +27,7 @@ use crate::dom::paintworkletglobalscope::{PaintWorkletGlobalScope, PaintWorkletT use crate::dom::testworkletglobalscope::{TestWorkletGlobalScope, TestWorkletTask}; use crate::dom::worklet::WorkletExecutor; use crate::script_module::ScriptFetchOptions; -use crate::script_runtime::JSContext; +use crate::script_runtime::{CanGc, JSContext}; use crate::script_thread::MainThreadScriptMsg; #[dom_struct] @@ -114,7 +114,7 @@ impl WorkletGlobalScope { } /// Evaluate a JS script in this global. - pub fn evaluate_js(&self, script: &str) -> bool { + pub fn evaluate_js(&self, script: &str, can_gc: CanGc) -> bool { debug!("Evaluating Dom in a worklet."); rooted!(in (*GlobalScope::get_cx()) let mut rval = UndefinedValue()); self.globalscope.evaluate_js_on_global_with_result( @@ -122,6 +122,7 @@ impl WorkletGlobalScope { rval.handle_mut(), ScriptFetchOptions::default_classic_script(&self.globalscope), self.globalscope.api_base_url(), + can_gc, ) } diff --git a/components/script/script_module.rs b/components/script/script_module.rs index 5c6c992f73a..9bba80a051a 100644 --- a/components/script/script_module.rs +++ b/components/script/script_module.rs @@ -541,7 +541,7 @@ impl ModuleTree { } #[allow(unsafe_code)] - pub fn report_error(&self, global: &GlobalScope) { + pub fn report_error(&self, global: &GlobalScope, can_gc: CanGc) { let module_error = self.rethrow_error.borrow(); if let Some(exception) = &*module_error { @@ -552,7 +552,12 @@ impl ModuleTree { exception.handle(), ExceptionStackBehavior::Capture, ); - report_pending_exception(*GlobalScope::get_cx(), true, InRealm::Entered(&ar)); + report_pending_exception( + *GlobalScope::get_cx(), + true, + InRealm::Entered(&ar), + can_gc, + ); } } } diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index 849eee166f5..9aa8c4d5a8a 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -1028,7 +1028,7 @@ impl ScriptThread { // TODO: check according to https://w3c.github.io/webappsec-csp/#should-block-navigation-request if let Some(window) = trusted_global.root().downcast::() { if ScriptThread::check_load_origin(&load_data.load_origin, &window.get_url().origin()) { - ScriptThread::eval_js_url(&trusted_global.root(), &mut load_data); + ScriptThread::eval_js_url(&trusted_global.root(), &mut load_data, CanGc::note()); sender .send((pipeline_id, ScriptMsg::LoadUrl(load_data, replace))) .unwrap(); @@ -1738,7 +1738,7 @@ impl ScriptThread { } if document.has_skipped_resize_observations() { - document.deliver_resize_loop_error_notification(); + document.deliver_resize_loop_error_notification(can_gc); } // TODO(#31870): Implement step 17: if the focused area of doc is not a focusable area, @@ -2578,7 +2578,7 @@ impl ScriptThread { Some(window) => { let global = window.upcast::(); let _aes = AutoEntryScript::new(global); - devtools::handle_evaluate_js(global, s, reply) + devtools::handle_evaluate_js(global, s, reply, can_gc) }, None => warn!("Message sent to closed pipeline {}.", id), }, @@ -2660,11 +2660,13 @@ impl ScriptThread { match msg { WebDriverScriptCommand::ExecuteScript(script, reply) => { let window = self.documents.borrow().find_window(pipeline_id); - return webdriver_handlers::handle_execute_script(window, script, reply); + return webdriver_handlers::handle_execute_script(window, script, reply, can_gc); }, WebDriverScriptCommand::ExecuteAsyncScript(script, reply) => { let window = self.documents.borrow().find_window(pipeline_id); - return webdriver_handlers::handle_execute_async_script(window, script, reply); + return webdriver_handlers::handle_execute_async_script( + window, script, reply, can_gc, + ); }, _ => (), } @@ -4007,7 +4009,7 @@ impl ScriptThread { /// Turn javascript: URL into JS code to eval, according to the steps in /// - pub fn eval_js_url(global_scope: &GlobalScope, load_data: &mut LoadData) { + pub fn eval_js_url(global_scope: &GlobalScope, load_data: &mut LoadData, can_gc: CanGc) { // This slice of the URL’s serialization is equivalent to (5.) to (7.): // Start with the scheme data of the parsed URL; // append question mark and query component, if any; @@ -4025,6 +4027,7 @@ impl ScriptThread { jsval.handle_mut(), ScriptFetchOptions::default_classic_script(global_scope), global_scope.api_base_url(), + can_gc, ); load_data.js_eval_result = if jsval.get().is_string() { diff --git a/components/script/timers.rs b/components/script/timers.rs index e4348a1b01f..4f64e529787 100644 --- a/components/script/timers.rs +++ b/components/script/timers.rs @@ -558,6 +558,7 @@ impl JsTimerTask { rval.handle_mut(), ScriptFetchOptions::default_classic_script(&global), global.api_base_url(), + CanGc::note(), ); }, InternalTimerCallback::FunctionTimerCallback(ref function, ref arguments) => { diff --git a/components/script/webdriver_handlers.rs b/components/script/webdriver_handlers.rs index a81f1828b76..ea794ebba7c 100644 --- a/components/script/webdriver_handlers.rs +++ b/components/script/webdriver_handlers.rs @@ -289,6 +289,7 @@ pub fn handle_execute_script( window: Option>, eval: String, reply: IpcSender, + can_gc: CanGc, ) { match window { Some(window) => { @@ -301,6 +302,7 @@ pub fn handle_execute_script( rval.handle_mut(), ScriptFetchOptions::default_classic_script(global), global.api_base_url(), + can_gc, ); jsval_to_webdriver(*cx, window.upcast::(), rval.handle()) }; @@ -319,6 +321,7 @@ pub fn handle_execute_async_script( window: Option>, eval: String, reply: IpcSender, + can_gc: CanGc, ) { match window { Some(window) => { @@ -331,6 +334,7 @@ pub fn handle_execute_async_script( rval.handle_mut(), ScriptFetchOptions::default_classic_script(global), global.api_base_url(), + can_gc, ); }, None => {