script: Add generic root storage to StructuredCloneReader. (#36640)

This reduces the boilerplate necessary for adding new
serializable/transferable interfaces to the structured cloning code. We
always need to root the deserialized objects when performing a read
operation, but we don't actually need the concrete object types in the
majority of cases. By storing a list of rooted JS object values, we can
push generic reflector objects into it, and extract the types we need
(MessagePort) at the very end.

Testing: Existing WPT structured cloning tests will provide coverage.

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

components/script/dom/bindings/serializable.rs

@@ -11,7 +11,7 @@ use base::id::{Index, NamespaceIndex, PipelineNamespaceId};
 
 use crate::dom::bindings::reflector::DomObject;
 use crate::dom::bindings::root::DomRoot;
-use crate::dom::bindings::structuredclone::{StructuredData, StructuredDataReader};
+use crate::dom::bindings::structuredclone::StructuredData;
 use crate::dom::globalscope::GlobalScope;
 use crate::script_runtime::CanGc;
 
@@ -65,13 +65,7 @@ where
 
     /// Returns the field of [StructuredDataReader]/[StructuredDataWriter] that
     /// should be used to read/store serialized instances of this type.
-    fn serialized_storage(
-        data: StructuredData<'_>,
-    ) -> &mut Option<HashMap<NamespaceIndex<Self::Index>, Self::Data>>;
-
-    /// Returns the field of [StructuredDataReader] that should be used to store
-    /// deserialized instances of this type.
-    fn deserialized_storage(
-        reader: &mut StructuredDataReader,
-    ) -> &mut Option<HashMap<StorageKey, DomRoot<Self>>>;
+    fn serialized_storage<'a>(
+        data: StructuredData<'a, '_>,
+    ) -> &'a mut Option<HashMap<NamespaceIndex<Self::Index>, Self::Data>>;
 }

components/script/dom/bindings/structuredclone.rs

@@ -16,13 +16,14 @@ use constellation_traits::{
     BlobImpl, DomException, DomPoint, MessagePortImpl, Serializable as SerializableInterface,
     StructuredSerializedData, Transferrable as TransferrableInterface,
 };
+use js::gc::RootedVec;
 use js::glue::{
     CopyJSStructuredCloneData, DeleteJSAutoStructuredCloneBuffer, GetLengthOfJSStructuredCloneData,
     NewJSAutoStructuredCloneBuffer, WriteBytesToJSStructuredCloneData,
 };
 use js::jsapi::{
-    CloneDataPolicy, HandleObject as RawHandleObject, JS_ClearPendingException, JS_ReadUint32Pair,
-    JS_STRUCTURED_CLONE_VERSION, JS_WriteUint32Pair, JSContext, JSObject,
+    CloneDataPolicy, HandleObject as RawHandleObject, Heap, JS_ClearPendingException,
+    JS_ReadUint32Pair, JS_STRUCTURED_CLONE_VERSION, JS_WriteUint32Pair, JSContext, JSObject,
     JSStructuredCloneCallbacks, JSStructuredCloneReader, JSStructuredCloneWriter,
     MutableHandleObject as RawMutableHandleObject, StructuredCloneScope, TransferableOwnership,
 };
@@ -93,7 +94,7 @@ fn reader_for_type(
 ) -> unsafe fn(
     &GlobalScope,
     *mut JSStructuredCloneReader,
-    &mut StructuredDataReader,
+    &mut StructuredDataReader<'_>,
     CanGc,
 ) -> *mut JSObject {
     match val {
@@ -107,7 +108,7 @@ fn reader_for_type(
 unsafe fn read_object<T: Serializable>(
     owner: &GlobalScope,
     r: *mut JSStructuredCloneReader,
-    sc_reader: &mut StructuredDataReader,
+    sc_reader: &mut StructuredDataReader<'_>,
     can_gc: CanGc,
 ) -> *mut JSObject {
     let mut name_space: u32 = 0;
@@ -136,9 +137,8 @@ unsafe fn read_object<T: Serializable>(
     }
 
     if let Ok(obj) = T::deserialize(owner, serialized, can_gc) {
-        let destination = T::deserialized_storage(sc_reader).get_or_insert_with(HashMap::new);
         let reflector = obj.reflector().get_jsobject().get();
-        destination.insert(storage_key, obj);
+        sc_reader.roots.push(Heap::boxed(reflector));
         return reflector;
     }
     warn!("Reading structured data failed in {:?}.", owner.get_url());
@@ -191,7 +191,7 @@ unsafe extern "C" fn read_callback(
         "tag should be higher than StructuredCloneTags::Min"
     );
 
-    let sc_reader = &mut *(closure as *mut StructuredDataReader);
+    let sc_reader = &mut *(closure as *mut StructuredDataReader<'_>);
     let in_realm_proof = AlreadyInRealm::assert_for_cx(SafeJSContext::from_ptr(cx));
     let global = GlobalScope::from_context(cx, InRealm::Already(&in_realm_proof));
     for serializable in SerializableInterface::iter() {
@@ -259,7 +259,8 @@ unsafe extern "C" fn write_callback(
 
 fn receiver_for_type(
     val: TransferrableInterface,
-) -> fn(&GlobalScope, &mut StructuredDataReader, u64, RawMutableHandleObject) -> Result<(), ()> {
+) -> fn(&GlobalScope, &mut StructuredDataReader<'_>, u64, RawMutableHandleObject) -> Result<(), ()>
+{
     match val {
         TransferrableInterface::MessagePort => receive_object::<MessagePort>,
         TransferrableInterface::ReadableStream => receive_object::<ReadableStream>,
@@ -269,7 +270,7 @@ fn receiver_for_type(
 
 fn receive_object<T: Transferable>(
     owner: &GlobalScope,
-    sc_reader: &mut StructuredDataReader,
+    sc_reader: &mut StructuredDataReader<'_>,
     extra_data: u64,
     return_object: RawMutableHandleObject,
 ) -> Result<(), ()> {
@@ -305,13 +306,12 @@ fn receive_object<T: Transferable>(
         );
     };
 
-    if let Ok(received) = T::transfer_receive(owner, id, serialized) {
-        return_object.set(received.reflector().rootable().get());
-        let storage = T::deserialized_storage(sc_reader).get_or_insert_with(Vec::new);
-        storage.push(received);
-        return Ok(());
-    }
-    Err(())
+    let Ok(received) = T::transfer_receive(owner, id, serialized) else {
+        return Err(());
+    };
+    return_object.set(received.reflector().rootable().get());
+    sc_reader.roots.push(Heap::boxed(return_object.get()));
+    Ok(())
 }
 
 unsafe extern "C" fn read_transfer_callback(
@@ -324,7 +324,7 @@ unsafe extern "C" fn read_transfer_callback(
     closure: *mut raw::c_void,
     return_object: RawMutableHandleObject,
 ) -> bool {
-    let sc_reader = &mut *(closure as *mut StructuredDataReader);
+    let sc_reader = &mut *(closure as *mut StructuredDataReader<'_>);
     let in_realm_proof = AlreadyInRealm::assert_for_cx(SafeJSContext::from_ptr(cx));
     let owner = GlobalScope::from_context(cx, InRealm::Already(&in_realm_proof));
 
@@ -489,8 +489,8 @@ static STRUCTURED_CLONE_CALLBACKS: JSStructuredCloneCallbacks = JSStructuredClon
     sabCloned: Some(sab_cloned_callback),
 };
 
-pub(crate) enum StructuredData<'a> {
-    Reader(&'a mut StructuredDataReader),
+pub(crate) enum StructuredData<'a, 'b> {
+    Reader(&'a mut StructuredDataReader<'b>),
     Writer(&'a mut StructuredDataWriter),
 }
 
@@ -503,19 +503,11 @@ pub(crate) struct DOMErrorRecord {
 /// Reader and writer structs for results from, and inputs to, structured-data read/write operations.
 /// <https://html.spec.whatwg.org/multipage/#safe-passing-of-structured-data>
 #[repr(C)]
-pub(crate) struct StructuredDataReader {
+pub(crate) struct StructuredDataReader<'a> {
     /// A struct of error message.
-    pub(crate) errors: DOMErrorRecord,
-    /// A map of deserialized blobs, stored temporarily here to keep them rooted.
-    pub(crate) blobs: Option<HashMap<StorageKey, DomRoot<Blob>>>,
-    /// A map of deserialized points, stored temporarily here to keep them rooted.
-    pub(crate) points_read_only: Option<HashMap<StorageKey, DomRoot<DOMPointReadOnly>>>,
-    pub(crate) dom_points: Option<HashMap<StorageKey, DomRoot<DOMPoint>>>,
-    /// A map of deserialized exceptions, stored temporarily here to keep them rooted.
-    pub(crate) dom_exceptions: Option<HashMap<StorageKey, DomRoot<DOMException>>>,
-    /// A vec of transfer-received DOM ports,
-    /// to be made available to script through a message event.
-    pub(crate) message_ports: Option<Vec<DomRoot<MessagePort>>>,
+    errors: DOMErrorRecord,
+    /// Rooted copies of every deserialized object to ensure they are not garbage collected.
+    roots: RootedVec<'a, Box<Heap<*mut JSObject>>>,
     /// A map of port implementations,
     /// used as part of the "transfer-receiving" steps of ports,
     /// to produce the DOM ports stored in `message_ports` above.
@@ -528,12 +520,6 @@ pub(crate) struct StructuredDataReader {
     pub(crate) points: Option<HashMap<DomPointId, DomPoint>>,
     /// A map of serialized exceptions.
     pub(crate) exceptions: Option<HashMap<DomExceptionId, DomException>>,
-
-    /// <https://streams.spec.whatwg.org/#rs-transfer>
-    pub(crate) readable_streams: Option<Vec<DomRoot<ReadableStream>>>,
-
-    /// <https://streams.spec.whatwg.org/#ws-transfer>
-    pub(crate) writable_streams: Option<Vec<DomRoot<WritableStream>>>,
 }
 
 /// A data holder for transferred and serialized objects.
@@ -618,19 +604,14 @@ pub(crate) fn read(
 ) -> Fallible<Vec<DomRoot<MessagePort>>> {
     let cx = GlobalScope::get_cx();
     let _ac = enter_realm(global);
+    rooted_vec!(let mut roots);
     let mut sc_reader = StructuredDataReader {
-        blobs: None,
-        message_ports: None,
-        points_read_only: None,
-        dom_points: None,
-        dom_exceptions: None,
+        roots,
         port_impls: data.ports.take(),
         blob_impls: data.blobs.take(),
         points: data.points.take(),
         exceptions: data.exceptions.take(),
         errors: DOMErrorRecord { message: None },
-        readable_streams: None,
-        writable_streams: None,
     };
     let sc_reader_ptr = &mut sc_reader as *mut _;
     unsafe {
@@ -666,8 +647,15 @@ pub(crate) fn read(
 
         DeleteJSAutoStructuredCloneBuffer(scbuf);
 
+        let mut message_ports = vec![];
+        for reflector in sc_reader.roots.iter() {
+            let Ok(message_port) = root_from_object::<MessagePort>(reflector.get(), *cx) else {
+                continue;
+            };
+            message_ports.push(message_port);
+        }
         // Any transfer-received port-impls should have been taken out.
         assert!(sc_reader.port_impls.is_none());
-        Ok(sc_reader.message_ports.take().unwrap_or_default())
+        Ok(message_ports)
     }
 }

components/script/dom/bindings/transferable.rs

@@ -12,7 +12,7 @@ use base::id::NamespaceIndex;
 
 use crate::dom::bindings::reflector::DomObject;
 use crate::dom::bindings::root::DomRoot;
-use crate::dom::bindings::structuredclone::{StructuredData, StructuredDataReader};
+use crate::dom::bindings::structuredclone::StructuredData;
 use crate::dom::globalscope::GlobalScope;
 pub(crate) trait Transferable: DomObject
 where
@@ -32,8 +32,7 @@ where
         serialized: Self::Data,
     ) -> Result<DomRoot<Self>, ()>;
 
-    fn serialized_storage(
-        data: StructuredData<'_>,
-    ) -> &mut Option<HashMap<NamespaceIndex<Self::Index>, Self::Data>>;
-    fn deserialized_storage(reader: &mut StructuredDataReader) -> &mut Option<Vec<DomRoot<Self>>>;
+    fn serialized_storage<'a>(
+        data: StructuredData<'a, '_>,
+    ) -> &'a mut Option<HashMap<NamespaceIndex<Self::Index>, Self::Data>>;
 }

components/script/dom/blob.rs

@@ -24,9 +24,9 @@ use crate::dom::bindings::codegen::UnionTypes::ArrayBufferOrArrayBufferViewOrBlo
 use crate::dom::bindings::error::{Error, Fallible};
 use crate::dom::bindings::reflector::{DomGlobal, Reflector, reflect_dom_object_with_proto};
 use crate::dom::bindings::root::DomRoot;
-use crate::dom::bindings::serializable::{Serializable, StorageKey};
+use crate::dom::bindings::serializable::Serializable;
 use crate::dom::bindings::str::DOMString;
-use crate::dom::bindings::structuredclone::{StructuredData, StructuredDataReader};
+use crate::dom::bindings::structuredclone::StructuredData;
 use crate::dom::globalscope::GlobalScope;
 use crate::dom::promise::Promise;
 use crate::dom::readablestream::ReadableStream;
@@ -119,18 +119,14 @@ impl Serializable for Blob {
         Ok(deserialized_blob)
     }
 
-    fn serialized_storage(reader: StructuredData<'_>) -> &mut Option<HashMap<BlobId, Self::Data>> {
+    fn serialized_storage<'a>(
+        reader: StructuredData<'a, '_>,
+    ) -> &'a mut Option<HashMap<BlobId, Self::Data>> {
         match reader {
             StructuredData::Reader(r) => &mut r.blob_impls,
             StructuredData::Writer(w) => &mut w.blobs,
         }
     }
-
-    fn deserialized_storage(
-        data: &mut StructuredDataReader,
-    ) -> &mut Option<HashMap<StorageKey, DomRoot<Self>>> {
-        &mut data.blobs
-    }
 }
 
 /// Extract bytes from BlobParts, used by Blob and File constructor

components/script/dom/domexception.rs

@@ -17,9 +17,9 @@ use crate::dom::bindings::reflector::{
     Reflector, reflect_dom_object, reflect_dom_object_with_proto,
 };
 use crate::dom::bindings::root::DomRoot;
-use crate::dom::bindings::serializable::{Serializable, StorageKey};
+use crate::dom::bindings::serializable::Serializable;
 use crate::dom::bindings::str::DOMString;
-use crate::dom::bindings::structuredclone::{StructuredData, StructuredDataReader};
+use crate::dom::bindings::structuredclone::StructuredData;
 use crate::dom::globalscope::GlobalScope;
 use crate::script_runtime::CanGc;
 
@@ -252,18 +252,12 @@ impl Serializable for DOMException {
         ))
     }
 
-    fn serialized_storage(
-        data: StructuredData<'_>,
-    ) -> &mut Option<HashMap<DomExceptionId, Self::Data>> {
+    fn serialized_storage<'a>(
+        data: StructuredData<'a, '_>,
+    ) -> &'a mut Option<HashMap<DomExceptionId, Self::Data>> {
         match data {
             StructuredData::Reader(reader) => &mut reader.exceptions,
             StructuredData::Writer(writer) => &mut writer.exceptions,
         }
     }
-
-    fn deserialized_storage(
-        reader: &mut StructuredDataReader,
-    ) -> &mut Option<HashMap<StorageKey, DomRoot<Self>>> {
-        &mut reader.dom_exceptions
-    }
 }

components/script/dom/dompoint.rs

@@ -14,8 +14,8 @@ use crate::dom::bindings::codegen::Bindings::DOMPointReadOnlyBinding::DOMPointRe
 use crate::dom::bindings::error::Fallible;
 use crate::dom::bindings::reflector::reflect_dom_object_with_proto;
 use crate::dom::bindings::root::DomRoot;
-use crate::dom::bindings::serializable::{Serializable, StorageKey};
-use crate::dom::bindings::structuredclone::{StructuredData, StructuredDataReader};
+use crate::dom::bindings::serializable::Serializable;
+use crate::dom::bindings::structuredclone::StructuredData;
 use crate::dom::dompointreadonly::{DOMPointReadOnly, DOMPointWriteMethods};
 use crate::dom::globalscope::GlobalScope;
 use crate::script_runtime::CanGc;
@@ -163,18 +163,12 @@ impl Serializable for DOMPoint {
         ))
     }
 
-    fn serialized_storage(
-        data: StructuredData<'_>,
-    ) -> &mut Option<HashMap<DomPointId, Self::Data>> {
+    fn serialized_storage<'a>(
+        data: StructuredData<'a, '_>,
+    ) -> &'a mut Option<HashMap<DomPointId, Self::Data>> {
         match data {
             StructuredData::Reader(reader) => &mut reader.points,
             StructuredData::Writer(writer) => &mut writer.points,
         }
     }
-
-    fn deserialized_storage(
-        reader: &mut StructuredDataReader,
-    ) -> &mut Option<HashMap<StorageKey, DomRoot<Self>>> {
-        &mut reader.dom_points
-    }
 }

components/script/dom/dompointreadonly.rs

@@ -15,8 +15,8 @@ use crate::dom::bindings::codegen::Bindings::DOMPointReadOnlyBinding::DOMPointRe
 use crate::dom::bindings::error::Fallible;
 use crate::dom::bindings::reflector::{Reflector, reflect_dom_object_with_proto};
 use crate::dom::bindings::root::DomRoot;
-use crate::dom::bindings::serializable::{Serializable, StorageKey};
-use crate::dom::bindings::structuredclone::{StructuredData, StructuredDataReader};
+use crate::dom::bindings::serializable::Serializable;
+use crate::dom::bindings::structuredclone::StructuredData;
 use crate::dom::globalscope::GlobalScope;
 use crate::script_runtime::CanGc;
 
@@ -172,18 +172,12 @@ impl Serializable for DOMPointReadOnly {
         ))
     }
 
-    fn serialized_storage(
-        data: StructuredData<'_>,
-    ) -> &mut Option<HashMap<DomPointId, Self::Data>> {
+    fn serialized_storage<'a>(
+        data: StructuredData<'a, '_>,
+    ) -> &'a mut Option<HashMap<DomPointId, Self::Data>> {
         match data {
             StructuredData::Reader(r) => &mut r.points,
             StructuredData::Writer(w) => &mut w.points,
         }
     }
-
-    fn deserialized_storage(
-        reader: &mut StructuredDataReader,
-    ) -> &mut Option<HashMap<StorageKey, DomRoot<Self>>> {
-        &mut reader.points_read_only
-    }
 }

components/script/dom/messageport.rs

@@ -23,7 +23,7 @@ use crate::dom::bindings::error::{Error, ErrorResult, ErrorToJsval};
 use crate::dom::bindings::inheritance::Castable;
 use crate::dom::bindings::reflector::{DomGlobal, reflect_dom_object};
 use crate::dom::bindings::root::DomRoot;
-use crate::dom::bindings::structuredclone::{self, StructuredData, StructuredDataReader};
+use crate::dom::bindings::structuredclone::{self, StructuredData};
 use crate::dom::bindings::trace::RootedTraceableBox;
 use crate::dom::bindings::transferable::Transferable;
 use crate::dom::bindings::utils::set_dictionary_property;
@@ -268,18 +268,14 @@ impl Transferable for MessagePort {
         Ok(transferred_port)
     }
 
-    fn serialized_storage(
-        data: StructuredData<'_>,
-    ) -> &mut Option<HashMap<MessagePortId, Self::Data>> {
+    fn serialized_storage<'a>(
+        data: StructuredData<'a, '_>,
+    ) -> &'a mut Option<HashMap<MessagePortId, Self::Data>> {
         match data {
             StructuredData::Reader(r) => &mut r.port_impls,
             StructuredData::Writer(w) => &mut w.ports,
         }
     }
-
-    fn deserialized_storage(reader: &mut StructuredDataReader) -> &mut Option<Vec<DomRoot<Self>>> {
-        &mut reader.message_ports
-    }
 }
 
 impl MessagePortMethods<crate::DomTypeHolder> for MessagePort {

components/script/dom/readablestream.rs

@@ -59,7 +59,7 @@ use crate::realms::{enter_realm, InRealm};
 use crate::script_runtime::{CanGc, JSContext as SafeJSContext};
 use crate::dom::promisenativehandler::{Callback, PromiseNativeHandler};
 use crate::dom::bindings::transferable::Transferable;
-use crate::dom::bindings::structuredclone::{StructuredData, StructuredDataReader};
+use crate::dom::bindings::structuredclone::StructuredData;
 
 use super::bindings::buffer_source::HeapBufferSource;
 use super::bindings::codegen::Bindings::ReadableStreamBYOBReaderBinding::ReadableStreamBYOBReaderReadOptions;
@@ -2247,16 +2247,12 @@ impl Transferable for ReadableStream {
     }
 
     /// Note: we are relying on the port transfer, so the data returned here are related to the port.
-    fn serialized_storage(
-        data: StructuredData<'_>,
-    ) -> &mut Option<HashMap<MessagePortId, Self::Data>> {
+    fn serialized_storage<'a>(
+        data: StructuredData<'a, '_>,
+    ) -> &'a mut Option<HashMap<MessagePortId, Self::Data>> {
         match data {
             StructuredData::Reader(r) => &mut r.port_impls,
             StructuredData::Writer(w) => &mut w.ports,
         }
     }
-
-    fn deserialized_storage(reader: &mut StructuredDataReader) -> &mut Option<Vec<DomRoot<Self>>> {
-        &mut reader.readable_streams
-    }
 }

components/script/dom/writablestream.rs

@@ -27,7 +27,7 @@ use crate::dom::bindings::conversions::ConversionResult;
 use crate::dom::bindings::error::{Error, Fallible};
 use crate::dom::bindings::reflector::{DomGlobal, Reflector, reflect_dom_object_with_proto};
 use crate::dom::bindings::root::{Dom, DomRoot, MutNullableDom};
-use crate::dom::bindings::structuredclone::{StructuredData, StructuredDataReader};
+use crate::dom::bindings::structuredclone::StructuredData;
 use crate::dom::bindings::transferable::Transferable;
 use crate::dom::countqueuingstrategy::{extract_high_water_mark, extract_size_algorithm};
 use crate::dom::domexception::{DOMErrorName, DOMException};
@@ -1182,16 +1182,12 @@ impl Transferable for WritableStream {
     }
 
     /// Note: we are relying on the port transfer, so the data returned here are related to the port.
-    fn serialized_storage(
-        data: StructuredData<'_>,
-    ) -> &mut Option<HashMap<MessagePortId, Self::Data>> {
+    fn serialized_storage<'a>(
+        data: StructuredData<'a, '_>,
+    ) -> &'a mut Option<HashMap<MessagePortId, Self::Data>> {
         match data {
             StructuredData::Reader(r) => &mut r.port_impls,
             StructuredData::Writer(w) => &mut w.ports,
         }
     }
-
-    fn deserialized_storage(reader: &mut StructuredDataReader) -> &mut Option<Vec<DomRoot<Self>>> {
-        &mut reader.writable_streams
-    }
 }