Make ImageData more spec compliant (#37620)

I updated webidl, and all changes that bring, currently we still not
support float16array (there is no support in FF either). Most notable
change is refactored new_*/Constructors methods, that should now handle
HeapBufferSource more properly (with less unsafe).

Testing: Existing WPT tests
try run: https://github.com/sagudev/servo/actions/runs/15806083404
Fixes: #37618 (although not tested)

---------

Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com>

components/script/dom/imagedata.rs

@@ -3,7 +3,6 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
 use std::borrow::Cow;
-use std::ptr;
 use std::vec::Vec;
 
 use dom_struct::dom_struct;
@@ -12,11 +11,13 @@ use ipc_channel::ipc::IpcSharedMemory;
 use js::gc::CustomAutoRooterGuard;
 use js::jsapi::JSObject;
 use js::rust::HandleObject;
-use js::typedarray::{ClampedU8, CreateWith, Uint8ClampedArray};
+use js::typedarray::{ClampedU8, Uint8ClampedArray};
 
 use super::bindings::buffer_source::{HeapBufferSource, create_heap_buffer_source_with_length};
 use crate::dom::bindings::buffer_source::create_buffer_source;
-use crate::dom::bindings::codegen::Bindings::CanvasRenderingContext2DBinding::ImageDataMethods;
+use crate::dom::bindings::codegen::Bindings::CanvasRenderingContext2DBinding::{
+    ImageDataMethods, ImageDataPixelFormat, ImageDataSettings, PredefinedColorSpace,
+};
 use crate::dom::bindings::error::{Error, Fallible};
 use crate::dom::bindings::reflector::{Reflector, reflect_dom_object_with_proto};
 use crate::dom::bindings::root::DomRoot;
@@ -28,8 +29,11 @@ pub(crate) struct ImageData {
     reflector_: Reflector,
     width: u32,
     height: u32,
+    /// <https://html.spec.whatwg.org/multipage/#dom-imagedata-data>
     #[ignore_malloc_size_of = "mozjs"]
     data: HeapBufferSource<ClampedU8>,
+    pixel_format: ImageDataPixelFormat,
+    color_space: PredefinedColorSpace,
 }
 
 impl ImageData {
@@ -47,105 +51,102 @@ impl ImageData {
                     "The requested image size exceeds the supported range".to_owned(),
                 ))?;
 
-        unsafe {
-            let cx = GlobalScope::get_cx();
-            rooted!(in (*cx) let mut js_object = ptr::null_mut::<JSObject>());
+        let settings = ImageDataSettings {
+            colorSpace: Some(PredefinedColorSpace::Srgb),
+            pixelFormat: ImageDataPixelFormat::Rgba_unorm8,
+        };
+
         if let Some(ref mut d) = data {
             d.resize(len as usize, 0);
 
-                let typed_array =
-                    create_buffer_source::<ClampedU8>(cx, &d[..], js_object.handle_mut(), can_gc)
-                        .map_err(|_| Error::JSFailed)?;
+            let cx = GlobalScope::get_cx();
+            rooted!(in (*cx) let mut js_object = std::ptr::null_mut::<JSObject>());
+            auto_root!(in(*cx) let data = create_buffer_source::<ClampedU8>(cx, &d[..], js_object.handle_mut(), can_gc)
+                        .map_err(|_| Error::JSFailed)?);
 
-                let data = CreateWith::Slice(&d[..]);
-                Uint8ClampedArray::create(*cx, data, js_object.handle_mut()).unwrap();
-                auto_root!(in(*cx) let data = typed_array);
-                Self::new_with_data(global, None, width, Some(height), data, can_gc)
+            Self::Constructor_(global, None, can_gc, data, width, Some(height), &settings)
         } else {
-                Self::new_without_data(global, None, width, height, can_gc)
-            }
+            Self::Constructor(global, None, can_gc, width, height, &settings)
         }
     }
 
-    #[allow(unsafe_code)]
-    fn new_with_data(
+    #[allow(clippy::too_many_arguments)]
+    /// <https://html.spec.whatwg.org/multipage/#initialize-an-imagedata-object>
+    fn initialize(
+        pixels_per_row: u32,
+        rows: u32,
+        settings: &ImageDataSettings,
+        source: Option<CustomAutoRooterGuard<Uint8ClampedArray>>,
+        default_color_space: Option<PredefinedColorSpace>,
         global: &GlobalScope,
         proto: Option<HandleObject>,
-        width: u32,
-        opt_height: Option<u32>,
-        data: CustomAutoRooterGuard<Uint8ClampedArray>,
         can_gc: CanGc,
     ) -> Fallible<DomRoot<ImageData>> {
-        let heap_typed_array = HeapBufferSource::<ClampedU8>::from_view(data);
-
-        let typed_array = match heap_typed_array.get_typed_array() {
-            Ok(array) => array,
-            Err(_) => {
-                return Err(Error::Type(
-                    "Argument to Image data is not an Uint8ClampedArray".to_owned(),
-                ));
-            },
-        };
-
-        let byte_len = unsafe { typed_array.as_slice().len() } as u32;
-        if byte_len == 0 || byte_len % 4 != 0 {
+        // 1. If source was given:
+        let data = if let Some(source) = source {
+            // 1. If settings["pixelFormat"] equals "rgba-unorm8" and source is not a Uint8ClampedArray,
+            // then throw an "InvalidStateError" DOMException.
+            // 2. If settings["pixelFormat"] is "rgba-float16" and source is not a Float16Array,
+            // then throw an "InvalidStateError" DOMException.
+            if !matches!(settings.pixelFormat, ImageDataPixelFormat::Rgba_unorm8) {
+                // we currently support only rgba-unorm8
                 return Err(Error::InvalidState);
             }
-
-        let len = byte_len / 4;
-        if width == 0 || len % width != 0 {
-            return Err(Error::IndexSize);
+            // 3. Initialize the data attribute of imageData to source.
+            HeapBufferSource::<ClampedU8>::from_view(source)
+        } else {
+            // 2. Otherwise (source was not given):
+            match settings.pixelFormat {
+                ImageDataPixelFormat::Rgba_unorm8 => {
+                    // 1. If settings["pixelFormat"] is "rgba-unorm8",
+                    // then initialize the data attribute of imageData to a new Uint8ClampedArray object.
+                    // The Uint8ClampedArray object must use a new ArrayBuffer for its storage,
+                    // and must have a zero byte offset and byte length equal to the length of its storage, in bytes.
+                    // The storage ArrayBuffer must have a length of 4 × rows × pixelsPerRow bytes.
+                    // 3. If the storage ArrayBuffer could not be allocated,
+                    // then rethrow the RangeError thrown by JavaScript, and return.
+                    create_heap_buffer_source_with_length(
+                        GlobalScope::get_cx(),
+                        4 * rows * pixels_per_row,
+                        can_gc,
+                    )?
+                },
+                // 3. Otherwise, if settings["pixelFormat"] is "rgba-float16",
+                // then initialize the data attribute of imageData to a new Float16Array object.
+                // The Float16Array object must use a new ArrayBuffer for its storage,
+                // and must have a zero byte offset and byte length equal to the length of its storage, in bytes.
+                // The storage ArrayBuffer must have a length of 8 × rows × pixelsPerRow bytes.
+                // not implemented yet
             }
+        };
+        // 3. Initialize the width attribute of imageData to pixelsPerRow.
+        let width = pixels_per_row;
+        // 4. Initialize the height attribute of imageData to rows.
+        let height = rows;
+        // 5. Initialize the pixelFormat attribute of imageData to settings["pixelFormat"].
+        let pixel_format = settings.pixelFormat;
+        // 6. If settings["colorSpace"] exists,
+        // then initialize the colorSpace attribute of imageData to settings["colorSpace"].
+        let color_space = settings
+            .colorSpace
+            // 7. Otherwise, if defaultColorSpace was given,
+            // then initialize the colorSpace attribute of imageData to defaultColorSpace.
+            .or(default_color_space)
+            // 8. Otherwise, initialize the colorSpace attribute of imageData to "srgb".
+            .unwrap_or(PredefinedColorSpace::Srgb);
 
-        let height = len / width;
-        if opt_height.is_some_and(|x| height != x) {
-            return Err(Error::IndexSize);
-        }
-
-        let imagedata = Box::new(ImageData {
+        Ok(reflect_dom_object_with_proto(
+            Box::new(ImageData {
                 reflector_: Reflector::new(),
                 width,
                 height,
-            data: heap_typed_array,
-        });
-
-        Ok(reflect_dom_object_with_proto(
-            imagedata, global, proto, can_gc,
-        ))
-    }
-
-    fn new_without_data(
-        global: &GlobalScope,
-        proto: Option<HandleObject>,
-        width: u32,
-        height: u32,
-        can_gc: CanGc,
-    ) -> Fallible<DomRoot<ImageData>> {
-        // If one or both of sw and sh are zero, then throw an "IndexSizeError" DOMException.
-        if width == 0 || height == 0 {
-            return Err(Error::IndexSize);
-        }
-
-        // When a constructor is called for an ImageData that is too large, other browsers throw
-        // IndexSizeError rather than RangeError here, so we do the same.
-        let len =
-            pixels::compute_rgba8_byte_length_if_within_limit(width as usize, height as usize)
-                .ok_or(Error::IndexSize)?;
-
-        let cx = GlobalScope::get_cx();
-
-        let heap_typed_array =
-            create_heap_buffer_source_with_length::<ClampedU8>(cx, len as u32, can_gc)?;
-
-        let imagedata = Box::new(ImageData {
-            reflector_: Reflector::new(),
-            width,
-            height,
-            data: heap_typed_array,
-        });
-
-        Ok(reflect_dom_object_with_proto(
-            imagedata, global, proto, can_gc,
+                data,
+                pixel_format,
+                color_space,
+            }),
+            global,
+            proto,
+            can_gc,
         ))
     }
 
@@ -153,16 +154,6 @@ impl ImageData {
         self.data.is_detached_buffer(GlobalScope::get_cx())
     }
 
-    #[allow(unsafe_code)]
-    pub(crate) fn to_shared_memory(&self) -> IpcSharedMemory {
-        IpcSharedMemory::from_bytes(unsafe { self.as_slice() })
-    }
-
-    #[allow(unsafe_code)]
-    pub(crate) unsafe fn get_rect(&self, rect: Rect<u32>) -> Cow<[u8]> {
-        pixels::rgba8_get_rect(self.as_slice(), self.get_size().to_u32(), rect)
-    }
-
     pub(crate) fn get_size(&self) -> Size2D<u32> {
         Size2D::new(self.Width(), self.Height())
     }
@@ -180,9 +171,23 @@ impl ImageData {
         // this ImageData instance will never change. The method is thus unsafe
         // because the array may be manipulated from JS while the reference
         // is live.
+        unsafe {
             let ptr: *const [u8] = internal_data.as_slice() as *const _;
             &*ptr
         }
+    }
+
+    /// Nothing must change the array on the JS side while the slice is live.
+    #[allow(unsafe_code)]
+    pub(crate) unsafe fn get_rect(&self, rect: Rect<u32>) -> Cow<[u8]> {
+        pixels::rgba8_get_rect(unsafe { self.as_slice() }, self.get_size().to_u32(), rect)
+    }
+
+    #[allow(unsafe_code)]
+    pub(crate) fn to_shared_memory(&self) -> IpcSharedMemory {
+        // This is safe because we copy the slice content
+        IpcSharedMemory::from_bytes(unsafe { self.as_slice() })
+    }
 
     #[allow(unsafe_code)]
     pub(crate) fn to_vec(&self) -> Vec<u8> {
@@ -197,10 +202,23 @@ impl ImageDataMethods<crate::DomTypeHolder> for ImageData {
         global: &GlobalScope,
         proto: Option<HandleObject>,
         can_gc: CanGc,
-        width: u32,
-        height: u32,
+        sw: u32,
+        sh: u32,
+        settings: &ImageDataSettings,
     ) -> Fallible<DomRoot<Self>> {
-        Self::new_without_data(global, proto, width, height, can_gc)
+        // 1. If one or both of sw and sh are zero, then throw an "IndexSizeError" DOMException.
+        if sw == 0 || sh == 0 {
+            return Err(Error::IndexSize);
+        }
+
+        // When a constructor is called for an ImageData that is too large, other browsers throw
+        // IndexSizeError rather than RangeError here, so we do the same.
+        pixels::compute_rgba8_byte_length_if_within_limit(sw as usize, sh as usize)
+            .ok_or(Error::IndexSize)?;
+
+        // 2. Initialize this given sw, sh, and settings.
+        // 3. Initialize the image data of this to transparent black.
+        Self::initialize(sw, sh, settings, None, None, global, proto, can_gc)
     }
 
     /// <https://html.spec.whatwg.org/multipage/#dom-imagedata-with-data>
@@ -209,10 +227,47 @@ impl ImageDataMethods<crate::DomTypeHolder> for ImageData {
         proto: Option<HandleObject>,
         can_gc: CanGc,
         data: CustomAutoRooterGuard<Uint8ClampedArray>,
-        width: u32,
-        opt_height: Option<u32>,
+        sw: u32,
+        sh: Option<u32>,
+        settings: &ImageDataSettings,
     ) -> Fallible<DomRoot<Self>> {
-        Self::new_with_data(global, proto, width, opt_height, data, can_gc)
+        // 1. Let bytesPerPixel be 4 if settings["pixelFormat"] is "rgba-unorm8"; otherwise 8.
+        let bytes_per_pixel = match settings.pixelFormat {
+            ImageDataPixelFormat::Rgba_unorm8 => 4,
+        };
+        // 2. Let length be the buffer source byte length of data.
+        let length = data.len();
+        if length == 0 {
+            return Err(Error::InvalidState);
+        }
+        // 3. If length is not a nonzero integral multiple of bytesPerPixel,
+        // then throw an "InvalidStateError" DOMException.
+        if length % bytes_per_pixel != 0 {
+            return Err(Error::InvalidState);
+        }
+        // 4. Let length be length divided by bytesPerPixel.
+        let length = length / bytes_per_pixel;
+        // 5. If length is not an integral multiple of sw, then throw an "IndexSizeError" DOMException.
+        if sw == 0 || length % sw as usize != 0 {
+            return Err(Error::IndexSize);
+        }
+        // 6. Let height be length divided by sw.
+        let height = length / sw as usize;
+        // 7. If sh was given and its value is not equal to height, then throw an "IndexSizeError" DOMException.
+        if sh.is_some_and(|x| height != x as usize) {
+            return Err(Error::IndexSize);
+        }
+        // 8. Initialize this given sw, sh, settings, and source set to data.
+        Self::initialize(
+            sw,
+            height as u32,
+            settings,
+            Some(data),
+            None,
+            global,
+            proto,
+            can_gc,
+        )
     }
 
     /// <https://html.spec.whatwg.org/multipage/#dom-imagedata-width>
@@ -229,4 +284,14 @@ impl ImageDataMethods<crate::DomTypeHolder> for ImageData {
     fn GetData(&self, _: JSContext) -> Fallible<Uint8ClampedArray> {
         self.data.get_typed_array().map_err(|_| Error::JSFailed)
     }
+
+    /// <https://html.spec.whatwg.org/multipage/#dom-imagedata-pixelformat>
+    fn PixelFormat(&self) -> ImageDataPixelFormat {
+        self.pixel_format
+    }
+
+    /// <https://html.spec.whatwg.org/multipage/#dom-imagedata-colorspace>
+    fn ColorSpace(&self) -> PredefinedColorSpace {
+        self.color_space
+    }
 }

components/script_bindings/import.rs

@@ -18,6 +18,9 @@ pub(crate) mod base {
     pub(crate) use js::panic::maybe_resume_unwind;
     pub(crate) use js::rust::wrappers::Call;
     pub(crate) use js::rust::{HandleObject, HandleValue, MutableHandleObject, MutableHandleValue};
+    pub(crate) use js::typedarray::{
+        ArrayBuffer, ArrayBufferView, Float32Array, Float64Array, Uint8Array, Uint8ClampedArray,
+    };
 
     pub(crate) use crate::callback::{
         CallSetup, CallbackContainer, CallbackFunction, CallbackInterface, CallbackObject,
@@ -84,9 +87,6 @@ pub(crate) mod module {
         CustomAutoRooterGuard, GCMethods, Handle, MutableHandle, get_context_realm,
         get_object_class, get_object_realm,
     };
-    pub(crate) use js::typedarray::{
-        ArrayBuffer, ArrayBufferView, Float32Array, Float64Array, Uint8Array, Uint8ClampedArray,
-    };
     pub(crate) use js::{
         JS_CALLEE, JSCLASS_GLOBAL_SLOT_COUNT, JSCLASS_IS_DOMJSCLASS, JSCLASS_IS_GLOBAL,
         JSCLASS_RESERVED_SLOTS_MASK, typedarray,

components/script_bindings/webidls/CanvasRenderingContext2D.webidl

@@ -16,6 +16,10 @@ typedef (HTMLOrSVGImageElement or
          /*VideoFrame or*/
          /*CSSImageValue*/ CSSStyleValue) CanvasImageSource;
 
+enum PredefinedColorSpace { "srgb"/*, "display-p3"*/ };
+
+enum CanvasColorType { "unorm8", "float16" };
+
 enum CanvasFillRule { "nonzero", "evenodd" };
 
 [Exposed=Window]
@@ -253,17 +257,28 @@ interface CanvasPattern {
   //undefined setTransform(optional DOMMatrix2DInit transform = {});
 };
 
+// TODO: Float16Array
+typedef Uint8ClampedArray ImageDataArray;
+
+enum ImageDataPixelFormat { "rgba-unorm8"/*, "rgba-float16"*/ };
+
+dictionary ImageDataSettings {
+  PredefinedColorSpace colorSpace;
+  ImageDataPixelFormat pixelFormat = "rgba-unorm8";
+};
+
 [Exposed=(Window,Worker),
  Serializable]
 interface ImageData {
-  [Throws] constructor(unsigned long sw, unsigned long sh/*, optional ImageDataSettings settings = {}*/);
-  [Throws] constructor(Uint8ClampedArray data, unsigned long sw, optional unsigned long sh
-              /*, optional ImageDataSettings settings = {}*/);
+  [Throws] constructor(unsigned long sw, unsigned long sh, optional ImageDataSettings settings = {});
+  [Throws] constructor(ImageDataArray data, unsigned long sw,
+                       optional unsigned long sh, optional ImageDataSettings settings = {});
 
   readonly attribute unsigned long width;
   readonly attribute unsigned long height;
-  [Throws] readonly attribute Uint8ClampedArray data;
-  //readonly attribute PredefinedColorSpace colorSpace;
+  [Throws] readonly attribute ImageDataArray data;
+  readonly attribute ImageDataPixelFormat pixelFormat;
+  readonly attribute PredefinedColorSpace colorSpace;
 };
 
 [Exposed=(Window,Worker)]

tests/wpt/meta/html/canvas/element/color-type/2d.color.type.u8p3.to.f16srgb.to.u8p3.html.ini

@@ -0,0 +1,3 @@
+[2d.color.type.u8p3.to.f16srgb.to.u8p3.html]
+  [test srgb float16 canvas storing 8-bit display-p3 data accurately]
+    expected: FAIL

tests/wpt/meta/html/canvas/element/pixel-manipulation/2d.imageData.createImageBitmap.srgb.rgba.unorm8.html.ini

@@ -1,3 +0,0 @@
-[2d.imageData.createImageBitmap.srgb.rgba.unorm8.html]
-  [Verify round-trip of 8-bit sRGB data ImageData through ImageBitmap]
-    expected: FAIL

tests/wpt/meta/html/canvas/element/pixel-manipulation/2d.imageData.object.properties.html.ini

@@ -1,3 +0,0 @@
-[2d.imageData.object.properties.html]
-  [ImageData objects have the right properties]
-    expected: FAIL

tests/wpt/meta/html/canvas/offscreen/color-type/2d.color.type.u8p3.to.f16srgb.to.u8p3.html.ini

@@ -0,0 +1,3 @@
+[2d.color.type.u8p3.to.f16srgb.to.u8p3.html]
+  [test srgb float16 canvas storing 8-bit display-p3 data accurately]
+    expected: FAIL

tests/wpt/meta/html/canvas/offscreen/color-type/2d.color.type.u8p3.to.f16srgb.to.u8p3.worker.js.ini

@@ -0,0 +1,3 @@
+[2d.color.type.u8p3.to.f16srgb.to.u8p3.worker.html]
+  [test srgb float16 canvas storing 8-bit display-p3 data accurately]
+    expected: FAIL

tests/wpt/meta/html/canvas/offscreen/pixel-manipulation/2d.imageData.createImageBitmap.srgb.rgba.unorm8.html.ini

@@ -1,3 +0,0 @@
-[2d.imageData.createImageBitmap.srgb.rgba.unorm8.html]
-  [Verify round-trip of 8-bit sRGB data ImageData through ImageBitmap]
-    expected: FAIL

tests/wpt/meta/html/canvas/offscreen/pixel-manipulation/2d.imageData.createImageBitmap.srgb.rgba.unorm8.worker.js.ini

@@ -1,3 +0,0 @@
-[2d.imageData.createImageBitmap.srgb.rgba.unorm8.worker.html]
-  [Verify round-trip of 8-bit sRGB data ImageData through ImageBitmap]
-    expected: FAIL

tests/wpt/meta/html/canvas/offscreen/pixel-manipulation/2d.imageData.object.properties.html.ini

@@ -1,3 +0,0 @@
-[2d.imageData.object.properties.html]
-  [ImageData objects have the right properties]
-    expected: FAIL

tests/wpt/meta/html/canvas/offscreen/pixel-manipulation/2d.imageData.object.properties.worker.js.ini

@@ -1,3 +0,0 @@
-[2d.imageData.object.properties.worker.html]
-  [ImageData objects have the right properties]
-    expected: FAIL

tests/wpt/meta/html/dom/idlharness.any.js.ini

@@ -2,9 +2,6 @@
   [CanvasPattern interface: operation setTransform(optional DOMMatrix2DInit)]
     expected: FAIL
 
-  [ImageData interface: attribute colorSpace]
-    expected: FAIL
-
   [Path2D interface: operation roundRect(unrestricted double, unrestricted double, unrestricted double, unrestricted double, optional (unrestricted double or DOMPointInit or sequence<(unrestricted double or DOMPointInit)>))]
     expected: FAIL
 
@@ -185,9 +182,6 @@
   [OffscreenCanvasRenderingContext2D interface: attribute lang]
     expected: FAIL
 
-  [ImageData interface: attribute pixelFormat]
-    expected: FAIL
-
 
 [idlharness.any.shadowrealm-in-window.html]
   expected: ERROR

tests/wpt/meta/html/dom/idlharness.https.html.ini

@@ -950,9 +950,6 @@
   [CanvasRenderingContext2D interface: document.createElement("canvas").getContext("2d") must inherit property "textRendering" with the proper type]
     expected: FAIL
 
-  [ImageData interface: attribute colorSpace]
-    expected: FAIL
-
   [ImageData interface: new ImageData(10, 10) must inherit property "colorSpace" with the proper type]
     expected: FAIL
 
@@ -4349,12 +4346,6 @@
   [CanvasPattern interface: operation setTransform(optional DOMMatrix2DInit)]
     expected: FAIL
 
-  [ImageData interface: attribute colorSpace]
-    expected: FAIL
-
-  [ImageData interface: new ImageData(10, 10) must inherit property "colorSpace" with the proper type]
-    expected: FAIL
-
   [Path2D interface: operation roundRect(unrestricted double, unrestricted double, unrestricted double, unrestricted double, optional (unrestricted double or DOMPointInit or sequence<(unrestricted double or DOMPointInit)>))]
     expected: FAIL
 
@@ -5216,12 +5207,6 @@
   [OffscreenCanvasRenderingContext2D interface: attribute lang]
     expected: FAIL
 
-  [ImageData interface: attribute pixelFormat]
-    expected: FAIL
-
-  [ImageData interface: new ImageData(10, 10) must inherit property "pixelFormat" with the proper type]
-    expected: FAIL
-
   [CustomElementRegistry interface: operation initialize(Node)]
     expected: FAIL