From dc0e7587bf965047358254be1da5efa41079ee96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20W=C3=BClker?= Date: Sun, 11 May 2025 17:27:33 +0200 Subject: [PATCH] Don't attempt to resize Offscreencanvas without a rendering context (#36855) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the canvas context mode is a placeholder then we shouldn't resize the context. Testing: Includes a new test Fixes: https://github.com/servo/servo/issues/36846 --------- Signed-off-by: Simon Wülker --- components/script/canvas_context.rs | 6 +- components/script/dom/htmlcanvaselement.rs | 55 +++++++++++-------- components/script/dom/offscreencanvas.rs | 20 +++++-- tests/wpt/mozilla/meta/MANIFEST.json | 7 +++ ...encanvas-then-change-dimensions-crash.html | 7 +++ 5 files changed, 65 insertions(+), 30 deletions(-) create mode 100644 tests/wpt/mozilla/tests/mozilla/canvas/transfer-control-to-offscreencanvas-then-change-dimensions-crash.html diff --git a/components/script/canvas_context.rs b/components/script/canvas_context.rs index 7dfdf48e3f5..ec388e039f1 100644 --- a/components/script/canvas_context.rs +++ b/components/script/canvas_context.rs @@ -125,7 +125,11 @@ impl CanvasContext for RenderingContext { fn resize(&self) { match self { - RenderingContext::Placeholder(context) => (*context.context().unwrap()).resize(), + RenderingContext::Placeholder(offscreen_canvas) => { + if let Some(context) = offscreen_canvas.context() { + context.resize() + } + }, RenderingContext::Context2d(context) => context.resize(), RenderingContext::WebGL(context) => context.resize(), RenderingContext::WebGL2(context) => context.resize(), diff --git a/components/script/dom/htmlcanvaselement.rs b/components/script/dom/htmlcanvaselement.rs index 56e008839ba..c2bfc9c2d7f 100644 --- a/components/script/dom/htmlcanvaselement.rs +++ b/components/script/dom/htmlcanvaselement.rs @@ -103,10 +103,14 @@ impl EncodedImageType { } } +/// #[dom_struct] pub(crate) struct HTMLCanvasElement { htmlelement: HTMLElement, - context: DomRefCell>, + + /// + context_mode: DomRefCell>, + // This id and hashmap are used to keep track of ongoing toBlob() calls. callback_id: Cell, #[ignore_malloc_size_of = "not implemented for webidl callbacks"] @@ -121,7 +125,7 @@ impl HTMLCanvasElement { ) -> HTMLCanvasElement { HTMLCanvasElement { htmlelement: HTMLElement::new_inherited(local_name, prefix, document), - context: DomRefCell::new(None), + context_mode: DomRefCell::new(None), callback_id: Cell::new(0), blob_callbacks: RefCell::new(HashMap::new()), } @@ -146,7 +150,7 @@ impl HTMLCanvasElement { } fn recreate_contexts_after_resize(&self) { - if let Some(ref context) = *self.context.borrow() { + if let Some(ref context) = *self.context_mode.borrow() { context.resize() } } @@ -156,14 +160,14 @@ impl HTMLCanvasElement { } pub(crate) fn origin_is_clean(&self) -> bool { - match *self.context.borrow() { + match *self.context_mode.borrow() { Some(ref context) => context.origin_is_clean(), _ => true, } } pub(crate) fn mark_as_dirty(&self) { - if let Some(ref context) = *self.context.borrow() { + if let Some(ref context) = *self.context_mode.borrow() { context.mark_as_dirty() } } @@ -193,7 +197,7 @@ impl LayoutHTMLCanvasElementHelpers for LayoutDom<'_, HTMLCanvasElement> { #[allow(unsafe_code)] fn data(self) -> HTMLCanvasData { let source = unsafe { - match self.unsafe_get().context.borrow_for_layout().as_ref() { + match self.unsafe_get().context_mode.borrow_for_layout().as_ref() { Some(RenderingContext::Context2d(context)) => { context.to_layout().canvas_data_source() }, @@ -221,7 +225,7 @@ impl LayoutHTMLCanvasElementHelpers for LayoutDom<'_, HTMLCanvasElement> { impl HTMLCanvasElement { pub(crate) fn context(&self) -> Option> { - ref_filter_map(self.context.borrow(), |ctx| ctx.as_ref()) + ref_filter_map(self.context_mode.borrow(), |ctx| ctx.as_ref()) } fn get_or_init_2d_context(&self, can_gc: CanGc) -> Option> { @@ -235,7 +239,8 @@ impl HTMLCanvasElement { let window = self.owner_window(); let size = self.get_size(); let context = CanvasRenderingContext2D::new(window.as_global_scope(), self, size, can_gc); - *self.context.borrow_mut() = Some(RenderingContext::Context2d(Dom::from_ref(&*context))); + *self.context_mode.borrow_mut() = + Some(RenderingContext::Context2d(Dom::from_ref(&*context))); Some(context) } @@ -263,7 +268,7 @@ impl HTMLCanvasElement { attrs, can_gc, )?; - *self.context.borrow_mut() = Some(RenderingContext::WebGL(Dom::from_ref(&*context))); + *self.context_mode.borrow_mut() = Some(RenderingContext::WebGL(Dom::from_ref(&*context))); Some(context) } @@ -288,7 +293,7 @@ impl HTMLCanvasElement { let attrs = Self::get_gl_attributes(cx, options)?; let canvas = HTMLCanvasElementOrOffscreenCanvas::HTMLCanvasElement(DomRoot::from_ref(self)); let context = WebGL2RenderingContext::new(&window, &canvas, size, attrs, can_gc)?; - *self.context.borrow_mut() = Some(RenderingContext::WebGL2(Dom::from_ref(&*context))); + *self.context_mode.borrow_mut() = Some(RenderingContext::WebGL2(Dom::from_ref(&*context))); Some(context) } @@ -315,7 +320,7 @@ impl HTMLCanvasElement { .expect("Failed to get WebGPU channel") .map(|channel| { let context = GPUCanvasContext::new(&global_scope, self, channel, can_gc); - *self.context.borrow_mut() = + *self.context_mode.borrow_mut() = Some(RenderingContext::WebGPU(Dom::from_ref(&*context))); context }) @@ -323,7 +328,7 @@ impl HTMLCanvasElement { /// Gets the base WebGLRenderingContext for WebGL or WebGL 2, if exists. pub(crate) fn get_base_webgl_context(&self) -> Option> { - match *self.context.borrow() { + match *self.context_mode.borrow() { Some(RenderingContext::WebGL(ref context)) => Some(DomRoot::from_ref(context)), Some(RenderingContext::WebGL2(ref context)) => Some(context.base_context()), _ => None, @@ -352,7 +357,7 @@ impl HTMLCanvasElement { } pub(crate) fn get_image_data(&self) -> Option { - match self.context.borrow().as_ref() { + match self.context_mode.borrow().as_ref() { Some(context) => context.get_image_data(), None => { let size = self.get_size(); @@ -431,12 +436,12 @@ impl HTMLCanvasElementMethods for HTMLCanvasElement { // https://html.spec.whatwg.org/multipage/#dom-canvas-width make_uint_getter!(Width, "width", DEFAULT_WIDTH); - // https://html.spec.whatwg.org/multipage/#dom-canvas-width - // When setting the value of the width or height attribute, if the context mode of the canvas element - // is set to placeholder, the user agent must throw an "InvalidStateError" DOMException and leave the - // attribute's value unchanged. + /// fn SetWidth(&self, value: u32, can_gc: CanGc) -> Fallible<()> { - if let Some(RenderingContext::Placeholder(_)) = *self.context.borrow() { + // > When setting the value of the width or height attribute, if the context mode of the canvas element + // > is set to placeholder, the user agent must throw an "InvalidStateError" DOMException and leave the + // > attribute's value unchanged. + if let Some(RenderingContext::Placeholder(_)) = *self.context_mode.borrow() { return Err(Error::InvalidState); } @@ -453,9 +458,12 @@ impl HTMLCanvasElementMethods for HTMLCanvasElement { // https://html.spec.whatwg.org/multipage/#dom-canvas-height make_uint_getter!(Height, "height", DEFAULT_HEIGHT); - // https://html.spec.whatwg.org/multipage/#dom-canvas-height + /// fn SetHeight(&self, value: u32, can_gc: CanGc) -> Fallible<()> { - if let Some(RenderingContext::Placeholder(_)) = *self.context.borrow() { + // > When setting the value of the width or height attribute, if the context mode of the canvas element + // > is set to placeholder, the user agent must throw an "InvalidStateError" DOMException and leave the + // > attribute's value unchanged. + if let Some(RenderingContext::Placeholder(_)) = *self.context_mode.borrow() { return Err(Error::InvalidState); } @@ -478,7 +486,7 @@ impl HTMLCanvasElementMethods for HTMLCanvasElement { can_gc: CanGc, ) -> Fallible> { // Always throw an InvalidState exception when the canvas is in Placeholder mode (See table in the spec). - if let Some(RenderingContext::Placeholder(_)) = *self.context.borrow() { + if let Some(RenderingContext::Placeholder(_)) = *self.context_mode.borrow() { return Err(Error::InvalidState); } @@ -622,7 +630,7 @@ impl HTMLCanvasElementMethods for HTMLCanvasElement { /// fn TransferControlToOffscreen(&self, can_gc: CanGc) -> Fallible> { - if self.context.borrow().is_some() { + if self.context_mode.borrow().is_some() { // Step 1. // If this canvas element's context mode is not set to none, throw an "InvalidStateError" DOMException. return Err(Error::InvalidState); @@ -641,8 +649,9 @@ impl HTMLCanvasElementMethods for HTMLCanvasElement { Some(&Dom::from_ref(self)), can_gc, ); + // Step 4. Set this canvas element's context mode to placeholder. - *self.context.borrow_mut() = + *self.context_mode.borrow_mut() = Some(RenderingContext::Placeholder(offscreen_canvas.as_traced())); // Step 5. Return offscreenCanvas. diff --git a/components/script/dom/offscreencanvas.rs b/components/script/dom/offscreencanvas.rs index 9947d35f4e0..bceed49ac7d 100644 --- a/components/script/dom/offscreencanvas.rs +++ b/components/script/dom/offscreencanvas.rs @@ -24,12 +24,20 @@ use crate::dom::htmlcanvaselement::HTMLCanvasElement; use crate::dom::offscreencanvasrenderingcontext2d::OffscreenCanvasRenderingContext2D; use crate::script_runtime::{CanGc, JSContext}; +/// #[dom_struct] pub(crate) struct OffscreenCanvas { eventtarget: EventTarget, width: Cell, height: Cell, + + /// Represents both the [bitmap] and the [context mode] of the canvas. + /// + /// [bitmap]: https://html.spec.whatwg.org/multipage/#offscreencanvas-bitmap + /// [context mode]: https://html.spec.whatwg.org/multipage/#offscreencanvas-context-mode context: DomRefCell>, + + /// placeholder: Option>, } @@ -119,7 +127,7 @@ impl OffscreenCanvas { } impl OffscreenCanvasMethods for OffscreenCanvas { - // https://html.spec.whatwg.org/multipage/#dom-offscreencanvas + /// fn Constructor( global: &GlobalScope, proto: Option, @@ -131,7 +139,7 @@ impl OffscreenCanvasMethods for OffscreenCanvas { Ok(offscreencanvas) } - // https://html.spec.whatwg.org/multipage/#dom-offscreencanvas-getcontext + /// fn GetContext( &self, _cx: JSContext, @@ -155,12 +163,12 @@ impl OffscreenCanvasMethods for OffscreenCanvas { } } - // https://html.spec.whatwg.org/multipage/#dom-offscreencanvas-width + /// fn Width(&self) -> u64 { self.width.get() } - // https://html.spec.whatwg.org/multipage/#dom-offscreencanvas-width + /// fn SetWidth(&self, value: u64, can_gc: CanGc) { self.width.set(value); @@ -173,12 +181,12 @@ impl OffscreenCanvasMethods for OffscreenCanvas { } } - // https://html.spec.whatwg.org/multipage/#dom-offscreencanvas-height + /// fn Height(&self) -> u64 { self.height.get() } - // https://html.spec.whatwg.org/multipage/#dom-offscreencanvas-height + /// fn SetHeight(&self, value: u64, can_gc: CanGc) { self.height.set(value); diff --git a/tests/wpt/mozilla/meta/MANIFEST.json b/tests/wpt/mozilla/meta/MANIFEST.json index 296804116f8..033a13106ac 100644 --- a/tests/wpt/mozilla/meta/MANIFEST.json +++ b/tests/wpt/mozilla/meta/MANIFEST.json @@ -16,6 +16,13 @@ null, {} ] + ], + "transfer-control-to-offscreencanvas-then-change-dimensions-crash.html": [ + "bc0c7afd6ab2e1c313bb61ea4a5c68b402105f5d", + [ + null, + {} + ] ] }, "datatransferitem-crash.html": [ diff --git a/tests/wpt/mozilla/tests/mozilla/canvas/transfer-control-to-offscreencanvas-then-change-dimensions-crash.html b/tests/wpt/mozilla/tests/mozilla/canvas/transfer-control-to-offscreencanvas-then-change-dimensions-crash.html new file mode 100644 index 00000000000..bc0c7afd6ab --- /dev/null +++ b/tests/wpt/mozilla/tests/mozilla/canvas/transfer-control-to-offscreencanvas-then-change-dimensions-crash.html @@ -0,0 +1,7 @@ + + +