[webkit-reviews] review granted: [Bug 230136] Implement 'gamepad' Permissions Policy : [Attachment 444478] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Marcos Caceres <marcos at marcosc.com>'s
request for review:
Bug 230136: Implement 'gamepad' Permissions Policy
https://bugs.webkit.org/show_bug.cgi?id=230136

Attachment 444478: Patch

https://bugs.webkit.org/attachment.cgi?id=444478&action=review




--- Comment #7 from Darin Adler <darin at apple.com> ---
Comment on attachment 444478
  --> https://bugs.webkit.org/attachment.cgi?id=444478
Patch

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

Feels like the test coverage is a bit light.

> Source/WebCore/Modules/gamepad/Navigator+Gamepad.idl:34
> +    [
> +	 EnabledBySetting=GamepadsEnabled,
> +	 CallWith=Document
> +    ] sequence<Gamepad> getGamepads();

I don’t think we have to break this out vertically just to fit one more
attribute in. Unless this is copying and pasting directly from a specification
or something.

    [EnabledBySetting=GamepadsEnabled, CallWith=Document] sequence<Gamepad>
getGamepads();

> Source/WebCore/html/FeaturePolicy.cpp:53
> +	   return "Gampad";

Typo: "Gamepad".

> Source/WebCore/html/FeaturePolicy.cpp:215
> +	   if (item.startsWith("gamepad")) {

Not new, but why all the startsWith in this function? Is it valid to write
"gamepadnotreallyimeantsomethingelse"?

> LayoutTests/http/tests/gamepad/gamepad-allow-attribute.https-expected.txt:4
> +CONSOLE MESSAGE: Feature policy 'Gampad' check failed for iframe with origin
'https://localhost:8443' and allow attribute 'gamepad 'none''.
> +CONSOLE MESSAGE: Feature policy 'Gampad' check failed for iframe with origin
'https://127.0.0.1:8443' and allow attribute 'gamepad 'none''.
> +CONSOLE MESSAGE: Feature policy 'Gampad' check failed for iframe with origin
'https://localhost:8443' and allow attribute 'gamepad 'self''.
> +CONSOLE MESSAGE: Feature policy 'Gampad' check failed for iframe with origin
'https://127.0.0.1:8443' and allow attribute 'gamepad https://localhost:8443'.

Here we have "Gampad".