clippy: Add safety documentation and clean up unsafe methods (#33748)

This change:

1. Adds safety documentation where it was missing.
2. Limits the scope of unsafe code in some cases to where it is actually
   unsafe.
3. Converts some free functions to associated functions and methods,
   thereby making them more likely to be called safely.

Signed-off-by: Martin Robinson <mrobinson@igalia.com>

components/script/dom/bindings/reflector.rs

@@ -76,6 +76,10 @@ impl Reflector {
     }
 
     /// Initialize the reflector. (May be called only once.)
+    ///
+    /// # Safety
+    ///
+    /// The provided [`JSObject`] pointer must point to a valid [`JSObject`].
     pub unsafe fn set_jsobject(&self, object: *mut JSObject) {
         assert!(self.object.get().is_null());
         assert!(!object.is_null());
@@ -123,6 +127,10 @@ impl DomObject for Reflector {
 /// A trait to initialize the `Reflector` for a DOM object.
 pub trait MutDomObject: DomObject {
     /// Initializes the Reflector
+    ///
+    /// # Safety
+    ///
+    /// The provided [`JSObject`] pointer must point to a valid [`JSObject`].
     unsafe fn init_reflector(&self, obj: *mut JSObject);
 }
 

components/script/dom/bindings/trace.rs

@@ -69,8 +69,19 @@ use crate::script_thread::IncompleteParserContexts;
 use crate::task::TaskBox;
 
 /// A trait to allow tracing only DOM sub-objects.
+///
+/// # Safety
+///
+/// This trait is unsafe; if it is implemented incorrectly, the GC may end up collecting objects
+/// that are still reachable.
 pub unsafe trait CustomTraceable {
     /// Trace `self`.
+    ///
+    /// # Safety
+    ///
+    /// The `JSTracer` argument must point to a valid `JSTracer` in memory. In addition,
+    /// implementors of this method must ensure that all active objects are properly traced
+    /// or else the garbage collector may end up collecting objects that are still reachable.
     unsafe fn trace(&self, trc: *mut JSTracer);
 }
 

components/script/dom/dedicatedworkerglobalscope.rs

@@ -55,8 +55,8 @@ use crate::fetch::load_whole_resource;
 use crate::realms::{enter_realm, AlreadyInRealm, InRealm};
 use crate::script_runtime::ScriptThreadEventCategory::WorkerEvent;
 use crate::script_runtime::{
-    new_child_runtime, CanGc, CommonScriptMsg, JSContext as SafeJSContext, Runtime, ScriptChan,
-    ScriptPort, ThreadSafeJSContext,
+    CanGc, CommonScriptMsg, JSContext as SafeJSContext, Runtime, ScriptChan, ScriptPort,
+    ThreadSafeJSContext,
 };
 use crate::task_queue::{QueuedTask, QueuedTaskConversion, TaskQueue};
 use crate::task_source::networking::NetworkingTaskSource;
@@ -381,7 +381,7 @@ impl DedicatedWorkerGlobalScope {
                         }),
                         pipeline_id,
                     );
-                    new_child_runtime(parent, Some(task_source))
+                    Runtime::new_with_parent(Some(parent), Some(task_source))
                 };
 
                 let context_for_interrupt = runtime.thread_safe_js_context();

components/script/dom/serviceworkerglobalscope.rs

@@ -45,8 +45,7 @@ use crate::dom::workerglobalscope::WorkerGlobalScope;
 use crate::fetch::load_whole_resource;
 use crate::realms::{enter_realm, AlreadyInRealm, InRealm};
 use crate::script_runtime::{
-    new_rt_and_cx, CanGc, CommonScriptMsg, JSContext as SafeJSContext, Runtime, ScriptChan,
-    ThreadSafeJSContext,
+    CanGc, CommonScriptMsg, JSContext as SafeJSContext, Runtime, ScriptChan, ThreadSafeJSContext,
 };
 use crate::task_queue::{QueuedTask, QueuedTaskConversion, TaskQueue};
 use crate::task_source::TaskSourceName;
@@ -310,7 +309,7 @@ impl ServiceWorkerGlobalScope {
             .name(format!("SW:{}", script_url.debug_compact()))
             .spawn(move || {
                 thread_state::initialize(ThreadState::SCRIPT | ThreadState::IN_WORKER);
-                let runtime = new_rt_and_cx(None);
+                let runtime = Runtime::new(None);
                 let context_for_interrupt = runtime.thread_safe_js_context();
                 let _ = context_sender.send(context_for_interrupt);
 

components/script/dom/workerglobalscope.rs

@@ -54,9 +54,7 @@ use crate::dom::workerlocation::WorkerLocation;
 use crate::dom::workernavigator::WorkerNavigator;
 use crate::fetch;
 use crate::realms::{enter_realm, InRealm};
-use crate::script_runtime::{
-    get_reports, CanGc, CommonScriptMsg, JSContext, Runtime, ScriptChan, ScriptPort,
-};
+use crate::script_runtime::{CanGc, CommonScriptMsg, JSContext, Runtime, ScriptChan, ScriptPort};
 use crate::task::TaskCanceller;
 use crate::task_source::dom_manipulation::DOMManipulationTaskSource;
 use crate::task_source::file_reading::FileReadingTaskSource;
@@ -539,8 +537,7 @@ impl WorkerGlobalScope {
             CommonScriptMsg::Task(_, task, _, _) => task.run_box(),
             CommonScriptMsg::CollectReports(reports_chan) => {
                 let cx = self.get_cx();
-                let path_seg = format!("url({})", self.get_url());
-                let reports = unsafe { get_reports(*cx, path_seg) };
+                let reports = cx.get_reports(format!("url({})", self.get_url()));
                 reports_chan.send(reports);
             },
         }

components/script/dom/worklet.rs

@@ -49,7 +49,7 @@ use crate::dom::workletglobalscope::{
 };
 use crate::fetch::load_whole_resource;
 use crate::realms::InRealm;
-use crate::script_runtime::{new_rt_and_cx, CommonScriptMsg, Runtime, ScriptThreadEventCategory};
+use crate::script_runtime::{CommonScriptMsg, Runtime, ScriptThreadEventCategory};
 use crate::script_thread::{MainThreadScriptMsg, ScriptThread};
 use crate::task::TaskBox;
 use crate::task_source::TaskSourceName;
@@ -490,7 +490,7 @@ impl WorkletThread {
                     global_init: init.global_init,
                     global_scopes: HashMap::new(),
                     control_buffer: None,
-                    runtime: new_rt_and_cx(None),
+                    runtime: Runtime::new(None),
                     should_gc: false,
                     gc_threshold: MIN_GC_THRESHOLD,
                 });