Send the real key code in deleteSurroundingText

On android, when user presses DEL or FORWARD_DEL using IME, Chrome generates key events with 0 keycode. This breaks lots of websites who count on having the real keycode in the key event listeners, especially for the control keys such as DEL and FORWARD_DEL.

This CL fixes the issue for the most common case when a single char is deleted. Instead of triggering deleteSurroudingText path with synthetic key events of key code 0, Chrome sends native key events with real key code.

BUG=118639

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@283989 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 95 insertions, 11 deletions

diff --git a/content/public/android/java/src/org/chromium/content/browser/input/AdapterInputConnection.java b/content/public/android/java/src/org/chromium/content/browser/input/AdapterInputConnection.java
index 9352f37..de9f9cc 100644
--- a/content/public/android/java/src/org/chromium/content/browser/input/AdapterInputConnection.java
+++ b/content/public/android/java/src/org/chromium/content/browser/input/AdapterInputConnection.java
@@ -318,13 +318,36 @@ public class AdapterInputConnection extends BaseInputConnection {
         if (DEBUG) {
             Log.w(TAG, "deleteSurroundingText [" + beforeLength + " " + afterLength + "]");
         }
+        int originalBeforeLength = beforeLength;
+        int originalAfterLength = afterLength;
         int availableBefore = Selection.getSelectionStart(mEditable);
         int availableAfter = mEditable.length() - Selection.getSelectionEnd(mEditable);
         beforeLength = Math.min(beforeLength, availableBefore);
         afterLength = Math.min(afterLength, availableAfter);
         super.deleteSurroundingText(beforeLength, afterLength);
         updateSelectionIfRequired();
-        return mImeAdapter.deleteSurroundingText(beforeLength, afterLength);
+
+        // 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.
+        int keyCode = KeyEvent.KEYCODE_UNKNOWN;
+        if (originalBeforeLength == 1 && originalAfterLength == 0)
+            keyCode = KeyEvent.KEYCODE_DEL;
+        else if (originalBeforeLength == 0 && originalAfterLength == 1)
+            keyCode = KeyEvent.KEYCODE_FORWARD_DEL;
+
+        boolean result = true;
+        if (keyCode == KeyEvent.KEYCODE_UNKNOWN) {
+            result = mImeAdapter.sendSyntheticKeyEvent(
+                    ImeAdapter.sEventTypeRawKeyDown, SystemClock.uptimeMillis(), keyCode, 0);
+            result &= mImeAdapter.deleteSurroundingText(beforeLength, afterLength);
+            result &= mImeAdapter.sendSyntheticKeyEvent(
+                    ImeAdapter.sEventTypeKeyUp, SystemClock.uptimeMillis(), keyCode, 0);
+        } else {
+            mImeAdapter.sendKeyEventWithKeyCode(
+                    keyCode, KeyEvent.FLAG_SOFT_KEYBOARD | KeyEvent.FLAG_KEEP_TOUCH_MODE);
+        }
+        return result;
     }
 
     /**
diff --git a/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java b/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java
index 73735e0..f515057 100644
--- a/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java
+++ b/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java
@@ -408,15 +408,7 @@ public class ImeAdapter {
     boolean deleteSurroundingText(int beforeLength, int afterLength) {
         mViewEmbedder.onImeEvent(false);
         if (mNativeImeAdapterAndroid == 0) return false;
-        // Can't send the deletion key code yet because it will delete an extra char at the end.
-        // Also the deleteSurroundingText message is not always ordered properly with key event
-        // messages yet.
-        // TODO(guohui): fix the ordering and send the deletion key code for single-char deletion.
-        sendSyntheticKeyEvent(
-                sEventTypeRawKeyDown, SystemClock.uptimeMillis(), KeyEvent.KEYCODE_UNKNOWN, 0);
         nativeDeleteSurroundingText(mNativeImeAdapterAndroid, beforeLength, afterLength);
-        sendSyntheticKeyEvent(
-                sEventTypeKeyUp, SystemClock.uptimeMillis(), KeyEvent.KEYCODE_UNKNOWN, 0);
         return true;
     }
 
diff --git a/content/public/android/javatests/src/org/chromium/content/browser/input/AdapterInputConnectionTest.java b/content/public/android/javatests/src/org/chromium/content/browser/input/AdapterInputConnectionTest.java
index 83e22fe..a57f5ec 100644
--- a/content/public/android/javatests/src/org/chromium/content/browser/input/AdapterInputConnectionTest.java
+++ b/content/public/android/javatests/src/org/chromium/content/browser/input/AdapterInputConnectionTest.java
@@ -9,6 +9,8 @@ import android.os.IBinder;
 import android.os.ResultReceiver;
 import android.test.suitebuilder.annotation.MediumTest;
 import android.text.Editable;
+import android.text.Selection;
+import android.view.KeyEvent;
 import android.view.View;
 import android.view.inputmethod.EditorInfo;
 
@@ -27,6 +29,7 @@ public class AdapterInputConnectionTest extends ContentShellTestBase {
     private AdapterInputConnection mConnection;
     private TestInputMethodManagerWrapper mWrapper;
     private Editable mEditable;
+    private TestImeAdapter mImeAdapter;
 
     @Override
     public void setUp() throws Exception {
@@ -35,11 +38,11 @@ public class AdapterInputConnectionTest extends ContentShellTestBase {
         assertTrue("Page failed to load", waitForActiveShellToBeDoneLoading());
         mWrapper = new TestInputMethodManagerWrapper(getActivity());
         ImeAdapterDelegate delegate = new TestImeAdapterDelegate();
-        ImeAdapter imeAdapter = new TestImeAdapter(mWrapper, delegate);
+        mImeAdapter = new TestImeAdapter(mWrapper, delegate);
         EditorInfo info = new EditorInfo();
         mEditable = Editable.Factory.getInstance().newEditable("");
         mConnection = new AdapterInputConnection(
-                getContentViewCore().getContainerView(), imeAdapter, mEditable, info);
+                getContentViewCore().getContainerView(), mImeAdapter, mEditable, info);
     }
 
     @MediumTest
@@ -82,10 +85,59 @@ public class AdapterInputConnectionTest extends ContentShellTestBase {
         mWrapper.verifyUpdateSelectionCall(0, 4, 4, 0 ,4);
     }
 
+    @MediumTest
+    @Feature({"TextInput", "Main"})
+    public void testDeleteSurroundingText() throws Throwable {
+        // Tests back deletion of a single character with empty input.
+        mConnection.deleteSurroundingText(1, 0);
+        assertEquals(0, mImeAdapter.getDeleteSurroundingTextCallCount());
+        Integer[] keyEvents = mImeAdapter.getKeyEvents();
+        assertEquals(1, keyEvents.length);
+        assertEquals(KeyEvent.KEYCODE_DEL, keyEvents[0].intValue());
+
+        // Tests forward deletion of a single character with non-empty input.
+        mEditable.replace(0, mEditable.length(), " hello");
+        Selection.setSelection(mEditable, 0, 0);
+        mConnection.deleteSurroundingText(0, 1);
+        assertEquals(0, mImeAdapter.getDeleteSurroundingTextCallCount());
+        keyEvents = mImeAdapter.getKeyEvents();
+        assertEquals(2, keyEvents.length);
+        assertEquals(KeyEvent.KEYCODE_FORWARD_DEL, keyEvents[1].intValue());
+
+        // Tests back deletion of multiple characters with non-empty input.
+        mEditable.replace(0, mEditable.length(), "hello ");
+        Selection.setSelection(mEditable, mEditable.length(), mEditable.length());
+        mConnection.deleteSurroundingText(2, 0);
+        assertEquals(1, mImeAdapter.getDeleteSurroundingTextCallCount());
+        assertEquals(2, mImeAdapter.getKeyEvents().length);
+    }
+
     private static class TestImeAdapter extends ImeAdapter {
+        private final ArrayList<Integer> mKeyEventQueue = new ArrayList<Integer>();
+        private int mDeleteSurroundingTextCounter;
+
         public TestImeAdapter(InputMethodManagerWrapper wrapper, ImeAdapterDelegate embedder) {
             super(wrapper, embedder);
         }
+
+        @Override
+        public boolean deleteSurroundingText(int beforeLength, int afterLength) {
+            ++mDeleteSurroundingTextCounter;
+            return true;
+        }
+
+        @Override
+        public void sendKeyEventWithKeyCode(int keyCode, int flags) {
+            mKeyEventQueue.add(keyCode);
+        }
+
+        public int getDeleteSurroundingTextCallCount() {
+            return mDeleteSurroundingTextCounter;
+        }
+
+        public Integer[] getKeyEvents() {
+            return mKeyEventQueue.toArray(new Integer[mKeyEventQueue.size()]);
+        }
     }
 
     private static class TestInputMethodManagerWrapper extends InputMethodManagerWrapper {
diff --git a/content/renderer/render_widget.cc b/content/renderer/render_widget.cc
index 85e4286..2336840 100644
--- a/content/renderer/render_widget.cc
+++ b/content/renderer/render_widget.cc
@@ -913,6 +913,23 @@ void RenderWidget::OnHandleInputEvent(const blink::WebInputEvent* input_event,
     return;
   base::AutoReset<WebInputEvent::Type> handling_event_type_resetter(
       &handling_event_type_, input_event->type);
+#if defined(OS_ANDROID)
+  // On Android, when the delete key or forward delete key is pressed using IME,
+  // |AdapterInputConnection| generates input key events to make sure all JS
+  // listeners that monitor KeyUp and KeyDown events receive the proper key
+  // code. Since this input key event comes from IME, we need to set the
+  // IME event guard here to make sure it does not interfere with other IME
+  // events.
+  scoped_ptr<ImeEventGuard> ime_event_guard_maybe;
+  if (WebInputEvent::isKeyboardEventType(input_event->type)) {
+    const WebKeyboardEvent& key_event =
+        *static_cast<const WebKeyboardEvent*>(input_event);
+    if (key_event.nativeKeyCode == AKEYCODE_FORWARD_DEL ||
+        key_event.nativeKeyCode == AKEYCODE_DEL) {
+      ime_event_guard_maybe.reset(new ImeEventGuard(this));
+    }
+  }
+#endif
 
   base::AutoReset<const ui::LatencyInfo*> resetter(&current_event_latency_info_,
                                                    &latency_info);