Last Comment Bug 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 ]
: The OS X file picker dialog has stopped working, crashes [@ libobjc.A.dylib@0...
Status: VERIFIED FIXED
: crash, regression, reproducible
Product: Core
Classification: Components
Component: Widget: Cocoa (show other bugs)
: Trunk
: All Mac OS X
: -- critical (vote)
: mozilla6
Assigned To: Steven Michaud [:smichaud] (Retired)
:
Mentors:
Depends on:
Blocks: 646854
  Show dependency treegraph
 
Reported: 2011-04-26 10:29 PDT by :Ehsan Akhgari (out sick)
Modified: 2011-06-09 14:58 PDT (History)
9 users (show)
hskupin: in‑litmus-
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Fix (1.14 KB, patch)
2011-04-28 16:33 PDT, Steven Michaud [:smichaud] (Retired)
jaas: review+
Details | Diff | Review

Description :Ehsan Akhgari (out sick) 2011-04-26 10:29:00 PDT
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) 2011-04-26 10:34:09 PDT
Related to the recent landing of bug 646854, maybe?
Comment 2 Dave Townsend [:mossop] 2011-04-26 11:14:07 PDT
(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 - use ni] 2011-04-26 11:41:16 PDT
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 - use ni] 2011-04-26 11:51:46 PDT
Possible regression range based on crash-stats data:

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e00435bb54b5&tochange=503696b78379
Comment 5 Henrik Skupin (:whimboo) 2011-04-27 08:38:02 PDT
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) 2011-04-27 08:39:11 PDT
(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) 2011-04-27 10:32:25 PDT
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) 2011-04-28 13:23:45 PDT
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 - use ni] 2011-04-28 13:37:38 PDT
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) 2011-04-28 13:52:37 PDT
:-)
Comment 11 Steven Michaud [:smichaud] (Retired) 2011-04-28 16:33:19 PDT
Created attachment 528990 [details] [diff] [review]
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) 2011-04-29 04:57:22 PDT
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) 2011-04-29 08:28:06 PDT
> [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) 2011-04-29 08:47:53 PDT
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) 2011-04-29 09:01:24 PDT
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) 2011-04-29 09:10:04 PDT
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) 2011-04-29 09:29:29 PDT
(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) 2011-04-29 09:43:50 PDT
>> 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) 2011-05-02 01:19:32 PDT
Verified fixed with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:6.0a1) Gecko/20110501 Firefox/6.0a1

Note You need to log in before you can comment on or make changes to this bug.