Closed Bug 1293560 Opened 4 years ago Closed Last year

Crash in mozilla::gfx::ScaledFontWin::GetFontFileData

Categories

(Core :: Graphics: Text, defect, critical)

Unspecified
Windows 7
defect
Not set
critical

Tracking

()

RESOLVED WONTFIX
Tracking Status
firefox50 --- affected
firefox51 --- affected

People

(Reporter: ting, Assigned: eflores)

Details

(Keywords: crash)

Crash Data

Attachments

(2 files, 1 obsolete file)

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.
Any ideas?
Flags: needinfo?(edwin)
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...
Flags: needinfo?(edwin)
Flags: needinfo?(edwin)
Attached patch Fix the error message (obsolete) — Splinter Review
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.
Assignee: nobody → edwin
Status: NEW → ASSIGNED
Flags: needinfo?(edwin)
Attachment #8780091 - Flags: review?(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?
Attachment #8780091 - Attachment is obsolete: true
Attachment #8780091 - Flags: review?(jfkthame)
Attachment #8780504 - Flags: review?(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.
Attachment #8780504 - Flags: review?(jfkthame) → review+
Pushed by eflores@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d829642ec65d
Fix debug output in ScaledFontWin::GetFontFileData - r=jfkthame
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 )
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
Attached patch 1293560.patchSplinter Review
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.
Attachment #8793228 - Flags: review?(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?
Closing because no crashes reported for 12 weeks.
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.