Turns out that the |platform_window_| in WindowTreeHostPlatform is created as a StubWindow by AshWindowTreeHostUnified [1]. The StubWindow does nothing when SetCapture() is called.

Oshima, and Kyle, is that the expected behavior?


[1]: https://cs.chromium.org/chromium/src/ash/host/ash_window_tree_host_unified.cc?type=cs&q=AshWindowTreeHostUnified&l=52

That's not the issue. Problem is that passive grab should happen on mirroring window, not on the stub window hosting desktop.

There are actually two problems here. If you patch in https://crrev.com/c/705115 then it's a little more obvious. The unified mouse warp controller only supports two displays. I see a lot of this error when running with three or four displays in unified mode.

[139039:139039:1011/093202.049751:ERROR:unified_mouse_warp_controller.cc(123)] Only two displays are supported

After patching in https://crrev.com/c/705115 you can only move between the first and second display.

Luckily, the other problem still exists with only two displays. I'm running with the following command line flags:

--ash-host-window-bounds=10+10-600x600,+660+10-600x600 --ash-enable-unified-desktop

If you start dragging a window from a point on display 2 back and hit the left edge, the mouse warp controller moves the cursor to the rightmost edge of display 1. The window moves as if the cursor is past the leftmost edge of display 1 instead of where the cursor actually is.

The problem is that display 2 still has capture since the mouse hasn't been released. UnifiedMouseWarpController moves the cursor from display 2 to display 1. The mouse event coordinates are still in relation to display 2, so the X coordinate is negative, since display 1 is to the left of display 2. The mouse event gets converted to screen coordinates by MirroringScreenPositionClient which is for the whole unified desktop area. I added a couple log statements seen here.

[145931:145931:1011/100327.379709:ERROR:x11_window_base.cc(257)] MoveCursorTo() 598,48 on 10,28 600x600
[145931:145931:1011/100327.379826:ERROR:mirror_window_controller.cc(66)] MirroringScreenPositionClient::ConvertHostPointToScreen initial_point=-52,46
[145931:145931:1011/100327.379847:ERROR:mirror_window_controller.cc(71)] MirroringScreenPositionClient::ConvertHostPointToScreen converted_point=-52,46

The problem is MirroringScreenPositionClient doesn't take into the different XWindows since everything has the same root aura::Window. I'm not sure how/if the USE_X11 build handled this? I would assume USE_X11 had the same problem, since it's the X11 implicit capture that is causing the problem.

> The unified mouse warp controller only supports two displays.

That's what afakhry@ is trying to fix.

> The problem is that display 2 still has capture since the mouse hasn't been released.

Yet, ozone dispatches dispatches event to display 1 because it's relying on explicit capture.

I'm fine to change the behavior, but it has to be consistent between unified and non unified scenario.

> [139039:139039:1011/093202.049751:ERROR:unified_mouse_warp_controller.cc(123)] Only two displays are supported

I fixed this problem as part of this CL: https://chromium-review.googlesource.com/c/chromium/src/+/683094

> The problem is that display 2 still has capture since the mouse hasn't been released...

Does that mean that there is an implicit capture by X11 (which sends the located events relative to display 2), and an explicit capture done by ozone (which doesn't work in this case since it tried to capture the unified stub window, but not the mirroring one)?

On a real device:

   - In non-unified mode, explicit capture works because we forward the SetCapture() call to the DrmWindowHost(), and hence events keeps getting sent to the source display no matter where you are now.

   - In unified mode, since explicit capture doesn't work (because of the stub window), the events are sent to the current display (not the source display), and the |event->location()| is relative to the **current** display as well (no negative coordinates).

Oshima believes that this is the wrong way to implement passive grab, because if SetCapture() ends up being not called (like in the case of unified desktop mirroring hosts), then it won't work.


I have a work-around fix for this that involves using |event->root_location()|, but for a long term solution and to unify the behavior between X11 and ozone, what was the intended behavior when ozone was implemented? (this comment: "For now Ozone works in a similar manner as X11..." [1] makes it sound like ozone was intended to match the behavior of X11).


[1]: https://cs.chromium.org/chromium/src/ash/display/screen_position_controller.cc?type=cs&q=ScreenPositionController::ConvertHostPointToRelativeToRootWindow&l=49

> Does that mean that there is an implicit capture by X11 (which sends the located events relative to display 2), and an explicit capture done by ozone (which doesn't work in this case since it tried to capture the unified stub window, but not the mirroring one)?

Yes I think so. There is explicit capture in X11WindowOzone. This works the same as DrmWindowHost. There is also X11 implicit passive grab that I think is always there? Maybe there is some way to toggle it off.

> what was the intended behavior when ozone was implemented?

I'm not sure what the Ozone DRM intended behaviour was. For Ozone X11 I just tried to make sure that dragging between windows worked correctly and tests passed. I also didn't know unified mode existed at that point. +sadrul who might be able to clarify.

Is there anything actionable needed for this bug?

Me and oshima considered two alternatives.
- Either work around it in the unified desktop case, or
- remove passive grab entirely.

But we decided to do this after we land the Unified Desktop changes.

That sgtm.

It would be nice if ash was consistent here, which I think means doing a grab on the real display when in unified mode. This way we wouldn't have two possible paths for events. It would also mean you see consistent behavior when running on the desktop (ozone-x11 backend) vs real device (ozone-drm backend).

Oshima and I agreed before that we should get rid of the grab entirely. We had that for historical reasons when we were dependent on X11. The Ozone implementation simulated the same behavior for no reason. We think it will be better if we remove it completely.

If you do that, won't you see much different behavior when running on desktop chromeos vs actual device? It seems it would be nice if the two behaved the same.

The goal was to also undo the passive grab done by x11, so the behavior in the end should be the same.

Correction. The grab was fpr old, non ozone x11 impl (it was x11 that is grabbing the input). We can remove grab from both ozone-x11 and ozone-drm and can have consistent behavior on both.

As long as we are consistent, I'm happy.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/84b41dc990a9810ae67a3f6583faadf72422fdb0

commit 84b41dc990a9810ae67a3f6583faadf72422fdb0
Author: Scott Violet <sky@chromium.org>
Date: Thu Jan 25 06:24:57 2018

window-service: don't do a grab when in unified mode

Classic ash doesn't do grabs when in unified mode. For compatibility
this patch makes the window-service not execute a grab when in unified
mode. To execute a grab leads to ash doing the wrong event conversion.

BUG= 804460 ,773348
TEST=covered by test

Change-Id: I6d402ec4eb5d68946f98757cc4a56e850bd1e8b0
Reviewed-on: https://chromium-review.googlesource.com/884569
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531828}
[modify] https://crrev.com/84b41dc990a9810ae67a3f6583faadf72422fdb0/services/ui/ws/display_manager.cc
[modify] https://crrev.com/84b41dc990a9810ae67a3f6583faadf72422fdb0/services/ui/ws/display_manager.h
[modify] https://crrev.com/84b41dc990a9810ae67a3f6583faadf72422fdb0/services/ui/ws/test_utils.cc
[modify] https://crrev.com/84b41dc990a9810ae67a3f6583faadf72422fdb0/services/ui/ws/test_utils.h
[modify] https://crrev.com/84b41dc990a9810ae67a3f6583faadf72422fdb0/services/ui/ws/window_manager_state.cc
[modify] https://crrev.com/84b41dc990a9810ae67a3f6583faadf72422fdb0/services/ui/ws/window_manager_state_unittest.cc