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

This is a reland of 5bd4f41d
Original change's description:
> [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}

Bug: 783995
Change-Id: I3f224ecc53cd18d84d1a8e97316245b817d5f4db
Reviewed-on: https://chromium-review.googlesource.com/841244Reviewed-by: Donn Denman <donnd@chromium.org>
Reviewed-by: Yusuf Ozuysal <yusufo@chromium.org>
Commit-Queue: Donn Denman <donnd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527076}

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.
@@ -1398,16 +1397,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);
         }
     }
@@ -1502,15 +1494,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();
                     }
@@ -1556,16 +1548,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);