[Ozone] Fix disabling keyboard in maximized mode

[Ozone] Fix disabling keyboard in maximized mode

This CL:
- Fixes EventConverterEvdev::SetAllowedKeys() to block disallowed keys
- Dispatches a synthetic key release event for any blocked pressed keys when
  EventConverterEvdev::SetAllowedKeys() is called instead of waiting till the
  user releases the key.

BUG=465611
TEST=EventConverterEvdevImplTest.*

Committed: https://crrev.com/988070a509bbec03a4389a0d6a013ace1d765826
Cr-Commit-Position: refs/heads/master@{#320170}

[pkotwicz, 2015-03-10 23:56:53]

Patchset #1 (id:1) has been deleted

[pkotwicz, 2015-03-10 23:58:06]

Patchset #1 (id:20001) has been deleted

[pkotwicz, 2015-03-11 00:04:21]

Patchset #1 (id:40001) has been deleted

[pkotwicz, 2015-03-11 00:06:18]

Patchset #1 (id:60001) has been deleted

[pkotwicz, 2015-03-11 00:20:34]

pkotwicz@chromium.org changed reviewers:
+ spang@chromium.org

[pkotwicz, 2015-03-11 00:20:34]

spang@ PTAL

I release the pressed keys when EventConverterEvdev::SetAllowedKeys() is called
because
- I think it is more correct
- Because it makes it easier to keep
|GestureInterpreterLibevdevCros::prev_key_state_| when keys are disabled

[spang, 2015-03-11 18:52:29]

On 2015/03/11 00:20:34, pkotwicz wrote:
> spang@ PTAL
> 
> I release the pressed keys when EventConverterEvdev::SetAllowedKeys() is
called
> because
> - I think it is more correct
> - Because it makes it easier to keep
> |GestureInterpreterLibevdevCros::prev_key_state_| when keys are disabled

Can you explain what's wrong? The bug is pretty sparse.

[pkotwicz, 2015-03-11 19:17:18]

In EventConverterEvdevImpl::OnKeyChange(), "down && allowed_keys_ &&
allowed_keys_->count(key_code)" is wrong.

It should be:
"down && allowed_keys_ && !allowed_keys_->count(key_code)"

We have different handling in EventConverterEvdevImpl than in
GestureInterpreterLibevdevCros. So it is possible to get a key which repeats
forever if the keyboard is disabled via GestureInterpreterLibevdevCros.

[spang, 2015-03-11 19:20:20]

On 2015/03/11 19:17:18, pkotwicz wrote:
> In EventConverterEvdevImpl::OnKeyChange(), "down && allowed_keys_ &&
> allowed_keys_->count(key_code)" is wrong.
> 
> It should be:
> "down && allowed_keys_ && !allowed_keys_->count(key_code)"
> 

Doh!

> We have different handling in EventConverterEvdevImpl than in
> GestureInterpreterLibevdevCros. So it is possible to get a key which repeats
> forever if the keyboard is disabled via GestureInterpreterLibevdevCros.

Yeah, I know :(

But it's only for internal keyboards and they don't take this code ptah.

[spang, 2015-03-11 19:30:46]

On 2015/03/11 19:20:20, spang wrote:
> On 2015/03/11 19:17:18, pkotwicz wrote:
> > In EventConverterEvdevImpl::OnKeyChange(), "down && allowed_keys_ &&
> > allowed_keys_->count(key_code)" is wrong.
> > 
> > It should be:
> > "down && allowed_keys_ && !allowed_keys_->count(key_code)"
> > 
> 
> Doh!
> 
> > We have different handling in EventConverterEvdevImpl than in
> > GestureInterpreterLibevdevCros. So it is possible to get a key which repeats
> > forever if the keyboard is disabled via GestureInterpreterLibevdevCros.
> 
> Yeah, I know :(
> 
> But it's only for internal keyboards and they don't take this code ptah.

I think the gestures version is still buggy if you press and then release key
with the filter. Because the internal state gets updated when you press the key
even it's filtered, and then we'll allow the release through the filter in the
exception.

Fortunately duplicate releases are much less problematic than missing ones. So
the impact probably isn't bad.

But honestly I would almost rather just remove key blocking from the libevdev
path. It's more complicated than the other path AND it's dead code.

[spang, 2015-03-11 19:31:57]

https://codereview.chromium.org/992323004/diff/80001/ui/events/ozone/evdev/ev...
File ui/events/ozone/evdev/event_converter_evdev_impl.cc (right):

https://codereview.chromium.org/992323004/diff/80001/ui/events/ozone/evdev/ev...
ui/events/ozone/evdev/event_converter_evdev_impl.cc:96: 
// Release keys immediately even if they are still pressed.

[pkotwicz, 2015-03-11 21:19:36]

Patchset #2 (id:100001) has been deleted

[pkotwicz, 2015-03-11 21:23:40]

spang@, can you please take another look? I have completely removed the
implementation of GestureInterpreterLibevdevCros::SetAllowedKeys() and
GestureInterpreterLibevdevCros::AllowAllKeys() as you suggested

[spang, 2015-03-11 21:47:21]

lgtm

[pkotwicz, 2015-03-11 21:56:58]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2015-03-11 21:58:04]

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

[commit-bot: I haz the power, 2015-03-11 22:31:32]

Committed patchset #2 (id:120001)

[commit-bot: I haz the power, 2015-03-11 22:32:28]

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