Issue metadata
Sign in to add a comment
|
Touch-clicking can cause Chromium to exit with DCHECK.
Reported by
seadu...@amazon.com,
Oct 16
|
||||||||||||||||||||||
Issue descriptionSteps to reproduce the problem: * Browse to 'bing.com' and search 'face' * Scroll down and select article for faces on Wikipedia * Press the link multiple times as the page is loading Starting with 861a8b2697787f4ace52605e053d0388c57b68f7 Chromium on Android devices will exit with a DCHECK when pressing elements multiple times when the browser is still loading the page. I am able to reproduce this by searching on 'bing.com', searching 'face' and then pressing the wikipedia article for faces multiple times, I then get the following (with the values at the end changing everytime) What is the expected behavior? The page should load What went wrong? SIGABRT is thrown with the following message ``` Abort message: '[FATAL:touch_disposition_gesture_filter.cc(185)] Check failed: packet.unique_touch_event_id() != Head().front().unique_touch_event_id() (56 vs. 56) ``` Did this work before? Yes https://chromium.googlesource.com/chromium/src/+/c135680f43581688a4620e96676b98cb58465597 Chrome version: Master Channel: n/a OS Version: v70/master Flash Version: This happens in v70, so might be worth triaging soon (not sure the effect it will have on release Android users) Also if anyone can point me in the right direction for fixing this would appreciate it, I am not familiar with this area so digging through it now. This was also discussed on https://bugs.chromium.org/p/chromium/issues/detail?id=867087 Someone that is more familiar should look at this soon, I don't know what effect it may have on v70 users. thanks!
,
Oct 17
,
Oct 17
@seaduboi: Could you please let us know where and to check DCHECK? On connecting mobile to desktop and inspecting doesn't show this result in console. And also not observing faces on Wikipedia article on scrolling down, screencast would help in further triaging. Also cc'ing wjmaclean@ from above mentioned cr 861a8b2697787f4ace52605e053d0388c57b68f7 for further inputs. Thanks!
,
Oct 17
@seaduboi: When you indicate that it did work before, do you mean it used to work as of r582684?
,
Oct 17
FWIW, I suspect given the unusual repro instructions, and the fact that it's a DCHECK issue, I don't *think* this will have a strong impact on M70 users, though I could be wrong.
,
Oct 17
,
Oct 17
@wjmaclean Yes it worked with r582684, but starting with r582685 I get the DCHECK. Sorry I don't have a automated/reliable reproduce. I will figure out what exactly is causing this. I haven't seen any issues in release builds! Crash rates seem consistent with prior releases, but still would like to get to the bottom of the DCHECK. I always assume the worst when I don't understand something fully. thanks!
,
Oct 17
Thank you for providing more feedback. Adding the requester to the cc list. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Oct 18
It wouldn't be possible to reproduce the issue unless we have a reliable repro steps. Hence adding Needs-Feedback label till we get some info. Thanks!
,
Oct 18
I think I know what the issue is. I'll put up a CL today for seaduboi@ to try.
,
Oct 18
seaduboi@ - I've put up a CL at https://chromium-review.googlesource.com/c/chromium/src/+/1288891 Could you please try this on a local build and see if it solves the issue for you? The CL description gives my explanation of why I think we're hitting the DCHECK at present.
,
Oct 19
@wjmaclean That fixes the issue for me. Thank you for taking time to look into is!
,
Oct 19
No worries ... thanks for your report, and help with testing!
,
Oct 25
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0da589ad1be1bf58188bb1191fa5304eab9bdd30 commit 0da589ad1be1bf58188bb1191fa5304eab9bdd30 Author: W. James MacLean <wjmaclean@chromium.org> Date: Thu Oct 25 21:46:42 2018 Update DCHECK to handle case where event with same ID is already in queue. This CL corrects a DCHECK which can now be triggered as a consequence of https://chromium-review.googlesource.com/1169528. Prior to that CL, when a new gesture is synthesized from a TOUCH_TIMEOUT such as a GestureShowPress or GestureLongPress, it got a unique_touch_event_id of 0. The CL changed this so the event would take on the unique_touch_event_id of the event that caused it. This means it's possible for two adjacent events in the queue to share the same value, thus causing the DCHECK to fire. Here we modify the DCHECK so that any duplicate events, which will all have the TOUCH_TIMEOUT source attribute, don't trigger the DCHECK. TBR=tdresser@chromium.org Bug: 896042 Change-Id: I51c45903a2ca2d0dac8e3939ff8193246f843380 Reviewed-on: https://chromium-review.googlesource.com/c/1288891 Reviewed-by: James MacLean <wjmaclean@chromium.org> Commit-Queue: James MacLean <wjmaclean@chromium.org> Cr-Commit-Position: refs/heads/master@{#602867} [modify] https://crrev.com/0da589ad1be1bf58188bb1191fa5304eab9bdd30/ui/events/gesture_detection/touch_disposition_gesture_filter.cc
,
Oct 25
Marking this as fixed, please re-open if the problem continues.
,
Oct 25
Thanks @wjmaclean for getting this all the way through!
,
Oct 25
,
Oct 25
This bug requires manual review: M71 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Oct 30
Merge approved to 71, branch 3578.
,
Oct 30
,
Oct 30
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/44f39cc872d605ee441ca6bae91441b08a0c6e5a commit 44f39cc872d605ee441ca6bae91441b08a0c6e5a Author: W. James MacLean <wjmaclean@chromium.org> Date: Tue Oct 30 14:55:37 2018 Update DCHECK to handle case where event with same ID is already in queue. This CL corrects a DCHECK which can now be triggered as a consequence of https://chromium-review.googlesource.com/1169528. Prior to that CL, when a new gesture is synthesized from a TOUCH_TIMEOUT such as a GestureShowPress or GestureLongPress, it got a unique_touch_event_id of 0. The CL changed this so the event would take on the unique_touch_event_id of the event that caused it. This means it's possible for two adjacent events in the queue to share the same value, thus causing the DCHECK to fire. Here we modify the DCHECK so that any duplicate events, which will all have the TOUCH_TIMEOUT source attribute, don't trigger the DCHECK. TBR=tdresser@chromium.org Bug: 896042 Change-Id: I51c45903a2ca2d0dac8e3939ff8193246f843380 Reviewed-on: https://chromium-review.googlesource.com/c/1288891 Reviewed-by: James MacLean <wjmaclean@chromium.org> Commit-Queue: James MacLean <wjmaclean@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#602867}(cherry picked from commit 0da589ad1be1bf58188bb1191fa5304eab9bdd30) Reviewed-on: https://chromium-review.googlesource.com/c/1307251 Cr-Commit-Position: refs/branch-heads/3578@{#391} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} [modify] https://crrev.com/44f39cc872d605ee441ca6bae91441b08a0c6e5a/ui/events/gesture_detection/touch_disposition_gesture_filter.cc
,
Oct 30
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/44f39cc872d605ee441ca6bae91441b08a0c6e5a Commit: 44f39cc872d605ee441ca6bae91441b08a0c6e5a Author: wjmaclean@chromium.org Commiter: wjmaclean@chromium.org Date: 2018-10-30 14:55:37 +0000 UTC Update DCHECK to handle case where event with same ID is already in queue. This CL corrects a DCHECK which can now be triggered as a consequence of https://chromium-review.googlesource.com/1169528. Prior to that CL, when a new gesture is synthesized from a TOUCH_TIMEOUT such as a GestureShowPress or GestureLongPress, it got a unique_touch_event_id of 0. The CL changed this so the event would take on the unique_touch_event_id of the event that caused it. This means it's possible for two adjacent events in the queue to share the same value, thus causing the DCHECK to fire. Here we modify the DCHECK so that any duplicate events, which will all have the TOUCH_TIMEOUT source attribute, don't trigger the DCHECK. TBR=tdresser@chromium.org Bug: 896042 Change-Id: I51c45903a2ca2d0dac8e3939ff8193246f843380 Reviewed-on: https://chromium-review.googlesource.com/c/1288891 Reviewed-by: James MacLean <wjmaclean@chromium.org> Commit-Queue: James MacLean <wjmaclean@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#602867}(cherry picked from commit 0da589ad1be1bf58188bb1191fa5304eab9bdd30) Reviewed-on: https://chromium-review.googlesource.com/c/1307251 Cr-Commit-Position: refs/branch-heads/3578@{#391} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by bugsnash@chromium.org
, Oct 17