servo / servo

Servo, the embeddable, independent, memory-safe, modular, parallel web rendering engine
https://servo.org
Mozilla Public License 2.0
27.84k stars 2.98k forks source link

ImageData can't be structured-cloned by postMessage #25148

Open pshaughn opened 4 years ago

pshaughn commented 4 years ago

This is tested in WPT workers/interfaces/DedicatedWorkerGlobalScope/postMessage/imagedata-cloned-canvas-in-array.html and structured-clone-imagedata.html; postMessage throws DataCloneError.

pshaughn commented 4 years ago

21715 is a prerequisite for doing this right.

jdm commented 4 years ago

Not necessarily; we could continue to hand-write serialization code in https://github.com/servo/servo/blob/master/components/script/dom/bindings/structuredclone.rs.

pshaughn commented 4 years ago

Yes, we could. The set of types that need structured-cloning is finite, and most of them are basically the same as an ArrayBuffer plus some constant-size fields.

gterzian commented 4 years ago

Since #21715 was opened, the situation has improved also quite a lot, if I say so myself. Although so far the "improvements" have been focused on supporting more "complicated" DOM objects, like Blob and Messageport.

So we do already have dom::bindings::serializable::Serializable, and I had a quick look at ImageData, I think implementing it for that would be mostly about gettting a Vec<u8> out of the Uint8ClampedArray stored as Heap<*mut JSobject>, and adding a bit of glue in structuredclone.rs, but it wouldn't be hugely manual with actually having to write the Vec<u8> using the mozjs structured clone API like was still done when #21715 was first opened.

So I don't think it would be very hard, I can help if someone wants to do it. If you look at Blob, it should be like that, but easier because you don't have to deal with the complications of Blob that can be seen in globalscope.rs.

gterzian commented 4 years ago

If you look at ImageData.as_slice, it would be essentially that followed by into_vec'ing it, then you would need to clone the height and width, and store the whole thing in the StructuredDataHolder, probably in a simple Vec since you don't need to identify the image data uniquely I think(like you needed with a blob).

So in script_traits::serializeable.rs, you could add something like:

#[derive(Debug, Deserialize, MallocSizeOf, Serialize)]
pub struct SerializedImageData {
    data: Vec<u8>,
    width: u32,
    height: u32,
}

Then in structuredclone.rs, add an Option<Vec<SerializedImageData>> to both variants of StructuredDataHolder, and write into/read from those in the implementation of dom::bindings::serializable::Serializable for ImageData.

gterzian commented 4 years ago

You would also need to add the Vec<SerializedImageData> to script_traits::StructuredSerializedData

gterzian commented 4 years ago

(and yes some of this is boilerplate that could be generated as suggested in https://github.com/servo/servo/issues/21715, the "hardest" part is the actual implementation of dom::bindings::serializable::Serializable, and even that should not be much harder than calling ImageData.as_slice)