[Chromoting] Minor changes to TapGestureDetector

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

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 package org.chromium.chromoting;
 
 import android.content.Context;
 import android.graphics.PointF;
 import android.os.Handler;
 import android.os.Message;
@@ skipping 24 matching lines @@
          * Notified when a long-touch event occurs.
          *
          * @param pointerCount The number of fingers held down.
          * @param x The x coordinate of the initial finger tapped.
          * @param y The y coordinate of the initial finger tapped.
          */
         void onLongPress(int pointerCount, float x, float y);
     }
 
     /** The listener to which notifications are sent. */
-    private OnTapListener mListener;
+    private final OnTapListener mListener;

[Lambros, 2016/06/02 18:11:30]

Are you adding 'final' simply because this implementation doesn't modify it?

This isn't *logically* final. You could easily add a setListener() method
without breaking anything.

There is a small cost to adding 'final' (not just the extra typing and clutter).
The problem is that 'final' can discourage other developers from removing it,
which is why I prefer not to use it unnecessarily.

But I don't feel strongly, there are some benefits as well, and I'm not aware of
any style guidance.

[Hzj_jie, 2016/06/02 22:20:04]

In my opinion -- it's similar as the 'final' of SelfRemovableParameterRunnable
-- it would be better to 'final' it unless we need to make it not 'final'. For
these fields, finalizing them can help the readers to understand their
references won't be changed during the lifetime of this instance.

 
     /** Handler used for posting tasks to be executed in the future. */
-    private Handler mHandler;
+    private final Handler mHandler;
 
     /** The maximum number of fingers seen in the gesture. */
     private int mPointerCount = 0;
 
     /** The coordinates of the first finger down seen in the gesture. */
     private PointF mInitialPoint;
 
     /**
      * Stores the location of each down MotionEvent (by pointer ID), for detecting motion of any
      * pointer beyond the TouchSlop region.
      */
     private SparseArray<PointF> mInitialPositions = new SparseArray<PointF>();

[Lambros, 2016/06/02 18:11:30]

This could be final as well?

[Hzj_jie, 2016/06/02 22:20:04]

Done.

 
     /**
      * Threshold squared-distance, in pixels, to use for motion-detection. If a finger moves less
      * than this distance, the gesture is still eligible to be a tap event.
      */
     private int mTouchSlopSquare;

[Lambros, 2016/06/02 18:11:30]

And this?

[Hzj_jie, 2016/06/02 22:20:04]

Done.

 
     /** Set to true whenever motion is detected in the gesture, or a long-touch is triggered. */
     private boolean mTapCancelled = false;
 
     // This static inner class holds a WeakReference to the outer object, to avoid triggering the
     // lint HandlerLeak warning.
     private static class EventHandler extends Handler {
         private final WeakReference<TapGestureDetector> mDetector;
 
         public EventHandler(TapGestureDetector detector) {
             mDetector = new WeakReference<TapGestureDetector>(detector);
         }
 
         @Override
         public void handleMessage(Message message) {
             TapGestureDetector detector = mDetector.get();
             if (detector != null) {
+                detector.mTapCancelled = true;
                 detector.mListener.onLongPress(
                         detector.mPointerCount, detector.mInitialPoint.x, detector.mInitialPoint.y);
-                detector.mTapCancelled = true;
+                detector.mInitialPoint = null;
             }
         }
     }
 
     public TapGestureDetector(Context context, OnTapListener listener) {
         mListener = listener;
         mHandler = new EventHandler(this);
         ViewConfiguration config = ViewConfiguration.get(context);
         int touchSlop = config.getScaledTouchSlop();
         mTouchSlopSquare = touchSlop * touchSlop;
     }
 
     /** Analyzes the touch event to determine whether to notify the listener. */
     public boolean onTouchEvent(MotionEvent event) {
         boolean handled = false;
         switch (event.getActionMasked()) {
             case MotionEvent.ACTION_DOWN:
                 reset();
+                trackDownEvent(event);
+
                 // Cause a long-press notification to be triggered after the timeout.
                 mHandler.sendEmptyMessageDelayed(0, ViewConfiguration.getLongPressTimeout());
-                trackDownEvent(event);
                 mPointerCount = 1;
                 break;
 
             case MotionEvent.ACTION_POINTER_DOWN:
                 trackDownEvent(event);
                 mPointerCount = Math.max(mPointerCount, event.getPointerCount());
                 break;
 
             case MotionEvent.ACTION_MOVE:
                 if (!mTapCancelled) {
@@ skipping 86 matching lines @@
         mPointerCount = 0;
         mInitialPositions.clear();
         mTapCancelled = false;
     }
 
     /** Cancels any pending long-touch notifications from the message-queue. */
     private void cancelLongTouchNotification() {
         mHandler.removeMessages(0);
     }
 }