From 1d58e1c7800370100ba15b77b7cea4ebd88ef326 Mon Sep 17 00:00:00 2001 From: Scott Violet Date: Thu, 31 Aug 2017 01:25:41 +0000 Subject: [PATCH] chromeos: give priority to certain targets during event dispatch Currently if an event has an explicitly set target (as happens in mus) we potentially have two paths: 1. Try FindTargetInRootWindow() and if that returns non-null, than that becomes the initial target. 2. Walk up from target returning the first ancestor with an event_targeter. We then most likely (but not always) call FindTargetForEvent() to reevaluate who should be the target. Problem is, if the target is not the root, then we skip the targets that take priority. For example, if there is capture there is no guarantee the capture window will be returned. This patch makes it so that if there is a capture window, mouse pressed handler, or gesture touch lock handler, then it takes priority and gets the event. This makes mus match classic behavior. This fixs behavior where during window resize the size of the window might bounce around. The bouncing happens if the window with capture doesn't get the event. BUG=none TEST=none Change-Id: I519a9126f8aada86498e13900dd0c499312eaae4 Reviewed-on: https://chromium-review.googlesource.com/643948 Reviewed-by: Sadrul Chowdhury Commit-Queue: Scott Violet Cr-Commit-Position: refs/heads/master@{#498703} --- ui/aura/mus/window_tree_client_unittest.cc | 10 ++- ui/aura/window_event_dispatcher.cc | 30 +++++++ ui/aura/window_event_dispatcher.h | 1 + ui/aura/window_event_dispatcher_unittest.cc | 58 ++++++++++++- ui/aura/window_targeter.cc | 82 +++++++++++++------ ui/aura/window_targeter.h | 12 +++ ui/events/event.h | 2 +- ui/events/event_processor.cc | 67 ++++++++------- ui/events/event_processor.h | 5 ++ ui/events/test/test_event_processor.cc | 4 + ui/events/test/test_event_processor.h | 1 + ui/views/test/event_generator_delegate_mac.mm | 3 + ui/views/widget/root_view.cc | 5 ++ ui/views/widget/root_view.h | 1 + 14 files changed, 219 insertions(+), 62 deletions(-) diff --git a/ui/aura/mus/window_tree_client_unittest.cc b/ui/aura/mus/window_tree_client_unittest.cc index 9fb47c757421c8..60bd21c87f36e3 100644 --- a/ui/aura/mus/window_tree_client_unittest.cc +++ b/ui/aura/mus/window_tree_client_unittest.cc @@ -1085,11 +1085,12 @@ TEST_F(WindowTreeClientClientTest, InputEventCaptureWindow) { // child1 has a custom targeter set which would always return itself as the // target window therefore event should go to child1. const gfx::Point event_location(50, 60); + const gfx::Point root_location; uint32_t event_id = 1; window_delegate1->set_event_id(event_id); window_delegate2->set_event_id(event_id); std::unique_ptr ui_event( - new ui::MouseEvent(ui::ET_MOUSE_MOVED, event_location, gfx::Point(), + new ui::MouseEvent(ui::ET_MOUSE_MOVED, event_location, root_location, ui::EventTimeForNow(), ui::EF_NONE, 0)); window_tree_client()->OnWindowInputEvent( event_id, server_id(child1.get()), window_tree_host->display_id(), @@ -1103,7 +1104,8 @@ TEST_F(WindowTreeClientClientTest, InputEventCaptureWindow) { window_delegate1->reset(); window_delegate2->reset(); - // The same event should go to child2 if child2 is the capture window. + // Set capture to |child2|. Capture takes precedence, and because the event is + // converted local coordinates are converted from the root. std::unique_ptr capture_client( base::MakeUnique()); client::SetCaptureClient(top_level, capture_client.get()); @@ -1120,7 +1122,9 @@ TEST_F(WindowTreeClientClientTest, InputEventCaptureWindow) { window_tree()->GetEventResult(event_id)); EXPECT_EQ(0, window_delegate1->move_count()); EXPECT_EQ(1, window_delegate2->move_count()); - EXPECT_EQ(gfx::Point(30, 30), window_delegate2->last_event_location()); + gfx::Point root_point_in_child2; + Window::ConvertPointToTarget(top_level, child2.get(), &root_point_in_child2); + EXPECT_EQ(root_point_in_child2, window_delegate2->last_event_location()); child2.reset(); child1.reset(); window_tree_host.reset(); diff --git a/ui/aura/window_event_dispatcher.cc b/ui/aura/window_event_dispatcher.cc index 9b3aa8fe2ba0f4..0794030d757734 100644 --- a/ui/aura/window_event_dispatcher.cc +++ b/ui/aura/window_event_dispatcher.cc @@ -443,6 +443,36 @@ void WindowEventDispatcher::ReleaseNativeCapture() { //////////////////////////////////////////////////////////////////////////////// // WindowEventDispatcher, ui::EventProcessor implementation: + +ui::EventTarget* WindowEventDispatcher::GetInitialEventTarget( + ui::Event* event) { + if (Env::GetInstance()->mode() == Env::Mode::LOCAL || + !event->IsLocatedEvent() || !event->target()) { + return nullptr; + } + + ui::LocatedEvent* located_event = event->AsLocatedEvent(); + + Window* priority_target = static_cast( + event_targeter_->GetPriorityTargetInRootWindow(window(), *located_event)); + if (!priority_target) + return nullptr; + + // The event has a target but we need to dispatch it using the normal path. + // Reset the target and location so the normal flow is used. + ui::Event::DispatcherApi(event).set_target(nullptr); + located_event->set_location_f(located_event->root_location_f()); + if (event_targeter_->ProcessEventIfTargetsDifferentRootWindow( + window(), static_cast(priority_target), event)) { + // Make sure the event was marked handled so that EventProcessor doesn't + // attempt to process the event as well. + event->SetHandled(); + return nullptr; + } + located_event->ConvertLocationToTarget(window(), priority_target); + return priority_target; +} + ui::EventTarget* WindowEventDispatcher::GetRootForEvent(ui::Event* event) { if (Env::GetInstance()->mode() == Env::Mode::LOCAL) return window(); diff --git a/ui/aura/window_event_dispatcher.h b/ui/aura/window_event_dispatcher.h index 9bd63a79d245aa..e3ee61c1ead134 100644 --- a/ui/aura/window_event_dispatcher.h +++ b/ui/aura/window_event_dispatcher.h @@ -184,6 +184,7 @@ class AURA_EXPORT WindowEventDispatcher : public ui::EventProcessor, void ReleaseNativeCapture() override; // Overridden from ui::EventProcessor: + ui::EventTarget* GetInitialEventTarget(ui::Event* event) override; ui::EventTarget* GetRootForEvent(ui::Event* event) override; void OnEventProcessingStarted(ui::Event* event) override; void OnEventProcessingFinished(ui::Event* event) override; diff --git a/ui/aura/window_event_dispatcher_unittest.cc b/ui/aura/window_event_dispatcher_unittest.cc index 2f5e9f1275ad0c..e8dbf59c9c61c6 100644 --- a/ui/aura/window_event_dispatcher_unittest.cc +++ b/ui/aura/window_event_dispatcher_unittest.cc @@ -32,6 +32,7 @@ #include "ui/aura/test/test_window_delegate.h" #include "ui/aura/test/test_windows.h" #include "ui/aura/window.h" +#include "ui/aura/window_targeter.h" #include "ui/aura/window_tracker.h" #include "ui/base/hit_test.h" #include "ui/display/screen.h" @@ -484,7 +485,7 @@ TEST_P(WindowEventDispatcherTest, ScrollEventDispatch) { namespace { -// FilterFilter that tracks the types of events it's seen. +// ui::EventHandler that tracks the types of events it's seen. class EventFilterRecorder : public ui::EventHandler { public: typedef std::vector Events; @@ -2909,6 +2910,57 @@ TEST_F(WindowEventDispatcherMusTest, UseDefaultTargeterToFindTarget2) { namespace { +class ExplicitWindowTargeter : public WindowTargeter { + public: + explicit ExplicitWindowTargeter(Window* target) : target_(target) {} + ~ExplicitWindowTargeter() override = default; + + // WindowTargeter: + ui::EventTarget* FindTargetForEvent(ui::EventTarget* root, + ui::Event* event) override { + return target_; + } + ui::EventTarget* FindNextBestTarget(ui::EventTarget* previous_target, + ui::Event* event) override { + return nullptr; + } + + private: + Window* target_; + + DISALLOW_COPY_AND_ASSIGN(ExplicitWindowTargeter); +}; + +} // namespace + +TEST_F(WindowEventDispatcherMusTest, TargetCaptureWindow) { + NonClientDelegate w1_delegate; + NonClientDelegate w2_delegate; + std::unique_ptr w1( + CreateNormalWindow(-1, root_window(), &w1_delegate)); + std::unique_ptr w2( + CreateNormalWindow(-1, root_window(), &w2_delegate)); + NonClientDelegate w2_child_delegate; + std::unique_ptr w2_child( + CreateNormalWindow(-1, w2.get(), &w2_child_delegate)); + ExplicitWindowTargeter* w2_targeter = + new ExplicitWindowTargeter(w2_child.get()); + w2->SetEventTargeter(base::WrapUnique(w2_targeter)); + w2->SetCapture(); + ASSERT_TRUE(w2->HasCapture()); + const gfx::Point mouse_location(15, 25); + ui::MouseEvent mouse(ui::ET_MOUSE_PRESSED, mouse_location, mouse_location, + ui::EventTimeForNow(), ui::EF_LEFT_MOUSE_BUTTON, + ui::EF_LEFT_MOUSE_BUTTON); + ui::Event::DispatcherApi(&mouse).set_target(w1.get()); + DispatchEventUsingWindowDispatcher(&mouse); + EXPECT_EQ(0, w1_delegate.mouse_event_count()); + EXPECT_EQ(1, w2_delegate.mouse_event_count()); + EXPECT_EQ(0, w2_child_delegate.mouse_event_count()); +} + +namespace { + class LocationRecordingEventHandler : public ui::EventHandler { public: LocationRecordingEventHandler() = default; @@ -2941,12 +2993,12 @@ TEST_F(WindowEventDispatcherMusTest, RootLocationDoesntChange) { std::unique_ptr window( test::CreateTestWindowWithBounds(gfx::Rect(0, 0, 10, 20), root_window())); std::unique_ptr child_window( - test::CreateTestWindowWithBounds(gfx::Rect(5, 6, 10, 20), window.get())); + CreateNormalWindow(-1, window.get(), nullptr)); test::EnvTestHelper().SetAlwaysUseLastMouseLocation(false); LocationRecordingEventHandler event_handler; - root_window()->AddPreTargetHandler(&event_handler); + child_window->AddPreTargetHandler(&event_handler); const gfx::Point mouse_location(1, 2); gfx::Point root_location(mouse_location); diff --git a/ui/aura/window_targeter.cc b/ui/aura/window_targeter.cc index 8be8e145d7d363..e04383a8f9e04a 100644 --- a/ui/aura/window_targeter.cc +++ b/ui/aura/window_targeter.cc @@ -47,8 +47,9 @@ WindowTargeter::GetExtraHitTestShapeRects(Window* target) const { return nullptr; } -Window* WindowTargeter::FindTargetInRootWindow(Window* root_window, - const ui::LocatedEvent& event) { +Window* WindowTargeter::GetPriorityTargetInRootWindow( + Window* root_window, + const ui::LocatedEvent& event) { DCHECK_EQ(root_window, root_window->GetRootWindow()); // Mouse events should be dispatched to the window that processed the @@ -71,8 +72,27 @@ Window* WindowTargeter::FindTargetInRootWindow(Window* root_window, ui::GestureRecognizer::Get()->GetTouchLockedTarget(touch); if (consumer) return static_cast(consumer); - consumer = ui::GestureRecognizer::Get()->GetTargetForLocation( - event.location_f(), touch.source_device_id()); + } + + return nullptr; +} + +Window* WindowTargeter::FindTargetInRootWindow(Window* root_window, + const ui::LocatedEvent& event) { + DCHECK_EQ(root_window, root_window->GetRootWindow()); + + Window* priority_target = GetPriorityTargetInRootWindow(root_window, event); + if (priority_target) + return priority_target; + + if (event.IsTouchEvent()) { + // Query the gesture-recognizer to find targets for touch events. + const ui::TouchEvent& touch = *event.AsTouchEvent(); + // GetTouchLockedTarget() is handled in GetPriorityTargetInRootWindow(). + DCHECK(!ui::GestureRecognizer::Get()->GetTouchLockedTarget(touch)); + ui::GestureConsumer* consumer = + ui::GestureRecognizer::Get()->GetTargetForLocation( + event.location_f(), touch.source_device_id()); if (consumer) return static_cast(consumer); @@ -84,34 +104,44 @@ Window* WindowTargeter::FindTargetInRootWindow(Window* root_window, return nullptr; } +bool WindowTargeter::ProcessEventIfTargetsDifferentRootWindow( + Window* root_window, + Window* target, + ui::Event* event) { + if (root_window->Contains(target)) + return false; + + // |window| is the root window, but |target| is not a descendent of + // |window|. So do not allow dispatching from here. Instead, dispatch the + // event through the WindowEventDispatcher that owns |target|. + Window* new_root = target->GetRootWindow(); + DCHECK(new_root); + if (event->IsLocatedEvent()) { + // The event has been transformed to be in |target|'s coordinate system. + // But dispatching the event through the EventProcessor requires the event + // to be in the host's coordinate system. So, convert the event to be in + // the root's coordinate space, and then to the host's coordinate space by + // applying the host's transform. + ui::LocatedEvent* located_event = event->AsLocatedEvent(); + located_event->ConvertLocationToTarget(target, new_root); + WindowTreeHost* window_tree_host = new_root->GetHost(); + located_event->UpdateForRootTransform( + window_tree_host->GetRootTransform(), + window_tree_host->GetRootTransformForLocalEventCoordinates()); + } + ignore_result(new_root->GetHost()->event_sink()->OnEventFromSource(event)); + return true; +} + ui::EventTarget* WindowTargeter::FindTargetForEvent(ui::EventTarget* root, ui::Event* event) { Window* window = static_cast(root); Window* target = event->IsKeyEvent() ? FindTargetForKeyEvent(window, *event->AsKeyEvent()) : FindTargetForNonKeyEvent(window, event); - if (target && !window->parent() && !window->Contains(target)) { - // |window| is the root window, but |target| is not a descendent of - // |window|. So do not allow dispatching from here. Instead, dispatch the - // event through the WindowEventDispatcher that owns |target|. - Window* new_root = target->GetRootWindow(); - DCHECK(new_root); - if (event->IsLocatedEvent()) { - // The event has been transformed to be in |target|'s coordinate system. - // But dispatching the event through the EventProcessor requires the event - // to be in the host's coordinate system. So, convert the event to be in - // the root's coordinate space, and then to the host's coordinate space by - // applying the host's transform. - ui::LocatedEvent* located_event = static_cast(event); - located_event->ConvertLocationToTarget(target, new_root); - WindowTreeHost* window_tree_host = new_root->GetHost(); - located_event->UpdateForRootTransform( - window_tree_host->GetRootTransform(), - window_tree_host->GetRootTransformForLocalEventCoordinates()); - } - ignore_result(new_root->GetHost()->event_sink()->OnEventFromSource(event)); - - target = nullptr; + if (target && !window->parent() && + ProcessEventIfTargetsDifferentRootWindow(window, target, event)) { + return nullptr; } return target; } diff --git a/ui/aura/window_targeter.h b/ui/aura/window_targeter.h index cb07c106c62f08..1aea7f82b73aaa 100644 --- a/ui/aura/window_targeter.h +++ b/ui/aura/window_targeter.h @@ -60,9 +60,21 @@ class AURA_EXPORT WindowTargeter : public ui::EventTargeter { virtual std::unique_ptr GetExtraHitTestShapeRects( Window* target) const; + // If there is a target that takes priority over normal WindowTargeter (such + // as a capture window) this returns it. + Window* GetPriorityTargetInRootWindow(Window* root_window, + const ui::LocatedEvent& event); + Window* FindTargetInRootWindow(Window* root_window, const ui::LocatedEvent& event); + // If |target| is not a child of |root_window|, then converts |event| to + // be relative to |root_window| and dispatches the event to |root_window|. + // Returns false if the |target| is a child of |root_window|. + bool ProcessEventIfTargetsDifferentRootWindow(Window* root_window, + Window* target, + ui::Event* event); + // ui::EventTargeter: ui::EventTarget* FindTargetForEvent(ui::EventTarget* root, ui::Event* event) override; diff --git a/ui/events/event.h b/ui/events/event.h index 6b5ccbd309684b..6186190077c163 100644 --- a/ui/events/event.h +++ b/ui/events/event.h @@ -378,7 +378,7 @@ class EVENTS_EXPORT LocatedEvent : public Event { gfx::Point offset = gfx::ToFlooredPoint(location_); T::ConvertPointToTarget(source, target, &offset); gfx::Vector2d diff = gfx::ToFlooredPoint(location_) - offset; - location_= location_ - diff; + location_ = location_ - diff; } protected: diff --git a/ui/events/event_processor.cc b/ui/events/event_processor.cc index f80cb91b6404cd..316dcdd8c986ca 100644 --- a/ui/events/event_processor.cc +++ b/ui/events/event_processor.cc @@ -22,45 +22,54 @@ EventDispatchDetails EventProcessor::OnEventFromSource(Event* event) { event_to_dispatch = event_copy.get(); } + EventDispatchDetails details; OnEventProcessingStarted(event_to_dispatch); - EventTarget* target = nullptr; - EventTargeter* targeter = nullptr; + // GetInitialEventTarget() may handle the event. + EventTarget* initial_target = event_to_dispatch->handled() + ? nullptr + : GetInitialEventTarget(event_to_dispatch); if (!event_to_dispatch->handled()) { - EventTarget* root = GetRootForEvent(event_to_dispatch); - DCHECK(root); - targeter = root->GetEventTargeter(); - if (targeter) { - target = targeter->FindTargetForEvent(root, event_to_dispatch); - } else { - targeter = GetDefaultEventTargeter(); - if (event_to_dispatch->target()) - target = root; - else + EventTarget* target = initial_target; + EventTargeter* targeter = nullptr; + + if (!target) { + EventTarget* root = GetRootForEvent(event_to_dispatch); + DCHECK(root); + targeter = root->GetEventTargeter(); + if (targeter) { target = targeter->FindTargetForEvent(root, event_to_dispatch); + } else { + targeter = GetDefaultEventTargeter(); + if (event_to_dispatch->target()) + target = root; + else + target = targeter->FindTargetForEvent(root, event_to_dispatch); + } + DCHECK(targeter); } - DCHECK(targeter); - } - EventDispatchDetails details; - while (target) { - details = DispatchEvent(target, event_to_dispatch); + while (target) { + details = DispatchEvent(target, event_to_dispatch); - if (!dispatch_original_event) { - if (event_to_dispatch->stopped_propagation()) - event->StopPropagation(); - else if (event_to_dispatch->handled()) - event->SetHandled(); - } + if (!dispatch_original_event) { + if (event_to_dispatch->stopped_propagation()) + event->StopPropagation(); + else if (event_to_dispatch->handled()) + event->SetHandled(); + } - if (details.dispatcher_destroyed) - return details; + if (details.dispatcher_destroyed) + return details; - if (details.target_destroyed || event->handled()) - break; + if (details.target_destroyed || event->handled() || + target == initial_target) { + break; + } - target = targeter->FindNextBestTarget(target, event_to_dispatch); + DCHECK(targeter); + target = targeter->FindNextBestTarget(target, event_to_dispatch); + } } - OnEventProcessingFinished(event); return details; } diff --git a/ui/events/event_processor.h b/ui/events/event_processor.h index 7be5f2d3498806..cdb834b7e8841a 100644 --- a/ui/events/event_processor.h +++ b/ui/events/event_processor.h @@ -23,6 +23,11 @@ class EVENTS_EXPORT EventProcessor : public EventDispatcherDelegate, // EventSink overrides: EventDispatchDetails OnEventFromSource(Event* event) override; + // Returns the initial target for the event. A value of null means use the + // root and default targeter to find the target. This pass may process the + // event. + virtual EventTarget* GetInitialEventTarget(Event* event) = 0; + // Returns the EventTarget with the right EventTargeter that we should use for // dispatching this |event|. virtual EventTarget* GetRootForEvent(Event* event) = 0; diff --git a/ui/events/test/test_event_processor.cc b/ui/events/test/test_event_processor.cc index dd83c4c630f896..748d47c9a2c4e7 100644 --- a/ui/events/test/test_event_processor.cc +++ b/ui/events/test/test_event_processor.cc @@ -37,6 +37,10 @@ bool TestEventProcessor::CanDispatchToTarget(EventTarget* target) { return true; } +EventTarget* TestEventProcessor::GetInitialEventTarget(Event* event) { + return nullptr; +} + EventTarget* TestEventProcessor::GetRootForEvent(Event* event) { return root_.get(); } diff --git a/ui/events/test/test_event_processor.h b/ui/events/test/test_event_processor.h index 1e484a45a593e5..8ff9e5a525623d 100644 --- a/ui/events/test/test_event_processor.h +++ b/ui/events/test/test_event_processor.h @@ -36,6 +36,7 @@ class TestEventProcessor : public EventProcessor { // EventProcessor: bool CanDispatchToTarget(EventTarget* target) override; + EventTarget* GetInitialEventTarget(Event* event) override; EventTarget* GetRootForEvent(Event* event) override; EventTargeter* GetDefaultEventTargeter() override; EventDispatchDetails OnEventFromSource(Event* event) override; diff --git a/ui/views/test/event_generator_delegate_mac.mm b/ui/views/test/event_generator_delegate_mac.mm index 3203a23dd4d4ea..af8c4001cca90f 100644 --- a/ui/views/test/event_generator_delegate_mac.mm +++ b/ui/views/test/event_generator_delegate_mac.mm @@ -272,6 +272,9 @@ IMP CurrentEventMethod() { ui::EventSink* GetEventSink() override { return this; } // Overridden from ui::EventProcessor: + ui::EventTarget* GetInitialEventTarget(ui::Event* event) override { + return nullptr; + } ui::EventTarget* GetRootForEvent(ui::Event* event) override { return this; } ui::EventTargeter* GetDefaultEventTargeter() override { return this->GetEventTargeter(); diff --git a/ui/views/widget/root_view.cc b/ui/views/widget/root_view.cc index acc2aaf61d0058..e1e866fc1a63a2 100644 --- a/ui/views/widget/root_view.cc +++ b/ui/views/widget/root_view.cc @@ -241,6 +241,11 @@ View* RootView::GetFocusTraversableParentView() { //////////////////////////////////////////////////////////////////////////////// // RootView, ui::EventProcessor overrides: +ui::EventTarget* RootView::GetInitialEventTarget(ui::Event* event) { + // Views has no special initial target. + return nullptr; +} + ui::EventTarget* RootView::GetRootForEvent(ui::Event* event) { return this; } diff --git a/ui/views/widget/root_view.h b/ui/views/widget/root_view.h index 17ae8112718a10..6d8761fe80ea82 100644 --- a/ui/views/widget/root_view.h +++ b/ui/views/widget/root_view.h @@ -93,6 +93,7 @@ class VIEWS_EXPORT RootView : public View, View* GetFocusTraversableParentView() override; // Overridden from ui::EventProcessor: + ui::EventTarget* GetInitialEventTarget(ui::Event* event) override; ui::EventTarget* GetRootForEvent(ui::Event* event) override; ui::EventTargeter* GetDefaultEventTargeter() override; void OnEventProcessingStarted(ui::Event* event) override;