104.53 KB, text/plain
2.25 KB, patch
|Details | Diff | Splinter Review|
6.70 KB, text/plain; charset=UTF-8
8.10 KB, text/plain
708 bytes, text/plain
1.33 KB, patch
|Details | Diff | Splinter Review|
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
The assertion fails with aFontEntry->mFamilyName being "MetaWeb-Bold" and Name() returning "MetaWeb-bold".
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.
Assignee: nobody → cam
Status: NEW → ASSIGNED
Attachment #8454866 - Flags: review?(jdaggett)
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...
Component: GFX: Color Management → Graphics: Text
OS: Windows 7 → All
Hardware: x86 → All
Version: unspecified → Trunk
(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.
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!
Alessio, is there a URL that consistently triggers the assertion for you?
Duplicate of this bug: 1041330
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.
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.
(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.
(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.
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.
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 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.
Attachment #8460131 - Flags: feedback+
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.
Attachment #8460131 - Flags: review?(jdaggett) → review+
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
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.
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.
I think that's worth doing, and you should request approval.
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.
Attachment #8454866 - Flags: approval-mozilla-aurora?
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.
Thanks Bob, I didn't realise this caused wider problems.
Attachment #8454866 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
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
Attachment #8460131 - Flags: approval-mozilla-aurora+
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.
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 " ###!!! 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?
No I don't think it's related.
(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.
You need to log in before you can comment on or make changes to this bug.