From ac76463306b9b09faac803380a69be386e40fbee Mon Sep 17 00:00:00 2001 From: Jean Chalard Date: Tue, 22 Oct 2013 10:51:11 +0900 Subject: [PATCH] Add a sequence number to SuggestedWords. This allows testing for suggestion freshness in an asynchronous suggestions world. Bug: 11301597 Change-Id: Ic76cd17568598d8534aec81e037f9e37f52eb6b4 --- .../android/inputmethod/latin/LatinIME.java | 124 +++++++++++------- .../android/inputmethod/latin/Suggest.java | 18 ++- .../inputmethod/latin/SuggestedWords.java | 14 ++ 3 files changed, 100 insertions(+), 56 deletions(-) diff --git a/java/src/com/android/inputmethod/latin/LatinIME.java b/java/src/com/android/inputmethod/latin/LatinIME.java index 1bce9af9e..e43ec8c92 100644 --- a/java/src/com/android/inputmethod/latin/LatinIME.java +++ b/java/src/com/android/inputmethod/latin/LatinIME.java @@ -1818,12 +1818,15 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen @Override public boolean handleMessage(final Message msg) { + // TODO: straighten message passing - we don't need two kinds of messages calling + // each other. switch (msg.what) { case MSG_UPDATE_GESTURE_PREVIEW_AND_SUGGESTION_STRIP: - updateBatchInput((InputPointers)msg.obj); + updateBatchInput((InputPointers)msg.obj, msg.arg2 /* sequenceNumber */); break; case MSG_GET_SUGGESTED_WORDS: - mLatinIme.getSuggestedWords(msg.arg1, (OnGetSuggestedWordsCallback) msg.obj); + mLatinIme.getSuggestedWords(msg.arg1 /* sessionId */, + msg.arg2 /* sequenceNumber */, (OnGetSuggestedWordsCallback) msg.obj); break; } return true; @@ -1840,14 +1843,15 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen } // Run in the Handler thread. - private void updateBatchInput(final InputPointers batchPointers) { + private void updateBatchInput(final InputPointers batchPointers, final int sequenceNumber) { synchronized (mLock) { if (!mInBatchInput) { // Batch input has ended or canceled while the message was being delivered. return; } - getSuggestedWordsGestureLocked(batchPointers, new OnGetSuggestedWordsCallback() { + getSuggestedWordsGestureLocked(batchPointers, sequenceNumber, + new OnGetSuggestedWordsCallback() { @Override public void onGetSuggestedWords(final SuggestedWords suggestedWords) { mLatinIme.mHandler.showGesturePreviewAndSuggestionStrip( @@ -1858,12 +1862,13 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen } // Run in the UI thread. - public void onUpdateBatchInput(final InputPointers batchPointers) { + public void onUpdateBatchInput(final InputPointers batchPointers, + final int sequenceNumber) { if (mHandler.hasMessages(MSG_UPDATE_GESTURE_PREVIEW_AND_SUGGESTION_STRIP)) { return; } - mHandler.obtainMessage(MSG_UPDATE_GESTURE_PREVIEW_AND_SUGGESTION_STRIP, batchPointers) - .sendToTarget(); + mHandler.obtainMessage(MSG_UPDATE_GESTURE_PREVIEW_AND_SUGGESTION_STRIP, 0 /* arg1 */, + sequenceNumber /* arg2 */, batchPointers /* obj */).sendToTarget(); } public void onCancelBatchInput() { @@ -1877,7 +1882,8 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen // Run in the UI thread. public void onEndBatchInput(final InputPointers batchPointers) { synchronized(mLock) { - getSuggestedWordsGestureLocked(batchPointers, new OnGetSuggestedWordsCallback() { + getSuggestedWordsGestureLocked(batchPointers, SuggestedWords.NOT_A_SEQUENCE_NUMBER, + new OnGetSuggestedWordsCallback() { @Override public void onGetSuggestedWords(final SuggestedWords suggestedWords) { mInBatchInput = false; @@ -1892,10 +1898,10 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen // {@link LatinIME#getSuggestedWords(int)} method calls with same session id have to // be synchronized. private void getSuggestedWordsGestureLocked(final InputPointers batchPointers, - final OnGetSuggestedWordsCallback callback) { + final int sequenceNumber, final OnGetSuggestedWordsCallback callback) { mLatinIme.mWordComposer.setBatchInputPointers(batchPointers); mLatinIme.getSuggestedWordsOrOlderSuggestionsAsync(Suggest.SESSION_GESTURE, - new OnGetSuggestedWordsCallback() { + sequenceNumber, new OnGetSuggestedWordsCallback() { @Override public void onGetSuggestedWords(SuggestedWords suggestedWords) { final int suggestionCount = suggestedWords.size(); @@ -1910,9 +1916,10 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen }); } - public void getSuggestedWords(final int sessionId, + public void getSuggestedWords(final int sessionId, final int sequenceNumber, final OnGetSuggestedWordsCallback callback) { - mHandler.obtainMessage(MSG_GET_SUGGESTED_WORDS, sessionId, 0, callback).sendToTarget(); + mHandler.obtainMessage(MSG_GET_SUGGESTED_WORDS, sessionId, sequenceNumber, callback) + .sendToTarget(); } private void onDestroy() { @@ -1933,11 +1940,28 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen } } + /* The sequence number member is only used in onUpdateBatchInput. It is increased each time + * auto-commit happens. The reason we need this is, when auto-commit happens we trim the + * input pointers that are held in a singleton, and to know how much to trim we rely on the + * results of the suggestion process that is held in mSuggestedWords. + * However, the suggestion process is asynchronous, and sometimes we may enter the + * onUpdateBatchInput method twice without having recomputed suggestions yet, or having + * received new suggestions generated from not-yet-trimmed input pointers. In this case, the + * mIndexOfTouchPointOfSecondWords member will be out of date, and we must not use it lest we + * remove an unrelated number of pointers (possibly even more than are left in the input + * pointers, leading to a crash). + * To avoid that, we increase the sequence number each time we auto-commit and trim the + * input pointers, and we do not use any suggested words that have been generated with an + * earlier sequence number. + */ + private int mAutoCommitSequenceNumber = 1; @Override public void onUpdateBatchInput(final InputPointers batchPointers) { if (mSettings.getCurrent().mPhraseGestureEnabled) { final SuggestedWordInfo candidate = mSuggestedWords.getAutoCommitCandidate(); - if (null != candidate) { + // If these suggested words have been generated with out of date input pointers, then + // we skip auto-commit (see comments above on the mSequenceNumber member). + if (null != candidate && mSuggestedWords.mSequenceNumber >= mAutoCommitSequenceNumber) { if (candidate.mSourceDict.shouldAutoCommit(candidate)) { final String[] commitParts = candidate.mWord.split(" ", 2); batchPointers.shift(candidate.mIndexOfTouchPointOfSecondWord); @@ -1946,10 +1970,11 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen mSpaceState = SPACE_STATE_PHANTOM; mKeyboardSwitcher.updateShiftState(); mWordComposer.setCapitalizedModeAtStartComposingTime(getActualCapsMode()); + ++mAutoCommitSequenceNumber; } } } - mInputUpdater.onUpdateBatchInput(batchPointers); + mInputUpdater.onUpdateBatchInput(batchPointers, mAutoCommitSequenceNumber); } // This method must run in UI Thread. @@ -2477,7 +2502,7 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen final AsyncResultHolder holder = new AsyncResultHolder(); getSuggestedWordsOrOlderSuggestionsAsync(Suggest.SESSION_TYPING, - new OnGetSuggestedWordsCallback() { + SuggestedWords.NOT_A_SEQUENCE_NUMBER, new OnGetSuggestedWordsCallback() { @Override public void onGetSuggestedWords(final SuggestedWords suggestedWords) { holder.set(suggestedWords); @@ -2492,7 +2517,7 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen } } - private void getSuggestedWords(final int sessionId, + private void getSuggestedWords(final int sessionId, final int sequenceNumber, final OnGetSuggestedWordsCallback callback) { final Keyboard keyboard = mKeyboardSwitcher.getKeyboard(); final Suggest suggest = mSuggest; @@ -2517,18 +2542,19 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen } suggest.getSuggestedWords(mWordComposer, prevWord, keyboard.getProximityInfo(), currentSettings.mBlockPotentiallyOffensive, currentSettings.mCorrectionEnabled, - additionalFeaturesOptions, sessionId, callback); + additionalFeaturesOptions, sessionId, sequenceNumber, callback); } private void getSuggestedWordsOrOlderSuggestionsAsync(final int sessionId, - final OnGetSuggestedWordsCallback callback) { - mInputUpdater.getSuggestedWords(sessionId, new OnGetSuggestedWordsCallback() { - @Override - public void onGetSuggestedWords(SuggestedWords suggestedWords) { - callback.onGetSuggestedWords(maybeRetrieveOlderSuggestions( - mWordComposer.getTypedWord(), suggestedWords)); - } - }); + final int sequenceNumber, final OnGetSuggestedWordsCallback callback) { + mInputUpdater.getSuggestedWords(sessionId, sequenceNumber, + new OnGetSuggestedWordsCallback() { + @Override + public void onGetSuggestedWords(SuggestedWords suggestedWords) { + callback.onGetSuggestedWords(maybeRetrieveOlderSuggestions( + mWordComposer.getTypedWord(), suggestedWords)); + } + }); } private SuggestedWords maybeRetrieveOlderSuggestions(final String typedWord, @@ -2861,30 +2887,30 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen // We come here if there weren't any suggestion spans on this word. We will try to // compute suggestions for it instead. mInputUpdater.getSuggestedWords(Suggest.SESSION_TYPING, - new OnGetSuggestedWordsCallback() { - @Override - public void onGetSuggestedWords( - final SuggestedWords suggestedWordsIncludingTypedWord) { - final SuggestedWords suggestedWords; - if (suggestedWordsIncludingTypedWord.size() > 1) { - // We were able to compute new suggestions for this word. - // Remove the typed word, since we don't want to display it in this case. - // The #getSuggestedWordsExcludingTypedWord() method sets willAutoCorrect to - // false. - suggestedWords = suggestedWordsIncludingTypedWord - .getSuggestedWordsExcludingTypedWord(); - } else { - // No saved suggestions, and we were unable to compute any good one either. - // Rather than displaying an empty suggestion strip, we'll display the - // original word alone in the middle. - // Since there is only one word, willAutoCorrect is false. - suggestedWords = suggestedWordsIncludingTypedWord; - } - // We need to pass typedWord because mWordComposer.mTypedWord may differ from - // typedWord. - unsetIsAutoCorrectionIndicatorOnAndCallShowSuggestionStrip(suggestedWords, - typedWord); - }}); + SuggestedWords.NOT_A_SEQUENCE_NUMBER, new OnGetSuggestedWordsCallback() { + @Override + public void onGetSuggestedWords( + final SuggestedWords suggestedWordsIncludingTypedWord) { + final SuggestedWords suggestedWords; + if (suggestedWordsIncludingTypedWord.size() > 1) { + // We were able to compute new suggestions for this word. + // Remove the typed word, since we don't want to display it in this + // case. The #getSuggestedWordsExcludingTypedWord() method sets + // willAutoCorrect to false. + suggestedWords = suggestedWordsIncludingTypedWord + .getSuggestedWordsExcludingTypedWord(); + } else { + // No saved suggestions, and we were unable to compute any good one + // either. Rather than displaying an empty suggestion strip, we'll + // display the original word alone in the middle. + // Since there is only one word, willAutoCorrect is false. + suggestedWords = suggestedWordsIncludingTypedWord; + } + // We need to pass typedWord because mWordComposer.mTypedWord may + // differ from typedWord. + unsetIsAutoCorrectionIndicatorOnAndCallShowSuggestionStrip( + suggestedWords, typedWord); + }}); } else { // We found suggestion spans in the word. We'll create the SuggestedWords out of // them, and make willAutoCorrect false. diff --git a/java/src/com/android/inputmethod/latin/Suggest.java b/java/src/com/android/inputmethod/latin/Suggest.java index c270d47d0..72b9c417c 100644 --- a/java/src/com/android/inputmethod/latin/Suggest.java +++ b/java/src/com/android/inputmethod/latin/Suggest.java @@ -217,15 +217,17 @@ public final class Suggest { public void getSuggestedWords(final WordComposer wordComposer, final String prevWordForBigram, final ProximityInfo proximityInfo, final boolean blockOffensiveWords, final boolean isCorrectionEnabled, - final int[] additionalFeaturesOptions, final int sessionId, + final int[] additionalFeaturesOptions, final int sessionId, final int sequenceNumber, final OnGetSuggestedWordsCallback callback) { LatinImeLogger.onStartSuggestion(prevWordForBigram); if (wordComposer.isBatchMode()) { getSuggestedWordsForBatchInput(wordComposer, prevWordForBigram, proximityInfo, - blockOffensiveWords, additionalFeaturesOptions, sessionId, callback); + blockOffensiveWords, additionalFeaturesOptions, sessionId, sequenceNumber, + callback); } else { getSuggestedWordsForTypingInput(wordComposer, prevWordForBigram, proximityInfo, - blockOffensiveWords, isCorrectionEnabled, additionalFeaturesOptions, callback); + blockOffensiveWords, isCorrectionEnabled, additionalFeaturesOptions, + sequenceNumber, callback); } } @@ -234,7 +236,8 @@ public final class Suggest { private void getSuggestedWordsForTypingInput(final WordComposer wordComposer, final String prevWordForBigram, final ProximityInfo proximityInfo, final boolean blockOffensiveWords, final boolean isCorrectionEnabled, - final int[] additionalFeaturesOptions, final OnGetSuggestedWordsCallback callback) { + final int[] additionalFeaturesOptions, final int sequenceNumber, + final OnGetSuggestedWordsCallback callback) { final int trailingSingleQuotesCount = wordComposer.trailingSingleQuotesCount(); final BoundedTreeSet suggestionsSet = new BoundedTreeSet(sSuggestedWordInfoComparator, MAX_SUGGESTIONS); @@ -347,7 +350,7 @@ public final class Suggest { hasAutoCorrection, /* willAutoCorrect */ false /* isPunctuationSuggestions */, false /* isObsoleteSuggestions */, - !wordComposer.isComposingWord() /* isPrediction */)); + !wordComposer.isComposingWord() /* isPrediction */, sequenceNumber)); } // Retrieves suggestions for the batch input @@ -355,7 +358,8 @@ public final class Suggest { private void getSuggestedWordsForBatchInput(final WordComposer wordComposer, final String prevWordForBigram, final ProximityInfo proximityInfo, final boolean blockOffensiveWords, final int[] additionalFeaturesOptions, - final int sessionId, final OnGetSuggestedWordsCallback callback) { + final int sessionId, final int sequenceNumber, + final OnGetSuggestedWordsCallback callback) { final BoundedTreeSet suggestionsSet = new BoundedTreeSet(sSuggestedWordInfoComparator, MAX_SUGGESTIONS); @@ -408,7 +412,7 @@ public final class Suggest { false /* willAutoCorrect */, false /* isPunctuationSuggestions */, false /* isObsoleteSuggestions */, - false /* isPrediction */)); + false /* isPrediction */, sequenceNumber)); } private static ArrayList getSuggestionsInfoListWithDebugInfo( diff --git a/java/src/com/android/inputmethod/latin/SuggestedWords.java b/java/src/com/android/inputmethod/latin/SuggestedWords.java index fed4cdbbb..97c89dd4e 100644 --- a/java/src/com/android/inputmethod/latin/SuggestedWords.java +++ b/java/src/com/android/inputmethod/latin/SuggestedWords.java @@ -29,6 +29,7 @@ import java.util.HashSet; public final class SuggestedWords { public static final int INDEX_OF_TYPED_WORD = 0; public static final int INDEX_OF_AUTO_CORRECTION = 1; + public static final int NOT_A_SEQUENCE_NUMBER = -1; private static final ArrayList EMPTY_WORD_INFO_LIST = CollectionUtils.newArrayList(0); @@ -43,6 +44,7 @@ public final class SuggestedWords { public final boolean mIsPunctuationSuggestions; public final boolean mIsObsoleteSuggestions; public final boolean mIsPrediction; + public final int mSequenceNumber; // Sequence number for auto-commit. private final ArrayList mSuggestedWordInfoList; public SuggestedWords(final ArrayList suggestedWordInfoList, @@ -51,12 +53,24 @@ public final class SuggestedWords { final boolean isPunctuationSuggestions, final boolean isObsoleteSuggestions, final boolean isPrediction) { + this(suggestedWordInfoList, typedWordValid, willAutoCorrect, isPunctuationSuggestions, + isObsoleteSuggestions, isPrediction, NOT_A_SEQUENCE_NUMBER); + } + + public SuggestedWords(final ArrayList suggestedWordInfoList, + final boolean typedWordValid, + final boolean willAutoCorrect, + final boolean isPunctuationSuggestions, + final boolean isObsoleteSuggestions, + final boolean isPrediction, + final int sequenceNumber) { mSuggestedWordInfoList = suggestedWordInfoList; mTypedWordValid = typedWordValid; mWillAutoCorrect = willAutoCorrect; mIsPunctuationSuggestions = isPunctuationSuggestions; mIsObsoleteSuggestions = isObsoleteSuggestions; mIsPrediction = isPrediction; + mSequenceNumber = sequenceNumber; } public boolean isEmpty() {