Input Injection API on Ozone

Input Injection API on Ozone

This CL implements an input injection API (SystemInputInjector) on Ozone,
which would be used by Remote assistance.

SystemInputInjector is implemented by InputInjectorEvDev on EvDev.
It is created by EventFactoryEv so that it can share its
modifiers, keyboard and cursor state when generating corresponding
ui::Events.

BUG=436279
Committed: https://crrev.com/1bbda3e31177e98dc44147625ff82c0b7af400f0
Cr-Commit-Position: refs/heads/master@{#305764}

[kelvinp, 2014-11-25 01:00:28]

Patchset #1 (id:1) has been deleted

[kelvinp, 2014-11-25 01:04:12]

kelvinp@chromium.org changed reviewers:
+ kpschoedel@chromium.org, sadrul@chromium.org, spang@chromium.org

[kelvinp, 2014-11-25 04:22:01]

Patchset #1 (id:20001) has been deleted

[kelvinp, 2014-11-25 04:43:07]

kelvinp@chromium.org changed reviewers:
- kpschoedel@chromium.org

[kelvinp, 2014-11-25 04:43:08]

PTAL

spang@  This is roughly the same as the design that we talked about except the
two changes below:
1. A new instance of SystemInputInjector is created for each client.
2. I separated PostEvent() into different functions for each event type.
The rationale behind the changes are documented in the comments.
Do you think it is a good idea?
If you want to see how the new API's will be called.  You can take a look at
https://codereview.chromium.org/756643006

https://codereview.chromium.org/755883002/diff/40001/ui/events/ozone/evdev/ev...
File ui/events/ozone/evdev/event_factory_evdev.h (right):

https://codereview.chromium.org/755883002/diff/40001/ui/events/ozone/evdev/ev...
ui/events/ozone/evdev/event_factory_evdev.h:47: scoped_ptr<SystemInputInjector>
CreateSystemInputInjector();
Why CreateSystemInputInjector() instead of GetSystemInputInjector()?
A new instance of the input injector is created for each client so that we can
track the the modifiers applied by any particular client.  In this way, even if
a client sends a Ctrl down without a Ctrl Up, we can reset the modifiers when
the instance is destroyed to prevent modifier keys from getting stuck.

https://codereview.chromium.org/755883002/diff/40001/ui/ozone/public/system_i...
File ui/ozone/public/system_input_injector.h (right):

https://codereview.chromium.org/755883002/diff/40001/ui/ozone/public/system_i...
ui/ozone/public/system_input_injector.h:31: virtual void MoveCursorTo(const
gfx::PointF& location) = 0;
Why separate injection functions instead of one PostEvent?
I find the PostEvent(Event* event) signature too generic and can lead to
ambiguity like:
1. What event types are supported?
2. Should I inject a MouseMoved event or call MoveCursorTo?
3. What flags in the event are ignored?
4. Are the mouse location in device independent pixels or physical pixels.
Separating different event types into different function addresses those
ambiguity.

[kelvinp, 2014-11-25 04:53:46]

kelvinp@chromium.org changed reviewers:
+ kpschoedel@chromium.org

[kelvinp, 2014-11-25 04:53:47]

Unit test will be added in a later patch set.
kevin@ can you take a look at the signature of the InjectKeyPress API in
SytemInputInjector?

[kpschoedel, 2014-11-25 15:39:24]

https://codereview.chromium.org/755883002/diff/40001/ui/ozone/public/system_i...
File ui/ozone/public/system_input_injector.h (right):

https://codereview.chromium.org/755883002/diff/40001/ui/ozone/public/system_i...
ui/ozone/public/system_input_injector.h:44: virtual void InjectKeyPress(DomKey
physical_key, bool down) = 0;
ui::DomCode for physical keys.
ui/events/keycodes/dom3/dom_code.h

Practically speaking dom_code == static_cast<DomCode>(usb_code) but it might be
desirable to hide that behind a KeycodeConverter::UsbKeycodeToDomCode()
function.

[spang, 2014-11-25 19:58:29]

https://codereview.chromium.org/755883002/diff/40001/ui/events/ozone/evdev/in...
File ui/events/ozone/evdev/input_injector_evdev.cc (right):

https://codereview.chromium.org/755883002/diff/40001/ui/events/ozone/evdev/in...
ui/events/ozone/evdev/input_injector_evdev.cc:13:
InputInjectorEvDev::InputInjectorEvDev(EventModifiersEvdev* modifiers,
I've been spelling this "Evdev" - let's be consistent.

https://codereview.chromium.org/755883002/diff/40001/ui/events/ozone/evdev/in...
ui/events/ozone/evdev/input_injector_evdev.cc:39: LOG(FATAL) << "Invalid flags:
" << button << " for the button parameter";
fatal seems a little harsh.. maybe just warn?

https://codereview.chromium.org/755883002/diff/40001/ui/events/ozone/evdev/in...
ui/events/ozone/evdev/input_injector_evdev.cc:61:
modifiers_->GetModifierFlags())));
Why is changed_button_flags set to all modifiers (some of which may not even be
buttons)? I think just pass "0".

[kelvinp, 2014-11-25 21:15:20]

Patchset #2 (id:60001) has been deleted

[kelvinp, 2014-11-25 21:16:49]

PTAL

https://codereview.chromium.org/755883002/diff/40001/ui/events/ozone/evdev/in...
File ui/events/ozone/evdev/input_injector_evdev.cc (right):

https://codereview.chromium.org/755883002/diff/40001/ui/events/ozone/evdev/in...
ui/events/ozone/evdev/input_injector_evdev.cc:13:
InputInjectorEvDev::InputInjectorEvDev(EventModifiersEvdev* modifiers,
On 2014/11/25 19:58:29, spang wrote:
> I've been spelling this "Evdev" - let's be consistent.

Done.

https://codereview.chromium.org/755883002/diff/40001/ui/events/ozone/evdev/in...
ui/events/ozone/evdev/input_injector_evdev.cc:39: LOG(FATAL) << "Invalid flags:
" << button << " for the button parameter";
On 2014/11/25 19:58:29, spang wrote:
> fatal seems a little harsh.. maybe just warn?

Done.

https://codereview.chromium.org/755883002/diff/40001/ui/events/ozone/evdev/in...
ui/events/ozone/evdev/input_injector_evdev.cc:61:
modifiers_->GetModifierFlags())));
On 2014/11/25 19:58:29, spang wrote:
> Why is changed_button_flags set to all modifiers (some of which may not even
be
> buttons)? I think just pass "0".

Done.

https://codereview.chromium.org/755883002/diff/40001/ui/ozone/public/system_i...
File ui/ozone/public/system_input_injector.h (right):

https://codereview.chromium.org/755883002/diff/40001/ui/ozone/public/system_i...
ui/ozone/public/system_input_injector.h:44: virtual void InjectKeyPress(DomKey
physical_key, bool down) = 0;
On 2014/11/25 15:39:24, kpschoedel wrote:
> ui::DomCode for physical keys.
> ui/events/keycodes/dom3/dom_code.h
> 
> Practically speaking dom_code == static_cast<DomCode>(usb_code) but it might
be
> desirable to hide that behind a KeycodeConverter::UsbKeycodeToDomCode()
> function.
> 

Good idea.  I will add the function
KeycodeConverter::UsbKeycodeToDomCode()
in a separate change

[spang, 2014-11-25 21:47:06]

lgtm

https://codereview.chromium.org/755883002/diff/80001/ui/events/ozone/evdev/in...
File ui/events/ozone/evdev/input_injector_evdev.cc (right):

https://codereview.chromium.org/755883002/diff/80001/ui/events/ozone/evdev/in...
ui/events/ozone/evdev/input_injector_evdev.cc:46:
callback_.Run(make_scoped_ptr(new MouseEvent(
bad indent

[kelvinp, 2014-11-26 00:27:22]

Patchset #3 (id:100001) has been deleted

[kelvinp, 2014-11-26 00:27:26]

The CQ bit was checked by kelvinp@chromium.org

[commit-bot: I haz the power, 2014-11-26 00:28:36]

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

[commit-bot: I haz the power, 2014-11-26 02:39:42]

Committed patchset #3 (id:120001)

[commit-bot: I haz the power, 2014-11-26 02:40:31]

Patchset 3 (id:??) landed as
https://crrev.com/1bbda3e31177e98dc44147625ff82c0b7af400f0
Cr-Commit-Position: refs/heads/master@{#305764}