Split StructuredCloneHolder into Reader and Writer structs (#34792)

Signed-off-by: webbeef <me@webbeef.org>

5 files changed, 67 insertions, 111 deletions

diff --git a/components/script/dom/bindings/serializable.rs b/components/script/dom/bindings/serializable.rs
index 204144a4eab..46c6a7b5b5b 100644
--- a/components/script/dom/bindings/serializable.rs
+++ b/components/script/dom/bindings/serializable.rs
@@ -6,7 +6,7 @@
 //! (<https://html.spec.whatwg.org/multipage/#serializable-objects>).
 
 use crate::dom::bindings::reflector::DomObject;
-use crate::dom::bindings::structuredclone::StructuredDataHolder;
+use crate::dom::bindings::structuredclone::{StructuredDataReader, StructuredDataWriter};
 use crate::dom::globalscope::GlobalScope;
 use crate::script_runtime::CanGc;
 
@@ -22,11 +22,11 @@ pub struct StorageKey {
 /// <https://html.spec.whatwg.org/multipage/#serializable>
 pub trait Serializable: DomObject {
     /// <https://html.spec.whatwg.org/multipage/#serialization-steps>
-    fn serialize(&self, sc_holder: &mut StructuredDataHolder) -> Result<StorageKey, ()>;
+    fn serialize(&self, sc_writer: &mut StructuredDataWriter) -> Result<StorageKey, ()>;
     /// <https://html.spec.whatwg.org/multipage/#deserialization-steps>
     fn deserialize(
         owner: &GlobalScope,
-        sc_holder: &mut StructuredDataHolder,
+        sc_reader: &mut StructuredDataReader,
         extra_data: StorageKey,
         can_gc: CanGc,
     ) -> Result<(), ()>;
diff --git a/components/script/dom/bindings/structuredclone.rs b/components/script/dom/bindings/structuredclone.rs
index 8a8959bb1c2..39ba918e383 100644
--- a/components/script/dom/bindings/structuredclone.rs
+++ b/components/script/dom/bindings/structuredclone.rs
@@ -56,7 +56,7 @@ pub(super) enum StructuredCloneTags {
 unsafe fn read_blob(
     owner: &GlobalScope,
     r: *mut JSStructuredCloneReader,
-    sc_holder: &mut StructuredDataHolder,
+    sc_reader: &mut StructuredDataReader,
     can_gc: CanGc,
 ) -> *mut JSObject {
     let mut name_space: u32 = 0;
@@ -67,12 +67,8 @@ unsafe fn read_blob(
         &mut index as *mut u32
     ));
     let storage_key = StorageKey { index, name_space };
-    if <Blob as Serializable>::deserialize(owner, sc_holder, storage_key, can_gc).is_ok() {
-        let blobs = match sc_holder {
-            StructuredDataHolder::Read { blobs, .. } => blobs,
-            _ => panic!("Unexpected variant of StructuredDataHolder"),
-        };
-        if let Some(blobs) = blobs {
+    if <Blob as Serializable>::deserialize(owner, sc_reader, storage_key, can_gc).is_ok() {
+        if let Some(blobs) = &sc_reader.blobs {
             let blob = blobs
                 .get(&storage_key)
                 .expect("No blob found at storage key.");
@@ -90,9 +86,9 @@ unsafe fn write_blob(
     owner: &GlobalScope,
     blob: DomRoot<Blob>,
     w: *mut JSStructuredCloneWriter,
-    sc_holder: &mut StructuredDataHolder,
+    sc_writer: &mut StructuredDataWriter,
 ) -> bool {
-    if let Ok(storage_key) = blob.serialize(sc_holder) {
+    if let Ok(storage_key) = blob.serialize(sc_writer) {
         assert!(JS_WriteUint32Pair(
             w,
             StructuredCloneTags::DomBlob as u32,
@@ -133,7 +129,7 @@ unsafe extern "C" fn read_callback(
         return read_blob(
             &GlobalScope::from_context(cx, InRealm::Already(&in_realm_proof)),
             r,
-            &mut *(closure as *mut StructuredDataHolder),
+            &mut *(closure as *mut StructuredDataReader),
             CanGc::note(),
         );
     }
@@ -153,7 +149,7 @@ unsafe extern "C" fn write_callback(
             &GlobalScope::from_context(cx, InRealm::Already(&in_realm_proof)),
             blob,
             w,
-            &mut *(closure as *mut StructuredDataHolder),
+            &mut *(closure as *mut StructuredDataWriter),
         );
     }
     false
@@ -170,12 +166,12 @@ unsafe extern "C" fn read_transfer_callback(
     return_object: RawMutableHandleObject,
 ) -> bool {
     if tag == StructuredCloneTags::MessagePort as u32 {
-        let sc_holder = &mut *(closure as *mut StructuredDataHolder);
+        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));
         if <MessagePort as Transferable>::transfer_receive(
             &owner,
-            sc_holder,
+            sc_reader,
             extra_data,
             return_object,
         )
@@ -200,8 +196,8 @@ unsafe extern "C" fn write_transfer_callback(
     if let Ok(port) = root_from_object::<MessagePort>(*obj, cx) {
         *tag = StructuredCloneTags::MessagePort as u32;
         *ownership = TransferableOwnership::SCTAG_TMO_CUSTOM;
-        let sc_holder = &mut *(closure as *mut StructuredDataHolder);
-        if let Ok(data) = port.transfer(sc_holder) {
+        let sc_writer = &mut *(closure as *mut StructuredDataWriter);
+        if let Ok(data) = port.transfer(sc_writer) {
             *extra_data = data;
             return true;
         }
@@ -257,31 +253,30 @@ static STRUCTURED_CLONE_CALLBACKS: JSStructuredCloneCallbacks = JSStructuredClon
     sabCloned: Some(sab_cloned_callback),
 };
 
-/// A data holder for results from, and inputs to, structured-data read/write operations.
+/// 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>
-pub enum StructuredDataHolder {
-    Read {
-        /// A map of deserialized blobs, stored temporarily here to keep them rooted.
-        blobs: Option<HashMap<StorageKey, DomRoot<Blob>>>,
-        /// A vec of transfer-received DOM ports,
-        /// to be made available to script through a message event.
-        message_ports: Option<Vec<DomRoot<MessagePort>>>,
-        /// 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.
-        port_impls: Option<HashMap<MessagePortId, MessagePortImpl>>,
-        /// A map of blob implementations,
-        /// used as part of the "deserialize" steps of blobs,
-        /// to produce the DOM blobs stored in `blobs` above.
-        blob_impls: Option<HashMap<BlobId, BlobImpl>>,
-    },
-    /// A data holder for transferred and serialized objects.
-    Write {
-        /// Transferred ports.
-        ports: Option<HashMap<MessagePortId, MessagePortImpl>>,
-        /// Serialized blobs.
-        blobs: Option<HashMap<BlobId, BlobImpl>>,
-    },
+pub struct StructuredDataReader {
+    /// A map of deserialized blobs, stored temporarily here to keep them rooted.
+    pub blobs: Option<HashMap<StorageKey, DomRoot<Blob>>>,
+    /// A vec of transfer-received DOM ports,
+    /// to be made available to script through a message event.
+    pub message_ports: Option<Vec<DomRoot<MessagePort>>>,
+    /// 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.
+    pub port_impls: Option<HashMap<MessagePortId, MessagePortImpl>>,
+    /// A map of blob implementations,
+    /// used as part of the "deserialize" steps of blobs,
+    /// to produce the DOM blobs stored in `blobs` above.
+    pub blob_impls: Option<HashMap<BlobId, BlobImpl>>,
+}
+
+/// A data holder for transferred and serialized objects.
+pub struct StructuredDataWriter {
+    /// Transferred ports.
+    pub ports: Option<HashMap<MessagePortId, MessagePortImpl>>,
+    /// Serialized blobs.
+    pub blobs: Option<HashMap<BlobId, BlobImpl>>,
 }
 
 /// Writes a structured clone. Returns a `DataClone` error if that fails.
@@ -295,11 +290,11 @@ pub fn write(
         if let Some(transfer) = transfer {
             transfer.to_jsval(*cx, val.handle_mut());
         }
-        let mut sc_holder = StructuredDataHolder::Write {
+        let mut sc_writer = StructuredDataWriter {
             ports: None,
             blobs: None,
         };
-        let sc_holder_ptr = &mut sc_holder as *mut _;
+        let sc_writer_ptr = &mut sc_writer as *mut _;
 
         let scbuf = NewJSAutoStructuredCloneBuffer(
             StructuredCloneScope::DifferentProcess,
@@ -317,7 +312,7 @@ pub fn write(
             StructuredCloneScope::DifferentProcess,
             &policy,
             &STRUCTURED_CLONE_CALLBACKS,
-            sc_holder_ptr as *mut raw::c_void,
+            sc_writer_ptr as *mut raw::c_void,
             val.handle(),
         );
         if !result {
@@ -332,15 +327,10 @@ pub fn write(
 
         DeleteJSAutoStructuredCloneBuffer(scbuf);
 
-        let (mut blob_impls, mut port_impls) = match sc_holder {
-            StructuredDataHolder::Write { blobs, ports } => (blobs, ports),
-            _ => panic!("Unexpected variant of StructuredDataHolder"),
-        };
-
         let data = StructuredSerializedData {
             serialized: data,
-            ports: port_impls.take(),
-            blobs: blob_impls.take(),
+            ports: sc_writer.ports.take(),
+            blobs: sc_writer.blobs.take(),
         };
 
         Ok(data)
@@ -356,13 +346,13 @@ pub fn read(
 ) -> Result<Vec<DomRoot<MessagePort>>, ()> {
     let cx = GlobalScope::get_cx();
     let _ac = enter_realm(global);
-    let mut sc_holder = StructuredDataHolder::Read {
+    let mut sc_reader = StructuredDataReader {
         blobs: None,
         message_ports: None,
         port_impls: data.ports.take(),
         blob_impls: data.blobs.take(),
     };
-    let sc_holder_ptr = &mut sc_holder as *mut _;
+    let sc_reader_ptr = &mut sc_reader as *mut _;
     unsafe {
         let scbuf = NewJSAutoStructuredCloneBuffer(
             StructuredCloneScope::DifferentProcess,
@@ -387,25 +377,16 @@ pub fn read(
                 allowSharedMemoryObjects_: false,
             },
             &STRUCTURED_CLONE_CALLBACKS,
-            sc_holder_ptr as *mut raw::c_void,
+            sc_reader_ptr as *mut raw::c_void,
         );
 
         DeleteJSAutoStructuredCloneBuffer(scbuf);
 
         if result {
-            let (mut message_ports, port_impls) = match sc_holder {
-                StructuredDataHolder::Read {
-                    message_ports,
-                    port_impls,
-                    ..
-                } => (message_ports, port_impls),
-                _ => panic!("Unexpected variant of StructuredDataHolder"),
-            };
-
             // Any transfer-received port-impls should have been taken out.
-            assert!(port_impls.is_none());
+            assert!(sc_reader.port_impls.is_none());
 
-            match message_ports.take() {
+            match sc_reader.message_ports.take() {
                 Some(ports) => return Ok(ports),
                 None => return Ok(Vec::with_capacity(0)),
             }
diff --git a/components/script/dom/bindings/transferable.rs b/components/script/dom/bindings/transferable.rs
index 6c75bfe2828..d97fc28eb52 100644
--- a/components/script/dom/bindings/transferable.rs
+++ b/components/script/dom/bindings/transferable.rs
@@ -8,14 +8,14 @@
 use js::jsapi::MutableHandleObject;
 
 use crate::dom::bindings::reflector::DomObject;
-use crate::dom::bindings::structuredclone::StructuredDataHolder;
+use crate::dom::bindings::structuredclone::{StructuredDataReader, StructuredDataWriter};
 use crate::dom::globalscope::GlobalScope;
 
 pub trait Transferable: DomObject {
-    fn transfer(&self, sc_holder: &mut StructuredDataHolder) -> Result<u64, ()>;
+    fn transfer(&self, sc_writer: &mut StructuredDataWriter) -> Result<u64, ()>;
     fn transfer_receive(
         owner: &GlobalScope,
-        sc_holder: &mut StructuredDataHolder,
+        sc_reader: &mut StructuredDataReader,
         extra_data: u64,
         return_object: MutableHandleObject,
     ) -> Result<(), ()>;
diff --git a/components/script/dom/blob.rs b/components/script/dom/blob.rs
index e92d64d1958..07b542da9b3 100644
--- a/components/script/dom/blob.rs
+++ b/components/script/dom/blob.rs
@@ -25,7 +25,7 @@ use crate::dom::bindings::reflector::{reflect_dom_object_with_proto, DomObject,
 use crate::dom::bindings::root::DomRoot;
 use crate::dom::bindings::serializable::{Serializable, StorageKey};
 use crate::dom::bindings::str::DOMString;
-use crate::dom::bindings::structuredclone::StructuredDataHolder;
+use crate::dom::bindings::structuredclone::{StructuredDataReader, StructuredDataWriter};
 use crate::dom::globalscope::GlobalScope;
 use crate::dom::promise::Promise;
 use crate::dom::readablestream::ReadableStream;
@@ -93,12 +93,7 @@ impl Blob {
 
 impl Serializable for Blob {
     /// <https://w3c.github.io/FileAPI/#ref-for-serialization-steps>
-    fn serialize(&self, sc_holder: &mut StructuredDataHolder) -> Result<StorageKey, ()> {
-        let blob_impls = match sc_holder {
-            StructuredDataHolder::Write { blobs, .. } => blobs,
-            _ => panic!("Unexpected variant of StructuredDataHolder"),
-        };
-
+    fn serialize(&self, sc_writer: &mut StructuredDataWriter) -> Result<StorageKey, ()> {
         let blob_id = self.blob_id;
 
         // 1. Get a clone of the blob impl.
@@ -108,7 +103,7 @@ impl Serializable for Blob {
         let new_blob_id = blob_impl.blob_id();
 
         // 2. Store the object at a given key.
-        let blobs = blob_impls.get_or_insert_with(HashMap::new);
+        let blobs = sc_writer.blobs.get_or_insert_with(HashMap::new);
         blobs.insert(new_blob_id, blob_impl);
 
         let PipelineNamespaceId(name_space) = new_blob_id.namespace_id;
@@ -130,7 +125,7 @@ impl Serializable for Blob {
     /// <https://w3c.github.io/FileAPI/#ref-for-deserialization-steps>
     fn deserialize(
         owner: &GlobalScope,
-        sc_holder: &mut StructuredDataHolder,
+        sc_reader: &mut StructuredDataReader,
         storage_key: StorageKey,
         can_gc: CanGc,
     ) -> Result<(), ()> {
@@ -145,27 +140,21 @@ impl Serializable for Blob {
             index,
         };
 
-        let (blobs, blob_impls) = match sc_holder {
-            StructuredDataHolder::Read {
-                blobs, blob_impls, ..
-            } => (blobs, blob_impls),
-            _ => panic!("Unexpected variant of StructuredDataHolder"),
-        };
-
         // 2. Get the transferred object from its storage, using the key.
-        let blob_impls_map = blob_impls
+        let blob_impls_map = sc_reader
+            .blob_impls
             .as_mut()
             .expect("The SC holder does not have any blob impls");
         let blob_impl = blob_impls_map
             .remove(&id)
             .expect("No blob to be deserialized found.");
         if blob_impls_map.is_empty() {
-            *blob_impls = None;
+            sc_reader.blob_impls = None;
         }
 
         let deserialized_blob = Blob::new(owner, blob_impl, can_gc);
 
-        let blobs = blobs.get_or_insert_with(HashMap::new);
+        let blobs = sc_reader.blobs.get_or_insert_with(HashMap::new);
         blobs.insert(storage_key, deserialized_blob);
 
         Ok(())
diff --git a/components/script/dom/messageport.rs b/components/script/dom/messageport.rs
index 03e3e4228a4..69a2b4a519f 100644
--- a/components/script/dom/messageport.rs
+++ b/components/script/dom/messageport.rs
@@ -23,7 +23,7 @@ use crate::dom::bindings::error::{Error, ErrorResult};
 use crate::dom::bindings::inheritance::Castable;
 use crate::dom::bindings::reflector::{reflect_dom_object, DomObject};
 use crate::dom::bindings::root::DomRoot;
-use crate::dom::bindings::structuredclone::{self, StructuredDataHolder};
+use crate::dom::bindings::structuredclone::{self, StructuredDataReader, StructuredDataWriter};
 use crate::dom::bindings::trace::RootedTraceableBox;
 use crate::dom::bindings::transferable::Transferable;
 use crate::dom::eventtarget::EventTarget;
@@ -163,16 +163,11 @@ impl MessagePort {
 
 impl Transferable for MessagePort {
     /// <https://html.spec.whatwg.org/multipage/#message-ports:transfer-steps>
-    fn transfer(&self, sc_holder: &mut StructuredDataHolder) -> Result<u64, ()> {
+    fn transfer(&self, sc_writer: &mut StructuredDataWriter) -> Result<u64, ()> {
         if self.detached.get() {
             return Err(());
         }
 
-        let port_impls = match sc_holder {
-            StructuredDataHolder::Write { ports, .. } => ports,
-            _ => panic!("Unexpected variant of StructuredDataHolder"),
-        };
-
         self.detached.set(true);
         let id = self.message_port_id();
 
@@ -180,12 +175,12 @@ impl Transferable for MessagePort {
         let transferred_port = self.global().mark_port_as_transferred(id);
 
         // 2. Store the transferred object at a given key.
-        if let Some(ports) = port_impls.as_mut() {
+        if let Some(ports) = sc_writer.ports.as_mut() {
             ports.insert(*id, transferred_port);
         } else {
             let mut ports = HashMap::new();
             ports.insert(*id, transferred_port);
-            *port_impls = Some(ports);
+            sc_writer.ports = Some(ports);
         }
 
         let PipelineNamespaceId(name_space) = (id).namespace_id;
@@ -207,19 +202,10 @@ impl Transferable for MessagePort {
     /// <https://html.spec.whatwg.org/multipage/#message-ports:transfer-receiving-steps>
     fn transfer_receive(
         owner: &GlobalScope,
-        sc_holder: &mut StructuredDataHolder,
+        sc_reader: &mut StructuredDataReader,
         extra_data: u64,
         return_object: MutableHandleObject,
     ) -> Result<(), ()> {
-        let (message_ports, port_impls) = match sc_holder {
-            StructuredDataHolder::Read {
-                message_ports,
-                port_impls,
-                ..
-            } => (message_ports, port_impls),
-            _ => panic!("Unexpected variant of StructuredDataHolder"),
-        };
-
         // 1. Re-build the key for the storage location
         // of the transferred object.
         let big: [u8; 8] = extra_data.to_ne_bytes();
@@ -244,11 +230,11 @@ impl Transferable for MessagePort {
 
         // 2. Get the transferred object from its storage, using the key.
         // Assign the transfer-received port-impl, and total number of transferred ports.
-        let (ports_len, port_impl) = if let Some(ports) = port_impls.as_mut() {
+        let (ports_len, port_impl) = if let Some(ports) = sc_reader.port_impls.as_mut() {
             let ports_len = ports.len();
             let port_impl = ports.remove(&id).expect("Transferred port to be stored");
             if ports.is_empty() {
-                *port_impls = None;
+                sc_reader.port_impls = None;
             }
             (ports_len, port_impl)
         } else {
@@ -262,12 +248,12 @@ impl Transferable for MessagePort {
         return_object.set(transferred_port.reflector().rootable().get());
 
         // Store the DOM port where it will be passed along to script in the message-event.
-        if let Some(ports) = message_ports.as_mut() {
+        if let Some(ports) = sc_reader.message_ports.as_mut() {
             ports.push(transferred_port);
         } else {
             let mut ports = Vec::with_capacity(ports_len);
             ports.push(transferred_port);
-            *message_ports = Some(ports);
+            sc_reader.message_ports = Some(ports);
         }
 
         Ok(())