Browser crash while adjusting zoom in the device simulator
Reported by
henry71...@gmail.com,
Oct 31
|
||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/72.0.3596.0 Safari/537.36 Steps to reproduce the problem: 1. Go to any website 2. Open Devtools, then enable the device simulator 3. (In the device simulator) Zoom in the page a little bit (Shift Button + Mouse Drag) 4. (In the device simulator) Zoom all the way out 5. Crash! What is the expected behavior? What went wrong? Browser crash while adjusting zoom in the device simulator Did this work before? N/A Chrome version: 72.0.3596.0 Channel: canary OS Version: OS X 10.14.0 Flash Version:
,
Oct 31
Thanks for the report, I can repro on Linux, not on Mac. Narrow bisect to: https://chromium.googlesource.com/chromium/src/+log/6bd8396acb447e99b39a858d7c201ec603cdd2f8..3b28095869d8938998c4d14be9d569a65b4fe31a Suspect: https://chromium.googlesource.com/chromium/src/+/c2469abf7b94fa0d0303aa3615899eb4bc3deb36 wjmaclean@: have there been other related crash reports?
,
Nov 1
No, there haven't been other crash reports. Does anyone have a stack trace?
,
Nov 1
Marking this as Pri-1 since there's a browser crash involved. I have a CL up to fix this at https://chromium-review.googlesource.com/c/chromium/src/+/1312619 So this appears to be a somewhat long-standing issue that has been exposed by the suspect CL. The TouchEmulator used by DevTools sort of 'cheats' when it creates a Pinch event, i.e. it doesn't send out touch events with two touch points and let the GestureProvider create the PinchEvents like would normally happen. Instead, when the shift key is depressed, it intercepts GestureScrollBegin/Update/End and coerces them into their Pinch event counterpart. This works up to a point, but the root cause of this bug is that while the GestureProvider would never create a GestureFlingStart in the midst of a GesturePinch sequence, it will in this case. When GestureFlingStart hits TouchEmulator::OnGestureEvent(), it sends a synthetic GestureScrollEnd *without a unique touch event id*. This makes its way to RenderWidgetHostInputEventRouter::DispatchTouchscreenGestureEvent (but with a null |target|), causing a crash when we try to get the transform to a null view. 'Flinging' at the end of an emulated pinch is this only mechanism to trigger this whole mess, so I'm not surprised we didn't see it sooner. That explains why it's a little tricky to reproduce ... you need to pinch aggressively (or perhaps have your Mac touchpad set to be fast?).
,
Nov 1
Issue 901167 has been merged into this issue.
,
Nov 2
We haven't landed the fix yet, so this will continue to reproduce until we do. Thanks for the trackpad speed :-) I was mostly thinking out loud about why some people might be more likely to trigger this bug than others.
,
Nov 5
Since this crash been seen on M71(Issue#901167) and ranked as top crasher on Beta channel marking the bug as M71 stable blocker so that this won't slip into Stable.
,
Nov 5
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/903831909fced0ea6fd6b612233942f0e2b2de63 commit 903831909fced0ea6fd6b612233942f0e2b2de63 Author: W. James MacLean <wjmaclean@chromium.org> Date: Mon Nov 05 20:06:54 2018 TouchEmulator must put a touch event id on ScrollEnd. Because of the unusual way that the TouchEmulator creates pinch events, it's possible for a GestureFlingStart (created by the GestureRecognizer) to terminate a pinch sequence. In that event a synthetic GestureScrollEnd is inserted. Prior to this CL, that GestureScrollEnd had a zero value for unique_touch_event_id, which in turn caused problems when it was sent to DispatchTouchscreenGestureEvent, along with a null target. Bug: 900496 Change-Id: I290757980def3f55a0111a2e31f8424ce06e96f1 Reviewed-on: https://chromium-review.googlesource.com/c/1312619 Commit-Queue: James MacLean <wjmaclean@chromium.org> Reviewed-by: Ken Buchanan <kenrb@chromium.org> Cr-Commit-Position: refs/heads/master@{#605432} [modify] https://crrev.com/903831909fced0ea6fd6b612233942f0e2b2de63/content/browser/renderer_host/input/touch_emulator.cc [modify] https://crrev.com/903831909fced0ea6fd6b612233942f0e2b2de63/content/browser/renderer_host/input/touch_emulator.h [modify] https://crrev.com/903831909fced0ea6fd6b612233942f0e2b2de63/content/browser/renderer_host/render_widget_host_browsertest.cc [modify] https://crrev.com/903831909fced0ea6fd6b612233942f0e2b2de63/content/browser/renderer_host/render_widget_host_input_event_router.cc [modify] https://crrev.com/903831909fced0ea6fd6b612233942f0e2b2de63/content/browser/renderer_host/render_widget_host_view_base.h
,
Nov 5
Once this bakes, we should merge it to M-71. We just need to merge the one-line from TouchEmulator::ScrollEnd(). Adding the flag now as I'm OOO after today (though I may be able to do the merge, hoping kenrb@ can do the merge if I can't).
,
Nov 5
Pls update bug with canary result tomorrow,
,
Nov 6
The NextAction date has arrived: 2018-11-06
,
Nov 6
How is the change looking in canary?
,
Nov 6
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
,
Nov 6
So far there aren't any crashes on latest Chrome canary i.e., 72.0.3603.0(This is the Chrome version which picked the CL from comment#9)
,
Nov 6
Approving merge to M71 branch 3578 based on comment #15. Pls merge tomorrow morning if canary data continue to look good. Thank you.
,
Nov 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b08d394d0b57f000a89f3c0260ef4a89e21906bc commit b08d394d0b57f000a89f3c0260ef4a89e21906bc Author: W. James MacLean <wjmaclean@chromium.org> Date: Wed Nov 07 21:19:54 2018 TouchEmulator must put a touch event id on ScrollEnd. Because of the unusual way that the TouchEmulator creates pinch events, it's possible for a GestureFlingStart (created by the GestureRecognizer) to terminate a pinch sequence. In that event a synthetic GestureScrollEnd is inserted. Prior to this CL, that GestureScrollEnd had a zero value for unique_touch_event_id, which in turn caused problems when it was sent to DispatchTouchscreenGestureEvent, along with a null target. Bug: 900496 Change-Id: I290757980def3f55a0111a2e31f8424ce06e96f1 Reviewed-on: https://chromium-review.googlesource.com/c/1312619 Commit-Queue: James MacLean <wjmaclean@chromium.org> Reviewed-by: Ken Buchanan <kenrb@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#605432}(cherry picked from commit 903831909fced0ea6fd6b612233942f0e2b2de63) Reviewed-on: https://chromium-review.googlesource.com/c/1324413 Cr-Commit-Position: refs/branch-heads/3578@{#572} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} [modify] https://crrev.com/b08d394d0b57f000a89f3c0260ef4a89e21906bc/content/browser/renderer_host/input/touch_emulator.cc [modify] https://crrev.com/b08d394d0b57f000a89f3c0260ef4a89e21906bc/content/browser/renderer_host/input/touch_emulator.h [modify] https://crrev.com/b08d394d0b57f000a89f3c0260ef4a89e21906bc/content/browser/renderer_host/render_widget_host_browsertest.cc [modify] https://crrev.com/b08d394d0b57f000a89f3c0260ef4a89e21906bc/content/browser/renderer_host/render_widget_host_input_event_router.cc [modify] https://crrev.com/b08d394d0b57f000a89f3c0260ef4a89e21906bc/content/browser/renderer_host/render_widget_host_view_base.h
,
Nov 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b08d394d0b57f000a89f3c0260ef4a89e21906bc Commit: b08d394d0b57f000a89f3c0260ef4a89e21906bc Author: wjmaclean@chromium.org Commiter: kenrb@chromium.org Date: 2018-11-07 21:19:54 +0000 UTC TouchEmulator must put a touch event id on ScrollEnd. Because of the unusual way that the TouchEmulator creates pinch events, it's possible for a GestureFlingStart (created by the GestureRecognizer) to terminate a pinch sequence. In that event a synthetic GestureScrollEnd is inserted. Prior to this CL, that GestureScrollEnd had a zero value for unique_touch_event_id, which in turn caused problems when it was sent to DispatchTouchscreenGestureEvent, along with a null target. Bug: 900496 Change-Id: I290757980def3f55a0111a2e31f8424ce06e96f1 Reviewed-on: https://chromium-review.googlesource.com/c/1312619 Commit-Queue: James MacLean <wjmaclean@chromium.org> Reviewed-by: Ken Buchanan <kenrb@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#605432}(cherry picked from commit 903831909fced0ea6fd6b612233942f0e2b2de63) Reviewed-on: https://chromium-review.googlesource.com/c/1324413 Cr-Commit-Position: refs/branch-heads/3578@{#572} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} |
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by krajshree@chromium.org
, Oct 31