Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1371)

Unified Diff: chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchSelectionController.java

Issue 2211353002: [TTS] Gather surrounding text on Tap before any UX. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Removed unused code and consolidated two functions, updated comments. Created 4 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/action/ResolvedSearchAction.java » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchSelectionController.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchSelectionController.java b/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchSelectionController.java
index 6234318ec73fae0d255163bce462adb6a5ece342..350327a261537d359fb9eea84ac0c6651c2c63ec 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchSelectionController.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchSelectionController.java
@@ -10,10 +10,15 @@ 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;
@@ -22,8 +27,7 @@ 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.
*/
@@ -79,10 +83,19 @@ public class ContextualSearchSelectionController {
// 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) {
@@ -113,6 +126,8 @@ public class ContextualSearchSelectionController {
// handles (have a long-press selection) and the tap was consumed.
if (!(consumed && mSelectionType == SelectionType.LONG_PRESS)) {
scheduleInvalidTapNotification();
+
+ if (mHasIdentifiedUnhandledTap) createSearchAction();
}
}
}
@@ -326,6 +341,7 @@ public class ContextualSearchSelectionController {
*/
private void handleSelection(String selection, SelectionType type) {
mShouldHandleSelectionModification = true;
+ destroySearchAction();
boolean isValidSelection = validateSelectionSuppression(selection);
mHandler.handleSelection(selection, isValidSelection, type, mX, mY);
}
@@ -338,6 +354,8 @@ public class ContextualSearchSelectionController {
mLastTapState = null;
mLastScrollTimeNs = 0;
mIsContextMenuShown = false;
+ mHasIdentifiedUnhandledTap = false;
+ mTapTimeNanoseconds = 0;
}
/**
@@ -360,6 +378,8 @@ public class ContextualSearchSelectionController {
/**
* Handles an unhandled tap gesture.
+ * @param x The x coordinate.
+ * @param y The y coordinate.
*/
void handleShowUnhandledTapUIIfNeeded(int x, int y) {
mWasTapGestureDetected = false;
@@ -367,37 +387,16 @@ public class ContextualSearchSelectionController {
// 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;
@@ -406,9 +405,96 @@ public class ContextualSearchSelectionController {
}
/**
+ * Processes a {@link SearchAction}.
+ * 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() {
+ destroySearchAction();
+ 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;
}
« no previous file with comments | « no previous file | chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/action/ResolvedSearchAction.java » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698