Closed Bug 1385462 Opened 7 years ago Closed 7 years ago

Crash in gfxTextRun::AccumulateMetricsForRun when no vector font is installed

Categories

(Core :: Graphics: Text, defect)

56 Branch
x86_64
Linux
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla57
Tracking Status
firefox-esr52 --- wontfix
firefox55 --- wontfix
firefox56 --- verified
firefox57 --- verified

People

(Reporter: bugs, Assigned: jfkthame)

References

Details

(Keywords: crash)

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is 
report bp-4f718bb7-eafd-403b-a025-7b95d0170728.
=============================================================

When no TTF or OTF fonts are installed, going to any Twitter page reproduces that crash.

Other web pages and the chrome are using the default bitmap fonts provided with Xorg, as well as the coloured emoji font for numbers.

Other web pages don’t seem to trigger that crash.
The crash here occurs because the gfxTextRun measurement/drawing code assumes that when GlyphRunIterator::NextRun returns true, we have a usable glyph run (with a non-null font), but NextRun fails to ensure this is the case. Fixing this prevents the crashes here. I'll file a separate bug for the fact that we end up rendering hexboxes for text that is too far from any of the available bitmap font sizes, in the case where no scalable fonts are installed, which is what leads to twitter.com (and other pages with large headings) hitting this issue.
Attachment #8894452 - Flags: review?(lsalzman)
Assignee: nobody → jfkthame
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
For additional context, this occurs when the SizeDistance function in gfxFcPlatformFontList.cpp[1] ends up rejecting all the available faces for all the available font families because they're all bitmap sizes, and the requested size is too far away.

(If we always pass 'true' for the aForceScalable flag here, the problem doesn't arise, but of course this would change the font matching behavior when both scalable and non-scalable fonts are installed, so needs further consideration.)

[1] https://dxr.mozilla.org/mozilla-central/rev/52285ea5e54c73d3ed824544cef2ee3f195f05e6/gfx/thebes/gfxFcPlatformFontList.cpp#1071
Blocks: 1388059
Attachment #8894452 - Flags: review?(lsalzman) → review+
Pushed by jkew@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/551898614c2c
Ensure GlyphRunIterator::NextRun returns false immediately if the textrun's only glyphrun does not have a valid font. r=lsalzman
https://hg.mozilla.org/integration/mozilla-inbound/rev/551898614c2c8e44fd249cf7f924c9e43d3eff01
Bug 1385462 - Ensure GlyphRunIterator::NextRun returns false immediately if the textrun's only glyphrun does not have a valid font. r=lsalzman
https://hg.mozilla.org/mozilla-central/rev/551898614c2c
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Comment on attachment 8894452 [details] [diff] [review]
Ensure GlyphRunIterator::NextRun returns false immediately if the textrun's only glyphrun does not have a valid font

Approval Request Comment
[Feature/Bug causing the regression]: 1350783
[User impact if declined]: potential crash for users with only bitmap fonts
[Is this code covered by automated tests?]: normal use of this code is exercised throughout tests (part of the main text-rendering code), but the specific edge case addressed here does not arise (dependent on the system's font configuration)
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: on Linux, uninstall *all* scalable fonts (truetype, opentype, type1), leaving only X11 bitmap fonts enabled; then visit sites that have text in large font sizes
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: just adds a check so the iterator will return false immediately in the degenerate case where no valid glyph run is present
[String changes made/needed]: none
Attachment #8894452 - Flags: approval-mozilla-beta?
Blocks: 1389553
Comment on attachment 8894452 [details] [diff] [review]
Ensure GlyphRunIterator::NextRun returns false immediately if the textrun's only glyphrun does not have a valid font

Seems like an edge case but OK, let's uplift to fix the crash.
Attachment #8894452 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify+
Given that ESR52/TB52 are also hitting these crashes and the patch is a one-liner, I think we should nominate it for approval there as well.
Flags: needinfo?(jfkthame)
Keywords: crash
Comment on attachment 8894452 [details] [diff] [review]
Ensure GlyphRunIterator::NextRun returns false immediately if the textrun's only glyphrun does not have a valid font

Trivial patch to fix crash for Linux users with no vector fonts installed; see comment 6, comment 9.
Flags: needinfo?(jfkthame)
Attachment #8894452 - Flags: approval-mozilla-esr52?
Doesn't apply to esr52:

grafting 415147:551898614c2c "Bug 1385462 - Ensure GlyphRunIterator::NextRun returns false immediately if the textrun's only glyphrun does not have a valid font. r=lsalzman"
merging gfx/thebes/gfxTextRun.cpp
 warning: conflicts while merging gfx/thebes/gfxTextRun.cpp! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use 'hg resolve' and 'hg graft --continue')
Flags: needinfo?(jfkthame)
Looks like this would need a rebase around bug 1360620.  Probably not worth it considering the low rate.
Flags: needinfo?(jfkthame)
Attachment #8894452 - Flags: approval-mozilla-esr52? → approval-mozilla-esr52-
Depends on: 1390635
Reproduced the initial crash using an old Nightly build from 2017-07-28: 
- bp-84510421-213e-4cac-ba9e-8202a0170911
- bp-a11c8c9b-4d6a-44bf-8f47-4ddeb0170911

Verified that using Firefox 56 beta 10 the crash does not occur anymore on Ubuntu 16.04 64bit.
Flags: qe-verify+
Mozilla/5.0 (X11; Linux x86_64; rv:58.0) Gecko/20100101 Firefox/58.0
Build ID: 20171003100226

Reproduced the initial crash using Nightly build from 2017-07-28:
- https://crash-stats.mozilla.com/report/index/fb7958c2-6d21-4ff3-bb7a-8be500171003

Verified as fixed using Nightly 57.0a1 (2017-09-04) and latest Nightly 58.0a1 (20171003100226) on Ubuntu 16.04 x64.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.