Inject vr controller events into Android Native UI.

Inject vr controller events into Android Native UI.

Generalizes the MotionEventSynthesizer to support input into arbitrary Java views, and connects VR controller/gesture output to the MotionEventSynthesizer.

This also fixes some rendering bugs where onDraw was not being called after scrolling preventing the texture from updating correctly.

BUG=661609
Review-Url: https://codereview.chromium.org/2694103006
Cr-Commit-Position: refs/heads/master@{#453004}
Committed: https://chromium.googlesource.com/chromium/src/+/8ba7911391a74da0f186b82801d3d6279a197589

[mthiesse, 2017-02-15 22:26:31]

mthiesse@chromium.org changed reviewers:
+ asimjour@chromium.org, bshe@chromium.org, tedchoc@chromium.org

[mthiesse, 2017-02-15 22:26:32]

tedchoc@ please review content/public/android changes.

bshe@ please review vr_shell/ changes.

[mthiesse, 2017-02-15 23:41:30]

mthiesse@chromium.org changed reviewers:
+ aelias@chromium.org
- tedchoc@chromium.org

[mthiesse, 2017-02-15 23:41:31]

-tedchoc who's OOO, +aelias. Please review content/public/android changes.

[aelias_OOO_until_Jul13, 2017-02-16 03:29:15]

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

[aelias_OOO_until_Jul13, 2017-02-16 03:29:17]

Hmm, we're in the middle of redesigning input dispatching on Android, and this
might sit strangely in the new design.  +jinsukkim who wrote
https://docs.google.com/document/d/1Fpvv6aaNwVDTsvHKhgEXZjyEoEgQoKvtBxM-SiDnQ...
(not final design yet).  Thoughts?

[Jinsuk Kim, 2017-02-16 04:14:48]

On 2017/02/16 03:29:17, aelias wrote:
> Hmm, we're in the middle of redesigning input dispatching on Android, and this
> might sit strangely in the new design.  +jinsukkim who wrote
>
https://docs.google.com/document/d/1Fpvv6aaNwVDTsvHKhgEXZjyEoEgQoKvtBxM-SiDnQ...
> (not final design yet).  Thoughts?

Well VrShell is not the only thing that uses ContentView[Core] so I don't have a
concern at this moment. Looks like it is being extended to support not only
WebContents but also native pages.

[cjgrant, 2017-02-16 15:29:33]

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

[cjgrant, 2017-02-16 15:29:34]

https://codereview.chromium.org/2694103006/diff/20001/chrome/browser/android/...
File chrome/browser/android/vr_shell/android_ui_gesture_target.cc (right):

https://codereview.chromium.org/2694103006/diff/20001/chrome/browser/android/...
chrome/browser/android/vr_shell/android_ui_gesture_target.cc:85: void
AndroidUiGestureTarget::SetScrollDeltas(JNIEnv* env,
nit: I think the style guide prefers packing as many args onto each line as will
fit, no?

https://codereview.chromium.org/2694103006/diff/20001/chrome/browser/android/...
File chrome/browser/android/vr_shell/android_ui_gesture_target.h (right):

https://codereview.chromium.org/2694103006/diff/20001/chrome/browser/android/...
chrome/browser/android/vr_shell/android_ui_gesture_target.h:29: ActionInvalid =
-1,
What about losing the Action prefix on these?  vr_shell::Action::Invalid seems
sufficient?

https://codereview.chromium.org/2694103006/diff/20001/content/public/android/...
File
content/public/android/java/src/org/chromium/content/browser/MotionEventSynthesizer.java
(right):

https://codereview.chromium.org/2694103006/diff/20001/content/public/android/...
content/public/android/java/src/org/chromium/content/browser/MotionEventSynthesizer.java:157:
if (ACTION_HOVER_EXIT == action) androidAction = MotionEvent.ACTION_HOVER_ENTER;
= MotionEvent.ACTION_HOVER_EXIT, presumably.

[mthiesse, 2017-02-16 15:53:32]

On 2017/02/16 04:14:48, Jinsuk Kim wrote:
> On 2017/02/16 03:29:17, aelias wrote:
> > Hmm, we're in the middle of redesigning input dispatching on Android, and
this
> > might sit strangely in the new design.  +jinsukkim who wrote
> >
>
https://docs.google.com/document/d/1Fpvv6aaNwVDTsvHKhgEXZjyEoEgQoKvtBxM-SiDnQ...
> > (not final design yet).  Thoughts?
> 
> Well VrShell is not the only thing that uses ContentView[Core] so I don't have
a
> concern at this moment. Looks like it is being extended to support not only
> WebContents but also native pages.

That's right, we're extending it to support both WebContents and native pages.
I'm not sure how this will shake out in the future, but I doubt the change here
will impact things much either way.

It would be nice though if in the future native pages were also wrapped in a
ViewAndroid so we could unify input paths.

[mthiesse, 2017-02-16 15:53:50]

https://codereview.chromium.org/2694103006/diff/20001/chrome/browser/android/...
File chrome/browser/android/vr_shell/android_ui_gesture_target.cc (right):

https://codereview.chromium.org/2694103006/diff/20001/chrome/browser/android/...
chrome/browser/android/vr_shell/android_ui_gesture_target.cc:85: void
AndroidUiGestureTarget::SetScrollDeltas(JNIEnv* env,
On 2017/02/16 15:29:34, cjgrant wrote:
> nit: I think the style guide prefers packing as many args onto each line as
will
> fit, no?

Apparently not, git cl format sets it to this.

https://codereview.chromium.org/2694103006/diff/20001/chrome/browser/android/...
File chrome/browser/android/vr_shell/android_ui_gesture_target.h (right):

https://codereview.chromium.org/2694103006/diff/20001/chrome/browser/android/...
chrome/browser/android/vr_shell/android_ui_gesture_target.h:29: ActionInvalid =
-1,
On 2017/02/16 15:29:34, cjgrant wrote:
> What about losing the Action prefix on these?  vr_shell::Action::Invalid seems
> sufficient?

Done.

https://codereview.chromium.org/2694103006/diff/20001/content/public/android/...
File
content/public/android/java/src/org/chromium/content/browser/MotionEventSynthesizer.java
(right):

https://codereview.chromium.org/2694103006/diff/20001/content/public/android/...
content/public/android/java/src/org/chromium/content/browser/MotionEventSynthesizer.java:157:
if (ACTION_HOVER_EXIT == action) androidAction = MotionEvent.ACTION_HOVER_ENTER;
On 2017/02/16 15:29:34, cjgrant wrote:
> = MotionEvent.ACTION_HOVER_EXIT, presumably.

Done. Oops.

[aelias_OOO_until_Jul13, 2017-02-17 00:29:22]

content/public/android/ lgtm

[Ted C, 2017-02-17 05:30:06]

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

[Ted C, 2017-02-17 05:30:07]

https://codereview.chromium.org/2694103006/diff/40001/content/public/android/...
File
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
(right):

https://codereview.chromium.org/2694103006/diff/40001/content/public/android/...
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java:2000:
return new MotionEventSynthesizer(getContainerView(), getWindowAndroid());
when is this created/lifetime?  the window android can change for a
contentviewcore via tab reparenting.  We should make sure this doesn't hold onto
a stale one and gets recreated on window moves.

[mthiesse, 2017-02-21 21:44:55]

https://codereview.chromium.org/2694103006/diff/40001/content/public/android/...
File
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
(right):

https://codereview.chromium.org/2694103006/diff/40001/content/public/android/...
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java:2000:
return new MotionEventSynthesizer(getContainerView(), getWindowAndroid());
On 2017/02/17 05:30:07, Ted C wrote:
> when is this created/lifetime?  the window android can change for a
> contentviewcore via tab reparenting.  We should make sure this doesn't hold
onto
> a stale one and gets recreated on window moves.

Ah, good point. There are two options here, we can either store a reference to
the MotionEventSynthesizer here in CVC and add an updateWindowAndroid function
to it, or in RenderWidgetHostViewAndroid::OnDetachedFromWindow() we can call
RenderWidgetHostImpl::SetView() which resets the MotionEventSynthesizer.

Any opinions on which is the better solution?

[Ted C, 2017-02-22 05:21:25]

https://codereview.chromium.org/2694103006/diff/40001/content/public/android/...
File
content/public/android/java/src/org/chromium/content/browser/MotionEventSynthesizer.java
(right):

https://codereview.chromium.org/2694103006/diff/40001/content/public/android/...
content/public/android/java/src/org/chromium/content/browser/MotionEventSynthesizer.java:39:
public MotionEventSynthesizer(View target, WindowAndroid window) {
actually, could this just take a
https://cs.chromium.org/chromium/src/content/public/android/java/src/org/chro...

Then just call getWindowAndroid in the places below and maybe it all would just
work?

[mthiesse, 2017-02-23 23:41:47]

https://codereview.chromium.org/2694103006/diff/40001/content/public/android/...
File
content/public/android/java/src/org/chromium/content/browser/MotionEventSynthesizer.java
(right):

https://codereview.chromium.org/2694103006/diff/40001/content/public/android/...
content/public/android/java/src/org/chromium/content/browser/MotionEventSynthesizer.java:39:
public MotionEventSynthesizer(View target, WindowAndroid window) {
On 2017/02/22 05:21:25, Ted C (OOO till Monday) wrote:
> actually, could this just take a
>
https://cs.chromium.org/chromium/src/content/public/android/java/src/org/chro...
> 
> Then just call getWindowAndroid in the places below and maybe it all would
just
> work?

That's perfect, I didn't know that was a thing. Thanks!

[bshe, 2017-02-24 16:44:27]

On 2017/02/23 23:41:47, mthiesse wrote:
>
https://codereview.chromium.org/2694103006/diff/40001/content/public/android/...
> File
>
content/public/android/java/src/org/chromium/content/browser/MotionEventSynthesizer.java
> (right):
> 
>
https://codereview.chromium.org/2694103006/diff/40001/content/public/android/...
>
content/public/android/java/src/org/chromium/content/browser/MotionEventSynthesizer.java:39:
> public MotionEventSynthesizer(View target, WindowAndroid window) {
> On 2017/02/22 05:21:25, Ted C (OOO till Monday) wrote:
> > actually, could this just take a
> >
>
https://cs.chromium.org/chromium/src/content/public/android/java/src/org/chro...
> > 
> > Then just call getWindowAndroid in the places below and maybe it all would
> just
> > work?
> 
> That's perfect, I didn't know that was a thing. Thanks!

lgtm

[mthiesse, 2017-02-24 16:59:11]

The CQ bit was checked by mthiesse@chromium.org

[mthiesse, 2017-02-24 16:59:11]

The patchset sent to the CQ was uploaded after l-g-t-m from aelias@chromium.org
Link to the patchset: https://codereview.chromium.org/2694103006/#ps60001
(title: "Use WindowAndroidProvider")

[commit-bot: I haz the power, 2017-02-24 16:59:31]

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, 2017-02-24 17:02:12]

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

[commit-bot: I haz the power, 2017-02-24 17:02:13]

Try jobs failed on following builders:
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)

[mthiesse, 2017-02-24 18:59:24]

The CQ bit was checked by mthiesse@chromium.org

[mthiesse, 2017-02-24 18:59:24]

The patchset sent to the CQ was uploaded after l-g-t-m from aelias@chromium.org,
bshe@chromium.org
Link to the patchset: https://codereview.chromium.org/2694103006/#ps80001
(title: "rebase")

[commit-bot: I haz the power, 2017-02-24 19:00:03]

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, 2017-02-24 19:15:04]

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

[commit-bot: I haz the power, 2017-02-24 19:15:05]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[mthiesse, 2017-02-24 20:22:59]

The CQ bit was checked by mthiesse@chromium.org

[mthiesse, 2017-02-24 20:22:59]

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

[commit-bot: I haz the power, 2017-02-24 20:23:38]

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, 2017-02-24 23:58:38]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1487967779436850,
"parent_rev": "93dc0c3c948d14b5b11f45408e5af9cbd1a5ddd1", "commit_rev":
"8ba7911391a74da0f186b82801d3d6279a197589"}

[commit-bot: I haz the power, 2017-02-25 00:00:11]

Description was changed from

==========
Inject vr controller events into Android Native UI.

Generalizes the MotionEventSynthesizer to support input into arbitrary Java
views, and connects VR controller/gesture output to the MotionEventSynthesizer.

This also fixes some rendering bugs where onDraw was not being called after
scrolling preventing the texture from updating correctly.

BUG=661609
==========

to

==========
Inject vr controller events into Android Native UI.

Generalizes the MotionEventSynthesizer to support input into arbitrary Java
views, and connects VR controller/gesture output to the MotionEventSynthesizer.

This also fixes some rendering bugs where onDraw was not being called after
scrolling preventing the texture from updating correctly.

BUG=661609

Review-Url: https://codereview.chromium.org/2694103006
Cr-Commit-Position: refs/heads/master@{#453004}
Committed:
https://chromium.googlesource.com/chromium/src/+/8ba7911391a74da0f186b82801d3...
==========

[commit-bot: I haz the power, 2017-02-25 00:00:13]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/8ba7911391a74da0f186b82801d3...