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

Unified Diff: content/public/android/java/src/org/chromium/content/browser/SelectionPopupController.java

Issue 2740103006: Implement SmartText selection. (Closed)
Patch Set: Addressed some comments Created 3 years, 9 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
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 c45ec5aca4667aeb45a3f0e9dd621e122ef2d4ee..e6cb5c873c06b1995e6909a34ffd3923f7f8dfe0 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
@@ -54,7 +54,7 @@ import java.util.List;
*/
@TargetApi(Build.VERSION_CODES.M)
public class SelectionPopupController extends ActionModeCallbackHelper {
- private static final String TAG = "cr.SelectionPopCtlr"; // 20 char limit
+ private static final String TAG = "SelectionPopupCtlr"; // 20 char limit
/**
* Android Intent size limitations prevent sending over a megabyte of data. Limit
@@ -70,6 +70,12 @@ public class SelectionPopupController extends ActionModeCallbackHelper {
// most such trailing, async delays.
private static final int SHOW_DELAY_MS = 300;
+ // Display operation for ActionMode (show or invalidate) that is delayed because
+ // the selected text needs to be classified before. See comment to mPostponedDisplayOp.
+ private static final int POSTPONED_NONE = 0;
+ private static final int POSTPONED_SHOW = 1;
+ private static final int POSTPONED_INVALIDATE = 2;
+
private final Context mContext;
private final WindowAndroid mWindowAndroid;
private final WebContents mWebContents;
@@ -115,6 +121,67 @@ public class SelectionPopupController extends ActionModeCallbackHelper {
// The client that processes textual selection, or null if none exists.
private SelectionClient mSelectionClient;
+ // The classificaton result of the selected text or null if ContextSelectionProvider
+ // could not classify the selection.
boliu 2017/03/20 20:27:26 what's the state of this if there's no selection?
Tima Vaisburd 2017/03/21 02:07:02 Null. I rewrote as "The classificaton result of th
+ private ContextSelectionProvider.Result mClassificationResult;
+
+ // Postponed display operation for ActionMode.
+ // TODO(timav): explain better.
boliu 2017/03/20 20:27:26 later when?
Tima Vaisburd 2017/03/21 02:07:02 Added an explanation.
+ private int mPostponedDisplayOp;
+
+ // The callback object that delivers result from SelectionClient.
+ private ContextSelectionProvider.ResultCallback mSelectonCallback =
amaralp 2017/03/21 02:34:14 Why does this need to be a member of the class? Wh
Tima Vaisburd 2017/03/21 20:50:12 No good reason.
+ new ContextSelectionProvider.ResultCallback() {
+ @Override
+ public void onClassified(ContextSelectionProvider.Result result) {
+ Log.v(TAG, "onClassified");
boliu 2017/03/20 20:27:26 .
Tima Vaisburd 2017/03/21 02:07:02 No more.
+
+ // If the selection does not exist any more, discard |result|.
+ if (!mHasSelection) {
+ mPostponedDisplayOp = POSTPONED_NONE;
+ return;
+ }
+
+ // Update the selection range if needed.
+ if (!(result.startAdjust == 0 && result.endAdjust == 0)) {
+ // This call causes SELECTION_HANDLES_MOVED event
+ mWebContents.adjustSelectionByCharacterOffset(
+ result.startAdjust, result.endAdjust);
+ }
+
+ final boolean hadPriorResult = mClassificationResult != null;
+ mClassificationResult = result.hasNamedAction() ? result : null;
+
+ // For simplicity always update the menu if we need to show assist item.
+ // Also we need to update the menu if the assist item is going to disappear.
+ if (mClassificationResult != null || hadPriorResult) mNeedsPrepare = true;
+
+ switch (mPostponedDisplayOp) {
+ case POSTPONED_NONE:
+ break;
+
+ case POSTPONED_SHOW:
+ // TODO(timav): if the |result| contained the selection adjustment and
amaralp 2017/03/21 02:34:14 One solution for the flickering could be to have t
Tima Vaisburd 2017/03/21 20:50:12 I like that idea although I receive a feedback tha
+ // we called adjustSelectionByCharacterOffset() above, there is flicker
+ // if SELECTION_HANDLES_MOVED comes later and invalidates selection
+ // rectangle. Consider postponing till SELECTION_HANDLES_MOVED or
+ // introduce delay.
+ showActionMode();
+ mPostponedDisplayOp = POSTPONED_NONE;
+ break;
+
+ case POSTPONED_INVALIDATE:
+ invalidateActionMode();
+ mPostponedDisplayOp = POSTPONED_NONE;
+ break;
+
+ default:
+ assert false : "Invalid postponed display operation type.";
+ break;
+ }
+ }
+ };
+
/**
* Create {@link SelectionPopupController} instance.
* @param context Context for action mode.
@@ -145,6 +212,8 @@ public class SelectionPopupController extends ActionModeCallbackHelper {
hideActionModeTemporarily(hideDuration);
}
};
+
+ mSelectionClient = ContextSelectionClient.create(mSelectonCallback, window, webContents);
}
/**
@@ -189,15 +258,18 @@ public class SelectionPopupController extends ActionModeCallbackHelper {
*
* <p>Action mode in floating mode is tried first, and then falls back to
* a normal one.
- * @return {@code true} if the action mode started successfully or is already on.
+ * <p> If the action mode cannot be created the selection is cleared.
*/
- public boolean showActionMode() {
- if (isEmpty()) return false;
+ public void showActionMode() {
boliu 2017/03/20 20:27:26 maybe call this showActionModeOrClearOnFailure
Ted C 2017/03/20 20:35:50 Ah, that is why the other code changed.
Tima Vaisburd 2017/03/21 02:07:02 Done.
+ if (isEmpty()) {
+ clearSelection();
+ return;
+ }
// Just refreshes the view if it is already showing.
if (isActionModeValid()) {
invalidateActionMode();
- return true;
+ return;
}
if (mView.getParent() != null) {
@@ -213,7 +285,7 @@ public class SelectionPopupController extends ActionModeCallbackHelper {
mActionMode = actionMode;
}
mUnselectAllOnDismiss = true;
- return isActionModeValid();
+ if (!isActionModeValid()) clearSelection();
}
@TargetApi(Build.VERSION_CODES.M)
@@ -295,6 +367,7 @@ public class SelectionPopupController extends ActionModeCallbackHelper {
*/
@Override
public void finishActionMode() {
+ mPostponedDisplayOp = POSTPONED_NONE;
if (isActionModeValid()) {
mActionMode.finish();
@@ -314,6 +387,7 @@ public class SelectionPopupController extends ActionModeCallbackHelper {
assert canHideActionMode();
mHidden = false;
mView.removeCallbacks(mRepeatingHideRunnable);
+ hideActionModeTemporarily(SHOW_DELAY_MS);
mPendingInvalidateContentRect = false;
}
@@ -328,7 +402,7 @@ public class SelectionPopupController extends ActionModeCallbackHelper {
/**
* @see ActionMode#invalidateContentRect()
*/
- public void invalidateContentRect() {
+ private void invalidateContentRect() {
if (supportsFloatingActionMode()) {
if (mHidden) {
mPendingInvalidateContentRect = true;
@@ -431,6 +505,7 @@ public class SelectionPopupController extends ActionModeCallbackHelper {
private void createActionMenu(ActionMode mode, Menu menu) {
mNeedsPrepare = false;
initializeMenu(mContext, mode, menu);
+ updateContextDependantMenuItem(menu);
if (!isSelectionEditable() || !canPaste()) {
menu.removeItem(R.id.select_action_menu_paste);
@@ -473,6 +548,18 @@ public class SelectionPopupController extends ActionModeCallbackHelper {
return clipMgr.hasPrimaryClip();
}
+ private void updateContextDependantMenuItem(Menu menu) {
+ Log.v(TAG, "updateContextDependantItem");
boliu 2017/03/20 20:27:26 .
Tima Vaisburd 2017/03/21 02:07:02 Removed. Also s/dependant/dependent/.
+ menu.removeItem(R.id.select_action_menu_context_dep);
+
+ if (mClassificationResult == null) return;
+
+ Log.v(TAG, "updateContextDependantMenuItem: adding item " + mClassificationResult.label);
+ menu.add(Menu.NONE, R.id.select_action_menu_context_dep, 1, mClassificationResult.label)
+ .setIcon(mClassificationResult.icon)
+ .setShowAsAction(MenuItem.SHOW_AS_ACTION_ALWAYS);
+ }
+
/**
* Intialize the menu items for processing text, if there is any.
*/
@@ -514,7 +601,10 @@ public class SelectionPopupController extends ActionModeCallbackHelper {
int id = item.getItemId();
int groupId = item.getGroupId();
- if (id == R.id.select_action_menu_select_all) {
+ if (id == R.id.select_action_menu_context_dep) {
+ doContextDependentAction();
+ mode.finish();
+ } else if (id == R.id.select_action_menu_select_all) {
selectAll();
} else if (id == R.id.select_action_menu_cut) {
cut();
@@ -573,6 +663,30 @@ public class SelectionPopupController extends ActionModeCallbackHelper {
}
/**
+ * Perform an action that depends on the semantics of the selected text.
+ */
+ @VisibleForTesting
+ void doContextDependentAction() {
+ if (mClassificationResult == null) return;
+
+ assert mClassificationResult.onClickListener != null
+ || mClassificationResult.intent != null;
+
+ if (mClassificationResult.onClickListener != null) {
+ mClassificationResult.onClickListener.onClick(mView);
+ return;
+ }
+
+ if (mClassificationResult.intent != null) {
+ Context context = mWindowAndroid.getContext().get();
+ if (context == null) return;
+
+ context.startActivity(mClassificationResult.intent);
+ return;
+ }
+ }
+
+ /**
* Perform a select all action.
*/
@VisibleForTesting
@@ -750,7 +864,7 @@ public class SelectionPopupController extends ActionModeCallbackHelper {
void restoreSelectionPopupsIfNecessary() {
if (mHasSelection && !isActionModeValid()) {
- if (!showActionMode()) clearSelection();
+ showActionMode();
}
}
@@ -767,7 +881,11 @@ public class SelectionPopupController extends ActionModeCallbackHelper {
mSelectionRect.set(left, top, right, bottom);
mHasSelection = true;
mUnselectAllOnDismiss = true;
- if (!showActionMode()) clearSelection();
+ if (mSelectionClient != null && mSelectionClient.sendsMenuUpdates()) {
+ mPostponedDisplayOp = POSTPONED_SHOW;
+ } else {
+ showActionMode();
+ }
break;
case SelectionEventType.SELECTION_HANDLES_MOVED:
@@ -787,7 +905,11 @@ public class SelectionPopupController extends ActionModeCallbackHelper {
break;
case SelectionEventType.SELECTION_HANDLE_DRAG_STOPPED:
- hideActionMode(false);
+ if (mSelectionClient != null && mSelectionClient.sendsMenuUpdates()) {
+ mPostponedDisplayOp = POSTPONED_INVALIDATE;
+ } else {
+ hideActionMode(false);
+ }
break;
case SelectionEventType.INSERTION_HANDLE_SHOWN:
@@ -848,6 +970,7 @@ public class SelectionPopupController extends ActionModeCallbackHelper {
* end if applicable.
*/
void clearSelection() {
+ mClassificationResult = null;
if (mWebContents == null || isEmpty()) return;
mWebContents.collapseSelection();
}

Powered by Google App Engine
This is Rietveld 408576698