[TTS] Basic Tap Suppression functionality.

chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/TapFarFromPreviousSuppression.java

Index: chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/TapFarFromPreviousSuppression.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/TapFarFromPreviousSuppression.java b/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/TapFarFromPreviousSuppression.java
index 5596c4b1827a737f43be774faa54c0791e82b668..d0adeb68e27a456fe08e687baba985ce94906ec3 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/TapFarFromPreviousSuppression.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/TapFarFromPreviousSuppression.java
@@ -4,12 +4,15 @@
 
 package org.chromium.chrome.browser.contextualsearch;
 
+import android.text.TextUtils;
+
 class TapFarFromPreviousSuppression extends ContextualSearchHeuristic {
     private static final double RETAP_DISTANCE_SQUARED_DP = Math.pow(75, 2);
 
     private final ContextualSearchTapState mPreviousTapState;
     private final float mPxToDp;
     private final boolean mShouldHandleTap;
+    private final boolean mIsSelectionEmpty;
 
     /**
      * Constructs a heuristic to determine if the current Tap should be suppressed because it is
@@ -19,10 +22,11 @@ class TapFarFromPreviousSuppression extends ContextualSearchHeuristic {
      * @param y The y coordinate of the tap gesture.
      */
     TapFarFromPreviousSuppression(ContextualSearchSelectionController controller,
-                                  ContextualSearchTapState previousTapState, int x, int y) {
+            ContextualSearchTapState previousTapState, int x, int y) {
         mPxToDp = controller.getPxToDp();
         mPreviousTapState = previousTapState;
         mShouldHandleTap = shouldHandleTap(x, y);
+        mIsSelectionEmpty = TextUtils.isEmpty(controller.getSelectedText());
     }
 
     @Override
@@ -36,7 +40,7 @@ class TapFarFromPreviousSuppression extends ContextualSearchHeuristic {
     private boolean shouldHandleTap(int x, int y) {
         if (mPreviousTapState == null) return true;
 
-        return mPreviousTapState.wasSuppressed() || wasTapCloseToPreviousTap(x, y);
+        return mIsSelectionEmpty || wasTapCloseToPreviousTap(x, y);

[twellington, 2016/06/27 18:44:01]

Shouldn't we return false (don't handle tap) if the selection is empty?

[pedro (no code reviews), 2016/06/27 22:14:29]

Tap recognition happens before any selection is issued. Empty selection here
seems to be used as a proxy for determining when the previous tap was not
valid/handled, given that a (unhandled) tap will cause the caret to be
positioned where the used tap. And the caret is essentially a empty selection
(selection with length zero).

In either case, I think it's worth putting a comment here because it might not
immediately obvious what this is doing.

[Donn Denman, 2016/06/28 23:39:08]

Decided to go back to the other logic (details in my other reply).  Sorry for
the confusion, I thought this would be simpler but there are issues.

[Donn Denman, 2016/06/28 23:39:08]

Updated the comments to make this more clear.
Using the selection should be clearer for the code, and better aligned with the
purpose of this subfeature (as described in the new class comment).  However
this seems to be another case where the selection is not shown but the
controller thinks it is.  Reverted this class to the way it was before, modulo
comments and name changes for accessors.

     }
 
     /**