652854 - The OS X file picker dialog has stopped working, crashes [@ libobjc.A.dylib@0x511c ] [@ libobjc.A.dylib@0x5120 ] [@ libobjc.A.dylib@0x512d ] [@ libobjc.A.dylib@0x4b86 ]

Status: VERIFIED FIXED

(Core :: Widget: Cocoa, defect)

Description

[(no longer active)]

STR:

1. Open Add-ons Manager.
2. From the tools icon, select the Install From File option.  Nothing happens, and nothing gets logged into the error console.
3. Open a new tab, or a new window, or the error console.  This crashes the browser.

https://crash-stats.mozilla.com/report/index/35178b09-66fb-48fb-9ee0-678dd2110426
https://crash-stats.mozilla.com/report/index/b0cfbcf8-3252-43cd-a622-e62892110426

Comment 1

[Chris Lawson (gone)]

Related to the recent landing of bug 646854, maybe?

Comment 2

[Dave Townsend [:mossop]]

(In reply to comment #1)
> Related to the recent landing of bug 646854, maybe?

Just tested with that backed out and the problem still exists

Comment 3

[Marcia Knous [:marcia]]

I get this error in the console when following the steps in Comment 0:

4/26/11 11:38:38 AM	firefox-bin[1733]	Mozilla has caught an Obj-C exception [NSInvalidArgumentException: *** -[NSURL initWithString:relativeToURL:]: nil string parameter]
4/26/11 11:38:38 AM	[0x0-0x38038].org.mozilla.nightly	2011-04-26 11:38:38.611 firefox-bin[1733:903] Mozilla has caught an Obj-C exception [NSInvalidArgumentException: *** -[NSURL initWithString:relativeToURL:]: nil string parameter]

Comment 4

[Marcia Knous [:marcia]]

Possible regression range based on crash-stats data:

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e00435bb54b5&tochange=503696b78379

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Tested the regression with local builds and those match the range as given by Marcia:

PASS: 20110422030558 http://hg.mozilla.org/mozilla-central/rev/e00435bb54b5
FAIL: 20110423030545 http://hg.mozilla.org/mozilla-central/rev/503696b78379

I highly suspect bug 646854 which should have been caused this regression.

author	Mounir Lamouri <mounir.lamouri(at)gmail.com>
	Thu Apr 21 19:39:12 2011 +0200 (at Thu Apr 21 19:39:12 2011 +0200)
changeset 68423	5c0d5ac6118b

Line of code:
http://hg.mozilla.org/mozilla-central/annotate/c5e8cc100248/widget/src/cocoa/nsFilePicker.mm#l312

First 6 frames of the stack:
0 	libobjc.A.dylib 	libobjc.A.dylib@0x511c 	
1 	XUL 	-[NSPopUpButtonObserver menuChangedItem:] 	widget/src/cocoa/nsFilePicker.mm:312
2 	Foundation 	_nsnote_callback 	
3 	CoreFoundation 	CFArrayAppendValue 	
4 	CoreFoundation 	__CFXNotificationPost 	
5 	CoreFoundation 	___CFBasicHashFindBucket1

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

(In reply to comment #2)
> > Related to the recent landing of bug 646854, maybe?
> 
> Just tested with that backed out and the problem still exists

Oh, I missed that comment. I will do a hg bisect instead.

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Tested both changesets and with aaa99fe3ee29 the file picker is working. So it has to be bug 646854 which caused this misbehavior.

Comment 8

[Steven Michaud [:smichaud] (Retired)]

Crashes [@ libobjc.A.dylib@0x4b86 ] appear to happen only on OS X 10.7 (Lion) -- available as a developer preview from Apple.

Comment 9

[Marcia Knous [:marcia]]

Those 10.7 crashes are likely mine as I was trying to repro it on 10.7.

(In reply to comment #8)
> Crashes [@ libobjc.A.dylib@0x4b86 ] appear to happen only on OS X 10.7 (Lion)
> -- available as a developer preview from Apple.

Comment 10

[Steven Michaud [:smichaud] (Retired)]

:-)

Comment 11

[Steven Michaud [:smichaud] (Retired)]

Attachment: Fix

This bug has a very simple fix.

The problem is that PanelDefaultDirectory() can return NULL if you
haven't opened or saved a file since you created/cleaned your profile.
When it does this (and sets theDir to NULL), [NSURL initWithString:]
ends up being called with a NULL parameter, and throws an exception.
This causes execution to jump to a Mozilla exception handler, and then
to the end of nsFilePicker::GetLocalFiles().  Which means that 1) the
file picker fails to open and 2) [NSNotificationCenter removeObserver]
never gets called on 'observer' (the NSPopUpButtonObserver object
created just before the file picker is supposed to open).

I've found that I could do File : Open File once without crashing --
the file picker simply fails to open.  But the second time I do File :
Open I crash -- presumably because the first 'observer' object has
somehow gotten deleted.

It might be better to ensure that PanelDefaultDirectory() never
returns NULL.  But that would probably be a lot of work.  My current
patch has the advantage of being very simple.

A tryserver build should be available by tomorrow morning.

Comment 12

[Mounir Lamouri (:mounir)]

Sorry if I haven't been responsive, I'm currently abroad for a community event...
And thank you Steven for writing the patch :)

Comment 13

[Steven Michaud [:smichaud] (Retired)]

> [fixed in cedar]

I was going to land this on the trunk today ... but what should I do now?

When's the next merge from cedar?

Comment 14

[Steven Michaud [:smichaud] (Retired)]

It appears that cedar won't be merged with mozilla-central "for a
while".  So I'm going to land this patch on mozilla-central.

> And thank you Steven for writing the patch :)

You're quite welcome.

Comment 15

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 528990 [details] [diff] [review]
Fix

Landed on mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/3185c4159bec

Comment 16

[Steven Michaud [:smichaud] (Retired)]

Bug 646584's target milestone is mozilla6, and it's patch has only
landed on the trunk.  So it looks like this patch won't be needed on
any of the branches.

Comment 17

[Mounir Lamouri (:mounir)]

(In reply to comment #14)
> It appears that cedar won't be merged with mozilla-central "for a
> while".  So I'm going to land this patch on mozilla-central.

Who said that? I've just merged cedar and mozilla-central...
I'm trying to regularly push checkin-needed patches to cedar and merge it with mozilla-central. Though, it was hard this week...

Comment 18

[Steven Michaud [:smichaud] (Retired)]

>> It appears that cedar won't be merged with mozilla-central "for a
>> while".  So I'm going to land this patch on mozilla-central.
>
> Who said that?

http://tinderbox.mozilla.org/showbuilds.cgi?tree=Cedar says it :-)

"note that cedar won't merge back with mozilla-central for 'a while'"

Also, I looked back a couple of days in the mozilla-central pushlog at
http://hg.mozilla.org/mozilla-central/pushloghtml, and didn't see any
merges from cedar.

I see you've done one just now, though.

Comment 19

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Verified fixed with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:6.0a1) Gecko/20110501 Firefox/6.0a1