New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 747131 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Last visit > 30 days ago
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Touch events don't work in --mash on Pixel 1

Project Member Reported by e...@chromium.org, Jul 20 2017

Issue description

On at least Pixel 1 devices, touch events don't bubble up to event dispatch
with --mash, though touch events work with --mus.

At the low level, in Ozone, in EventFactoryEvdev::DispatchTouchEvent(), events
are being generated and sent to through the PlatformEventSource(). This works
properly. What differs between --mash and --mus is in
DrmWindowHost::CanDispatchEvent(). This method does explicit checking on
whether an event is a touch event, and if it is, it does additional bounds
checking based on an associated display_id. This works in --mus, but earlies
out in --mash because the DeviceDataManager does not have an association
between the touch device and the display id.

OK, why does the DeviceDataManager not have a mapping between the touch device
and the display id?

Trace this down to where we set that data: the TouchTransformController. It has
a configuration specific |setter_| object which actually performs the local or
remote setting, but I believe that's a distraction. The data being sent to it
in TouchTransformController::UpdateTouchTransforms() is wrong; the list of
touch device transforms is empty in --mash mode, so there are no mappings sent
between display ids and touch device ids!

But why is *that*?

The TTC looks in the ManagedDisplay for an associated input device list. It is
this list which is empty at the time that we build the data which gets sent to
the TTC setter. Where does a ManagedDisplay get its input device information
from?

 

Comment 1 by e...@chromium.org, Jul 20 2017

OK, so where does ManagedDisplayInfo get its data from? The association is set
in display::AssociateTouchscreens() in touchscreen_util.cc. This is being
called in the ash process instead of in the mus server.

Comment 2 by sadrul@chromium.org, Jul 20 2017

Cc: sky@chromium.org
+sky@ because this may be related to the simple/simplified display management work.

Comment 3 by sky@chromium.org, Jul 20 2017

My guess is this logic comes from DisplayChangeObserver, which perhaps isn't created in mash? DisplayChangeObserver seems to be using ozone directly (InputDeviceManager), which we shouldn't be using in mus or mash.

Comment 4 by sky@chromium.org, Jul 20 2017

If DisplayChangeObserver is in fact the code we need, then I suspect we need to make it work with InputDeviceClient (at least for mus/mash). Ideally we should make it so there is one class that talks mojom that everyone uses. That way we don't have to worry about which code is allowed to use what.

Comment 5 by sky@chromium.org, Jul 21 2017

Actually, InputDeviceManager is wired up correctly for mus/mash. For mus/mash you end up using InputDeviceClient, which talks to the real InputDeviceManager in mus via mojo. So, perhaps the issue is we aren't creating DisplayChangeObserver at all. 
DisplayChangeObserver is what tells DisplayManager about the displays, so without that in mustash I think all you would get is a black screen on device.

Ash owns all the display management code again with simplified display management, so display::DisplayManager, display::ManagedDisplayInfo, etc. will only exist in the ash process. TouchTransformController |setter_| should forward the necessary information to mus-ws and set things in DeviceDataManager.

One of the following things should happen:
1. The first time DisplayChangeObserver::OnDisplayModeChanged() runs ui::InputDeviceManager::GetInstance()->GetTouchscreenDevices() isn't empty on Pixel 1.
2. If not #1, then a little bit later InputDeviceClient gets the initial list of touchsreen devices and it notifies observers. DisplayChangeObserver::OnTouchscreenDeviceConfigurationChanged() should run now and that should call DisplayChangeObserver::OnDisplayModeChanged() again.

Is one of those things happening in mustash?
Project Member

Comment 7 by bugdroid1@chromium.org, Jul 27 2017

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

commit 8e6b846f7cd265ddc7e6d060966db51b27b177a1
Author: Elliot Glaysher <erg@chromium.org>
Date: Thu Jul 27 17:37:12 2017

Fix touch events in mash.

The InputDeviceClient which listens for device configuration events is created
in mash, but didn't connect to the ui service to be an observer. Now connects
to the InputDeviceServer on startup.

Bug:  747131 
Change-Id: Id2ad0bbc05d2caf8f7a369c2f01bc5e4eb967813
Reviewed-on: https://chromium-review.googlesource.com/588009
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Elliot Glaysher <erg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490013}
[modify] https://crrev.com/8e6b846f7cd265ddc7e6d060966db51b27b177a1/ash/mus/window_manager.cc

Comment 8 by e...@chromium.org, Jul 27 2017

Status: Fixed (was: Assigned)

Comment 9 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)
Components: -MUS Internals>Services>WindowService

Sign in to add a comment