[Chromoting] USB -> Mac keycode data.

USB -> Mac keycode data.
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=126690

[Wez, 2012-03-14 17:55:29]

lgtm

http://codereview.chromium.org/9704016/diff/4001/remoting/host/event_executor...
File remoting/host/event_executor_mac.cc (right):

http://codereview.chromium.org/9704016/diff/4001/remoting/host/event_executor...
remoting/host/event_executor_mac.cc:25: #define INVALID_KEYCODE 0xffff
nit: It seems strange to define INVALID_KEYCODE and then not use it in the
actual tables.

http://codereview.chromium.org/9704016/diff/4001/remoting/host/event_executor...
remoting/host/event_executor_mac.cc:227: if (usb_keycode == 0)
Wrong indentation, throughout this function.

http://codereview.chromium.org/9704016/diff/4001/remoting/host/event_executor...
remoting/host/event_executor_mac.cc:230: for (uint i = 0; i <
arraysize(usb_keycode_map); i++)
Use braces for this "for".

http://codereview.chromium.org/9704016/diff/4001/remoting/host/event_executor...
remoting/host/event_executor_mac.cc:238: int win_keycode = event.keycode();
nit: Move this inside the else block, since it's only needed there.

http://codereview.chromium.org/9704016/diff/4001/remoting/host/usb_keycode_map.h
File remoting/host/usb_keycode_map.h (right):

http://codereview.chromium.org/9704016/diff/4001/remoting/host/usb_keycode_ma...
remoting/host/usb_keycode_map.h:32: USB_KEYMAP(0x010082, 0x0096, 0x0000,
0xffff),  // SystemSleep
Since the "invalid" code differs between Windows, Mac & Linux, it would be good
to document that in this table.  One way would be to add an explicit line
mapping USB#0 to the platform invalid codes.

Since the table-lookup code is duplicated across platforms, the two directions
should move into this header, and then the invalid-code information could be
documented there, or via a second macro define in the calling code before
including this file.

[garykac, 2012-03-14 18:25:44]

http://codereview.chromium.org/9704016/diff/4001/remoting/host/event_executor...
File remoting/host/event_executor_mac.cc (right):

http://codereview.chromium.org/9704016/diff/4001/remoting/host/event_executor...
remoting/host/event_executor_mac.cc:25: #define INVALID_KEYCODE 0xffff
On 2012/03/14 17:55:29, Wez wrote:
> nit: It seems strange to define INVALID_KEYCODE and then not use it in the
> actual tables.

I did initially, but it made the table look heavy with all the UPPER_CASE
DEFINE_MACROS.

I'll re-add it.

http://codereview.chromium.org/9704016/diff/4001/remoting/host/event_executor...
remoting/host/event_executor_mac.cc:227: if (usb_keycode == 0)
On 2012/03/14 17:55:29, Wez wrote:
> Wrong indentation, throughout this function.

Damn Xcode text editor (I didn't have it set up properly at the time). Done.

http://codereview.chromium.org/9704016/diff/4001/remoting/host/event_executor...
remoting/host/event_executor_mac.cc:230: for (uint i = 0; i <
arraysize(usb_keycode_map); i++)
On 2012/03/14 17:55:29, Wez wrote:
> Use braces for this "for".

Done.

http://codereview.chromium.org/9704016/diff/4001/remoting/host/event_executor...
remoting/host/event_executor_mac.cc:238: int win_keycode = event.keycode();
On 2012/03/14 17:55:29, Wez wrote:
> nit: Move this inside the else block, since it's only needed there.

Done.

[Wez, 2012-03-14 20:38:01]

http://codereview.chromium.org/9704016/diff/4001/remoting/host/event_executor...
File remoting/host/event_executor_mac.cc (right):

http://codereview.chromium.org/9704016/diff/4001/remoting/host/event_executor...
remoting/host/event_executor_mac.cc:25: #define INVALID_KEYCODE 0xffff
On 2012/03/14 18:25:44, garykac wrote:
> On 2012/03/14 17:55:29, Wez wrote:
> > nit: It seems strange to define INVALID_KEYCODE and then not use it in the
> > actual tables.
> 
> I did initially, but it made the table look heavy with all the UPPER_CASE
> DEFINE_MACROS.
> 
> I'll re-add it.

This was a nit; if it makes the table heavy then feel free to ignore, and just
add a comment to document the difference in invalid codes.