New issue
Advanced search Search tips

Issue 901546 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 6
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocking:
issue 890071



Sign in to add a comment

TabDragging/DetachToBrowserTabDragControllerTest.DragWithMaskedWindows is failing with SingleProcessMash

Project Member Reported by mukai@chromium.org, Nov 2

Issue description

TabDragging/DetachToBrowserTabDragControllerTest.DragWithMaskedWindows/* (both 0 and 1) are failing with SingleProcessMash
 
Cc: osh...@chromium.org sky@chromium.org
This is failing since the window mask rules created by the browser isn't well affecting the logic in Ash.

It is a bit unclear to me if we really want to keep this test case though. Oshima@ originally added this test case for supporting chat-head like hangout window (the app window which has hittest masks). Do we still want to support that?
What api is this using to change the shape of the window?
It's event targeter. I think the question is if the client can use event targeting logic to identify the target for given location.
Labels: Pri-2
Is that how apps achieve the behaviors?  I mean, the window targeter with masks for chathead-like UI?
My understanding is that a browser needs a way to find a window at given location using the same targeting logic that server uses in order to implement tab dragging.

Ash does use targeter to control event targetering so i think it should work regardless of whether or not we have such app right now. (and it doesn't mean we will not)

Since browser window can't see other windows, I guess we need a find-window api
(if we don't have it yet)?
A window targeter installed within browser would work as expected but may not work well with the window finder in Mash. I've introduced TopmostWindowObserver for this purpose but it doesn't deal with this type of special targeter.

I am still wondering if this is still the case happening on actual ChromeOS UI session or now it's a hypothetical use of targeter.  I believe Chathead UI of Hangout is gone. Are there any real apps doing this? Where are the actual code of window targeter?
ChromeOS does use WindowTageter in many places

https://cs.chromium.org/search/?q=SetEventTargeter+file:exo&sq=package:chromium&type=cs
https://cs.chromium.org/search/?q=WindowTargeter+file:src/ash/&type=cs

I believe there are targeters that aren't important for this case, but I don't think we
can guarantee that it will not cause an issue if ignored.

Don't worry about the targeters which resides within Ash, it works as same. The targeter within the browser is the question.
The problem is that targeter in ash should be also taken into account when finding the browser window, because there are other windows on the desktop. 
In the test scenario of DragWithMaskedWindows, browser process creates a window targeter and Ash knows nothing about that.

Is it the actual case of things like chatheads, or is this just how the test was written for the classic environment? Who creates the window targeter and where are those targeters installed?
Project Member

Comment 13 by bugdroid1@chromium.org, Nov 6

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

commit a6966377234dde60fdf90502ffbe29770d604c4f
Author: Jun Mukai <mukai@chromium.org>
Date: Tue Nov 06 22:29:02 2018

Fix on DragWithMaskedWindows

The current combination of WindowTargeter with MaskedWindowDelegate
isn't working well with Mash. Instead of doing this, it needs to
use window targeter's new API for setting window masks.

The GetEventHandlerForPoint should be fixed in a way to follow
this new way.

BUG= 901546 
TEST=the new test case also covers

Change-Id: I0b095777450fab3fa28ff9ebd8473667f644ab9e
Reviewed-on: https://chromium-review.googlesource.com/c/1320689
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Jun Mukai <mukai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605853}
[modify] https://crrev.com/a6966377234dde60fdf90502ffbe29770d604c4f/ash/wm/window_finder.cc
[modify] https://crrev.com/a6966377234dde60fdf90502ffbe29770d604c4f/ash/wm/window_finder_unittest.cc
[modify] https://crrev.com/a6966377234dde60fdf90502ffbe29770d604c4f/chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc
[modify] https://crrev.com/a6966377234dde60fdf90502ffbe29770d604c4f/testing/buildbot/filters/chromeos.single_process_mash.interactive_ui_tests.filter

Status: Fixed (was: Assigned)

Sign in to add a comment