Summary: | [WebXR] Pass an unsigned long to cancelAnimationCallback() as handle | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Sergio Villar Senin <svillar> | ||||
Component: | New Bugs | Assignee: | Sergio Villar Senin <svillar> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | cdumez, darin, dbates, esprehn+autocc, ews-watchlist, kondapallykalyan, ross.kirsling, svillar, webkit-bug-importer, youennf, ysuzuki, zan | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Bug Depends on: | 213047 | ||||||
Bug Blocks: | 208988 | ||||||
Attachments: |
|
Description
Sergio Villar Senin
2020-05-29 09:51:07 PDT
Created attachment 400595 [details]
Patch
Gentle ping 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 Committed r262718: <https://trac.webkit.org/changeset/262718> 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.
(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. (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. Re-opened since this is blocked by bug 213047 Committed r262898: <https://trac.webkit.org/changeset/262898> |