Bug 238719 – [WebGPU] Add support for the "is valid to use with" operation to WebGPU objects

• Status: RESOLVED
• Resolution: FIXED
• Component: WebGPU

[Myles C. Maxfield]

.

[Myles C. Maxfield]

Created attachment 456535 [details]
Patch

[Myles C. Maxfield]

Created attachment 456546 [details]
Patch

[Kimmo Kinnunen]

Comment on attachment 456546 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=456546&action=review

also changes refcounts to thread safe (which is probably good)

> Source/WebGPU/WebGPU/ObjectBase.h:40
> +    ~ObjectBase();

Surprisingly ThreadSafeRefCounted has a footgun  where the destructor is not a virtual.
So written this way, Ref<ObjectBase> would never run the subclass destructors, which is a hard to find down the line if you ever end up using that.
It's better to mark this virtual.

If you don't intend to support Ref<ObjectBase>, then it's better to leave the threadsaferefcounted in the held classes.

[Myles C. Maxfield]

Comment on attachment 456546 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=456546&action=review

>> Source/WebGPU/WebGPU/ObjectBase.h:40
>> +    ~ObjectBase();
> 
> Surprisingly ThreadSafeRefCounted has a footgun  where the destructor is not a virtual.
> So written this way, Ref<ObjectBase> would never run the subclass destructors, which is a hard to find down the line if you ever end up using that.
> It's better to mark this virtual.
> 
> If you don't intend to support Ref<ObjectBase>, then it's better to leave the threadsaferefcounted in the held classes.

Thinking about this more, I think there may actually be a better way of doing this. The whole point of this ObjectBase class is just to hold the isValidToUseWith() function, which is already templated. So, if it's already templated, it doesn't really need to actually exist in the base class. It can just be a free function and call .device() on the templated type. Doing it that way will get rid of this whole type hierarchy, and the problem you're describing here, as well as reducing total amount of code.

[Myles C. Maxfield]

Committed r292558 (249396@trunk): <https://commits.webkit.org/249396@trunk>

[Radar WebKit Bug Importer]

<rdar://problem/91440590>

[Myles C. Maxfield]

Committed r292574 (?): <https://commits.webkit.org/r292574>

[Dan Glastonbury]

(In reply to Myles C. Maxfield from comment #4)
> [...] It
> can just be a free function and call .device() on the templated type. Doing
> it that way will get rid of this whole type hierarchy, and the problem
> you're describing here, as well as reducing total amount of code.

I like the sound of that!