Bug 212529 – [WebXR] Pass an unsigned long to cancelAnimationCallback() as handle

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Sergio Villar Senin]

[WebXR] Pass an unsigned long to cancelAnimationCallback() as handle

[Sergio Villar Senin]

Created attachment 400595 [details]
Patch

[Sergio Villar Senin]

Gentle ping

[youenn fablet]

Comment on attachment 400595 [details]
Patch

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

> Source/WebCore/Modules/webxr/WebXRSession.cpp:212
> +void WebXRSession::cancelAnimationFrame(XRFrameRequestCallback::Id callbackId)

I would call it XRFrameRequestCallback::Identifier

[Sergio Villar Senin]

Committed r262718: <https://trac.webkit.org/changeset/262718>

[Radar WebKit Bug Importer]

<rdar://problem/64120637>

[Darin Adler]

Comment on attachment 400595 [details]
Patch

IDL "unsigned long" is the same as C++ "unsigned" in WebKit, so all the "unsigned long" and "long" should be changed to "unsigned" and "int" in the C headers.

[Sergio Villar Senin]

(In reply to Darin Adler from comment #6)
> Comment on attachment 400595 [details]
> Patch
> 
> IDL "unsigned long" is the same as C++ "unsigned" in WebKit, so all the
> "unsigned long" and "long" should be changed to "unsigned" and "int" in the
> C headers.

OK, I've seen unsigned long is some other headers in Modules/ so I was not really sure.(In reply to Darin Adler from comment #6)
> Comment on attachment 400595 [details]
> Patch
> 
> IDL "unsigned long" is the same as C++ "unsigned" in WebKit, so all the
> "unsigned long" and "long" should be changed to "unsigned" and "int" in the
> C headers.

Thanks for this, I've seen "unsigned long" in some other modules so I thought it was also OK. I've just filled bug 213020 to address WebXR changes.

[Darin Adler]

(In reply to Sergio Villar Senin from comment #7)
> Thanks for this, I've seen "unsigned long" in some other modules so I
> thought it was also OK. I've just filled bug 213020 to address WebXR changes.

It’s worthwhile to fix those to consistently not use the "long" C types. Should be fairly simple task.

Using "long" for 32-bit integers isn't super-actively harmful but can lead to problems where two types are involved where the types should be the same, even possibly leading to template code bloat or need to have two functions where we'd like to have only one. And it’s basically a difference where there should be no difference. The fact that 32-bit is named "long" in IDL is, in effect, unrelated to the "long" in C.

[Yusuke Suzuki]

Re-opened since this is blocked by bug 213047

[Sergio Villar Senin]

Committed r262898: <https://trac.webkit.org/changeset/262898>