Implement basic point-and-touch mouse input for Android client

Implement basic point-and-touch mouse input for Android client

One can now use a remote computer as if it were a touchscreen device.  This also trims down our requested permissions to see whether all devices are okay without the somewhat intrusive MANAGE_ACCOUNTS.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=213209

[solb, 2013-07-20 05:17:40]

This is designed to be applied on top of
https://codereview.chromium.org/19482019.

[garykac, 2013-07-20 21:25:26]

https://codereview.chromium.org/19500017/diff/1/remoting/client/jni/chromotin...
File remoting/client/jni/chromoting_jni_instance.cc (right):

https://codereview.chromium.org/19500017/diff/1/remoting/client/jni/chromotin...
remoting/client/jni/chromoting_jni_instance.cc:88:
protocol::MouseEvent_MouseButton button) {
This should be a MouseEvent so that you have all the mouse event information.

https://codereview.chromium.org/19500017/diff/1/remoting/client/jni/chromotin...
remoting/client/jni/chromoting_jni_instance.cc:106:
static_cast<protocol::MouseEvent_MouseButton>(abs(button)));
abs(button)...

This whole static_cast ickiness goes away if you use the MouseEvent's button and
button_down values.

https://codereview.chromium.org/19500017/diff/1/remoting/client/jni/chromotin...
File remoting/client/jni/chromoting_jni_instance.h (right):

https://codereview.chromium.org/19500017/diff/1/remoting/client/jni/chromotin...
remoting/client/jni/chromoting_jni_instance.h:61: // A negative value of
|button| releases the button described by -|button|.
Button press/release should be stored as a boolean rather than overloading the
button id in this manner.

The approach used here leads to code like:
   if (button.id < 0)
rather than:
   if (button.pressed)
The latter being much more readable.

Also, MouseEvent_MouseButton is an enum defined to have only positive values.
Storing invalid values here can lead to madness.

[solb, 2013-07-22 19:13:47]

This one should *actually* be ready for review now.

https://codereview.chromium.org/19500017/diff/1/remoting/android/java/src/org...
File remoting/android/java/src/org/chromium/chromoting/DesktopView.java (right):

https://codereview.chromium.org/19500017/diff/1/remoting/android/java/src/org...
remoting/android/java/src/org/chromium/chromoting/DesktopView.java:257:
mMouseButton = 1;  // TODO Name me!
Sorry; I had forgotten how rough this revision was! I shouldn't have sent it out
for review on Friday...

https://codereview.chromium.org/19500017/diff/1/remoting/client/jni/chromotin...
File remoting/client/jni/chromoting_jni_instance.cc (right):

https://codereview.chromium.org/19500017/diff/1/remoting/client/jni/chromotin...
remoting/client/jni/chromoting_jni_instance.cc:88:
protocol::MouseEvent_MouseButton button) {
On 2013/07/20 21:25:26, garykac wrote:
> This should be a MouseEvent so that you have all the mouse event information.

Since this is being called from the JNI binding in jni_interface.cc, I just
added a boolean pressed flag here to avoid making that binding do anything more
than forwarding the call.

https://codereview.chromium.org/19500017/diff/1/remoting/client/jni/chromotin...
remoting/client/jni/chromoting_jni_instance.cc:106:
static_cast<protocol::MouseEvent_MouseButton>(abs(button)));
On 2013/07/20 21:25:26, garykac wrote:
> abs(button)...
> 
> This whole static_cast ickiness goes away if you use the MouseEvent's button
and
> button_down values.

I accomplished this with a boolean flag, as aforementioned.

https://codereview.chromium.org/19500017/diff/1/remoting/client/jni/chromotin...
File remoting/client/jni/chromoting_jni_instance.h (right):

https://codereview.chromium.org/19500017/diff/1/remoting/client/jni/chromotin...
remoting/client/jni/chromoting_jni_instance.h:61: // A negative value of
|button| releases the button described by -|button|.
On 2013/07/20 21:25:26, garykac wrote:
> Button press/release should be stored as a boolean rather than overloading the
> button id in this manner.
> 
> The approach used here leads to code like:
>    if (button.id < 0)
> rather than:
>    if (button.pressed)
> The latter being much more readable.
> 
> Also, MouseEvent_MouseButton is an enum defined to have only positive values.
> Storing invalid values here can lead to madness.

Done.

[garykac, 2013-07-22 23:43:38]

https://codereview.chromium.org/19500017/diff/16001/remoting/android/java/src...
File remoting/android/java/src/org/chromium/chromoting/DesktopView.java (right):

https://codereview.chromium.org/19500017/diff/16001/remoting/android/java/src...
remoting/android/java/src/org/chromium/chromoting/DesktopView.java:49: private
static class MouseButton {
this seems more complex than necessary. why not just use mMouseId and
mMousePressed in DesktopView

[solb, 2013-07-23 00:30:00]

https://codereview.chromium.org/19500017/diff/16001/remoting/android/java/src...
File remoting/android/java/src/org/chromium/chromoting/DesktopView.java (right):

https://codereview.chromium.org/19500017/diff/16001/remoting/android/java/src...
remoting/android/java/src/org/chromium/chromoting/DesktopView.java:49: private
static class MouseButton {
On 2013/07/22 23:43:38, garykac wrote:
> this seems more complex than necessary. why not just use mMouseId and
> mMousePressed in DesktopView

Done.

[garykac, 2013-07-23 00:49:27]

lgtm

[commit-bot: I haz the power, 2013-07-23 00:53:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/solb@chromium.org/19500017/25002

[commit-bot: I haz the power, 2013-07-23 01:15:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/solb@chromium.org/19500017/38007

[commit-bot: I haz the power, 2013-07-23 01:40:00]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-07-23 16:54:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/solb@chromium.org/19500017/38007

[commit-bot: I haz the power, 2013-07-23 20:16:46]

Change committed as 213209