Explicitly tell Smart Select whether or not to suggest

content/public/android/java/src/org/chromium/content/browser/SelectionPopupController.java

Index: content/public/android/java/src/org/chromium/content/browser/SelectionPopupController.java
diff --git a/content/public/android/java/src/org/chromium/content/browser/SelectionPopupController.java b/content/public/android/java/src/org/chromium/content/browser/SelectionPopupController.java
index 6493e4abb98f9cef067f2f883888a7c320db3d72..3067b1131e5ca7f979d0a186050f300ecbb2e7fa 100644
--- a/content/public/android/java/src/org/chromium/content/browser/SelectionPopupController.java
+++ b/content/public/android/java/src/org/chromium/content/browser/SelectionPopupController.java
@@ -927,7 +927,8 @@ public class SelectionPopupController extends ActionModeCallbackHelper {
                 mSelectionRect.set(left, top, right, bottom);
                 mHasSelection = true;
                 mUnselectAllOnDismiss = true;
-                if (mSelectionClient == null || !mSelectionClient.sendsSelectionPopupUpdates()) {
+                if (mSelectionClient == null

[Tima Vaisburd, 2017/05/05 01:57:06]

Nice, I think another option to think of would be to completely separate the
mSelectionClient that is used for the smart text from the one used for
contextual search. If we had two clients instead of one we could have called
ContextualSearchClient (probably renamed do SmartTextSomething) here directly
and not pollute interface with the irrelevant methods.

If you keep the current structure though I'd prefer enums, so it would look like
requestSelectionPopupUpdates(SUGGEST_AND_CLASIFY).

[boliu, 2017/05/05 02:11:50]

true /* suggest */ is good enough for me. I mean do you think you'll add more
things to the enum? (also side note, avoid enums on android, use @IntDef
instead)

nit: invert the if here. positive conditions are slightly easier to read imo..

[Tima Vaisburd, 2017/05/05 16:54:41]

It is absolutely unrelated to this particular CL but the right audience, so I'd
like to say that now I think I did the wrong design of the smart selection.
Pedro was right when proposing to initiate IPC from Blink.
Given the latest requirements (a tap within the "smartly selected" area should
work differently, maybe animation) maybe it would be better to
  long press -> Blink finds one word -> IPC for a new range ->
  Blink selected and remembers the augmented range

And the classification remains where it is right now. Then it would be
SUGGEST, CLASSIFY operations, and may be no enums at all.

[amaralp, 2017/05/05 21:18:31]

Do you know if Android has already implemented the animation?
I'll keep the booleans for now.

+                        || !mSelectionClient.requestSelectionPopupUpdates(true)) {
                     showActionModeOrClearOnFailure();
                 } else {
                     // Rely on |mSelectionClient| sending a classification request and the request
@@ -949,6 +950,7 @@ public class SelectionPopupController extends ActionModeCallbackHelper {
                 mHasSelection = false;
                 mUnselectAllOnDismiss = false;
                 mSelectionRect.setEmpty();
+                if (mSelectionClient != null) mSelectionClient.cancelAllRequests();
                 finishActionMode();
                 break;
 
@@ -957,7 +959,8 @@ public class SelectionPopupController extends ActionModeCallbackHelper {
                 break;
 
             case SelectionEventType.SELECTION_HANDLE_DRAG_STOPPED:
-                if (mSelectionClient == null || !mSelectionClient.sendsSelectionPopupUpdates()) {
+                if (mSelectionClient == null
+                        || !mSelectionClient.requestSelectionPopupUpdates(false)) {
                     hideActionMode(false);
                 }
                 // Otherwise rely on |mSelectionClient| sending a classification request and the