Don't kill key events even when it's VKEY_UNKNOWN.

Don't kill key events even when it's VKEY_UNKNOWN.

We're killing key events with key_code = VKEY_UNKNOWN in WindowTargeter, but it's wrong.  There exists not a few hardware keys used to type characters (such as ä, í) whose key code falls into VKEY_UNKNOWN.  We should not ignore key events just because their key code falls into VKEY_UNKNOWN.  VKEY_UNKNOWN means just that we failed to map a key code to our small set of keys.  Even if it's not a well-known standard key, it's a key.

This CL allows all the key events which have a character be despatched to the target window regardless of key code.

If we need to ignore some special keys, we should explicitly list them up.

NOTE against Issue 119407 ( https://codereview.chromium.org/9835032 )
XF86XK_PowerOff no longer converts into VKEY_UNKNOWN, it converts into VKEY_POWER (=0x98).

BUG=363037
TEST=Manually done with a UK keyboard, which has keycode 94.  Or with HHKB with running xmodmap -e 'keycode 100 = adiaeresis Adiaeresis'.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=270347

[Yuki, 2014-05-09 09:19:54]

Sadrul, could you review this CL?

I'd like to land this CL in M35 stable because there is no way to input 'ä' on
German keyboards, and many cases for other international keyboards.  Note 'ä' is
a popular regular key on German keyboards.

[sadrul, 2014-05-09 15:49:59]

On 2014/05/09 09:19:54, Yuki wrote:
> Sadrul, could you review this CL?
> 
> I'd like to land this CL in M35 stable because there is no way to input 'ä' on
> German keyboards, and many cases for other international keyboards.  Note 'ä'
is
> a popular regular key on German keyboards.

We have been blocking UNKNOWN key-events from being dispatched for a very long
time (since https://codereview.chromium.org/9835032, 2years+ back). We only
change it recently in https://codereview.chromium.org/243143002 to ignore
UNKNOWN only for fabricated key-events. Did something else change recently? Is
the second CL the one we need to revert?

We should have ime-related tests for this. Otherwise things like this will
continue to break.

[Yuki, 2014-05-09 16:46:21]

On 2014/05/09 15:49:59, sadrul wrote:
> On 2014/05/09 09:19:54, Yuki wrote:
> > Sadrul, could you review this CL?
> > 
> > I'd like to land this CL in M35 stable because there is no way to input 'ä'
on
> > German keyboards, and many cases for other international keyboards.  Note
'ä'
> is
> > a popular regular key on German keyboards.
> 
> We have been blocking UNKNOWN key-events from being dispatched for a very long
> time (since https://codereview.chromium.org/9835032, 2years+ back). We only
> change it recently in https://codereview.chromium.org/243143002 to ignore
> UNKNOWN only for fabricated key-events. Did something else change recently? Is
> the second CL the one we need to revert?
> 
> We should have ime-related tests for this. Otherwise things like this will
> continue to break.

Nothing changed recently, it has been broken on Linux Aura from the beginning. 
I think that CrOS doesn't hit this problem because the supported hardware
keyboards are very limited and they (fortunately?) don't have keycode=94, etc. 
CrOS has just been lucky.

Reverting the second CL doesn't help solve this issue.  Ignoring all UNKNOWN
keys is just wrong.  I checked that Firefox emits keydown/up events with
keycode=0 and char='ä'.  We have to support such keys in the same manner.

Again, I think that ignoring all the keycode=0 events is wrong.  If we want to
ignore XF86PowerOff key, we should have ignored that key only.  There are not a
few valid character keys with keycode=0.

I agree that we should add more tests.

[sadrul, 2014-05-09 17:24:04]

If we are to land this, then you need to include changes to make sure we don't
send unexpected events to the renderer (including, but may not be limited to,
the power button). Otherwise, this is going to cause farther issues. You should
also add tests (if not with this change, in subsequent CLs)

[Yuki, 2014-05-12 09:02:06]

As per your concern, I changed this CL a little more conservative.  This CL
makes it pass the key events with keycode==0 && char!=0 to renderers.  char!=0
is a new condition.

Also I added test cases to WindowEventDispatcherTest.

Also added a note about https://codereview.chromium.org/9835032 in the CL
description.
I think it's right to pass any key events to renderers, and it's also right that
some of the keys fall into keycode=0 because they're unknown keys.  If a web
site had an issue with it, it must be a bug of the web site.

Having said that, I agree that a drastic change would be risky.  So I changed
the condition to keycode==0 && char!=0.  Firefox has been sending these key
events with international keyboards, so it must be okay.  Otherwise, it must be
an issue of web sites.

[sadrul, 2014-05-14 03:17:38]

LGTM

[Yuki, 2014-05-14 03:21:22]

The CQ bit was checked by yukishiino@chromium.org

[commit-bot: I haz the power, 2014-05-14 03:22:43]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/yukishiino@chromium.org/278843002/60001

[commit-bot: I haz the power, 2014-05-14 06:34:16]

Change committed as 270347