New issue
Advanced search Search tips

Issue 900363 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

Mash: 'Check failed: got_touch_point' on overview mode tab dragging touch

Project Member Reported by mukai@chromium.org, Oct 30

Issue description

- run with --enable-features=SingleProcessMash, enabling touch
- enter into the tablet mode
- create a new browser window
- drag a browser window tab with touch
- drop the dragged tab into an existing window

This causes a crash with "Check failed: got_touch_point."
 
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 1

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

commit 0b5ca9b697200ea57e0420beb147f4fa8fbd153d
Author: Jun Mukai <mukai@chromium.org>
Date: Thu Nov 01 01:27:23 2018

Transfer touch events back to the original when the window move ends

The current behavior makes TransferEventsTo() before the window
move starts, but don't care at its end. This causes errors like
 crbug.com/900363 . But we can't simply transfer back to the original
window unconditionally since someone (like TabDragController) may
also want to continue dragging on a window other than the original
source window.

This CL introduces a new scoped class to control this; it makes
TransferEventsTo first, and then invokes TransferEventsTo back
again at the end, but it skips invoking the second transfer if
someone else also transfers the touch events on the same window.

BUG= 900363 
TEST=the new test case

Change-Id: I81792872a13f0e4bb2c88b526c92cf689d656b71
Reviewed-on: https://chromium-review.googlesource.com/c/1310501
Commit-Queue: Jun Mukai <mukai@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604473}
[modify] https://crrev.com/0b5ca9b697200ea57e0420beb147f4fa8fbd153d/ui/views/mus/desktop_window_tree_host_mus.cc
[modify] https://crrev.com/0b5ca9b697200ea57e0420beb147f4fa8fbd153d/ui/views/mus/desktop_window_tree_host_mus_unittest.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Nov 1

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

commit 8993d59085b2eb5e4fd86cc477de01721fa3f789
Author: Findit <findit-for-me@appspot.gserviceaccount.com>
Date: Thu Nov 01 06:13:34 2018

Revert "Transfer touch events back to the original when the window move ends"

This reverts commit 0b5ca9b697200ea57e0420beb147f4fa8fbd153d.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 604473 as the
culprit for flakes in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/flake/flake-culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyQwsSDEZsYWtlQ3VscHJpdCIxY2hyb21pdW0vMGI1Y2E5YjY5NzIwMGVhNTdlMDQyMGJlYjE0N2Y0ZmE4ZmJkMTUzZAw

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.chromiumos/linux-chromeos-dbg/8644

Sample Failed Step: views_mus_unittests

Sample Flaky Test: DesktopWindowTreeHostMusTest.WindowMoveShouldNotTransfersBack

Original change's description:
> Transfer touch events back to the original when the window move ends
> 
> The current behavior makes TransferEventsTo() before the window
> move starts, but don't care at its end. This causes errors like
>  crbug.com/900363 . But we can't simply transfer back to the original
> window unconditionally since someone (like TabDragController) may
> also want to continue dragging on a window other than the original
> source window.
> 
> This CL introduces a new scoped class to control this; it makes
> TransferEventsTo first, and then invokes TransferEventsTo back
> again at the end, but it skips invoking the second transfer if
> someone else also transfers the touch events on the same window.
> 
> BUG= 900363 
> TEST=the new test case
> 
> Change-Id: I81792872a13f0e4bb2c88b526c92cf689d656b71
> Reviewed-on: https://chromium-review.googlesource.com/c/1310501
> Commit-Queue: Jun Mukai <mukai@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#604473}

No-Presubmit: true
No-Tree-Checks: true
No-Try: true
BUG= 900363 ,  900848 

Change-Id: Ie326f3b57b2869b0981beb73344fbb530ca21721
Reviewed-on: https://chromium-review.googlesource.com/c/1312613
Cr-Commit-Position: refs/heads/master@{#604528}
[modify] https://crrev.com/8993d59085b2eb5e4fd86cc477de01721fa3f789/ui/views/mus/desktop_window_tree_host_mus.cc
[modify] https://crrev.com/8993d59085b2eb5e4fd86cc477de01721fa3f789/ui/views/mus/desktop_window_tree_host_mus_unittest.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Nov 1

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

commit d97470374c265a0b307bbfbc25c9e7ae2148fb13
Author: Jun Mukai <mukai@chromium.org>
Date: Thu Nov 01 21:47:47 2018

Reland "Transfer touch events back to the original when the window move ends"

This is a reland of crrev.com/604473 with fixes of:
- addressing some review comments which are missed in CQ
- the test flakiness

The test flakiness comes from my misunderstanding; this test case
actually creates a window server process which runs test_ws.
The default behavior of PerformWindowMove in test_ws is to fail
immediately.

Therefore this CL changes test_ws -- PerformWindowMove now
remembers the done_callback, and then call it when cancel is
requested.

BUG= 900363 
TEST=repeat 20 views_mus_unittests

Change-Id: I61bcb06bf626a73d0a9d998c0bb1511feea45494
Reviewed-on: https://chromium-review.googlesource.com/c/1312985
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Jun Mukai <mukai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604715}
[modify] https://crrev.com/d97470374c265a0b307bbfbc25c9e7ae2148fb13/services/ws/test_ws/test_window_service.cc
[modify] https://crrev.com/d97470374c265a0b307bbfbc25c9e7ae2148fb13/services/ws/test_ws/test_window_service.h
[modify] https://crrev.com/d97470374c265a0b307bbfbc25c9e7ae2148fb13/ui/views/mus/desktop_window_tree_host_mus.cc
[modify] https://crrev.com/d97470374c265a0b307bbfbc25c9e7ae2148fb13/ui/views/mus/desktop_window_tree_host_mus_unittest.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Nov 2

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

commit 790f54b0e7ec638ad5d1cd44bf6da291034dcd10
Author: Jun Mukai <mukai@chromium.org>
Date: Fri Nov 02 23:05:35 2018

reenable TabDragging DragToOverviewWindow/1

Fixed by crrev.com/604972 and crrev.com/604715

BUG= 900363 ,  890071 
TEST=trybot

Change-Id: I726db69c603595a2f4ec4f49a6976b9f57c737c8
Reviewed-on: https://chromium-review.googlesource.com/c/1314598
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Jun Mukai <mukai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605086}
[modify] https://crrev.com/790f54b0e7ec638ad5d1cd44bf6da291034dcd10/testing/buildbot/filters/chromeos.single_process_mash.interactive_ui_tests.filter

Status: Fixed (was: Started)

Sign in to add a comment