Commit 6f32bb34 authored by Ria Jiang's avatar Ria Jiang Committed by Commit Bot

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

This reverts commit 63b59877.

Reason for revert: Mistakenly thought that cursor provider would be updated if necessary in https://cs.chromium.org/chromium/src/services/ui/ws/event_dispatcher.cc?type=cs&l=754 when called from WindowNoLongerValidTarget. But we only update mouse_cursor_source_window and not cursor provider because it was not the capture_window.

Original change's description:
> 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: Sadrul Chowdhury <sadrul@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#519162}

TBR=sadrul@chromium.org,riajiang@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 788185
Change-Id: Ib1c2b4366eda94ccbf8f4d0ff22cf9012a27803b
Reviewed-on: https://chromium-review.googlesource.com/792150Reviewed-by: default avatarRia Jiang <riajiang@chromium.org>
Reviewed-by: default avatarScott Violet <sky@chromium.org>
Commit-Queue: Ria Jiang <riajiang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519544}
parent 6fe52aa2
......@@ -909,16 +909,12 @@ 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 =
display_manager_->GetWindowManagerDisplayRoot(new_parent);
new_parent ? display_manager_->GetWindowManagerDisplayRoot(new_parent)
: nullptr;
UpdateNativeCursorFromMouseLocation(new_display_root);
if (old_display_root != new_display_root)
UpdateNativeCursorFromMouseLocation(old_display_root);
......
......@@ -309,8 +309,9 @@ 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().
EXPECT_FALSE(wms->event_dispatcher()->mouse_cursor_source_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());
}
// 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