Issue metadata
Sign in to add a comment
|
Regression:Browser crash is seen on 'Edit bookmark' overlay on touch device
Reported by
vineetha...@etouch.net,
Jun 18 2018
|
||||||||||||||||||||||
Issue descriptionChrome Version: 69.0.3464.0 (Official Build) Revision 3c26b60e3842fee660bcff5eb35aa0587d795f02-refs/branch-heads/3464@{#1}(64 bit) OS: Windows 10(Touch device) What steps will reproduce the problem? (1) Launch chrome, click on the bookmark star icon to open 'Bookmark added' overlay. (2) Click on 'More..' button to open 'Edit bookmark' overlay. (3) Select 'Bookmarks bar' and then click on 'New folder' button to create a New Folder under Bookmarks bar. (4) Tap touch on the added New folder to open context menu. (5) Now select the 'Delete' option from the context menu using tap touch and observe. Actual Result : Browser gets crashed on deleting new folder in 'Edit bookmark' overlay. Expected Result: Browser should not get crashed on deleting new folder in 'Edit bookmark' overlay . Crash IDs: d857de340ffdbebb (Local Crash ID: 23e022ae-33af-49f7-9ce0-e951fe95ae6f) a05898abb53af231 (Local Crash ID: db982a0b-034d-4b54-bbc7-233f5e323e38) This is regression issue broken in ‘M-66’, below is the manual bisect info: Good build: 66.0.3345.0(Revision: 536027) Bad build : 66.0.3346.0(Revision: 536238) Unable to provide bisect using per-revision script,hence re-bisected with old script: Narrow Bisect info : https://chromium.googlesource.com/chromium/src/+log/9acf995e9af77804c7367e7cb2587962250e8654..76e19961dc5ff8f27812cef9de0e4b85621f2e7c?pretty=fuller&n=10000 Suspect: r536136? @dtapuska: Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner. Note: This issue is not seen on Windows(7,8,8.1), Mac(10.12.6, 10.13.1, 10.13.5, 10.13.6)and Linux(14.04) OS. Thank You!
,
Jun 18 2018
Navid can you assign someone to investigate this?
,
Jun 18 2018
I'll take a look myself to see whether it is something related to our recent refactoring or not.
,
Jun 21 2018
Lan, the crash seem to be caused by the use of WM_POINTER and when I disabled the flag it doesn't happen anymore. Would you be able to investigate this further. I used the lab machines for touch screen Windows in case you need it.
,
Jun 21 2018
,
Jun 21 2018
Sure, I will take a look.
,
Jul 10
Issue 862201 has been merged into this issue.
,
Jul 31
,
Aug 7
Issue 871198 has been merged into this issue.
,
Aug 8
,
Aug 8
This is regressed since M66, so we're not planning to block M69 beta for this. Pls let us know ASAP if there is any concern here.
,
Aug 8
,
Aug 8
Issue 871198 is the same problem as this one, and it has ReleaseBlock-Beta. That is why I marked this one ReleaseBlock-Beta as well. Do you think it is possible we merge this one to M69 once it is landed?
,
Aug 8
Based on the crash data as of now I don't consider this as a Beta blocker for Chrome version 69. For Chrome version 70 this should be a Beta blocker based on number of crash reports on last week Dev i.e., 70.0.3510.2(389 crashes from 80 clients) and too early to call that the crash rate has dropped on latest Dev i.e, 70.0.3514.0. Since we have a testcase to reproduce the issue, can we please get the fix sooner on M70 and merge back to M69 branch.
,
Aug 8
Thank you pbommana@. Based on comment #14, not a blocker for M69 Beta. Also unless, we see a spike in M69 for this crash compare to previous versions and fix is fully safe to merge, I would hesitate to take this merge in for M69 as this is regressed since M66.
,
Aug 8
,
Aug 8
This should be still a Beta blocker for M70 based on comment #14.
,
Aug 8
,
Aug 9
Removing M67 and M68.
,
Aug 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b7199f26a97df56c79b3853c94359b847db19a16 commit b7199f26a97df56c79b3853c94359b847db19a16 Author: lanwei <lanwei@chromium.org> Date: Mon Aug 13 19:05:58 2018 Move ReleaseNumber before dispatching the events In HWNDMessageHandler::HandlePointerEventTypeTouch, the ReleaseNumber is after the touch events are dispatched, which will cause a crash when calling id_generator_, because the current |HWNDMessageHandler| is already destroyed so is |id_generator_|, when the current window is closed and new window pops out. We should move it before we dispatch the events. Bug: 853681 Change-Id: I0f2a6319a746aa0b38111693fa89ca639dad05fd Reviewed-on: https://chromium-review.googlesource.com/1166039 Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org> Commit-Queue: Lan Wei <lanwei@chromium.org> Cr-Commit-Position: refs/heads/master@{#582656} [modify] https://crrev.com/b7199f26a97df56c79b3853c94359b847db19a16/ui/views/win/hwnd_message_handler.cc
,
Aug 14
Update : Rechecked the above issue on Windows 10 touch device with latest Canary Chrome version #70.0.3522.0 and the issue is fixed. Kindly refer the attached screen cast.
,
Aug 14
,
Aug 14
[Auto-generated comment by a script] We noticed that this issue is targeted for M-69; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-69 label, otherwise remove Merge-TBD label. Thanks.
,
Aug 14
I asked govind@ yesterday, it seems that it is very close to M69 stable launch and they are trying to take only fully critical merges in. This fix will not be in M69.
,
Aug 14
Issue 874198 has been merged into this issue.
,
Aug 14
Issue 872312 has been merged into this issue.
,
Aug 14
lanwei@, how safe is the change to merge to M69?
,
Aug 14
I think it is very safe, it moves the code caused crash to a if statement to prevent the crash happen.
,
Aug 14
The crash count went down today because of this fix.
,
Aug 14
Approving merge to M69 branch 3497 based on comments #21, #28, #29 and per offline chat with lanwei@. Pls merge now. Thank you.
,
Aug 14
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/df2922be3247f8e4cd56728075dd021860b07fde commit df2922be3247f8e4cd56728075dd021860b07fde Author: lanwei <lanwei@chromium.org> Date: Tue Aug 14 21:07:25 2018 Move ReleaseNumber before dispatching the events In HWNDMessageHandler::HandlePointerEventTypeTouch, the ReleaseNumber is after the touch events are dispatched, which will cause a crash when calling id_generator_, because the current |HWNDMessageHandler| is already destroyed so is |id_generator_|, when the current window is closed and new window pops out. We should move it before we dispatch the events. TBR=lanwei@chromium.org (cherry picked from commit b7199f26a97df56c79b3853c94359b847db19a16) Bug: 853681 Change-Id: I0f2a6319a746aa0b38111693fa89ca639dad05fd Reviewed-on: https://chromium-review.googlesource.com/1166039 Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org> Commit-Queue: Lan Wei <lanwei@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#582656} Reviewed-on: https://chromium-review.googlesource.com/1175103 Reviewed-by: Lan Wei <lanwei@chromium.org> Cr-Commit-Position: refs/branch-heads/3497@{#633} Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753} [modify] https://crrev.com/df2922be3247f8e4cd56728075dd021860b07fde/ui/views/win/hwnd_message_handler.cc
,
Aug 16
Update : Rechecked the above issue on Windows 10 touch device with Chrome Beta version #69.0.3497.42 and the issue is fixed. Kindly refer the attached screen cast.
,
Aug 24
Issue 877349 has been merged into this issue.
,
Oct 3
Issue 889894 has been merged into this issue. |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by rbasuvula@chromium.org
, Jun 18 2018