Generate non-located windowsKeyCode for the WebInputEvents.

Generate non-located windowsKeyCode for the WebInputEvents.

The documentation states that non-located keys should be generated;
however the windowsKeyCode in fact is a located key code that is sent.

This makes it consistent with Windows; as the field isn't located and
was never intended to be. This corrects the location field for android
as it was incorrect for keypad. It also removes duplicated code between
blink and chrome.

BUG=524626, 362551
Committed: https://crrev.com/8f3768781792d088730de219b8abbc23c2d259d1
Cr-Commit-Position: refs/heads/master@{#349702}

[dtapuska, 2015-09-01 20:12:37]

dtapuska@chromium.org changed reviewers:
+ aelias@chromium.org, kpschoedel@chromium.org, sadrul@chromium.org,
wez@chromium.org

[kpschoedel, 2015-09-01 20:51:22]

https://codereview.chromium.org/1308063007/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/web_input_event_aura.cc (right):

https://codereview.chromium.org/1308063007/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/web_input_event_aura.cc:87:
webkit_event.windowsKeyCode = event.key_code();
I suspect mojo/converters/input_events/input_events_type_converters.cc and
components/view_manager/display_manager.cc should also change and then
GetLocatedWindowsKeyboardCode() can go.

[dtapuska, 2015-09-01 20:54:11]

https://codereview.chromium.org/1308063007/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/web_input_event_aura.cc (right):

https://codereview.chromium.org/1308063007/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/web_input_event_aura.cc:87:
webkit_event.windowsKeyCode = event.key_code();
On 2015/09/01 20:51:22, kpschoedel wrote:
> I suspect mojo/converters/input_events/input_events_type_converters.cc and
> components/view_manager/display_manager.cc should also change and then
> GetLocatedWindowsKeyboardCode() can go.

ui::Event modifiers didn't seem to be serialized across mojo; so I wasn't sure
about that.

[aelias_OOO_until_Jul13, 2015-09-02 00:41:33]

Android changes lgtm

[kpschoedel, 2015-09-02 15:05:45]

Non-owning lgtm.

https://codereview.chromium.org/1308063007/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/web_input_event_aura.cc (right):

https://codereview.chromium.org/1308063007/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/web_input_event_aura.cc:87:
webkit_event.windowsKeyCode = event.key_code();
On 2015/09/01 20:54:11, dtapuska wrote:
> ui::Event modifiers didn't seem to be serialized across mojo; so I wasn't sure
> about that.

I think the intent is for the ui::Event → input_events_type_converters.cc →
display_manager.cc → Blink path to have the same effect at the direct ui::Event
→ Blink path, but mojo clearly has some work left there.

[Wez, 2015-09-03 06:05:47]

https://codereview.chromium.org/1308063007/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/input/web_input_event_builders_android.cc
(right):

https://codereview.chromium.org/1308063007/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/input/web_input_event_builders_android.cc:24:
static blink::WebInputEvent::Modifiers GetLocationModifiersFromAndoridKeyCode(
typo: Andorid -> Android

https://codereview.chromium.org/1308063007/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/input/web_input_event_builders_android.cc:71: 
nit: Remove this blank line?

https://codereview.chromium.org/1308063007/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/input/web_input_event_builders_win.cc
(right):

https://codereview.chromium.org/1308063007/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/input/web_input_event_builders_win.cc:73:
result.setKeyIdentifierFromWindowsKeyCode();
Is this even used any more? I thought Windows used Aura now?

https://codereview.chromium.org/1308063007/diff/1/ui/events/keycodes/keyboard...
File ui/events/keycodes/keyboard_code_conversion_android.cc (right):

https://codereview.chromium.org/1308063007/diff/1/ui/events/keycodes/keyboard...
ui/events/keycodes/keyboard_code_conversion_android.cc:11: KeyboardCode
KeyboardCodeFromAndroidKeyCode(int keycode) {
Couldn't we just replace this function with a keycode->DomCode mapping, and then
use the keycode_conversion routines to translate from that to location,
KeyboardCode, etc, given that this function already assumes a US English layout?

[dtapuska, 2015-09-03 13:39:40]

https://codereview.chromium.org/1308063007/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/input/web_input_event_builders_android.cc
(right):

https://codereview.chromium.org/1308063007/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/input/web_input_event_builders_android.cc:24:
static blink::WebInputEvent::Modifiers GetLocationModifiersFromAndoridKeyCode(
On 2015/09/03 06:05:47, Wez wrote:
> typo: Andorid -> Android

Done.

https://codereview.chromium.org/1308063007/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/input/web_input_event_builders_android.cc:71: 
On 2015/09/03 06:05:47, Wez wrote:
> nit: Remove this blank line?

Done.

https://codereview.chromium.org/1308063007/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/input/web_input_event_builders_win.cc
(right):

https://codereview.chromium.org/1308063007/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/input/web_input_event_builders_win.cc:73:
result.setKeyIdentifierFromWindowsKeyCode();
On 2015/09/03 06:05:47, Wez wrote:
> Is this even used any more? I thought Windows used Aura now?

Windows is ifdef'd in the aura build to use a builder from the actual native
event instead of the aura event so that it can correctly handle key_char I
believe.

Specifcally see:

https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/re...

https://codereview.chromium.org/1308063007/diff/1/ui/events/keycodes/keyboard...
File ui/events/keycodes/keyboard_code_conversion_android.cc (right):

https://codereview.chromium.org/1308063007/diff/1/ui/events/keycodes/keyboard...
ui/events/keycodes/keyboard_code_conversion_android.cc:11: KeyboardCode
KeyboardCodeFromAndroidKeyCode(int keycode) {
On 2015/09/03 06:05:47, Wez wrote:
> Couldn't we just replace this function with a keycode->DomCode mapping, and
then
> use the keycode_conversion routines to translate from that to location,
> KeyboardCode, etc, given that this function already assumes a US English
layout?

It makes more sense to me to go from scancode->DomCode (via evdev code); and
keeping keycode->MS VKEY consistent since "keycode" is already mapped via the
android keylayout files. Perhaps I'm wrong?

[dtapuska, 2015-09-03 14:05:49]

https://codereview.chromium.org/1308063007/diff/1/ui/events/keycodes/keyboard...
File ui/events/keycodes/keyboard_code_conversion_android.cc (right):

https://codereview.chromium.org/1308063007/diff/1/ui/events/keycodes/keyboard...
ui/events/keycodes/keyboard_code_conversion_android.cc:11: KeyboardCode
KeyboardCodeFromAndroidKeyCode(int keycode) {
On 2015/09/03 13:39:40, dtapuska wrote:
> On 2015/09/03 06:05:47, Wez wrote:
> > Couldn't we just replace this function with a keycode->DomCode mapping, and
> then
> > use the keycode_conversion routines to translate from that to location,
> > KeyboardCode, etc, given that this function already assumes a US English
> layout?
> 
> It makes more sense to me to go from scancode->DomCode (via evdev code); and
> keeping keycode->MS VKEY consistent since "keycode" is already mapped via the
> android keylayout files. Perhaps I'm wrong?
> 

Ok I dug a little more and I do think we can avoid this table. See my other
change where I populate the DomCode; once that is available we should be able to
call DomCodeToUsLayoutKeyboardCode and remove this table.

I'll block another patch of this CL to remove the table on that other CL.

[dtapuska, 2015-09-08 17:18:26]

https://codereview.chromium.org/1308063007/diff/1/ui/events/keycodes/keyboard...
File ui/events/keycodes/keyboard_code_conversion_android.cc (right):

https://codereview.chromium.org/1308063007/diff/1/ui/events/keycodes/keyboard...
ui/events/keycodes/keyboard_code_conversion_android.cc:11: KeyboardCode
KeyboardCodeFromAndroidKeyCode(int keycode) {
On 2015/09/03 06:05:47, Wez wrote:
> Couldn't we just replace this function with a keycode->DomCode mapping, and
then
> use the keycode_conversion routines to translate from that to location,
> KeyboardCode, etc, given that this function already assumes a US English
layout?

Done.

[dtapuska, 2015-09-08 17:21:38]

https://codereview.chromium.org/1308063007/diff/60001/ui/events/keycodes/keyb...
File ui/events/keycodes/keyboard_code_conversion_android.h (left):

https://codereview.chromium.org/1308063007/diff/60001/ui/events/keycodes/keyb...
ui/events/keycodes/keyboard_code_conversion_android.h:12: 
Yes this file is empty; and in context of this change could be removed. But when
I add the DomKey support for Android; it adds a function in this file.

[Wez, 2015-09-09 06:06:11]

https://codereview.chromium.org/1308063007/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/input/web_input_event_builders_win.cc
(right):

https://codereview.chromium.org/1308063007/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/input/web_input_event_builders_win.cc:73:
result.setKeyIdentifierFromWindowsKeyCode();
On 2015/09/03 13:39:40, dtapuska wrote:
> On 2015/09/03 06:05:47, Wez wrote:
> > Is this even used any more? I thought Windows used Aura now?
> 
> Windows is ifdef'd in the aura build to use a builder from the actual native
> event instead of the aura event so that it can correctly handle key_char I
> believe.
> 
> Specifcally see:
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/re...

Acknowledged.

https://codereview.chromium.org/1308063007/diff/60001/content/browser/rendere...
File content/browser/renderer_host/input/web_input_event_util.cc (right):

https://codereview.chromium.org/1308063007/diff/60001/content/browser/rendere...
content/browser/renderer_host/input/web_input_event_util.cc:55:
blink::WebInputEvent::Modifiers DomCodeToWebInputEventModifiers(
Don't we already have this for Aura? Could we get away with just one common
copy?

https://codereview.chromium.org/1308063007/diff/60001/ui/events/keycodes/keyb...
File ui/events/keycodes/keyboard_code_conversion_android.h (right):

https://codereview.chromium.org/1308063007/diff/60001/ui/events/keycodes/keyb...
ui/events/keycodes/keyboard_code_conversion_android.h:11: namespace ui {
Looks like this and the cc file can go away entirely?

[Wez, 2015-09-09 06:11:26]

lgtm

[sadrul, 2015-09-09 06:22:40]

stamp lgtm

[dtapuska, 2015-09-09 18:45:49]

https://codereview.chromium.org/1308063007/diff/60001/content/browser/rendere...
File content/browser/renderer_host/input/web_input_event_util.cc (right):

https://codereview.chromium.org/1308063007/diff/60001/content/browser/rendere...
content/browser/renderer_host/input/web_input_event_util.cc:55:
blink::WebInputEvent::Modifiers DomCodeToWebInputEventModifiers(
On 2015/09/09 06:06:11, Wez wrote:
> Don't we already have this for Aura? Could we get away with just one common
> copy?

This is making it the common copy.. I removed this from aura and put it here; to
be used both by android and aura (and mac in the future).

https://codereview.chromium.org/1308063007/diff/60001/ui/events/keycodes/keyb...
File ui/events/keycodes/keyboard_code_conversion_android.h (right):

https://codereview.chromium.org/1308063007/diff/60001/ui/events/keycodes/keyb...
ui/events/keycodes/keyboard_code_conversion_android.h:11: namespace ui {
On 2015/09/09 06:06:11, Wez wrote:
> Looks like this and the cc file can go away entirely?

See comment on the left side of this file.. I stated that I'd add to this file
in the next change.

[dtapuska, 2015-09-18 16:44:57]

The CQ bit was checked by dtapuska@chromium.org

[dtapuska, 2015-09-18 16:44:58]

The patchset sent to the CQ was uploaded after l-g-t-m from aelias@chromium.org,
kpschoedel@chromium.org, sadrul@chromium.org, wez@chromium.org
Link to the patchset: https://codereview.chromium.org/1308063007/#ps100001
(title: "Fix unittests")

[commit-bot: I haz the power, 2015-09-18 16:45:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1308063007/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1308063007/100001

[commit-bot: I haz the power, 2015-09-18 17:27:28]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-09-18 17:28:06]

Patchset 6 (id:??) landed as
https://crrev.com/8f3768781792d088730de219b8abbc23c2d259d1
Cr-Commit-Position: refs/heads/master@{#349702}