766326 - ###!!! ASSERTION: invalid default font returned by GetDefaultFont: 'defaultFont'

Status: RESOLVED FIXED

Description

[Brad Lassey [:blassey] (use needinfo?)]

I'm seeing this assertion very often on Android. If it really is an error condition, we should fix it. If it is not, we should remove the assertion.

Comment 1

[Joe Drew (not getting mail)]

Is this a bad assertion?

Comment 2

[Brad Lassey [:blassey] (use needinfo?)]

https://mxr.mozilla.org/mozilla-central/source/gfx/thebes/gfxFont.cpp#2953

Comment 3

[Jonathan Kew (:jfkthame)]

It's not a bad assertion, in that we really really expect that GetDefaultFont() will return something usable, to avoid the risk of ending up with a fontGroup that has no actual fonts at all, leading to all kinds of sadness when we ask for metrics, etc.

Hmm... on Android, gfxFT2FontList::GetDefaultFont() is hardcoded to look for Droid Sans. That's probably not a good thing on ICS, where I believe it has been replaced by Roboto. I'll post a patch...

Comment 4

[Jonathan Kew (:jfkthame)]

Created attachment 635027 [details] [diff] [review]
patch, try Roboto as well as Droid Sans for Android default font

I think this should help, if I'm understanding the cause correctly.

Comment 5

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 635027 [details] [diff] [review]
patch, try Roboto as well as Droid Sans for Android default font

Review of attachment 635027 [details] [diff] [review]:
-----------------------------------------------------------------

nit, reverse the order of Droid Sans and Roboto. If both happen to be on a device, I think we'd rather have Roboto

Comment 6

[Jonathan Kew (:jfkthame)]

Pushed to inbound, rearranged to try Roboto first, then Droid Sans.
https://hg.mozilla.org/integration/mozilla-inbound/rev/e24a42876bc5

I'm assuming this will stop these assertions on ICS devices, but don't have one on hand for testing; please re-open if it doesn't solve the problem.

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/e24a42876bc5

Comment 8

[John Daggett (:jtd)]

Brad, does this fix the assertions you were seeing?

Comment 9

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 635027 [details] [diff] [review]
patch, try Roboto as well as Droid Sans for Android default font

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: 
Testing completed (on m-c, etc.): 
Risk to taking this patch (and alternatives if risky): 
String or UUID changes made by this patch:

Comment 10

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 635027 [details] [diff] [review]
patch, try Roboto as well as Droid Sans for Android default font

so tiny and low risk it's not worth an approval request form completion, eh? :)

Comment 11

[Jonathan Kew (:jfkthame)]

Pushed to Aurora (15) and Beta (14):
https://hg.mozilla.org/releases/mozilla-aurora/rev/abb6f9e021dd
https://hg.mozilla.org/releases/mozilla-beta/rev/94b04b595448