script: Reduce usage of Trusted in Node::insert. (#37762)

These changes introduce a new kind of task that uses a variation of the
`task!` syntax. Existing `task!` usages create task structs that have a
`Send` bound, which requires the use of `Trusted<T>` to reference a DOM
object T inside of the task closure. The new syntax replaces the `Send`
bound with a `JSTraceable` bound, which requires explicit capture
clauses with types. This looks like:
```rust
task!(ScriptPrepare: {script: DomRoot<HTMLScriptElement>} |script| {
    script.prepare(CanGc::note());
}),
```

The capture clauses must list every value that will be referenced from
the closure's environment—these values are moved into fields in the
generated task structure, which allows them to be traced by the GC as
part of a generated JSTraceable implementation. Since the closure itself
is not a `move` closure, any attempts to reference values not explicitly
captured will generate a borrow checker error since the closure requires
the `'static` lifetime.

Testing: Existing WPT tests exercise these code paths. I also attempted
to write incorrect tasks that capture references or use values not
explicitly captured, and the compiler correctly errors out.
Fixes: part of #35517

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

components/script/dom/document.rs

@@ -212,7 +212,7 @@ use crate::realms::{AlreadyInRealm, InRealm, enter_realm};
 use crate::script_runtime::{CanGc, ScriptThreadEventCategory};
 use crate::script_thread::{ScriptThread, with_script_thread};
 use crate::stylesheet_set::StylesheetSetRef;
-use crate::task::TaskBox;
+use crate::task::NonSendTaskBox;
 use crate::task_source::TaskSourceName;
 use crate::timers::OneshotTimerCallback;
 
@@ -477,7 +477,7 @@ pub(crate) struct Document {
     script_and_layout_blockers: Cell<u32>,
     /// List of tasks to execute as soon as last script/layout blocker is removed.
     #[ignore_malloc_size_of = "Measuring trait objects is hard"]
-    delayed_tasks: DomRefCell<Vec<Box<dyn TaskBox>>>,
+    delayed_tasks: DomRefCell<Vec<Box<dyn NonSendTaskBox>>>,
     /// <https://html.spec.whatwg.org/multipage/#completely-loaded>
     completely_loaded: Cell<bool>,
     /// Set of shadow roots connected to the document tree.
@@ -4338,7 +4338,7 @@ impl Document {
     }
 
     /// Enqueue a task to run as soon as any JS and layout blockers are removed.
-    pub(crate) fn add_delayed_task<T: 'static + TaskBox>(&self, task: T) {
+    pub(crate) fn add_delayed_task<T: 'static + NonSendTaskBox>(&self, task: T) {
         self.delayed_tasks.borrow_mut().push(Box::new(task));
     }
 

components/script/dom/htmlscriptelement.rs

@@ -1414,15 +1414,15 @@ impl VirtualMethods for HTMLScriptElement {
         }
 
         if self.upcast::<Node>().is_connected() && !self.parser_inserted.get() {
-            let script = Trusted::new(self);
+            let script = DomRoot::from_ref(self);
             // This method can be invoked while there are script/layout blockers present
             // as DOM mutations have not yet settled. We use a delayed task to avoid
             // running any scripts until the DOM tree is safe for interactions.
-            self.owner_document()
+            self.owner_document().add_delayed_task(
-                .add_delayed_task(task!(ScriptPrepare: move || {
+                task!(ScriptPrepare: |script: DomRoot<HTMLScriptElement>| {
-                    let this = script.root();
+                    script.prepare(CanGc::note());
-                    this.prepare(CanGc::note());
+                }),
-                }));
+            );
         }
     }
 

components/script/dom/node.rs

@@ -82,7 +82,6 @@ use crate::dom::bindings::inheritance::{
     Castable, CharacterDataTypeId, ElementTypeId, EventTargetTypeId, HTMLElementTypeId, NodeTypeId,
     SVGElementTypeId, SVGGraphicsElementTypeId, TextTypeId,
 };
-use crate::dom::bindings::refcounted::Trusted;
 use crate::dom::bindings::reflector::{DomObject, DomObjectWrap, reflect_dom_object_with_proto};
 use crate::dom::bindings::root::{Dom, DomRoot, DomSlice, LayoutDom, MutNullableDom, ToLayout};
 use crate::dom::bindings::str::{DOMString, USVString};
@@ -2563,10 +2562,7 @@ impl Node {
         for node in new_nodes {
             // Step 11.1 For each shadow-including inclusive descendant inclusiveDescendant of node,
             //           in shadow-including tree order, append inclusiveDescendant to staticNodeList.
-            static_node_list.extend(
+            static_node_list.extend(node.traverse_preorder(ShadowIncluding::Yes));
-                node.traverse_preorder(ShadowIncluding::Yes)
-                    .map(|n| Trusted::new(&*n)),
-            );
         }
 
         // We use a delayed task for this step to work around an awkward interaction between
@@ -2579,13 +2575,15 @@ impl Node {
         // 2) post_connection_steps from Node::insert,
         // we use a delayed task that will run as soon as Node::insert removes its
         // script/layout blocker.
-        parent_document.add_delayed_task(task!(PostConnectionSteps: move || {
+        parent_document.add_delayed_task(
-            // Step 12. For each node of staticNodeList, if node is connected, then run the
+            task!(PostConnectionSteps: |static_node_list: Vec<DomRoot<Node>>| {
-            //          post-connection steps with node.
+                // Step 12. For each node of staticNodeList, if node is connected, then run the
-            for node in static_node_list.iter().map(Trusted::root).filter(|n| n.is_connected()) {
+                //          post-connection steps with node.
-                vtable_for(&node).post_connection_steps();
+                for node in static_node_list.iter().filter(|n| n.is_connected()) {
-            }
+                    vtable_for(node).post_connection_steps();
-        }));
+                }
+            }),
+        );
 
         parent_document.remove_script_and_layout_blocker();
         from_document.remove_script_and_layout_blocker();

components/script/dom/userscripts.rs

@@ -5,12 +5,13 @@
 use std::rc::Rc;
 
 use js::jsval::UndefinedValue;
+use script_bindings::root::DomRoot;
 
-use crate::dom::bindings::refcounted::Trusted;
 use crate::dom::bindings::str::DOMString;
 use crate::dom::htmlheadelement::HTMLHeadElement;
 use crate::dom::htmlscriptelement::SourceCode;
 use crate::dom::node::NodeTraits;
+use crate::dom::window::Window;
 use crate::script_module::ScriptFetchOptions;
 use crate::script_runtime::CanGc;
 
@@ -20,9 +21,8 @@ pub(crate) fn load_script(head: &HTMLHeadElement) {
     if userscripts.is_empty() {
         return;
     }
-    let window = Trusted::new(doc.window());
+    let win = DomRoot::from_ref(doc.window());
-    doc.add_delayed_task(task!(UserScriptExecute: move || {
+    doc.add_delayed_task(task!(UserScriptExecute: |win: DomRoot<Window>| {
-        let win = window.root();
         let cx = win.get_cx();
         rooted!(in(*cx) let mut rval = UndefinedValue());
 

components/script/task.rs

@@ -9,6 +9,36 @@ use std::sync::Arc;
 use std::sync::atomic::{AtomicBool, Ordering};
 
 macro_rules! task {
+    ($name:ident: |$($field:ident: $field_type:ty$(,)*)*| $body:tt) => {{
+        #[allow(non_camel_case_types)]
+        struct $name<F> {
+            $($field: $field_type,)*
+            task: F,
+        }
+        #[allow(unsafe_code)]
+        unsafe impl<F> crate::JSTraceable for $name<F> {
+            #[allow(unsafe_code)]
+            unsafe fn trace(&self, tracer: *mut ::js::jsapi::JSTracer) {
+                $(self.$field.trace(tracer);)*
+                // We cannot trace the actual task closure. This is safe because
+                // all referenced values from within the closure are either borrowed
+                // or moved into fields in the struct (and therefore traced).
+            }
+        }
+        impl<F> crate::task::NonSendTaskOnce for $name<F>
+        where
+            F: ::std::ops::FnOnce($($field_type,)*),
+        {
+            fn run_once(self) {
+                (self.task)($(self.$field,)*);
+            }
+        }
+        $name {
+            $($field,)*
+            task: |$($field: $field_type,)*| $body,
+        }
+    }};
+
     ($name:ident: move || $body:tt) => {{
         #[allow(non_camel_case_types)]
         struct $name<F>(F);
@@ -28,9 +58,9 @@ macro_rules! task {
     }};
 }
 
-/// A task that can be run. The name method is for profiling purposes.
+/// A task that can be sent between threads and run.
+/// The name method is for profiling purposes.
 pub(crate) trait TaskOnce: Send {
-    #[allow(unsafe_code)]
     fn name(&self) -> &'static str {
         ::std::any::type_name::<Self>()
     }
@@ -38,6 +68,11 @@ pub(crate) trait TaskOnce: Send {
     fn run_once(self);
 }
 
+/// A task that must be run on the same thread it originated in.
+pub(crate) trait NonSendTaskOnce: crate::JSTraceable {
+    fn run_once(self);
+}
+
 /// A boxed version of `TaskOnce`.
 pub(crate) trait TaskBox: Send {
     fn name(&self) -> &'static str;
@@ -45,6 +80,20 @@ pub(crate) trait TaskBox: Send {
     fn run_box(self: Box<Self>);
 }
 
+/// A boxed version of `NonSendTaskOnce`.
+pub(crate) trait NonSendTaskBox: crate::JSTraceable {
+    fn run_box(self: Box<Self>);
+}
+
+impl<T> NonSendTaskBox for T
+where
+    T: NonSendTaskOnce,
+{
+    fn run_box(self: Box<Self>) {
+        self.run_once()
+    }
+}
+
 impl<T> TaskBox for T
 where
     T: TaskOnce,