Commit f230ac0a authored by Yuichiro Hanada's avatar Yuichiro Hanada Committed by Chromium LUCI CQ

Do not use the event target window if it's already destroyed by the held events.

WindowEventDispatcher can defer the processing of mouse/touch events
until a non-mouse/touch events comes or it's explicitly released.
If a target window of the deferred window destroys another window in
their event handler, the destruction is also deferred.
It can cause use-after-free if the following non-mouse/touch event's
target is the window being destroyed because searching of the target
window happens before the release of the deferred mouse events.

Bug: 1099985
Test: aura_unittests
Change-Id: Ia386bb99edd1374d1d39038ff44b3bfc73c26fa5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2576983Reviewed-by: default avatarSadrul Chowdhury <sadrul@chromium.org>
Commit-Queue: Yuichiro Hanada <yhanada@chromium.org>
Cr-Commit-Position: refs/heads/master@{#838584}
parent 7c017cd5
...@@ -519,6 +519,8 @@ ui::EventDispatchDetails WindowEventDispatcher::PreDispatchEvent( ...@@ -519,6 +519,8 @@ ui::EventDispatchDetails WindowEventDispatcher::PreDispatchEvent(
event->time_stamp()); event->time_stamp());
} }
WindowTracker target_window_tracker;
target_window_tracker.Add(target_window);
if (!dispatching_held_event_) { if (!dispatching_held_event_) {
bool can_be_held = IsEventCandidateForHold(*event); bool can_be_held = IsEventCandidateForHold(*event);
if (!move_hold_count_ || !can_be_held) { if (!move_hold_count_ || !can_be_held) {
...@@ -529,6 +531,12 @@ ui::EventDispatchDetails WindowEventDispatcher::PreDispatchEvent( ...@@ -529,6 +531,12 @@ ui::EventDispatchDetails WindowEventDispatcher::PreDispatchEvent(
return details; return details;
} }
} }
if (target_window_tracker.windows().empty()) {
// The event target is destroyed while processing the held event.
DispatchDetails details;
details.target_destroyed = true;
return details;
}
DispatchDetails details; DispatchDetails details;
if (event->IsMouseEvent()) { if (event->IsMouseEvent()) {
......
...@@ -3108,4 +3108,84 @@ TEST_F(WindowEventDispatcherTest, TouchEventWithScaledWindow) { ...@@ -3108,4 +3108,84 @@ TEST_F(WindowEventDispatcherTest, TouchEventWithScaledWindow) {
root_window()->RemovePreTargetHandler(&root_recorder); root_window()->RemovePreTargetHandler(&root_recorder);
} }
// A test case for crbug.com/1099985
TEST_F(WindowEventDispatcherTest, TargetIsDestroyedByHeldEvent) {
EventFilterRecorder recorder;
root_window()->AddPreTargetHandler(&recorder);
// Create a window which should be a target of all MouseEvent in this tests.
test::TestWindowDelegate delegate;
std::unique_ptr<aura::Window> mouse_target(CreateTestWindowWithDelegate(
&delegate, 1, gfx::Rect(0, 0, 100, 100), root_window()));
// Create a window which has a focus, so should receive all KeyEvents.
ConsumeKeyHandler key_handler;
// Not using std::unique_ptr<> intentionally
aura::Window* focused(test::CreateTestWindowWithBounds(
gfx::Rect(200, 200, 100, 100), root_window()));
focused->SetProperty(client::kSkipImeProcessing, true);
focused->AddPostTargetHandler(&key_handler);
focused->Show();
focused->Focus();
// Make sure that the key event goes to the |focused| window.
ui::KeyEvent key_press(ui::ET_KEY_PRESSED, ui::VKEY_A, ui::EF_NONE);
DispatchEventUsingWindowDispatcher(&key_press);
EXPECT_EQ(1, key_handler.num_key_events());
key_handler.Reset();
ui::MouseEvent mouse_move_event(ui::ET_MOUSE_MOVED, gfx::Point(1, 1),
gfx::Point(1, 1), ui::EventTimeForNow(), 0,
0);
DispatchEventUsingWindowDispatcher(&mouse_move_event);
// Discard MOUSE_ENTER.
recorder.Reset();
host()->dispatcher()->HoldPointerMoves();
// The dragged event should not be sent to the |target| window because
// WindowEventDispatcher is holding it now.
ui::MouseEvent mouse_dragged_event(ui::ET_MOUSE_DRAGGED, gfx::Point(0, 0),
gfx::Point(0, 0), ui::EventTimeForNow(), 0,
0);
DispatchEventUsingWindowDispatcher(&mouse_dragged_event);
EXPECT_TRUE(recorder.events().empty());
// Create a event handler which destroys the |focused| window when it sees any
// mouse event.
class Handler : public ui::test::TestEventHandler {
public:
explicit Handler(aura::Window* focused) : focused_(focused) {}
~Handler() override = default;
Handler(const Handler&) = delete;
Handler& operator=(const Handler&) = delete;
// Overridden from ui::EventHandler:
void OnMouseEvent(ui::MouseEvent* event) override {
ui::test::TestEventHandler::OnMouseEvent(event);
LOG(ERROR) << "|focused_| is being deleted";
// !!!
delete focused_;
}
private:
aura::Window* focused_;
};
Handler mouse_handler(focused);
mouse_target->AddPostTargetHandler(&mouse_handler);
// Sending a key event should stop the hold and the mouse event goes to the
// |target| window.
// The key event should not be sent to the handler because the focused window
// is destroyed before the event is dispatched.
ui::KeyEvent key_press2(ui::ET_KEY_PRESSED, ui::VKEY_A, ui::EF_NONE);
DispatchEventUsingWindowDispatcher(&key_press2);
EXPECT_EQ(1u, recorder.events().size());
EXPECT_EQ(0, key_handler.num_key_events());
EXPECT_EQ(1, mouse_handler.num_mouse_events());
root_window()->RemovePreTargetHandler(&recorder);
}
} // namespace aura } // namespace aura
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment