New issue
Advanced search Search tips

Issue 874215 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 21
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: ----



Sign in to add a comment

Multi Touch to close a context menu with SEND_GESTURE_EVENTS_TO_OWNER causes all gesture events to break on a given widget until the widget is reset.

Project Member Reported by newcomer@chromium.org, Aug 14

Issue description

Chrome Version: 70.0.3519.0, but I have seen this repro on much earlier versions.


What steps will reproduce the problem?
(1) Show a context menu on an app icon by long press (the source of the menu must have the RunType SEND_GESTURE_EVENTS_TO_OWNER, so either a launcher or shelf app icon will work).
(2) Without releasing or moving the finger, touch the screen with a second finger.

What is the expected result?
The widget that owned the menu should continue to receive events as normal.

What happens instead?
1. The widget no longer receives GestureEvents, but can receive mouse events. 
2. All further touch (gesture) events at event_handler.cc as TouchEvents but these events never make it to the view.

Not sure exactly what's going on here. This bug has been around for a while (likely since the birth of SEND_GESTURE_EVENTS_TO_OWNER but we never had repro steps).

This is a P-1 for M-70 because this bug requires the user to kill the widget (app list) then show a new one, but this is not possible with the home launcher, so users will need to log in/out to make their AppList respond to touch events.

If this is reproduced on the shelf, the shelf becomes unresponsive until the user logs in/out.

 
Description: Show this description
Description: Show this description
Description: Show this description
Description: Show this description
Cc: newcomer@chromium.org weidongg@chromium.org jen...@chromium.org
 Issue 873772  has been merged into this issue.
 Issue 869679  has been merged into this issue.
https://chromium-review.googlesource.com/c/chromium/src/+/1179250

See CL description for details of my investigation.
Project Member

Comment 8 by bugdroid1@chromium.org, Aug 20

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

commit 5401a4db529d71f7093c0dbb08f2d2de305f1a87
Author: Xiyuan Xia <xiyuan@chromium.org>
Date: Mon Aug 20 18:56:40 2018

MenuController ignore touch if owner wants gesture

Gesture recognizer handles touches in two phases: event pre-dispatch
and post-dispatch. Closing MenuHost during dispatching skips the
post-dispatch phase because target (MenuHost) is destroyed. As
a result, the touch is never acked and gesture recognizer stops
generating any further gestures.

The above problem could be resolved either by asynchronously
closing the menu, or manually ack the touch event. The next problem
is that MenuController could not mark the touch event as handled.
Otherwise, gesture recognizer cancels the current sequence without
GESTURE_END event and breaks the owner's expectation.

If MenuController does not mark the touch as handled, gesture
recognizer would then translate the 2nd touch pressed into a
two-finger tap and causing the menu to flash because it is shown
again.

Given all that, a reasonable fix would be make MenuController to
skip touch events handling when the owner wants the current gesture
sequence. The problems would not happen if MenuHost is not closed
in middle of touch events dispatching. Side effect is that menu is
no longer closed on the 2nd figure down and could only be closed
after all figures are lifted, which is probably okay.

Bug:  874215 
Test: MenuControllerTest.NoTouchCloseWhenSendingGesturesToOwner
Change-Id: I1818b6c35c80006c999f96ddbea38fd1f79948c3
Reviewed-on: https://chromium-review.googlesource.com/1179250
Reviewed-by: Michael Wasserman <msw@chromium.org>
Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584519}
[modify] https://crrev.com/5401a4db529d71f7093c0dbb08f2d2de305f1a87/ui/views/controls/menu/menu_controller.cc
[modify] https://crrev.com/5401a4db529d71f7093c0dbb08f2d2de305f1a87/ui/views/controls/menu/menu_controller_unittest.cc

Status: Fixed (was: Untriaged)
Marking as fixed. Thank you Xiyuan@.

Sign in to add a comment