New issue
Advanced search Search tips

Issue 852439 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 26
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug

Blocking:
issue 814675
issue 837684



Sign in to add a comment

Provide implementation of EventInjector for ws2

Project Member Reported by sky@chromium.org, Jun 13 2018

Issue description

This is used by remote desktop and some tests.
 

Comment 1 by sky@chromium.org, Jun 13 2018

Blocking: 837684

Comment 2 by sky@chromium.org, Jun 15 2018

Owner: sky@chromium.org
Status: Started (was: Untriaged)

Comment 3 by sky@chromium.org, Jun 22 2018

Owner: ----
Status: Available (was: Started)
I started this, but didn't getting very far. This is going to be mildly tricky. In particular the code is going to have to detect the case of event being dispatched locally, remotely and not at all (there is some code that caches and dispatches later). In the remote case we'll have to start using real event ids again, and have the injector watch for the ack.

I will grab this again when back if no one finishes it before then.
Owner: sky@chromium.org
Status: Started (was: Available)
Blocking: 814675
Project Member

Comment 6 by bugdroid1@chromium.org, Jul 20

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d6e0e783886b8fbb81bba501f1a2c0fb4f06e1e8

commit d6e0e783886b8fbb81bba501f1a2c0fb4f06e1e8
Author: Scott Violet <sky@chromium.org>
Date: Fri Jul 20 00:12:09 2018

chromeos: creates RemotingEventInjector

ui::mojom::EventInjector served two purposes:
1. for testing code to inject events.
2. as an implementation of SystemInputInjector

The needs of SystemInputInjector are more narrow, and better served with a
direct API that does not have a callback as well as unnecessary conversion.

My plan for 1 is to rename EventInjector to TestEventInjector and hopefully
make only available for tests.

BUG= 852439 
TEST=none

Change-Id: I29624aaf4a4943003d5d54ef6f9a75ef76d63130
Reviewed-on: https://chromium-review.googlesource.com/1142542
Reviewed-by: Michael Wasserman <msw@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576706}
[modify] https://crrev.com/d6e0e783886b8fbb81bba501f1a2c0fb4f06e1e8/ash/ws/window_service_delegate_impl.cc
[modify] https://crrev.com/d6e0e783886b8fbb81bba501f1a2c0fb4f06e1e8/ash/ws/window_service_delegate_impl.h
[modify] https://crrev.com/d6e0e783886b8fbb81bba501f1a2c0fb4f06e1e8/services/ui/manifest.json
[modify] https://crrev.com/d6e0e783886b8fbb81bba501f1a2c0fb4f06e1e8/services/ui/public/interfaces/BUILD.gn
[modify] https://crrev.com/d6e0e783886b8fbb81bba501f1a2c0fb4f06e1e8/services/ui/public/interfaces/event_injector.mojom
[add] https://crrev.com/d6e0e783886b8fbb81bba501f1a2c0fb4f06e1e8/services/ui/public/interfaces/remoting_event_injector.mojom
[modify] https://crrev.com/d6e0e783886b8fbb81bba501f1a2c0fb4f06e1e8/services/ui/ws2/BUILD.gn
[add] https://crrev.com/d6e0e783886b8fbb81bba501f1a2c0fb4f06e1e8/services/ui/ws2/remoting_event_injector.cc
[add] https://crrev.com/d6e0e783886b8fbb81bba501f1a2c0fb4f06e1e8/services/ui/ws2/remoting_event_injector.h
[modify] https://crrev.com/d6e0e783886b8fbb81bba501f1a2c0fb4f06e1e8/services/ui/ws2/window_service.cc
[modify] https://crrev.com/d6e0e783886b8fbb81bba501f1a2c0fb4f06e1e8/services/ui/ws2/window_service.h
[modify] https://crrev.com/d6e0e783886b8fbb81bba501f1a2c0fb4f06e1e8/services/ui/ws2/window_service_delegate.cc
[modify] https://crrev.com/d6e0e783886b8fbb81bba501f1a2c0fb4f06e1e8/services/ui/ws2/window_service_delegate.h
[modify] https://crrev.com/d6e0e783886b8fbb81bba501f1a2c0fb4f06e1e8/ui/aura/env.cc
[modify] https://crrev.com/d6e0e783886b8fbb81bba501f1a2c0fb4f06e1e8/ui/aura/mus/system_input_injector_mus.cc
[modify] https://crrev.com/d6e0e783886b8fbb81bba501f1a2c0fb4f06e1e8/ui/aura/mus/system_input_injector_mus.h
[modify] https://crrev.com/d6e0e783886b8fbb81bba501f1a2c0fb4f06e1e8/ui/aura/mus/system_input_injector_mus_unittest.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Jul 24

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/38fe288f2e99faa56772ed0e7b3eb709d4258dcd

commit 38fe288f2e99faa56772ed0e7b3eb709d4258dcd
Author: Scott Violet <sky@chromium.org>
Date: Tue Jul 24 23:02:08 2018

chromeos: creates TestEventInjector for tests to inject events

This interface is *only* exposed in tests. It's to be used for tests that
want to generate events and ensure they are processed.

My plan is remove the existing interface
(services/ui/public/interfaces/event_injector.mojom) soonish.

BUG= 852439 
TEST=covered by tests

Change-Id: I2df1e5d428564f78b7280087a4f5453ce76b55b7
Reviewed-on: https://chromium-review.googlesource.com/1144472
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577724}
[modify] https://crrev.com/38fe288f2e99faa56772ed0e7b3eb709d4258dcd/ash/BUILD.gn
[modify] https://crrev.com/38fe288f2e99faa56772ed0e7b3eb709d4258dcd/ash/mojo_test_interface_factory.cc
[modify] https://crrev.com/38fe288f2e99faa56772ed0e7b3eb709d4258dcd/ash/mojo_test_interface_factory.h
[add] https://crrev.com/38fe288f2e99faa56772ed0e7b3eb709d4258dcd/ash/mojo_test_interface_factory_unittest.cc
[modify] https://crrev.com/38fe288f2e99faa56772ed0e7b3eb709d4258dcd/ash/test/ash_test_helper.cc
[modify] https://crrev.com/38fe288f2e99faa56772ed0e7b3eb709d4258dcd/ash/test/ash_test_helper.h
[modify] https://crrev.com/38fe288f2e99faa56772ed0e7b3eb709d4258dcd/ash/ws/window_service_owner.cc
[modify] https://crrev.com/38fe288f2e99faa56772ed0e7b3eb709d4258dcd/ash/ws/window_service_owner.h
[modify] https://crrev.com/38fe288f2e99faa56772ed0e7b3eb709d4258dcd/chrome/test/base/chrome_test_launcher.cc
[modify] https://crrev.com/38fe288f2e99faa56772ed0e7b3eb709d4258dcd/services/ui/manifest.json
[modify] https://crrev.com/38fe288f2e99faa56772ed0e7b3eb709d4258dcd/services/ui/public/interfaces/BUILD.gn
[add] https://crrev.com/38fe288f2e99faa56772ed0e7b3eb709d4258dcd/services/ui/public/interfaces/test_event_injector.mojom
[modify] https://crrev.com/38fe288f2e99faa56772ed0e7b3eb709d4258dcd/services/ui/ws2/BUILD.gn
[add] https://crrev.com/38fe288f2e99faa56772ed0e7b3eb709d4258dcd/services/ui/ws2/injected_event_handler.cc
[add] https://crrev.com/38fe288f2e99faa56772ed0e7b3eb709d4258dcd/services/ui/ws2/injected_event_handler.h
[add] https://crrev.com/38fe288f2e99faa56772ed0e7b3eb709d4258dcd/services/ui/ws2/injected_event_handler_unittest.cc
[add] https://crrev.com/38fe288f2e99faa56772ed0e7b3eb709d4258dcd/services/ui/ws2/test_event_injector.cc
[add] https://crrev.com/38fe288f2e99faa56772ed0e7b3eb709d4258dcd/services/ui/ws2/test_event_injector.h
[add] https://crrev.com/38fe288f2e99faa56772ed0e7b3eb709d4258dcd/services/ui/ws2/test_event_injector_delegate.h
[modify] https://crrev.com/38fe288f2e99faa56772ed0e7b3eb709d4258dcd/services/ui/ws2/window_service.cc
[modify] https://crrev.com/38fe288f2e99faa56772ed0e7b3eb709d4258dcd/services/ui/ws2/window_service.h
[add] https://crrev.com/38fe288f2e99faa56772ed0e7b3eb709d4258dcd/services/ui/ws2/window_service_observer.h
[add] https://crrev.com/38fe288f2e99faa56772ed0e7b3eb709d4258dcd/services/ui/ws2/window_service_observer_unittest.cc
[modify] https://crrev.com/38fe288f2e99faa56772ed0e7b3eb709d4258dcd/services/ui/ws2/window_tree.cc
[modify] https://crrev.com/38fe288f2e99faa56772ed0e7b3eb709d4258dcd/services/ui/ws2/window_tree.h
[modify] https://crrev.com/38fe288f2e99faa56772ed0e7b3eb709d4258dcd/ui/aura/window_event_dispatcher.cc
[modify] https://crrev.com/38fe288f2e99faa56772ed0e7b3eb709d4258dcd/ui/aura/window_event_dispatcher_observer.h

Project Member

Comment 8 by bugdroid1@chromium.org, Jul 25

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fedcd311c7e6f09ec90e2b7b4fce8d098312897e

commit fedcd311c7e6f09ec90e2b7b4fce8d098312897e
Author: Scott Violet <sky@chromium.org>
Date: Wed Jul 25 05:35:31 2018

chromeos: use TestEventInjector instead of EventInjector

And move some code to test only.

BUG= 852439 
TEST=none

Change-Id: I690fa56d770b27aa147c32e4d5634208d95a6786
Reviewed-on: https://chromium-review.googlesource.com/1149099
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577809}
[modify] https://crrev.com/fedcd311c7e6f09ec90e2b7b4fce8d098312897e/services/ui/test_ws/BUILD.gn
[modify] https://crrev.com/fedcd311c7e6f09ec90e2b7b4fce8d098312897e/services/ui/test_ws/test_ws.cc
[modify] https://crrev.com/fedcd311c7e6f09ec90e2b7b4fce8d098312897e/services/ui/ws2/test_event_injector_delegate.h
[modify] https://crrev.com/fedcd311c7e6f09ec90e2b7b4fce8d098312897e/ui/aura/BUILD.gn
[modify] https://crrev.com/fedcd311c7e6f09ec90e2b7b4fce8d098312897e/ui/aura/test/ui_controls_factory_ozone.cc
[modify] https://crrev.com/fedcd311c7e6f09ec90e2b7b4fce8d098312897e/ui/views/mus/BUILD.gn
[modify] https://crrev.com/fedcd311c7e6f09ec90e2b7b4fce8d098312897e/ui/views/mus/drag_interactive_uitest.cc
[modify] https://crrev.com/fedcd311c7e6f09ec90e2b7b4fce8d098312897e/ui/views/mus/mus_client.cc
[modify] https://crrev.com/fedcd311c7e6f09ec90e2b7b4fce8d098312897e/ui/views/mus/mus_client.h
[modify] https://crrev.com/fedcd311c7e6f09ec90e2b7b4fce8d098312897e/ui/views/mus/views_mus_test_suite.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Jul 26

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dc8e78ba30a096437670e03e2bbe21e81558419d

commit dc8e78ba30a096437670e03e2bbe21e81558419d
Author: Scott Violet <sky@chromium.org>
Date: Thu Jul 26 16:03:32 2018

chromeos: nukes TestEventInjector in favor of EventInjector

I had created TestEventInjector in hopes of being able to get rid of
EventInjector, but that isn't possible. We rely on event injector for two real
world cases: autoclick (an a11y feature that can generate mouse clicks) and
for telemetry. Both of these need event injection enabled in production code.

The telemetry and autoclick cases don't care about being notified when
processing is completed, so I introduced a special function for this case. I
am doing this because the code needed to verify an event has been processed is
non-trivial, and I would like to avoid running it when possible.

BUG= 852439 
TEST=none

Change-Id: I2691151ffaa9b6519f7462582d0f1aadd2010850
Reviewed-on: https://chromium-review.googlesource.com/1150503
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578323}
[modify] https://crrev.com/dc8e78ba30a096437670e03e2bbe21e81558419d/ash/BUILD.gn
[modify] https://crrev.com/dc8e78ba30a096437670e03e2bbe21e81558419d/ash/components/autoclick/autoclick_application.cc
[modify] https://crrev.com/dc8e78ba30a096437670e03e2bbe21e81558419d/ash/mojo_test_interface_factory.cc
[modify] https://crrev.com/dc8e78ba30a096437670e03e2bbe21e81558419d/ash/mojo_test_interface_factory.h
[delete] https://crrev.com/1f19d9f92ac70daa935d42babb3f3e3b3f5cc23b/ash/mojo_test_interface_factory_unittest.cc
[modify] https://crrev.com/dc8e78ba30a096437670e03e2bbe21e81558419d/ash/test/ash_test_helper.cc
[modify] https://crrev.com/dc8e78ba30a096437670e03e2bbe21e81558419d/ash/ws/window_service_delegate_impl.cc
[modify] https://crrev.com/dc8e78ba30a096437670e03e2bbe21e81558419d/ash/ws/window_service_delegate_impl.h
[modify] https://crrev.com/dc8e78ba30a096437670e03e2bbe21e81558419d/ash/ws/window_service_owner.cc
[modify] https://crrev.com/dc8e78ba30a096437670e03e2bbe21e81558419d/ash/ws/window_service_owner.h
[modify] https://crrev.com/dc8e78ba30a096437670e03e2bbe21e81558419d/chrome/test/base/chrome_test_launcher.cc
[modify] https://crrev.com/dc8e78ba30a096437670e03e2bbe21e81558419d/services/ui/manifest.json
[modify] https://crrev.com/dc8e78ba30a096437670e03e2bbe21e81558419d/services/ui/public/interfaces/BUILD.gn
[modify] https://crrev.com/dc8e78ba30a096437670e03e2bbe21e81558419d/services/ui/public/interfaces/event_injector.mojom
[delete] https://crrev.com/1f19d9f92ac70daa935d42babb3f3e3b3f5cc23b/services/ui/public/interfaces/test_event_injector.mojom
[modify] https://crrev.com/dc8e78ba30a096437670e03e2bbe21e81558419d/services/ui/test_ws/BUILD.gn
[modify] https://crrev.com/dc8e78ba30a096437670e03e2bbe21e81558419d/services/ui/test_ws/test_ws.cc
[modify] https://crrev.com/dc8e78ba30a096437670e03e2bbe21e81558419d/services/ui/ws2/BUILD.gn
[add] https://crrev.com/dc8e78ba30a096437670e03e2bbe21e81558419d/services/ui/ws2/event_injector.cc
[add] https://crrev.com/dc8e78ba30a096437670e03e2bbe21e81558419d/services/ui/ws2/event_injector.h
[modify] https://crrev.com/dc8e78ba30a096437670e03e2bbe21e81558419d/services/ui/ws2/injected_event_handler.h
[delete] https://crrev.com/1f19d9f92ac70daa935d42babb3f3e3b3f5cc23b/services/ui/ws2/test_event_injector.cc
[delete] https://crrev.com/1f19d9f92ac70daa935d42babb3f3e3b3f5cc23b/services/ui/ws2/test_event_injector.h
[delete] https://crrev.com/1f19d9f92ac70daa935d42babb3f3e3b3f5cc23b/services/ui/ws2/test_event_injector_delegate.h
[modify] https://crrev.com/dc8e78ba30a096437670e03e2bbe21e81558419d/services/ui/ws2/window_service.cc
[modify] https://crrev.com/dc8e78ba30a096437670e03e2bbe21e81558419d/services/ui/ws2/window_service.h
[modify] https://crrev.com/dc8e78ba30a096437670e03e2bbe21e81558419d/services/ui/ws2/window_service_delegate.cc
[modify] https://crrev.com/dc8e78ba30a096437670e03e2bbe21e81558419d/services/ui/ws2/window_service_delegate.h
[modify] https://crrev.com/dc8e78ba30a096437670e03e2bbe21e81558419d/ui/aura/BUILD.gn
[modify] https://crrev.com/dc8e78ba30a096437670e03e2bbe21e81558419d/ui/aura/event_injector.cc
[modify] https://crrev.com/dc8e78ba30a096437670e03e2bbe21e81558419d/ui/aura/test/ui_controls_factory_ozone.cc
[modify] https://crrev.com/dc8e78ba30a096437670e03e2bbe21e81558419d/ui/views/mus/BUILD.gn
[modify] https://crrev.com/dc8e78ba30a096437670e03e2bbe21e81558419d/ui/views/mus/drag_interactive_uitest.cc

Status: Fixed (was: Started)

Sign in to add a comment