From f1d8aa46f9172c2d8864d0d2161aa8220d036cc9 Mon Sep 17 00:00:00 2001 From: Jean Chalard Date: Fri, 20 Sep 2013 18:01:32 +0900 Subject: [PATCH] Detect cases where rotation messes with initialization ...and do a best effort to fix it. Bug: 10323080 Bug: 10252066 Change-Id: Icb3c9fe85005406bdfce0b7bb143ba0a910a0ddb --- .../android/inputmethod/latin/Constants.java | 3 + .../android/inputmethod/latin/LatinIME.java | 82 +++++++++++++++++-- .../latin/RichInputConnection.java | 61 +++++++++++--- 3 files changed, 129 insertions(+), 17 deletions(-) diff --git a/java/src/com/android/inputmethod/latin/Constants.java b/java/src/com/android/inputmethod/latin/Constants.java index 28d9e8652..11c22109b 100644 --- a/java/src/com/android/inputmethod/latin/Constants.java +++ b/java/src/com/android/inputmethod/latin/Constants.java @@ -138,6 +138,9 @@ public final class Constants { public static final int SPELL_CHECKER_COORDINATE = -3; public static final int EXTERNAL_KEYBOARD_COORDINATE = -4; + // A hint on how many characters to cache from the TextView. A good value of this is given by + // how many characters we need to be able to almost always find the caps mode. + public static final int EDITOR_CONTENTS_CACHE_SIZE = 1024; // Must be equal to MAX_WORD_LENGTH in native/jni/src/defines.h public static final int DICTIONARY_MAX_WORD_LENGTH = 48; diff --git a/java/src/com/android/inputmethod/latin/LatinIME.java b/java/src/com/android/inputmethod/latin/LatinIME.java index dead53032..f4f0620d3 100644 --- a/java/src/com/android/inputmethod/latin/LatinIME.java +++ b/java/src/com/android/inputmethod/latin/LatinIME.java @@ -233,6 +233,7 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen private static final int MSG_RESUME_SUGGESTIONS = 4; private static final int MSG_REOPEN_DICTIONARIES = 5; private static final int MSG_ON_END_BATCH_INPUT = 6; + private static final int MSG_RESET_CACHES = 7; private static final int ARG1_NOT_GESTURE_INPUT = 0; private static final int ARG1_DISMISS_GESTURE_FLOATING_PREVIEW_TEXT = 1; @@ -297,6 +298,10 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen case MSG_ON_END_BATCH_INPUT: latinIme.onEndBatchInputAsyncInternal((SuggestedWords) msg.obj); break; + case MSG_RESET_CACHES: + latinIme.retryResetCaches(msg.arg1 == 1 /* tryResumeSuggestions */, + msg.arg2 /* remainingTries */); + break; } } @@ -313,6 +318,12 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen sendMessageDelayed(obtainMessage(MSG_RESUME_SUGGESTIONS), mDelayUpdateSuggestions); } + public void postResetCaches(final boolean tryResumeSuggestions, final int remainingTries) { + removeMessages(MSG_RESET_CACHES); + sendMessage(obtainMessage(MSG_RESET_CACHES, tryResumeSuggestions ? 1 : 0, + remainingTries, null)); + } + public void cancelUpdateSuggestionStrip() { removeMessages(MSG_UPDATE_SUGGESTION_STRIP); } @@ -852,7 +863,6 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen // span, so we should reset our state unconditionally, even if restarting is true. mEnteredText = null; resetComposingState(true /* alsoResetLastComposedWord */); - if (isDifferentTextField) mHandler.postResumeSuggestions(); mDeleteCount = 0; mSpaceState = SPACE_STATE_NONE; mRecapitalizeStatus.deactivate(); @@ -871,8 +881,16 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen } mSuggestedWords = SuggestedWords.EMPTY; - mConnection.resetCachesUponCursorMove(editorInfo.initialSelStart, - false /* shouldFinishComposition */); + // Sometimes, while rotating, for some reason the framework tells the app we are not + // connected to it and that means we can't refresh the cache. In this case, schedule a + // refresh later. + if (!mConnection.resetCachesUponCursorMoveAndReturnSuccess(editorInfo.initialSelStart, + false /* shouldFinishComposition */)) { + // We try resetting the caches up to 5 times before giving up. + mHandler.postResetCaches(isDifferentTextField, 5 /* remainingTries */); + } else { + if (isDifferentTextField) mHandler.postResumeSuggestions(); + } if (isDifferentTextField) { mainKeyboardView.closing(); @@ -899,6 +917,9 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen mLastSelectionStart = editorInfo.initialSelStart; mLastSelectionEnd = editorInfo.initialSelEnd; + // In some cases (namely, after rotation of the device) editorInfo.initialSelStart is lying + // so we try using some heuristics to find out about these and fix them. + tryFixLyingCursorPosition(); mHandler.cancelUpdateSuggestionStrip(); mHandler.cancelDoubleSpacePeriodTimer(); @@ -918,6 +939,35 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen if (TRACE) Debug.startMethodTracing("/data/trace/latinime"); } + /** + * Try to get the text from the editor to expose lies the framework may have been + * telling us. Concretely, when the device rotates, the frameworks tells us about where the + * cursor used to be initially in the editor at the time it first received the focus; this + * may be completely different from the place it is upon rotation. Since we don't have any + * means to get the real value, try at least to ask the text view for some characters and + * detect the most damaging cases: when the cursor position is declared to be much smaller + * than it really is. + */ + private void tryFixLyingCursorPosition() { + final CharSequence textBeforeCursor = + mConnection.getTextBeforeCursor(Constants.EDITOR_CONTENTS_CACHE_SIZE, 0); + if (null == textBeforeCursor) { + mLastSelectionStart = mLastSelectionEnd = NOT_A_CURSOR_POSITION; + } else { + final int textLength = textBeforeCursor.length(); + if (textLength > mLastSelectionStart + || (textLength < Constants.EDITOR_CONTENTS_CACHE_SIZE + && mLastSelectionStart < Constants.EDITOR_CONTENTS_CACHE_SIZE)) { + mLastSelectionStart = textLength; + // We can't figure out the value of mLastSelectionEnd :( + // But at least if it's smaller than mLastSelectionStart something is wrong + if (mLastSelectionStart > mLastSelectionEnd) { + mLastSelectionEnd = mLastSelectionStart; + } + } + } + } + // Initialization of personalization debug settings. This must be called inside // onStartInputView. private void initPersonalizationDebugSettings(SettingsValues currentSettingsValues) { @@ -1072,7 +1122,7 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen // argument as true. But in all cases where we don't reset the entire input state, // we still want to tell the rich input connection about the new cursor position so // that it can update its caches. - mConnection.resetCachesUponCursorMove(newSelStart, + mConnection.resetCachesUponCursorMoveAndReturnSuccess(newSelStart, false /* shouldFinishComposition */); } @@ -1308,7 +1358,8 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen } else { setSuggestedWords(settingsValues.mSuggestPuncList, false); } - mConnection.resetCachesUponCursorMove(newCursorPosition, shouldFinishComposition); + mConnection.resetCachesUponCursorMoveAndReturnSuccess(newCursorPosition, + shouldFinishComposition); } private void resetComposingState(final boolean alsoResetLastComposedWord) { @@ -2852,6 +2903,27 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen mHandler.postUpdateSuggestionStrip(); } + /** + * Retry resetting caches in the rich input connection. + * + * When the editor can't be accessed we can't reset the caches, so we schedule a retry. + * This method handles the retry, and re-schedules a new retry if we still can't access. + * We only retry up to 5 times before giving up. + * + * @param tryResumeSuggestions Whether we should resume suggestions or not. + * @param remainingTries How many times we may try again before giving up. + */ + private void retryResetCaches(final boolean tryResumeSuggestions, final int remainingTries) { + if (!mConnection.resetCachesUponCursorMoveAndReturnSuccess(mLastSelectionStart, false)) { + if (0 < remainingTries) { + mHandler.postResetCaches(tryResumeSuggestions, remainingTries - 1); + } + return; + } + tryFixLyingCursorPosition(); + if (tryResumeSuggestions) mHandler.postResumeSuggestions(); + } + private void revertCommit() { final String previousWord = mLastComposedWord.mPrevWord; final String originallyTypedWord = mLastComposedWord.mTypedWord; diff --git a/java/src/com/android/inputmethod/latin/RichInputConnection.java b/java/src/com/android/inputmethod/latin/RichInputConnection.java index 2ecf1463f..fa7f23efb 100644 --- a/java/src/com/android/inputmethod/latin/RichInputConnection.java +++ b/java/src/com/android/inputmethod/latin/RichInputConnection.java @@ -73,9 +73,6 @@ public final class RichInputConnection { * This contains the currently composing text, as LatinIME thinks the TextView is seeing it. */ private final StringBuilder mComposingText = new StringBuilder(); - // A hint on how many characters to cache from the TextView. A good value of this is given by - // how many characters we need to be able to almost always find the caps mode. - private static final int DEFAULT_TEXT_CACHE_SIZE = 100; private final InputMethodService mParent; InputConnection mIC; @@ -93,7 +90,8 @@ public final class RichInputConnection { r.token = 1; r.flags = 0; final ExtractedText et = mIC.getExtractedText(r, 0); - final CharSequence beforeCursor = getTextBeforeCursor(DEFAULT_TEXT_CACHE_SIZE, 0); + final CharSequence beforeCursor = getTextBeforeCursor(Constants.EDITOR_CONTENTS_CACHE_SIZE, + 0); final StringBuilder internal = new StringBuilder().append(mCommittedTextBeforeComposingText) .append(mComposingText); if (null == et || null == beforeCursor) return; @@ -142,19 +140,56 @@ public final class RichInputConnection { if (DEBUG_PREVIOUS_TEXT) checkConsistencyForDebug(); } - public void resetCachesUponCursorMove(final int newCursorPosition, + /** + * Reset the cached text and retrieve it again from the editor. + * + * This should be called when the cursor moved. It's possible that we can't connect to + * the application when doing this; notably, this happens sometimes during rotation, probably + * because of a race condition in the framework. In this case, we just can't retrieve the + * data, so we empty the cache and note that we don't know the new cursor position, and we + * return false so that the caller knows about this and can retry later. + * + * @param newCursorPosition The new position of the cursor, as received from the system. + * @param shouldFinishComposition Whether we should finish the composition in progress. + * @return true if we were able to connect to the editor successfully, false otherwise. When + * this method returns false, the caches could not be correctly refreshed so they were only + * reset: the caller should try again later to return to normal operation. + */ + public boolean resetCachesUponCursorMoveAndReturnSuccess(final int newCursorPosition, final boolean shouldFinishComposition) { mExpectedCursorPosition = newCursorPosition; mComposingText.setLength(0); mCommittedTextBeforeComposingText.setLength(0); - final CharSequence textBeforeCursor = getTextBeforeCursor(DEFAULT_TEXT_CACHE_SIZE, 0); - if (null != textBeforeCursor) mCommittedTextBeforeComposingText.append(textBeforeCursor); + mIC = mParent.getCurrentInputConnection(); + // Call upon the inputconnection directly since our own method is using the cache, and + // we want to refresh it. + final CharSequence textBeforeCursor = null == mIC ? null : + mIC.getTextBeforeCursor(Constants.EDITOR_CONTENTS_CACHE_SIZE, 0); + if (null == textBeforeCursor) { + // For some reason the app thinks we are not connected to it. This looks like a + // framework bug... Fall back to ground state and return false. + mExpectedCursorPosition = INVALID_CURSOR_POSITION; + Log.e(TAG, "Unable to connect to the editor to retrieve text... will retry later"); + return false; + } + mCommittedTextBeforeComposingText.append(textBeforeCursor); + final int lengthOfTextBeforeCursor = textBeforeCursor.length(); + if (lengthOfTextBeforeCursor > newCursorPosition + || (lengthOfTextBeforeCursor < Constants.EDITOR_CONTENTS_CACHE_SIZE + && newCursorPosition < Constants.EDITOR_CONTENTS_CACHE_SIZE)) { + // newCursorPosition may be lying -- when rotating the device (probably a framework + // bug). If we have less chars than we asked for, then we know how many chars we have, + // and if we got more than newCursorPosition says, then we know it was lying. In both + // cases the length is more reliable + mExpectedCursorPosition = lengthOfTextBeforeCursor; + } if (null != mIC && shouldFinishComposition) { mIC.finishComposingText(); if (ProductionFlag.USES_DEVELOPMENT_ONLY_DIAGNOSTICS) { ResearchLogger.richInputConnection_finishComposingText(); } } + return true; } private void checkBatchEdit() { @@ -233,7 +268,8 @@ public final class RichInputConnection { // getCapsMode should be updated to be able to return a "not enough info" result so that // we can get more context only when needed. if (TextUtils.isEmpty(mCommittedTextBeforeComposingText) && 0 != mExpectedCursorPosition) { - final CharSequence textBeforeCursor = getTextBeforeCursor(DEFAULT_TEXT_CACHE_SIZE, 0); + final CharSequence textBeforeCursor = getTextBeforeCursor( + Constants.EDITOR_CONTENTS_CACHE_SIZE, 0); if (!TextUtils.isEmpty(textBeforeCursor)) { mCommittedTextBeforeComposingText.append(textBeforeCursor); } @@ -364,7 +400,7 @@ public final class RichInputConnection { if (DEBUG_BATCH_NESTING) checkBatchEdit(); if (DEBUG_PREVIOUS_TEXT) checkConsistencyForDebug(); final CharSequence textBeforeCursor = - getTextBeforeCursor(DEFAULT_TEXT_CACHE_SIZE + (end - start), 0); + getTextBeforeCursor(Constants.EDITOR_CONTENTS_CACHE_SIZE + (end - start), 0); mCommittedTextBeforeComposingText.setLength(0); if (!TextUtils.isEmpty(textBeforeCursor)) { final int indexOfStartOfComposingText = @@ -406,7 +442,8 @@ public final class RichInputConnection { } mExpectedCursorPosition = start; mCommittedTextBeforeComposingText.setLength(0); - mCommittedTextBeforeComposingText.append(getTextBeforeCursor(DEFAULT_TEXT_CACHE_SIZE, 0)); + mCommittedTextBeforeComposingText.append( + getTextBeforeCursor(Constants.EDITOR_CONTENTS_CACHE_SIZE, 0)); } public void commitCorrection(final CorrectionInfo correctionInfo) { @@ -525,9 +562,9 @@ public final class RichInputConnection { if (mIC == null || sep == null) { return null; } - final CharSequence before = mIC.getTextBeforeCursor(1000, + final CharSequence before = mIC.getTextBeforeCursor(Constants.EDITOR_CONTENTS_CACHE_SIZE, InputConnection.GET_TEXT_WITH_STYLES); - final CharSequence after = mIC.getTextAfterCursor(1000, + final CharSequence after = mIC.getTextAfterCursor(Constants.EDITOR_CONTENTS_CACHE_SIZE, InputConnection.GET_TEXT_WITH_STYLES); if (before == null || after == null) { return null;