[TTS] Add an ACK message to SelectWordAroundCaret.

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

Index: chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java b/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java
index 9bfd26bb04bbd700d3852ad078480fdf5310565d..19da667209dfac9c97f44b5b355188ff20bf05e8 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java
@@ -168,6 +168,10 @@ public class ContextualSearchManager implements ContextualSearchManagementDelega
     private ContextualSearchHeuristics mHeuristics;
     private QuickAnswersHeuristic mQuickAnswersHeuristic;
 
+    // Counter for how many times we've called SelectWordAroundCaret without an ACK returned.
+    // TODO(donnd): replace with a more systematic approach using the InternalStateController.
+    private int mSelectWordAroundCaretCounter;
+
     /**
      * The delegate that is responsible for promoting a {@link ContentViewCore} to a {@link Tab}
      * when necessary.
@@ -1213,7 +1217,21 @@ public class ContextualSearchManager implements ContextualSearchManagementDelega
         return false;
     }
 
-    // TODO(donnd): add handling of an ACK to selectWordAroundCaret (crbug.com/435778 has details).
+    @Override
+    public void selectWordAroundCaretAck(boolean didSelect, int startAdjust, int endAdjust) {
+        if (mSelectWordAroundCaretCounter > 0) mSelectWordAroundCaretCounter--;
+        if (mSelectWordAroundCaretCounter > 0
+                || !mInternalStateController.isStillWorkingOn(InternalState.START_SHOWING_TAP_UI)) {

[aelias_OOO_until_Jul13, 2017/05/05 21:13:22]

Can this line be deleted now?  It seems the scenario is fully covered by the
counter.

[Donn Denman, 2017/05/05 21:32:40]

No, I don't think so.  The scenario is a first tap starts the
SelectWordAroundCaret.  When the ACK calls in to here we may already have gotten
a second tap and started processing it, yet not gotten to the point of calling
SelectWordAroundCaret.  We want to abort in that case.

[aelias_OOO_until_Jul13, 2017/05/05 21:47:01]

I don't get it, you increment the counter right next to setting your state
machine to InternalState.START_SHOWING_TAP_UI.  How could we possibly be in
START_SHOWING_TAP_UI state with a nonzero counter?

[Donn Denman, 2017/05/05 22:00:36]

We simply might be in some other state at the time the renderer returns with
this ACK.  So we need to check the current state too.

+            return;
+        }
+
+        if (didSelect) {
+            assert mContext != null;
+            mContext.onSelectionAdjusted(startAdjust, endAdjust);
+            showSelectionAsSearchInBar(mSelectionController.getSelectedText());
+        }
+        mInternalStateController.notifyFinishedWorkOn(InternalState.START_SHOWING_TAP_UI);
+    }
 
     /**
      * @return Whether the display is in a full-screen video overlay mode.
@@ -1298,9 +1316,6 @@ public class ContextualSearchManager implements ContextualSearchManagementDelega
         if (!selection.isEmpty()) {
             ContextualSearchUma.logSelectionIsValid(selectionValid);
 
-            // Update the context so it knows the selection has changed.
-            if (mContext != null) mContext.updateContextFromSelection(selection);
-
             if (selectionValid && mSearchPanel != null) {
                 mSearchPanel.updateBasePageSelectionYPx(y);
                 if (!mSearchPanel.isShowing()) {
@@ -1308,19 +1323,7 @@ public class ContextualSearchManager implements ContextualSearchManagementDelega
                 }
                 showSelectionAsSearchInBar(selection);
 
-                // TODO(donnd): remove this complication when we get an ACK message from
-                // selectWordAroundCaret (see crbug.com/435778).
-                if (type == SelectionType.TAP) {
-                    // Make sure we have a context -- we'll need one to show the UI.
-                    if (mContext == null) {
-                        // Some unknown failure happened, hide the UI.
-                        hideContextualSearch(StateChangeReason.UNKNOWN);
-                        return;
-                    }
-
-                    mInternalStateController.notifyFinishedWorkOn(
-                            InternalState.START_SHOWING_TAP_UI);
-                } else {
+                if (type == SelectionType.LONG_PRESS) {
                     mInternalStateController.enter(InternalState.LONG_PRESS_RECOGNIZED);
                 }
             } else {
@@ -1440,6 +1443,7 @@ public class ContextualSearchManager implements ContextualSearchManagementDelega
                 if (baseWebContents != null && mPolicy.isTapSupported()) {
                     mInternalStateController.notifyStartingWorkOn(
                             InternalState.START_SHOWING_TAP_UI);
+                    mSelectWordAroundCaretCounter++;
                     baseWebContents.selectWordAroundCaret();
                     // Let the policy know that a valid tap gesture has been received.
                     mPolicy.registerTap();