New issue
Advanced search Search tips

Issue 769507 link

Starred by 2 users

Issue metadata

Status: Duplicate
Merged: issue 867074
Owner: ----
Closed: Aug 15
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocked on:
issue 571688

Blocking:
issue 867074



Sign in to add a comment

mash: Touch tab dragging needs the gesture recognizer

Project Member Reported by jamescook@chromium.org, Sep 27 2017

Issue description

Chrome ToT r504699, OS R63-9972, link, with --mash

* Open a couple tabs in a maximized window
* Touch-drag one out
* Touch-drag the tab around for a while

I'm not sure exactly what action triggers the crash.

[8188:8188:0927/142429.267008:689477623:ERROR:drag_window_resizer.cc(24)] Not implemented reached in virtual void ash::mus::DragWindowResizer::Drag(const gfx::Point &, int)
[8187:8187:0927/142429.267222:FATAL:tab_drag_controller.cc(1759)] Check failed: got_touch_point. 
#0 0x7f20b7fe070c base::debug::StackTrace::StackTrace()
#1 0x7f20b7ff661e logging::LogMessage::~LogMessage()
#2 0x7f20ba395418 TabDragController::GetCursorScreenPoint()
#3 0x7f20ba395205 TabDragController::OnWidgetBoundsChanged()
#4 0x7f20b95e0644 views::Widget::OnNativeWidgetMove()
#5 0x7f20bb53bbfd views::DesktopNativeWidgetAura::OnHostMovedInPixels()
#6 0x7f20b91a3387 aura::WindowTreeHost::OnHostMovedInPixels()
#7 0x7f20b91a413a aura::WindowTreeHostPlatform::OnBoundsChanged()
#8 0x7f20b9e81aec views::DesktopWindowTreeHostMus::SetBoundsInPixels()
#9 0x7f20b9197f6e aura::WindowTreeHostMus::SetBoundsFromServer()
#10 0x7f20b918c9d9 aura::WindowTreeClient::SetWindowBoundsFromServer()
#11 0x7f20b918f84e aura::WindowTreeClient::OnWindowBoundsChanged()
#12 0x7f20b665b65a ui::mojom::WindowTreeClientStubDispatch::Accept()
#13 0x7f20b8ba50b8 mojo::InterfaceEndpointClient::HandleValidatedMessage()
#14 0x7f20b8babe77 mojo::internal::MultiplexRouter::ProcessIncomingMessage()
#15 0x7f20b8bab64d mojo::internal::MultiplexRouter::Accept()
#16 0x7f20b8ba4342 mojo::Connector::ReadSingleMessage()
#17 0x7f20b8ba4ac1 mojo::Connector::ReadAllAvailableMessages()
#18 0x7f20b66bb2c2 mojo::SimpleWatcher::DiscardReadyState()
#19 0x7f20b8ba08f4 mojo::SimpleWatcher::OnHandleReady()
#20 0x7f20b7fe0f09 base::debug::TaskAnnotator::RunTask()
#21 0x7f20b7ffc267 base::MessageLoop::RunTask()
#22 0x7f20b7ffca66 base::MessageLoop::DoWork()
#23 0x7f20b7ffe3e9 base::MessagePumpLibevent::Run()
#24 0x7f20b801f02a base::RunLoop::Run()
#25 0x7f20b9e813b5 views::DesktopWindowTreeHostMus::RunMoveLoop()
#26 0x7f20ba394ba1 TabDragController::RunMoveLoop()
#27 0x7f20ba396466 TabDragController::DetachIntoNewBrowserAndRunMoveLoop()
#28 0x7f20ba395bdc TabDragController::DragBrowserToNewTabStrip()
#29 0x7f20ba394dec TabDragController::ContinueDragging()
#30 0x7f20ba3934f9 TabDragController::Drag()
#31 0x7f20ba20b823 TabStrip::ContinueDrag()
#32 0x7f20ba2105be TabStrip::OnGestureEvent()
#33 0x7f20b897c9fb ui::EventDispatcher::ProcessEvent()
#34 0x7f20b897c83c ui::EventDispatcherDelegate::DispatchEvent()
#35 0x7f20bb26535f ui::EventProcessor::OnEventFromSource()
#36 0x7f20b897d47d ui::EventSource::SendEventToSink()
#37 0x7f20b897c9fb ui::EventDispatcher::ProcessEvent()
#38 0x7f20b897c83c ui::EventDispatcherDelegate::DispatchEvent()
#39 0x7f20b919e487 aura::WindowEventDispatcher::ProcessGestures()
#40 0x7f20b91a040a aura::WindowEventDispatcher::PostDispatchEvent()
#41 0x7f20b897c8d7 ui::EventDispatcherDelegate::DispatchEvent()
#42 0x7f20bb26535f ui::EventProcessor::OnEventFromSource()
#43 0x7f20b897d47d ui::EventSource::SendEventToSink()
#44 0x7f20b9190c55 aura::WindowTreeClient::OnWindowInputEvent()
#45 0x7f20b665acb8 ui::mojom::WindowTreeClientStubDispatch::Accept()
#46 0x7f20b8ba50b8 mojo::InterfaceEndpointClient::HandleValidatedMessage()
#47 0x7f20b8babe77 mojo::internal::MultiplexRouter::ProcessIncomingMessage()
#48 0x7f20b8bab64d mojo::internal::MultiplexRouter::Accept()
#49 0x7f20b8ba4342 mojo::Connector::ReadSingleMessage()
#50 0x7f20b8ba4ac1 mojo::Connector::ReadAllAvailableMessages()
#51 0x7f20b66bb2c2 mojo::SimpleWatcher::DiscardReadyState()
#52 0x7f20b8ba08f4 mojo::SimpleWatcher::OnHandleReady()
#53 0x7f20b7fe0f09 base::debug::TaskAnnotator::RunTask()
#54 0x7f20b7ffc267 base::MessageLoop::RunTask()
#55 0x7f20b7ffca66 base::MessageLoop::DoWork()
#56 0x7f20b7ffe3e9 base::MessagePumpLibevent::Run()
#57 0x7f20b801f02a base::RunLoop::Run()
#58 0x7f20b7cef708 ChromeBrowserMainParts::MainMessageLoopRun()
#59 0x7f20b68e6ab4 content::BrowserMainLoop::RunMainMessageLoopParts()
#60 0x7f20b68e8ee2 content::BrowserMainRunnerImpl::Run()
#61 0x7f20b68e23cc content::BrowserMain()

 

Comment 1 by e...@chromium.org, Oct 10 2017

The crash is handling in chromeos specific code in tab_drag_controller.cc, where instead of just grabbing the current cursor screen point, it asks the gesture recognizer where the current touch point is if touch is down.

When running this not on device, but on linux with --touch-devices=<x> to emulate touch events, you can see the process which results in this crash easier:

1) Open a window with 2 tabs.
2) Start a drag of a tab out of the window.
3) While the touch is still down (mouse down emulated), notice that the new window was created for the drag, but it isn't moving as you drag around.
4) Release the drag. Notice the window hasn't resopnded.
5) Start a new drag down on the tab. This is where it crashes.

Comment 2 by e...@chromium.org, Oct 12 2017

There are a few parts to this bug.

Fixing the immediate crash is easy; just add a !ash_util::IsMashRunning() check to the block in tab_drag_controller.cc which is trying to directly access the GestureRecognizer. In classic and mus, grabbing data from the one global gesture recognizer isn't a problem. In mash, the instance of the gesture recognizer isn't getting the gesture data during the move loop.

Fixing touch tab dragging is a bit more involved. In mash, move loops are an orchestration between the window server, window manager and client. At first glance, it looks like the touch parts of move loops are just unimplemented, and this is why the new window isn't moving once you drag the new window out of the old one.

Comment 3 by e...@chromium.org, Oct 12 2017

The old way of dragging used by classic ash and mus has a single drag client at the top of the aura hierarchy which coordinates everything. Since everything is in a single process, in a single aura::Window tree, the TabDragController code can use the built in GestureRecognizer::TransferEventsTo() code to switch the gesture stream from one window to another.

Things don't work like that in mash. In mash, when a window requests going into a move loop, the window manager takes capture and starts receiving the move events. It takes responsibility of moving the window. This is analogous to how things work on Windows and Linux.

The window manager takes over on request of the application. The application starts the move in response to a ET_TOUCH_DOWN event. The internals of the GestureRecognizer in the window manager never see a TOUCH DOWN event, and become confused, rejecting all subsequent move events. This explains the behaviour where the window manager appears to be receiving the touch events, but isn't acting on them.
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 12 2017

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

commit 89c78dd72524196c048a31456d5dbf7f7b285d33
Author: Elliot Glaysher <erg@chromium.org>
Date: Thu Oct 12 23:38:22 2017

Fix TabDragController crash in mash.

Currently, TabDragController assumes that if you're in ash, that there's a
single GestureRecognizer that it can go to to get the global state. In mush,
this assumption is wrong. In move loops, window moving is handled by the window
server redirecting events to the window manager. However, the window manager
appears to not be doing anything with these events.

This disables getting the "cursor" position from touch for the time being. This
fixes the crash, but it doesn't make dragging work. It's a short term hack
until the bigger problem of touch event routing gets figured out.

Bug:  769507 
Change-Id: Iee2e605aec4ae3393ffeee3956ff7aa4732721a3
Reviewed-on: https://chromium-review.googlesource.com/716566
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Elliot Glaysher <erg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508541}
[modify] https://crrev.com/89c78dd72524196c048a31456d5dbf7f7b285d33/chrome/browser/ui/views/tabs/tab_drag_controller.cc

Comment 5 by e...@chromium.org, Oct 13 2017

Blockedon: 571688
Labels: -Pri-1 Pri-2
Summary: mash: Touch tab dragging needs the gesture recognizer (was: mash: TabDragController CHECK failure got_touch_point when dragging tab)
This no longer crashes after #4 landed.

However, tab dragging does not work because there's no equivalent to TransferEventsTo() across processes. This is needed for this to work, but this is only broken in mash so this isn't high priority.

Comment 6 by e...@chromium.org, Mar 6 2018

Owner: ----
Status: Available (was: Assigned)
Mass unassigning bugs
Owner: pkasting@chromium.org
pkasting@ could you please triage?

Comment 8 by sky@chromium.org, Apr 10 2018

Components: -UI>Shell>WindowManager
Owner: ----
This is chromeos specific, and for mash. I'm marking available for now.
Components: Internals>Services>Ash
Labels: -Proj-Mustash-Mash
Blocking: 867074
Labels: Proj-Mustash
Mergedinto: 867074
Status: Duplicate (was: Available)
This will be fixed as part of 867074.

Sign in to add a comment