[TTS] Add an ACK message to SelectWordAroundCaret.

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

 // Copyright 2015 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.chrome.browser.contextualsearch;
 
 import android.annotation.SuppressLint;
 import android.os.Handler;
 import android.text.TextUtils;
 import android.view.View;
@@ skipping 150 matching lines @@
     private ContextualSearchRequest mSearchRequest;
     private ContextualSearchRequest mLastSearchRequestLoaded;
 
     /** Whether the Accessibility Mode is enabled. */
     private boolean mIsAccessibilityModeEnabled;
 
     /** Tap Experiments and other variable behavior. */
     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.
      */
     public interface ContextualSearchTabPromotionDelegate {
         /**
          * Called when {@code searchContentViewCore} should be promoted to a {@link Tab}.
          * @param searchUrl The Search URL to be promoted.
          */
         void createContextualSearchTab(String searchUrl);
@@ skipping 1025 matching lines @@
         if (!isOverlayVideoMode()) {
             mSelectionController.handleShowUnhandledTapUIIfNeeded(x, y);
         }
     }
 
     @Override
     public boolean sendsSelectionPopupUpdates() {
         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.
      */
     private boolean isOverlayVideoMode() {
         return mActivity.getFullscreenManager() != null
                 && mActivity.getFullscreenManager().isOverlayVideoMode();
     }
 
     // ============================================================================================
@@ skipping 64 matching lines @@
      * to expand the selection to a whole word.
      */
     @Override
     public void handleSelection(String selection, boolean selectionValid, SelectionType type,
             float x, float y) {
         if (mIsAccessibilityModeEnabled) return;
 
         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()) {
                     mSearchPanel.getPanelMetrics().onSelectionEstablished(selection);
                 }
                 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 {
                 hideContextualSearch(StateChangeReason.INVALID_SELECTION);
             }
         }
     }
 
     @Override
     public void handleSelectionDismissal() {
@@ skipping 99 matching lines @@
             @Override
             public void startShowingTapUi() {
                 WebContents baseWebContents = getBaseWebContents();
                 // TODO(donnd): Call isTapSupported earlier so we don't waste time gathering
                 // surrounding text and deciding suppression when unsupported, or remove the whole
                 // idea of unsupported taps in favor of deciding suppression better.
                 // Details in crbug.com/715297.
                 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();
                 } else {
                     mInternalStateController.reset(StateChangeReason.UNKNOWN);
                 }
             }
 
             /**
              * Waits for possible Tap gesture that's near enough to the previous tap to be
@@ skipping 123 matching lines @@
     private native void nativeStartSearchTermResolutionRequest(long nativeContextualSearchManager,
             ContextualSearchContext contextualSearchContext, WebContents baseWebContents);
     protected native void nativeGatherSurroundingText(long nativeContextualSearchManager,
             ContextualSearchContext contextualSearchContext, WebContents baseWebContents);
     private native void nativeEnableContextualSearchJsApiForOverlay(
             long nativeContextualSearchManager, WebContents overlayWebContents);
     // Don't call these directly, instead call the private methods that cache the results.
     private native String nativeGetTargetLanguage(long nativeContextualSearchManager);
     private native String nativeGetAcceptLanguages(long nativeContextualSearchManager);
 }