Last Comment Bug 659182 - Crashing on startup [@ DisableFontActivation() ]
: Crashing on startup [@ DisableFontActivation() ]
Status: RESOLVED FIXED
jesse nominated without comment
: crash, regression
Product: Core
Classification: Components
Component: Layout: Text (show other bugs)
: Trunk
: x86_64 Mac OS X
: -- blocker (vote)
: ---
Assigned To: John Daggett (:jtd)
:
:
Mentors:
Depends on:
Blocks: 567552
  Show dependency treegraph
 
Reported: 2011-05-23 16:04 PDT by Sam Liu
Modified: 2011-06-13 10:01 PDT (History)
7 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-


Attachments
Stacktrace (7.52 KB, text/plain)
2011-05-23 16:04 PDT, Sam Liu
no flags Details
patch, null check bundle ids before releasing (609 bytes, patch)
2011-05-23 18:18 PDT, John Daggett (:jtd)
roc: review+
Details | Diff | Splinter Review

Description Sam Liu 2011-05-23 16:04:07 PDT
Created attachment 534604 [details]
Stacktrace
Comment 1 Sam Liu 2011-05-23 16:04:36 PDT
Note: Local builds crash but nightly build does not crash.
Comment 2 John Daggett (:jtd) 2011-05-23 17:45:24 PDT
Hmmm, when you crash what are the values of mainBundleID and mainBundle?  It looks like you're stopping on a breakpoint, what's the stack when it actually crashes?
Comment 3 John Daggett (:jtd) 2011-05-23 18:10:16 PDT
Hmmm, unlike other API's CFRelease(NULL) is not allowed.

http://developer.apple.com/library/mac/#documentation/CoreFoundation/Reference/CFTypeRef/Reference/reference.html#//apple_ref/c/func/CFRelease
Comment 4 John Daggett (:jtd) 2011-05-23 18:18:12 PDT
Created attachment 534647 [details] [diff] [review]
patch, null check bundle ids before releasing
Comment 5 Robert O'Callahan (:roc) (email my personal email if necessary) 2011-05-23 18:20:38 PDT
Comment on attachment 534647 [details] [diff] [review]
patch, null check bundle ids before releasing

Review of attachment 534647 [details] [diff] [review]:
-----------------------------------------------------------------
Comment 6 Christian :Biesinger (don't email me, ping me on IRC) 2011-05-23 23:37:19 PDT
http://hg.mozilla.org/mozilla-central/rev/58083c2147e2
Comment 7 Jonathan Kew (:jfkthame) 2011-05-24 01:44:26 PDT
Is there a need to get the bundle ID at all? I notice that the doc for CTFontManagerSetAutoActivationSetting says "If NULL, the current application bundle is used"; wouldn't that serve the purpose?
Comment 8 John Daggett (:jtd) 2011-05-24 04:59:52 PDT
(In reply to comment #7)
> Is there a need to get the bundle ID at all? I notice that the doc for
> CTFontManagerSetAutoActivationSetting says "If NULL, the current application
> bundle is used"; wouldn't that serve the purpose?

Nope.  I coded the initial patch on bug 567552 that way and it turns out passing in NULL affects the *global* setting, not a per-process setting.  If you experiment with this you'll see that it clears the "Automatic Font Activation" pref in FontBook.  Seems to be an Apple docs bug, unless I missing something.
Comment 9 Jonathan Kew (:jfkthame) 2011-05-24 05:03:18 PDT
(In reply to comment #8)
> (In reply to comment #7)
> > Is there a need to get the bundle ID at all? I notice that the doc for
> > CTFontManagerSetAutoActivationSetting says "If NULL, the current application
> > bundle is used"; wouldn't that serve the purpose?
> 
> Nope.  I coded the initial patch on bug 567552 that way and it turns out
> passing in NULL affects the *global* setting, not a per-process setting.  If
> you experiment with this you'll see that it clears the "Automatic Font
> Activation" pref in FontBook.  Seems to be an Apple docs bug, unless I
> missing something.

Fair enough; I didn't try it myself, just wondered based on what the doc said. Reported the bug to Apple?
Comment 10 John Daggett (:jtd) 2011-05-24 05:55:00 PDT
(In reply to comment #9)
> Fair enough; I didn't try it myself, just wondered based on what the doc
> said. Reported the bug to Apple?

No, but you're right, I should.
Comment 11 Sam Liu 2011-05-24 10:09:59 PDT
Fixed as of rev 58083c2147e2
Comment 12 Asa Dotzler [:asa] 2011-05-31 14:40:16 PDT
please nominate the patch with a risk analysis if you think it's safe for taking into 6 Aurora.
Comment 13 Jonathan Kew (:jfkthame) 2011-05-31 14:57:14 PDT
It looks like this is on Aurora already. It's listed in:
https://hg.mozilla.org/releases/mozilla-aurora/filelog/d8b797975700/gfx/thebes/gfxPlatformMac.cpp

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