Fix a bug where the cache would be out of sync

During recorrection, the cursor position when calling
commitText is not necessarily at the end of the
composing text.
Besides, RichInputConnection assumes the cursor is
always after any composing text. This is not correct,
but in the practice, it seems all code paths work.
We should fix this in the future.

Bug: 13060691
Change-Id: I15f71fff62d36e80cf6e4a022c5e78af634b199d

2 files changed, 54 insertions, 1 deletions

diff --git a/java/src/com/android/inputmethod/latin/RichInputConnection.java b/java/src/com/android/inputmethod/latin/RichInputConnection.java
index eb1899ca2..ebad9bc0d 100644
--- a/java/src/com/android/inputmethod/latin/RichInputConnection.java
+++ b/java/src/com/android/inputmethod/latin/RichInputConnection.java
@@ -230,6 +230,9 @@ public final class RichInputConnection {
     public void finishComposingText() {
         if (DEBUG_BATCH_NESTING) checkBatchEdit();
         if (DEBUG_PREVIOUS_TEXT) checkConsistencyForDebug();
+        // TODO: this is not correct! The cursor is not necessarily after the composing text.
+        // In the practice right now this is only called when input ends so it will be reset so
+        // it works, but it's wrong and should be fixed.
         mCommittedTextBeforeComposingText.append(mComposingText);
         mComposingText.setLength(0);
         if (null != mIC) {
@@ -244,6 +247,9 @@ public final class RichInputConnection {
         if (DEBUG_BATCH_NESTING) checkBatchEdit();
         if (DEBUG_PREVIOUS_TEXT) checkConsistencyForDebug();
         mCommittedTextBeforeComposingText.append(text);
+        // TODO: the following is exceedingly error-prone. Right now when the cursor is in the
+        // middle of the composing word mComposingText only holds the part of the composing text
+        // that is before the cursor, so this actually works, but it's terribly confusing. Fix this.
         mExpectedSelStart += text.length() - mComposingText.length();
         mExpectedSelEnd = mExpectedSelStart;
         mComposingText.setLength(0);
@@ -347,6 +353,9 @@ public final class RichInputConnection {
 
     public void deleteSurroundingText(final int beforeLength, final int afterLength) {
         if (DEBUG_BATCH_NESTING) checkBatchEdit();
+        // TODO: the following is incorrect if the cursor is not immediately after the composition.
+        // Right now we never come here in this case because we reset the composing state before we
+        // come here in this case, but we need to fix this.
         final int remainingChars = mComposingText.length() - beforeLength;
         if (remainingChars >= 0) {
             mComposingText.setLength(remainingChars);
@@ -447,8 +456,12 @@ public final class RichInputConnection {
                 getTextBeforeCursor(Constants.EDITOR_CONTENTS_CACHE_SIZE + (end - start), 0);
         mCommittedTextBeforeComposingText.setLength(0);
         if (!TextUtils.isEmpty(textBeforeCursor)) {
+            // The cursor is not necessarily at the end of the composing text, but we have its
+            // position in mExpectedSelStart and mExpectedSelEnd. In this case we want the start
+            // of the text, so we should use mExpectedSelStart. In other words, the composing
+            // text starts (mExpectedSelStart - start) characters before the end of textBeforeCursor
             final int indexOfStartOfComposingText =
-                    Math.max(textBeforeCursor.length() - (end - start), 0);
+                    Math.max(textBeforeCursor.length() - (mExpectedSelStart - start), 0);
             mComposingText.append(textBeforeCursor.subSequence(indexOfStartOfComposingText,
                     textBeforeCursor.length()));
             mCommittedTextBeforeComposingText.append(
@@ -544,6 +557,9 @@ public final class RichInputConnection {
             final int checkLength = LOOKBACK_CHARACTER_NUM - 1;
             final String reference = prev.length() <= checkLength ? prev.toString()
                     : prev.subSequence(prev.length() - checkLength, prev.length()).toString();
+            // TODO: right now the following works because mComposingText holds the part of the
+            // composing text that is before the cursor, but this is very confusing. We should
+            // fix it.
             final StringBuilder internal = new StringBuilder()
                     .append(mCommittedTextBeforeComposingText).append(mComposingText);
             if (internal.length() > checkLength) {
diff --git a/tests/src/com/android/inputmethod/latin/InputLogicTests.java b/tests/src/com/android/inputmethod/latin/InputLogicTests.java
index 2c23d743f..b2992fca4 100644
--- a/tests/src/com/android/inputmethod/latin/InputLogicTests.java
+++ b/tests/src/com/android/inputmethod/latin/InputLogicTests.java
@@ -385,4 +385,41 @@ public class InputLogicTests extends InputTestsBase {
         final SuggestedWords suggestedWords = mLatinIME.getSuggestedWords();
         assertEquals("no prediction after period", 0, suggestedWords.size());
     }
+
+    public void testPredictionsAfterRecorrection() {
+        final String PREFIX = "A ";
+        final String WORD_TO_TYPE = "Barack";
+        final String FIRST_NON_TYPED_SUGGESTION = "Barrack";
+        final int endOfPrefix = PREFIX.length();
+        final int endOfWord = endOfPrefix + WORD_TO_TYPE.length();
+        final int endOfSuggestion = endOfPrefix + FIRST_NON_TYPED_SUGGESTION.length();
+        final int indexForManualCursor = endOfPrefix + 3; // +3 because it's after "Bar" in "Barack"
+        type(PREFIX);
+        mLatinIME.onUpdateSelection(0, 0, endOfPrefix, endOfPrefix, -1, -1);
+        type(WORD_TO_TYPE);
+        pickSuggestionManually(1, FIRST_NON_TYPED_SUGGESTION);
+        mLatinIME.onUpdateSelection(endOfPrefix, endOfPrefix, endOfSuggestion, endOfSuggestion,
+                -1, -1);
+        runMessages();
+        type(" ");
+        mLatinIME.onUpdateSelection(endOfSuggestion, endOfSuggestion,
+                endOfSuggestion + 1, endOfSuggestion + 1, -1, -1);
+        sleep(DELAY_TO_WAIT_FOR_PREDICTIONS);
+        runMessages();
+        // Simulate a manual cursor move
+        mInputConnection.setSelection(indexForManualCursor, indexForManualCursor);
+        mLatinIME.onUpdateSelection(endOfSuggestion + 1, endOfSuggestion + 1,
+                indexForManualCursor, indexForManualCursor, -1, -1);
+        sleep(DELAY_TO_WAIT_FOR_PREDICTIONS);
+        runMessages();
+        pickSuggestionManually(0, WORD_TO_TYPE);
+        mLatinIME.onUpdateSelection(indexForManualCursor, indexForManualCursor,
+                endOfWord, endOfWord, -1, -1);
+        sleep(DELAY_TO_WAIT_FOR_PREDICTIONS);
+        runMessages();
+        // Test the first prediction is displayed
+        final SuggestedWords suggestedWords = mLatinIME.getSuggestedWords();
+        assertEquals("predictions after recorrection", "Obama",
+                suggestedWords.size() > 0 ? suggestedWords.getWord(0) : null);
+    }
 }