New issue
Advanced search Search tips

Issue 805714 link

Starred by 1 user

Issue metadata

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

Blocking:
issue 731255



Sign in to add a comment

mus: Touch event locations incorrect for unified display mode

Project Member Reported by msw@chromium.org, Jan 24 2018

Issue description

mus: Touch event locations incorrect for unified display mode

(1) Run ToT chrome on device per go/simplechrome (eg. cros 10328.0.0 / chrome 66.0.3329.0 ToT @ #530902 on cyan, similar on link)
(2) Add --mus and --ash-enable-unified-desktop flags to /etc/chrome_dev.conf per go/mustash-intro (or enable corresponding about:flags)
(3) Connect an external display to use the device in unified mode
(4) Attempt to use touch interaction (eg. drag a browser window)
Expected: Touch interaction works as expected
Actual: Touch window-dragging event locations are offset (windows slide around)

Taps also don't seem to reach their intended targets.
This defect was split out from  Issue 804460 .
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jan 26 2018

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

commit 1423331f0f4c970708468f354a71f1a9714fedda
Author: Mike Wasserman <msw@chromium.org>
Date: Fri Jan 26 18:47:44 2018

mus: Fix event processing for touch events in unified mode

This refines my earlier work done in: http://crrev.com/c/842467

Move event target nulling and root location use to WindowTreeClient, to ensure events reach MusUnifiedEventTargeter.
Otherwise, WindowEventDispatcher::GetInitialEventTarget will use the touch gesture recognizer's target (returned by GetPriorityTargetInRootWindow) before performing the mirror display transformation on the event's location.

Inline DispatchEventToTarget for additional clarity.

Bug:  805714 
Test: Mus touch events work in Chrome OS's unified desktop mode.
Change-Id: Ic4844c85cc24a9f9885e7705ff27cb57b305d245
Reviewed-on: https://chromium-review.googlesource.com/887250
Commit-Queue: Michael Wasserman <msw@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532030}
[modify] https://crrev.com/1423331f0f4c970708468f354a71f1a9714fedda/ash/host/ash_window_tree_host_mus_unified.cc
[modify] https://crrev.com/1423331f0f4c970708468f354a71f1a9714fedda/ui/aura/mus/window_tree_client.cc

Comment 2 by msw@chromium.org, Jan 26 2018

Status: Fixed (was: Started)
Project Member

Comment 3 by bugdroid1@chromium.org, Jan 29 2018

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

commit 751cc85737f6fd902b2f371318b3f88686adba44
Author: Makoto Shimazu <shimazu@chromium.org>
Date: Mon Jan 29 05:23:06 2018

Revert "mus: Fix event processing for touch events in unified mode"

This reverts commit 1423331f0f4c970708468f354a71f1a9714fedda.

Reason for revert: speculative attempt due to unhappy MSan ChromiumOS bot.
See also  crbug.com/806638 .

Original change's description:
> mus: Fix event processing for touch events in unified mode
> 
> This refines my earlier work done in: http://crrev.com/c/842467
> 
> Move event target nulling and root location use to WindowTreeClient, to ensure events reach MusUnifiedEventTargeter.
> Otherwise, WindowEventDispatcher::GetInitialEventTarget will use the touch gesture recognizer's target (returned by GetPriorityTargetInRootWindow) before performing the mirror display transformation on the event's location.
> 
> Inline DispatchEventToTarget for additional clarity.
> 
> Bug:  805714 
> Test: Mus touch events work in Chrome OS's unified desktop mode.
> Change-Id: Ic4844c85cc24a9f9885e7705ff27cb57b305d245
> Reviewed-on: https://chromium-review.googlesource.com/887250
> Commit-Queue: Michael Wasserman <msw@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#532030}

TBR=sky@chromium.org,msw@chromium.org

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

Bug:  805714 
Change-Id: I02333546f91d0d0f495153c8556aa9820612673d
Reviewed-on: https://chromium-review.googlesource.com/890841
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Commit-Queue: Makoto Shimazu <shimazu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532328}
[modify] https://crrev.com/751cc85737f6fd902b2f371318b3f88686adba44/ash/host/ash_window_tree_host_mus_unified.cc
[modify] https://crrev.com/751cc85737f6fd902b2f371318b3f88686adba44/ui/aura/mus/window_tree_client.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Jan 29 2018

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

commit 738278ebbe1b7600e893ff8b134a2f3a7af2862f
Author: Michael Wasserman <msw@chromium.org>
Date: Mon Jan 29 17:37:47 2018

Revert "Revert "mus: Fix event processing for touch events in unified mode""

This reverts commit 751cc85737f6fd902b2f371318b3f88686adba44.

Reason for revert: Incorrect speculative revert.
The actual change causing this failure was likely http://crrev.com/c/887282 for Issue 805937
That CL actually touched the failing test, and its revert caused the bot to pass:
https://ci.chromium.org/buildbot/chromium.memory/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29/25915
I'm reverting this seemingly incorrect speculative revert of my CL.

Original change's description:
> Revert "mus: Fix event processing for touch events in unified mode"
> 
> This reverts commit 1423331f0f4c970708468f354a71f1a9714fedda.
> 
> Reason for revert: speculative attempt due to unhappy MSan ChromiumOS bot.
> See also  crbug.com/806638 .
> 
> Original change's description:
> > mus: Fix event processing for touch events in unified mode
> > 
> > This refines my earlier work done in: http://crrev.com/c/842467
> > 
> > Move event target nulling and root location use to WindowTreeClient, to ensure events reach MusUnifiedEventTargeter.
> > Otherwise, WindowEventDispatcher::GetInitialEventTarget will use the touch gesture recognizer's target (returned by GetPriorityTargetInRootWindow) before performing the mirror display transformation on the event's location.
> > 
> > Inline DispatchEventToTarget for additional clarity.
> > 
> > Bug:  805714 
> > Test: Mus touch events work in Chrome OS's unified desktop mode.
> > Change-Id: Ic4844c85cc24a9f9885e7705ff27cb57b305d245
> > Reviewed-on: https://chromium-review.googlesource.com/887250
> > Commit-Queue: Michael Wasserman <msw@chromium.org>
> > Reviewed-by: Scott Violet <sky@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#532030}
> 
> TBR=sky@chromium.org,msw@chromium.org
> 
> # Not skipping CQ checks because original CL landed > 1 day ago.
> 
> Bug:  805714 
> Change-Id: I02333546f91d0d0f495153c8556aa9820612673d
> Reviewed-on: https://chromium-review.googlesource.com/890841
> Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
> Commit-Queue: Makoto Shimazu <shimazu@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#532328}

TBR=sky@chromium.org,msw@chromium.org,shimazu@chromium.org

Change-Id: I6c7b91429abdddb4f9db881c40c64f53141bc707
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  805714 
Reviewed-on: https://chromium-review.googlesource.com/890822
Reviewed-by: Michael Wasserman <msw@chromium.org>
Commit-Queue: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532476}
[modify] https://crrev.com/738278ebbe1b7600e893ff8b134a2f3a7af2862f/ash/host/ash_window_tree_host_mus_unified.cc
[modify] https://crrev.com/738278ebbe1b7600e893ff8b134a2f3a7af2862f/ui/aura/mus/window_tree_client.cc

Components: -Internals>MUS Internals>Services>WindowService
Components: -MUS

Sign in to add a comment