New issue
Advanced search Search tips

Issue 815161 link

Starred by 4 users

Issue metadata

Status: Verified
Owner:
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: 2018-03-12
OS: Mac
Pri: 1
Type: Bug



Sign in to add a comment

DownloadItemController receives handleButtonClick: after dealloc

Project Member Reported by ellyjo...@chromium.org, Feb 23 2018

Issue description

Call stack:

0x000000010ed3efad	(Google Chrome Framework -objc_zombie.mm:234 )	(anonymous namespace)::ZombieObjectCrash(objc_object*, objc_selector*, objc_selector*)
0x000000010ed3f021	(Google Chrome Framework -objc_zombie.mm:287 )	-[CrZombie performSelector:withObject:]
0x00007fff8697c079	(libsystem_trace.dylib + 0x00002079 )	
0x00007fff9366adbc	(AppKit + 0x002b1dbc )	-[NSApplication sendAction:to:from:]
0x000000010d34200a	(Google Chrome Framework -chrome_browser_application_mac.mm:232 )	__43-[BrowserCrApplication sendAction:to:from:]_block_invoke
0x000000010d6fc769	(Google Chrome Framework + 0x01dc7769 )	base::mac::CallWithEHFrame(void () block_pointer)
0x000000010d341f05	(Google Chrome Framework -chrome_browser_application_mac.mm:231 )	-[BrowserCrApplication sendAction:to:from:]
0x00007fff9367cf11	(AppKit + 0x002c3f11 )	-[NSControl sendAction:to:]
0x000000010e279b57	(Google Chrome Framework -hover_button.mm:123 )	-[HoverButton mouseDown:]
0x00007fff93bce3c8	(AppKit + 0x008153c8 )	-[NSWindow _handleMouseDownEvent:isDelayedEvent:]
0x00007fff93bcf3ac	(AppKit + 0x008163ac )	-[NSWindow _reallySendEvent:isDelayedEvent:]
0x00007fff9360e538	(AppKit + 0x00255538 )	-[NSWindow sendEvent:]
0x000000010fe7e82e	(Google Chrome Framework -chrome_event_processing_window.mm:78 )	-[ChromeEventProcessingWindow sendEvent:]
0x00007fff9358ea37	(AppKit + 0x001d5a37 )	-[NSApplication sendEvent:]
0x000000010d342419	(Google Chrome Framework -chrome_browser_application_mac.mm:267 )	__34-[BrowserCrApplication sendEvent:]_block_invoke
0x000000010d6fc769	(Google Chrome Framework + 0x01dc7769 )	base::mac::CallWithEHFrame(void () block_pointer)
0x000000010d3421f5	(Google Chrome Framework -chrome_browser_application_mac.mm:251 )	-[BrowserCrApplication sendEvent:]
0x00007fff933f5df1	(AppKit + 0x0003cdf1 )	-[NSApplication run]
0x000000010d70b56b	(Google Chrome Framework -message_pump_mac.mm:806 )	base::MessagePumpNSApplication::DoRun(base::MessagePump::Delegate*)
0x000000010d70a0ed	(Google Chrome Framework -message_pump_mac.mm:180 )	base::MessagePumpCFRunLoopBase::Run(base::MessagePump::Delegate*)
0x000000010d72e254	(Google Chrome Framework -run_loop.cc:130 )	<name omitted>
0x000000010d347937	(Google Chrome Framework -chrome_browser_main.cc:1973 )	ChromeBrowserMainParts::MainMessageLoopRun(int*)
0x000000010c0b8923	(Google Chrome Framework -browser_main_loop.cc:1236 )	content::BrowserMainLoop::RunMainMessageLoopParts()
0x000000010c0bb2f1	(Google Chrome Framework -browser_main_runner.cc:145 )	content::BrowserMainRunnerImpl::Run()
0x000000010c0b4f1b	(Google Chrome Framework -browser_main.cc:46 )	content::BrowserMain(content::MainFunctionParams const&)
0x000000010d2fa75f	(Google Chrome Framework -content_main_runner.cc:717 )	content::ContentMainRunnerImpl::Run()
0x000000010ec0de4a	(Google Chrome Framework -main.cc:456 )	service_manager::Main(service_manager::MainParams const&)
0x000000010d2f9ca3	(Google Chrome Framework -content_main.cc:19 )	content::ContentMain(content::ContentMainParams const&)
0x000000010b939369	(Google Chrome Framework -chrome_main.cc:129 )	ChromeMain
0x000000010b6cedd3	(Google Chrome -chrome_exe_main_mac.cc:165 )	main
0x00007fff922cf5ac	(libdyld.dylib + 0x000035ac )

Dealloc stack:

0x0340a512 [Google Chrome Framework -	 objc_zombie.mm:134] (anonymous namespace)::ZombieDealloc(objc_object*, objc_selector*)
0x0003bf80 [AppKit +	 0x3bf80] -[NSResponder dealloc]
0x003282a2 [AppKit +	 0x3282a2] -[NSViewController dealloc]
0x04564907 [Google Chrome Framework -	 download_item_controller.mm:131] -[DownloadItemController dealloc]
0x000929a9 [AppKit +	 0x929a9] -[NSViewController release]
0x045674e0 [Google Chrome Framework -	 download_shelf_controller.mm:313] -[DownloadShelfController removeDownload:isShelfClosing:]
0x008513c0 [Google Chrome Framework -	 weak_ptr.h:240] content::DownloadItemImpl::UpdateObservers()
0x00856ae7 [Google Chrome Framework -	 download_item_impl.cc:1831] content::DownloadItemImpl::Completed()
0x0085693e [Google Chrome Framework -	 download_item_impl.cc:1771] content::DownloadItemImpl::OnDownloadRenamedToFinalName(content::DownloadInterruptReason, base::FilePath const&)
0x0084e71f [Google Chrome Framework -	 callback_internal.h:164] base::internal::Invoker<base::internal::BindState<base::RepeatingCallback<void (content::DownloadInterruptReason, base::FilePath const&)>, content::DownloadInterruptReason, base::FilePath>, void ()>::RunOnce(base::internal::BindStateBase*)
0x01daed8c [Google Chrome Framework -	 callback_forward.h:11] base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*)
0x01dd3984 [Google Chrome Framework -	 vector:639] base::MessageLoop::RunTask(base::PendingTask*)
0x01dd3e89 [Google Chrome Framework -	 message_loop.cc:455] base::MessageLoop::DoWork()
0x01dd5caa [Google Chrome Framework -	 message_pump_mac.mm:453] base::MessagePumpCFRunLoopBase::RunWork()
0x01dc776a [Google Chrome Framework +	 0x1dc776a] base::mac::CallWithEHFrame(void () block_pointer)
0x01dd55cf [Google Chrome Framework -	 message_pump_mac.mm:432] base::MessagePumpCFRunLoopBase::RunWorkSource(void*)
0x000aa7e1 [CoreFoundation +	 0xaa7e1] 
0x00089f0c [CoreFoundation +	 0x89f0c] 
0x0008942f [CoreFoundation +	 0x8942f] 
0x00088e28 [CoreFoundation +	 0x88e28] 

This is the #8 beta crash on Mac right now.
 
Oh, and a sample crash ID is 3ed251fdc2e2d5ed.
Project Member

Comment 2 by sheriffbot@chromium.org, Feb 23 2018

Labels: FoundIn-M-66 Fracas
Users experienced this crash on the following builds:

Mac Dev 66.0.3346.8 -  0.10 CPM, 2 reports, 2 clients (signature [Cocoa Zombie] -[DownloadItemController handleButtonClick:])

If this update was incorrect, please add "Fracas-Wrong" label to prevent future updates.

- Go/Fracas
Project Member

Comment 3 by sheriffbot@chromium.org, Feb 28 2018

Labels: FoundIn-M-65
Users experienced this crash on the following builds:

Mac Beta 65.0.3325.88 -  0.10 CPM, 5 reports, 5 clients (signature [Cocoa Zombie] -[DownloadItemController handleButtonClick:])

If this update was incorrect, please add "Fracas-Wrong" label to prevent future updates.

- Go/Fracas

Comment 4 by sdy@chromium.org, Mar 2 2018

Status: Started (was: Assigned)
It looks like this only affects macOS earlier than 10.13 :/.
Cc: pbomm...@chromium.org gov...@chromium.org ellyjo...@chromium.org
Labels: ReleaseBlock-Stable M-65
This is currently top#4 browser crash on latest Chrome stable i.e., 65.0.3325.146 with 73 crashes from 72 clients. 

please find the crash history here : https://goto.google.com/wgvnd

Comment 6 by gov...@chromium.org, Mar 10 2018

We're planning stable respin early next week. 
sdy@ and  ellyjones@, could you ptal comment #5. If it is real blocker, it needs to be fixed ASAP. Thank you.

Comment 7 by gov...@chromium.org, Mar 10 2018

Labels: M-66

Comment 8 by gov...@chromium.org, Mar 10 2018

NextAction: 2018-03-12
Project Member

Comment 9 by bugdroid1@chromium.org, Mar 10 2018

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

commit e73502d816bfe339d9ef76d365c143166f6d1a26
Author: Sidney San Martín <sdy@chromium.org>
Date: Sat Mar 10 06:26:44 2018

Clear the download item's target on dealloc.

This fixes a crash if the download item controller goes away while the
button is tracking the mouse and then sends its action on mouse up.

Bug:  815161 
Change-Id: I4ab377ca5fe190e3d95aa81b989c13c96b46f5b4
Reviewed-on: https://chromium-review.googlesource.com/957843
Commit-Queue: Avi Drissman <avi@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542357}
[modify] https://crrev.com/e73502d816bfe339d9ef76d365c143166f6d1a26/chrome/browser/ui/cocoa/download/download_item_controller.mm

Thank you sdy@ for quick fix.
 Please update the bug with canary result on Monday morning and request a merge to M66 and M65 if canary result looks good. Thank you.
The NextAction date has arrived: 2018-03-12

Comment 12 by ajha@chromium.org, Mar 12 2018

Fix is not available in the latest available canary(67.0.3368.0 cut @542340). We will have to wait for new canary to verify this fix or to check if this hasnot introduced any regression.

Comment 13 by sdy@chromium.org, Mar 12 2018

I just uploaded a CL that unit tests the fix (https://crrev.com/c/958209). Based on the test, I feel confident that the fix is correct.
Project Member

Comment 14 by bugdroid1@chromium.org, Mar 12 2018

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

commit 9cd1e6375511429c74b9e5f20aba2c69413084e1
Author: Sidney San Martín <sdy@chromium.org>
Date: Mon Mar 12 15:41:52 2018

Add a unit test for a download being removed from the shelf while its button is pressed.

Bug:  815161 
Change-Id: I1c55ee5d540faab02d8383776c041dad76317f1e
Reviewed-on: https://chromium-review.googlesource.com/958209
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Sidney San Martín <sdy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542491}
[modify] https://crrev.com/9cd1e6375511429c74b9e5f20aba2c69413084e1/chrome/browser/ui/cocoa/download/download_item_controller_unittest.mm

Comment 15 by sdy@chromium.org, Mar 12 2018

Labels: Merge-Request-66 Merge-Request-65
Status: Fixed (was: Started)
Project Member

Comment 16 by sheriffbot@chromium.org, Mar 12 2018

Labels: -Merge-Request-65 Merge-Review-65 Hotlist-Merge-Review
This bug requires manual review: Request affecting a post-stable build
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 17 by bugdroid1@chromium.org, Mar 12 2018

Labels: merge-merged-3368
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/095124b89302cccce7715fc44900dee80e76a269

commit 095124b89302cccce7715fc44900dee80e76a269
Author: Sidney San Martín <sdy@chromium.org>
Date: Mon Mar 12 18:04:33 2018

Clear the download item's target on dealloc.

This fixes a crash if the download item controller goes away while the
button is tracking the mouse and then sends its action on mouse up.

Bug:  815161 
Change-Id: I4ab377ca5fe190e3d95aa81b989c13c96b46f5b4
Reviewed-on: https://chromium-review.googlesource.com/957843
Commit-Queue: Avi Drissman <avi@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#542357}(cherry picked from commit e73502d816bfe339d9ef76d365c143166f6d1a26)
Reviewed-on: https://chromium-review.googlesource.com/958985
Reviewed-by: Sidney San Martín <sdy@chromium.org>
Cr-Commit-Position: refs/branch-heads/3368@{#1}
Cr-Branched-From: 0f36d3901569535a63173a1835f8dfbcf7b66d60-refs/heads/master@{#542340}
[modify] https://crrev.com/095124b89302cccce7715fc44900dee80e76a269/chrome/browser/ui/cocoa/download/download_item_controller.mm

Comment 18 by sdy@chromium.org, Mar 12 2018

I am comfortable merging this to M65 without canary coverage.
Labels: -Merge-Review-65 Merge-Approved-65
Approving merge to M65 branch 3325 based on comment #18 and per offline chat with sdy@.

Also triggered new canary of branch 3368 with this merge in. Pls verify this on new canary.
Project Member

Comment 20 by bugdroid1@chromium.org, Mar 12 2018

Labels: -merge-approved-65 merge-merged-3325
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0294d597a95077148c3e9a9aa51eec180bdd0e1d

commit 0294d597a95077148c3e9a9aa51eec180bdd0e1d
Author: Sidney San Martín <sdy@chromium.org>
Date: Mon Mar 12 18:13:29 2018

Clear the download item's target on dealloc.

This fixes a crash if the download item controller goes away while the
button is tracking the mouse and then sends its action on mouse up.

Bug:  815161 
Change-Id: I4ab377ca5fe190e3d95aa81b989c13c96b46f5b4
Reviewed-on: https://chromium-review.googlesource.com/957843
Commit-Queue: Avi Drissman <avi@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#542357}(cherry picked from commit e73502d816bfe339d9ef76d365c143166f6d1a26)
Reviewed-on: https://chromium-review.googlesource.com/958986
Reviewed-by: Sidney San Martín <sdy@chromium.org>
Cr-Commit-Position: refs/branch-heads/3325@{#693}
Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369}
[modify] https://crrev.com/0294d597a95077148c3e9a9aa51eec180bdd0e1d/chrome/browser/ui/cocoa/download/download_item_controller.mm

Pls verify this crash on canary version 67.0.3368.1 or higher.

Comment 22 by ajha@chromium.org, Mar 13 2018

Just to update, there have been no crashes reported on 67.0.3368.1 of Mac so far, for '[Cocoa Zombie] -[DownloadItemController handleButtonClick:]', also not seeing any new major regression on Mac canary version 67.0.3368.1 based on the available crash data.

Comment 23 by sdy@chromium.org, Mar 13 2018

Status: Verified (was: Fixed)
Great to hear.
Project Member

Comment 24 by sheriffbot@chromium.org, Mar 13 2018

Labels: -Merge-Request-66 Merge-Approved-66 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M66. Please go ahead and merge the CL to branch 3359 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 25 by bugdroid1@chromium.org, Mar 13 2018

Labels: -merge-approved-66 merge-merged-3359
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/43909c98644197bd9b35947d5c60f6699edb28c5

commit 43909c98644197bd9b35947d5c60f6699edb28c5
Author: Sidney San Martín <sdy@chromium.org>
Date: Tue Mar 13 17:43:32 2018

Clear the download item's target on dealloc.

This fixes a crash if the download item controller goes away while the
button is tracking the mouse and then sends its action on mouse up.

Bug:  815161 
Change-Id: I4ab377ca5fe190e3d95aa81b989c13c96b46f5b4
Reviewed-on: https://chromium-review.googlesource.com/957843
Commit-Queue: Avi Drissman <avi@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#542357}(cherry picked from commit e73502d816bfe339d9ef76d365c143166f6d1a26)
Reviewed-on: https://chromium-review.googlesource.com/960934
Reviewed-by: Sidney San Martín <sdy@chromium.org>
Cr-Commit-Position: refs/branch-heads/3359@{#208}
Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}
[modify] https://crrev.com/43909c98644197bd9b35947d5c60f6699edb28c5/chrome/browser/ui/cocoa/download/download_item_controller.mm

So far with 50% roll out of Stable 65.0.3325.162 on Mac there aren't any crashes. 
Thank you Sidney.

Comment 27 by sdy@chromium.org, Mar 16 2018

Great to hear. Thanks for checking.

Sign in to add a comment