Closed Bug 1260770 Opened 4 years ago Closed 4 years ago

crash in gfxDWriteFontList::MakePlatformFont

Categories

(Core :: Graphics, defect, critical)

x86
Windows NT
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla46
Tracking Status
firefox45 --- affected
firefox46 --- fixed

People

(Reporter: calixte, Assigned: bas.schouten)

References

Details

(Keywords: crash, Whiteboard: [gfx-noted])

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is 
report bp-c299aa20-f63d-4aa6-be69-89fca2160330.
=============================================================

This signature spiked during the two last days: it rose by 96% (from 100 to 196 between 2016-03-27 and 2016-03-29).
Seems this "stopped" with 45, right?
Seems bug 1245765 could have improved things, but that's only 47, doesn't explain why I'm not seeing problems with 46.
Whiteboard: [gfx-noted]
Flags: needinfo?(jfkthame)
Relevant conversation happening in bug 1250669. I'm worried this goes up in crash count once we sort out bug 1189715 (for example); Bas, is there something we can patch here?  Bug 1250669 comment 29 sounds "promising".
Assignee: nobody → bas
Flags: needinfo?(bas)
It looks to me like the (one-line) patch from bug 1208833 should be backed out: it's wrong to kill the DWrite factory once we've started to use DWrite fonts. (And the crash in comment 0 looks like exactly the sort of thing we'd hit after doing that.)

But I'll leave it to Bas to say whether there's anything else we should do besides just backing that out.
Flags: needinfo?(jfkthame)
Bug 1149318 should be considered if it applies.
This is the safest solution I can think of, it doesn't bring back a now untested configuration on startup. But it does mean that on a device reset we won't remove DirectWrite. (i.e. said now untested configuration could only occur in a situation where we already are in trouble now anyway)
Flags: needinfo?(bas)
Attachment #8740546 - Flags: review?(milan)
Comment on attachment 8740546 [details] [diff] [review]
Only clear the DWrite factory if we didn't have one to begin with

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

I like it.  Do we want to yell in either of these cases, with an error or a note?
Attachment #8740546 - Flags: review?(milan) → review+
(In reply to Milan Sreckovic [:milan] from comment #7)
> Comment on attachment 8740546 [details] [diff] [review]
> Only clear the DWrite factory if we didn't have one to begin with
> 
> Review of attachment 8740546 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I like it.  Do we want to yell in either of these cases, with an error or a
> note?

Not really I think, we can already tell that we had a device reset and lost D2D from other data in our crash report I'd say.
Comment on attachment 8740546 [details] [diff] [review]
Only clear the DWrite factory if we didn't have one to begin with

Approval Request Comment
[Feature/regressing bug #]: Bug 1208833
[User impact if declined]: Some crashes when D2D is switched off after device reset
[Describe test coverage new/current, TreeHerder]: Beta only bug
[Risks and why]: Extremely low, only clearing DWrite factory -less- often. At the very worst would cause some slightly lesser quality fonts.
[String/UUID change made/needed]: None
Attachment #8740546 - Flags: approval-mozilla-beta?
Comment on attachment 8740546 [details] [diff] [review]
Only clear the DWrite factory if we didn't have one to begin with

Fix/backout for a worrisome crash on beta only.
Attachment #8740546 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
https://hg.mozilla.org/releases/mozilla-beta/rev/40538c2ee09d
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in before you can comment on or make changes to this bug.