1293560 - Crash in mozilla::gfx::ScaledFontWin::GetFontFileData

Status: RESOLVED WONTFIX

(Core :: Graphics: Text, defect)

Description

[Ting-Yu Chou [:ting] (away)]

This bug was filed from the Socorro interface and is 
report bp-60cdec43-04db-44fa-9b57-d58e82160808.
=============================================================

#12 of 0807 Nightly on Windows, 8 crashes from 2 installations.

Comment 1

[Ting-Yu Chou [:ting] (away)]

Any ideas?

Comment 2

[Jonathan Kew [:jfkthame]]

The crash report says

  GraphicsCriticalError	|[C0][GFX1 24]: Failed to get index for face name |true|. (t=75.2149) 

which seems a very strange "face name". Do we need a cast or conversion or something to make it report mLogFont.lfFaceName properly there? It kinda looks like gfxDevCrash may have interpreted it as a bool...

Comment 3

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: Fix the error message

Better fix up that error so we can get some useful data out of this.

Are font names usually okay to truncate to narrow chars? Most of our logging assumes 8-bit characters, sadly.

Comment 4

[Jonathan Kew [:jfkthame]]

Most font names are ASCII, so truncating would be harmless, but it's not absolutely guaranteed; in particular, there are some CJK fonts that have names in Chinese or Japanese characters. How about using NS_ConvertUTF16toUTF8 to safely get a utf-8 representation rather than simply truncating the 16-bit code units?

Comment 5

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: Convert font name to UTF8 in error message

Comment 6

[Jonathan Kew [:jfkthame]]

Comment on attachment 8780504 [details] [diff] [review]
Convert font name to UTF8 in error message

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

Yeah, that looks like it should work.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d829642ec65d
Fix debug output in ScaledFontWin::GetFontFileData - r=jfkthame

Comment 8

[Alice0775 White]

I can reproduce the crash when print out on Windows8.1 VM.
And I can still crash on latest m-i cset:ef40e047b7b4bda1eb2d52f277866afaac3d0e2d.

Reproducible : 100%

Steps to reproduce:
1. Open about:home
2. File > Print "Microsoft XPS Document Writer"
3. Enter filename and Click Save

Actual Results:
Tab crashes
bp-615d8647-3566-430f-96fe-908502160817 on Nightly cset:fe895421dfbe1f1f8f1fc6a39bb20774423a6d74
( bp-ca847f2e-2cda-4b45-822d-fd4e72160817 on m-i cset:ef40e047b7b4bda1eb2d52f277866afaac3d0e2d )

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

Crash volume for signature 'mozilla::gfx::ScaledFontWin::GetFontFileData':
 - nightly (version 51): 58 crashes from 2016-08-01.
 - aurora  (version 50): 191 crashes from 2016-08-01.
 - beta    (version 49): 0 crashes from 2016-08-02.
 - release (version 48): 0 crashes from 2016-07-25.
 - esr     (version 45): 0 crashes from 2016-05-02.

Crash volume on the last weeks (Week N is from 08-22 to 08-28):
            W. N-1  W. N-2  W. N-3
 - nightly       8      23      24
 - aurora       52      66      25
 - beta          0       0       0
 - release       0       0       0
 - esr           0       0       0

Affected platform: Windows

Crash rank on the last 7 days:
             Browser Content     Plugin
 - nightly           #103
 - aurora            #12
 - beta
 - release
 - esr

Comment 11

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: 1293560.patch

This appears to be coming from fonts in TrueType collections which have more than one full name per font, in most (all?) instances due to having different full names for different languages.

This patch adds a new method to various SFNT parsing classes, GetU16FullNames(), which takes a Vector<u16string> and fills it with *all* of the full names it finds. It also changes GetIndexForU16Name() to check against all of the full names for each font in a TTC.

Comment 12

[Jonathan Kew [:jfkthame]]

I may well be missing something here, but offhand I don't see why we should need to get and check against *all* the full names from each face. These are simply identifiers that we're using internally, aren't they? (Not names exposed to web content, where an author might have used any of the available languages and so we need to recognize them all.)

So provided we consistently return the same name from GetU16FullName() as we use for lookup in GetIndexForU16Name(), shouldn't it all Just Work? Where do we get lost?

Comment 13

[Sylvestre Ledru [:Sylvestre]]

Closing because no crashes reported for 12 weeks.