Forward GenericMotionEvent to EventForwarder

Forward GenericMotionEvent to EventForwarder

This CL sends down generic motion event (scroll, button clicks)
through ViewAndroid tree.

- The native class for JoystickScrollProvider was removed in order
to make use of ViewAndroid tree path.
- MotionEventAndroid was augmented to hold data for wheel scroll
event so that we can recycle ViewAndroid::HitTest for the event.
- Joystick-related classes are created lazily upon handling the first joystick event.

BUG=703023
Review-Url: https://codereview.chromium.org/2770613002
Cr-Commit-Position: refs/heads/master@{#460335}
Committed: https://chromium.googlesource.com/chromium/src/+/b40b402c7c6bfef5538ca4b28f8acc36764b5f70

[Jinsuk Kim, 2017-03-22 08:15:27]

The CQ bit was checked by jinsukkim@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-22 08:15:44]

Dry run: 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-03-22 08:39:28]

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

[commit-bot: I haz the power, 2017-03-22 08:39:29]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[Jinsuk Kim, 2017-03-24 08:03:10]

The CQ bit was checked by jinsukkim@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-24 08:03:23]

Dry run: 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-03-24 08:51:15]

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

[commit-bot: I haz the power, 2017-03-24 08:51:16]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[Jinsuk Kim, 2017-03-24 10:11:11]

The CQ bit was checked by jinsukkim@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-24 10:11:21]

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

[Jinsuk Kim, 2017-03-24 11:11:40]

Description was changed from

==========
Forward GenericMotionEvent to EventForwarder

This CL sends down generic motion event (scroll, button clicks)
through ViewAndroid tree.

- The native class for JoystickScrollProvider was removed in order
to make use of ViewAndroid tree path.
- MotionEventAndroid was augmented to hold wheel scroll event so
that we can recycle ViewAndroid::HitTest for the event.
- Joystick-related classes were moved to |org.chromium.ui.input|.

BUG=703023
==========

to

==========
Forward GenericMotionEvent to EventForwarder

This CL sends down generic motion event (scroll, button clicks)
through ViewAndroid tree.

- The native class for JoystickScrollProvider was removed in order
to make use of ViewAndroid tree path.
- MotionEventAndroid was augmented to hold data for wheel scroll
event so that we can recycle ViewAndroid::HitTest for the event.
- Joystick-related classes were moved to |org.chromium.ui.input|,
and created lazily upon handling the first joystick event.

BUG=703023
==========

[commit-bot: I haz the power, 2017-03-24 11:40:26]

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

[commit-bot: I haz the power, 2017-03-24 11:40:27]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Jinsuk Kim, 2017-03-26 23:15:59]

The CQ bit was checked by jinsukkim@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-26 23:16:14]

Dry run: 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-03-26 23:58:32]

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

[commit-bot: I haz the power, 2017-03-26 23:58:34]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Jinsuk Kim, 2017-03-27 05:21:45]

Patchset #6 (id:100001) has been deleted

[Jinsuk Kim, 2017-03-27 05:21:58]

Patchset #5 (id:80001) has been deleted

[Jinsuk Kim, 2017-03-27 05:22:07]

Patchset #1 (id:1) has been deleted

[Jinsuk Kim, 2017-03-27 05:22:18]

Patchset #1 (id:20001) has been deleted

[Jinsuk Kim, 2017-03-27 05:22:28]

Patchset #1 (id:40001) has been deleted

[Jinsuk Kim, 2017-03-27 05:22:37]

Patchset #1 (id:60001) has been deleted

[Jinsuk Kim, 2017-03-27 05:29:40]

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

[Jinsuk Kim, 2017-03-27 05:29:41]

Note: GenericMotionEvent is not completely pulled out of CVC, because it still
depends on several properties in CVC like RenderCoordinates, viewport size, etc.
Migrating them all would make the CL too big. This CL goes as far as rerouting
the events at CVC.onGenericMotionEvents to EventForwarder. Am planning to do
clean up the rest of the  refactoring in follow-up CLs.

aelias@ please review changes ui/events/android/
boliu@ please review changes in content/, ui/android/

[boliu, 2017-03-27 17:44:44]

https://codereview.chromium.org/2770613002/diff/120001/ui/android/java/src/or...
File ui/android/java/src/org/chromium/ui/base/EventForwarder.java (right):

https://codereview.chromium.org/2770613002/diff/120001/ui/android/java/src/or...
ui/android/java/src/org/chromium/ui/base/EventForwarder.java:215: public
JoystickScrollProvider createJoystickScrollProvider(
Having JoystickScrollProvider and whatnot in ui makes no sense to me. Why can't
these things just stay in content?

[Jinsuk Kim, 2017-03-27 22:42:06]

https://codereview.chromium.org/2770613002/diff/120001/ui/android/java/src/or...
File ui/android/java/src/org/chromium/ui/base/EventForwarder.java (right):

https://codereview.chromium.org/2770613002/diff/120001/ui/android/java/src/or...
ui/android/java/src/org/chromium/ui/base/EventForwarder.java:215: public
JoystickScrollProvider createJoystickScrollProvider(
On 2017/03/27 17:44:44, boliu wrote:
> Having JoystickScrollProvider and whatnot in ui makes no sense to me. Why
can't
> these things just stay in content?

The motivation is to migrate these input events flow from CVC to EventForwarder.
Unlike simple touch events, generic motion events need to take care of a few
other stuff. One of them is joystick events. Without moving this, it seems not
possible (at least to me) to do the migration. 

Got any other idea?

[Jinsuk Kim, 2017-03-27 22:42:46]

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

[Jinsuk Kim, 2017-03-27 22:42:46]

Forgot to add all the reviewers..

[boliu, 2017-03-27 22:54:23]

https://codereview.chromium.org/2770613002/diff/120001/ui/android/java/src/or...
File ui/android/java/src/org/chromium/ui/base/EventForwarder.java (right):

https://codereview.chromium.org/2770613002/diff/120001/ui/android/java/src/or...
ui/android/java/src/org/chromium/ui/base/EventForwarder.java:215: public
JoystickScrollProvider createJoystickScrollProvider(
On 2017/03/27 22:42:06, Jinsuk Kim wrote:
> On 2017/03/27 17:44:44, boliu wrote:
> > Having JoystickScrollProvider and whatnot in ui makes no sense to me. Why
> can't
> > these things just stay in content?
> 
> The motivation is to migrate these input events flow from CVC to
EventForwarder.
> Unlike simple touch events, generic motion events need to take care of a few
> other stuff. One of them is joystick events. Without moving this, it seems not
> possible (at least to me) to do the migration. 
> 
> Got any other idea?

afaict, the dependency (from higher to lower) is content -> JoyStick ->
ui/EventHandler. Is that correct? ie I don't see anything in ui or EventHandler
actually using any of this JoyStick code. So why is it in ui at all?

Pulling joystick stuff out of CVC is fine, but I don't see why it has to be in
ui?

[Jinsuk Kim, 2017-03-27 22:59:51]

https://codereview.chromium.org/2770613002/diff/120001/ui/android/java/src/or...
File ui/android/java/src/org/chromium/ui/base/EventForwarder.java (right):

https://codereview.chromium.org/2770613002/diff/120001/ui/android/java/src/or...
ui/android/java/src/org/chromium/ui/base/EventForwarder.java:215: public
JoystickScrollProvider createJoystickScrollProvider(
On 2017/03/27 22:54:22, boliu wrote:
> On 2017/03/27 22:42:06, Jinsuk Kim wrote:
> > On 2017/03/27 17:44:44, boliu wrote:
> > > Having JoystickScrollProvider and whatnot in ui makes no sense to me. Why
> > can't
> > > these things just stay in content?
> > 
> > The motivation is to migrate these input events flow from CVC to
> EventForwarder.
> > Unlike simple touch events, generic motion events need to take care of a few
> > other stuff. One of them is joystick events. Without moving this, it seems
not
> > possible (at least to me) to do the migration. 
> > 
> > Got any other idea?
> 
> afaict, the dependency (from higher to lower) is content -> JoyStick ->
> ui/EventHandler. Is that correct? ie I don't see anything in ui or
EventHandler
> actually using any of this JoyStick code. So why is it in ui at all?
> 
> Pulling joystick stuff out of CVC is fine, but I don't see why it has to be in
> ui?

.. wish it had been discussed in the crbug.com/703023 before, which I had
created just for that. EventForwarder has a reference to it. So it needs to be
where ui can have an access to, not in content any more.

[Jinsuk Kim, 2017-03-27 23:04:25]

https://codereview.chromium.org/2770613002/diff/120001/ui/android/java/src/or...
File ui/android/java/src/org/chromium/ui/base/EventForwarder.java (right):

https://codereview.chromium.org/2770613002/diff/120001/ui/android/java/src/or...
ui/android/java/src/org/chromium/ui/base/EventForwarder.java:215: public
JoystickScrollProvider createJoystickScrollProvider(
On 2017/03/27 22:59:51, Jinsuk Kim wrote:
> On 2017/03/27 22:54:22, boliu wrote:
> > On 2017/03/27 22:42:06, Jinsuk Kim wrote:
> > > On 2017/03/27 17:44:44, boliu wrote:
> > > > Having JoystickScrollProvider and whatnot in ui makes no sense to me.
Why
> > > can't
> > > > these things just stay in content?
> > > 
> > > The motivation is to migrate these input events flow from CVC to
> > EventForwarder.
> > > Unlike simple touch events, generic motion events need to take care of a
few
> > > other stuff. One of them is joystick events. Without moving this, it seems
> not
> > > possible (at least to me) to do the migration. 
> > > 
> > > Got any other idea?
> > 
> > afaict, the dependency (from higher to lower) is content -> JoyStick ->
> > ui/EventHandler. Is that correct? ie I don't see anything in ui or
> EventHandler
> > actually using any of this JoyStick code. So why is it in ui at all?
> > 
> > Pulling joystick stuff out of CVC is fine, but I don't see why it has to be
in
> > ui?
> 
> .. wish it had been discussed in the crbug.com/703023 before, which I had
> created just for that. EventForwarder has a reference to it. So it needs to be
> where ui can have an access to, not in content any more.

May I was not clear. I meant that EventForwarder provides the path to native for
JoystickScrollProvider here.

[boliu, 2017-03-27 23:19:17]

https://codereview.chromium.org/2770613002/diff/120001/ui/android/java/src/or...
File ui/android/java/src/org/chromium/ui/base/EventForwarder.java (right):

https://codereview.chromium.org/2770613002/diff/120001/ui/android/java/src/or...
ui/android/java/src/org/chromium/ui/base/EventForwarder.java:215: public
JoystickScrollProvider createJoystickScrollProvider(
On 2017/03/27 23:04:24, Jinsuk Kim wrote:
> On 2017/03/27 22:59:51, Jinsuk Kim wrote:
> > On 2017/03/27 22:54:22, boliu wrote:
> > > On 2017/03/27 22:42:06, Jinsuk Kim wrote:
> > > > On 2017/03/27 17:44:44, boliu wrote:
> > > > > Having JoystickScrollProvider and whatnot in ui makes no sense to me.
> Why
> > > > can't
> > > > > these things just stay in content?
> > > > 
> > > > The motivation is to migrate these input events flow from CVC to
> > > EventForwarder.
> > > > Unlike simple touch events, generic motion events need to take care of a
> few
> > > > other stuff. One of them is joystick events. Without moving this, it
seems
> > not
> > > > possible (at least to me) to do the migration. 
> > > > 
> > > > Got any other idea?
> > > 
> > > afaict, the dependency (from higher to lower) is content -> JoyStick ->
> > > ui/EventHandler. Is that correct? ie I don't see anything in ui or
> > EventHandler
> > > actually using any of this JoyStick code. So why is it in ui at all?
> > > 
> > > Pulling joystick stuff out of CVC is fine, but I don't see why it has to
be
> in
> > > ui?
> > 
> > .. wish it had been discussed in the crbug.com/703023 before, which I had
> > created just for that. EventForwarder has a reference to it. So it needs to
be
> > where ui can have an access to, not in content any more.
> 
> May I was not clear. I meant that EventForwarder provides the path to native
for
> JoystickScrollProvider here. 

You can just make onMouseWheelEvent public here then?

[Jinsuk Kim, 2017-03-28 02:25:32]

Patchset #2 (id:140001) has been deleted

[Jinsuk Kim, 2017-03-28 02:31:47]

Patchset #3 (id:180001) has been deleted

[Jinsuk Kim, 2017-03-28 02:31:53]

Patchset #2 (id:160001) has been deleted

[Jinsuk Kim, 2017-03-28 02:40:23]

Description was changed from

==========
Forward GenericMotionEvent to EventForwarder

This CL sends down generic motion event (scroll, button clicks)
through ViewAndroid tree.

- The native class for JoystickScrollProvider was removed in order
to make use of ViewAndroid tree path.
- MotionEventAndroid was augmented to hold data for wheel scroll
event so that we can recycle ViewAndroid::HitTest for the event.
- Joystick-related classes were moved to |org.chromium.ui.input|,
and created lazily upon handling the first joystick event.

BUG=703023
==========

to

==========
Forward GenericMotionEvent to EventForwarder

This CL sends down generic motion event (scroll, button clicks)
through ViewAndroid tree.

- The native class for JoystickScrollProvider was removed in order
to make use of ViewAndroid tree path.
- MotionEventAndroid was augmented to hold data for wheel scroll
event so that we can recycle ViewAndroid::HitTest for the event.
- Joystick-related classes are created lazily upon handling the first joystick
event.

BUG=703023
==========

[Jinsuk Kim, 2017-03-28 02:40:33]

PTAL

https://codereview.chromium.org/2770613002/diff/120001/ui/android/java/src/or...
File ui/android/java/src/org/chromium/ui/base/EventForwarder.java (right):

https://codereview.chromium.org/2770613002/diff/120001/ui/android/java/src/or...
ui/android/java/src/org/chromium/ui/base/EventForwarder.java:215: public
JoystickScrollProvider createJoystickScrollProvider(
On 2017/03/27 23:19:17, boliu wrote:
> On 2017/03/27 23:04:24, Jinsuk Kim wrote:
> > On 2017/03/27 22:59:51, Jinsuk Kim wrote:
> > > On 2017/03/27 22:54:22, boliu wrote:
> > > > On 2017/03/27 22:42:06, Jinsuk Kim wrote:
> > > > > On 2017/03/27 17:44:44, boliu wrote:
> > > > > > Having JoystickScrollProvider and whatnot in ui makes no sense to
me.
> > Why
> > > > > can't
> > > > > > these things just stay in content?
> > > > > 
> > > > > The motivation is to migrate these input events flow from CVC to
> > > > EventForwarder.
> > > > > Unlike simple touch events, generic motion events need to take care of
a
> > few
> > > > > other stuff. One of them is joystick events. Without moving this, it
> seems
> > > not
> > > > > possible (at least to me) to do the migration. 
> > > > > 
> > > > > Got any other idea?
> > > > 
> > > > afaict, the dependency (from higher to lower) is content -> JoyStick ->
> > > > ui/EventHandler. Is that correct? ie I don't see anything in ui or
> > > EventHandler
> > > > actually using any of this JoyStick code. So why is it in ui at all?
> > > > 
> > > > Pulling joystick stuff out of CVC is fine, but I don't see why it has to
> be
> > in
> > > > ui?
> > > 
> > > .. wish it had been discussed in the crbug.com/703023 before, which I had
> > > created just for that. EventForwarder has a reference to it. So it needs
to
> be
> > > where ui can have an access to, not in content any more.
> > 
> > May I was not clear. I meant that EventForwarder provides the path to native
> for
> > JoystickScrollProvider here. 
> 
> You can just make onMouseWheelEvent public here then?

Done. Moved Joystick.+Provider classes back to content/

[boliu, 2017-03-28 16:28:43]

lgtm

https://codereview.chromium.org/2770613002/diff/220001/content/public/android...
File
content/public/android/java/src/org/chromium/content/browser/input/JoystickZoomProvider.java
(right):

https://codereview.chromium.org/2770613002/diff/220001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/input/JoystickZoomProvider.java:95:
mZoomXcoord = xcoord;
maybe should be fixed later since this is an existing bug, but what happens when
the view changes size?

https://codereview.chromium.org/2770613002/diff/220001/ui/android/view_androi...
File ui/android/view_android.cc (right):

https://codereview.chromium.org/2770613002/diff/220001/ui/android/view_androi...
ui/android/view_android.cc:312: bool
ViewAndroid::SendMouseWheelEventToClient(ViewClient* client,
add "// static" to all static methods

[aelias_OOO_until_Jul13, 2017-03-28 19:08:30]

lgtm

[Jinsuk Kim, 2017-03-28 23:17:44]

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

[Jinsuk Kim, 2017-03-28 23:17:45]

Also removed AnimationIntervalProvider.java, which I forgot to do in the prev
patch.

sadrul@ could you approve ui/android/DEPS that has a new dependency on a file in
ui/events?

https://codereview.chromium.org/2770613002/diff/220001/content/public/android...
File
content/public/android/java/src/org/chromium/content/browser/input/JoystickZoomProvider.java
(right):

https://codereview.chromium.org/2770613002/diff/220001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/input/JoystickZoomProvider.java:95:
mZoomXcoord = xcoord;
On 2017/03/28 16:28:42, boliu wrote:
> maybe should be fixed later since this is an existing bug, but what happens
when
> the view changes size?

Acknowledged.

https://codereview.chromium.org/2770613002/diff/220001/ui/android/view_androi...
File ui/android/view_android.cc (right):

https://codereview.chromium.org/2770613002/diff/220001/ui/android/view_androi...
ui/android/view_android.cc:312: bool
ViewAndroid::SendMouseWheelEventToClient(ViewClient* client,
On 2017/03/28 16:28:42, boliu wrote:
> add "// static" to all static methods

Done.

[sadrul, 2017-03-29 02:01:54]

lgtm

[Jinsuk Kim, 2017-03-29 05:45:13]

The CQ bit was checked by jinsukkim@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-29 05:45:48]

Dry run: 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-03-29 07:07:01]

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

[commit-bot: I haz the power, 2017-03-29 07:07:03]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Jinsuk Kim, 2017-03-29 09:33:02]

The CQ bit was checked by jinsukkim@chromium.org

[Jinsuk Kim, 2017-03-29 09:33:03]

The patchset sent to the CQ was uploaded after l-g-t-m from aelias@chromium.org,
boliu@chromium.org
Link to the patchset: https://codereview.chromium.org/2770613002/#ps260001
(title: "comments")

[commit-bot: I haz the power, 2017-03-29 09:33:16]

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-03-29 09:39:12]

CQ is committing da patch.

Bot data: {"patchset_id": 260001, "attempt_start_ts": 1490779982661170,
"parent_rev": "a9b00b963270f09b359251568257001734442383", "commit_rev":
"b40b402c7c6bfef5538ca4b28f8acc36764b5f70"}

[commit-bot: I haz the power, 2017-03-29 09:40:00]

Description was changed from

==========
Forward GenericMotionEvent to EventForwarder

This CL sends down generic motion event (scroll, button clicks)
through ViewAndroid tree.

- The native class for JoystickScrollProvider was removed in order
to make use of ViewAndroid tree path.
- MotionEventAndroid was augmented to hold data for wheel scroll
event so that we can recycle ViewAndroid::HitTest for the event.
- Joystick-related classes are created lazily upon handling the first joystick
event.

BUG=703023
==========

to

==========
Forward GenericMotionEvent to EventForwarder

This CL sends down generic motion event (scroll, button clicks)
through ViewAndroid tree.

- The native class for JoystickScrollProvider was removed in order
to make use of ViewAndroid tree path.
- MotionEventAndroid was augmented to hold data for wheel scroll
event so that we can recycle ViewAndroid::HitTest for the event.
- Joystick-related classes are created lazily upon handling the first joystick
event.

BUG=703023

Review-Url: https://codereview.chromium.org/2770613002
Cr-Commit-Position: refs/heads/master@{#460335}
Committed:
https://chromium.googlesource.com/chromium/src/+/b40b402c7c6bfef5538ca4b28f8a...
==========

[commit-bot: I haz the power, 2017-03-29 09:40:05]

Committed patchset #5 (id:260001) as
https://chromium.googlesource.com/chromium/src/+/b40b402c7c6bfef5538ca4b28f8a...