Bug 213020 - [WebXR] unsigned long in IDL should be translated as unsigned in C++ code
Summary: [WebXR] unsigned long in IDL should be translated as unsigned in C++ code
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sergio Villar Senin
URL:
Keywords: InRadar
Depends on:
Blocks: 208988
  Show dependency treegraph
 
Reported: 2020-06-10 04:24 PDT by Sergio Villar Senin
Modified: 2020-06-19 00:33 PDT (History)
6 users (show)

See Also:


Attachments
Patch (5.24 KB, patch)
2020-06-10 04:28 PDT, Sergio Villar Senin
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sergio Villar Senin 2020-06-10 04:24:34 PDT
[WebXR] unsigned long in IDL should be translated as unsigned in C++ code
Comment 1 Sergio Villar Senin 2020-06-10 04:28:24 PDT
Created attachment 401533 [details]
Patch
Comment 2 Darin Adler 2020-06-10 10:02:34 PDT
Comment on attachment 401533 [details]
Patch

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

> Source/WebCore/Modules/webxr/WebXRSession.cpp:203
> +    unsigned newId = ++m_nextCallbackId;

This makes it clear that m_nextCallbackId is misnamed. Because we can see here that in practice it contains the last already-used callback ID, not the next callback ID to be used.

> Source/WebCore/Modules/webxr/WebXRSession.h:77
> +    void cancelAnimationFrame(unsigned handle);

A little inelegant that the header calls this "handle", but the .cpp file calls it "callbackId".
Comment 3 Sergio Villar Senin 2020-06-10 13:03:40 PDT
Comment on attachment 401533 [details]
Patch

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

Thanks for the quick review!

>> Source/WebCore/Modules/webxr/WebXRSession.cpp:203
>> +    unsigned newId = ++m_nextCallbackId;
> 
> This makes it clear that m_nextCallbackId is misnamed. Because we can see here that in practice it contains the last already-used callback ID, not the next callback ID to be used.

Right. I'll do the following, initialize m_nextCallbackId to 1 and the replace the ++m_nextCallbackId by m_nextCallbackId++

>> Source/WebCore/Modules/webxr/WebXRSession.h:77
>> +    void cancelAnimationFrame(unsigned handle);
> 
> A little inelegant that the header calls this "handle", but the .cpp file calls it "callbackId".

ACK, I'll use a more consistent naming.
Comment 4 Sergio Villar Senin 2020-06-19 00:32:21 PDT
Committed r263256: <https://trac.webkit.org/changeset/263256>
Comment 5 Radar WebKit Bug Importer 2020-06-19 00:33:19 PDT
<rdar://problem/64522477>