[Remoting Android] Move setAnimationEnabled to TouchInputHandler

[Remoting Android] Move setAnimationEnabled to TouchInputHandler

Since GlDisplay already triggers an onCanvasRendered() event, TouchInputHandler
just simply needs to listen to that event decide what to do inside its scope.
This CL moves the setAnimationEnabled logic into TouchInputHandler since
DesktopView doesn't need to do anything in this process.

BUG=641123
Committed: https://crrev.com/706c49eb66412c04352e53273266dfe08946e7ef
Cr-Commit-Position: refs/heads/master@{#415558}

[Yuwei, 2016-08-26 20:34:31]

yuweih@chromium.org changed reviewers:
+ joedow@chromium.org, zijiehe@chromium.org

[Yuwei, 2016-08-26 20:34:32]

ptal

https://codereview.chromium.org/2281983004/diff/20001/remoting/android/java/s...
File remoting/android/java/src/org/chromium/chromoting/DesktopView.java (right):

https://codereview.chromium.org/2281983004/diff/20001/remoting/android/java/s...
remoting/android/java/src/org/chromium/chromoting/DesktopView.java:50: 
DesktopView.onCanvasRendered() is just temporary. Finally all events will be
removed from DesktopView and there will be a simpler way to forward events from
GlDisplay.

[joedow, 2016-08-29 22:11:34]

lgtm

[Yuwei, 2016-08-29 22:16:41]

joedow@ Thanks!

zijiehe@ Any last minute comments?

[Hzj_jie, 2016-08-29 22:17:18]

On 2016/08/29 22:16:41, Yuwei wrote:
> joedow@ Thanks!
> 
> zijiehe@ Any last minute comments?

Yes, looking. Sorry for the late response.

[Hzj_jie, 2016-08-29 22:20:23]

https://codereview.chromium.org/2281983004/diff/20001/remoting/android/java/s...
File remoting/android/java/src/org/chromium/chromoting/DesktopView.java (right):

https://codereview.chromium.org/2281983004/diff/20001/remoting/android/java/s...
remoting/android/java/src/org/chromium/chromoting/DesktopView.java:50: 
On 2016/08/26 20:34:32, Yuwei wrote:
> DesktopView.onCanvasRendered() is just temporary. Finally all events will be
> removed from DesktopView and there will be a simpler way to forward events
from
> GlDisplay.

I believe mOnTouch is still raised from GlDesktopView, right?

https://codereview.chromium.org/2281983004/diff/20001/remoting/android/java/s...
File remoting/android/java/src/org/chromium/chromoting/TouchInputHandler.java
(right):

https://codereview.chromium.org/2281983004/diff/20001/remoting/android/java/s...
remoting/android/java/src/org/chromium/chromoting/TouchInputHandler.java:217:
mProcessAnimationRunnable = new Event.ParameterRunnable<Void>() {
Why do not use SelfRemovableParameterRunner? It seems like
SelfRemovableParameterRunner is the designed for this scenario.

https://codereview.chromium.org/2281983004/diff/20001/remoting/android/java/s...
remoting/android/java/src/org/chromium/chromoting/TouchInputHandler.java:262:
mOnCanvasRenderedListenerKey = null;
This seems a little bit strange to me. Is there any reason enabled == false but
mOnCanvasRenderedListenerKey == null?

[Yuwei, 2016-08-29 23:24:25]

ptal

joedow@ would you like to revisit this CL since I have made some fundamental
changes?

https://codereview.chromium.org/2281983004/diff/20001/remoting/android/java/s...
File remoting/android/java/src/org/chromium/chromoting/DesktopView.java (right):

https://codereview.chromium.org/2281983004/diff/20001/remoting/android/java/s...
remoting/android/java/src/org/chromium/chromoting/DesktopView.java:50: 
On 2016/08/29 22:20:23, Hzj_jie wrote:
> On 2016/08/26 20:34:32, Yuwei wrote:
> > DesktopView.onCanvasRendered() is just temporary. Finally all events will be
> > removed from DesktopView and there will be a simpler way to forward events
> from
> > GlDisplay.
> 
> I believe mOnTouch is still raised from GlDesktopView, right?

Yep, but DesktopView to be precise. That's a view's event so we probably can't
get rid of it.

https://codereview.chromium.org/2281983004/diff/20001/remoting/android/java/s...
File remoting/android/java/src/org/chromium/chromoting/TouchInputHandler.java
(right):

https://codereview.chromium.org/2281983004/diff/20001/remoting/android/java/s...
remoting/android/java/src/org/chromium/chromoting/TouchInputHandler.java:217:
mProcessAnimationRunnable = new Event.ParameterRunnable<Void>() {
On 2016/08/29 22:20:23, Hzj_jie wrote:
> Why do not use SelfRemovableParameterRunner? It seems like
> SelfRemovableParameterRunner is the designed for this scenario.

Agreed.

Done.

https://codereview.chromium.org/2281983004/diff/20001/remoting/android/java/s...
remoting/android/java/src/org/chromium/chromoting/TouchInputHandler.java:262:
mOnCanvasRenderedListenerKey = null;
On 2016/08/29 22:20:23, Hzj_jie wrote:
> This seems a little bit strange to me. Is there any reason enabled == false
but
> mOnCanvasRenderedListenerKey == null?

I think that case would never happen. I probably had added this just for sanity.
This is obsolete and removed though.

[Hzj_jie, 2016-08-29 23:28:57]

LGTM for me. Thank you.

[Yuwei, 2016-08-30 22:11:21]

joedow@ if you have no other comments then I will commit this CL by the end of
today. Thanks!

[Yuwei, 2016-08-31 00:26:26]

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

[commit-bot: I haz the power, 2016-08-31 00:26: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, 2016-08-31 01:12:25]

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

[commit-bot: I haz the power, 2016-08-31 01:12:25]

Dry run: This issue passed the CQ dry run.

[Yuwei, 2016-08-31 03:34:52]

The CQ bit was checked by yuweih@chromium.org

[Yuwei, 2016-08-31 03:34:53]

The patchset sent to the CQ was uploaded after l-g-t-m from joedow@chromium.org
Link to the patchset: https://codereview.chromium.org/2281983004/#ps40001
(title: "Reviewer's Feedback")

[commit-bot: I haz the power, 2016-08-31 03:35:12]

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-08-31 04:31:26]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-08-31 04:33:46]

Description was changed from

==========
[Remoting Android] Move setAnimationEnabled to TouchInputHandler

Since GlDisplay already triggers an onCanvasRendered() event, TouchInputHandler
just simply needs to listen to that event decide what to do inside its scope.
This CL moves the setAnimationEnabled logic into TouchInputHandler since
DesktopView doesn't need to do anything in this process.

BUG=641123
==========

to

==========
[Remoting Android] Move setAnimationEnabled to TouchInputHandler

Since GlDisplay already triggers an onCanvasRendered() event, TouchInputHandler
just simply needs to listen to that event decide what to do inside its scope.
This CL moves the setAnimationEnabled logic into TouchInputHandler since
DesktopView doesn't need to do anything in this process.

BUG=641123

Committed: https://crrev.com/706c49eb66412c04352e53273266dfe08946e7ef
Cr-Commit-Position: refs/heads/master@{#415558}
==========

[commit-bot: I haz the power, 2016-08-31 04:33:47]

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