968626 - crash in ft_ansi_stream_close

Status: RESOLVED FIXED

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

Description

[Kevin Brosnan]

This bug was filed from the Socorro interface and is 
report bp-1082f548-f151-4f61-8170-509982140206.
=============================================================

Crashed loading http://octoverse.github.com/ Nexus 5

Did we take any harfbuz changes recently?

Comment 1

[Jonathan Kew [:jfkthame]]

This seems more likely related to the recent FreeType update, bug 966795.

Comment 2

[Jonathan Kew [:jfkthame]]

(In reply to Kevin Brosnan from comment #0)
> This bug was filed from the Socorro interface and is 
> report bp-1082f548-f151-4f61-8170-509982140206.
> =============================================================
> 
> Crashed loading http://octoverse.github.com/ Nexus 5

Was this a one-off crash, or can you reproduce it? I tried visiting http://octoverse.github.com/ with current Nightly on a Nexus 10, but have not been able to reproduce here.

Comment 3

[James Willcox (:snorp) ([e-mail]) (he/him)]

Nexus 4 reproduced too

Comment 5

[Jonathan Kew [:jfkthame]]

Unless we have an immediate fix here, I think we should back out the FreeType 2.5.2 update (bug 966795) for now.

Comment 6

[Jonathan Kew [:jfkthame]]

(In reply to Jonathan Kew (:jfkthame) from comment #2)
> I tried visiting
> http://octoverse.github.com/ with current Nightly on a Nexus 10, but have
> not been able to reproduce here.

After updating my tablet to Android 4.4.2, I can now reproduce this crash on both http://octoverse.github.com and http://snappy.paul.cx.

The crash is triggered by the presence of the new NotoColorEmoji.ttf font, which does not contain any TrueType outlines, but only color bitmap glyphs using Google's newly-proposed CBDT and CBLC tables. FreeType just added support for this format, but it's crashing as soon as we hit an Emoji character, font-matching finds this font as a fallback (instead of the old AndroidEmoji), and we try to shape text using it.

I'm not sure yet whether this is actually a bug in the FreeType code itself, or a no-longer-valid assumption in the cairo or thebes code that's interfacing with the FT font. But it means users on Android 4.4.2 will crash on virtually any page that uses Unicode emoji characters. (E.g. a simple Google search for "emoji" is likely to instantly crash due to characters in the search results it returns.)

(Note that we can't currently test this font via @font-face, as OTS will strip the bitmap tables from it. So testing requires it to be installed locally on the device.)

Comment 7

[Eugen Sawin [:esawin]]

Can reproduce on Nexus 7 Android 4.4.2.

The reported site contains colored PNG bitmaps (glyph_format==17), but with disabled support (requires libpng support) FreeType calls an uninitialized loader at http://dxr.mozilla.org/mozilla-central/source/modules/freetype2/src/sfnt/ttsbit.c#1033

Comment 8

[Eugen Sawin [:esawin]]

Attachment: unimplemented-png-loading.patch

Comment 9

[Eugen Sawin [:esawin]]

Attachment: unimplemented-png-loading.patch

Comment 10

[(not currently active) Ted Mielczarek]

Tested this patch on my Nexus 4, Android 4.4.2, loading http://www.rogerebert.com/reviews/the-lego-movie-2014

Nightly crashes on load of that page, my build with this patch doesn't.

Comment 11

[Jonathan Kew [:jfkthame]]

Comment on attachment 8372333 [details] [diff] [review]
unimplemented-png-loading.patch

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

This prevents the crash, but unfortunately it still leaves us with broken rendering - e.g. the sidebar on emojipedia.org is almost entirely blank, whereas a version of Firefox from before the FreeType update renders all the text fine (just with gray boxes for the emoji chars).

I suspect that when we try to render one of the bitmaps, and freetype now returns an error (rather than crashing us), this is putting the cairo context into an error state and then nothing further gets drawn to that context until we reinitialize stuff. Or something like that.

Comment 12

[(not currently active) Ted Mielczarek]

For reference, the emojipedia.org sidebar with this patch:
http://i.imgur.com/jHII3Zg.png

Can we back out the FreeType update to get Nightly back into a usable state until this gets sorted out?

Comment 13

[Eugen Sawin [:esawin]]

(In reply to Jonathan Kew (:jfkthame) from comment #11)
> Comment on attachment 8372333 [details] [diff] [review]
> unimplemented-png-loading.patch
> 
> Review of attachment 8372333 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This prevents the crash, but unfortunately it still leaves us with broken
> rendering - e.g. the sidebar on emojipedia.org is almost entirely blank,
> whereas a version of Firefox from before the FreeType update renders all the
> text fine (just with gray boxes for the emoji chars).
> 
> I suspect that when we try to render one of the bitmaps, and freetype now
> returns an error (rather than crashing us), this is putting the cairo
> context into an error state and then nothing further gets drawn to that
> context until we reinitialize stuff. Or something like that.

The FreeType 2.5.0.1 code path for glyph_format 17 looks identical to the patched version, the difference being: in 2.5.0.1 the function fails with error Invalid_Table, where in the patched version the function fails with error Unimplemented_Feature.

For reference, this is how the http://emojipedia.org sidebar looks on
Fennec 27.0: http://people.mozilla.org/~esawin/fennec-27.png
patched: http://people.mozilla.org/~esawin/fennec-patched.png

I assume there is an issue with the site itself?

Comment 14

[Jonathan Kew [:jfkthame]]

Hmm, no, I think there's another pre-existing issue with Firefox, probably from bug 867735 (FreeType 2.5.0.1). It turned out I had an older release on my Nexus, and with that, the emojipedia site renders better; but after updating to the current release version, I see the same failed rendering as your screenshot.

So OK, let's take this crash-fix immediately, but then we need a followup bug about making this font actually work properly - and not make the adjacent text fail too.

Comment 15

[Jonathan Kew [:jfkthame]]

Comment on attachment 8372333 [details] [diff] [review]
unimplemented-png-loading.patch

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

Switching this to r+, so that we can stop the crashes. Followup needed to make the font actually work!

Don't forget to add the patch file mentioned in README to the tree (did you miss the "hg add"?)

Comment 16

[Eugen Sawin [:esawin]]

Attachment: unimplemented-png-loading.patch

Added missing patch file.

Comment 17

[Eugen Sawin [:esawin]]

Attachment: unimplemented-png-loading.patch

Comment 18

[James Willcox (:snorp) ([e-mail]) (he/him)]

This is also crashing on reddit.

Comment 19

[Jonathan Kew [:jfkthame]]

(In reply to James Willcox (:snorp) ([e-mail]) from comment #18)
> This is also crashing on reddit.

It's liable to crash on any site that uses Unicode emoji characters, unless it explicitly provides and uses a different font for them. That'll include many sites where users post comments - all it takes is a cute smiley, and >boom<.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8eca810b7b8b

Comment 21

[Jonathan Kew [:jfkthame]]

Reported upstream at https://savannah.nongnu.org/bugs/index.php?41507.

Comment 22

[Sylvestre Ledru [:Sylvestre]]

Firefox 29 is impacted?

Comment 23

[Jonathan Kew [:jfkthame]]

(In reply to Sylvestre Ledru [:sylvestre] from comment #22)
> Firefox 29 is impacted?

No. This was caused by the update to FreeType 2.5.2 in bug 966795, which landed for Fx30. Fx29 still has FreeType 2.5.0.1, which did not have the bug.

(Firefox 29 - and earlier - does suffer from failure to actually render emoji characters on Android 4.4.2, as it doesn't support the color-bitmap font format, but it won't crash like this.)

Comment 24

[Jonathan Kew [:jfkthame]]

FTR, filed bug 969814 about actually supporting the color bitmap font.

Comment 25

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/8eca810b7b8b

Comment 26

[Benjamin Kerensa [:bkerensa]]

Kevin, Do you think this still needs tracking?

Comment 27

[Kevin Brosnan [Ex-Mozilla]]

No. The only crashes reported are from users that need to update.

Comment 28

[Michael Wu [:mwu]]

Requesting blocking for 11mb system image savings on tarako. See also bug 987357 comment 7 .

Comment 29

[Michael Wu [:mwu]]

https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/308ec4499d7f