Fix input injector on Linux to release all keys before injecting text event.

Fix input injector on Linux to release all keys before injecting text event.

When '<' and '>' Android keyboard generates KEYCODE_SHIFT_LEFT followed by
KEYCODE_COMMA or KEYCODE_PERIOD. Android client sends shift event as a key
event and then the actual character is sent as a text event. As result
when the input injector tries to inject text event shift key is pressed
and as result injected keys are not interpreted correctly. Fixed input
injector to release all keys before trying to inject text events.

BUG=407463
Committed: https://crrev.com/90253bddc46a12973aee4c863dca037a29b4a1ed
Cr-Commit-Position: refs/heads/master@{#309508}

[Sergey Ulanov, 2014-12-22 22:51:27]

sergeyu@chromium.org changed reviewers:
+ lambroslambrou@chromium.org

[Sergey Ulanov, 2014-12-22 22:51:35]

Patchset #1 (id:1) has been deleted

[Lambros, 2014-12-22 23:05:11]

lgtm

https://codereview.chromium.org/806223008/diff/20001/remoting/host/input_inje...
File remoting/host/input_injector_x11.cc (right):

https://codereview.chromium.org/806223008/diff/20001/remoting/host/input_inje...
remoting/host/input_injector_x11.cc:333: for (std::set<int>::iterator it =
pressed_keys_.begin();
nit: Prefer const_iterator
Could use C++11 here: for (int key : pressed_keys_) ?

[Sergey Ulanov, 2014-12-23 00:13:28]

https://codereview.chromium.org/806223008/diff/20001/remoting/host/input_inje...
File remoting/host/input_injector_x11.cc (right):

https://codereview.chromium.org/806223008/diff/20001/remoting/host/input_inje...
remoting/host/input_injector_x11.cc:333: for (std::set<int>::iterator it =
pressed_keys_.begin();
On 2014/12/22 23:05:11, Lambros wrote:
> nit: Prefer const_iterator
> Could use C++11 here: for (int key : pressed_keys_) ?

Done.

[Sergey Ulanov, 2014-12-23 00:13:37]

The CQ bit was checked by sergeyu@chromium.org

[commit-bot: I haz the power, 2014-12-23 00:13:55]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/806223008/40001

[commit-bot: I haz the power, 2014-12-23 00:57:54]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2014-12-23 00:59:08]

Patchset 2 (id:??) landed as
https://crrev.com/90253bddc46a12973aee4c863dca037a29b4a1ed
Cr-Commit-Position: refs/heads/master@{#309508}

[Wez, 2014-12-23 17:31:42]

This problem will affect the other host platforms as well, surely?

Can we move this into an InputFilter so that the functionality is re-usable,
please?

It might also make more sense to implement this at the sender rather than in
each input injector.

[Sergey Ulanov, 2014-12-23 18:27:27]

On 2014/12/23 17:31:42, Wez wrote:
> This problem will affect the other host platforms as well, surely?

No. On Mac and Windows there are APIs to inject Unicode text. It's only problem
with X11 where we have to emulate key presses for each key to inject a
TextEvent. 

> 
> Can we move this into an InputFilter so that the functionality is re-usable,
> please?

I don't think we need this in other places, as far as I can tell.

> 
> It might also make more sense to implement this at the sender rather than in
> each input injector.

Currently the protocol doesn't define how TextEvent interacts with KeyEvent. We
could require on the protocol layer that TextEvent is sent only when no keys are
pressed, but then we will still need to check it on the host side. Because this
is a Linux-specific restrictions I think it's better to just make the host
handle this case properly.

[Wez, 2014-12-24 17:19:17]

On 2014/12/23 18:27:27, Sergey Ulanov wrote:
> On 2014/12/23 17:31:42, Wez wrote:
> > This problem will affect the other host platforms as well, surely?
> 
> No. On Mac and Windows there are APIs to inject Unicode text. It's only
problem
> with X11 where we have to emulate key presses for each key to inject a
> TextEvent. 
> 
> > 
> > Can we move this into an InputFilter so that the functionality is re-usable,
> > please?
> 
> I don't think we need this in other places, as far as I can tell.
> 
> > 
> > It might also make more sense to implement this at the sender rather than in
> > each input injector.
> 
> Currently the protocol doesn't define how TextEvent interacts with KeyEvent.
We
> could require on the protocol layer that TextEvent is sent only when no keys
are
> pressed, but then we will still need to check it on the host side. Because
this
> is a Linux-specific restrictions I think it's better to just make the host
> handle this case properly.

The issue on other platforms is how applications handle direct text input events
in the context of modifiers apparently being pressed, even if the OS itself
happens not to get too confused.