Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch from WebIDL arrays to FrozenArray<>s in the IDLs. #62

Merged
merged 1 commit into from Jul 26, 2017

Conversation

rakuco
Copy link
Member

@rakuco rakuco commented Jul 17, 2017

Arrays have not existed in the WebIDL spec since 2015, so the Gamepad
spec (as well as the accompanying Gamepad Extensions one) were providing
invalid IDL definitions.

Switch to FrozenArray and clarify in prose that some attributes should be
cached by the user agent until new values have to be returned.

Fixes #28.

@rakuco
Copy link
Member Author

rakuco commented Jul 17, 2017

@bzbarsky @domenic @luser @toji et al: please take a look.

I left out any bits saying Gamepad#axes should be cached because they're just doubles, but I'm OK with adding that if you think it makes more sense.

I'd also appreciate it if the people implementing the spec could also chime in and say if it makes sense to cache these values (and whether they do change) at all -- I'm coming to this from a WebIDL and bindings perspective.

@bzbarsky
Copy link

I left out any bits saying Gamepad#axes should be cached because they're just doubles

The array entries are doubles, but the actual thing returned is an Array object and it being cached or not is quite observable, no? It needs similar language too.

@rakuco
Copy link
Member Author

rakuco commented Jul 17, 2017

The array entries are doubles, but the actual thing returned is an Array object and it being cached or not is quite observable, no?

Yes. My comment was more on the lines of whether it actually made sense to cache the Array at all or not (and avoid converting the same array to JS every time).

@bzbarsky
Copy link

I'm not sure what you mean by "whether it actually made sense to cache the Array". You want gamepad.axes === gamepad.axes to be true, right? So some caching needs to happen somewhere.

@rakuco
Copy link
Member Author

rakuco commented Jul 17, 2017

You want gamepad.axes === gamepad.axes to be true, right?

I wasn't sure if this was actually needed or not. If we want that to be true just like it should be for buttons and hapticActuators, then yes. I'll update the PR with a version that makes all 3 properties cached.

Arrays have not existed in the WebIDL spec since 2015, so the Gamepad
spec (as well as the accompanying Gamepad Extensions one) were providing
invalid IDL definitions.

Switch to FrozenArray<T> and clarify in prose that some attributes should be
cached by the user agent until new values have to be returned.

Fixes w3c#28.
@bzbarsky
Copy link

I wasn't sure if this was actually needed or not.

Please see https://w3ctag.github.io/design-principles/#attributes-like-data third bullet point.

@rakuco
Copy link
Member Author

rakuco commented Jul 17, 2017

Please see https://w3ctag.github.io/design-principles/#attributes-like-data third bullet point.

Thanks, that's very helpful :)

@luser luser merged commit 62febc1 into w3c:gh-pages Jul 26, 2017
@rakuco rakuco deleted the idl-arrays-in-spec branch July 31, 2017 15:38
MXEBot pushed a commit to mirror/chromium that referenced this pull request Aug 3, 2017
Sync our IDL file with w3c/gamepad#62 ("Switch from
WebIDL arrays to FrozenArray<>s in the IDLs").

WebIDL has not had array types since 2015, so finally make our IDL files
compliant with modern WebIDL following the spec fix.

It is important to note that this change modifies the existing behavior
slightly.
- |axes| and |buttons| are now frozen objects with all the related
  consequences for its properties and prototype.
- Those two attributes now return the same _object_ until their values
  change instead of always returning a new object on access.

Doing so aligns our code with both the spec as well as Gecko, which has done
the above ever since it implemented the Gamepad spec.

Bug: 740875
Change-Id: Ifb618c9d4f8860eb55efc882e701dae7390808a5
Reviewed-on: https://chromium-review.googlesource.com/595979
Reviewed-by: Matt Reynolds <mattreynolds@chromium.org>
Commit-Queue: Raphael Kubo da Costa (rakuco) <raphael.kubo.da.costa@intel.com>
Cr-Commit-Position: refs/heads/master@{#491440}
Ms2ger pushed a commit to web-platform-tests/wpt that referenced this pull request Sep 13, 2017
Follow-up to w3c/gamepad#62, which switched Gamepad#axes and Gamepad#buttons
to FrozenArray<>s.
rachelandrew pushed a commit to rachelandrew/web-platform-tests that referenced this pull request Nov 8, 2017
Follow-up to w3c/gamepad#62, which switched Gamepad#axes and Gamepad#buttons
to FrozenArray<>s.
jakearchibald pushed a commit to jakearchibald/web-platform-tests that referenced this pull request Nov 16, 2017
Follow-up to w3c/gamepad#62, which switched Gamepad#axes and Gamepad#buttons
to FrozenArray<>s.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants