New issue
Advanced search Search tips

Issue 887725 link

Starred by 1 user
Status: Fixed
Owner:
msw@chromium.org
Closed: Oct 16
Cc:
jamescook@chromium.org
sky@chromium.org
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

views: support PointerWatcher without mus, refactor

Project Member Reported by msw@chromium.org, Sep 20 
views: support PointerWatcher without mus, refactor

PointerWatcher is defined in ui/views/ but requires ui/views/mus/*
It also doesn't work if the WindowServer is not in use.

There's also two places to add/remove watchers:
1) ash::Shell::[Add|Remove]PointerWatcher() (for Views components in Ash)
2) Mus-only PointerWatcherEventRouter::[Add|Remove]PointerWatcher() (for MusClient users).

We should probably merge PointerWatcher with MouseWatcher and EventMonitor.
(it'd be nice to have one class supporting all these similar scenarios)
It might be nice if these concepts were in the ui namespace, etc. instead of views.

I'm going to add some ViewsDelegate plumbing temporarily...

Comment 1 by jamescook@chromium.org, Sep 20 

PointerWatcher doesn't require mus, at least not when used in //ash. PointerWatcherAdapter is-a ui::EventHandler that makes PointerWatcher work in classic.

(FWIW, I'm in favor of removing all PointerWatchers from //ash, see  issue 872450  )

Comment 2 by sky@chromium.org, Sep 20 

The problem is you can't use PointerWatcher from views code (because there is no way to attach it). PointerWatcher is useful from within views for things like MouseWatcher. As Mike says, views also has EventMonitor, which is a superset of PointerWatcher. We should consolidate these types, and make EventMonitor clear what it is needed for.
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 20 

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

commit 0663980bf2c4f570626981f59d7cfbadd855ba85
Author: Mike Wasserman <msw@chromium.org>
Date: Thu Sep 20 23:36:30 2018

ws: Add touch selection controller support for pointer watcher.

Allows ws clients to close touch ui on events outside their windows.

Add a new PointerWatcher plumbing to [Mus]ViewsDelegate.
Wires up watchers to the Mus-only PointerWatcherEventRouter.
(ui/views/mus/* is not allowed as a dependency of ui/views/*)

Destroy the touch selection ui on PointerWatcher mouse events.

Bug:  884394 ,  887725 
Test: Mouse events outside the KSV window close KSV text touch ui.
Change-Id: I0e41da4d03719001191f7d5ab669bcc1805bd63c
Reviewed-on: https://chromium-review.googlesource.com/1232955
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593001}
[modify] https://crrev.com/0663980bf2c4f570626981f59d7cfbadd855ba85/ui/base/touch/touch_editing_controller.h
[modify] https://crrev.com/0663980bf2c4f570626981f59d7cfbadd855ba85/ui/views/mus/mus_views_delegate.cc
[modify] https://crrev.com/0663980bf2c4f570626981f59d7cfbadd855ba85/ui/views/mus/mus_views_delegate.h
[modify] https://crrev.com/0663980bf2c4f570626981f59d7cfbadd855ba85/ui/views/touchui/touch_selection_controller_impl.cc
[modify] https://crrev.com/0663980bf2c4f570626981f59d7cfbadd855ba85/ui/views/touchui/touch_selection_controller_impl.h
[modify] https://crrev.com/0663980bf2c4f570626981f59d7cfbadd855ba85/ui/views/views_delegate.cc
[modify] https://crrev.com/0663980bf2c4f570626981f59d7cfbadd855ba85/ui/views/views_delegate.h
Project Member

Comment 4 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
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 16 

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

commit ecdccd71e2ea47293f38879c1e7294cd748307ba
Author: Mike Wasserman <msw@chromium.org>
Date: Tue Oct 16 05:48:26 2018

ws: Refactor views::EventMonitor to use ui::EventObserver

EventObserver has support for Window Service clients.
EventObserver prevents changes to events and dispatch.

Encapsulate FullscreenControlHost's EventMonitor; destroy with widget.
Move Mac's dispatch changes to the one user, MenuPreTargetHandlerMac.

Bug:  887725 ,  799428 
Change-Id: I1dcb9f7786885210ea56b0c3c3f7134e21228de1
Reviewed-on: https://chromium-review.googlesource.com/c/1258217
Commit-Queue: Michael Wasserman <msw@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599873}
[modify] https://crrev.com/ecdccd71e2ea47293f38879c1e7294cd748307ba/ash/assistant/assistant_ui_controller.cc
[modify] https://crrev.com/ecdccd71e2ea47293f38879c1e7294cd748307ba/ash/assistant/assistant_ui_controller.h
[modify] https://crrev.com/ecdccd71e2ea47293f38879c1e7294cd748307ba/chrome/browser/ui/views/frame/browser_view.cc
[modify] https://crrev.com/ecdccd71e2ea47293f38879c1e7294cd748307ba/chrome/browser/ui/views/frame/browser_view.h
[modify] https://crrev.com/ecdccd71e2ea47293f38879c1e7294cd748307ba/chrome/browser/ui/views/fullscreen_control/fullscreen_control_host.cc
[modify] https://crrev.com/ecdccd71e2ea47293f38879c1e7294cd748307ba/chrome/browser/ui/views/fullscreen_control/fullscreen_control_host.h
[modify] https://crrev.com/ecdccd71e2ea47293f38879c1e7294cd748307ba/chrome/browser/ui/views/fullscreen_control/fullscreen_control_view_interactive_uitest.cc
[modify] https://crrev.com/ecdccd71e2ea47293f38879c1e7294cd748307ba/chrome/browser/ui/views/location_bar/location_bar_bubble_delegate_view.cc
[modify] https://crrev.com/ecdccd71e2ea47293f38879c1e7294cd748307ba/chrome/browser/ui/views/location_bar/location_bar_bubble_delegate_view.h
[modify] https://crrev.com/ecdccd71e2ea47293f38879c1e7294cd748307ba/chrome/browser/ui/views/tabs/tab_drag_controller.cc
[modify] https://crrev.com/ecdccd71e2ea47293f38879c1e7294cd748307ba/chrome/browser/ui/views/tabs/tab_drag_controller.h
[modify] https://crrev.com/ecdccd71e2ea47293f38879c1e7294cd748307ba/ui/views/controls/menu/menu_pre_target_handler_mac.h
[modify] https://crrev.com/ecdccd71e2ea47293f38879c1e7294cd748307ba/ui/views/controls/menu/menu_pre_target_handler_mac.mm
[modify] https://crrev.com/ecdccd71e2ea47293f38879c1e7294cd748307ba/ui/views/event_monitor.h
[modify] https://crrev.com/ecdccd71e2ea47293f38879c1e7294cd748307ba/ui/views/event_monitor_aura.cc
[modify] https://crrev.com/ecdccd71e2ea47293f38879c1e7294cd748307ba/ui/views/event_monitor_aura.h
[modify] https://crrev.com/ecdccd71e2ea47293f38879c1e7294cd748307ba/ui/views/event_monitor_mac.h
[modify] https://crrev.com/ecdccd71e2ea47293f38879c1e7294cd748307ba/ui/views/event_monitor_mac.mm
[modify] https://crrev.com/ecdccd71e2ea47293f38879c1e7294cd748307ba/ui/views/event_monitor_unittest.cc
[modify] https://crrev.com/ecdccd71e2ea47293f38879c1e7294cd748307ba/ui/views/mouse_watcher.cc
[modify] https://crrev.com/ecdccd71e2ea47293f38879c1e7294cd748307ba/ui/views/widget/widget_unittest.cc

Comment 6 by msw@chromium.org, Oct 16

Status: Fixed (was: Assigned)
►

Sign in to add a comment