Add new platform gamepad abstractions These include a PlatformGamepad object and platform strategies to support either in-process or out-of-process gamepad input. The code is basically unused here, but goes hand-in-hand with the patch that will be reviewed separately in https://bugs.webkit.org/show_bug.cgi?id=134324
Created attachment 233864 [details] Patch v1
Comment on attachment 233864 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=233864&action=review > Source/WebCore/WebCore.xcodeproj/project.pbxproj:7212 > - 07AC46FF1952102100EE9723 /* ISOVTTCue.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = ISOVTTCue.cpp; path = ISOVTTCue.cpp; sourceTree = "<group>"; }; > - 07AC47001952102100EE9723 /* ISOVTTCue.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = ISOVTTCue.h; path = ISOVTTCue.h; sourceTree = "<group>"; }; > + 07AC46FF1952102100EE9723 /* ISOVTTCue.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = ISOVTTCue.cpp; sourceTree = "<group>"; }; > + 07AC47001952102100EE9723 /* ISOVTTCue.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ISOVTTCue.h; sourceTree = "<group>"; }; Is this intentional? > Source/WebCore/platform/GamepadStrategy.h:50 > +#endif // ENABLE(GAMEPAD) Nit: I think we agreed to not bother with these style comments > Source/WebKit2/WebProcess/WebCoreSupport/WebPlatformStrategies.cpp:594 > +#endif // PLATFORM(IOS) > > -#endif > +#endif // PLATFORM(COCOA) But here you're adding them, so maybe my memory is incorrect and they should be around :)
Created attachment 233866 [details] Patch v2 - Patch that should apply
(In reply to comment #2) > (From update of attachment 233864 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=233864&action=review > > > Source/WebCore/WebCore.xcodeproj/project.pbxproj:7212 > > - 07AC46FF1952102100EE9723 /* ISOVTTCue.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = ISOVTTCue.cpp; path = ISOVTTCue.cpp; sourceTree = "<group>"; }; > > - 07AC47001952102100EE9723 /* ISOVTTCue.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = ISOVTTCue.h; path = ISOVTTCue.h; sourceTree = "<group>"; }; > > + 07AC46FF1952102100EE9723 /* ISOVTTCue.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = ISOVTTCue.cpp; sourceTree = "<group>"; }; > > + 07AC47001952102100EE9723 /* ISOVTTCue.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ISOVTTCue.h; sourceTree = "<group>"; }; > > Is this intentional? Nope, and it's gone in the updated patch. > > > Source/WebCore/platform/GamepadStrategy.h:50 > > +#endif // ENABLE(GAMEPAD) > > Nit: I think we agreed to not bother with these style comments I think we agreed to not bother *always* especially when they're short blocks where it would be pointless. In this case... > > > Source/WebKit2/WebProcess/WebCoreSupport/WebPlatformStrategies.cpp:594 > > +#endif // PLATFORM(IOS) > > > > -#endif > > +#endif // PLATFORM(COCOA) > > But here you're adding them, so maybe my memory is incorrect and they should be around :) I added them because I couldn't follow what these blocks were! Thanks for the review!
http://trac.webkit.org/changeset/170459
We are trying not to add any more PlatformStrategies, and instead are aiming to remove them. Please back this out and let's discuss other mechanisms you can use to achieve what it is you are trying to do.
(In reply to comment #6) > We are trying not to add any more PlatformStrategies, and instead are aiming to remove them. Please back this out and let's discuss other mechanisms you can use to achieve what it is you are trying to do. I have already updated Gamepad API and did not use PlatformStrategies, please take a look at my patch and tell me what do you think about this change: https://bugs.webkit.org/show_bug.cgi?id=133850
(In reply to comment #6) > We are trying not to add any more PlatformStrategies, and instead are aiming to remove them. Please back this out and let's discuss other mechanisms you can use to achieve what it is you are trying to do. I have a good non-platformStrategies based solution for this I'll get in today.
(In reply to comment #7) > (In reply to comment #6) > > We are trying not to add any more PlatformStrategies, and instead are aiming to remove them. Please back this out and let's discuss other mechanisms you can use to achieve what it is you are trying to do. > > I have already updated Gamepad API and did not use PlatformStrategies, please take a look at my patch and tell me what do you think about this change: > https://bugs.webkit.org/show_bug.cgi?id=133850 Wow, coincidental! Unfortunately there's a lot of needs the work you've done there don't fulfill. For example, your patch doesn't allow for WebKit2 implementations (which is the very reason I used platform strategies in the first place) I'll comment more in your bug(s) later today. But I bet we can use things like your layout test work going forward!
Filed a new bug to remove the use of PlatformStrategies (https://bugs.webkit.org/show_bug.cgi?id=134348), re-resolving this one.