Bug 69451

Created attachment 109841 [details] Patch

Please wait for approval from fishd@chromium.org before submitting because this patch contains changes to the Chromium public API.

Comment on attachment 109841 [details] Patch Attachment 109841 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9964033 New failing tests: fast/dom/navigator-detached-no-crash.html

Comment on attachment 109841 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=109841&action=review > Source/WebKit/chromium/public/WebViewClient.h:303 > + // Gamepad ------------------------------------------------------------- nit: new line after section divider. please preserve the 2 new lines between sections. it seems like the concept of a gamepad really belongs more at the platform layer of webkit. this seems like something you would get via the WebKitPlatformSupport interface instead of the WebViewClient. i'd imagine that you probably need something to be contextual to the WebView however. can you explain that part? maybe we could meet up to review this in a little more detail?

There hasn't been any agreement or support for this feature on webkit-dev mailing list. To the contrary, there has been much disconcert. One of the ways to move forward with prototyping that we've been discussing was working on a branch. This seems like a great candidate for a branch, as merging to trunk shouldn't be hard.

(In reply to comment #5) > There hasn't been any agreement or support for this feature on webkit-dev mailing list. To the contrary, there has been much disconcert. > > One of the ways to move forward with prototyping that we've been discussing was working on a branch. This seems like a great candidate for a branch, as merging to trunk shouldn't be hard. I don't think this needs to be developed on a branch. I think this is low cost to develop on the trunk behind a flag. It is largely orthogonal to existing code, so it should have low impact. This is being actively developed by Mozilla and Google, and we'd like to proceed to experiment with it in our developer channel (disabled by default behind a command line flag of course). Can you please share your feedback on the API with the w3c-events working group?

Alexey, perhaps you missed this one? https://bugs.webkit.org/show_bug.cgi?id=66859#c5 Notice that there is a spec under development.

I don't have any feedback that I'm willing to share with the working group at this time. I see that Simon Fraser objected against adding this to WebKit, as well as against the idea itself. How was that resolved? Quoting <http://www.webkit.org/projects/goals.html>, "WebKit is an engineering project not a science project. For new features to be adopted into WebKit, we strongly prefer for the technology or at least the use case for it to be proven." "WebKit is not the solution to every problem. We focus on web content, not complete solutions to every imaginable technology need." > Notice that there is a spec under development. Having a spec under development is certainly an important step. For what it's worth, the spec doesn't exist where bug 66859 comment 5 points to, even though I can find it in version control history. Also, <http://www.w3.org/2010/webevents/charter/Overview.html> doesn't list gamepad/joystick interfaces in scope or deliverables. I can see informal discussion with some WG members being in support for recharter, but it hasn't happened yet.

(In reply to comment #8) > I don't have any feedback that I'm willing to share with the working group at this time. How should we interpret your silence? > I see that Simon Fraser objected against adding this to WebKit, as well as against the idea itself. How was that resolved? It was resolved by creating a W3C spec and working to recharter the W3C-events WG. > Quoting <http://www.webkit.org/projects/goals.html>, > > "WebKit is an engineering project not a science project. For new features to be adopted into WebKit, we strongly prefer for the technology or at least the use case for it to be proven." > > "WebKit is not the solution to every problem. We focus on web content, not complete solutions to every imaginable technology need." I don't understand what point you are making. Mozilla and Google are working to add support for game controllers to the web platform. It is an engineering project based on a strong use case. I get over 5 million results when I google for "flash gamepad" and nearly 17 million for "flash gamecontroller." It seems pretty obvious that HTML5/WebGL-based games would benefit immensely from a gamepad interface. I honestly believe that there are cases where developing a new API on the trunk could be expensive in terms of its overhead to ports that do not care about the API. This does not seem to be anywhere close to that. We're talking about a light touch on the DOM here. > > Notice that there is a spec under development. > > Having a spec under development is certainly an important step. Yes, lack of a spec seemed to be Simon's main objection initially. He then proceeded to wonder about other possible manifestations of such an API. I think we would very much welcome his input on the API design. Now is a great time to get involved! > For what it's worth, the spec doesn't exist where bug 66859 comment 5 points to, even though I can find it in version control history. Yes, I noticed that too. Given that it was recently updated, I suspect a snafu that will be easily fixed. > Also, <http://www.w3.org/2010/webevents/charter/Overview.html> doesn't list gamepad/joystick interfaces in scope or deliverables. I can see informal discussion with some WG members being in support for recharter, but it hasn't happened yet. Yes, the WG is being rechartered to include this work.

(In reply to comment #8) > For what it's worth, the spec doesn't exist where bug 66859 comment 5 points to, even though I can find it in version control history. Apologies, I've updated to the correct the URL above. > Also, <http://www.w3.org/2010/webevents/charter/Overview.html> doesn't list gamepad/joystick interfaces in scope or deliverables. I can see informal discussion with some WG members being in support for recharter, but it hasn't happened yet. The Draft from the WG Chair is here if you're interested in commenting on the rechartering process: http://www.w3.org/2010/webevents/charter/2011/Overview.html

Darin, these would all be good comments for a webkit-dev discussion. I suggest resuming the thread there. When suggesting people to actively join development of every spec they have concerns about, please remember that this is a significant time investment. One who cares about immature and/or harmful specs not being in WebKit doesn't necessarily have any interest in making each of them good. Ditto for convincing every person on Internet that they are harmful. The concern here as I understand it is that providing low level access to every possible controller creates fragmentation, with purportedly "HTML" content that only works on a few devices. There is no clear cut border here - it's been mentioned that even touch events can be seen as rare - and then I advocate that adding more mouse specific events is a bad idea for the same reason. Finally, I agree that the cost of having this in trunk under an ifdef is not very high. This also means that developing on branch is relatively low cost. Since this is still at prototyping stage, I thought that it could be a small feature to polish prototype-on-branch approach on.

(In reply to comment #11) > Darin, these would all be good comments for a webkit-dev discussion. I suggest resuming the thread there. Yes, we should certainly do that. > When suggesting people to actively join development of every spec they have concerns about, please remember that this is a significant time investment. My point is that we are working with others on this feature, and it would be helpful for them to hear your concerns too. We will struggle to properly articulate/channel your concerns because we do not fully understand them. You can surely communicate them better than anyone else. I agree that building new APIs takes time. We are not looking to ship something rushed or poor in quality. We are looking to prototype, learn and improve the design. > Finally, I agree that the cost of having this in trunk under an ifdef is not very high. This also means that developing on branch is relatively low cost. Since this is still at prototyping stage, I thought that it could be a small feature to polish prototype-on-branch approach on. Doing the development on trunk makes it vastly easier for us to get feedback from developers. They can switch on a flag, try out the feature, and give us feedback. A branch just adds cost. Since there is little burden on the trunk, it seems obvious to me that we should do the work on the trunk.

(In reply to comment #4) > it seems like the concept of a gamepad really belongs more at the platform layer of > webkit. this seems like something you would get via the WebKitPlatformSupport interface > instead of the WebViewClient. i'd imagine that you probably need something to be contextual > to the WebView however. can you explain that part? maybe we could meet up to review this > in a little more detail? Here's the implementation approach I'm thinking of taking now (hopefully more sensible?): WebCore/page/Navigator.idl adds ".gamepads" and bindings. WebCore/page adds Gamepad.idl and GamepadList.idl along with bindings to expose to Javascript. The accessor in Navigator calls into WebCore/platform/chromium/PlatformSupport.h to "pollGamepads()". That's implemented by WebKit/chromium/src/PlatformSupport.cpp which forwards to something in WebKit/chromium/public/WebKitPlatformSupport.h (something like webPollGamepads() but with a better name). It converts from WebGamepads (a POD shared memory representation) to the binding- & WebCore-friendly representation in GamepadList.idl/Gamepad.idl. chromium/public/ adds WebGamepads.h, a fixed size buffer representation of gamepad data. On the Chromium side, the browser uses a background thread to poll the hardware into a WebGamepads structure, and implements "webPollGamepads()" which just returns a handle/pointer to that shared memory.

(In reply to comment #13) ... > Here's the implementation approach I'm thinking of taking now (hopefully more sensible?): > > WebCore/page/Navigator.idl adds ".gamepads" and bindings. WebCore/page adds Gamepad.idl and GamepadList.idl along with bindings to expose to Javascript. > > The accessor in Navigator calls into WebCore/platform/chromium/PlatformSupport.h to "pollGamepads()". We normally define a cross-platform header in WebCore/platform/ that defines the API cross-platform code should use to talk to the platform. Then, we provide alternate implementations of that header based on the needs of specific ports. For Chromium, the implementation would use PlatformSupport as you describe. Other ports might be implemented by using OS APIs directly.

(In reply to comment #14) > > We normally define a cross-platform header in WebCore/platform/ that defines the API > cross-platform code should use to talk to the platform. Then, we provide alternate > implementations of that header based on the needs of specific ports. I see, thank you.

Created attachment 110692 [details] plumb through platform rather than client

Comment on attachment 110692 [details] plumb through platform rather than client Attachment 110692 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/10026917

Created attachment 110704 [details] remove unnecessary include

As discussed in webkit-dev, I don't think that we should accept an implementation of a spec that's so immature that it doesn't even have a meaningful name.

(In reply to comment #19) > As discussed in webkit-dev, I don't think that we should accept an implementation of a spec that's so immature that it doesn't even have a meaningful name. And naming concerns should still be discussed in the WG. The spec focuses on gaming use-cases, and as such, the most common implementation of the spec will almost surely be gamepads, hence the name. It was generalized to support other devices as there was no reason not be flexible, but calling it something vague like "inputdevices" or "controllers" isn't helpful.

Comment on attachment 110704 [details] remove unnecessary include To be clear, my comment is meant to block the patch, marking r- to that effect. Someone who cares about this feature should work with the WG to make the spec reach some minimal maturity level, like at least having a name that's not actively misleading.

> The spec focuses on gaming use-cases, and as such, the most common implementation of the spec will almost surely be gamepads, hence the name. It was generalized to support other devices as there was no reason not be flexible, but calling it something vague like "inputdevices" or "controllers" isn't helpful. My recollection is that in the webkit-dev discussion, there was consensus that this is a poor name. If you disagree and think that the spec is mature enough to be in WebKit, please post to webkit-dev. That's where we decide what goes in WebKit.

(In reply to comment #21) > (From update of attachment 110704 [details]) > To be clear, my comment is meant to block the patch, marking r- to that effect. > > Someone who cares about this feature should work with the WG to make the spec reach some minimal maturity level, like at least having a name that's not actively misleading. Alexey, I don't think it is appropriate for you to block this patch on the basis you have stated. You seem to be the one, who doesn't like the name, but you are reluctant to complain about it on the WG. The folks developing this feature are obviously happy with the name, so it doesn't seem to make sense for them to try to channel you in their discussions on the WG. From the scope section of the spec: "Specifically, we choose to only support two categories of input from devices: buttons and axes." Now, I can actually see why "gamepad" might be an OK name for this feature. If you think about a gamepad, you immediately imagine those two categories of information. It just happens to be true that those same two categories of information can be used to express data from other types of input devices. I don't think that makes the name actively misleading. Maybe the name could be better. I always like to ponder better names for things. Besides, this is all being done behind an ENABLE flag. No one is trying to make a long-term commitment to this API as it exists today. We've been very clear that we want to get this out and into the hands of developers so they can give us feedback. The best way for us to do that is via nightly builds. The name of the API can and may change. By the way, would you mind pointing me to the spec for requestFullScreen?

Created attachment 115477 [details] remove custom binding code, add DRT hooks and tests

Created attachment 115675 [details] update Skipped for various ports

Created attachment 116114 [details] remove everything except platform plumbing and DRT/testing

Comment on attachment 116114 [details] remove everything except platform plumbing and DRT/testing Attachment 116114 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10539023

Comment on attachment 116114 [details] remove everything except platform plumbing and DRT/testing It looks like you are going to need to update WebKit/chromium/DEPS to pull in a version of webkit_support.h that provides SetGamepadData.

Created attachment 116120 [details] update WebKit/chromium/DEPS

Created attachment 116123 [details] try a different chromium rev

Created attachment 116126 [details] add apparently newly-needed chromium deps

Comment on attachment 116126 [details] add apparently newly-needed chromium deps View in context: https://bugs.webkit.org/attachment.cgi?id=116126&action=review > Source/WebCore/platform/Gamepads.h:35 > +void sampleGamepads(GamepadList* into); Can we put this in Source/WebCore/platform/gamepad/Gamepads.h ? I presume there will be more gamepad-related files in platform. > Source/WebKit/chromium/DEPS:35 > - 'chromium_rev': '109696' > + 'chromium_rev': '110994' Please roll Chromium DEPS in a separate patch. > Source/WebKit/chromium/src/Gamepads.cpp:38 > +namespace WebCore { Code in the WebCore namespace should be in WebCore. > Source/WebKit/chromium/src/Gamepads.cpp:48 > + for (unsigned i = 0; i < WebKit::WebGamepads::itemsLengthCap; ++i) { Code in the WebCore namespace should not refer to any code in the WebKit namespace.

(In reply to comment #32) > (From update of attachment 116126 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=116126&action=review > > > Source/WebCore/platform/Gamepads.h:35 > > +void sampleGamepads(GamepadList* into); > > Can we put this in Source/WebCore/platform/gamepad/Gamepads.h ? I presume there will be more gamepad-related files in platform. I don't think there will be, perhaps a total of 2 files. Would you still prefer it moved? > > > Source/WebKit/chromium/DEPS:35 > > - 'chromium_rev': '109696' > > + 'chromium_rev': '110994' > > Please roll Chromium DEPS in a separate patch. OK, https://bugs.webkit.org/show_bug.cgi?id=72911 > > > Source/WebKit/chromium/src/Gamepads.cpp:38 > > +namespace WebCore { > > Code in the WebCore namespace should be in WebCore. > > > Source/WebKit/chromium/src/Gamepads.cpp:48 > > + for (unsigned i = 0; i < WebKit::WebGamepads::itemsLengthCap; ++i) { > > Code in the WebCore namespace should not refer to any code in the WebKit namespace. Eep, those are due to ignorance about layering (and maybe picking a poor copy/paste source). The goal was to have a prototype in platform for WebCore to available call, and then to have that functionality be implemented by the port. Is that a bad idea?

> I don't think there will be, perhaps a total of 2 files. Would you still prefer it moved? Nope. That sounds fine. > > > Source/WebKit/chromium/src/Gamepads.cpp:48 > > > + for (unsigned i = 0; i < WebKit::WebGamepads::itemsLengthCap; ++i) { > > > > Code in the WebCore namespace should not refer to any code in the WebKit namespace. > > Eep, those are due to ignorance about layering (and maybe picking a poor copy/paste source). The goal was to have a prototype in platform for WebCore to available call, and then to have that functionality be implemented by the port. Is that a bad idea? The usual approach is for WebCore code in WebCore/platform to call WebKit/chromium code via PlatformSupport. The PlatformSupport calls then get bounced to the embedder via WebKitPlatformSupport.

(In reply to comment #34) > > Eep, those are due to ignorance about layering (and maybe picking a poor copy/paste source). The goal was to have a prototype in platform for WebCore to available call, and then to have that functionality be implemented by the port. Is that a bad idea? > > The usual approach is for WebCore code in WebCore/platform to call WebKit/chromium code via PlatformSupport. The PlatformSupport calls then get bounced to the embedder via WebKitPlatformSupport. Good catch, Adam!

(In reply to comment #34) > The usual approach is for WebCore code in WebCore/platform to call WebKit/chromium code via PlatformSupport. The PlatformSupport calls then get bounced to the embedder via WebKitPlatformSupport. I see, so the usage of WebKitPlatformSupport and translation in Gamepads.cpp should really be in chromium/PlatformSupport.cpp instead, and in fact, Gamepads.h/cpp aren't needed at all. (It's fine for page/Navigator to call in WebCore/platform/PlatformSupport directly, right?)

Usually we have a header and a CPP file in WebCore/platform to that calls into PlatformSupport, but that can be just a call-through if that's all you need. If you're doing that, you probably want: WebCore/platform/Gamepads.h WebCore/platform/chromium/GamespadsChromium.cpp because calling PlatformSupport is basically Chromium-specific. I know this looks like a lot of layers without purpose, but the idea is that other ports might implement this functionality in terms of other APIs, such as Mac OS X native APIs. In that case, they'd have GamepadsMac.mm in WebCore/platform/mac.

Created attachment 116151 [details] fix layering violations

Improved? Sorry if I misunderstood still.

Comment on attachment 116151 [details] fix layering violations The layering stuff looks great. Thanks! (I haven't reviewed the details of the gamepad-specific code or the changes to Tools.)

Comment on attachment 116151 [details] fix layering violations Attachment 116151 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10554019

(still (In reply to comment #41) > (From update of attachment 116151 [details]) > Attachment 116151 [details] did not pass chromium-ews (chromium-xvfb): > Output: http://queues.webkit.org/results/10554019 (still trying to figure out how to roll chromium/DEPS, things seem kinda broken)

Created attachment 116284 [details] no change, just to EWS again with DEPS rolled

Comment on attachment 116284 [details] no change, just to EWS again with DEPS rolled Clearing flags on attachment: 116284 Committed r101065: <http://trac.webkit.org/changeset/101065>

All reviewed patches have been landed. Closing bug.

Hi, I know this bug is closed but just want to try posting my question in case anyone is still following: I would like to know the status of Gamepad API spec and also its implementation status as I see many differences in published version of spec, it's latest editor's draft and its implementation in various browsers. Some examples, - Published draft explains navigator.gamepads interface but its replaced by getGamepads() method in editor's draft. published: http://www.w3.org/TR/gamepad/ latest ED: https://dvcs.w3.org/hg/gamepad/raw-file/default/gamepad.html -Mozilla wiki talks about GamepadButtonEvent, GamepadAxisEvent, GamepadButtonDown, GamepadButtonDown, GamepadAxisMove. But its not yet there in spec. https://wiki.mozilla.org/GamepadAPI -As compared to published draft, editor's draft has some extra attributes in Gamepad interface (attributes: mapping, connected) Please let me know if the support is still experimental and spec is still being discussed.

The specification is still a work in progress. The WebKit support for any of the specification versions has fell behind. Just for starters, the gamepadconnected and gamepaddisconnected events are not supported in the current implementation. I believe Blink and Mozilla are most interested in the specification, so their implementations are much more likely to be up to date with the current version of the specification. The two vendors are also most (if not the only) active in standardizing the Gamepad API.