[TTS] Gather surrounding text on Tap before any UX.

chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchSelectionController.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.os.Handler;
 
 import org.chromium.base.VisibleForTesting;
 import org.chromium.chrome.browser.ChromeActivity;
 import org.chromium.chrome.browser.compositor.bottombar.OverlayPanel;
 import org.chromium.chrome.browser.contextualsearch.ContextualSearchBlacklist.BlacklistReason;
+import org.chromium.chrome.browser.contextualsearch.action.ResolvedSearchAction;
+import org.chromium.chrome.browser.contextualsearch.action.SearchAction;
+import org.chromium.chrome.browser.contextualsearch.action.SearchActionListener;
+import org.chromium.chrome.browser.contextualsearch.gesture.SearchGestureHost;
 import org.chromium.chrome.browser.preferences.ChromePreferenceManager;
 import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.content.browser.ContentViewCore;
 import org.chromium.content_public.browser.GestureStateListener;
+import org.chromium.content_public.browser.WebContents;
 import org.chromium.ui.touch_selection.SelectionEventType;
 
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
 /**
  * Controls selection gesture interaction for Contextual Search.
  */
-public class ContextualSearchSelectionController {
-
+public class ContextualSearchSelectionController implements SearchGestureHost {
     /**
      * The type of selection made by the user.
      */
     public enum SelectionType {
         UNDETERMINED,
         TAP,
         LONG_PRESS
     }
 
     // The number of milliseconds to wait for a selection change after a tap before considering
@@ skipping 35 matching lines @@
     private boolean mShouldHandleSelectionModification;
     private boolean mDidExpandSelection;
 
     // Position of the selection.
     private float mX;
     private float mY;
 
     // The time of the most last scroll activity, or 0 if none.
     private long mLastScrollTimeNs;
 
+    // When the last tap gesture happened.
+    private long mTapTimeNanoseconds;
+
     // Tracks whether a Context Menu has just been shown and the UX has been dismissed.
     // The selection may be unreliable until the next reset.  See crbug.com/628436.
     private boolean mIsContextMenuShown;
 
+    // Set to true when we have identified that a pending tap has not been handled by Blink.
+    private boolean mHasIdentifiedUnhandledTap;
+
+    // The current Search action.
+    private SearchAction mSearchAction;
+
     private class ContextualSearchGestureStateListener extends GestureStateListener {
         @Override
         public void onScrollStarted(int scrollOffsetY, int scrollExtentY) {
             mHandler.handleScroll();
         }
 
         @Override
         public void onScrollEnded(int scrollOffsetY, int scrollExtentY) {
             mLastScrollTimeNs = System.nanoTime();
         }
@@ skipping 10 matching lines @@
         // notification in this case.
         // See crbug.com/444114.
         @Override
         public void onSingleTap(boolean consumed, int x, int y) {
             // We may be notified that a tap has happened even when the system consumed the event.
             // This is being used to support tapping on an existing selection to show the selection
             // handles.  We should process this tap unless we have already shown the selection
             // handles (have a long-press selection) and the tap was consumed.
             if (!(consumed && mSelectionType == SelectionType.LONG_PRESS)) {
                 scheduleInvalidTapNotification();
+
+                if (mHasIdentifiedUnhandledTap) createSearchAction();
             }
         }
     }
 
     /**
      * Constructs a new Selection controller for the given activity.  Callbacks will be issued
      * through the given selection handler.
      * @param activity The {@link ChromeActivity} to control.
      * @param handler The handler for callbacks.
      */
@@ skipping 205 matching lines @@
     }
 
     /**
      * Resets all internal state of this class, including the tap state.
      */
     private void resetAllStates() {
         resetSelectionStates();
         mLastTapState = null;
         mLastScrollTimeNs = 0;
         mIsContextMenuShown = false;
+        mHasIdentifiedUnhandledTap = false;
+        mTapTimeNanoseconds = 0;
     }
 
     /**
      * Resets all of the internal state of this class that handles the selection.
      */
     private void resetSelectionStates() {
         mSelectionType = SelectionType.UNDETERMINED;
         mSelectedText = null;
 
         mWasTapGestureDetected = false;
     }
 
     /**
      * Should be called when a new Tab is selected.
      * Resets all of the internal state of this class.
      */
     void onTabSelected() {
         resetAllStates();
     }
 
     /**
      * Handles an unhandled tap gesture.
+     * @param x The x coordinate.
+     * @param y The y coordinate.
      */
     void handleShowUnhandledTapUIIfNeeded(int x, int y) {
         mWasTapGestureDetected = false;
         // TODO(donnd): shouldn't we check == TAP here instead of LONG_PRESS?
         // TODO(donnd): refactor to avoid needing a new handler API method as suggested by Pedro.
         if (mSelectionType != SelectionType.LONG_PRESS) {
             mWasTapGestureDetected = true;
-            long tapTimeNanoseconds = System.nanoTime();
-            // TODO(donnd): add a policy method to get adjusted tap count.
-            ChromePreferenceManager prefs = ChromePreferenceManager.getInstance(mActivity);
-            int adjustedTapsSinceOpen = prefs.getContextualSearchTapCount()
-                    - prefs.getContextualSearchTapQuickAnswerCount();
-            TapSuppressionHeuristics tapHeuristics =
-                    new TapSuppressionHeuristics(this, mLastTapState, x, y, adjustedTapsSinceOpen);
-            // TODO(donnd): Move to be called when the panel closes to work with states that change.
-            tapHeuristics.logConditionState();
-            // Tell the manager what it needs in order to log metrics on whether the tap would have
-            // been suppressed if each of the heuristics were satisfied.
-            mHandler.handleMetricsForWouldSuppressTap(tapHeuristics);
+            mTapTimeNanoseconds = System.nanoTime();
+
+            // NOTE(donnd): We first acknowledge that a unhandled tap was identified, but
+            // we don't do anything now. Instead we'll wait for the onSingleTap() event to fire,
+            // and only do something when an unhandled tap was identified. onSingleTap() will
+            // always get fired, as opposed to showUnhandledTapUIIfNeeded().
+            mHasIdentifiedUnhandledTap = true;
+
             mX = x;
             mY = y;
-            boolean shouldSuppressTap = tapHeuristics.shouldSuppressTap();
-            if (shouldSuppressTap) {
-                mHandler.handleSuppressedTap();
-            } else {
-                // TODO(donnd): Find a better way to determine that a navigation will be triggered
-                // by the tap, or merge with other time-consuming actions like gathering surrounding
-                // text or detecting page mutations.
-                new Handler().postDelayed(new Runnable() {
-                    @Override
-                    public void run() {
-                        mHandler.handleValidTap();
-                    }
-                }, TAP_NAVIGATION_DETECTION_DELAY);
-            }
-            // Remember the tap state for subsequent tap evaluation.
-            mLastTapState =
-                    new ContextualSearchTapState(x, y, tapTimeNanoseconds, shouldSuppressTap);
         } else {
             // Long press; reset last tap state.
             mLastTapState = null;
             mHandler.handleInvalidTap();
         }
     }
 
     /**
+     * Processes a {@link SearchAction}.
+     * This should be called when the associated {@code SearchAction} has built its context (by
+     * gathering surrounding text if needed, etc).

[Theresa, 2016/09/01 16:49:36]

nit: ..., etc) but before a selection has been established?

[Donn Denman, 2016/09/02 16:40:43]

Done.  Now reads: "This should be called when the associated {@code
SearchAction} has built its context (by gathering surrounding text if needed,
etc) but before showing any UX."

+     * @param searchAction The {@link SearchAction} for this Tap gesture.
+     * @param x The x coordinate.
+     * @param y The y coordinate.
+     */
+    void processSearchAction(SearchAction searchAction, int x, int y) {
+        // TODO(donnd): consider using the supplied searchAction, or remove if used from native!
+        // TODO(donnd): add a policy method to get adjusted tap count.
+        ChromePreferenceManager prefs = ChromePreferenceManager.getInstance(mActivity);
+        int adjustedTapsSinceOpen = prefs.getContextualSearchTapCount()
+                - prefs.getContextualSearchTapQuickAnswerCount();
+        TapSuppressionHeuristics tapHeuristics =
+                new TapSuppressionHeuristics(this, mLastTapState, x, y, adjustedTapsSinceOpen);
+        // TODO(donnd): Move to be called when the panel closes to work with states that change.
+        tapHeuristics.logConditionState();
+        // Tell the manager what it needs in order to log metrics on whether the tap would have
+        // been suppressed if each of the heuristics were satisfied.
+        mHandler.handleMetricsForWouldSuppressTap(tapHeuristics);
+
+        boolean shouldSuppressTap = tapHeuristics.shouldSuppressTap();
+        if (shouldSuppressTap) {
+            mHandler.handleSuppressedTap();
+        } else {
+            // TODO(donnd): Find a better way to determine that a navigation will be triggered
+            // by the tap, or merge with other time-consuming actions like gathering surrounding
+            // text or detecting page mutations.
+            new Handler().postDelayed(new Runnable() {
+                @Override
+                public void run() {
+                    mHandler.handleValidTap();
+                }
+            }, TAP_NAVIGATION_DETECTION_DELAY);
+        }
+        if (mTapTimeNanoseconds == 0) throw new RuntimeException("Tap time not set!");
+        // Remember the tap state for subsequent tap evaluation.
+        mLastTapState = new ContextualSearchTapState(x, y, mTapTimeNanoseconds, shouldSuppressTap);
+    }
+
+    /**
+     * Creates the current {@link SearchAction}.
+     */
+    void createSearchAction() {

[pedro (no code reviews), 2016/08/31 22:38:22]

The SearchAction should also be destroyed when a long press gesture is detected,
even if it's not using the SearchAction class.

Also, it should be destroyed when a invalid tap/gesture is detected, I would
think.

[Donn Denman, 2016/09/02 16:40:43]

Done: Added a destroySearchAction to handleSelectionChanged so anytime we get a
new selection we'll destroy the search action (for the previous selection).

+        if (mSearchAction != null) destroySearchAction();

[Theresa, 2016/09/01 16:49:36]

nit: destroySearchAction() checks if mSearchAction == null, so we don't need to
check here

[Donn Denman, 2016/09/02 16:40:43]

Done.

+        mSearchAction = new ResolvedSearchAction(new SearchActionListener() {
+
+            @Override
+            protected void onContextReady(SearchAction action) {
+                processSearchAction(action, (int) mX, (int) mY);
+            }
+        }, this);
+
+        mSearchAction.extractContext();
+    }
+
+    /**
+     * Destroys the current {@link SearchAction}.
+     */
+    private void destroySearchAction() {
+        if (mSearchAction == null) return;
+
+        mSearchAction.destroyAction();
+        mSearchAction = null;
+    }
+
+    // ============================================================================================
+    // SearchGestureHost
+    // ============================================================================================
+
+    @Override
+    public WebContents getTabWebContents() {
+        Tab currentTab = mActivity.getActivityTab();
+        return currentTab != null ? currentTab.getWebContents() : null;
+    }
+
+    @Override
+    public void dismissGesture() {
+        destroySearchAction();
+    }
+
+    // ============================================================================================
+    // Utilities
+    // ============================================================================================
+
+    /**
      * @return The Base Page's {@link ContentViewCore}, or {@code null} if there is no current tab.
      */
     ContentViewCore getBaseContentView() {
+        // TODO(donnd): switch to using WebContents over ContentViewCore.
         Tab currentTab = mActivity.getActivityTab();
         return currentTab != null ? currentTab.getContentViewCore() : null;
     }
 
     /**
      * Expands the current selection by the specified amounts.
      * @param selectionStartAdjust The start offset adjustment of the selection to use to highlight
      *                             the search term.
      * @param selectionEndAdjust The end offset adjustment of the selection to use to highlight
      *                           the search term.
@@ skipping 175 matching lines @@
         // Starts are inclusive and ends are non-inclusive for both GSAContext & matcher.
         while (matcher.find()) {
             if (startOffset >= matcher.start() && endOffset <= matcher.end()) {
                 return true;
             }
         }
 
         return false;
     }
 }