New issue
Advanced search Search tips

Issue 853681 link

Starred by 10 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 14
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug-Regression



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 description

Chrome 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!


 
ActualVideo.mp4
1.2 MB View Download
ExpectedVideo.mp4
996 KB View Download
Stack trace for the provided crash id:
--------------------------------------
Thread 0 (id: 0x28ec) CRASHED [EXCEPTION_ACCESS_VIOLATION_READ @ 0x00000060 ] MAGIC SIGNATURE THREAD
Stack Quality100%Show frame trust levels
0x00007fffce3a69fc	(chrome.dll -xhash:701 )	std::_Hash<std::_Umap_traits<int,IPC::Listener *,std::_Uhash_compare<int,std::hash<int>,std::equal_to<int> >,std::allocator<std::pair<const int,IPC::Listener *> >,0> >::equal_range
0x00007fffcea37e98	(chrome.dll -xhash:650 )	std::_Hash<std::_Umap_traits<unsigned int,unsigned int,std::_Uhash_compare<unsigned int,base_hash::hash<unsigned int>,std::equal_to<unsigned int> >,std::allocator<std::pair<const unsigned int,unsigned int> >,0> >::count
0x00007fffcfc14ab7	(chrome.dll -sequential_id_generator.cc:52 )	ui::SequentialIDGenerator::ReleaseNumber(unsigned int)
0x00007fffcf7985b4	(chrome.dll -hwnd_message_handler.cc:2923 )	views::HWNDMessageHandler::HandlePointerEventTypeTouch(unsigned int,unsigned __int64,__int64)
0x00007fffcf796f21	(chrome.dll -hwnd_message_handler.cc:1860 )	views::HWNDMessageHandler::OnPointerEvent(unsigned int,unsigned __int64,__int64)
0x00007fffcdc228d9	(chrome.dll -hwnd_message_handler.h:347 )	views::HWNDMessageHandler::_ProcessWindowMessage(HWND__ *,unsigned int,unsigned __int64,__int64,__int64 &,unsigned long)
0x00007fffcdc22043	(chrome.dll -hwnd_message_handler.cc:934 )	views::HWNDMessageHandler::OnWndProc(unsigned int,unsigned __int64,__int64)
0x00007fffcda4c01e	(chrome.dll -wrapped_window_proc.h:76 )	base::win::WrappedWindowProc<&gfx::WindowImpl::WndProc(HWND__ *,unsigned int,unsigned __int64,__int64)>(HWND__ *,unsigned int,unsigned __int64,__int64)
0x00007ffffd89b85c	(USER32.dll + 0x0000b85c )	UserCallWinProcCheckWow(_ACTIVATION_CONTEXT *,__int64 (*)(tagWND *,unsigned int,unsigned __int64,__int64),HWND__ *,_WM_VALUE,unsigned __int64,__int64,void *,int)
0x00007ffffd89b1ee	(USER32.dll + 0x0000b1ee )	DispatchMessageWorker
0x00007fffcdd5fa8b	(chrome.dll -message_pump_win.cc:366 )	base::MessagePumpForUI::ProcessMessageHelper(tagMSG const &)
0x00007fffcdace026	(chrome.dll -message_pump_win.cc:169 )	base::MessagePumpForUI::DoRunLoop()
0x00007fffcda24067	(chrome.dll -message_pump_win.cc:56 )	base::MessagePumpWin::Run(base::MessagePump::Delegate *)
0x00007fffcd9df150	(chrome.dll -run_loop.cc:102 )	base::RunLoop::Run()
0x00007fffcdd5c233	(chrome.dll -chrome_browser_main.cc:2053 )	ChromeBrowserMainParts::MainMessageLoopRun(int *)
0x00007fffcdd5c037	(chrome.dll -browser_main_loop.cc:976 )	content::BrowserMainLoop::RunMainMessageLoopParts()
0x00007fffcdd5bfe2	(chrome.dll -browser_main_runner_impl.cc:169 )	content::BrowserMainRunnerImpl::Run()
0x00007fffce53c02e	(chrome.dll -browser_main.cc:51 )	content::BrowserMain(content::MainFunctionParams const &,std::unique_ptr<content::BrowserProcessSubThread,std::default_delete<content::BrowserProcessSubThread> >)
0x00007fffceaa0e72	(chrome.dll -content_main_runner_impl.cc:621 )	content::RunBrowserProcessMain(content::MainFunctionParams const &,content::ContentMainDelegate *,std::unique_ptr<content::BrowserProcessSubThread,std::default_delete<content::BrowserProcessSubThread> >)
0x00007fffcd9d59db	(chrome.dll -content_main_runner_impl.cc:983 )	content::ContentMainRunnerImpl::Run()
0x00007fffcd9c5172	(chrome.dll -main.cc:459 )	service_manager::Main(service_manager::MainParams const &)
0x00007fffcd9c4a07	(chrome.dll -content_main.cc:19 )	content::ContentMain(content::ContentMainParams const &)
0x00007fffcd9c1af1	(chrome.dll -chrome_main.cc:101 )	ChromeMain
0x00007ff73d1d35d5	(chrome.exe -main_dll_loader_win.cc:201 )	MainDllLoader::Launch(HINSTANCE__ *,base::TimeTicks)
0x00007ff73d1d1698	(chrome.exe -chrome_exe_main_win.cc:230 )	wWinMain
0x00007ff73d290b65	(chrome.exe -exe_common.inl:283 )	__scrt_common_main_seh
0x00007ffffd0d1fe3	(KERNEL32.dll + 0x00011fe3 )	BaseThreadInitThunk
0x00007fffffbef060	(ntdll.dll + 0x0006f060 )	RtlUserThreadStart
Cc: dtapu...@chromium.org
Owner: nzolghadr@chromium.org
Navid can you assign someone to investigate this?
Status: Started (was: Assigned)
I'll take a look myself to see whether it is something related to our recent refactoring or not.
Owner: lanwei@chromium.org
Status: Assigned (was: Started)
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.
Cc: nzolghadr@chromium.org

Comment 6 by lanwei@chromium.org, Jun 21 2018

Sure, I will take a look.

Issue 862201 has been merged into this issue.
Status: Started (was: Assigned)
Issue 871198 has been merged into this issue.
Labels: ReleaseBlock-Beta
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.  
Cc: pbomm...@chromium.org
 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?
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.
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.
Labels: -ReleaseBlock-Beta
This should be still a Beta blocker for M70 based on comment #14.
Labels: ReleaseBlock-Beta
Labels: -Target-67 -Target-68 Target-70
Removing M67 and M68. 
Project Member

Comment 20 by bugdroid1@chromium.org, 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

Labels: TE-Verified-M70 TE-Verified-70.0.3522.0
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.
CanaryBehaviour.mp4
654 KB View Download
Status: Fixed (was: Started)
Labels: Merge-TBD
[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.
Labels: -Merge-TBD -Target-69
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.
Issue 874198 has been merged into this issue.
Issue 872312 has been merged into this issue.
 lanwei@, how safe is the change to merge to M69? 
I think it is very safe, it moves the code caused crash to a if statement to prevent the crash happen.
Labels: Merge-Request-69
The crash count went down today because of this fix.
Labels: -Merge-Request-69 Merge-Approved-69
Approving merge to M69 branch 3497 based on comments #21, #28, #29 and per offline chat with  lanwei@. Pls merge now. Thank you.
Project Member

Comment 31 by bugdroid1@chromium.org, Aug 14

Labels: -merge-approved-69 merge-merged-3497
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

Labels: TE-Verified-M69 TE-Verified-69.0.3497.42
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.
BetaBehaviour.mp4
1.0 MB View Download
Issue 877349 has been merged into this issue.
Issue 889894 has been merged into this issue.

Sign in to add a comment