New issue
Advanced search Search tips

Issue 872450 link

Starred by 1 user
Status: Fixed
Owner:
msw@chromium.org
Closed: Oct 2
Cc:
jamescook@chromium.org
sky@chromium.org
est...@chromium.org
mustash-bugs@google.com
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

Convert PointerWatchers in ash back to ui::EventHandler

Project Member Reported by jamescook@chromium.org, Aug 8 
Now that mash runs the window service code inside the ash process, ash can inspect all incoming events and optionally cancel them. See  issue 646998 .

This means we only need PointerWatcher in remote mojo apps and in the browser. We don't need it inside ash.

These can be switched back to EventHandler:

https://cs.chromium.org/search/?q=f:src/ash+-f:src/ash/components+public.*PointerWatcher&sq=package:chromium&type=cs

Amusingly, this will delete my mustash project starter bug, and I will have come full circle.

Comment 1 by jamescook@chromium.org, Aug 14

Labels: -Proj-Mustash Proj-Mash-WS2

Comment 2 by jamescook@chromium.org, Aug 22

Status: Started (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 23 

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

commit 1723281f4514b58fe1a1ef17c66a92e79517827c
Author: James Cook <jamescook@chromium.org>
Date: Thu Aug 23 19:48:28 2018

chromeos: Convert OverflowBubble to ui::EventHandler

Now that ash and the ui service are in the same process we don't need
to use PointerWatcher in ash anymore.

This is a partial manual revert of:
https://codereview.chromium.org/1934123004/

Bug:  595851 ,  872450 
Test: ash_unittests
Change-Id: Ifbbb407210d0b3ec8d20993bff7a0f2cf44ba38e
Reviewed-on: https://chromium-review.googlesource.com/1186032
Commit-Queue: James Cook <jamescook@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585583}
[modify] https://crrev.com/1723281f4514b58fe1a1ef17c66a92e79517827c/ash/shelf/overflow_bubble.cc
[modify] https://crrev.com/1723281f4514b58fe1a1ef17c66a92e79517827c/ash/shelf/overflow_bubble.h
Project Member

Comment 4 by bugdroid1@chromium.org, Sep 24 

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

commit 455fc184db1479464a06ef4101daa0e06a923341
Author: James Cook <jamescook@chromium.org>
Date: Mon Sep 24 19:32:05 2018

chromeos: Convert PointerMetricsRecorder to ui::EventHandler

PointerWatchers aren't necessary in ash anymore. ui::EventHandler does
everything that PointerWatcher can do, plus can mutate events. Once
we eliminate PointerWatcher from ash we can delete some glue code I
added to make them work.

Also drop support for ui::EventPointerType::POINTER_TYPE_UNKNOWN,
which is never recorded in practice. (I don't think we ever generate
events with this type.)

Bug:  872450 ,  786214 
Test: ash_unittests
Change-Id: I08f781dd6e26b34870e7947c3a14e42edaf91080
Reviewed-on: https://chromium-review.googlesource.com/1239455
Reviewed-by: Xiaoyin Hu <xiaoyinh@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593631}
[modify] https://crrev.com/455fc184db1479464a06ef4101daa0e06a923341/ash/metrics/pointer_metrics_recorder.cc
[modify] https://crrev.com/455fc184db1479464a06ef4101daa0e06a923341/ash/metrics/pointer_metrics_recorder.h
[modify] https://crrev.com/455fc184db1479464a06ef4101daa0e06a923341/ash/metrics/pointer_metrics_recorder_unittest.cc

Comment 5 by jamescook@chromium.org, Sep 26

Cc: msw@chromium.org
+msw fyi
Project Member

Comment 6 by bugdroid1@chromium.org, Sep 27 

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

commit bde297a394226078b17dba89963edaa80e928953
Author: Mike Wasserman <msw@chromium.org>
Date: Thu Sep 27 19:51:28 2018

ash: Use EventHandler instead of PointerWatcher in TrayEventFilter

Partial revert of https://codereview.chromium.org/1918183003
PointerWatcher is no longer needed with Ash hosting the Window Service.

Bug:  872450 
Test: No regressions with clicks inside/outside system tray bubbles.
Change-Id: I987ee0aab77cd2c0efcd1106c7c30e0663349654
Reviewed-on: https://chromium-review.googlesource.com/1249902
Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594824}
[modify] https://crrev.com/bde297a394226078b17dba89963edaa80e928953/ash/system/tray/tray_event_filter.cc
[modify] https://crrev.com/bde297a394226078b17dba89963edaa80e928953/ash/system/tray/tray_event_filter.h
[modify] https://crrev.com/bde297a394226078b17dba89963edaa80e928953/ash/system/tray/tray_event_filter_unittest.cc
Project Member

Comment 7 by bugdroid1@chromium.org, Sep 28 

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

commit 282af28e868da371103680453127d09afdeb2f19
Author: Mike Wasserman <msw@chromium.org>
Date: Fri Sep 28 02:00:25 2018

ash: Use EventHandler instead of PointerWatcher in ShelfTooltipManager

PointerWatcher is no longer needed with Ash hosting the Window Service.
Some minor cleanup.

Bug:  872450 
Test: No regressions with shelf tooltip behavior.
Change-Id: I8d1b2bf008c54ae921dcabc67dc236d7d5801e11
Reviewed-on: https://chromium-review.googlesource.com/1250048
Commit-Queue: Michael Wasserman <msw@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594954}
[modify] https://crrev.com/282af28e868da371103680453127d09afdeb2f19/ash/shelf/shelf_tooltip_manager.cc
[modify] https://crrev.com/282af28e868da371103680453127d09afdeb2f19/ash/shelf/shelf_tooltip_manager.h
[modify] https://crrev.com/282af28e868da371103680453127d09afdeb2f19/ash/shelf/shelf_tooltip_manager_unittest.cc
[modify] https://crrev.com/282af28e868da371103680453127d09afdeb2f19/ash/shelf/shelf_view.cc
[modify] https://crrev.com/282af28e868da371103680453127d09afdeb2f19/ash/shelf/shelf_view.h
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 1 

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

commit 257312a525f9b7dadc54e5607753ce05d0e7cd39
Author: Mike Wasserman <msw@chromium.org>
Date: Mon Oct 01 23:11:53 2018

ash: Use EventHandler instead of PointerWatcher in PaletteTray

PointerWatcher is no longer needed with Ash hosting the Window Service.
Add StylusEventHandler to notify PaletteTray on stylus events.
Fix odd logic for showing the bubble; remove event handler when done.

Convert PaletteWelcomeBubble to EventHandler and simplify show/close.
Update tests and perform some minor cleanup.

Bug:  872450 
Test: No (bad) behavior changes in palette tray / stylus welcome bubble.
Change-Id: If8d6da88d6a70979978f227f10ebb8357fb0d34e
Reviewed-on: https://chromium-review.googlesource.com/1252910
Commit-Queue: Michael Wasserman <msw@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Sammie Quon <sammiequon@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595628}
[modify] https://crrev.com/257312a525f9b7dadc54e5607753ce05d0e7cd39/ash/system/palette/palette_tray.cc
[modify] https://crrev.com/257312a525f9b7dadc54e5607753ce05d0e7cd39/ash/system/palette/palette_tray.h
[modify] https://crrev.com/257312a525f9b7dadc54e5607753ce05d0e7cd39/ash/system/palette/palette_tray_unittest.cc
[modify] https://crrev.com/257312a525f9b7dadc54e5607753ce05d0e7cd39/ash/system/palette/palette_welcome_bubble.cc
[modify] https://crrev.com/257312a525f9b7dadc54e5607753ce05d0e7cd39/ash/system/palette/palette_welcome_bubble.h
[modify] https://crrev.com/257312a525f9b7dadc54e5607753ce05d0e7cd39/ash/system/palette/palette_welcome_bubble_unittest.cc

Comment 9 by jamescook@chromium.org, Oct 2

Cc: -msw@chromium.org jamescook@chromium.org
Owner: msw@chromium.org
msw is working on this.

Mike, if there are any you'd like me to take care of, just let me know.

Comment 10 by msw@chromium.org, Oct 2

Cc: est...@chromium.org
Status: Fixed (was: Started)
The only remaining instances in ash code are the tap visualizer (needed) and those related to immersive, which I think also need to be PointerWatchers (for client apps like the browser to show/hide the frame...).

I think this is fixed, but reopen if not. I'm looking at  Issue 887725  now.

Comment 11 by jamescook@chromium.org, Oct 2 

If all the PointerWatchers in //ash (not //ash/components) are gone then ash::PointerWatcherAdapter can be removed.

Comment 12 by msw@chromium.org, Oct 2 

Indeed, that's what I hoped for, but... immersive might complicate that.
It seems like Ash windows might also use the immersive code (not sure if that's actually used).
If that's actually the case, then we'd have to make immersive support running in ash and in apps.
Evan, do you know if Ash windows can go immersive?

Comment 13 by est...@chromium.org, Oct 3 

yes, windows for which Ash draws the frame can go immersive (e.g. platform apps). The differences for Chrome and Ash running immersive are captured in the ImmersiveContext interface (ImmersiveContextMus vs ImmersiveContextAsh).
Project Member

Comment 14 by bugdroid1@chromium.org, Oct 12 

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

commit 4310fc145ccae10b1dfb9f6c9aa284e08ad93478
Author: Mike Wasserman <msw@chromium.org>
Date: Fri Oct 12 18:33:16 2018

ws: Add EventObserver interface and window service support

Replaces PointerWatcher with a more generalized EventObserver.
EventObserver supports local code and remote window-service clients.

Ash & remote client aura::Envs host their respective EventObservers.
Ash notifies local observers via a pre-target event handler adapter.

WindowTreeClients forwards requested event types to WindowTree.
WindowTree notifies WindowTreeClients of input and observed events.

Needed to support more existing ws client pre-target event handlers.
Replaces the remaining ws mouse/touch->pointer event conversions.

A followup CL will rebase views::EventMonitor on EventObserver:
 https://chromium-review.googlesource.com/c/chromium/src/+/1258217

Bug:  887725 ,  872450 ,  865781 
Test: No --show-taps, immersive, touch edit handle, etc. changes.
Change-Id: I5ed870a3be948bf48448ed1a5e54a2bf4512e3b0
Reviewed-on: https://chromium-review.googlesource.com/c/1269806
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599303}
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ash/BUILD.gn
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ash/ash_service_unittest.cc
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ash/components/tap_visualizer/tap_renderer.cc
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ash/components/tap_visualizer/tap_renderer.h
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ash/components/tap_visualizer/tap_visualizer_app.cc
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ash/components/tap_visualizer/tap_visualizer_app.h
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ash/components/tap_visualizer/tap_visualizer_app_unittest.cc
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ash/metrics/pointer_metrics_recorder_unittest.cc
[delete] https://crrev.com/a1f93ce2de4aaa5fabbaaab5aa628dca9ffccd8d/ash/pointer_watcher_adapter.cc
[delete] https://crrev.com/a1f93ce2de4aaa5fabbaaab5aa628dca9ffccd8d/ash/pointer_watcher_adapter.h
[delete] https://crrev.com/a1f93ce2de4aaa5fabbaaab5aa628dca9ffccd8d/ash/pointer_watcher_adapter_unittest.cc
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ash/public/cpp/immersive/immersive_context.h
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ash/public/cpp/immersive/immersive_fullscreen_controller.cc
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ash/public/cpp/immersive/immersive_fullscreen_controller.h
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ash/shelf/shelf_view.cc
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ash/shelf/shelf_view_unittest.cc
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ash/shell.cc
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ash/shell.h
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ash/system/palette/palette_tray.cc
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ash/wm/immersive_context_ash.cc
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ash/wm/immersive_context_ash.h
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/chrome/browser/ui/views/frame/immersive_context_mus.cc
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/chrome/browser/ui/views/frame/immersive_context_mus.h
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/content/renderer/mus/renderer_window_tree_client.cc
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/content/renderer/mus/renderer_window_tree_client.h
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/services/ws/BUILD.gn
[add] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/services/ws/event_observer_helper.cc
[add] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/services/ws/event_observer_helper.h
[delete] https://crrev.com/a1f93ce2de4aaa5fabbaaab5aa628dca9ffccd8d/services/ws/pointer_watcher.cc
[delete] https://crrev.com/a1f93ce2de4aaa5fabbaaab5aa628dca9ffccd8d/services/ws/pointer_watcher.h
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/services/ws/public/mojom/window_tree.mojom
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/services/ws/test_change_tracker.cc
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/services/ws/test_change_tracker.h
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/services/ws/test_window_tree_client.cc
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/services/ws/test_window_tree_client.h
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/services/ws/window_tree.cc
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/services/ws/window_tree.h
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/services/ws/window_tree_client_unittest.cc
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/services/ws/window_tree_unittest.cc
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ui/aura/env.cc
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ui/aura/env.h
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ui/aura/mus/window_tree_client.cc
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ui/aura/mus/window_tree_client.h
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ui/aura/mus/window_tree_client_delegate.h
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ui/aura/mus/window_tree_client_unittest.cc
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ui/aura/test/aura_test_base.cc
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ui/aura/test/aura_test_base.h
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ui/aura/test/event_generator_delegate_aura.cc
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ui/aura/test/event_generator_delegate_aura.h
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ui/aura/test/mus/test_window_tree.cc
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ui/aura/test/mus/test_window_tree.h
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ui/aura/test/mus/test_window_tree_client_delegate.cc
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ui/aura/test/mus/test_window_tree_client_delegate.h
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ui/aura/test/mus/window_tree_client_private.cc
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ui/aura/test/mus/window_tree_client_private.h
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ui/aura/window_event_dispatcher.cc
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ui/events/BUILD.gn
[add] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ui/events/event_observer.h
[rename] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ui/events/events_exports.cc
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ui/events/test/event_generator.cc
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ui/events/test/event_generator.h
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ui/views/BUILD.gn
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ui/views/mus/BUILD.gn
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ui/views/mus/mus_client.cc
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ui/views/mus/mus_client.h
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ui/views/mus/mus_views_delegate.cc
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ui/views/mus/mus_views_delegate.h
[delete] https://crrev.com/a1f93ce2de4aaa5fabbaaab5aa628dca9ffccd8d/ui/views/mus/pointer_watcher_event_router.cc
[delete] https://crrev.com/a1f93ce2de4aaa5fabbaaab5aa628dca9ffccd8d/ui/views/mus/pointer_watcher_event_router.h
[delete] https://crrev.com/a1f93ce2de4aaa5fabbaaab5aa628dca9ffccd8d/ui/views/mus/pointer_watcher_event_router_unittest.cc
[delete] https://crrev.com/a1f93ce2de4aaa5fabbaaab5aa628dca9ffccd8d/ui/views/pointer_watcher.h
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ui/views/touchui/touch_selection_controller_impl.cc
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ui/views/touchui/touch_selection_controller_impl.h
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ui/views/views_delegate.cc
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ui/views/views_delegate.h
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ui/wm/test/wm_test_helper.cc
[modify] https://crrev.com/4310fc145ccae10b1dfb9f6c9aa284e08ad93478/ui/wm/test/wm_test_helper.h
►

Sign in to add a comment