Deal with backspace keycodes getting sent by IME during compositions.

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
Committed: https://crrev.com/30b299cb04b4ba2330ef61a8a24c1e58513a0af2
Cr-Commit-Position: refs/heads/master@{#329954}

[aelias_OOO_until_Jul13, 2015-05-14 03:39:54]

aelias@chromium.org changed reviewers:
+ aurimas@chromium.org, bcwhite@chromium.org

[aelias_OOO_until_Jul13, 2015-05-14 03:47:16]

PTAL.  Hopefully this will finally get the backspace situation under control --
at least this patch seemed to work fine with Samsung and Google IMEs as well as
physical keyboards.

[aelias_OOO_until_Jul13, 2015-05-14 19:51:53]

Note that this is a fix for a 43 blocker.  Could you take a look?

I'll be leaving on a long flight at 7:30pm PST today.  If the review feedback is
minor, I'd like to fix and land before then so we have can start baking on
trunk.

[aurimas (slooooooooow), 2015-05-14 20:06:57]

Overall I think this looks very reasonable.

Could we add some tests to prevent future regressions?

https://codereview.chromium.org/1137673005/diff/1/content/public/android/java...
File
content/public/android/java/src/org/chromium/content/browser/input/AdapterInputConnection.java
(right):

https://codereview.chromium.org/1137673005/diff/1/content/public/android/java...
content/public/android/java/src/org/chromium/content/browser/input/AdapterInputConnection.java:419:
int unicodeChar = event.getUnicodeChar();
This code only previously got executed only when the key events where not
KEYCODE_DEL, KEYCODE_FORWARD_DEL. Should we keep that the same way? Does this
just work because delete does not have a unicode char?

[aelias_OOO_until_Jul13, 2015-05-14 21:25:10]

OK, added tests, PTAL.

https://codereview.chromium.org/1137673005/diff/1/content/public/android/java...
File
content/public/android/java/src/org/chromium/content/browser/input/AdapterInputConnection.java
(right):

https://codereview.chromium.org/1137673005/diff/1/content/public/android/java...
content/public/android/java/src/org/chromium/content/browser/input/AdapterInputConnection.java:419:
int unicodeChar = event.getUnicodeChar();
On 2015/05/14 20:06:57, aurimas wrote:
> This code only previously got executed only when the key events where not
> KEYCODE_DEL, KEYCODE_FORWARD_DEL. Should we keep that the same way? Does this
> just work because delete does not have a unicode char?

I think it doesn't have a unicode char, but in any case this points to the fact
that nested if statements are confusing/errorprone, so I removed the nesting and
made it a straight-line flow, which brings back the former els'ing logic here.

[aurimas (slooooooooow), 2015-05-14 21:32:05]

lgtm

[aelias_OOO_until_Jul13, 2015-05-14 21:54:01]

The CQ bit was checked by aelias@chromium.org

[commit-bot: I haz the power, 2015-05-14 21:55:34]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1137673005/60001

[commit-bot: I haz the power, 2015-05-14 22:29:08]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-05-14 22:29:49]

Patchset 4 (id:??) landed as
https://crrev.com/30b299cb04b4ba2330ef61a8a24c1e58513a0af2
Cr-Commit-Position: refs/heads/master@{#329954}