1037442 - Assertion failure: aFontEntry->mFamilyName.Equals(Name())

Status: VERIFIED FIXED

(Core :: Graphics: Text, defect)

Description

[Carsten Book [:Tomcat]]

Attachment: bughunter stack

found via bughunter on https://shop.rossmann-fotowelt.de/shop/viewCart.do

STR: Load https://shop.rossmann-fotowelt.de/shop/viewCart.do on a windows 7 trunk debug build as example

-> Crash/Assertion failure after some seconds during page load

Assertion failure: aFontEntry->mFamilyName.Equals(Name()), at /work/mozilla/builds/nightly/mozilla/gfx/thebes/gfxUserFontSet.h:109
gfxUserFontSet::AddFontFace [/work/mozilla/builds/nightly/mozilla/gfx/thebes/gfxUserFontSet.h:109]
nsUserFontSet::InsertRule [/work/mozilla/builds/nightly/mozilla/firefox-debug/layout/style/../../dist/include/nsAutoPtr.h:926]
nsUserFontSet::UpdateRules [/work/mozilla/builds/nightly/mozilla/layout/style/nsFontFaceLoader.cpp:455]
nsPresContext::FlushUserFontSet [/work/mozilla/builds/nightly/mozilla/layout/base/nsPresContext.cpp:2162]
PresShell::FlushPendingNotifications [/work/mozilla/builds/nightly/mozilla/layout/base/nsPresShell.cpp:4179]
nsRefreshDriver::Tick [/work/mozilla/builds/nightly/mozilla/layout/base/nsRefreshDriver.cpp:1193]
mozilla::RefreshDriverTimer::Tick [/work/mozilla/builds/nightly/mozilla/firefox-debug/layout/base/../../dist/include/nsTArray.h:323]
mozilla::RefreshDriverTimer::TimerTick [/work/mozilla/builds/nightly/mozilla/layout/base/nsRefreshDriver.cpp:189]
nsTimerImpl::Fire [/work/mozilla/builds/nightly/mozilla/xpcom/threads/nsTimerImpl.cpp:619]
nsTimerEvent::Run [/work/mozilla/builds/nightly/mozilla/firefox-debug/xpcom/threads/../../dist/include/nsAutoPtr.h:836]
nsThread::ProcessNextEvent [/work/mozilla/builds/nightly/mozilla/xpcom/threads/nsThread.cpp:748]
NS_ProcessNextEvent [/work/mozilla/builds/nightly/mozilla/xpcom/glue/nsThreadUtils.cpp:256]
mozilla::ipc::MessagePump::Run [/work/mozilla/builds/nightly/mozilla/ipc/glue/MessagePump.cpp:99]
MessageLoop::RunInternal [/work/mozilla/builds/nightly/mozilla/ipc/chromium/src/base/message_loop.cc:230]
MessageLoop::Run [/work/mozilla/builds/nightly/mozilla/ipc/chromium/src/base/message_loop.cc:196]
nsBaseAppShell::Run [/work/mozilla/builds/nightly/mozilla/widget/xpwidgets/nsBaseAppShell.cpp:166]
nsAppStartup::Run [/work/mozilla/builds/nightly/mozilla/toolkit/components/startup/nsAppStartup.cpp:279]
XREMain::XRE_mainRun [/work/mozilla/builds/nightly/mozilla/toolkit/xre/nsAppRunner.cpp:4013]
XREMain::XRE_main [/work/mozilla/builds/nightly/mozilla/toolkit/xre/nsAppRunner.cpp:4084]
XRE_main [/work/mozilla/builds/nightly/mozilla/toolkit/xre/nsAppRunner.cpp:4298]
do_main [/work/mozilla/builds/nightly/mozilla/browser/app/nsBrowserApp.cpp:282]
main [/work/mozilla/builds/nightly/mozilla/browser/app/nsBrowserApp.cpp:643]
libc.so.6 + 0x1ed1d
firefox + 0x4f89

Comment 1

[Cameron McCormack (:heycam)]

The assertion fails with aFontEntry->mFamilyName being "MetaWeb-Bold" and Name() returning "MetaWeb-bold".

Comment 2

[Cameron McCormack (:heycam)]

So I think it might make sense for the assertion to use EqualsIgnoreCase, since GetFamily/LookupFamily force the family name to lowercase before looking it up in mFontFamilies.

Comment 3

[Cameron McCormack (:heycam)]

Attachment: patch

Comment 4

[Cameron McCormack (:heycam)]

EqualsIgnoreCase seems to only work with with a char* argument, hence the 400% explosion in lines of code compared to what I expected to write...

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: gdb session from a different site

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to Cameron McCormack (:heycam) from comment #4)
> EqualsIgnoreCase seems to only work with with a char* argument, hence the
> 400% explosion in lines of code compared to what I expected to write...

This is because ASCII-case-insensitivity lives in xpcom but Unicode-case-insensitivity lives in intl/unicharutil/.

We could probably clean up the API, but the one line way to fix it is to add nsCaseInsensitiveStringComparator() as the second argument to Equals.

Comment 7

[Alessio Placitelli [:Dexter]]

Attachment: windows7_msvisualstudio_stacktrace.txt

I'm experimenting a similar assertion failure on my machine as well (Windows 7 x64) with my Firefox Nightly. My assertion is just at a different line:

"Assertion failure: aFontEntry->mFamilyName.Equals(Name()), at c:\mozilla-source\gfx\thebes\gfxFont.h:824"

By examining the content of both aFontEntry->mFamilyName and Name(), I've noticed that they contain different strings: "Gill Sans Ultra Bold" and "Gill Sans MT" in my case.

This happens all the times since I've updated to the latest revision from m-c. So please let me know if I can be of any help!

Comment 8

[Cameron McCormack (:heycam)]

Alessio, is there a URL that consistently triggers the assertion for you?

Comment 10

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

For me, this is a startup crash that makes it impossible to do anything with a debug build on my system. That is, this assertion happens at startup even when running "$OBJDIR/dist/bin/firefox -p -no-remote" before the profile manager even is displayed. My symptoms are identical including the same "Gills Sans" fonts trigger the assertion. I am using Windows 8.1.

Comment 11

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Attachment: Windows 8.1 log showing assertion failure at startup

Comment 12

[Jonathan Kew [:jfkthame]]

I suspect this is related to the hack in gfxDWriteFontList that "re-parents" faces named Gill Sans into the Gill Sans MT family; see

http://mxr.mozilla.org/mozilla-central/source/gfx/thebes/gfxDWriteFontList.cpp#1000

(For background, see bug 551313.)

We probably need to update family names within those font entries when moving them, or something like that.

Comment 13

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

(In reply to Carsten Book [:Tomcat] from comment #7)
> https://hg.mozilla.org/mozilla-central/rev/9ec5db6a1684
> https://hg.mozilla.org/mozilla-central/rev/df4473d66d38

I verified that backing out these two changesets from bug 1031187 locally fixes this bug for me.

Comment 14

[Alessio Placitelli [:Dexter]]

(In reply to Cameron McCormack (:heycam) from comment #8)
> Alessio, is there a URL that consistently triggers the assertion for you?

As for Brian, this happens to me as a startup crash of my debug build. I will check later this evening if baking out the two changesets fixes the problem locally for me as well.

Comment 15

[Jonathan Kew [:jfkthame]]

Attachment: patch 2 - fix the family name of Gill Sans faces when merging into Gill Sans MT under DWrite.

This is untested, as I'm not at my Windows box just now, but I think this is what'll be needed to fix the Gill Sans-related startup assertion on Windows.

Comment 16

[Alessio Placitelli [:Dexter]]

Thanks Jonathan, this patch fixes the problem for me (Windows 7 x64).

(In reply to Jonathan Kew (:jfkthame) from comment #15)
> Created attachment 8460131 [details] [diff] [review]
> patch 2 - fix the family name of Gill Sans faces when merging into Gill Sans
> MT under DWrite.
> 
> This is untested, as I'm not at my Windows box just now, but I think this is
> what'll be needed to fix the Gill Sans-related startup assertion on Windows.

Comment 17

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Comment on attachment 8460131 [details] [diff] [review]
patch 2 - fix the family name of Gill Sans faces when merging into Gill Sans MT under DWrite.

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

This patch (even without patch 1) fixes the startup assertion for me.

Comment 18

[John Daggett (:jtd)]

Comment on attachment 8460131 [details] [diff] [review]
patch 2 - fix the family name of Gill Sans faces when merging into Gill Sans MT under DWrite.

Looks good.

Comment 19

[Cameron McCormack (:heycam)]

https://tbpl.mozilla.org/?tree=Try&rev=58f3e768bcc2

Comment 20

[Cameron McCormack (:heycam)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/80fbf64cf433
https://hg.mozilla.org/integration/mozilla-inbound/rev/466e22cb3c7b

Comment 21

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/80fbf64cf433
https://hg.mozilla.org/mozilla-central/rev/466e22cb3c7b

Comment 23

[Bob Clary [:bc] (inactive)]

Verified fixed on Nightly/34 by retesting the urls.

[Tracking Requested - why for this release]: This regression appeared first in 33 and has been fixed on 34. Rather than let this regression ride the trains, we should stamp it out now before our users see it. For some developers it appears as a startup crash on debug builds.

Comment 24

[Cameron McCormack (:heycam)]

Users shouldn't see this, unless they're running a debug build.  Happy to land it on 33 though for developers testing things on that branch.

Comment 25

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I think that's worth doing, and you should request approval.

Comment 26

[Cameron McCormack (:heycam)]

Comment on attachment 8454866 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]: bug 1031187
[User impact if declined]: No end user release build impact, but can result in a debug build crashing (due to an assertion failure) on startup if certain fonts are installed, or Web pages with certain uses of @font-face / font-family are visited.
[Describe test coverage new/current, TBPL]: Been on m-c for a day, manual testing shows the patch resolves the problem.
[Risks and why]: Very low, as the patches do not cause any change in how fonts are processed, just the conditions under which the assertion fires.
[String/UUID change made/needed]: N/A

Consider this an approval request for the Part 2 patch as well.

Comment 27

[Bob Clary [:bc] (inactive)]

If I could chime in on the approval request as well. This assertion was the top crasher for the bughunter crash automation system. Eliminating it from Aurora would improve bughunter's ability to test for other more important crashes rather than being stuck repeatedly reproducing this issue for the next 6 weeks.

Comment 28

[Cameron McCormack (:heycam)]

Thanks Bob, I didn't realise this caused wider problems.

Comment 29

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8460131 [details] [diff] [review]
patch 2 - fix the family name of Gill Sans faces when merging into Gill Sans MT under DWrite.

Requested by comment #26

Comment 30

[Cameron McCormack (:heycam)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/8e8546266e99
https://hg.mozilla.org/releases/mozilla-aurora/rev/50ef305a562b

Comment 31

[Kamil Jozwiak [:kjozwiak]]

This has also fixed the startup assertion that I was getting with m-c. I compiled and ran m-c using changeset de8c0f0e74a2 without any problems.

Comment 32

[Ada [:adalucinet]]

Reproduced with Nightly 2014-07-11 debug build on Windows 7 64-bit: Nightly hangs and in the Console I get: "Assertion failure: aFontEntry->mFamilyName.Equals(Name()), at c:\builds\moz2_sla
ve\m-cen-w32-d-000000000000000000\build\gfx\thebes\gfxUserFontSet.h:109"
Verified as fixed on Firefox 33 beta 8 (Build ID: 20140929143202) and latest Aurora (Build ID: 20140929145202) debug builds, on Windows 7 x64, Mac OS X 10.9.5 and Ubuntu 14.04 32-bit - this assertion is no longer present.
Although, when closing beta debug build, I get a small hang and "[6500] ###!!! ASSERTION: Uh, IsInModalState() called w/o a reachable top window?
: 'Error', file c:/builds/moz2_slave/m-beta-w32-d-00000000000000000/build/dom/ba
se/nsGlobalWindow.cpp, line 8775". Could this be related with the previous assertion?

Comment 33

[Cameron McCormack (:heycam)]

No I don't think it's related.

Comment 34

[Ada [:adalucinet]]

(In reply to Cameron McCormack (:heycam) (away October 6 - November 5) from comment #33)
> No I don't think it's related.

Thanks. Marking accordingly.