Introduce a new type MaybeUnreflectedDom<T> (fixes #25701)

components/domobject_derive/lib.rs

@@ -58,7 +58,7 @@ fn expand_dom_object(input: syn::DeriveInput) -> proc_macro2::TokenStream {
         }
 
         impl #impl_generics crate::dom::bindings::reflector::MutDomObject for #name #ty_generics #where_clause {
-            fn init_reflector(&mut self, obj: *mut js::jsapi::JSObject) {
+            unsafe fn init_reflector(&self, obj: *mut js::jsapi::JSObject) {
                 self.#first_field_name.init_reflector(obj);
             }
         }

components/script/dom/bindings/codegen/CodegenRust.py

@@ -2632,35 +2632,6 @@ class CGConstructorEnabled(CGAbstractMethod):
         return CGList((CGGeneric(cond) for cond in conditions), " &&\n")
 
 
-def CreateBindingJSObject(descriptor):
-    assert not descriptor.isGlobal()
-    create = "let raw = Box::into_raw(object);\nlet _rt = RootedTraceable::new(&*raw);\n"
-    if descriptor.proxy:
-        create += """
-let handler = RegisterBindings::PROXY_HANDLERS[PrototypeList::Proxies::%s as usize];
-rooted!(in(*cx) let private = PrivateValue(raw as *const libc::c_void));
-let obj = NewProxyObject(*cx, handler,
-                         Handle::from_raw(UndefinedHandleValue),
-                         proto.get());
-assert!(!obj.is_null());
-SetProxyReservedSlot(obj, 0, &private.get());
-rooted!(in(*cx) let obj = obj);\
-""" % (descriptor.name)
-    else:
-        create += ("rooted!(in(*cx) let obj = JS_NewObjectWithGivenProto(\n"
-                   "    *cx, &Class.base as *const JSClass, proto.handle()));\n"
-                   "assert!(!obj.is_null());\n"
-                   "\n"
-                   "let val = PrivateValue(raw as *const libc::c_void);\n"
-                   "\n"
-                   "JS_SetReservedSlot(obj.get(), DOM_OBJECT_SLOT, &val);")
-    if descriptor.weakReferenceable:
-        create += """
-let val = PrivateValue(ptr::null());
-JS_SetReservedSlot(obj.get(), DOM_WEAK_SLOT, &val);"""
-    return create
-
-
 def InitUnforgeablePropertiesOnHolder(descriptor, properties):
     """
     Define the unforgeable properties on the unforgeable holder for
@@ -2738,23 +2709,62 @@ class CGWrapMethod(CGAbstractMethod):
 
     def definition_body(self):
         unforgeable = CopyUnforgeablePropertiesToInstance(self.descriptor)
-        create = CreateBindingJSObject(self.descriptor)
+        if self.descriptor.proxy:
+            create = """
+let handler = RegisterBindings::PROXY_HANDLERS[PrototypeList::Proxies::%(concreteType)s as usize];
+rooted!(in(*cx) let obj = NewProxyObject(
+    *cx,
+    handler,
+    Handle::from_raw(UndefinedHandleValue),
+    proto.get(),
+));
+assert!(!obj.is_null());
+SetProxyReservedSlot(
+    obj.get(),
+    0,
+    &PrivateValue(&*raw as *const %(concreteType)s as *const libc::c_void),
+);
+"""
+        else:
+            create = """
+rooted!(in(*cx) let obj = JS_NewObjectWithGivenProto(
+    *cx,
+    &Class.base,
+    proto.handle(),
+));
+assert!(!obj.is_null());
+JS_SetReservedSlot(
+    obj.get(),
+    DOM_OBJECT_SLOT,
+    &PrivateValue(&*raw as *const %(concreteType)s as *const libc::c_void),
+);
+"""
+        create = create % {"concreteType": self.descriptor.concreteType}
+        if self.descriptor.weakReferenceable:
+            create += """
+let val = PrivateValue(ptr::null());
+JS_SetReservedSlot(obj.get(), DOM_WEAK_SLOT, &val);
+"""
+
         return CGGeneric("""\
+let raw = Root::new(MaybeUnreflectedDom::from_box(object));
+
 let scope = scope.reflector().get_jsobject();
 assert!(!scope.get().is_null());
 assert!(((*get_object_class(scope.get())).flags & JSCLASS_IS_GLOBAL) != 0);
+let _ac = JSAutoRealm::new(*cx, scope.get());
 
 rooted!(in(*cx) let mut proto = ptr::null_mut::<JSObject>());
-let _ac = JSAutoRealm::new(*cx, scope.get());
 GetProtoObject(cx, scope, proto.handle_mut());
 assert!(!proto.is_null());
 
 %(createObject)s
+raw.init_reflector(obj.get());
 
 %(copyUnforgeable)s
-(*raw).init_reflector(obj.get());
 
-DomRoot::from_ref(&*raw)""" % {'copyUnforgeable': unforgeable, 'createObject': create})
+DomRoot::from_ref(&*raw)\
+""" % {'copyUnforgeable': unforgeable, 'createObject': create})
 
 
 class CGWrapGlobalMethod(CGAbstractMethod):
@@ -2773,6 +2783,7 @@ class CGWrapGlobalMethod(CGAbstractMethod):
 
     def definition_body(self):
         values = {
+            "concreteType": self.descriptor.concreteType,
             "unforgeable": CopyUnforgeablePropertiesToInstance(self.descriptor)
         }
 
@@ -2786,19 +2797,18 @@ class CGWrapGlobalMethod(CGAbstractMethod):
         values["members"] = "\n".join(members)
 
         return CGGeneric("""\
-let raw = Box::into_raw(object);
-let _rt = RootedTraceable::new(&*raw);
+let raw = Root::new(MaybeUnreflectedDom::from_box(object));
 
 rooted!(in(*cx) let mut obj = ptr::null_mut::<JSObject>());
 create_global_object(
     cx,
     &Class.base,
-    raw as *const libc::c_void,
+    &*raw as *const %(concreteType)s as *const libc::c_void,
     _trace,
     obj.handle_mut());
 assert!(!obj.is_null());
 
-(*raw).init_reflector(obj.get());
+raw.init_reflector(obj.get());
 
 let _ac = JSAutoRealm::new(*cx, obj.get());
 rooted!(in(*cx) let mut proto = ptr::null_mut::<JSObject>());
@@ -6060,8 +6070,10 @@ def generate_imports(config, cgthings, descriptors, callbacks=None, dictionaries
         'crate::dom::bindings::reflector::DomObject',
         'crate::dom::bindings::root::Dom',
         'crate::dom::bindings::root::DomRoot',
-        'crate::dom::bindings::root::OptionalHeapSetter',
         'crate::dom::bindings::root::DomSlice',
+        'crate::dom::bindings::root::MaybeUnreflectedDom',
+        'crate::dom::bindings::root::OptionalHeapSetter',
+        'crate::dom::bindings::root::Root',
         'crate::dom::bindings::utils::AsVoidPtr',
         'crate::dom::bindings::utils::DOMClass',
         'crate::dom::bindings::utils::DOMJSClass',
@@ -6086,7 +6098,6 @@ def generate_imports(config, cgthings, descriptors, callbacks=None, dictionaries
         'crate::dom::bindings::utils::set_dictionary_property',
         'crate::dom::bindings::utils::trace_global',
         'crate::dom::bindings::trace::JSTraceable',
-        'crate::dom::bindings::trace::RootedTraceable',
         'crate::dom::bindings::trace::RootedTraceableBox',
         'crate::dom::bindings::callback::CallSetup',
         'crate::dom::bindings::callback::CallbackContainer',

components/script/dom/bindings/reflector.rs

@@ -6,6 +6,7 @@
 
 use crate::dom::bindings::conversions::DerivedFrom;
 use crate::dom::bindings::root::DomRoot;
+use crate::dom::bindings::trace::JSTraceable;
 use crate::dom::globalscope::GlobalScope;
 use crate::script_runtime::JSContext;
 use js::jsapi::{Heap, JSObject};
@@ -53,7 +54,7 @@ impl Reflector {
     }
 
     /// Initialize the reflector. (May be called only once.)
-    pub fn set_jsobject(&mut self, object: *mut JSObject) {
+    pub unsafe fn set_jsobject(&self, object: *mut JSObject) {
         assert!(self.object.get().is_null());
         assert!(!object.is_null());
         self.object.set(object);
@@ -75,7 +76,7 @@ impl Reflector {
 }
 
 /// A trait to provide access to the `Reflector` for a DOM object.
-pub trait DomObject: 'static {
+pub trait DomObject: JSTraceable + 'static {
     /// Returns the receiver's reflector.
     fn reflector(&self) -> &Reflector;
 
@@ -97,11 +98,11 @@ impl DomObject for Reflector {
 /// A trait to initialize the `Reflector` for a DOM object.
 pub trait MutDomObject: DomObject {
     /// Initializes the Reflector
-    fn init_reflector(&mut self, obj: *mut JSObject);
+    unsafe fn init_reflector(&self, obj: *mut JSObject);
 }
 
 impl MutDomObject for Reflector {
-    fn init_reflector(&mut self, obj: *mut JSObject) {
+    unsafe fn init_reflector(&self, obj: *mut JSObject) {
         self.set_jsobject(obj)
     }
 }

components/script/dom/bindings/root.rs

@@ -62,7 +62,7 @@ where
     /// It cannot outlive its associated `RootCollection`, and it gives
     /// out references which cannot outlive this new `Root`.
     #[allow(unrooted_must_root)]
-    unsafe fn new(value: T) -> Self {
+    pub unsafe fn new(value: T) -> Self {
         debug_assert!(thread_state::get().is_script());
         STACK_ROOTS.with(|ref root_list| {
             let root_list = &*root_list.get().unwrap();
@@ -99,6 +99,32 @@ where
     }
 }
 
+unsafe impl<T> StableTraceObject for MaybeUnreflectedDom<T>
+where
+    T: DomObject,
+{
+    fn stable_trace_object<'a>(&'a self) -> *const dyn JSTraceable {
+        // The JSTraceable impl for Reflector doesn't actually do anything,
+        // so we need this shenanigan to actually trace the reflector of the
+        // T pointer in Dom<T>.
+        #[allow(unrooted_must_root)]
+        struct MaybeUnreflectedStackRoot<T>(T);
+        unsafe impl<T> JSTraceable for MaybeUnreflectedStackRoot<T>
+        where
+            T: DomObject,
+        {
+            unsafe fn trace(&self, tracer: *mut JSTracer) {
+                if self.0.reflector().get_jsobject().is_null() {
+                    self.0.trace(tracer);
+                } else {
+                    trace_reflector(tracer, "on stack", &self.0.reflector());
+                }
+            }
+        }
+        unsafe { &*(self.ptr.as_ptr() as *const T as *const MaybeUnreflectedStackRoot<T>) }
+    }
+}
+
 impl<T> Deref for Root<T>
 where
     T: Deref + StableTraceObject,
@@ -341,6 +367,36 @@ unsafe impl<T: DomObject> JSTraceable for Dom<T> {
     }
 }
 
+/// A traced reference to a DOM object that may not be reflected yet.
+#[unrooted_must_root_lint::must_root]
+pub struct MaybeUnreflectedDom<T> {
+    ptr: ptr::NonNull<T>,
+}
+
+impl<T> MaybeUnreflectedDom<T>
+where
+    T: DomObject,
+{
+    #[allow(unrooted_must_root)]
+    pub unsafe fn from_box(value: Box<T>) -> Self {
+        Self {
+            ptr: Box::into_raw_non_null(value),
+        }
+    }
+}
+
+impl<T> Deref for MaybeUnreflectedDom<T>
+where
+    T: DomObject,
+{
+    type Target = T;
+
+    fn deref(&self) -> &T {
+        debug_assert!(thread_state::get().is_script());
+        unsafe { &*self.ptr.as_ptr() }
+    }
+}
+
 /// An unrooted reference to a DOM object for use in layout. `Layout*Helpers`
 /// traits must be implemented on this.
 #[unrooted_must_root_lint::allow_unrooted_interior]

components/script/dom/bindings/trace.rs

@@ -910,35 +910,6 @@ impl RootedTraceableSet {
     }
 }
 
-/// Roots any JSTraceable thing
-///
-/// If you have a valid DomObject, use DomRoot.
-/// If you have GC things like *mut JSObject or JSVal, use rooted!.
-/// If you have an arbitrary number of DomObjects to root, use rooted_vec!.
-/// If you know what you're doing, use this.
-#[derive(JSTraceable)]
-pub struct RootedTraceable<'a, T: 'static + JSTraceable> {
-    ptr: &'a T,
-}
-
-impl<'a, T: JSTraceable + 'static> RootedTraceable<'a, T> {
-    /// DomRoot a JSTraceable thing for the life of this RootedTraceable
-    pub fn new(traceable: &'a T) -> RootedTraceable<'a, T> {
-        unsafe {
-            RootedTraceableSet::add(traceable);
-        }
-        RootedTraceable { ptr: traceable }
-    }
-}
-
-impl<'a, T: JSTraceable + 'static> Drop for RootedTraceable<'a, T> {
-    fn drop(&mut self) {
-        unsafe {
-            RootedTraceableSet::remove(self.ptr);
-        }
-    }
-}
-
 /// Roots any JSTraceable thing
 ///
 /// If you have a valid DomObject, use DomRoot.
@@ -957,12 +928,12 @@ unsafe impl<T: JSTraceable + 'static> JSTraceable for RootedTraceableBox<T> {
 }
 
 impl<T: JSTraceable + 'static> RootedTraceableBox<T> {
-    /// DomRoot a JSTraceable thing for the life of this RootedTraceable
+    /// DomRoot a JSTraceable thing for the life of this RootedTraceableBox
     pub fn new(traceable: T) -> RootedTraceableBox<T> {
         Self::from_box(Box::new(traceable))
     }
 
-    /// Consumes a boxed JSTraceable and roots it for the life of this RootedTraceable.
+    /// Consumes a boxed JSTraceable and roots it for the life of this RootedTraceableBox.
     pub fn from_box(boxed_traceable: Box<T>) -> RootedTraceableBox<T> {
         let traceable = Box::into_raw(boxed_traceable);
         unsafe {

components/script/dom/promise.rs

@@ -109,8 +109,8 @@ impl Promise {
                 reflector: Reflector::new(),
                 permanent_js_root: Heap::default(),
             };
-            let mut promise = Rc::new(promise);
-            Rc::get_mut(&mut promise).unwrap().init_reflector(obj.get());
+            let promise = Rc::new(promise);
+            promise.init_reflector(obj.get());
             promise.initialize(cx);
             promise
         }

components/script/dom/windowproxy.rs

@@ -166,7 +166,7 @@ impl WindowProxy {
 
             // Create a new browsing context.
             let current = Some(window.global().pipeline_id());
-            let mut window_proxy = Box::new(WindowProxy::new_inherited(
+            let window_proxy = Box::new(WindowProxy::new_inherited(
                 browsing_context_id,
                 top_level_browsing_context_id,
                 current,
@@ -212,7 +212,7 @@ impl WindowProxy {
             let cx = global_to_clone_from.get_cx();
 
             // Create a new browsing context.
-            let mut window_proxy = Box::new(WindowProxy::new_inherited(
+            let window_proxy = Box::new(WindowProxy::new_inherited(
                 browsing_context_id,
                 top_level_browsing_context_id,
                 None,

components/script/lib.rs

@@ -2,6 +2,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
+#![feature(box_into_raw_non_null)]
 #![feature(const_fn)]
 #![feature(core_intrinsics)]
 #![feature(drain_filter)]