Consider newCursorPosition better when composing or commiting text

commitText isn't considering the case of newCursorPosition != 1. We don't
have native code to handle it, but since

commitText = setComposingText + finishComposingText,

we can replace it as such.

Also, as for setComposingText, there is an error in BaseInputConnection
(Android framework), so we need to correct the selection after calling
super.setComposingText().

BUG=570920

Review URL: https://codereview.chromium.org/1576303002

Cr-Commit-Position: refs/heads/master@{#368809}

3 files changed, 112 insertions, 11 deletions

diff --git a/content/public/android/java/src/org/chromium/content/browser/input/AdapterInputConnection.java b/content/public/android/java/src/org/chromium/content/browser/input/AdapterInputConnection.java
index 2376c47..761592b 100644
--- a/content/public/android/java/src/org/chromium/content/browser/input/AdapterInputConnection.java
+++ b/content/public/android/java/src/org/chromium/content/browser/input/AdapterInputConnection.java
@@ -254,9 +254,28 @@ public class AdapterInputConnection extends BaseInputConnection {
     public boolean setComposingText(CharSequence text, int newCursorPosition) {
         Log.d(TAG, "setComposingText [%s] [%d]", text, newCursorPosition);
         mPendingAccent = 0;
+
+        Editable editable = getEditableInternal();
+        int selectionStartAfterReplacement = Selection.getSelectionStart(editable);
+        int selectionEndAfterReplacement = Selection.getSelectionStart(editable) + text.length();
+
         super.setComposingText(text, newCursorPosition);
+
+        // Due to an error in BaseInputConnection (b/21476564), the new cursor position
+        // may not be correct when newCursorPosition != 1. We fix it here:
+        if (newCursorPosition > 1) {
+            int newPos = selectionEndAfterReplacement - 1 + newCursorPosition;
+            int len = editable.length();
+            if (newPos > len) newPos = len;
+            Selection.setSelection(editable, newPos);
+        } else if (newCursorPosition <= 0) {
+            int newPos = selectionStartAfterReplacement + newCursorPosition;
+            if (newPos < 0) newPos = 0;
+            Selection.setSelection(editable, newPos);
+        }
+
         updateSelectionIfRequired();
-        return mImeAdapter.sendCompositionToNative(text, newCursorPosition, false);
+        return mImeAdapter.setComposingText(text, newCursorPosition);
     }
 
     /**
@@ -266,9 +285,18 @@ public class AdapterInputConnection extends BaseInputConnection {
     public boolean commitText(CharSequence text, int newCursorPosition) {
         Log.d(TAG, "commitText [%s] [%d]", text, newCursorPosition);
         mPendingAccent = 0;
-        super.commitText(text, newCursorPosition);
-        updateSelectionIfRequired();
-        return mImeAdapter.sendCompositionToNative(text, newCursorPosition, text.length() > 0);
+        if (newCursorPosition == 1) {
+            super.commitText(text, newCursorPosition);
+            updateSelectionIfRequired();
+            return mImeAdapter.commitText(text);
+        }
+
+        // This takes slightly longer, but commitText with newCursorPosition != 1 isn't
+        // implemented in the native side.
+        beginBatchEdit();
+        boolean result = setComposingText(text, newCursorPosition) && finishComposingText();
+        endBatchEdit();
+        return result;
     }
 
     /**
diff --git a/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java b/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java
index bdfd587..61aceec 100644
--- a/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java
+++ b/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java
@@ -450,7 +450,7 @@ public class ImeAdapter {
                 flags));
     }
 
-    boolean sendCompositionToNative(CharSequence text, int newCursorPosition, boolean isCommit) {
+    boolean commitText(CharSequence text) {
         if (mNativeImeAdapterAndroid == 0) return false;
         mViewEmbedder.onImeEvent();
 
@@ -458,12 +458,22 @@ public class ImeAdapter {
         nativeSendSyntheticKeyEvent(mNativeImeAdapterAndroid, WebInputEventType.RawKeyDown,
                 timestampMs, COMPOSITION_KEY_CODE, 0, 0);
 
-        if (isCommit) {
-            nativeCommitText(mNativeImeAdapterAndroid, text.toString());
-        } else {
-            nativeSetComposingText(
-                    mNativeImeAdapterAndroid, text, text.toString(), newCursorPosition);
-        }
+        nativeCommitText(mNativeImeAdapterAndroid, text.toString());
+
+        nativeSendSyntheticKeyEvent(mNativeImeAdapterAndroid, WebInputEventType.KeyUp, timestampMs,
+                COMPOSITION_KEY_CODE, 0, 0);
+        return true;
+    }
+
+    boolean setComposingText(CharSequence text, int newCursorPosition) {
+        if (mNativeImeAdapterAndroid == 0) return false;
+        mViewEmbedder.onImeEvent();
+
+        long timestampMs = SystemClock.uptimeMillis();
+        nativeSendSyntheticKeyEvent(mNativeImeAdapterAndroid, WebInputEventType.RawKeyDown,
+                timestampMs, COMPOSITION_KEY_CODE, 0, 0);
+
+        nativeSetComposingText(mNativeImeAdapterAndroid, text, text.toString(), newCursorPosition);
 
         nativeSendSyntheticKeyEvent(mNativeImeAdapterAndroid, WebInputEventType.KeyUp,
                 timestampMs, COMPOSITION_KEY_CODE, 0, 0);
diff --git a/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java b/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java
index f9800d3..bd6b973 100644
--- a/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java
+++ b/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java
@@ -991,6 +991,69 @@ public class ImeTest extends ContentShellTestBase {
         assertNoFurtherStateUpdate(1);
     }
 
+    @MediumTest
+    @Feature({"TextInput"})
+    public void testCommitTextWithDifferentCursorPositions() throws Throwable {
+        // Out of bound, treat this as commitText("ab", 1);
+        commitText("ab", 2);
+        assertEquals("ab", getTextBeforeCursor(10, 0));
+        assertEquals("", getTextAfterCursor(10, 0));
+        waitAndVerifyStatesAndCalls(1, "ab", 2, 2, -1, -1);
+
+        commitText("cd", 0);
+        assertEquals("ab", getTextBeforeCursor(10, 0));
+        assertEquals("cd", getTextAfterCursor(10, 0));
+        waitAndVerifyStatesAndCalls(3, "abcd", 2, 2, -1, -1);
+
+        commitText("ef", -1);
+        assertEquals("a", getTextBeforeCursor(10, 0));
+        assertEquals("befcd", getTextAfterCursor(10, 0));
+        waitAndVerifyStatesAndCalls(5, "abefcd", 1, 1, -1, -1);
+    }
+
+    @MediumTest
+    @Feature({"TextInput"})
+    public void testSetComposingTextWithDifferentCursorPositions() throws Throwable {
+        // Out of bound, treat this as commitText("ab", 1);
+        setComposingText("ab", 2);
+        finishComposingText();
+        assertEquals("ab", getTextBeforeCursor(10, 0));
+        assertEquals("", getTextAfterCursor(10, 0));
+        waitAndVerifyStatesAndCalls(1, "ab", 2, 2, -1, -1);
+
+        setComposingText("cd", 0);
+        finishComposingText();
+        assertEquals("ab", getTextBeforeCursor(10, 0));
+        assertEquals("cd", getTextAfterCursor(10, 0));
+        waitAndVerifyStatesAndCalls(3, "abcd", 2, 2, -1, -1);
+
+        setComposingText("ef", -1);
+        finishComposingText();
+        assertEquals("a", getTextBeforeCursor(10, 0));
+        assertEquals("befcd", getTextAfterCursor(10, 0));
+        waitAndVerifyStatesAndCalls(5, "abefcd", 1, 1, -1, -1);
+    }
+
+    private CharSequence getTextBeforeCursor(final int length, final int flags)
+            throws ExecutionException {
+        return ThreadUtils.runOnUiThreadBlocking(new Callable<CharSequence>() {
+            @Override
+            public CharSequence call() {
+                return mConnection.getTextBeforeCursor(length, flags);
+            }
+        });
+    }
+
+    private CharSequence getTextAfterCursor(final int length, final int flags)
+            throws ExecutionException {
+        return ThreadUtils.runOnUiThreadBlocking(new Callable<CharSequence>() {
+            @Override
+            public CharSequence call() {
+                return mConnection.getTextAfterCursor(length, flags);
+            }
+        });
+    }
+
     private void performGo(TestCallbackHelperContainer testCallbackHelperContainer)
             throws Throwable {
         final AdapterInputConnection inputConnection = mConnection;