Translate physical keyboard accents to IME compositions.

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

Index: content/public/android/java/src/org/chromium/content/browser/input/AdapterInputConnection.java
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 75c8bd841deba1ddb41d9b5db317a682e308232b..6e6239867fcbda8b61ebb3323f946f97c4694ec9 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
@@ -10,6 +10,7 @@ import android.text.InputType;
 import android.text.Selection;
 import android.text.TextUtils;
 import android.util.Log;
+import android.view.KeyCharacterMap;
 import android.view.KeyEvent;
 import android.view.View;
 import android.view.inputmethod.BaseInputConnection;
@@ -42,6 +43,7 @@ public class AdapterInputConnection extends BaseInputConnection {
 
     private boolean mSingleLine;
     private int mNumNestedBatchEdits = 0;
+    private int mPendingAccent;
 
     private int mLastUpdateSelectionStart = INVALID_SELECTION;
     private int mLastUpdateSelectionEnd = INVALID_SELECTION;
@@ -242,6 +244,7 @@ public class AdapterInputConnection extends BaseInputConnection {
     public boolean setComposingText(CharSequence text, int newCursorPosition) {
         if (DEBUG) Log.w(TAG, "setComposingText [" + text + "] [" + newCursorPosition + "]");
         if (maybePerformEmptyCompositionWorkaround(text)) return true;
+        mPendingAccent = 0;
         super.setComposingText(text, newCursorPosition);
         updateSelectionIfRequired();
         return mImeAdapter.checkCompositionQueueAndCallNative(text, newCursorPosition, false);
@@ -254,6 +257,7 @@ public class AdapterInputConnection extends BaseInputConnection {
     public boolean commitText(CharSequence text, int newCursorPosition) {
         if (DEBUG) Log.w(TAG, "commitText [" + text + "] [" + newCursorPosition + "]");
         if (maybePerformEmptyCompositionWorkaround(text)) return true;
+        mPendingAccent = 0;
         super.commitText(text, newCursorPosition);
         updateSelectionIfRequired();
         return mImeAdapter.checkCompositionQueueAndCallNative(text, newCursorPosition,
@@ -351,6 +355,12 @@ public class AdapterInputConnection extends BaseInputConnection {
         if (DEBUG) {
             Log.w(TAG, "deleteSurroundingText [" + beforeLength + " " + afterLength + "]");
         }
+
+        if (mPendingAccent != 0) {
+            mPendingAccent = 0;
+            finishComposingText();
+        }
+
         int originalBeforeLength = beforeLength;
         int originalAfterLength = afterLength;
         int availableBefore = Selection.getSelectionStart(mEditable);
@@ -403,15 +413,23 @@ public class AdapterInputConnection extends BaseInputConnection {
         int keycode = event.getKeyCode();
         int unicodeChar = event.getUnicodeChar();
 
+        // If we're not focused in a textbox or if this isn't a KeyDown event, no need to update
+        // composition state; just pass the key event through and return.
+        if (mImeAdapter.getTextInputType() == TextInputType.NONE
+                || action != KeyEvent.ACTION_DOWN) {

[aurimas (slooooooooow), 2015/06/03 00:44:20]

Would this result in us sending KeyUp events without corresponding KeyDown
events for keyevents with a combining accent? Do you foresee that being a
problem? I am assuming that we will fire KeyUp events in javascript, but now
KeyDown too.

[aelias_OOO_until_Jul13, 2015/06/03 01:15:01]

Yes, it does and I spent some time looking into if this could be avoided.  But,
the only two ways I can think to fix this are either storing state to remember
to suppress KeyUp (which is easy to get wrong and suppress the wrong thing
later), or write a completely new accent-specific implementation of
setComposingText() to avoid the additional KeyUp there, and both of those
changes seemed too disruptive to be worth it.  I observed accent keys on Mac
already do something similar (send 229 on Keydown but accent keycode on Keyup),
I think Javascript would rarely use KeyUp outside of game-style use cases, so
it's probably harmless in practice.

[bcwhite, 2015/06/03 15:28:58]

I've been trying this patch with a physical keyboard against
http://www.backgroundexposure.com/t/key-event-test.html and it's not working at
all.  My debugging indicates that TextInputType is staying as None and thus
avoiding the entire logic due to this short-circuit.

The core of it is that updateKeyboardVisibility() is being called with
showIfNeeded==false and the type is never being updated.  The type isn't really
important with a physical keyboard so it's never been a problem.

[aelias_OOO_until_Jul13, 2015/06/03 21:10:29]

Good catch, thanks.  It looks like this is a side effect of the logic in the
renderer to avoid bringing up the soft keyboard unless there's been a user
gesture.  I'll look at how to decouple that from this in the future, but as this
is for cherry-pick to 44 and the raw key event use case is minor, I just removed
the TextInputType early-return.

+            mImeAdapter.translateAndSendNativeEvents(event);
+            return true;
+        }
+
         // 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).  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) {
+        if (keycode == KeyEvent.KEYCODE_DEL) {
             deleteSurroundingTextImpl(1, 0, true);
-        } else if (action == KeyEvent.ACTION_DOWN && keycode == KeyEvent.KEYCODE_FORWARD_DEL) {
+        } else if (keycode == KeyEvent.KEYCODE_FORWARD_DEL) {

[aurimas (slooooooooow), 2015/06/03 00:44:20]

What would happen if we have DEL, FORWARD_DEL or ENTER key event between
combining accent and actual character events?

[aelias_OOO_until_Jul13, 2015/06/03 01:15:01]

I did try out those scenarios earlier today and have a unit test case covering
DEL.  The accent state and composition is terminated, and the accent will be
deleted (for DEL) or remain (for the others).   In both cases,
finishComposingText is called which also has "mPendingAccent = 0" in it.

             deleteSurroundingTextImpl(0, 1, true);
-        } else if (action == KeyEvent.ACTION_DOWN && keycode == KeyEvent.KEYCODE_ENTER) {
+        } else if (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.
             beginBatchEdit();
@@ -419,7 +437,32 @@ public class AdapterInputConnection extends BaseInputConnection {
             mImeAdapter.translateAndSendNativeEvents(event);
             endBatchEdit();
             return true;
-        } else if (action == KeyEvent.ACTION_UP && unicodeChar != 0) {
+        } else if ((unicodeChar & KeyCharacterMap.COMBINING_ACCENT) != 0) {
+            // Store a pending accent character and make it the current composition.
+            int pendingAccent = unicodeChar & KeyCharacterMap.COMBINING_ACCENT_MASK;
+            StringBuilder builder = new StringBuilder();
+            builder.appendCodePoint(pendingAccent);
+            setComposingText(builder.toString(), 1);
+            mPendingAccent = pendingAccent;
+            return true;
+        }
+
+        if (unicodeChar != 0) {
+            if (mPendingAccent != 0) {
+                int combined = KeyEvent.getDeadChar(mPendingAccent, unicodeChar);
+                if (combined != 0) {
+                    StringBuilder builder = new StringBuilder();
+                    builder.appendCodePoint(combined);
+                    commitText(builder.toString(), 1);
+                    return true;
+                }
+                // Noncombinable character; commit the accent character and fall through to sending
+                // the key event for the character afterwards.
+                finishComposingText();
+            }
+
+            // Update the mEditable state to reflect what Blink will do in response to the KeyDown
+            // for a unicode-mapped key event.
             int selectionStart = Selection.getSelectionStart(mEditable);
             int selectionEnd = Selection.getSelectionEnd(mEditable);
             if (selectionStart > selectionEnd) {
@@ -430,6 +473,7 @@ public class AdapterInputConnection extends BaseInputConnection {
             mEditable.replace(selectionStart, selectionEnd,
                     Character.toString((char) unicodeChar));
         }
+
         mImeAdapter.translateAndSendNativeEvents(event);
         return true;
     }
@@ -444,6 +488,7 @@ public class AdapterInputConnection extends BaseInputConnection {
             return true;
         }
 
+        mPendingAccent = 0;
         super.finishComposingText();
         updateSelectionIfRequired();
         mImeAdapter.finishComposingText();
@@ -472,6 +517,7 @@ public class AdapterInputConnection extends BaseInputConnection {
         if (DEBUG) Log.w(TAG, "restartInput");
         getInputMethodManagerWrapper().restartInput(mInternalView);
         mNumNestedBatchEdits = 0;
+        mPendingAccent = 0;
     }
 
     /**