Deal with backspace keycodes getting sent by IME during compositions.

As of KitKat, the standard Google IME always calls
deleteSurroundingText(1, 0) when the soft backspace button is pressed.
But Jellybean and below instead mixed in a backspace key event into the
IME stream, and the current Samsung keyboard still has this behavior.
If we only forward such key events to Blink during an ongoing
composition, then Blink will delete its string but the
BaseInputConnection won't update its state, leading to a desync
and strange bugs later.

Our sendKeyEvent method already had a workaround to call
deleteSurroundingText directly in such cases. sendKeyEvent is called
when Samsung keyboard is used with Chrome, but WebView instead goes
through the alternate Chrome.dispatchKeyEvent path.  For that reason we
currently have a Samsung+WebView specific desync bug.

This patch changes dispatchKeyEvent to call sendKeyEvent, like
https://codereview.chromium.org/759033002 did before it was reverted, to
extend the workaround to WebView.  This would have the unfortunate side
effect of causing a bug with backspace key-repeats on physical
keyboards, as the code there only applies backspace on keyup.  So this
patch changes it to keydown (which is better UX even aside from
key repeat) and also forwards the system-supplied key events directly
instead of throwing them out and generating artificial ones.

BUG=483514

Review URL: https://codereview.chromium.org/1137673005

Cr-Commit-Position: refs/heads/master@{#329954}

content/public/android/java/src/org/chromium/content/browser/input/AdapterInputConnection.java

@@ -343,6 +343,11 @@ public class AdapterInputConnection extends BaseInputConnection {
      */
     @Override
     public boolean deleteSurroundingText(int beforeLength, int afterLength) {
+        return deleteSurroundingTextImpl(beforeLength, afterLength, false);
+    }
+
+    private boolean deleteSurroundingTextImpl(
+            int beforeLength, int afterLength, boolean fromPhysicalKey) {
         if (DEBUG) {
             Log.w(TAG, "deleteSurroundingText [" + beforeLength + " " + afterLength + "]");
         }
@@ -355,6 +360,12 @@ public class AdapterInputConnection extends BaseInputConnection {
         super.deleteSurroundingText(beforeLength, afterLength);
         updateSelectionIfRequired();
 
+        // If this was called due to a physical key, no need to generate a key event here as
+        // the caller will take care of forwarding the original.
+        if (fromPhysicalKey) {
+            return true;
+        }
+
         // For single-char deletion calls |ImeAdapter.sendKeyEventWithKeyCode| with the real key
         // code. For multi-character deletion, executes deletion by calling
         // |ImeAdapter.deleteSurroundingText| and sends synthetic key events with a dummy key code.
@@ -387,43 +398,37 @@ public class AdapterInputConnection extends BaseInputConnection {
         if (DEBUG) {
             Log.w(TAG, "sendKeyEvent [" + event.getAction() + "] [" + event.getKeyCode() + "]");
         }
-        // If this is a key-up, and backspace/del or if the key has a character representation,
+
+        int action = event.getAction();
+        int keycode = event.getKeyCode();
+        int unicodeChar = event.getUnicodeChar();
+
+        // If this is backspace/del or if the key has a character representation,
         // need to update the underlying Editable (i.e. the local representation of the text
-        // being edited).
-        if (event.getAction() == KeyEvent.ACTION_UP) {
-            if (event.getKeyCode() == KeyEvent.KEYCODE_DEL) {
-                deleteSurroundingText(1, 0);
-                return true;
-            } else if (event.getKeyCode() == KeyEvent.KEYCODE_FORWARD_DEL) {
-                deleteSurroundingText(0, 1);
-                return true;
-            } else {
-                int unicodeChar = event.getUnicodeChar();
-                if (unicodeChar != 0) {
-                    int selectionStart = Selection.getSelectionStart(mEditable);
-                    int selectionEnd = Selection.getSelectionEnd(mEditable);
-                    if (selectionStart > selectionEnd) {
-                        int temp = selectionStart;
-                        selectionStart = selectionEnd;
-                        selectionEnd = temp;
-                    }
-                    mEditable.replace(selectionStart, selectionEnd,
-                            Character.toString((char) unicodeChar));
-                }
-            }
-        } else if (event.getAction() == KeyEvent.ACTION_DOWN) {
+        // being edited).  Some IMEs like Jellybean stock IME and Samsung IME mix in delete
+        // KeyPress events instead of calling deleteSurroundingText.
+        if (action == KeyEvent.ACTION_DOWN && keycode == KeyEvent.KEYCODE_DEL) {
+            deleteSurroundingTextImpl(1, 0, true);
+        } else if (action == KeyEvent.ACTION_DOWN && keycode == KeyEvent.KEYCODE_FORWARD_DEL) {
+            deleteSurroundingTextImpl(0, 1, true);
+        } else if (action == KeyEvent.ACTION_DOWN && keycode == KeyEvent.KEYCODE_ENTER) {
+            // Finish text composition when pressing enter, as that may submit a form field.
             // TODO(aurimas): remove this workaround when crbug.com/278584 is fixed.
-            if (event.getKeyCode() == KeyEvent.KEYCODE_ENTER) {
-                beginBatchEdit();
-                finishComposingText();
-                mImeAdapter.translateAndSendNativeEvents(event);
-                endBatchEdit();
-                return true;
-            } else if (event.getKeyCode() == KeyEvent.KEYCODE_DEL) {
-                return true;
-            } else if (event.getKeyCode() == KeyEvent.KEYCODE_FORWARD_DEL) {
-                return true;
+            beginBatchEdit();
+            finishComposingText();
+            mImeAdapter.translateAndSendNativeEvents(event);
+            endBatchEdit();
+            return true;
+        } else if (action == KeyEvent.ACTION_UP && unicodeChar != 0) {
+            int selectionStart = Selection.getSelectionStart(mEditable);
+            int selectionEnd = Selection.getSelectionEnd(mEditable);
+            if (selectionStart > selectionEnd) {
+                int temp = selectionStart;
+                selectionStart = selectionEnd;
+                selectionEnd = temp;
             }
+            mEditable.replace(selectionStart, selectionEnd,
+                    Character.toString((char) unicodeChar));
         }
         mImeAdapter.translateAndSendNativeEvents(event);
         return true;

content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java

@@ -309,6 +309,9 @@ public class ImeAdapter {
     }
 
     public boolean dispatchKeyEvent(KeyEvent event) {
+        if (mInputConnection != null) {
+            return mInputConnection.sendKeyEvent(event);
+        }
         return translateAndSendNativeEvents(event);
     }
 

content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java

@@ -528,6 +528,96 @@ public class ImeTest extends ContentShellTestBase {
         assertEquals("three word test", mConnection.getTextBeforeCursor(99, 0));
     }
 
+    @SmallTest
+    @Feature({"TextInput", "Main"})
+    public void testBackspaceKeycode() throws Throwable {
+        DOMUtils.focusNode(mWebContents, "textarea");
+        assertWaitForKeyboardStatus(true);
+
+        mConnection = (TestAdapterInputConnection) getAdapterInputConnection();
+        waitAndVerifyEditableCallback(mConnection.mImeUpdateQueue, 0, "", 0, 0, -1, -1);
+
+        // H
+        expectUpdateStateCall(mConnection);
+        commitText(mConnection, "h", 1);
+        assertEquals(KeyEvent.KEYCODE_H, mImeAdapter.mLastSyntheticKeyCode);
+        assertEquals("h", mConnection.getTextBeforeCursor(9, 0));
+        assertUpdateStateCall(mConnection, 1000);
+        assertEquals("h", mConnection.getTextBeforeCursor(9, 0));
+
+        // O
+        expectUpdateStateCall(mConnection);
+        commitText(mConnection, "o", 1);
+        assertEquals(KeyEvent.KEYCODE_O, mImeAdapter.mLastSyntheticKeyCode);
+        assertEquals("ho", mConnection.getTextBeforeCursor(9, 0));
+        assertUpdateStateCall(mConnection, 1000);
+        assertEquals("ho", mConnection.getTextBeforeCursor(9, 0));
+
+        // DEL, sent via dispatchKeyEvent like it is in Android WebView or a physical keyboard.
+        getInstrumentation().runOnMainSync(new Runnable() {
+            @Override
+            public void run() {
+                mContentViewCore.dispatchKeyEvent(
+                        new KeyEvent(KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_DEL));
+                mContentViewCore.dispatchKeyEvent(
+                        new KeyEvent(KeyEvent.ACTION_UP, KeyEvent.KEYCODE_DEL));
+            }
+        });
+
+        // DEL
+        expectUpdateStateCall(mConnection);
+        assertEquals("h", mConnection.getTextBeforeCursor(9, 0));
+        assertUpdateStateCall(mConnection, 1000);
+        assertEquals("h", mConnection.getTextBeforeCursor(9, 0));
+    }
+
+    @SmallTest
+    @Feature({"TextInput", "Main"})
+    public void testRepeatBackspaceKeycode() throws Throwable {
+        DOMUtils.focusNode(mWebContents, "textarea");
+        assertWaitForKeyboardStatus(true);
+
+        mConnection = (TestAdapterInputConnection) getAdapterInputConnection();
+        waitAndVerifyEditableCallback(mConnection.mImeUpdateQueue, 0, "", 0, 0, -1, -1);
+
+        // H
+        expectUpdateStateCall(mConnection);
+        commitText(mConnection, "h", 1);
+        assertEquals(KeyEvent.KEYCODE_H, mImeAdapter.mLastSyntheticKeyCode);
+        assertEquals("h", mConnection.getTextBeforeCursor(9, 0));
+        assertUpdateStateCall(mConnection, 1000);
+        assertEquals("h", mConnection.getTextBeforeCursor(9, 0));
+
+        // O
+        expectUpdateStateCall(mConnection);
+        commitText(mConnection, "o", 1);
+        assertEquals(KeyEvent.KEYCODE_O, mImeAdapter.mLastSyntheticKeyCode);
+        assertEquals("ho", mConnection.getTextBeforeCursor(9, 0));
+        assertUpdateStateCall(mConnection, 1000);
+        assertEquals("ho", mConnection.getTextBeforeCursor(9, 0));
+
+        // Multiple keydowns should each delete one character (this is for physical keyboard
+        // key-repeat).
+        getInstrumentation().runOnMainSync(new Runnable() {
+            @Override
+            public void run() {
+                mContentViewCore.dispatchKeyEvent(
+                        new KeyEvent(KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_DEL));
+                mContentViewCore.dispatchKeyEvent(
+                        new KeyEvent(KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_DEL));
+                mContentViewCore.dispatchKeyEvent(
+                        new KeyEvent(KeyEvent.ACTION_UP, KeyEvent.KEYCODE_DEL));
+            }
+        });
+
+        // DEL
+        expectUpdateStateCall(mConnection);
+        assertEquals("", mConnection.getTextBeforeCursor(9, 0));
+        assertUpdateStateCall(mConnection, 1000);
+        assertEquals("", mConnection.getTextBeforeCursor(9, 0));
+    }
+
+
     @SmallTest
     @Feature({"TextInput", "Main"})
     public void testKeyCodesWhileTypingText() throws Throwable {