Native to USB keycode conversion for Mac

Native to USB keycode conversion for Mac

Add code to convert from native Mac keycodes into USB keycodes so that Pepper can correctly populate the |usb_key_code| field in the InputEvent.
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=126956

[dmichael (off chromium), 2012-03-14 19:36:01]

I'll defer to wez about most of this; I'm not familiar with the data for that
table.

Does the .mm extension make sense? It looks like conformant C++ to me, and I
don't see any dependencies on Apple system libraries, so I would have expected
".cc".

http://codereview.chromium.org/9701030/diff/2001/webkit/plugins/ppapi/usb_key...
File webkit/plugins/ppapi/usb_key_code_conversion_mac.mm (right):

http://codereview.chromium.org/9701030/diff/2001/webkit/plugins/ppapi/usb_key...
webkit/plugins/ppapi/usb_key_code_conversion_mac.mm:135: {0x0700e0, 0x003b},  //
LeftControl
There's a discontinuity here. Is that expected & correct?

[garykac, 2012-03-14 20:26:01]

On 2012/03/14 19:36:01, dmichael wrote:
> I'll defer to wez about most of this; I'm not familiar with the data for that
> table.
> 
> Does the .mm extension make sense? It looks like conformant C++ to me, and I
> don't see any dependencies on Apple system libraries, so I would have expected
> ".cc".

Changed to .cc

[garykac, 2012-03-14 20:26:06]

http://codereview.chromium.org/9701030/diff/2001/webkit/plugins/ppapi/usb_key...
File webkit/plugins/ppapi/usb_key_code_conversion_mac.mm (right):

http://codereview.chromium.org/9701030/diff/2001/webkit/plugins/ppapi/usb_key...
webkit/plugins/ppapi/usb_key_code_conversion_mac.mm:135: {0x0700e0, 0x003b},  //
LeftControl
On 2012/03/14 19:36:02, dmichael wrote:
> There's a discontinuity here. Is that expected & correct?

Yes. In that space there's a bunch of special Keypad keys and a chunk of unused
codes before the ctrl/shift/cmd keys.

[dmichael (off chromium), 2012-03-14 20:50:26]

lgtm, but you may want to let wez weigh in

http://codereview.chromium.org/9701030/diff/2001/webkit/plugins/ppapi/usb_key...
File webkit/plugins/ppapi/usb_key_code_conversion_mac.mm (right):

http://codereview.chromium.org/9701030/diff/2001/webkit/plugins/ppapi/usb_key...
webkit/plugins/ppapi/usb_key_code_conversion_mac.mm:135: {0x0700e0, 0x003b},  //
LeftControl
On 2012/03/14 20:26:06, garykac wrote:
> On 2012/03/14 19:36:02, dmichael wrote:
> > There's a discontinuity here. Is that expected & correct?
> 
> Yes. In that space there's a bunch of special Keypad keys and a chunk of
unused
> codes before the ctrl/shift/cmd keys.
A short comment to that effect might help, just so it is clear to future readers
that it's not a mistake.

[garykac, 2012-03-14 21:05:01]

On Wed, Mar 14, 2012 at 1:50 PM, <dmichael@chromium.org> wrote:

> lgtm, but you may want to let wez weigh in


Will do.

On 2012/03/14 20:26:06, garykac wrote:
>
>> On 2012/03/14 19:36:02, dmichael wrote:
>> > There's a discontinuity here. Is that expected & correct?
>
>  Yes. In that space there's a bunch of special Keypad keys and a chunk of
>> unused
>
>  codes before the ctrl/shift/cmd keys.
>>
> A short comment to that effect might help, just so it is clear to future
> readers that it's not a mistake.
>

This table will be going away and be replaced by a shared, cross-platform
table which will have these unused ranges marked appropriately.
But I'll add a short comment here anyway.

Thanks!

[garykac, 2012-03-14 23:20:04]

A copy of the complete mapping table has been placed in this directory so that
it's easier to transition when it moves to a shared location.

[Wez, 2012-03-14 23:21:57]

LGTM!

[commit-bot: I haz the power, 2012-03-15 00:38:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/garykac@chromium.org/9701030/4003

[commit-bot: I haz the power, 2012-03-15 00:38:22]

Presubmit check for 9701030-4003 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit Messages **
If this change has an associated bug, add BUG=[bug number].

If this change requires manual test instructions to QA team, add
TEST=[instructions].

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    webkit

Presubmit checks took 2.4s to calculate.

[garykac, 2012-03-15 00:58:18]

+tony for webkit/glue

[tony, 2012-03-15 16:46:28]

Please elaborate in the change description.  E.g., you might want to mention
that this is for ppapi.

What's the final location for this file going to be?  Can you file a bug for
that work?

The change itself LGTM.

http://codereview.chromium.org/9701030/diff/4003/webkit/plugins/ppapi/usb_key...
File webkit/plugins/ppapi/usb_key_code_conversion_mac.cc (right):

http://codereview.chromium.org/9701030/diff/4003/webkit/plugins/ppapi/usb_key...
webkit/plugins/ppapi/usb_key_code_conversion_mac.cc:26: for (uint i = 0; i <
arraysize(usb_keycode_map); i++) {
Nit: If we keep usb_keycode_map sorted (looks like it currently is), we could
use bsearch here.

[dmichael (off chromium), 2012-03-15 17:03:08]

(still lgtm)

http://codereview.chromium.org/9701030/diff/4003/webkit/plugins/ppapi/usb_key...
File webkit/plugins/ppapi/usb_key_code_conversion_mac.cc (right):

http://codereview.chromium.org/9701030/diff/4003/webkit/plugins/ppapi/usb_key...
webkit/plugins/ppapi/usb_key_code_conversion_mac.cc:26: for (uint i = 0; i <
arraysize(usb_keycode_map); i++) {
On 2012/03/15 16:46:28, tony wrote:
> Nit: If we keep usb_keycode_map sorted (looks like it currently is), we could
> use bsearch here.
If we were to use binary search, I would recommend lower_bound from <algorithm>
instead of the C bsearch.

But I think the extra code complexity isn't really necessary for something of
this size that will fit in 2kb of memory. I'm happy with it either way.

http://codereview.chromium.org/9701030/diff/4003/webkit/plugins/ppapi/usb_key...
File webkit/plugins/ppapi/usb_keycode_map.h (right):

http://codereview.chromium.org/9701030/diff/4003/webkit/plugins/ppapi/usb_key...
webkit/plugins/ppapi/usb_keycode_map.h:377: };
please undef USB_KEYMAP

[Wez, 2012-03-15 17:10:52]

http://codereview.chromium.org/9701030/diff/4003/webkit/plugins/ppapi/usb_key...
File webkit/plugins/ppapi/usb_key_code_conversion_mac.cc (right):

http://codereview.chromium.org/9701030/diff/4003/webkit/plugins/ppapi/usb_key...
webkit/plugins/ppapi/usb_key_code_conversion_mac.cc:26: for (uint i = 0; i <
arraysize(usb_keycode_map); i++) {
On 2012/03/15 17:03:08, dmichael wrote:
> On 2012/03/15 16:46:28, tony wrote:
> > Nit: If we keep usb_keycode_map sorted (looks like it currently is), we
could
> > use bsearch here.
> If we were to use binary search, I would recommend lower_bound from
<algorithm>
> instead of the C bsearch.
> 
> But I think the extra code complexity isn't really necessary for something of
> this size that will fit in 2kb of memory. I'm happy with it either way.

Agreed.  We can optimize the lookup later if it proves too expensive.

[garykac, 2012-03-15 17:51:20]

On 2012/03/15 16:46:28, tony wrote:
> Please elaborate in the change description.  E.g., you might want to mention
> that this is for ppapi.

Done.

> What's the final location for this file going to be?  Can you file a bug for
> that work?

Done. crbug.com/118432


On 2012/03/15 17:03:08, dmichael wrote:
> 
>
http://codereview.chromium.org/9701030/diff/4003/webkit/plugins/ppapi/usb_key...
> webkit/plugins/ppapi/usb_keycode_map.h:377: };
> please undef USB_KEYMAP

Done.

I added an undefine in the usb_key_code_conversion_mac.cc file, so that it's
near the corresponding #define.
Although I can see the value of having a single #undef at the end of the _map.h
file.
Does Chromium have a style preference for this?

[dmichael (off chromium), 2012-03-15 18:26:41]

On 2012/03/15 17:51:20, garykac wrote:
> On 2012/03/15 16:46:28, tony wrote:
> > Please elaborate in the change description.  E.g., you might want to mention
> > that this is for ppapi.
> 
> Done.
> 
> > What's the final location for this file going to be?  Can you file a bug for
> > that work?
> 
> Done. crbug.com/118432
> 
> 
> On 2012/03/15 17:03:08, dmichael wrote:
> > 
> >
>
http://codereview.chromium.org/9701030/diff/4003/webkit/plugins/ppapi/usb_key...
> > webkit/plugins/ppapi/usb_keycode_map.h:377: };
> > please undef USB_KEYMAP
> 
> Done.
> 
> I added an undefine in the usb_key_code_conversion_mac.cc file, so that it's
> near the corresponding #define.
> Although I can see the value of having a single #undef at the end of the
_map.h
> file.
> Does Chromium have a style preference for this?
I don't think so. Putting it after the include as you have makes sense. I just
didn't read carefully enough.