[TTS] Fix ML bug: tap near previous selection, initialization.

Fixes a problem with ML not being applied correctly on a "retap", a tap
near the previous selection.  The problem is that the Search is still in
progress and the Bar still active when the retap is processed.

Adds a new Internal State TAP_GESTURE_COMMIT that allows code to run
during the initial stage of tap gesture processing.  This new state is
used to handle the retap by detecting if the panel is still open.

Loading the Ranker predictor is now moved to an earlier stage of tap
processing (using the new internal state) to allow it to be fully loaded
and ready to predict by the time all the prediction features have been
gathered.

Refactored writing the outcomes to Ranker in
ContextualSearchPanelMetrics so it can be called when appropriate.  Now
logging outcomes and resetting the CSRankerLogger is done whenever the
UI is hidden or a retap is detected.

BUG=783995

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

chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/contextualsearch/ContextualSearchPanelMetrics.java

@@ -120,21 +120,11 @@ public class ContextualSearchPanelMetrics {
             if (mWasContextualCardsDataShown) {
                 ContextualSearchUma.logContextualCardsResultsSeen(mWasSearchContentViewSeen);
             }
-            if (mRankerLogger != null) {
-                mRankerLogger.logOutcome(
-                        ContextualSearchRankerLogger.Feature.OUTCOME_WAS_CARDS_DATA_SHOWN,
-                        mWasContextualCardsDataShown);
-            }
             if (mWasQuickActionShown) {
                 ContextualSearchUma.logQuickActionResultsSeen(mWasSearchContentViewSeen,
                         mQuickActionCategory);
                 ContextualSearchUma.logQuickActionClicked(mWasQuickActionClicked,
                         mQuickActionCategory);
-                if (mRankerLogger != null) {
-                    mRankerLogger.logOutcome(
-                            ContextualSearchRankerLogger.Feature.OUTCOME_WAS_QUICK_ACTION_CLICKED,
-                            mWasQuickActionClicked);
-                }
             }
 
             if (mResultsSeenExperiments != null) {
@@ -142,10 +132,7 @@ public class ContextualSearchPanelMetrics {
                         mWasSearchContentViewSeen, mWasActivatedByTap);
                 mResultsSeenExperiments.logPanelViewedDurations(
                         panelViewDurationMs, mPanelOpenedBeyondPeekDurationMs);
-                if (mRankerLogger != null) {
-                    mResultsSeenExperiments.logRankerTapSuppressionOutcome(mRankerLogger);
-                }
-                mResultsSeenExperiments = null;
+                if (!isChained) mResultsSeenExperiments = null;
             }
             mPanelOpenedBeyondPeekDurationMs = 0;
 
@@ -153,24 +140,10 @@ public class ContextualSearchPanelMetrics {
                 boolean wasAnySuppressionHeuristicSatisfied = mWasAnyHeuristicSatisfiedOnPanelShow;
                 ContextualSearchUma.logAnyTapSuppressionHeuristicSatisfied(
                         mWasSearchContentViewSeen, wasAnySuppressionHeuristicSatisfied);
-                // Update The Ranker logger.
-                if (mRankerLogger != null) {
-                    // Tell Ranker about the primary outcome.
-                    mRankerLogger.logOutcome(
-                            ContextualSearchRankerLogger.Feature.OUTCOME_WAS_PANEL_OPENED,
-                            mWasSearchContentViewSeen);
-                    ContextualSearchUma.logRankerInference(mWasSearchContentViewSeen,
-                            mRankerLogger.getPredictionForTapSuppression());
-                }
                 ContextualSearchUma.logSelectionLengthResultsSeen(
                         mWasSearchContentViewSeen, mSelectionLength);
             }
 
-            // Reset writing to Ranker so whatever interactions occurred are recorded as a
-            // complete record.
-            if (mRankerLogger != null) mRankerLogger.writeLogAndReset();
-            mRankerLogger = null;
-
             // Notifications to Feature Engagement.
             ContextualSearchIPH.doSearchFinishedNotifications(profile, mWasSearchContentViewSeen,
                     mWasActivatedByTap, mWasContextualCardsDataShown);
@@ -350,6 +323,34 @@ public class ContextualSearchPanelMetrics {
         mRankerLogger = rankerLogger;
     }
 
+    /**
+     * Writes all the outcome features to the Ranker Logger and resets the logger.
+     * @param rankerLogger The {@link ContextualSearchRankerLogger} currently being used to measure
+     *                     or suppress the UI by Ranker.
+     */
+    public void writeRankerLoggerOutcomesAndReset() {
+        if (mRankerLogger != null && mWasActivatedByTap) {
+            // Tell Ranker about the primary outcome.
+            mRankerLogger.logOutcome(ContextualSearchRankerLogger.Feature.OUTCOME_WAS_PANEL_OPENED,
+                    mWasSearchContentViewSeen);
+            ContextualSearchUma.logRankerInference(
+                    mWasSearchContentViewSeen, mRankerLogger.getPredictionForTapSuppression());
+            mRankerLogger.logOutcome(
+                    ContextualSearchRankerLogger.Feature.OUTCOME_WAS_CARDS_DATA_SHOWN,
+                    mWasContextualCardsDataShown);
+            if (mWasQuickActionShown) {
+                mRankerLogger.logOutcome(
+                        ContextualSearchRankerLogger.Feature.OUTCOME_WAS_QUICK_ACTION_CLICKED,
+                        mWasQuickActionClicked);
+            }
+            if (mResultsSeenExperiments != null) {
+                mResultsSeenExperiments.logRankerTapSuppressionOutcome(mRankerLogger);
+            }
+            mRankerLogger.writeLogAndReset();
+            mRankerLogger = null;
+        }
+    }
+
     /**
      * Determine whether a new contextual search is starting.
      * @param toState The contextual search state that will be transitioned to.

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

@@ -12,6 +12,7 @@ import java.util.Set;
  */
 public class ContextualSearchHeuristics {
     protected Set<ContextualSearchHeuristic> mHeuristics;
+    private QuickAnswersHeuristic mQuickAnswersHeuristic;
 
     /**
      * Manages a set of heuristics.
@@ -96,4 +97,20 @@ public class ContextualSearchHeuristics {
             heuristic.logRankerTapSuppressionOutcome(logger);
         }
     }
+
+    /**
+     * Sets the {@link QuickAnswersHeuristic} so that it can be accessed externally by
+     * {@link #getQuickAnswersHeuristic}.
+     * @param quickAnswersHeuristic The active {@link QuickAnswersHeuristic}.
+     */
+    public void setQuickAnswersHeuristic(QuickAnswersHeuristic quickAnswersHeuristic) {
+        mQuickAnswersHeuristic = quickAnswersHeuristic;
+    }
+
+    /**
+     * @return The active {@link QuickAnswersHeuristic}.
+     */
+    public QuickAnswersHeuristic getQuickAnswersHeuristic() {
+        return mQuickAnswersHeuristic;
+    }
 }

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

@@ -77,6 +77,11 @@ class ContextualSearchInternalStateController {
          * within the waiting period we'll advance.
          */
         WAITING_FOR_POSSIBLE_TAP_ON_TAP_SELECTION,
+        /** The first state in the Tap-gesture processing pipeline where we know we're processing
+         * a Tap-gesture that won't be converted into a long-press (from tap on tap-selection).  It
+         * may later be suppressed or ignored due to being on an invalid character.
+         */
+        TAP_GESTURE_COMMIT,
         /** Gathers text surrounding the selection. */
         GATHERING_SURROUNDINGS,
         /** Decides if the gesture should trigger the UX or be suppressed. */
@@ -221,14 +226,13 @@ class ContextualSearchInternalStateController {
      *        or known.  Only needed when we enter the IDLE state.
      */
     private void startWorkingOn(InternalState state, @Nullable StateChangeReason reason) {
+        // All transitional states should be listed here, but not start states.
         switch (state) {
             case IDLE:
                 assert reason != null;
                 mStateHandler.hideContextualSearchUi(reason);
                 break;
 
-            case LONG_PRESS_RECOGNIZED:
-                break;
             case SHOWING_LONGPRESS_SEARCH:
                 mStateHandler.showContextualSearchLongpressUi();
                 break;
@@ -236,11 +240,12 @@ class ContextualSearchInternalStateController {
             case WAITING_FOR_POSSIBLE_TAP_NEAR_PREVIOUS:
                 mStateHandler.waitForPossibleTapNearPrevious();
                 break;
-            case TAP_RECOGNIZED:
-                break;
             case WAITING_FOR_POSSIBLE_TAP_ON_TAP_SELECTION:
                 mStateHandler.waitForPossibleTapOnTapSelection();
                 break;
+            case TAP_GESTURE_COMMIT:
+                mStateHandler.tapGestureCommit();
+                break;
             case GATHERING_SURROUNDINGS:
                 mStateHandler.gatherSurroundingText();
                 break;
@@ -304,10 +309,13 @@ class ContextualSearchInternalStateController {
                 if (mPreviousState != null && mPreviousState != InternalState.IDLE) {
                     transitionTo(InternalState.WAITING_FOR_POSSIBLE_TAP_ON_TAP_SELECTION);
                 } else {
-                    transitionTo(InternalState.GATHERING_SURROUNDINGS);
+                    transitionTo(InternalState.TAP_GESTURE_COMMIT);
                 }
                 break;
             case WAITING_FOR_POSSIBLE_TAP_ON_TAP_SELECTION:
+                transitionTo(InternalState.TAP_GESTURE_COMMIT);
+                break;
+            case TAP_GESTURE_COMMIT:
                 transitionTo(InternalState.GATHERING_SURROUNDINGS);
                 break;
             case GATHERING_SURROUNDINGS:

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

@@ -29,6 +29,13 @@ public interface ContextualSearchInternalStateHandler {
      */
     void showContextualSearchLongpressUi();
 
+    /**
+     * The first state in the Tap-gesture processing pipeline where we know we're processing
+     * a Tap-gesture that won't be converted into something else.  Starts the processing pipeline.
+     * @see ContextualSearchInternalStateController.InternalState#TAP_GESTURE_COMMIT
+     */
+    void tapGestureCommit();
+
     /**
      * Gathers text surrounding the current selection, which may have been created by either a Tap
      * or a Long-press gesture.

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

@@ -179,7 +179,6 @@ public class ContextualSearchManager
     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.
@@ -1389,16 +1388,9 @@ public class ContextualSearchManager
 
     @Override
     public void handleMetricsForWouldSuppressTap(ContextualSearchHeuristics tapHeuristics) {
-        mHeuristics = tapHeuristics;
-
-        // TODO(donnd): QuickAnswersHeuristic is getting added to TapSuppressionHeuristics and
-        // and getting considered in TapSuppressionHeuristics#shouldSuppressTap(). It should
-        // be a part of ContextualSearchHeuristics for logging purposes but not for suppression.
-        mQuickAnswersHeuristic = new QuickAnswersHeuristic();
-        mHeuristics.add(mQuickAnswersHeuristic);
-
+        mQuickAnswersHeuristic = tapHeuristics.getQuickAnswersHeuristic();
         if (mSearchPanel != null) {
-            mSearchPanel.getPanelMetrics().setResultsSeenExperiments(mHeuristics);
+            mSearchPanel.getPanelMetrics().setResultsSeenExperiments(tapHeuristics);
             mSearchPanel.getPanelMetrics().setRankerLogger(mTapSuppressionRankerLogger);
         }
     }
@@ -1493,15 +1485,15 @@ public class ContextualSearchManager
                 // Called when the IDLE state has been entered.
                 if (mContext != null) mContext.destroy();
                 mContext = null;
-                // Make sure we write to ranker and reset at the end of every search, even if it
-                // was a suppressed tap or longpress.
-                // TODO(donnd): Find a better place to just make a single call to this (now two).
-                mTapSuppressionRankerLogger.writeLogAndReset();
                 if (mSearchPanel == null) return;
 
+                // Make sure we write to Ranker and reset at the end of every search, even if the
+                // panel was not showing because it was a suppressed tap.
                 if (isSearchPanelShowing()) {
+                    mSearchPanel.getPanelMetrics().writeRankerLoggerOutcomesAndReset();
                     mSearchPanel.closePanel(reason, false);
                 } else {
+                    mTapSuppressionRankerLogger.writeLogAndReset();
                     if (mSelectionController.getSelectionType() == SelectionType.TAP) {
                         mSelectionController.clearSelection();
                     }
@@ -1547,16 +1539,25 @@ public class ContextualSearchManager
                 }
             }
 
+            /** First step where we're committed to processing the current Tap gesture. */
+            @Override
+            public void tapGestureCommit() {
+                mInternalStateController.notifyStartingWorkOn(InternalState.TAP_GESTURE_COMMIT);
+                // We may be processing a chained search (aka a retap -- a tap near a previous tap).
+                // If it's chained we need to log the outcomes and reset, because we won't be hiding
+                // the panel at the end of the previous search (we'll update it to the new Search).
+                if (isSearchPanelShowing()) {
+                    mSearchPanel.getPanelMetrics().writeRankerLoggerOutcomesAndReset();
+                }
+                // Set up the next batch of Ranker logging.
+                mTapSuppressionRankerLogger.setupLoggingForPage(getBaseWebContents());
+                mInternalStateController.notifyFinishedWorkOn(InternalState.TAP_GESTURE_COMMIT);
+            }
+
             /** Starts the process of deciding if we'll suppress the current Tap gesture or not. */
             @Override
             public void decideSuppression() {
                 mInternalStateController.notifyStartingWorkOn(InternalState.DECIDING_SUPPRESSION);
-
-                // Ranker will handle the suppression, but our legacy implementation uses
-                // TapSuppressionHeuristics (run from the ContextualSearchSelectionController).
-                // Usage includes tap-far-from-previous suppression.
-                mTapSuppressionRankerLogger.setupLoggingForPage(getBaseWebContents());
-
                 // TODO(donnd): Move handleShouldSuppressTap out of the Selection Controller.
                 mSelectionController.handleShouldSuppressTap(mContext, mTapSuppressionRankerLogger);
             }

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

@@ -81,6 +81,7 @@ public class ContextualSearchRankerLoggerImpl implements ContextualSearchRankerL
 
     // A for-testing copy of all the features to log setup so that it will survive a {@link #reset}.
     private Map<Feature, Object> mFeaturesLoggedForTesting;
+    private Map<Feature, Object> mOutcomesLoggedForTesting;
 
     /**
      * Constructs a Ranker Logger and associated native implementation to write Contextual Search
@@ -180,7 +181,7 @@ public class ContextualSearchRankerLoggerImpl implements ContextualSearchRankerL
                 for (Map.Entry<Feature, Object> entry : mFeaturesToLog.entrySet()) {
                     logObject(entry.getKey(), entry.getValue());
                 }
-                mFeaturesLoggedForTesting = mFeaturesToLog;
+                mOutcomesLoggedForTesting = mFeaturesToLog;
             }
             nativeWriteLogAndReset(mNativePointer);
         }
@@ -243,9 +244,9 @@ public class ContextualSearchRankerLoggerImpl implements ContextualSearchRankerL
     }
 
     /**
-     * Gets the current set of features to log or that have been logged.  Should only be used for
-     * testing purposes!
-     * @return The current set of features to log or that have been logged, or {@code null}.
+     * Gets the current set of features that have been logged.  Should only be used for testing
+     * purposes!
+     * @return The current set of features that have been logged, or {@code null}.
      */
     @VisibleForTesting
     @Nullable
@@ -253,6 +254,17 @@ public class ContextualSearchRankerLoggerImpl implements ContextualSearchRankerL
         return mFeaturesLoggedForTesting;
     }
 
+    /**
+     * Gets the current set of outcomes that have been logged.  Should only be used for
+     * testing purposes!
+     * @return The current set of outcomes that have been logged, or {@code null}.
+     */
+    @VisibleForTesting
+    @Nullable
+    Map<Feature, Object> getOutcomesLogged() {
+        return mOutcomesLoggedForTesting;
+    }
+
     // ============================================================================================
     // Native methods.
     // ============================================================================================

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

@@ -61,7 +61,6 @@ public class ContextualSearchSelectionController {
     private boolean mWasTapGestureDetected;
     // Reflects whether the last tap was valid and whether we still have a tap-based selection.
     private ContextualSearchTapState mLastTapState;
-    private boolean mIsWaitingForInvalidTapDetection;
     private boolean mShouldHandleSelectionModification;
     // Whether the selection was automatically expanded due to an adjustment (e.g. Resolve).
     private boolean mDidExpandSelection;
@@ -498,14 +497,6 @@ public class ContextualSearchSelectionController {
     // Misc.
     // ============================================================================================
 
-    /**
-     * @return whether a tap gesture has been detected, for testing.
-     */
-    @VisibleForTesting
-    boolean wasAnyTapGestureDetected() {
-        return mIsWaitingForInvalidTapDetection;
-    }
-
     /**
      * @return whether selection is empty, for testing.
      */

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

@@ -40,6 +40,10 @@ public class TapSuppressionHeuristics extends ContextualSearchHeuristics {
         mHeuristics.add(new BarOverlapTapSuppression(selectionController, y));
         // Second Tap ML Override.
         mHeuristics.add(new SecondTapMlOverride(selectionController, previousTapState, x, y));
+        // Quick Answer that appears in the Caption via the JS API.
+        QuickAnswersHeuristic quickAnswersHeuristic = new QuickAnswersHeuristic();
+        setQuickAnswersHeuristic(quickAnswersHeuristic);
+        mHeuristics.add(quickAnswersHeuristic);
     }
 
     /**

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

@@ -624,6 +624,9 @@ class ContextualSearchFakeServer
                 QuickActionCategory.NONE));
         registerFakeTapSearch(new FakeTapSearch("german", false, 200, "Deutsche", "Deutsche",
                 "alternate-term", "", false, 0, 0, "de", "", "", "", QuickActionCategory.NONE));
+        registerFakeTapSearch(new FakeTapSearch("intelligence", false, 200, "Intelligence",
+                "Intelligence", "alternate-term", "", false, 0, 0, "", "", "", "",
+                QuickActionCategory.NONE));
 
         // Register a resolving search of "States" that expands to "United States".
         registerFakeSlowResolveSearch(new FakeSlowResolveSearch("states", false, 200, "States",

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.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.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_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

@@ -862,20 +862,6 @@ public class ContextualSearchManagerTest {
         Assert.assertFalse(didChangeState);
     }
 
-    /**
-     * Waits for the manager to finish processing a gesture.
-     * Tells the manager that a gesture has started, and then waits for it to complete.
-     */
-    private void waitForGestureProcessing() {
-        CriteriaHelper.pollInstrumentationThread(
-                new Criteria("Gesture processing did not complete.") {
-                    @Override
-                    public boolean isSatisfied() {
-                        return !mSelectionController.wasAnyTapGestureDetected();
-                    }
-                }, TEST_TIMEOUT, DEFAULT_POLLING_INTERVAL);
-    }
-
     /**
      * Shorthand for a common sequence:
      * 1) Waits for gesture processing,
@@ -884,7 +870,6 @@ public class ContextualSearchManagerTest {
      * @throws InterruptedException
      */
     private void waitForGestureToClosePanelAndAssertNoSelection() throws InterruptedException {
-        waitForGestureProcessing();
         waitForPanelToClose();
         assertPanelClosedOrUndefined();
         Assert.assertTrue(TextUtils.isEmpty(getSelectedText()));
@@ -999,6 +984,8 @@ public class ContextualSearchManagerTest {
      */
     private void tapBasePageToClosePanel() throws InterruptedException {
         // TODO(pedrosimonetti): This is not reliable. Find a better approach.
+        // This taps on the panel in an area that will be selected if the "intelligence" node has
+        // been tap-selected, and that will cause it to be long-press selected.
         // We use the far right side (x == 0.9f) to prevent simulating a tap on top of an
         // existing long-press selection (the pins are a tap target). This might not work on RTL.
         // We are using y == 0.35f because otherwise it will fail for long press cases.
@@ -1151,30 +1138,31 @@ public class ContextualSearchManagerTest {
         assertWaitForSelectActionBarVisible(true);
     }
 
-    /** @return The value of the given logged feature, or {@code null} if not logged. */
-    private Object loggedToRanker(ContextualSearchRankerLogger.Feature feature) {
+    /** Gets the Ranker Logger and asserts if we can't. **/
+    private ContextualSearchRankerLoggerImpl getRankerLogger() {
         ContextualSearchRankerLoggerImpl rankerLogger =
                 (ContextualSearchRankerLoggerImpl) mManager.getRankerLogger();
         Assert.assertNotNull(rankerLogger);
-        return rankerLogger.getFeaturesLogged().get(feature);
+        return rankerLogger;
     }
 
-    /** Asserts that the given feature has been logged to Ranker. **/
-    private void assertLoggedToRanker(ContextualSearchRankerLogger.Feature feature) {
-        Assert.assertNotNull(loggedToRanker(feature));
+    /** @return The value of the given logged feature, or {@code null} if not logged. */
+    private Object loggedToRanker(ContextualSearchRankerLogger.Feature feature) {
+        return getRankerLogger().getFeaturesLogged().get(feature);
     }
 
     /** Asserts that all the expected features have been logged to Ranker. **/
     private void assertLoggedAllExpectedFeaturesToRanker() {
         for (ContextualSearchRankerLogger.Feature feature : EXPECTED_RANKER_FEATURES) {
-            assertLoggedToRanker(feature);
+            Assert.assertNotNull(loggedToRanker(feature));
         }
     }
 
     /** Asserts that all the expected outcomes have been logged to Ranker. **/
     private void assertLoggedAllExpectedOutcomesToRanker() {
         for (ContextualSearchRankerLogger.Feature feature : EXPECTED_RANKER_OUTCOMES) {
-            assertLoggedToRanker(feature);
+            Assert.assertNotNull("Expected this outcome to be logged: " + feature,
+                    getRankerLogger().getOutcomesLogged().get(feature));
         }
     }
 
@@ -1253,19 +1241,17 @@ public class ContextualSearchManagerTest {
     @SmallTest
     @Feature({"ContextualSearch"})
     public void testTap() throws InterruptedException, TimeoutException {
-        clickWordNode("intelligence");
-
-        Assert.assertEquals("Intelligence", mFakeServer.getSearchTermRequested());
-        fakeResponse(false, 200, "Intelligence", "display-text", "alternate-term", false);
-        assertContainsParameters("Intelligence", "alternate-term");
-        waitForPanelToPeek();
+        simulateTapSearch("intelligence");
         assertLoadedLowPriorityUrl();
 
         assertLoggedAllExpectedFeaturesToRanker();
         Assert.assertEquals(
                 true, loggedToRanker(ContextualSearchRankerLogger.Feature.IS_LONG_WORD));
         // The panel must be closed for outcomes to be logged.
-        closePanel();
+        // Close the panel by clicking far away in order to make sure the outcomes get logged by
+        // the hideContextualSearchUi call to writeRankerLoggerOutcomesAndReset.
+        clickWordNode("states-far");
+        waitForPanelToClose();
         assertLoggedAllExpectedOutcomesToRanker();
     }
 
@@ -1458,7 +1444,6 @@ public class ContextualSearchManagerTest {
         waitForPanelToPeek();
         assertLoadedNoUrl();  // No load after long-press until opening panel.
         clickNode("question-mark");
-        waitForGestureProcessing();
         waitForPanelToCloseAndSelectionEmpty();
         Assert.assertTrue(TextUtils.isEmpty(getSelectedText()));
         assertLoadedNoUrl();
@@ -1491,7 +1476,6 @@ public class ContextualSearchManagerTest {
     public void testTapGestureOnSpecialCharacterDoesntSelect()
             throws InterruptedException, TimeoutException {
         clickNode("question-mark");
-        waitForGestureProcessing();
         Assert.assertNull(getSelectedText());
         assertPanelClosedOrUndefined();
         assertLoadedNoUrl();
@@ -1560,13 +1544,12 @@ public class ContextualSearchManagerTest {
         waitForPanelToClose();
         Assert.assertNull(getSelectedText());
         clickNode("states-far");
-        waitForGestureProcessing();
         waitForPanelToPeek();
         Assert.assertEquals("States", getSelectedText());
     }
 
     /**
-     * Tests that sequential Tap gestures nearby keep selecting.
+     * Tests a "retap" -- that sequential Tap gestures nearby keep selecting.
      */
     @Test
     @SmallTest
@@ -1575,6 +1558,7 @@ public class ContextualSearchManagerTest {
         clickWordNode("states");
         Assert.assertEquals("States", getSelectedText());
         waitForPanelToPeek();
+        assertLoggedAllExpectedFeaturesToRanker();
         // Avoid issues with double-tap detection by ensuring sequential taps
         // aren't treated as such. Double-tapping can also select words much as
         // longpress, in turn showing the pins and preventing contextual tap
@@ -1586,9 +1570,12 @@ public class ContextualSearchManagerTest {
         // for the selection to change.
         clickNode("states-near");
         waitForSelectionToBe("StatesNear");
+        assertLoggedAllExpectedOutcomesToRanker();
+        assertLoggedAllExpectedFeaturesToRanker();
         Thread.sleep(ViewConfiguration.getDoubleTapTimeout());
         clickNode("states");
         waitForSelectionToBe("States");
+        assertLoggedAllExpectedOutcomesToRanker();
     }
 
     /**

chrome/android/junit/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchInternalStateTest.java

@@ -45,6 +45,11 @@ public class ContextualSearchInternalStateTest {
             mDidShow = true;
         }
 
+        @Override
+        public void tapGestureCommit() {
+            stubForWorkOnState(InternalState.TAP_GESTURE_COMMIT);
+        }
+
         @Override
         public void gatherSurroundingText() {
             stubForWorkOnState(InternalState.GATHERING_SURROUNDINGS);