From 1e514bcfe99388e73fc7ff4bcd996bc3eb20d196 Mon Sep 17 00:00:00 2001 From: Domenico Rizzo Date: Sun, 22 Mar 2026 21:33:35 +0100 Subject: [PATCH] webgl: Remove manual Drop implementation for WebGLQuery (#43544) Moves the cleanup logic to a separate helper struct to comply with the prohibition of manual `Drop` implementations for DOM types. Testing: WebGL tests just cover its parts Fixes: Partially #26488 Signed-off-by: Domenico Rizzo --- components/script/dom/webgl/webglquery.rs | 75 ++++++++++++------- .../script_bindings/codegen/Bindings.conf | 4 - 2 files changed, 50 insertions(+), 29 deletions(-) diff --git a/components/script/dom/webgl/webglquery.rs b/components/script/dom/webgl/webglquery.rs index b7539c70968..b6828f6ffdf 100644 --- a/components/script/dom/webgl/webglquery.rs +++ b/components/script/dom/webgl/webglquery.rs @@ -7,36 +7,73 @@ use std::cell::Cell; use canvas_traits::webgl::WebGLError::*; use canvas_traits::webgl::{WebGLCommand, WebGLQueryId, webgl_channel}; use dom_struct::dom_struct; +use script_bindings::weakref::WeakRef; use crate::dom::bindings::codegen::Bindings::WebGL2RenderingContextBinding::WebGL2RenderingContextConstants as constants; -use crate::dom::bindings::inheritance::Castable; use crate::dom::bindings::refcounted::Trusted; use crate::dom::bindings::reflector::{DomGlobal, reflect_dom_object}; use crate::dom::bindings::root::DomRoot; use crate::dom::webgl::webglobject::WebGLObject; use crate::dom::webgl::webglrenderingcontext::{Operation, WebGLRenderingContext}; +use crate::dom::webglrenderingcontext::capture_webgl_backtrace; use crate::script_runtime::CanGc; +#[derive(JSTraceable, MallocSizeOf)] +struct DroppableWebGLQuery { + context: WeakRef, + #[no_trace] + gl_id: WebGLQueryId, + marked_for_deletion: Cell, +} + +impl DroppableWebGLQuery { + fn send_with_fallibility(&self, command: WebGLCommand, fallibility: Operation) { + if let Some(root) = self.context.root() { + let result = root.sender().send(command, capture_webgl_backtrace()); + if matches!(fallibility, Operation::Infallible) { + result.expect("Operation failed"); + } + } + } + + fn delete(&self, operation_fallibility: Operation) { + if !self.marked_for_deletion.get() { + self.marked_for_deletion.set(true); + self.send_with_fallibility( + WebGLCommand::DeleteQuery(self.gl_id), + operation_fallibility, + ); + } + } +} + +impl Drop for DroppableWebGLQuery { + fn drop(&mut self) { + self.delete(Operation::Fallible); + } +} + #[dom_struct(associated_memory)] pub(crate) struct WebGLQuery { webgl_object: WebGLObject, - #[no_trace] - gl_id: WebGLQueryId, gl_target: Cell>, - marked_for_deletion: Cell, query_result_available: Cell>, query_result: Cell, + droppable: DroppableWebGLQuery, } impl WebGLQuery { fn new_inherited(context: &WebGLRenderingContext, id: WebGLQueryId) -> Self { Self { webgl_object: WebGLObject::new_inherited(context), - gl_id: id, gl_target: Cell::new(None), - marked_for_deletion: Cell::new(false), query_result_available: Cell::new(None), query_result: Cell::new(0), + droppable: DroppableWebGLQuery { + context: WeakRef::new(context), + gl_id: id, + marked_for_deletion: Cell::new(false), + }, } } @@ -57,7 +94,7 @@ impl WebGLQuery { context: &WebGLRenderingContext, target: u32, ) -> Result<(), canvas_traits::webgl::WebGLError> { - if self.marked_for_deletion.get() { + if self.droppable.marked_for_deletion.get() { return Err(InvalidOperation); } if let Some(current_target) = self.gl_target.get() { @@ -73,7 +110,7 @@ impl WebGLQuery { } self.gl_target.set(Some(target)); - context.send_command(WebGLCommand::BeginQuery(target, self.gl_id)); + context.send_command(WebGLCommand::BeginQuery(target, self.droppable.gl_id)); Ok(()) } @@ -82,7 +119,7 @@ impl WebGLQuery { context: &WebGLRenderingContext, target: u32, ) -> Result<(), canvas_traits::webgl::WebGLError> { - if self.marked_for_deletion.get() { + if self.droppable.marked_for_deletion.get() { return Err(InvalidOperation); } if let Some(current_target) = self.gl_target.get() { @@ -101,17 +138,11 @@ impl WebGLQuery { } pub(crate) fn delete(&self, operation_fallibility: Operation) { - if !self.marked_for_deletion.get() { - self.marked_for_deletion.set(true); - self.upcast().send_with_fallibility( - WebGLCommand::DeleteQuery(self.gl_id), - operation_fallibility, - ); - } + self.droppable.delete(operation_fallibility); } pub(crate) fn is_valid(&self) -> bool { - !self.marked_for_deletion.get() && self.target().is_some() + !self.droppable.marked_for_deletion.get() && self.target().is_some() } pub(crate) fn target(&self) -> Option { @@ -122,7 +153,7 @@ impl WebGLQuery { let (sender, receiver) = webgl_channel().unwrap(); context.send_command(WebGLCommand::GetQueryState( sender, - self.gl_id, + self.droppable.gl_id, constants::QUERY_RESULT_AVAILABLE, )); let is_available = receiver.recv().unwrap(); @@ -134,7 +165,7 @@ impl WebGLQuery { let (sender, receiver) = webgl_channel().unwrap(); context.send_command(WebGLCommand::GetQueryState( sender, - self.gl_id, + self.droppable.gl_id, constants::QUERY_RESULT, )); @@ -181,9 +212,3 @@ impl WebGLQuery { } } } - -impl Drop for WebGLQuery { - fn drop(&mut self) { - self.delete(Operation::Fallible); - } -} diff --git a/components/script_bindings/codegen/Bindings.conf b/components/script_bindings/codegen/Bindings.conf index f6dfdb93a24..f2804dc1381 100644 --- a/components/script_bindings/codegen/Bindings.conf +++ b/components/script_bindings/codegen/Bindings.conf @@ -824,10 +824,6 @@ DOMInterfaces = { 'additionalTraits': ['crate::interfaces::WebGL2RenderingContextHelpers'], }, -'WebGLQuery': { - 'allowDropImpl': True, -}, - 'WebGLRenderbuffer': { 'allowDropImpl': True, },