nyancrimew/LatinIME
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

[IL54.5] Use the cursor pos estimate, not the last value

Browse source 
For edit tasks, the estimate is actually the right thing to use.
This is really dangerous, but it will get rid of pretty much all
race conditions.

Change-Id: I2d5ca3ce45e32f1bd9c8b778421fd54b9c1f6f63
This commit is contained in:
Jean Chalard 2014-01-09 14:09:26 +09:00
parent ecf46df226
commit a224aafea3
2 changed files with 41 additions and 91 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

16
java/src/com/android/inputmethod/latin/LatinIME.java
View file

@ -880,8 +880,9 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen
// Notify ResearchLogger // Notify ResearchLogger
if (ProductionFlag.USES_DEVELOPMENT_ONLY_DIAGNOSTICS) { if (ProductionFlag.USES_DEVELOPMENT_ONLY_DIAGNOSTICS) {
ResearchLogger.latinIME_onFinishInputViewInternal(finishingInput, ResearchLogger.latinIME_onFinishInputViewInternal(finishingInput,
mInputLogic.mLastSelectionStart, // TODO[IL]: mInputLogic.mConnection should be private
mInputLogic.mLastSelectionEnd, getCurrentInputConnection()); mInputLogic.mConnection.getExpectedSelectionStart(),
mInputLogic.mConnection.getExpectedSelectionEnd(), getCurrentInputConnection());
} }
} }
@ -894,22 +895,18 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen
if (DEBUG) { if (DEBUG) {
Log.i(TAG, "onUpdateSelection: oss=" + oldSelStart Log.i(TAG, "onUpdateSelection: oss=" + oldSelStart
+ ", ose=" + oldSelEnd + ", ose=" + oldSelEnd
+ ", lss=" + mInputLogic.mLastSelectionStart
+ ", lse=" + mInputLogic.mLastSelectionEnd
+ ", nss=" + newSelStart + ", nss=" + newSelStart
+ ", nse=" + newSelEnd + ", nse=" + newSelEnd
+ ", cs=" + composingSpanStart + ", cs=" + composingSpanStart
+ ", ce=" + composingSpanEnd); + ", ce=" + composingSpanEnd);
} }
if (ProductionFlag.USES_DEVELOPMENT_ONLY_DIAGNOSTICS) { if (ProductionFlag.USES_DEVELOPMENT_ONLY_DIAGNOSTICS) {
ResearchLogger.latinIME_onUpdateSelection(mInputLogic.mLastSelectionStart, ResearchLogger.latinIME_onUpdateSelection(oldSelStart, oldSelEnd,
mInputLogic.mLastSelectionEnd,
oldSelStart, oldSelEnd, newSelStart, newSelEnd, composingSpanStart, oldSelStart, oldSelEnd, newSelStart, newSelEnd, composingSpanStart,
composingSpanEnd, mInputLogic.mConnection); composingSpanEnd, mInputLogic.mConnection);
} }
final boolean selectionChanged = mInputLogic.mLastSelectionStart != newSelStart final boolean selectionChanged = oldSelStart != newSelStart || oldSelEnd != newSelEnd;
|| mInputLogic.mLastSelectionEnd != newSelEnd;
// if composingSpanStart and composingSpanEnd are -1, it means there is no composing // if composingSpanStart and composingSpanEnd are -1, it means there is no composing
// span in the view - we can use that to narrow down whether the cursor was moved // span in the view - we can use that to narrow down whether the cursor was moved
@ -972,9 +969,6 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen
mKeyboardSwitcher.updateShiftState(); mKeyboardSwitcher.updateShiftState();
} }
// Make a note of the cursor position
mInputLogic.mLastSelectionStart = newSelStart;
mInputLogic.mLastSelectionEnd = newSelEnd;
mSubtypeState.currentSubtypeUsed(); mSubtypeState.currentSubtypeUsed();
} }

116
java/src/com/android/inputmethod/latin/inputlogic/InputLogic.java
View file

@ -86,10 +86,6 @@ public final class InputLogic {
public final RichInputConnection mConnection; public final RichInputConnection mConnection;
public final RecapitalizeStatus mRecapitalizeStatus = new RecapitalizeStatus(); public final RecapitalizeStatus mRecapitalizeStatus = new RecapitalizeStatus();
// Keep track of the last selection range to decide if we need to show word alternatives
public int mLastSelectionStart = Constants.NOT_A_CURSOR_POSITION;
public int mLastSelectionEnd = Constants.NOT_A_CURSOR_POSITION;
private int mDeleteCount; private int mDeleteCount;
private long mLastKeyTime; private long mLastKeyTime;
public final TreeSet<Long> mCurrentlyPressedHardwareKeys = CollectionUtils.newTreeSet(); public final TreeSet<Long> mCurrentlyPressedHardwareKeys = CollectionUtils.newTreeSet();
@ -129,12 +125,9 @@ public final class InputLogic {
mRecapitalizeStatus.deactivate(); mRecapitalizeStatus.deactivate();
mCurrentlyPressedHardwareKeys.clear(); mCurrentlyPressedHardwareKeys.clear();
mSuggestedWords = SuggestedWords.EMPTY; mSuggestedWords = SuggestedWords.EMPTY;
mLastSelectionStart = editorInfo.initialSelStart;
mLastSelectionEnd = editorInfo.initialSelEnd;
// In some cases (namely, after rotation of the device) editorInfo.initialSelStart is lying // 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. // so we try using some heuristics to find out about these and fix them.
mConnection.tryFixLyingCursorPosition(); mConnection.tryFixLyingCursorPosition();
tryFixLyingCursorPosition();
mInputLogicHandler = new InputLogicHandler(mLatinIME, this); mInputLogicHandler = new InputLogicHandler(mLatinIME, this);
} }
@ -338,7 +331,8 @@ public final class InputLogic {
if (mWordComposer.isCursorFrontOrMiddleOfComposingWord()) { if (mWordComposer.isCursorFrontOrMiddleOfComposingWord()) {
// If we are in the middle of a recorrection, we need to commit the recorrection // If we are in the middle of a recorrection, we need to commit the recorrection
// first so that we can insert the batch input at the current cursor position. // first so that we can insert the batch input at the current cursor position.
resetEntireInputState(settingsValues, mLastSelectionStart, mLastSelectionEnd); resetEntireInputState(settingsValues, mConnection.getExpectedSelectionStart(),
mConnection.getExpectedSelectionEnd());
} else if (wordComposerSize <= 1) { } else if (wordComposerSize <= 1) {
// We auto-correct the previous (typed, not gestured) string iff it's one character // We auto-correct the previous (typed, not gestured) string iff it's one character
// long. The reason for this is, even in the middle of gesture typing, you'll still // long. The reason for this is, even in the middle of gesture typing, you'll still
@ -467,7 +461,8 @@ public final class InputLogic {
if (mWordComposer.isCursorFrontOrMiddleOfComposingWord()) { if (mWordComposer.isCursorFrontOrMiddleOfComposingWord()) {
// If we are in the middle of a recorrection, we need to commit the recorrection // If we are in the middle of a recorrection, we need to commit the recorrection
// first so that we can insert the character at the current cursor position. // first so that we can insert the character at the current cursor position.
resetEntireInputState(settingsValues, mLastSelectionStart, mLastSelectionEnd); resetEntireInputState(settingsValues, mConnection.getExpectedSelectionStart(),
mConnection.getExpectedSelectionEnd());
} else { } else {
commitTyped(settingsValues, LastComposedWord.NOT_A_SEPARATOR); commitTyped(settingsValues, LastComposedWord.NOT_A_SEPARATOR);
} }
@ -517,7 +512,8 @@ public final class InputLogic {
if (mWordComposer.isCursorFrontOrMiddleOfComposingWord()) { if (mWordComposer.isCursorFrontOrMiddleOfComposingWord()) {
// If we are in the middle of a recorrection, we need to commit the recorrection // If we are in the middle of a recorrection, we need to commit the recorrection
// first so that we can insert the character at the current cursor position. // first so that we can insert the character at the current cursor position.
resetEntireInputState(settingsValues, mLastSelectionStart, mLastSelectionEnd); resetEntireInputState(settingsValues, mConnection.getExpectedSelectionStart(),
mConnection.getExpectedSelectionEnd());
isComposingWord = false; isComposingWord = false;
} }
// We want to find out whether to start composing a new word with this character. If so, // We want to find out whether to start composing a new word with this character. If so,
@ -604,7 +600,8 @@ public final class InputLogic {
if (mWordComposer.isCursorFrontOrMiddleOfComposingWord()) { if (mWordComposer.isCursorFrontOrMiddleOfComposingWord()) {
// If we are in the middle of a recorrection, we need to commit the recorrection // If we are in the middle of a recorrection, we need to commit the recorrection
// first so that we can insert the separator at the current cursor position. // first so that we can insert the separator at the current cursor position.
resetEntireInputState(settingsValues, mLastSelectionStart, mLastSelectionEnd); resetEntireInputState(settingsValues, mConnection.getExpectedSelectionStart(),
mConnection.getExpectedSelectionEnd());
} }
// isComposingWord() may have changed since we stored wasComposing // isComposingWord() may have changed since we stored wasComposing
if (mWordComposer.isComposingWord()) { if (mWordComposer.isComposingWord()) {
@ -696,7 +693,8 @@ public final class InputLogic {
if (mWordComposer.isCursorFrontOrMiddleOfComposingWord()) { if (mWordComposer.isCursorFrontOrMiddleOfComposingWord()) {
// If we are in the middle of a recorrection, we need to commit the recorrection // If we are in the middle of a recorrection, we need to commit the recorrection
// first so that we can remove the character at the current cursor position. // first so that we can remove the character at the current cursor position.
resetEntireInputState(settingsValues, mLastSelectionStart, mLastSelectionEnd); resetEntireInputState(settingsValues, mConnection.getExpectedSelectionStart(),
mConnection.getExpectedSelectionEnd());
// When we exit this if-clause, mWordComposer.isComposingWord() will return false. // When we exit this if-clause, mWordComposer.isComposingWord() will return false.
} }
if (mWordComposer.isComposingWord()) { if (mWordComposer.isComposingWord()) {
@ -756,15 +754,12 @@ public final class InputLogic {
// No cancelling of commit/double space/swap: we have a regular backspace. // No cancelling of commit/double space/swap: we have a regular backspace.
// We should backspace one char and restart suggestion if at the end of a word. // We should backspace one char and restart suggestion if at the end of a word.
if (mLastSelectionStart != mLastSelectionEnd) { if (mConnection.hasSelection()) {
// If there is a selection, remove it. // If there is a selection, remove it.
final int numCharsDeleted = mLastSelectionEnd - mLastSelectionStart; final int numCharsDeleted = mConnection.getExpectedSelectionEnd()
mConnection.setSelection(mLastSelectionEnd, mLastSelectionEnd); - mConnection.getExpectedSelectionStart();
// Reset mLastSelectionEnd to mLastSelectionStart. This is what is supposed to mConnection.setSelection(mConnection.getExpectedSelectionEnd(),
// happen, and if it's wrong, the next call to onUpdateSelection will correct it, mConnection.getExpectedSelectionEnd());
// but we want to set it right away to avoid it being used with the wrong values
// later (typically, in a subsequent press on backspace).
mLastSelectionEnd = mLastSelectionStart;
mConnection.deleteSurroundingText(numCharsDeleted, 0); mConnection.deleteSurroundingText(numCharsDeleted, 0);
if (ProductionFlag.USES_DEVELOPMENT_ONLY_DIAGNOSTICS) { if (ProductionFlag.USES_DEVELOPMENT_ONLY_DIAGNOSTICS) {
ResearchLogger.latinIME_handleBackspace(numCharsDeleted, ResearchLogger.latinIME_handleBackspace(numCharsDeleted,
@ -772,7 +767,7 @@ public final class InputLogic {
} }
} else { } else {
// There is no selection, just delete one character. // There is no selection, just delete one character.
if (Constants.NOT_A_CURSOR_POSITION == mLastSelectionEnd) { if (Constants.NOT_A_CURSOR_POSITION == mConnection.getExpectedSelectionEnd()) {
// This should never happen. // This should never happen.
Log.e(TAG, "Backspace when we don't know the selection position"); Log.e(TAG, "Backspace when we don't know the selection position");
} }
@ -961,38 +956,32 @@ public final class InputLogic {
* @param settingsValues The current settings values. * @param settingsValues The current settings values.
*/ */
private void performRecapitalization(final SettingsValues settingsValues) { private void performRecapitalization(final SettingsValues settingsValues) {
if (mLastSelectionStart == mLastSelectionEnd) { if (!mConnection.hasSelection()) {
return; // No selection return; // No selection
} }
// If we have a recapitalize in progress, use it; otherwise, create a new one. // If we have a recapitalize in progress, use it; otherwise, create a new one.
if (!mRecapitalizeStatus.isActive() if (!mRecapitalizeStatus.isActive()
|| !mRecapitalizeStatus.isSetAt(mLastSelectionStart, mLastSelectionEnd)) { || !mRecapitalizeStatus.isSetAt(mConnection.getExpectedSelectionStart(),
mConnection.getExpectedSelectionEnd())) {
final CharSequence selectedText = final CharSequence selectedText =
mConnection.getSelectedText(0 /* flags, 0 for no styles */); mConnection.getSelectedText(0 /* flags, 0 for no styles */);
if (TextUtils.isEmpty(selectedText)) return; // Race condition with the input connection if (TextUtils.isEmpty(selectedText)) return; // Race condition with the input connection
mRecapitalizeStatus.initialize(mLastSelectionStart, mLastSelectionEnd, mRecapitalizeStatus.initialize(mConnection.getExpectedSelectionStart(),
selectedText.toString(), mConnection.getExpectedSelectionEnd(), selectedText.toString(),
settingsValues.mLocale, settingsValues.mSpacingAndPunctuations.mWordSeparators); settingsValues.mLocale, settingsValues.mSpacingAndPunctuations.mWordSeparators);
// We trim leading and trailing whitespace. // We trim leading and trailing whitespace.
mRecapitalizeStatus.trim(); mRecapitalizeStatus.trim();
// Trimming the object may have changed the length of the string, and we need to
// reposition the selection handles accordingly. As this result in an IPC call,
// only do it if it's actually necessary, in other words if the recapitalize status
// is not set at the same place as before.
if (!mRecapitalizeStatus.isSetAt(mLastSelectionStart, mLastSelectionEnd)) {
mLastSelectionStart = mRecapitalizeStatus.getNewCursorStart();
mLastSelectionEnd = mRecapitalizeStatus.getNewCursorEnd();
}
} }
mConnection.finishComposingText(); mConnection.finishComposingText();
mRecapitalizeStatus.rotate(); mRecapitalizeStatus.rotate();
final int numCharsDeleted = mLastSelectionEnd - mLastSelectionStart; final int numCharsDeleted = mConnection.getExpectedSelectionEnd()
mConnection.setSelection(mLastSelectionEnd, mLastSelectionEnd); - mConnection.getExpectedSelectionStart();
mConnection.setSelection(mConnection.getExpectedSelectionEnd(),
mConnection.getExpectedSelectionEnd());
mConnection.deleteSurroundingText(numCharsDeleted, 0); mConnection.deleteSurroundingText(numCharsDeleted, 0);
mConnection.commitText(mRecapitalizeStatus.getRecapitalizedString(), 0); mConnection.commitText(mRecapitalizeStatus.getRecapitalizedString(), 0);
mLastSelectionStart = mRecapitalizeStatus.getNewCursorStart(); mConnection.setSelection(mRecapitalizeStatus.getNewCursorStart(),
mLastSelectionEnd = mRecapitalizeStatus.getNewCursorEnd(); mRecapitalizeStatus.getNewCursorEnd());
mConnection.setSelection(mLastSelectionStart, mLastSelectionEnd);
} }
private void performAdditionToUserHistoryDictionary(final SettingsValues settingsValues, private void performAdditionToUserHistoryDictionary(final SettingsValues settingsValues,
@ -1073,10 +1062,10 @@ public final class InputLogic {
// how to segment them yet. // how to segment them yet.
if (!settingsValues.mSpacingAndPunctuations.mCurrentLanguageHasSpaces) return; if (!settingsValues.mSpacingAndPunctuations.mCurrentLanguageHasSpaces) return;
// If the cursor is not touching a word, or if there is a selection, return right away. // If the cursor is not touching a word, or if there is a selection, return right away.
if (mLastSelectionStart != mLastSelectionEnd) return; if (mConnection.hasSelection()) return;
// If we don't know the cursor location, return. // If we don't know the cursor location, return.
if (mLastSelectionStart < 0) return; if (mConnection.getExpectedSelectionStart() < 0) return;
final int expectedCursorPosition = mLastSelectionStart + offset; // We know Start == End final int expectedCursorPosition = mConnection.getExpectedSelectionStart();
if (!mConnection.isCursorTouchingWord(settingsValues)) return; if (!mConnection.isCursorTouchingWord(settingsValues)) return;
final TextRange range = mConnection.getWordRangeAtCursor( final TextRange range = mConnection.getWordRangeAtCursor(
settingsValues.mSpacingAndPunctuations.mWordSeparators, settingsValues.mSpacingAndPunctuations.mWordSeparators,
@ -1290,7 +1279,8 @@ public final class InputLogic {
public int getCurrentRecapitalizeState() { public int getCurrentRecapitalizeState() {
if (!mRecapitalizeStatus.isActive() if (!mRecapitalizeStatus.isActive()
|| !mRecapitalizeStatus.isSetAt(mLastSelectionStart, mLastSelectionEnd)) { || !mRecapitalizeStatus.isSetAt(mConnection.getExpectedSelectionStart(),
mConnection.getExpectedSelectionEnd())) {
// Not recapitalizing at the moment // Not recapitalizing at the moment
return RecapitalizeStatus.NOT_A_RECAPITALIZE_MODE; return RecapitalizeStatus.NOT_A_RECAPITALIZE_MODE;
} }
@ -1643,7 +1633,8 @@ public final class InputLogic {
// of the auto-correction flash. At this moment, the "typedWord" argument is // of the auto-correction flash. At this moment, the "typedWord" argument is
// ignored by TextView. // ignored by TextView.
mConnection.commitCorrection( mConnection.commitCorrection(
new CorrectionInfo(mLastSelectionEnd - typedWord.length(), new CorrectionInfo(
mConnection.getExpectedSelectionEnd() - typedWord.length(),
typedWord, autoCorrection)); typedWord, autoCorrection));
} }
} }
@ -1691,41 +1682,6 @@ public final class InputLogic {
} }
} }
/**
* 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 = Constants.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)) {
// It should not be possible to have only one of those variables be
// NOT_A_CURSOR_POSITION, so if they are equal, either the selection is zero-sized
// (simple cursor, no selection) or there is no cursor/we don't know its pos
final boolean wasEqual = mLastSelectionStart == mLastSelectionEnd;
mLastSelectionStart = textLength;
// We can't figure out the value of mLastSelectionEnd :(
// But at least if it's smaller than mLastSelectionStart something is wrong,
// and if they used to be equal we also don't want to make it look like there is a
// selection.
if (wasEqual || mLastSelectionStart > mLastSelectionEnd) {
mLastSelectionEnd = mLastSelectionStart;
}
}
}
}
/** /**
* Retry resetting caches in the rich input connection. * Retry resetting caches in the rich input connection.
* *
@ -1743,7 +1699,8 @@ public final class InputLogic {
// TODO: remove these arguments // TODO: remove these arguments
final KeyboardSwitcher keyboardSwitcher, final LatinIME.UIHandler handler) { final KeyboardSwitcher keyboardSwitcher, final LatinIME.UIHandler handler) {
if (!mConnection.resetCachesUponCursorMoveAndReturnSuccess( if (!mConnection.resetCachesUponCursorMoveAndReturnSuccess(
mLastSelectionStart, mLastSelectionEnd, false)) { mConnection.getExpectedSelectionStart(), mConnection.getExpectedSelectionEnd(),
false)) {
if (0 < remainingTries) { if (0 < remainingTries) {
handler.postResetCaches(tryResumeSuggestions, remainingTries - 1); handler.postResetCaches(tryResumeSuggestions, remainingTries - 1);
return; return;
@ -1752,7 +1709,6 @@ public final class InputLogic {
// better to load the keyboard (less things will be broken). // better to load the keyboard (less things will be broken).
} }
mConnection.tryFixLyingCursorPosition(); mConnection.tryFixLyingCursorPosition();
tryFixLyingCursorPosition();
keyboardSwitcher.loadKeyboard(getCurrentInputEditorInfo(), settingsValues); keyboardSwitcher.loadKeyboard(getCurrentInputEditorInfo(), settingsValues);
if (tryResumeSuggestions) { if (tryResumeSuggestions) {
handler.postResumeSuggestions(); handler.postResumeSuggestions();