Gamepad: Add support for the Amazon Fire Game Controller

Gamepad: Add support for the Amazon Fire Game Controller

Manual verification of the mapping was done using:
http://www.html5rocks.com/en/tutorials/doodles/gamepad/gamepad-tester/tester.html

This also adds tests to check for mapping regressions for all gamepads.

BUG=453178, 373942
TEST=GamepadMappingsTest

Committed: https://crrev.com/983206f104e5495e5a126356ce8a0d965d8c6f1f
Cr-Commit-Position: refs/heads/master@{#313994}

[mstrum, 2015-01-29 01:48:23]

mstrum@amazon.com changed reviewers:
+ aurimas@chromium.org, cjhopman@chromium.org

[mstrum, 2015-01-29 02:30:07]

mstrum@amazon.com changed reviewers:
- cjhopman@chromium.org

[mstrum, 2015-01-29 03:01:25]

PTAL

[aurimas (slooooooooow), 2015-01-30 02:17:02]

aurimas@chromium.org changed reviewers:
+ jdduke@chromium.org

[aurimas (slooooooooow), 2015-01-30 02:17:02]

+jdduke can you take a look at this?

[jdduke (slow), 2015-01-30 17:01:31]

jdduke@chromium.org changed reviewers:
+ b.kelemen@samsung.com, skhatri@nvidia.com

[jdduke (slow), 2015-01-30 17:01:32]

Awesome, thanks (for the tests, too).

In general this looks good, adding some other devs who contributed some of the
initial code.

[kbalazs, 2015-01-30 19:00:42]

https://codereview.chromium.org/875813003/diff/40001/content/public/android/j...
File
content/public/android/java/src/org/chromium/content/browser/input/GamepadDevice.java
(right):

https://codereview.chromium.org/875813003/diff/40001/content/public/android/j...
content/public/android/java/src/org/chromium/content/browser/input/GamepadDevice.java:23:
// us to create cheap associative arrays. Originally this was dynamically set
based on how
I don't think we need to mention historical things in a comment. I would only
keep the first sentence.

https://codereview.chromium.org/875813003/diff/40001/content/public/android/j...
File
content/public/android/javatests/src/org/chromium/content/browser/input/GamepadMappingsTest.java
(right):

https://codereview.chromium.org/875813003/diff/40001/content/public/android/j...
content/public/android/javatests/src/org/chromium/content/browser/input/GamepadMappingsTest.java:20:
public class GamepadMappingsTest extends InstrumentationTestCase {
I'm not quite aware of the sort of testing options on Android, but isn't an
instrumentation test too heavy for this? I thought you want instrumentation test
when you need to simulate user input. This test does that in a way but it's a
rather programmatical so I wonder if we can make it into a unit test.

[jdduke (slow), 2015-01-30 19:07:00]

+jbudorick for potential updates on unit testing support.

https://codereview.chromium.org/875813003/diff/40001/content/public/android/j...
File
content/public/android/javatests/src/org/chromium/content/browser/input/GamepadMappingsTest.java
(right):

https://codereview.chromium.org/875813003/diff/40001/content/public/android/j...
content/public/android/javatests/src/org/chromium/content/browser/input/GamepadMappingsTest.java:20:
public class GamepadMappingsTest extends InstrumentationTestCase {
On 2015/01/30 19:00:42, kbalazs wrote:
> I'm not quite aware of the sort of testing options on Android, but isn't an
> instrumentation test too heavy for this? I thought you want instrumentation
test
> when you need to simulate user input. This test does that in a way but it's a
> rather programmatical so I wonder if we can make it into a unit test.

I could be wrong, but I believe the Android dependencies here
(android.view.KeyEvent, etc...) prevent us from making this a proper unit test.

[jbudorick, 2015-01-30 19:11:08]

jbudorick@chromium.org changed reviewers:
+ jbudorick@chromium.org

[jbudorick, 2015-01-30 19:11:09]

https://codereview.chromium.org/875813003/diff/40001/content/public/android/j...
File
content/public/android/javatests/src/org/chromium/content/browser/input/GamepadMappingsTest.java
(right):

https://codereview.chromium.org/875813003/diff/40001/content/public/android/j...
content/public/android/javatests/src/org/chromium/content/browser/input/GamepadMappingsTest.java:20:
public class GamepadMappingsTest extends InstrumentationTestCase {
On 2015/01/30 19:07:00, jdduke wrote:
> On 2015/01/30 19:00:42, kbalazs wrote:
> > I'm not quite aware of the sort of testing options on Android, but isn't an
> > instrumentation test too heavy for this? I thought you want instrumentation
> test
> > when you need to simulate user input. This test does that in a way but it's
a
> > rather programmatical so I wonder if we can make it into a unit test.
> 
> I could be wrong, but I believe the Android dependencies here
> (android.view.KeyEvent, etc...) prevent us from making this a proper unit
test.

This is correct, but this test is definitely a good candidate for Robolectric
conversion once we get that into chromium.

[kbalazs, 2015-01-30 19:14:38]

2015/01/30 19:11:09, jbudorick wrote:
>
https://codereview.chromium.org/875813003/diff/40001/content/public/android/j...
> File
>
content/public/android/javatests/src/org/chromium/content/browser/input/GamepadMappingsTest.java
> (right):
> 
>
https://codereview.chromium.org/875813003/diff/40001/content/public/android/j...
>
content/public/android/javatests/src/org/chromium/content/browser/input/GamepadMappingsTest.java:20:
> public class GamepadMappingsTest extends InstrumentationTestCase {
> On 2015/01/30 19:07:00, jdduke wrote:
> > On 2015/01/30 19:00:42, kbalazs wrote:
> > > I'm not quite aware of the sort of testing options on Android, but isn't
an
> > > instrumentation test too heavy for this? I thought you want
instrumentation
> > test
> > > when you need to simulate user input. This test does that in a way but
it's
> a
> > > rather programmatical so I wonder if we can make it into a unit test.
> > 
> > I could be wrong, but I believe the Android dependencies here
> > (android.view.KeyEvent, etc...) prevent us from making this a proper unit
> test.
> 
> This is correct, but this test is definitely a good candidate for Robolectric
> conversion once we get that into chromium.

Ok then, informal lgtm modulo the historical comment.

[mstrum, 2015-01-30 19:16:05]

On 2015/01/30 19:14:38, kbalazs wrote:
> 2015/01/30 19:11:09, jbudorick wrote:
> >
>
https://codereview.chromium.org/875813003/diff/40001/content/public/android/j...
> > File
> >
>
content/public/android/javatests/src/org/chromium/content/browser/input/GamepadMappingsTest.java
> > (right):
> > 
> >
>
https://codereview.chromium.org/875813003/diff/40001/content/public/android/j...
> >
>
content/public/android/javatests/src/org/chromium/content/browser/input/GamepadMappingsTest.java:20:
> > public class GamepadMappingsTest extends InstrumentationTestCase {
> > On 2015/01/30 19:07:00, jdduke wrote:
> > > On 2015/01/30 19:00:42, kbalazs wrote:
> > > > I'm not quite aware of the sort of testing options on Android, but isn't
> an
> > > > instrumentation test too heavy for this? I thought you want
> instrumentation
> > > test
> > > > when you need to simulate user input. This test does that in a way but
> it's
> > a
> > > > rather programmatical so I wonder if we can make it into a unit test.
> > > 
> > > I could be wrong, but I believe the Android dependencies here
> > > (android.view.KeyEvent, etc...) prevent us from making this a proper unit
> > test.
> > 
> > This is correct, but this test is definitely a good candidate for
Robolectric
> > conversion once we get that into chromium.
> 
> Ok then, informal lgtm modulo the historical comment.

Removed~

[jdduke (slow), 2015-01-30 19:20:53]

https://codereview.chromium.org/875813003/diff/60001/content/public/android/j...
File
content/public/android/javatests/src/org/chromium/content/browser/input/GamepadMappingsTest.java
(right):

https://codereview.chromium.org/875813003/diff/60001/content/public/android/j...
content/public/android/javatests/src/org/chromium/content/browser/input/GamepadMappingsTest.java:67:
GamepadMappings.mapToStandardGamepad(mMappedAxes, mMappedButtons, mRawAxes,
mRawButtons,
Hmm, I think this indentation is off, it should wrap with an 8-space indent on
the next line (also below).

https://codereview.chromium.org/875813003/diff/60001/content/public/android/j...
content/public/android/javatests/src/org/chromium/content/browser/input/GamepadMappingsTest.java:178:
Float.isNaN(mMappedAxes[i]));
For better or worse, style rules still apply for Java assertions (8-space indent
on newline).

I think you can now run "git cl format" and it should clean up the Java code.
Can you try that?

[mstrum, 2015-01-30 19:29:31]

On 2015/01/30 19:20:53, jdduke wrote:
>
https://codereview.chromium.org/875813003/diff/60001/content/public/android/j...
> File
>
content/public/android/javatests/src/org/chromium/content/browser/input/GamepadMappingsTest.java
> (right):
> 
>
https://codereview.chromium.org/875813003/diff/60001/content/public/android/j...
>
content/public/android/javatests/src/org/chromium/content/browser/input/GamepadMappingsTest.java:67:
> GamepadMappings.mapToStandardGamepad(mMappedAxes, mMappedButtons, mRawAxes,
> mRawButtons,
> Hmm, I think this indentation is off, it should wrap with an 8-space indent on
> the next line (also below).
> 
>
https://codereview.chromium.org/875813003/diff/60001/content/public/android/j...
>
content/public/android/javatests/src/org/chromium/content/browser/input/GamepadMappingsTest.java:178:
> Float.isNaN(mMappedAxes[i]));
> For better or worse, style rules still apply for Java assertions (8-space
indent
> on newline).
> 
> I think you can now run "git cl format" and it should clean up the Java code.
> Can you try that?

Done, the only manual changes I did after "git cl format" was un-inlining
@VisibleForTesting

[jdduke (slow), 2015-01-30 19:43:28]

lgtm, thanks!

[mstrum, 2015-01-30 20:16:21]

The CQ bit was checked by mstrum@amazon.com

[commit-bot: I haz the power, 2015-01-30 20:17:04]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/875813003/80001

[commit-bot: I haz the power, 2015-01-30 21:07:11]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-01-30 21:08:27]

Patchset 5 (id:??) landed as
https://crrev.com/983206f104e5495e5a126356ce8a0d965d8c6f1f
Cr-Commit-Position: refs/heads/master@{#313994}