Add APIs to the client plugin to re-map and trap key events.

Add APIs to the client plugin to re-map and trap key events.

Re-mapped events allow keys to be re-mapped by the plugin before being delivered to the host.

Trapped events are posted to the web-app in a trappedKeyEvent message, allowing more complex processing to be performed, at the cost of higher input latency. 

BUG=121787
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=131321

[Wez, 2012-04-06 23:51:07]

PTAL

[Sergey Ulanov, 2012-04-07 01:50:14]

LGTM with my nits addressed.

http://codereview.chromium.org/10025001/diff/2001/remoting/client/key_event_m...
File remoting/client/key_event_mapper.cc (right):

http://codereview.chromium.org/10025001/diff/2001/remoting/client/key_event_m...
remoting/client/key_event_mapper.cc:26: trapped_keys.insert(usb_keycode);
Add {}

http://codereview.chromium.org/10025001/diff/2001/remoting/client/key_event_m...
remoting/client/key_event_mapper.cc:33: mapped_keys.erase(in_usb_keycode);
add {}

http://codereview.chromium.org/10025001/diff/2001/remoting/client/key_event_m...
remoting/client/key_event_mapper.cc:53: InputFilter::InjectKeyEvent(new_event);
Do we want to trap remapped keys? Probably not, just want to make sure.

http://codereview.chromium.org/10025001/diff/2001/remoting/client/key_event_m...
File remoting/client/key_event_mapper.h (right):

http://codereview.chromium.org/10025001/diff/2001/remoting/client/key_event_m...
remoting/client/key_event_mapper.h:18: typedef base::Callback<void(uint32,bool)>
KeyTrapCallback;
Maybe define this inside KeyEventMapper?
nit: space after comma

http://codereview.chromium.org/10025001/diff/2001/remoting/client/key_event_m...
remoting/client/key_event_mapper.h:20: class KeyEventMapper : public
protocol::InputFilter {
Explain what this is for.

http://codereview.chromium.org/10025001/diff/2001/remoting/client/plugin/chro...
File remoting/client/plugin/chromoting_instance.cc (right):

http://codereview.chromium.org/10025001/diff/2001/remoting/client/plugin/chro...
remoting/client/plugin/chromoting_instance.cc:275: if
(!data->GetInteger("from_keycode", &from_keycode) ||
this should be fromKeycode and toKeycode

http://codereview.chromium.org/10025001/diff/2001/remoting/client/plugin/chro...
remoting/client/plugin/chromoting_instance.cc:494:
data->SetInteger("usb_keycode", usb_keycode);
usbKeycode

[Wez, 2012-04-08 00:12:41]

http://codereview.chromium.org/10025001/diff/2001/remoting/client/key_event_m...
File remoting/client/key_event_mapper.cc (right):

http://codereview.chromium.org/10025001/diff/2001/remoting/client/key_event_m...
remoting/client/key_event_mapper.cc:26: trapped_keys.insert(usb_keycode);
On 2012/04/07 01:50:14, sergeyu wrote:
> Add {}

Done.

http://codereview.chromium.org/10025001/diff/2001/remoting/client/key_event_m...
remoting/client/key_event_mapper.cc:33: mapped_keys.erase(in_usb_keycode);
On 2012/04/07 01:50:14, sergeyu wrote:
> add {}

Done.

http://codereview.chromium.org/10025001/diff/2001/remoting/client/key_event_m...
remoting/client/key_event_mapper.cc:53: InputFilter::InjectKeyEvent(new_event);
On 2012/04/07 01:50:14, sergeyu wrote:
> Do we want to trap remapped keys? Probably not, just want to make sure.

I couldn't think of any situation where that would be useful and hard to
implement with trapKey+injectKeyEvent.

The only difference is that trapKey posts the event back to the web-app
asynchronously, so it's possible for things to get re-ordered if the user types
amazingly quickly.

http://codereview.chromium.org/10025001/diff/2001/remoting/client/key_event_m...
File remoting/client/key_event_mapper.h (right):

http://codereview.chromium.org/10025001/diff/2001/remoting/client/key_event_m...
remoting/client/key_event_mapper.h:18: typedef base::Callback<void(uint32,bool)>
KeyTrapCallback;
On 2012/04/07 01:50:14, sergeyu wrote:
> Maybe define this inside KeyEventMapper?
> nit: space after comma

Done.

http://codereview.chromium.org/10025001/diff/2001/remoting/client/key_event_m...
remoting/client/key_event_mapper.h:20: class KeyEventMapper : public
protocol::InputFilter {
On 2012/04/07 01:50:14, sergeyu wrote:
> Explain what this is for.

Done.

http://codereview.chromium.org/10025001/diff/2001/remoting/client/plugin/chro...
File remoting/client/plugin/chromoting_instance.cc (right):

http://codereview.chromium.org/10025001/diff/2001/remoting/client/plugin/chro...
remoting/client/plugin/chromoting_instance.cc:275: if
(!data->GetInteger("from_keycode", &from_keycode) ||
On 2012/04/07 01:50:14, sergeyu wrote:
> this should be fromKeycode and toKeycode

Done.

Unfortunately injectKeyEvent uses |usb_keycode|, so I'll have to fix that in a
follow-up CL & see if I can get that merged to M19. :(

http://codereview.chromium.org/10025001/diff/2001/remoting/client/plugin/chro...
remoting/client/plugin/chromoting_instance.cc:494:
data->SetInteger("usb_keycode", usb_keycode);
On 2012/04/07 01:50:14, sergeyu wrote:
> usbKeycode

Done.

[commit-bot: I haz the power, 2012-04-08 00:19:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wez@chromium.org/10025001/4002

[commit-bot: I haz the power, 2012-04-08 04:22:51]

Try job failure for 10025001-4002 (retry) on win_rel for step "browser_tests".
It's a second try, previously, step "browser_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2012-04-09 00:02:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wez@chromium.org/10025001/4002

[commit-bot: I haz the power, 2012-04-09 02:22:25]

Change committed as 131321