Commit 63b59877 authored by Ria Jiang's avatar Ria Jiang Committed by Commit Bot

Don't call to update native cursor from WS when removing a window.

When we remove a window, e.g. opening the network settings page, which
is going to remove the status tray bubble window, we changes the
window hierarchy and updates cursor provider for the old parent [1],
which finds the target based on the last mouse location. Viz hasn't been
notified of this change yet and the status tray bubble window is still
in the window mappings in mus-ws (until destroyed), so we still finds
this status tray bubble window as target with --use-viz-hit-test. Then
when we adjust this target for modal window, we fail to find its root
window [2].

[1] https://cs.chromium.org/chromium/src/services/ui/ws/window_server.cc?gsn=GetRootForDrawn&l=920
[2] https://cs.chromium.org/chromium/src/services/ui/ws/event_dispatcher.cc?type=cs&l=362

This CL checks if we are actually removing a window before we call to
update native cursor in WS; the logic for moving a window from one
display to another remains unchanged. When we remove a window,
EventDispatcher::WindowNoLongerValidTarget is going update cursor if
necessary, so there's no need to call to update cursor in
WindowServer::OnWindowHierarchyChanged and do the work twice.

Bug: 788185
Change-Id: I48a7bfa4e981ae1b768768b0f011e53d6b968451
Reviewed-on: https://chromium-review.googlesource.com/788230
Commit-Queue: Ria Jiang <riajiang@chromium.org>
Reviewed-by: default avatarSadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519162}
parent c7fa8a9b
......@@ -909,12 +909,16 @@ void WindowServer::OnWindowHierarchyChanged(ServerWindow* window,
pending_system_modal_windows_.Remove(system_modal_window);
}
// This |window| is being removed, EventDispatcher::WindowNoLongerValidTarget
// is going to be notified to do the necessary update.
if (!new_parent)
return;
WindowManagerDisplayRoot* old_display_root =
old_parent ? display_manager_->GetWindowManagerDisplayRoot(old_parent)
: nullptr;
WindowManagerDisplayRoot* new_display_root =
new_parent ? display_manager_->GetWindowManagerDisplayRoot(new_parent)
: nullptr;
display_manager_->GetWindowManagerDisplayRoot(new_parent);
UpdateNativeCursorFromMouseLocation(new_display_root);
if (old_display_root != new_display_root)
UpdateNativeCursorFromMouseLocation(old_display_root);
......
......@@ -309,9 +309,8 @@ TEST_F(WindowTreeTest, EventDispatcherMouseCursorSourceWindowResetOnRemove) {
EXPECT_EQ(window, wms->event_dispatcher()->mouse_cursor_source_window());
window->parent()->Remove(window);
// The remove should reset the mouse_cursor_source_window(). The important
// thing is it changes to something other than |window|.
EXPECT_NE(window, wms->event_dispatcher()->mouse_cursor_source_window());
// The remove should reset the mouse_cursor_source_window().
EXPECT_FALSE(wms->event_dispatcher()->mouse_cursor_source_window());
}
// Verifies SetChildModalParent() works correctly.
......
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