From e69962e646912d926d03cfe960737ed64e6ccf60 Mon Sep 17 00:00:00 2001 From: "Ngo Iok Ui (Wu Yu Wei)" Date: Mon, 16 Jun 2025 14:39:56 +0900 Subject: [PATCH] canvas: prevent unwrap on offscreen canvas (#37460) Remove all unwrap usage on offscreen canvas to prevent panic. Testing: `tests/wpt/tests/html/canvas/offscreen/manual/the-offscreen-canvas/offscreencanvas.transfercontrol.to.offscreen-crash.html` Fixes: #37415 --------- Signed-off-by: Wu Yu Wei Signed-off-by: Yu Wei Wu Co-authored-by: Yu Wei Wu --- components/script/canvas_context.rs | 52 +++++++++++++------ .../script/dom/canvasrenderingcontext2d.rs | 4 +- .../dom/offscreencanvasrenderingcontext2d.rs | 4 +- .../script/dom/webgl2renderingcontext.rs | 2 +- .../script/dom/webglrenderingcontext.rs | 4 +- .../script/dom/webgpu/gpucanvascontext.rs | 4 +- tests/wpt/meta/MANIFEST.json | 7 +++ ...as.transfercontrol.to.offscreen-crash.html | 7 +++ 8 files changed, 58 insertions(+), 26 deletions(-) create mode 100644 tests/wpt/tests/html/canvas/offscreen/manual/the-offscreen-canvas/offscreencanvas.transfercontrol.to.offscreen-crash.html diff --git a/components/script/canvas_context.rs b/components/script/canvas_context.rs index 6911cd7c8fb..31e893729d1 100644 --- a/components/script/canvas_context.rs +++ b/components/script/canvas_context.rs @@ -35,7 +35,7 @@ pub(crate) trait CanvasContext { fn context_id(&self) -> Self::ID; - fn canvas(&self) -> HTMLCanvasElementOrOffscreenCanvas; + fn canvas(&self) -> Option; fn resize(&self); @@ -49,11 +49,14 @@ pub(crate) trait CanvasContext { } fn size(&self) -> Size2D { - self.canvas().size() + self.canvas() + .map(|canvas| canvas.size()) + .unwrap_or_default() } fn mark_as_dirty(&self) { - if let HTMLCanvasElementOrOffscreenCanvas::HTMLCanvasElement(canvas) = &self.canvas() { + if let Some(HTMLCanvasElementOrOffscreenCanvas::HTMLCanvasElement(canvas)) = &self.canvas() + { canvas.upcast::().dirty(NodeDamage::Other); } } @@ -61,7 +64,11 @@ pub(crate) trait CanvasContext { fn update_rendering(&self) {} fn onscreen(&self) -> bool { - match self.canvas() { + let Some(canvas) = self.canvas() else { + return false; + }; + + match canvas { HTMLCanvasElementOrOffscreenCanvas::HTMLCanvasElement(ref canvas) => { canvas.upcast::().is_connected() }, @@ -112,9 +119,9 @@ impl CanvasContext for RenderingContext { fn context_id(&self) -> Self::ID {} - fn canvas(&self) -> HTMLCanvasElementOrOffscreenCanvas { + fn canvas(&self) -> Option { match self { - RenderingContext::Placeholder(context) => (*context.context().unwrap()).canvas(), + RenderingContext::Placeholder(offscreen_canvas) => offscreen_canvas.context()?.canvas(), RenderingContext::Context2d(context) => context.canvas(), RenderingContext::WebGL(context) => context.canvas(), RenderingContext::WebGL2(context) => context.canvas(), @@ -140,8 +147,8 @@ impl CanvasContext for RenderingContext { fn get_image_data(&self) -> Option { match self { - RenderingContext::Placeholder(context) => { - (*context.context().unwrap()).get_image_data() + RenderingContext::Placeholder(offscreen_canvas) => { + offscreen_canvas.context()?.get_image_data() }, RenderingContext::Context2d(context) => context.get_image_data(), RenderingContext::WebGL(context) => context.get_image_data(), @@ -153,9 +160,9 @@ impl CanvasContext for RenderingContext { fn origin_is_clean(&self) -> bool { match self { - RenderingContext::Placeholder(context) => { - (*context.context().unwrap()).origin_is_clean() - }, + RenderingContext::Placeholder(offscreen_canvas) => offscreen_canvas + .context() + .is_none_or(|context| context.origin_is_clean()), RenderingContext::Context2d(context) => context.origin_is_clean(), RenderingContext::WebGL(context) => context.origin_is_clean(), RenderingContext::WebGL2(context) => context.origin_is_clean(), @@ -166,7 +173,10 @@ impl CanvasContext for RenderingContext { fn size(&self) -> Size2D { match self { - RenderingContext::Placeholder(context) => (*context.context().unwrap()).size(), + RenderingContext::Placeholder(offscreen_canvas) => offscreen_canvas + .context() + .map(|context| context.size()) + .unwrap_or_default(), RenderingContext::Context2d(context) => context.size(), RenderingContext::WebGL(context) => context.size(), RenderingContext::WebGL2(context) => context.size(), @@ -177,7 +187,11 @@ impl CanvasContext for RenderingContext { fn mark_as_dirty(&self) { match self { - RenderingContext::Placeholder(context) => (*context.context().unwrap()).mark_as_dirty(), + RenderingContext::Placeholder(offscreen_canvas) => { + if let Some(context) = offscreen_canvas.context() { + context.mark_as_dirty() + } + }, RenderingContext::Context2d(context) => context.mark_as_dirty(), RenderingContext::WebGL(context) => context.mark_as_dirty(), RenderingContext::WebGL2(context) => context.mark_as_dirty(), @@ -188,8 +202,10 @@ impl CanvasContext for RenderingContext { fn update_rendering(&self) { match self { - RenderingContext::Placeholder(context) => { - (*context.context().unwrap()).update_rendering() + RenderingContext::Placeholder(offscreen_canvas) => { + if let Some(context) = offscreen_canvas.context() { + context.update_rendering() + } }, RenderingContext::Context2d(context) => context.update_rendering(), RenderingContext::WebGL(context) => context.update_rendering(), @@ -201,7 +217,9 @@ impl CanvasContext for RenderingContext { fn onscreen(&self) -> bool { match self { - RenderingContext::Placeholder(context) => (*context.context().unwrap()).onscreen(), + RenderingContext::Placeholder(offscreen_canvas) => offscreen_canvas + .context() + .is_some_and(|context| context.onscreen()), RenderingContext::Context2d(context) => context.onscreen(), RenderingContext::WebGL(context) => context.onscreen(), RenderingContext::WebGL2(context) => context.onscreen(), @@ -227,7 +245,7 @@ impl CanvasContext for OffscreenRenderingContext { fn context_id(&self) -> Self::ID {} - fn canvas(&self) -> HTMLCanvasElementOrOffscreenCanvas { + fn canvas(&self) -> Option { match self { OffscreenRenderingContext::Context2d(context) => context.canvas(), } diff --git a/components/script/dom/canvasrenderingcontext2d.rs b/components/script/dom/canvasrenderingcontext2d.rs index d1430f87c05..859454a3064 100644 --- a/components/script/dom/canvasrenderingcontext2d.rs +++ b/components/script/dom/canvasrenderingcontext2d.rs @@ -140,8 +140,8 @@ impl CanvasContext for CanvasRenderingContext2D { self.canvas_state.get_canvas_id() } - fn canvas(&self) -> HTMLCanvasElementOrOffscreenCanvas { - self.canvas.clone() + fn canvas(&self) -> Option { + Some(self.canvas.clone()) } fn update_rendering(&self) { diff --git a/components/script/dom/offscreencanvasrenderingcontext2d.rs b/components/script/dom/offscreencanvasrenderingcontext2d.rs index d7ca0e9dc4d..f5ed2ca28e7 100644 --- a/components/script/dom/offscreencanvasrenderingcontext2d.rs +++ b/components/script/dom/offscreencanvasrenderingcontext2d.rs @@ -75,7 +75,7 @@ impl CanvasContext for OffscreenCanvasRenderingContext2D { self.context.context_id() } - fn canvas(&self) -> HTMLCanvasElementOrOffscreenCanvas { + fn canvas(&self) -> Option { self.context.canvas() } @@ -98,7 +98,7 @@ impl OffscreenCanvasRenderingContext2DMethods // https://html.spec.whatwg.org/multipage/offscreencontext2d-canvas fn Canvas(&self) -> DomRoot { match self.context.canvas() { - HTMLCanvasElementOrOffscreenCanvas::OffscreenCanvas(canvas) => canvas, + Some(HTMLCanvasElementOrOffscreenCanvas::OffscreenCanvas(canvas)) => canvas, _ => panic!("Should not be called from onscreen canvas"), } } diff --git a/components/script/dom/webgl2renderingcontext.rs b/components/script/dom/webgl2renderingcontext.rs index 8f2bf9c2fcc..b237304ad44 100644 --- a/components/script/dom/webgl2renderingcontext.rs +++ b/components/script/dom/webgl2renderingcontext.rs @@ -908,7 +908,7 @@ impl CanvasContext for WebGL2RenderingContext { self.base.context_id() } - fn canvas(&self) -> HTMLCanvasElementOrOffscreenCanvas { + fn canvas(&self) -> Option { self.base.canvas().clone() } diff --git a/components/script/dom/webglrenderingcontext.rs b/components/script/dom/webglrenderingcontext.rs index 0b34abc1756..1cd246e39d1 100644 --- a/components/script/dom/webglrenderingcontext.rs +++ b/components/script/dom/webglrenderingcontext.rs @@ -1914,8 +1914,8 @@ impl CanvasContext for WebGLRenderingContext { self.webgl_sender.context_id() } - fn canvas(&self) -> HTMLCanvasElementOrOffscreenCanvas { - self.canvas.clone() + fn canvas(&self) -> Option { + Some(self.canvas.clone()) } fn resize(&self) { diff --git a/components/script/dom/webgpu/gpucanvascontext.rs b/components/script/dom/webgpu/gpucanvascontext.rs index 9bf43c77bf5..b4b83da1e6d 100644 --- a/components/script/dom/webgpu/gpucanvascontext.rs +++ b/components/script/dom/webgpu/gpucanvascontext.rs @@ -294,8 +294,8 @@ impl CanvasContext for GPUCanvasContext { }) } - fn canvas(&self) -> HTMLCanvasElementOrOffscreenCanvas { - self.canvas.clone() + fn canvas(&self) -> Option { + Some(self.canvas.clone()) } } diff --git a/tests/wpt/meta/MANIFEST.json b/tests/wpt/meta/MANIFEST.json index 3d8326fe35b..f5bd98a4e6c 100644 --- a/tests/wpt/meta/MANIFEST.json +++ b/tests/wpt/meta/MANIFEST.json @@ -7834,6 +7834,13 @@ null, {} ] + ], + "offscreencanvas.transfercontrol.to.offscreen-crash.html": [ + "949f49c1065cb949cc146d2246a0159c777cc272", + [ + null, + {} + ] ] } }, diff --git a/tests/wpt/tests/html/canvas/offscreen/manual/the-offscreen-canvas/offscreencanvas.transfercontrol.to.offscreen-crash.html b/tests/wpt/tests/html/canvas/offscreen/manual/the-offscreen-canvas/offscreencanvas.transfercontrol.to.offscreen-crash.html new file mode 100644 index 00000000000..949f49c1065 --- /dev/null +++ b/tests/wpt/tests/html/canvas/offscreen/manual/the-offscreen-canvas/offscreencanvas.transfercontrol.to.offscreen-crash.html @@ -0,0 +1,7 @@ +Transfer Canvas without Context + + +