[Chromoting] Feedback animation has stopped

remoting/android/java/src/org/chromium/chromoting/DesktopView.java

Index: remoting/android/java/src/org/chromium/chromoting/DesktopView.java
diff --git a/remoting/android/java/src/org/chromium/chromoting/DesktopView.java b/remoting/android/java/src/org/chromium/chromoting/DesktopView.java
index e0ceeae98ab1ca6db89ff26539d2ec9ae0445bc0..fd5751bc057f7c830badcf7c0ab755ae4fcb2f70 100644
--- a/remoting/android/java/src/org/chromium/chromoting/DesktopView.java
+++ b/remoting/android/java/src/org/chromium/chromoting/DesktopView.java
@@ -201,16 +201,12 @@ public class DesktopView extends SurfaceView implements DesktopViewInterface,
 
         getHolder().unlockCanvasAndPost(canvas);
 
-        synchronized (mAnimationLock) {
-            if (mInputAnimationRunning) {
-                getHandler().postAtTime(new Runnable() {
-                    @Override
-                    public void run() {
-                        processAnimation();
-                    }
-                }, startTimeMs + 30);
+        getHandler().postAtTime(new Runnable() {
+            @Override

[Lambros, 2016/06/27 23:16:16]

I don't understand why removing the mInputAnimationRunning check should fix the
bug?
We don't want to have paint() called every 30ms, do we, if there's no animation
running?

[Hzj_jie, 2016/06/27 23:45:45]

FeedbackAnimator adds itself as SelfRemovableParameterRunnable into mOnPaint, so
if mOnPaint is not empty, we will need to repaint it in the next round. After we
moving AnimationJob into the same implementation, we do not need to lock and
change mInputAnimationRunning anymore.

[Yuwei, 2016/06/28 04:26:51]

If I understand it right, the bug is:

* When calling showInputFeedback(), a FeedbackAnimator is attached to the
|mOnPaint| event and requestRepaint() is called.
* In the middle of paint(), mOnPaint is triggered and the feedback circle is
drawn.
* On line 205, since mInputAnimationRunning has not been set to true,
processAnimation won't be called and nothing will happen after the first frame
is rendered :(

Removing the check will force processAnimation() to be called, which will keep
requesting repaint until FeedbackAnimator removes itself from the event listener
list, which fixes this problem...

processAnimation() will only request repaint when necessary but I still don't
feel right to always call/schedule processAnimation() even if there is no
animation and it has no side effect...

[Yuwei, 2016/06/28 08:43:40]

I think if you use mOnPaint for TouchInputHandler animations then you can remove
both mInputAnimationRunning and processAnimation() and do something like:

mOnPaint.raise(...);
if (!mOnPaint.isEmpty()) {
    getHandler().postAtTime(requestRepaint, startTimeMs + 30);
}

in the painting loop

[Hzj_jie, 2016/06/28 18:57:49]

Yes, that's the plan. This root cause of this bug is the mix of callback pattern
and event pattern.

+            public void run() {
+                processAnimation();
             }
-        }
+        }, startTimeMs + 30);
     }
 
     private void processAnimation() {