Added support for GVR controllers

Added support for GVR controllers

Currently uses a different instance of GvrApi than the VrShell, but it's not
clear if that's something that's going to cause a problem. Works well enough for
the time being.

BUG=389343
Committed: https://crrev.com/3f546189c1916581b887525d387892afe397ec52
Cr-Commit-Position: refs/heads/master@{#420240}

[bajones, 2016-09-18 22:30:58]

bajones@chromium.org changed reviewers:
+ bshe@chromium.org, klausw@chromium.org, mthiesse@chromium.org

[bajones, 2016-09-18 22:30:58]

Please take a look at the Daydream controller/Gamepad integration. Currently
only uses the non-presenting GvrApi described in
https://codereview.chromium.org/2343023002/, and does not attempt to switch over
to the VrShell's instance of GvrApi when it becomes available. This doesn't
appear to be causing problems, but we may find some weird disconnects in
tracking behavior that will make us want to change that down the line.

For now, though, this exposes the controller orientation, touchpad touch
position, touched state, and pressed state. App button is not exposed with the
expectation that VrShell will want to reserve it. 

Dependent on:
https://codereview.chromium.org/2300543003/
https://codereview.chromium.org/2300883003/
https://codereview.chromium.org/2343023002/

[mthiesse, 2016-09-18 23:21:48]

LGTM with nit.

https://codereview.chromium.org/2351573002/diff/20001/device/vr/android/gvr/g...
File device/vr/android/gvr/gvr_gamepad_data_fetcher.cc (right):

https://codereview.chromium.org/2351573002/diff/20001/device/vr/android/gvr/g...
device/vr/android/gvr/gvr_gamepad_data_fetcher.cc:71: WebGamepad& pad =
state->data;
Just to check my understanding, we're only exposing the touch pad and
orientation right? The other buttons are controlled by the system, or by
VrShell?

https://codereview.chromium.org/2351573002/diff/20001/device/vr/android/gvr/g...
device/vr/android/gvr/gvr_gamepad_data_fetcher.cc:76: CopyToWebUString(pad.id,
WebGamepad::idLengthCap,
Should this be WebGamepad::idLengthCap + 1 to include the null termination?

Upon searching I think your use of the value matches other uses of this value...


In your CL https://codereview.chromium.org/2300543003 where you update
WebGamepad.h, could you add a comment or something explaining that the
idLengthCap and mappingLengthCap include the null termination (hence the actual
length cap for the ID is one character less (the null termination is an artifact
of how the data is stored))? It's especially confusing because the other length
caps are for non-null-terminated data.

https://codereview.chromium.org/2351573002/diff/20001/device/vr/android/gvr/g...
device/vr/android/gvr/gvr_gamepad_data_fetcher.cc:100: // TODO: expose
orientation/acceleration/velocity
Should orientation still be in this TODO? Looks like it's being exposed.

[bshe, 2016-09-20 15:35:04]

lgtm with nits

https://codereview.chromium.org/2351573002/diff/60001/device/vr/android/gvr/g...
File device/vr/android/gvr/gvr_device_provider.cc (right):

https://codereview.chromium.org/2351573002/diff/60001/device/vr/android/gvr/g...
device/vr/android/gvr/gvr_device_provider.cc:73: std::unique_ptr<gvr::GvrApi>
controller_api_;
doesn't look like controller_api_ is used?

https://codereview.chromium.org/2351573002/diff/60001/device/vr/android/gvr/g...
File device/vr/android/gvr/gvr_gamepad_data_fetcher.cc (right):

https://codereview.chromium.org/2351573002/diff/60001/device/vr/android/gvr/g...
device/vr/android/gvr/gvr_gamepad_data_fetcher.cc:20: memcpy(dest, src.data(),
str_to_copy * sizeof(base::string16::value_type));
If dest_length - 1 < src.size and sizeof(base::string16::value_type) > 1, could
this cause a problem?

[mthiesse, 2016-09-20 16:02:06]

https://codereview.chromium.org/2351573002/diff/60001/device/vr/android/gvr/g...
File device/vr/android/gvr/gvr_gamepad_data_fetcher.cc (right):

https://codereview.chromium.org/2351573002/diff/60001/device/vr/android/gvr/g...
device/vr/android/gvr/gvr_gamepad_data_fetcher.cc:76: CopyToWebUString(pad.id,
WebGamepad::idLengthCap - 1,
I don't think this is the right fix, I think this was doing the correct thing
before, but the variable idLengthCap itself is ambiguous. (Now you're only
writing n-2 characters + null, instead of n-1 characters + null)

[bajones, 2016-09-21 22:28:11]

The CQ bit was checked by bajones@chromium.org

[bajones, 2016-09-21 22:28:12]

The patchset sent to the CQ was uploaded after l-g-t-m from
mthiesse@chromium.org, bshe@chromium.org
Link to the patchset: https://codereview.chromium.org/2351573002/#ps100001
(title: "Rebase")

[commit-bot: I haz the power, 2016-09-21 22:29:07]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-21 22:34:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-21 22:34:28]

Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[bajones, 2016-09-21 22:41:18]

bajones@chromium.org changed reviewers:
+ aelias@chromium.org

[bajones, 2016-09-21 22:41:18]

Oh, missed that I needed an owners review on the new DEPS on WebGamepads.h.
aelias@ can you OK that dependency?

[aelias_OOO_until_Jul13, 2016-09-22 00:23:26]

Seems OK enough but I don't have OWNERS on device/vr?

[aelias_OOO_until_Jul13, 2016-09-22 00:24:04]

I guess there are special target-OWNERS rules for deps, lgtm.

[bajones, 2016-09-22 00:24:22]

The CQ bit was checked by bajones@chromium.org

[commit-bot: I haz the power, 2016-09-22 00:24:54]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-22 00:29:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-22 00:29:30]

Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[bajones, 2016-09-22 01:30:44]

The CQ bit was checked by bajones@chromium.org

[bajones, 2016-09-22 01:30:45]

The patchset sent to the CQ was uploaded after l-g-t-m from aelias@chromium.org,
mthiesse@chromium.org, bshe@chromium.org
Link to the patchset: https://codereview.chromium.org/2351573002/#ps120001
(title: "Added appropriate Webkit dep to device/vr/BUILD.gn")

[commit-bot: I haz the power, 2016-09-22 01:31:07]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-22 02:22:09]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-09-22 02:23:39]

Description was changed from

==========
Added support for GVR controllers

Currently uses a different instance of GvrApi than the VrShell, but it's not
clear if that's something that's going to cause a problem. Works well enough for
the time being.

BUG=389343
==========

to

==========
Added support for GVR controllers

Currently uses a different instance of GvrApi than the VrShell, but it's not
clear if that's something that's going to cause a problem. Works well enough for
the time being.

BUG=389343

Committed: https://crrev.com/3f546189c1916581b887525d387892afe397ec52
Cr-Commit-Position: refs/heads/master@{#420240}
==========

[commit-bot: I haz the power, 2016-09-22 02:23:40]

Patchset 7 (id:??) landed as
https://crrev.com/3f546189c1916581b887525d387892afe397ec52
Cr-Commit-Position: refs/heads/master@{#420240}