[TTS] Fix Tap mojo race condition and update tests.

This CL implements a workaround for the race condition between the mojo
tap notification and the selection-changed notification.  Now we never enter
the SELECTION_CLEARED internal state unless there was a previous
selection that got cleared.

Before this CL every tap caused a transition to the SELECTION_CLEARED
internal state even when there was no previous selection.  This would usually
happen before the Tap notification, but now that's faster due to mojo so it
sometimes arrives beforehand.  With this change we ignore selection changes
from empty to empty so there's only one notification for most taps. Now we only
enter the SELECTION_CLEARED state when it's really needed (in the tap
near previous selection use-case).

Also does some minor test cleanup and re-enabling disabled tests that
were flaky.

BUG=818897

Change-Id: I269815ad087aabf93332e99a6c7d1208addabe3e
Reviewed-on: https://chromium-review.googlesource.com/1018300
Commit-Queue: Donn Denman <donnd@chromium.org>
Reviewed-by: Theresa <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552439}

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

@@ -4,12 +4,12 @@
 
 package org.chromium.chrome.browser.contextualsearch;
 
+import android.support.annotation.Nullable;
+
 import org.chromium.base.Log;
 import org.chromium.base.VisibleForTesting;
 import org.chromium.chrome.browser.compositor.bottombar.OverlayPanel.StateChangeReason;
 
-import javax.annotation.Nullable;
-
 /**
  * Controls the internal state of the Contextual Search Manager.
  * <p>
@@ -232,11 +232,9 @@ class ContextualSearchInternalStateController {
                 assert reason != null;
                 mStateHandler.hideContextualSearchUi(reason);
                 break;
-
             case SHOWING_LONGPRESS_SEARCH:
                 mStateHandler.showContextualSearchLongpressUi();
                 break;
-
             case WAITING_FOR_POSSIBLE_TAP_NEAR_PREVIOUS:
                 mStateHandler.waitForPossibleTapNearPrevious();
                 break;

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

@@ -236,7 +236,8 @@ public class ContextualSearchSelectionController {
             return;
         }
 
-        if (selection == null || selection.isEmpty()) {
+        if (TextUtils.isEmpty(selection) && !TextUtils.isEmpty(mSelectedText)) {
+            mSelectedText = selection;
             mHandler.handleSelectionCleared();
             // When the user taps on the page it will place the caret in that position, which
             // will trigger a onSelectionChanged event with an empty string.

chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchInternalStateControllerWrapper.java

@@ -15,11 +15,11 @@ import java.util.List;
  */
 class ContextualSearchInternalStateControllerWrapper
         extends ContextualSearchInternalStateController {
-    static final List<InternalState> EXPECTED_TAP_RESOLVE_SEQUENCE = CollectionUtil.newArrayList(
-            InternalState.SELECTION_CLEARED_RECOGNIZED, InternalState.TAP_RECOGNIZED,
-            InternalState.TAP_GESTURE_COMMIT, InternalState.GATHERING_SURROUNDINGS,
-            InternalState.DECIDING_SUPPRESSION, InternalState.START_SHOWING_TAP_UI,
-            InternalState.SHOW_FULL_TAP_UI, InternalState.RESOLVING);
+    static final List<InternalState> EXPECTED_TAP_RESOLVE_SEQUENCE =
+            CollectionUtil.newArrayList(InternalState.TAP_RECOGNIZED,
+                    InternalState.TAP_GESTURE_COMMIT, InternalState.GATHERING_SURROUNDINGS,
+                    InternalState.DECIDING_SUPPRESSION, InternalState.START_SHOWING_TAP_UI,
+                    InternalState.SHOW_FULL_TAP_UI, InternalState.RESOLVING);
     static final List<InternalState> EXPECTED_LONGPRESS_SEQUENCE =
             CollectionUtil.newArrayList(InternalState.LONG_PRESS_RECOGNIZED,
                     InternalState.GATHERING_SURROUNDINGS, InternalState.SHOWING_LONGPRESS_SEARCH);

chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManagerTest.java

@@ -171,7 +171,6 @@ public class ContextualSearchManagerTest {
     private ContextualSearchPolicy mPolicy;
     private ContextualSearchSelectionController mSelectionController;
     private EmbeddedTestServer mTestServer;
-    private boolean mPollInstrumentationThread;
 
     private float mDpToPx;
 
@@ -802,11 +801,10 @@ public class ContextualSearchManagerTest {
 
     /**
      * Waits for the Search Panel to enter the given {@code PanelState} and assert.
-     * Waits on the UI thread unless mPollInstrumentationThread is set.
      * @param state The {@link PanelState} to wait for.
      */
     private void waitForPanelToEnterState(final PanelState state) {
-        final Criteria panelStateCriteria = new Criteria() {
+        CriteriaHelper.pollUiThread(new Criteria() {
             @Override
             public boolean isSatisfied() {
                 if (mPanel == null) return false;
@@ -814,12 +812,7 @@ public class ContextualSearchManagerTest {
                         + "Instead, the current state is " + mPanel.getPanelState() + ".");
                 return mPanel.getPanelState() == state && !mPanel.isHeightAnimationRunning();
             }
-        };
-        if (mPollInstrumentationThread) {
-            CriteriaHelper.pollInstrumentationThread(panelStateCriteria);
-        } else {
-            CriteriaHelper.pollUiThread(panelStateCriteria);
-        }
+        });
     }
 
     /**
@@ -861,24 +854,18 @@ public class ContextualSearchManagerTest {
 
     /**
      * Waits for the selection to be empty.
-     * Waits on the UI thread unless mPollInstrumentationThread is set.
      * Use this method any time a test repeatedly establishes and dissolves a selection to ensure
      * that the selection has been completely dissolved before simulating the next selection event.
      * This is needed because the renderer's notification of a selection going away is async,
      * and a subsequent tap may think there's a current selection until it has been dissolved.
      */
     private void waitForSelectionEmpty() {
-        final Criteria selectionEmptyCriteria = new Criteria("Selection never empty.") {
+        CriteriaHelper.pollUiThread(new Criteria("Selection never empty.") {
             @Override
             public boolean isSatisfied() {
                 return mSelectionController.isSelectionEmpty();
             }
-        };
-        if (mPollInstrumentationThread) {
-            CriteriaHelper.pollInstrumentationThread(selectionEmptyCriteria);
-        } else {
-            CriteriaHelper.pollUiThread(selectionEmptyCriteria);
-        }
+        });
     }
 
     /**
@@ -1571,7 +1558,6 @@ public class ContextualSearchManagerTest {
     @Test
     @SmallTest
     @Feature({"ContextualSearch"})
-    @DisableIf.Build(sdk_is_less_than = Build.VERSION_CODES.LOLLIPOP, message = "crbug.com/818897")
     public void testLongPressGestureFollowedByTapDoesntSelect()
             throws InterruptedException, TimeoutException {
         longPressNode("intelligence");
@@ -2198,10 +2184,8 @@ public class ContextualSearchManagerTest {
      * of selection bounds, so this helps prevent a regression with that.
      */
     @Test
-    @DisabledTest(message = "crbug.com/828780")
     @LargeTest
     @Feature({"ContextualSearch"})
-    @DisableIf.Build(sdk_is_less_than = Build.VERSION_CODES.LOLLIPOP, message = "crbug.com/818897")
     public void testTapALot() throws InterruptedException, TimeoutException {
         for (int i = 0; i < 50; i++) {
             clickToTriggerPrefetch();
@@ -2209,20 +2193,6 @@ public class ContextualSearchManagerTest {
         }
     }
 
-    /**
-     * Tests a bunch of taps in a row, with the variation that we wait on the instrumentation
-     * thread instead of the UI thread for some wait sequences.
-     */
-    @Test
-    @DisabledTest(message = "crbug.com/828780")
-    @LargeTest
-    @Feature({"ContextualSearch"})
-    @DisableIf.Build(sdk_is_less_than = Build.VERSION_CODES.LOLLIPOP, message = "crbug.com/818897")
-    public void testTapALotInstrumentation() throws InterruptedException, TimeoutException {
-        mPollInstrumentationThread = true;
-        testTapALot();
-    }
-
     /**
      * Tests ContextualSearchManager#shouldInterceptNavigation for a case that an external
      * navigation has a user gesture.