659182 - Crashing on startup [@ DisableFontActivation() ]

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect)

Description

[Sam Liu]

Attachment: Stacktrace

Comment 1

[Sam Liu]

Note: Local builds crash but nightly build does not crash.

Comment 2

[John Daggett (:jtd)]

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)]

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)]

Attachment: patch, null check bundle ids before releasing

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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)]

http://hg.mozilla.org/mozilla-central/rev/58083c2147e2

Comment 7

[Jonathan Kew [:jfkthame]]

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)]

(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]]

(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)]

(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]

Fixed as of rev 58083c2147e2

Comment 12

[Asa Dotzler [:asa]]

please nominate the patch with a risk analysis if you think it's safe for taking into 6 Aurora.

Comment 13

[Jonathan Kew [:jfkthame]]

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