From 60c10d710d53b2e7f30cc90714827a7c31060df3 Mon Sep 17 00:00:00 2001 From: Rodion Borovyk Date: Mon, 30 Jun 2025 15:30:58 +0200 Subject: [PATCH] Set the placeholder canvas element of offscreenCanvas to a weak reference in transferControlToOffscreen() (#37764) Set the placeholder canvas element of offscreenCanvas to a weak reference in transferControlToOffscreen() Testing: I do not understand how to test it, suggestions appreciated Fixes: https://github.com/servo/servo/issues/35626 --------- Signed-off-by: Rodion Borovyk --- components/script/canvas_context.rs | 8 ++-- .../script/dom/canvasrenderingcontext2d.rs | 44 ++++++++++++++----- components/script/dom/htmlcanvaselement.rs | 3 +- components/script/dom/offscreencanvas.rs | 23 +++++----- .../script_bindings/codegen/Bindings.conf | 1 + 5 files changed, 52 insertions(+), 27 deletions(-) diff --git a/components/script/canvas_context.rs b/components/script/canvas_context.rs index 7570a5691af..bc666f2582d 100644 --- a/components/script/canvas_context.rs +++ b/components/script/canvas_context.rs @@ -7,7 +7,7 @@ use euclid::default::Size2D; use layout_api::HTMLCanvasData; use pixels::Snapshot; -use script_bindings::root::Dom; +use script_bindings::root::{Dom, DomRoot}; use webrender_api::ImageKey; use crate::dom::bindings::codegen::UnionTypes::HTMLCanvasElementOrOffscreenCanvas; @@ -81,7 +81,7 @@ pub(crate) trait CanvasContext { pub(crate) trait CanvasHelpers { fn size(&self) -> Size2D; - fn canvas(&self) -> Option<&HTMLCanvasElement>; + fn canvas(&self) -> Option>; } impl CanvasHelpers for HTMLCanvasElementOrOffscreenCanvas { @@ -94,9 +94,9 @@ impl CanvasHelpers for HTMLCanvasElementOrOffscreenCanvas { } } - fn canvas(&self) -> Option<&HTMLCanvasElement> { + fn canvas(&self) -> Option> { match self { - HTMLCanvasElementOrOffscreenCanvas::HTMLCanvasElement(canvas) => Some(canvas), + HTMLCanvasElementOrOffscreenCanvas::HTMLCanvasElement(canvas) => Some(canvas.clone()), HTMLCanvasElementOrOffscreenCanvas::OffscreenCanvas(canvas) => canvas.placeholder(), } } diff --git a/components/script/dom/canvasrenderingcontext2d.rs b/components/script/dom/canvasrenderingcontext2d.rs index 267a0648d4a..cb573a8bd2b 100644 --- a/components/script/dom/canvasrenderingcontext2d.rs +++ b/components/script/dom/canvasrenderingcontext2d.rs @@ -348,15 +348,25 @@ impl CanvasRenderingContext2DMethods for CanvasRenderingCo // https://html.spec.whatwg.org/multipage/#dom-context-2d-filltext fn FillText(&self, text: DOMString, x: f64, y: f64, max_width: Option, can_gc: CanGc) { - self.canvas_state - .fill_text(self.canvas.canvas(), text, x, y, max_width, can_gc); + self.canvas_state.fill_text( + self.canvas.canvas().as_deref(), + text, + x, + y, + max_width, + can_gc, + ); self.mark_as_dirty(); } // https://html.spec.whatwg.org/multipage/#textmetrics fn MeasureText(&self, text: DOMString, can_gc: CanGc) -> DomRoot { - self.canvas_state - .measure_text(&self.global(), self.canvas.canvas(), text, can_gc) + self.canvas_state.measure_text( + &self.global(), + self.canvas.canvas().as_deref(), + text, + can_gc, + ) } // https://html.spec.whatwg.org/multipage/#dom-context-2d-font @@ -367,7 +377,7 @@ impl CanvasRenderingContext2DMethods for CanvasRenderingCo // https://html.spec.whatwg.org/multipage/#dom-context-2d-font fn SetFont(&self, value: DOMString, can_gc: CanGc) { self.canvas_state - .set_font(self.canvas.canvas(), value, can_gc) + .set_font(self.canvas.canvas().as_deref(), value, can_gc) } // https://html.spec.whatwg.org/multipage/#dom-context-2d-textalign @@ -403,7 +413,7 @@ impl CanvasRenderingContext2DMethods for CanvasRenderingCo // https://html.spec.whatwg.org/multipage/#dom-context-2d-drawimage fn DrawImage(&self, image: CanvasImageSource, dx: f64, dy: f64) -> ErrorResult { self.canvas_state - .draw_image(self.canvas.canvas(), image, dx, dy) + .draw_image(self.canvas.canvas().as_deref(), image, dx, dy) } // https://html.spec.whatwg.org/multipage/#dom-context-2d-drawimage @@ -416,7 +426,7 @@ impl CanvasRenderingContext2DMethods for CanvasRenderingCo dh: f64, ) -> ErrorResult { self.canvas_state - .draw_image_(self.canvas.canvas(), image, dx, dy, dw, dh) + .draw_image_(self.canvas.canvas().as_deref(), image, dx, dy, dw, dh) } // https://html.spec.whatwg.org/multipage/#dom-context-2d-drawimage @@ -432,8 +442,18 @@ impl CanvasRenderingContext2DMethods for CanvasRenderingCo dw: f64, dh: f64, ) -> ErrorResult { - self.canvas_state - .draw_image__(self.canvas.canvas(), image, sx, sy, sw, sh, dx, dy, dw, dh) + self.canvas_state.draw_image__( + self.canvas.canvas().as_deref(), + image, + sx, + sy, + sw, + sh, + dx, + dy, + dw, + dh, + ) } // https://html.spec.whatwg.org/multipage/#dom-context-2d-moveto @@ -506,7 +526,7 @@ impl CanvasRenderingContext2DMethods for CanvasRenderingCo // https://html.spec.whatwg.org/multipage/#dom-context-2d-strokestyle fn SetStrokeStyle(&self, value: StringOrCanvasGradientOrCanvasPattern, can_gc: CanGc) { self.canvas_state - .set_stroke_style(self.canvas.canvas(), value, can_gc) + .set_stroke_style(self.canvas.canvas().as_deref(), value, can_gc) } // https://html.spec.whatwg.org/multipage/#dom-context-2d-strokestyle @@ -517,7 +537,7 @@ impl CanvasRenderingContext2DMethods for CanvasRenderingCo // https://html.spec.whatwg.org/multipage/#dom-context-2d-strokestyle fn SetFillStyle(&self, value: StringOrCanvasGradientOrCanvasPattern, can_gc: CanGc) { self.canvas_state - .set_fill_style(self.canvas.canvas(), value, can_gc) + .set_fill_style(self.canvas.canvas().as_deref(), value, can_gc) } // https://html.spec.whatwg.org/multipage/#dom-context-2d-createimagedata @@ -717,6 +737,6 @@ impl CanvasRenderingContext2DMethods for CanvasRenderingCo // https://html.spec.whatwg.org/multipage/#dom-context-2d-shadowcolor fn SetShadowColor(&self, value: DOMString, can_gc: CanGc) { self.canvas_state - .set_shadow_color(self.canvas.canvas(), value, can_gc) + .set_shadow_color(self.canvas.canvas().as_deref(), value, can_gc) } } diff --git a/components/script/dom/htmlcanvaselement.rs b/components/script/dom/htmlcanvaselement.rs index 8539bca6183..98d8637d942 100644 --- a/components/script/dom/htmlcanvaselement.rs +++ b/components/script/dom/htmlcanvaselement.rs @@ -23,6 +23,7 @@ use js::error::throw_type_error; use js::rust::{HandleObject, HandleValue}; use layout_api::HTMLCanvasData; use pixels::{Snapshot, SnapshotAlphaMode, SnapshotPixelFormat}; +use script_bindings::weakref::WeakRef; use servo_media::streams::MediaStreamType; use servo_media::streams::registry::MediaStreamId; use style::attr::AttrValue; @@ -675,7 +676,7 @@ impl HTMLCanvasElementMethods for HTMLCanvasElement { None, self.Width().into(), self.Height().into(), - Some(&Dom::from_ref(self)), + Some(WeakRef::new(self)), can_gc, ); diff --git a/components/script/dom/offscreencanvas.rs b/components/script/dom/offscreencanvas.rs index b890f872f8f..80caeef9dce 100644 --- a/components/script/dom/offscreencanvas.rs +++ b/components/script/dom/offscreencanvas.rs @@ -8,6 +8,7 @@ use dom_struct::dom_struct; use euclid::default::Size2D; use js::rust::{HandleObject, HandleValue}; use pixels::Snapshot; +use script_bindings::weakref::WeakRef; use crate::canvas_context::{CanvasContext, OffscreenRenderingContext}; use crate::dom::bindings::cell::{DomRefCell, Ref}; @@ -38,21 +39,21 @@ pub(crate) struct OffscreenCanvas { context: DomRefCell>, /// - placeholder: Option>, + placeholder: Option>, } impl OffscreenCanvas { pub(crate) fn new_inherited( width: u64, height: u64, - placeholder: Option<&HTMLCanvasElement>, + placeholder: Option>, ) -> OffscreenCanvas { OffscreenCanvas { eventtarget: EventTarget::new_inherited(), width: Cell::new(width), height: Cell::new(height), context: DomRefCell::new(None), - placeholder: placeholder.map(Dom::from_ref), + placeholder, } } @@ -61,7 +62,7 @@ impl OffscreenCanvas { proto: Option, width: u64, height: u64, - placeholder: Option<&HTMLCanvasElement>, + placeholder: Option>, can_gc: CanGc, ) -> DomRoot { reflect_dom_object_with_proto( @@ -126,8 +127,10 @@ impl OffscreenCanvas { Some(context) } - pub(crate) fn placeholder(&self) -> Option<&HTMLCanvasElement> { - self.placeholder.as_deref() + pub(crate) fn placeholder(&self) -> Option> { + self.placeholder + .as_ref() + .and_then(|placeholder| placeholder.root()) } } @@ -181,8 +184,8 @@ impl OffscreenCanvasMethods for OffscreenCanvas { canvas_context.resize(); } - if let Some(canvas) = &self.placeholder { - canvas.set_natural_width(value as _, can_gc); + if let Some(canvas) = self.placeholder() { + canvas.set_natural_width(value as _, can_gc) } } @@ -199,8 +202,8 @@ impl OffscreenCanvasMethods for OffscreenCanvas { canvas_context.resize(); } - if let Some(canvas) = &self.placeholder { - canvas.set_natural_height(value as _, can_gc); + if let Some(canvas) = self.placeholder() { + canvas.set_natural_height(value as _, can_gc) } } } diff --git a/components/script_bindings/codegen/Bindings.conf b/components/script_bindings/codegen/Bindings.conf index eb56cfee88c..23547d8ac5c 100644 --- a/components/script_bindings/codegen/Bindings.conf +++ b/components/script_bindings/codegen/Bindings.conf @@ -345,6 +345,7 @@ DOMInterfaces = { 'HTMLCanvasElement': { 'canGc': ['CaptureStream', 'GetContext', 'SetHeight', 'SetWidth', 'TransferControlToOffscreen'], + 'weakReferenceable': True, }, 'HTMLDataListElement': {