Fix updating the shift state upon backspace

Bug: 13514349
Change-Id: If4c9db12b0ab5be676f7a2f72715f469066ee537

12 files changed, 99 insertions, 45 deletions

diff --git a/java/src/com/android/inputmethod/event/DeadKeyCombiner.java b/java/src/com/android/inputmethod/event/DeadKeyCombiner.java
index 89e623a1d..bef4d8594 100644
--- a/java/src/com/android/inputmethod/event/DeadKeyCombiner.java
+++ b/java/src/com/android/inputmethod/event/DeadKeyCombiner.java
@@ -52,12 +52,16 @@ public class DeadKeyCombiner implements Combiner {
                 // how dead keys work).
                 // If the event is a space, we should commit the dead char alone, but if it's
                 // not, we need to commit both.
+                // TODO: this is not necessarily triggered by hardware key events, so it's not
+                // a good idea to masquerade as one. This should be typed as a software
+                // composite event or something.
                 return Event.createHardwareKeypressEvent(deadCodePoint, event.mKeyCode,
-                        Constants.CODE_SPACE == event.mCodePoint ? null : event /* next */);
+                        Constants.CODE_SPACE == event.mCodePoint ? null : event /* next */,
+                        false /* isKeyRepeat */);
             } else {
                 // We could combine the characters.
                 return Event.createHardwareKeypressEvent(resultingCodePoint, event.mKeyCode,
-                        null /* next */);
+                        null /* next */, false /* isKeyRepeat */);
             }
         }
     }
diff --git a/java/src/com/android/inputmethod/event/Event.java b/java/src/com/android/inputmethod/event/Event.java
index 6535d2d76..4a9163c8e 100644
--- a/java/src/com/android/inputmethod/event/Event.java
+++ b/java/src/com/android/inputmethod/event/Event.java
@@ -65,6 +65,8 @@ public class Event {
     // This event is a dead character, usually input by a dead key. Examples include dead-acute
     // or dead-abovering.
     final private static int FLAG_DEAD = 0x1;
+    // This event is coming from a key repeat, software or hardware.
+    final private static int FLAG_REPEAT = 0x2;
 
     final private int mEventType; // The type of event - one of the constants above
     // The code point associated with the event, if relevant. This is a unicode code point, and
@@ -130,16 +132,16 @@ public class Event {
     }
 
     public static Event createSoftwareKeypressEvent(final int codePoint, final int keyCode,
-            final int x, final int y) {
+            final int x, final int y, final boolean isKeyRepeat) {
         return new Event(EVENT_TYPE_INPUT_KEYPRESS, null /* text */, codePoint, keyCode, x, y,
-                null /* suggestedWordInfo */, FLAG_NONE, null /* next */);
+                null /* suggestedWordInfo */, isKeyRepeat ? FLAG_REPEAT : FLAG_NONE, null);
     }
 
     public static Event createHardwareKeypressEvent(final int codePoint, final int keyCode,
-            final Event next) {
+            final Event next, final boolean isKeyRepeat) {
         return new Event(EVENT_TYPE_INPUT_KEYPRESS, null /* text */, codePoint, keyCode,
                 Constants.EXTERNAL_KEYBOARD_COORDINATE, Constants.EXTERNAL_KEYBOARD_COORDINATE,
-                null /* suggestedWordInfo */, FLAG_NONE, next);
+                null /* suggestedWordInfo */, isKeyRepeat ? FLAG_REPEAT : FLAG_NONE, next);
     }
 
     // This creates an input event for a dead character. @see {@link #FLAG_DEAD}
@@ -228,6 +230,10 @@ public class Event {
         return 0 != (FLAG_DEAD & mFlags);
     }
 
+    public boolean isKeyRepeat() {
+        return 0 != (FLAG_REPEAT & mFlags);
+    }
+
     // Returns whether this is a fake key press from the suggestion strip. This happens with
     // punctuation signs selected from the suggestion strip.
     public boolean isSuggestionStripPress() {
diff --git a/java/src/com/android/inputmethod/event/HardwareKeyboardEventDecoder.java b/java/src/com/android/inputmethod/event/HardwareKeyboardEventDecoder.java
index da6780e08..05ba99923 100644
--- a/java/src/com/android/inputmethod/event/HardwareKeyboardEventDecoder.java
+++ b/java/src/com/android/inputmethod/event/HardwareKeyboardEventDecoder.java
@@ -46,9 +46,10 @@ public class HardwareKeyboardEventDecoder implements HardwareEventDecoder {
         // do not necessarily map to a unicode character. This represents a physical key, like
         // the key for 'A' or Space, but also Backspace or Ctrl or Caps Lock.
         final int keyCode = keyEvent.getKeyCode();
+        final boolean isKeyRepeat = (0 != keyEvent.getRepeatCount());
         if (KeyEvent.KEYCODE_DEL == keyCode) {
             return Event.createHardwareKeypressEvent(Event.NOT_A_CODE_POINT, Constants.CODE_DELETE,
-                    null /* next */);
+                    null /* next */, isKeyRepeat);
         }
         if (keyEvent.isPrintingKey() || KeyEvent.KEYCODE_SPACE == keyCode
                 || KeyEvent.KEYCODE_ENTER == keyCode) {
@@ -65,15 +66,16 @@ public class HardwareKeyboardEventDecoder implements HardwareEventDecoder {
                 // Latin IME decide what to do with it.
                 if (keyEvent.isShiftPressed()) {
                     return Event.createHardwareKeypressEvent(Event.NOT_A_CODE_POINT,
-                            Constants.CODE_SHIFT_ENTER, null /* next */);
+                            Constants.CODE_SHIFT_ENTER, null /* next */, isKeyRepeat);
                 } else {
                     return Event.createHardwareKeypressEvent(Constants.CODE_ENTER, keyCode,
-                            null /* next */);
+                            null /* next */, isKeyRepeat);
                 }
             }
             // If not Enter, then this is just a regular keypress event for a normal character
             // that can be committed right away, taking into account the current state.
-            return Event.createHardwareKeypressEvent(keyCode, codePointAndFlags, null /* next */);
+            return Event.createHardwareKeypressEvent(keyCode, codePointAndFlags, null /* next */,
+                    isKeyRepeat);
         }
         return Event.createNotHandledEvent();
     }
diff --git a/java/src/com/android/inputmethod/keyboard/EmojiPalettesView.java b/java/src/com/android/inputmethod/keyboard/EmojiPalettesView.java
index 4edc61690..8822ccec9 100644
--- a/java/src/com/android/inputmethod/keyboard/EmojiPalettesView.java
+++ b/java/src/com/android/inputmethod/keyboard/EmojiPalettesView.java
@@ -606,7 +606,8 @@ public final class EmojiPalettesView extends LinearLayout implements OnTabChange
             return;
         }
         final int code = (Integer) tag;
-        mKeyboardActionListener.onCodeInput(code, NOT_A_COORDINATE, NOT_A_COORDINATE);
+        mKeyboardActionListener.onCodeInput(code, NOT_A_COORDINATE, NOT_A_COORDINATE,
+                false /* isKeyRepeat */);
         mKeyboardActionListener.onReleaseKey(code, false /* withSliding */);
     }
 
@@ -634,7 +635,8 @@ public final class EmojiPalettesView extends LinearLayout implements OnTabChange
         if (code == Constants.CODE_OUTPUT_TEXT) {
             mKeyboardActionListener.onTextInput(key.getOutputText());
         } else {
-            mKeyboardActionListener.onCodeInput(code, NOT_A_COORDINATE, NOT_A_COORDINATE);
+            mKeyboardActionListener.onCodeInput(code, NOT_A_COORDINATE, NOT_A_COORDINATE,
+                    false /* isKeyRepeat */);
         }
         mKeyboardActionListener.onReleaseKey(code, false /* withSliding */);
     }
@@ -901,8 +903,8 @@ public final class EmojiPalettesView extends LinearLayout implements OnTabChange
         }
 
         private void handleKeyUp() {
-            mKeyboardActionListener.onCodeInput(
-                    Constants.CODE_DELETE, NOT_A_COORDINATE, NOT_A_COORDINATE);
+            mKeyboardActionListener.onCodeInput(Constants.CODE_DELETE,
+                    NOT_A_COORDINATE, NOT_A_COORDINATE, false /* isKeyRepeat */);
             mKeyboardActionListener.onReleaseKey(
                     Constants.CODE_DELETE, false /* withSliding */);
             ++mRepeatCount;
diff --git a/java/src/com/android/inputmethod/keyboard/KeyboardActionListener.java b/java/src/com/android/inputmethod/keyboard/KeyboardActionListener.java
index dc760e685..c565866b7 100644
--- a/java/src/com/android/inputmethod/keyboard/KeyboardActionListener.java
+++ b/java/src/com/android/inputmethod/keyboard/KeyboardActionListener.java
@@ -53,8 +53,10 @@ public interface KeyboardActionListener {
      *            {@link PointerTracker} or so, the value should be
      *            {@link Constants#NOT_A_COORDINATE}.If it's called on insertion from the
      *            suggestion strip, it should be {@link Constants#SUGGESTION_STRIP_COORDINATE}.
+     * @param isKeyRepeat true if this is a key repeat, false otherwise
      */
-    public void onCodeInput(int primaryCode, int x, int y);
+    // TODO: change this to send an Event object instead
+    public void onCodeInput(int primaryCode, int x, int y, boolean isKeyRepeat);
 
     /**
      * Sends a string of characters to the listener.
@@ -107,7 +109,7 @@ public interface KeyboardActionListener {
         @Override
         public void onReleaseKey(int primaryCode, boolean withSliding) {}
         @Override
-        public void onCodeInput(int primaryCode, int x, int y) {}
+        public void onCodeInput(int primaryCode, int x, int y, boolean isKeyRepeat) {}
         @Override
         public void onTextInput(String text) {}
         @Override
diff --git a/java/src/com/android/inputmethod/keyboard/MainKeyboardView.java b/java/src/com/android/inputmethod/keyboard/MainKeyboardView.java
index d8dd93a35..1cafd4162 100644
--- a/java/src/com/android/inputmethod/keyboard/MainKeyboardView.java
+++ b/java/src/com/android/inputmethod/keyboard/MainKeyboardView.java
@@ -626,8 +626,8 @@ public final class MainKeyboardView extends KeyboardView implements PointerTrack
             final int moreKeyCode = key.getMoreKeys()[0].mCode;
             tracker.onLongPressed();
             listener.onPressKey(moreKeyCode, 0 /* repeatCount */, true /* isSinglePointer */);
-            listener.onCodeInput(moreKeyCode,
-                    Constants.NOT_A_COORDINATE, Constants.NOT_A_COORDINATE);
+            listener.onCodeInput(moreKeyCode, Constants.NOT_A_COORDINATE,
+                    Constants.NOT_A_COORDINATE, false /* isKeyRepeat */);
             listener.onReleaseKey(moreKeyCode, false /* withSliding */);
             return;
         }
diff --git a/java/src/com/android/inputmethod/keyboard/MoreKeysKeyboardView.java b/java/src/com/android/inputmethod/keyboard/MoreKeysKeyboardView.java
index fc331970b..65242dd76 100644
--- a/java/src/com/android/inputmethod/keyboard/MoreKeysKeyboardView.java
+++ b/java/src/com/android/inputmethod/keyboard/MoreKeysKeyboardView.java
@@ -143,9 +143,10 @@ public class MoreKeysKeyboardView extends KeyboardView implements MoreKeysPanel
             mListener.onTextInput(mCurrentKey.getOutputText());
         } else if (code != Constants.CODE_UNSPECIFIED) {
             if (getKeyboard().hasProximityCharsCorrection(code)) {
-                mListener.onCodeInput(code, x, y);
+                mListener.onCodeInput(code, x, y, false /* isKeyRepeat */);
             } else {
-                mListener.onCodeInput(code, Constants.NOT_A_COORDINATE, Constants.NOT_A_COORDINATE);
+                mListener.onCodeInput(code, Constants.NOT_A_COORDINATE, Constants.NOT_A_COORDINATE,
+                        false /* isKeyRepeat */);
             }
         }
     }
diff --git a/java/src/com/android/inputmethod/keyboard/PointerTracker.java b/java/src/com/android/inputmethod/keyboard/PointerTracker.java
index 3539a874c..4777166ea 100644
--- a/java/src/com/android/inputmethod/keyboard/PointerTracker.java
+++ b/java/src/com/android/inputmethod/keyboard/PointerTracker.java
@@ -325,7 +325,7 @@ public final class PointerTracker implements PointerTrackerQueue.Element,
     // Note that we need primaryCode argument because the keyboard may in shifted state and the
     // primaryCode is different from {@link Key#mKeyCode}.
     private void callListenerOnCodeInput(final Key key, final int primaryCode, final int x,
-            final int y, final long eventTime) {
+            final int y, final long eventTime, final boolean isKeyRepeat) {
         final boolean ignoreModifierKey = mIsInDraggingFinger && key.isModifier();
         final boolean altersCode = key.altCodeWhileTyping() && sTimerProxy.isTypingState();
         final int code = altersCode ? key.getAltCode() : primaryCode;
@@ -350,10 +350,10 @@ public final class PointerTracker implements PointerTrackerQueue.Element,
                 sListener.onTextInput(key.getOutputText());
             } else if (code != Constants.CODE_UNSPECIFIED) {
                 if (mKeyboard.hasProximityCharsCorrection(code)) {
-                    sListener.onCodeInput(code, x, y);
+                    sListener.onCodeInput(code, x, y, isKeyRepeat);
                 } else {
                     sListener.onCodeInput(code,
-                            Constants.NOT_A_COORDINATE, Constants.NOT_A_COORDINATE);
+                            Constants.NOT_A_COORDINATE, Constants.NOT_A_COORDINATE, isKeyRepeat);
                 }
             }
         }
@@ -1204,7 +1204,7 @@ public final class PointerTracker implements PointerTrackerQueue.Element,
         }
 
         final int code = key.getCode();
-        callListenerOnCodeInput(key, code, x, y, eventTime);
+        callListenerOnCodeInput(key, code, x, y, eventTime, false /* isKeyRepeat */);
         callListenerOnRelease(key, code, false /* withSliding */);
     }
 
@@ -1229,7 +1229,8 @@ public final class PointerTracker implements PointerTrackerQueue.Element,
         final int nextRepeatCount = repeatCount + 1;
         startKeyRepeatTimer(nextRepeatCount);
         callListenerOnPressAndCheckKeyboardLayoutChange(key, repeatCount);
-        callListenerOnCodeInput(key, code, mKeyX, mKeyY, SystemClock.uptimeMillis());
+        callListenerOnCodeInput(key, code, mKeyX, mKeyY, SystemClock.uptimeMillis(),
+                true /* isKeyRepeat */);
     }
 
     private void startKeyRepeatTimer(final int repeatCount) {
diff --git a/java/src/com/android/inputmethod/latin/LatinIME.java b/java/src/com/android/inputmethod/latin/LatinIME.java
index 84558ca24..0db247c4c 100644
--- a/java/src/com/android/inputmethod/latin/LatinIME.java
+++ b/java/src/com/android/inputmethod/latin/LatinIME.java
@@ -1224,7 +1224,8 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen
 
     // Implementation of {@link KeyboardActionListener}.
     @Override
-    public void onCodeInput(final int codePoint, final int x, final int y) {
+    public void onCodeInput(final int codePoint, final int x, final int y,
+            final boolean isKeyRepeat) {
         final MainKeyboardView mainKeyboardView = mKeyboardSwitcher.getMainKeyboardView();
         // x and y include some padding, but everything down the line (especially native
         // code) needs the coordinates in the keyboard frame.
@@ -1250,7 +1251,7 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen
             mSubtypeSwitcher.switchToShortcutIME(this);
             // Still call the *#onCodeInput methods for readability.
         }
-        final Event event = createSoftwareKeypressEvent(codeToSend, keyX, keyY);
+        final Event event = createSoftwareKeypressEvent(codeToSend, keyX, keyY, isKeyRepeat);
         final InputTransaction completeInputTransaction =
                 mInputLogic.onCodeInput(mSettings.getCurrent(), event,
                         mKeyboardSwitcher.getKeyboardShiftMode(), mHandler);
@@ -1261,7 +1262,7 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen
     // A helper method to split the code point and the key code. Ultimately, they should not be
     // squashed into the same variable, and this method should be removed.
     private static Event createSoftwareKeypressEvent(final int keyCodeOrCodePoint, final int keyX,
-             final int keyY) {
+             final int keyY, final boolean isKeyRepeat) {
         final int keyCode;
         final int codePoint;
         if (keyCodeOrCodePoint <= 0) {
@@ -1271,7 +1272,7 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen
             keyCode = Event.NOT_A_KEY_CODE;
             codePoint = keyCodeOrCodePoint;
         }
-        return Event.createSoftwareKeypressEvent(codePoint, keyCode, keyX, keyY);
+        return Event.createSoftwareKeypressEvent(codePoint, keyCode, keyX, keyY, isKeyRepeat);
     }
 
     // Called from PointerTracker through the KeyboardActionListener interface
diff --git a/java/src/com/android/inputmethod/latin/inputlogic/InputLogic.java b/java/src/com/android/inputmethod/latin/inputlogic/InputLogic.java
index bf8467eb6..491d98074 100644
--- a/java/src/com/android/inputmethod/latin/inputlogic/InputLogic.java
+++ b/java/src/com/android/inputmethod/latin/inputlogic/InputLogic.java
@@ -884,10 +884,17 @@ public final class InputLogic {
         mSpaceState = SpaceState.NONE;
         mDeleteCount++;
 
-        // In many cases, we may have to put the keyboard in auto-shift state again. However
-        // we want to wait a few milliseconds before doing it to avoid the keyboard flashing
-        // during key repeat.
-        inputTransaction.requireShiftUpdate(InputTransaction.SHIFT_UPDATE_LATER);
+        // In many cases after backspace, we need to update the shift state. Normally we need
+        // to do this right away to avoid the shift state being out of date in case the user types
+        // backspace then some other character very fast. However, in the case of backspace key
+        // repeat, this can lead to flashiness when the cursor flies over positions where the
+        // shift state should be updated, so if this is a key repeat, we update after a small delay.
+        // Then again, even in the case of a key repeat, if the cursor is at start of text, it
+        // can't go any further back, so we can update right away even if it's a key repeat.
+        final int shiftUpdateKind =
+                inputTransaction.mEvent.isKeyRepeat() && mConnection.getExpectedSelectionStart() > 0
+                ? InputTransaction.SHIFT_UPDATE_LATER : InputTransaction.SHIFT_UPDATE_NOW;
+        inputTransaction.requireShiftUpdate(shiftUpdateKind);
 
         if (mWordComposer.isCursorFrontOrMiddleOfComposingWord()) {
             // If we are in the middle of a recorrection, we need to commit the recorrection
@@ -910,11 +917,6 @@ public final class InputLogic {
             }
             mConnection.setComposingText(getTextWithUnderline(mWordComposer.getTypedWord()), 1);
             inputTransaction.setRequiresUpdateSuggestions();
-            if (!mWordComposer.isComposingWord()) {
-                // If we just removed the last character, auto-caps mode may have changed so we
-                // need to re-evaluate.
-                inputTransaction.requireShiftUpdate(InputTransaction.SHIFT_UPDATE_NOW);
-            }
         } else {
             if (mLastComposedWord.canRevertCommit()) {
                 if (inputTransaction.mSettingsValues.mIsInternal) {
@@ -1025,8 +1027,6 @@ public final class InputLogic {
                 restartSuggestionsOnWordTouchedByCursor(inputTransaction.mSettingsValues,
                         true /* includeResumedWordInSuggestions */);
             }
-            // We just removed at least one character. We need to update the auto-caps state.
-            inputTransaction.requireShiftUpdate(InputTransaction.SHIFT_UPDATE_NOW);
         }
     }
 
diff --git a/tests/src/com/android/inputmethod/latin/InputTestsBase.java b/tests/src/com/android/inputmethod/latin/InputTestsBase.java
index e5f111ab6..260e534ee 100644
--- a/tests/src/com/android/inputmethod/latin/InputTestsBase.java
+++ b/tests/src/com/android/inputmethod/latin/InputTestsBase.java
@@ -254,7 +254,7 @@ public class InputTestsBase extends ServiceTestCase<LatinIMEForTests> {
     }
 
     // type(int) and type(String): helper methods to send a code point resp. a string to LatinIME.
-    protected void type(final int codePoint) {
+    protected void typeInternal(final int codePoint, final boolean isKeyRepeat) {
         // onPressKey and onReleaseKey are explicitly deactivated here, but they do happen in the
         // code (although multitouch/slide input and other factors make the sequencing complicated).
         // They are supposed to be entirely deconnected from the input logic from LatinIME point of
@@ -263,16 +263,26 @@ public class InputTestsBase extends ServiceTestCase<LatinIMEForTests> {
         // but keep them in mind if something breaks. Commenting them out as is should work.
         //mLatinIME.onPressKey(codePoint, 0 /* repeatCount */, true /* isSinglePointer */);
         final Key key = mKeyboard.getKey(codePoint);
-        if (key != null) {
+        if (key == null) {
+            mLatinIME.onCodeInput(codePoint, Constants.NOT_A_COORDINATE, Constants.NOT_A_COORDINATE,
+                    isKeyRepeat);
+        } else {
             final int x = key.getX() + key.getWidth() / 2;
             final int y = key.getY() + key.getHeight() / 2;
-            mLatinIME.onCodeInput(codePoint, x, y);
-            return;
+            mLatinIME.onCodeInput(codePoint, x, y, isKeyRepeat);
         }
-        mLatinIME.onCodeInput(codePoint, Constants.NOT_A_COORDINATE, Constants.NOT_A_COORDINATE);
+        // Also see the comment at the top of this function about onReleaseKey
         //mLatinIME.onReleaseKey(codePoint, false /* withSliding */);
     }
 
+    protected void type(final int codePoint) {
+        typeInternal(codePoint, false /* isKeyRepeat */);
+    }
+
+    protected void repeatKey(final int codePoint) {
+        typeInternal(codePoint, true /* isKeyRepeat */);
+    }
+
     protected void type(final String stringToType) {
         for (int i = 0; i < stringToType.length(); i = stringToType.offsetByCodePoints(i, 1)) {
             type(stringToType.codePointAt(i));
diff --git a/tests/src/com/android/inputmethod/latin/ShiftModeTests.java b/tests/src/com/android/inputmethod/latin/ShiftModeTests.java
index 806fad060..6fc9df793 100644
--- a/tests/src/com/android/inputmethod/latin/ShiftModeTests.java
+++ b/tests/src/com/android/inputmethod/latin/ShiftModeTests.java
@@ -53,4 +53,29 @@ public class ShiftModeTests extends InputTestsBase {
         type(" ");
         assertTrue("Caps after period space", isCapsModeAutoShifted());
     }
+
+    public void testBackspace() {
+        assertTrue("Initial auto caps state", isCapsModeAutoShifted());
+        type("A");
+        assertFalse("Caps state after one letter", isCapsModeAutoShifted());
+        type(Constants.CODE_DELETE);
+        assertTrue("Auto caps state at start after delete", isCapsModeAutoShifted());
+    }
+
+    public void testRepeatingBackspace() {
+        final String SENTENCE_TO_TYPE = "Test sentence. Another.";
+        final int BACKSPACE_COUNT =
+                SENTENCE_TO_TYPE.length() - SENTENCE_TO_TYPE.lastIndexOf(' ') - 1;
+
+        type(SENTENCE_TO_TYPE);
+        assertFalse("Caps after typing \"" + SENTENCE_TO_TYPE + "\"", isCapsModeAutoShifted());
+        type(Constants.CODE_DELETE);
+        for (int i = 1; i < BACKSPACE_COUNT; ++i) {
+            repeatKey(Constants.CODE_DELETE);
+        }
+        assertFalse("Caps immediately after repeating Backspace a lot", isCapsModeAutoShifted());
+        sleep(DELAY_TO_WAIT_FOR_PREDICTIONS);
+        runMessages();
+        assertTrue("Caps after a while after repeating Backspace a lot", isCapsModeAutoShifted());
+    }
 }