Remove ui::KeyEvent::platform_keycode_

Remove ui::KeyEvent::platform_keycode_

This field had been added as a temporary measure to support
X11 Keysym-based character composition under Ozone. Since
crrev.com/ed31ed it is no longer necessary.

BUG=442757
Committed: https://crrev.com/cf78ded8ae7f64968789c3bac48ea6e69df08d29
Cr-Commit-Position: refs/heads/master@{#349195}

[kpschoedel, 2015-08-28 17:40:50]

The CQ bit was checked by kpschoedel@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-08-28 17:41:12]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1312833006/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1312833006/1

[kpschoedel, 2015-08-28 17:41:52]

kpschoedel@chromium.org changed reviewers:
+ sadrul@chromium.org

[sadrul, 2015-08-28 17:48:19]

sadrul@chromium.org changed reviewers:
+ wez@chromium.org

[sadrul, 2015-08-28 17:48:19]

+wez@

lgtm!

[commit-bot: I haz the power, 2015-08-28 18:24:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-08-28 18:24:33]

Dry run: Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)

[kpschoedel, 2015-08-31 15:12:10]

Patchset #3 (id:40001) has been deleted

[kpschoedel, 2015-08-31 15:12:15]

Patchset #2 (id:20001) has been deleted

[kpschoedel, 2015-08-31 17:37:44]

Patchset #3 (id:80001) has been deleted

[kpschoedel, 2015-08-31 17:37:52]

Patchset #2 (id:60001) has been deleted

[kpschoedel, 2015-08-31 18:26:52]

kpschoedel@chromium.org changed reviewers:
+ erg@chromium.org

[kpschoedel, 2015-08-31 18:26:52]

+erg@ for pre-review of
//mojo/converters/input_events/input_events_type_converters.cc (see inline
comment)

https://codereview.chromium.org/1312833006/diff/100001/mojo/converters/input_...
File mojo/converters/input_events/input_events_type_converters.cc (right):

https://codereview.chromium.org/1312833006/diff/100001/mojo/converters/input_...
mojo/converters/input_events/input_events_type_converters.cc:207:
ui::KeycodeConverter::DomCodeToNativeKeycode(key_event->code());
The |ui::KeyEvent::platform_keycode()| despite its unfortunate name was the
wrong kind of code for this (keysym rather than keycode) if the comment in
input_events.mojom is to be believed. The RHS here is the same as for Blink's
|WebKeyboardEvent.nativeKeyCode|; however, it might be preferable to store the
UI Events |code| directly rather than a platform-specific scan code, as it's a
proper superset and is now exposed in Blink and PPAPI. The
|ui::KeyEvent::code()| is an integer encoding (essentially USB codes).

[Elliot Glaysher, 2015-08-31 19:48:40]

https://codereview.chromium.org/1312833006/diff/100001/mojo/converters/input_...
File mojo/converters/input_events/input_events_type_converters.cc (right):

https://codereview.chromium.org/1312833006/diff/100001/mojo/converters/input_...
mojo/converters/input_events/input_events_type_converters.cc:207:
ui::KeycodeConverter::DomCodeToNativeKeycode(key_event->code());
On 2015/08/31 18:26:52, kpschoedel wrote:
> The |ui::KeyEvent::platform_keycode()| despite its unfortunate name was the
> wrong kind of code for this (keysym rather than keycode) if the comment in
> input_events.mojom is to be believed. The RHS here is the same as for Blink's
> |WebKeyboardEvent.nativeKeyCode|; however, it might be preferable to store the
> UI Events |code| directly rather than a platform-specific scan code, as it's a
> proper superset and is now exposed in Blink and PPAPI. The
> |ui::KeyEvent::code()| is an integer encoding (essentially USB codes).

This probably shouldn't be a problem as long as
components/html_vierwer/blink_input_events_type_converters.cc 's usage of
native_key_code doesn't see any difference.

(This entire area is a mess; thank you for cleaning this up!)

[kpschoedel, 2015-08-31 21:41:32]

kpschoedel@chromium.org changed reviewers:
+ ben@chromium.org, samuong@chromium.org

[kpschoedel, 2015-08-31 21:41:33]

+ben for OWNER review of //mojo/converters and //components/view_manager

+samuong for //chrome/test/chromedriver

https://codereview.chromium.org/1312833006/diff/100001/mojo/converters/input_...
File mojo/converters/input_events/input_events_type_converters.cc (right):

https://codereview.chromium.org/1312833006/diff/100001/mojo/converters/input_...
mojo/converters/input_events/input_events_type_converters.cc:207:
ui::KeycodeConverter::DomCodeToNativeKeycode(key_event->code());
This change at least makes the ui→Mojo→Blink path behave the same as the
ui→Blink path. Dave Tapuska is now taking on cleaning up Blink keyboard events,
which I expect will lead to adding UI Events |key| and |code| here and possibly
removing this field.

[samuong, 2015-08-31 21:45:51]

chromedriver lgtm

[Wez, 2015-09-03 17:29:43]

So much LGTM

https://codereview.chromium.org/1312833006/diff/100001/mojo/converters/input_...
File mojo/converters/input_events/input_events_type_converters.cc (right):

https://codereview.chromium.org/1312833006/diff/100001/mojo/converters/input_...
mojo/converters/input_events/input_events_type_converters.cc:207:
ui::KeycodeConverter::DomCodeToNativeKeycode(key_event->code());
On 2015/08/31 21:41:33, kpschoedel wrote:
> This change at least makes the ui→Mojo→Blink path behave the same as the
> ui→Blink path. Dave Tapuska is now taking on cleaning up Blink keyboard
events,
> which I expect will lead to adding UI Events |key| and |code| here and
possibly
> removing this field.

Yes, that is the ultimate goal, though I suspect synthesizing DomKey under
Android will be tricky, and we need to make sure we cope w/ OSKs that don't
generate scancodes, etc. We could switch the Mojo interface over to DomCode
already, in principle.

Internally I don't think Blink really uses the nativeKeyCode, and historically
it's been incorrectly set in many places (e.g. all synthetic events had it set
to the VKEY, not to a platform-native keycode) so I'm not too worried about
breakage there.

[Elliot Glaysher, 2015-09-03 17:33:48]

lgtm

https://codereview.chromium.org/1312833006/diff/100001/mojo/converters/input_...
File mojo/converters/input_events/input_events_type_converters.cc (right):

https://codereview.chromium.org/1312833006/diff/100001/mojo/converters/input_...
mojo/converters/input_events/input_events_type_converters.cc:207:
ui::KeycodeConverter::DomCodeToNativeKeycode(key_event->code());
On 2015/09/03 17:29:43, Wez wrote:
> On 2015/08/31 21:41:33, kpschoedel wrote:
> > This change at least makes the ui→Mojo→Blink path behave the same as the
> > ui→Blink path. Dave Tapuska is now taking on cleaning up Blink keyboard
> events,
> > which I expect will lead to adding UI Events |key| and |code| here and
> possibly
> > removing this field.
> 
> Yes, that is the ultimate goal, though I suspect synthesizing DomKey under
> Android will be tricky, and we need to make sure we cope w/ OSKs that don't
> generate scancodes, etc. We could switch the Mojo interface over to DomCode
> already, in principle.
> 
> Internally I don't think Blink really uses the nativeKeyCode, and historically
> it's been incorrectly set in many places (e.g. all synthetic events had it set
> to the VKEY, not to a platform-native keycode) so I'm not too worried about
> breakage there.

I see. When I wrote this, I did a quick grep on blink, saw that nativeKeyCode
was used a bit, and then assumed that they was real usage. Awesome if that's
being pulled out.

[Ben Goodger (Google), 2015-09-15 23:58:27]

mojo lgtm

[kpschoedel, 2015-09-16 15:41:24]

The CQ bit was checked by kpschoedel@chromium.org

[kpschoedel, 2015-09-16 15:41:24]

The patchset sent to the CQ was uploaded after l-g-t-m from sadrul@chromium.org,
samuong@chromium.org, erg@chromium.org, ben@chromium.org, wez@chromium.org
Link to the patchset: https://codereview.chromium.org/1312833006/#ps120001
(title: "rebase")

[commit-bot: I haz the power, 2015-09-16 15:41:40]

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

[commit-bot: I haz the power, 2015-09-16 19:52:02]

Committed patchset #3 (id:120001)

[commit-bot: I haz the power, 2015-09-16 19:52:43]

Patchset 3 (id:??) landed as
https://crrev.com/cf78ded8ae7f64968789c3bac48ea6e69df08d29
Cr-Commit-Position: refs/heads/master@{#349195}

[commit-bot: I haz the power, 2015-09-23 12:59:32]

Patchset 3 (id:??) landed as
https://crrev.com/cf78ded8ae7f64968789c3bac48ea6e69df08d29
Cr-Commit-Position: refs/heads/master@{#349195}