mash: Event root_location, location are wrongly remembered |
||
Issue descriptionThrough debugging tab-dragging behavior, I've noticed that there's confusion of root_location of mouse events which causes weird offsetting calculation in Mash. This will affect both single-process and out-of-process mash. Here are my observations: - the last mouse location is recorded in aura::Env (as last_mouse_location). Two entities set this -- ui/aura/env_input_state_controller and ui/aura/mus/mus_mouse_location_updater - window-tree transmits the mouse events as-is, so the 'root_location' would be actually the location from the root-window in ash (i.e. relative to the display) - mus_mouse_location_updater record this 'root_location' as the last_mouse_location directly, while env_input_state_controller converts the root_location into screen-coordinate - since the 'root window' is created per browser window in browser process, this conversion adds the offset to 'root_location'. - tab-dragging sometimes this offsetted root_location can be picked up, which causes window moving weirdly. These inconsistency is weird, although I guess the root cause is sharing of the same 'root_location' -- window-tree shouldn't share its root_location as-is, instead it should compute the location from the actual root window which the browser process knows. However, I am afraid that this could break something else since it breaks an assumption -- root_location is relative to display in ChromeOS.
,
Aug 21
Ugh. I think we need to make Env's root_location match what it is in ash. Here's what we have: . To handle requests for the mouse location at random times ws2 maintains a shared memory buffer that is updated anytime the mouse moves. WindowTreeClient holds the shared memory in the client, and specifically Env calls to WindowTreeClient::GetCursorScreenPoint() to get this value. . During event processing we wanted to make sure calls to Env::last_mouse_location() returned the same value as the event being processed. MusMouseLocationUpdater is responsible for determining when Env uses the shared memory, or the value from the event. I think the right fix is to have MusMouseLocationUpdater take into account the offset of the display, so that it's screen relatve.
,
Aug 25
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cc619c2df891e5ef3b5d299ab332d048b285c442 commit cc619c2df891e5ef3b5d299ab332d048b285c442 Author: Jun Mukai <mukai@chromium.org> Date: Sat Aug 25 00:01:18 2018 Mash: make root_location in root window coordinate It seems that Event's root_location is in display's coordinate in WindowTreeClient (i.e. browser). This could complicate the coordinate conversions like aura::ScreenCoordinateClient and cause problems like crbug.com/876509 . Since root_location is relative to the root window in desktop aura (like desktop Linux), I believe we want to do so too on Mash. This is actually contradicting with crrev.com/583488 but I believe this is better for us. Bug: 876509 Test: unit tests pass, manually checked crbug.com/873763 behavior Change-Id: I51f03772e2872783cf5fb9ce78cdcfe3dd7b0e67 Reviewed-on: https://chromium-review.googlesource.com/1187134 Commit-Queue: Jun Mukai <mukai@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Reviewed-by: Michael Wasserman <msw@chromium.org> Cr-Commit-Position: refs/heads/master@{#586067} [modify] https://crrev.com/cc619c2df891e5ef3b5d299ab332d048b285c442/services/ui/ws2/window_tree.cc [modify] https://crrev.com/cc619c2df891e5ef3b5d299ab332d048b285c442/services/ui/ws2/window_tree.h [modify] https://crrev.com/cc619c2df891e5ef3b5d299ab332d048b285c442/services/ui/ws2/window_tree_unittest.cc [modify] https://crrev.com/cc619c2df891e5ef3b5d299ab332d048b285c442/ui/aura/env_input_state_controller.cc [modify] https://crrev.com/cc619c2df891e5ef3b5d299ab332d048b285c442/ui/aura/mus/mus_mouse_location_updater.cc [modify] https://crrev.com/cc619c2df891e5ef3b5d299ab332d048b285c442/ui/aura/window_targeter.cc [modify] https://crrev.com/cc619c2df891e5ef3b5d299ab332d048b285c442/ui/aura/window_targeter_unittest.cc
,
Aug 31
Jun, is this fixed now?
,
Aug 31
yes |
||
►
Sign in to add a comment |
||
Comment 1 by mukai@chromium.org
, Aug 21