Closed
Bug 1257124
Opened 8 years ago
Closed 8 years ago
crash in gfxDWriteFontFileLoader::CreateCustomFontFile
Categories
(Core :: General, defect)
Tracking
()
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox45 | --- | unaffected |
firefox46 | --- | fixed |
firefox47 | --- | fixed |
firefox48 | --- | fixed |
People
(Reporter: philipp, Assigned: bobowen)
References
Details
(Keywords: crash, regression)
Crash Data
Attachments
(1 file)
1.34 KB,
patch
|
bas.schouten
:
review+
ritu
:
approval-mozilla-aurora+
ritu
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
This bug was filed from the Socorro interface and is report bp-0715bf31-5d41-4701-8f13-68e272160315. ============================================================= Crashing Thread (0) Frame Module Signature Source 0 xul.dll gfxDWriteFontFileLoader::CreateCustomFontFile(FallibleTArray<unsigned char>&, IDWriteFontFile**, IDWriteFontFileStream**) gfx/thebes/gfxDWriteCommon.cpp 1 xul.dll gfxDWriteFontList::MakePlatformFont(nsAString_internal const&, unsigned short, short, unsigned char, unsigned char const*, unsigned int) gfx/thebes/gfxDWriteFontList.cpp 2 xul.dll gfxUserFontEntry::LoadPlatformFont(unsigned char const*, unsigned int&) gfx/thebes/gfxUserFontSet.cpp this crash signature is new in firefox 46 builds and seems to have been introduced by the work done in bug 1156742.
Assignee | ||
Comment 1•8 years ago
|
||
I suspect this is a morphing of the previous crash: https://crash-stats.mozilla.com/signature/?signature=gfxDWriteFontList%3A%3AMakePlatformFont This has disappeared in 46. So, my guess is that gfxWindowsPlatform::GetPlatform()->GetDWriteFactory() is returning null and the same thing was causing the crash before here: http://hg.mozilla.org/releases/mozilla-release/annotate/b6609650a911/gfx/thebes/gfxDWriteFontList.cpp#l822 Bas - does that make sense? Do we just null check and return E_FAIL or something?
Flags: needinfo?(bas)
Comment 2•8 years ago
|
||
That certainly could happen in some cases when getting an invalid webfont, yes.
Flags: needinfo?(bas)
Assignee | ||
Comment 3•8 years ago
|
||
(In reply to Bas Schouten (:bas.schouten) from comment #2) > That certainly could happen in some cases when getting an invalid webfont, > yes. Sorry Bas, I don't quite understand. How would an invalid webfont, cause gfxWindowsPlatform::GetPlatform()->GetDWriteFactory() to return null? Also looks like there are other crashes caused by this return null, like in bug 1244512. Either way here's the null check.
Attachment #8731788 -
Flags: review?(bas)
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → bobowen.code
Status: NEW → ASSIGNED
Updated•8 years ago
|
Attachment #8731788 -
Flags: review?(bas) → review+
Comment 4•8 years ago
|
||
(In reply to Bob Owen (:bobowen) from comment #3) > Created attachment 8731788 [details] [diff] [review] > Add null check for IDWriteFactory in > gfxDWriteFontFileLoader::CreateCustomFontFile > > (In reply to Bas Schouten (:bas.schouten) from comment #2) > > That certainly could happen in some cases when getting an invalid webfont, > > yes. > > Sorry Bas, I don't quite understand. > How would an invalid webfont, cause > gfxWindowsPlatform::GetPlatform()->GetDWriteFactory() to return null? > > Also looks like there are other crashes caused by this return null, like in > bug 1244512. > > Either way here's the null check. Sorry, I misunderstood your comment. The crash logs indicate a device reset happened on those machines. But I don't really see how that could lead to a nullptr DWriteFactory... That's rather strange really, but let's take this patch non-the-less, it's almost certain to somehow be the cause. Could you add a gfxCriticalError to that line?
Comment 6•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/85e76ff91cb9
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Assignee | ||
Comment 7•8 years ago
|
||
Comment on attachment 8731788 [details] [diff] [review] Add null check for IDWriteFactory in gfxDWriteFontFileLoader::CreateCustomFontFile Note: the patch that landed is slightly different. Approval Request Comment [Feature/regressing bug #]: This was probably an existing crash that was moved by bug 1156742. [User impact if declined]: Crash. [Describe test coverage new/current, TreeHerder]: This would be covered by existing Web Font tests. [Risks and why]: Low - simple null check. [String/UUID change made/needed]: None
Attachment #8731788 -
Flags: approval-mozilla-beta?
Attachment #8731788 -
Flags: approval-mozilla-aurora?
Comment on attachment 8731788 [details] [diff] [review] Add null check for IDWriteFactory in gfxDWriteFontFileLoader::CreateCustomFontFile Even though I did not see this crash on 47, the change seems simple enough to uplift to both Aurora47, Beta46.
Attachment #8731788 -
Flags: approval-mozilla-beta?
Attachment #8731788 -
Flags: approval-mozilla-beta+
Attachment #8731788 -
Flags: approval-mozilla-aurora?
Attachment #8731788 -
Flags: approval-mozilla-aurora+
Comment 9•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/dadf228376c8
Comment 10•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/bdfc2038590c
Comment 11•7 years ago
|
||
Moving from Core::Untriaged to Core::General https://bugzilla.mozilla.org/show_bug.cgi?id=1407598
Component: Untriaged → General
You need to log in
before you can comment on or make changes to this bug.
Description
•