Make ui::test::EventGenerator work with PointerEvents in mash

Make ui::test::EventGenerator work with PointerEvents in mash

In classic ash PointerWatchers are triggered by an EventHandler attached
to ash::Shell and are triggered in unit tests using EventGenerator.

In mash PointerWatchers are triggered by a PointerWatcherEventRouter
owned by ash::mus::WindowManager. That code path wasn't getting hit in
unit tests using EventGenerator.

Wiring it up fixes SystemTrayTest.PersistentBubble in mash. It will
probably fix several other tests disabled in mash. We'll find those as
we look at them individually -- there's no simple way to re-enable all
those tests to try them.

BUG=693790, 695559
TEST=ash_unittests, mash_unittests SystemTrayTest.PersisentBubble

Review-Url: https://codereview.chromium.org/2752903002
Cr-Commit-Position: refs/heads/master@{#457677}
Committed: https://chromium.googlesource.com/chromium/src/+/03ae5b6069bab8e87953072eeb75a192b6e6d6aa

[James Cook, 2017-03-15 18:47:53]

The CQ bit was checked by jamescook@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-15 18:48:20]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[James Cook, 2017-03-15 19:01:19]

jamescook@chromium.org changed reviewers:
+ sadrul@chromium.org

[James Cook, 2017-03-15 19:01:20]

sadrul, please take a look.

Ideally this would call into WindowTreeClient, but I looked at that and it's
hard to wire up. This approach runs the PointerWatcher code pathway starting
just above WindowTreeClient (WindowManager handles pointer watchers as a
WindowTreeClientDelegate).

https://codereview.chromium.org/2752903002/diff/1/ash/test/ash_test_base.cc
File ash/test/ash_test_base.cc (right):

https://codereview.chromium.org/2752903002/diff/1/ash/test/ash_test_base.cc#n...
ash/test/ash_test_base.cc:98: ->OnPointerEventObserved(event,
static_cast<aura::Window*>(target));
I think this cast is OK -- aura::test::EventGeneratorDelegateAura (the base
class) does this cast in several places.

[commit-bot: I haz the power, 2017-03-15 19:38:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-15 19:38:28]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[James Cook, 2017-03-15 21:03:53]

The CQ bit was checked by jamescook@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-15 21:04:38]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-15 21:39:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-15 21:39:50]

Dry run: This issue passed the CQ dry run.

[sadrul, 2017-03-16 03:22:47]

https://codereview.chromium.org/2752903002/diff/1/ash/test/ash_test_base.cc
File ash/test/ash_test_base.cc (right):

https://codereview.chromium.org/2752903002/diff/1/ash/test/ash_test_base.cc#n...
ash/test/ash_test_base.cc:88: const ui::PointerEvent& event) override {
Instead of doing this here, can you override this in EventGeneratorDelegateAura?

EventGeneratorDelegateAura can be changed to retain a pointer to the
WindowTreeClientDelegate, which gets set during construction (or right after
construction), which it then uses to trigger the watchers. This would allow this
to be used for tests in views too (see https://crbug.com/615033 for example).

[James Cook, 2017-03-16 19:10:44]

The CQ bit was checked by jamescook@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-16 19:11:38]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[James Cook, 2017-03-16 19:13:32]

Sadrul, please take another look.

https://codereview.chromium.org/2752903002/diff/1/ash/test/ash_test_base.cc
File ash/test/ash_test_base.cc (right):

https://codereview.chromium.org/2752903002/diff/1/ash/test/ash_test_base.cc#n...
ash/test/ash_test_base.cc:88: const ui::PointerEvent& event) override {
On 2017/03/16 03:22:47, sadrul wrote:
> Instead of doing this here, can you override this in
EventGeneratorDelegateAura?
> 
> EventGeneratorDelegateAura can be changed to retain a pointer to the
> WindowTreeClientDelegate, which gets set during construction (or right after
> construction), which it then uses to trigger the watchers. This would allow
this
> to be used for tests in views too (see https://crbug.com/615033 for example).

Great idea! By doing that I was able to inject the events directly into
WindowTreeClient, so it's even more similar to the production code path.

https://codereview.chromium.org/2752903002/diff/20001/ui/aura/env.h
File ui/aura/env.h (right):

https://codereview.chromium.org/2752903002/diff/20001/ui/aura/env.h#newcode151
ui/aura/env.h:151: // Intentionally not exposed publicly. Someday we might want
to support
I talked to Scott and he didn't want to expose this publicly in production but
said he was OK with exposing it for tests.

[commit-bot: I haz the power, 2017-03-16 20:41:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-16 20:41:29]

Dry run: This issue passed the CQ dry run.

[sadrul, 2017-03-17 00:08:36]

One nit. lgtm!

https://codereview.chromium.org/2752903002/diff/20001/ui/events/test/event_ge...
File ui/events/test/event_generator.h (right):

https://codereview.chromium.org/2752903002/diff/20001/ui/events/test/event_ge...
ui/events/test/event_generator.h:442: void MaybeDispatchToPointerWatchers(Event*
event);
I think MaybeDispatchToPointerWatchers() never mutates |event|, right? Can this
a const& instead of a pointer?

[James Cook, 2017-03-17 02:43:17]

Thanks for the review.

https://codereview.chromium.org/2752903002/diff/20001/ui/events/test/event_ge...
File ui/events/test/event_generator.h (right):

https://codereview.chromium.org/2752903002/diff/20001/ui/events/test/event_ge...
ui/events/test/event_generator.h:442: void MaybeDispatchToPointerWatchers(Event*
event);
On 2017/03/17 00:08:36, sadrul wrote:
> I think MaybeDispatchToPointerWatchers() never mutates |event|, right? Can
this
> a const& instead of a pointer?

Done.

[James Cook, 2017-03-17 02:43:41]

The CQ bit was checked by jamescook@chromium.org

[James Cook, 2017-03-17 02:43:42]

The patchset sent to the CQ was uploaded after l-g-t-m from sadrul@chromium.org
Link to the patchset: https://codereview.chromium.org/2752903002/#ps40001
(title: "review comments")

[commit-bot: I haz the power, 2017-03-17 02:44:05]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-17 03:40:07]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1489718621833420,
"parent_rev": "0828f5b83f4835d9772dab5bd0bea6d8d9704737", "commit_rev":
"03ae5b6069bab8e87953072eeb75a192b6e6d6aa"}

[commit-bot: I haz the power, 2017-03-17 03:41:17]

Description was changed from

==========
Make ui::test::EventGenerator work with PointerEvents in mash

In classic ash PointerWatchers are triggered by an EventHandler attached
to ash::Shell and are triggered in unit tests using EventGenerator.

In mash PointerWatchers are triggered by a PointerWatcherEventRouter
owned by ash::mus::WindowManager. That code path wasn't getting hit in
unit tests using EventGenerator.

Wiring it up fixes SystemTrayTest.PersistentBubble in mash. It will
probably fix several other tests disabled in mash. We'll find those as
we look at them individually -- there's no simple way to re-enable all
those tests to try them.

BUG=693790,695559
TEST=ash_unittests, mash_unittests SystemTrayTest.PersisentBubble
==========

to

==========
Make ui::test::EventGenerator work with PointerEvents in mash

In classic ash PointerWatchers are triggered by an EventHandler attached
to ash::Shell and are triggered in unit tests using EventGenerator.

In mash PointerWatchers are triggered by a PointerWatcherEventRouter
owned by ash::mus::WindowManager. That code path wasn't getting hit in
unit tests using EventGenerator.

Wiring it up fixes SystemTrayTest.PersistentBubble in mash. It will
probably fix several other tests disabled in mash. We'll find those as
we look at them individually -- there's no simple way to re-enable all
those tests to try them.

BUG=693790,695559
TEST=ash_unittests, mash_unittests SystemTrayTest.PersisentBubble

Review-Url: https://codereview.chromium.org/2752903002
Cr-Commit-Position: refs/heads/master@{#457677}
Committed:
https://chromium.googlesource.com/chromium/src/+/03ae5b6069bab8e87953072eeb75...
==========

[commit-bot: I haz the power, 2017-03-17 03:41:18]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/03ae5b6069bab8e87953072eeb75...