This bug was filed from the Socorro interface and is report bp-14b434dd-e621-4593-a8bd-486d70180228. ============================================================= Top 10 frames of crashing thread: 0 firefox mozalloc_abort memory/mozalloc/mozalloc_abort.cpp:33 1 firefox abort memory/mozalloc/mozalloc_abort.cpp:80 2 libxul.so std::panicking::rust_panic src/libpanic_abort/lib.rs:59 3 libxul.so std::panicking::rust_panic_with_hook src/libstd/panicking.rs:593 4 libxul.so std::panicking::begin_panic<&str> src/libstd/panicking.rs:538 5 libxul.so style::gecko_bindings::structs::root::mozilla::GeckoFont::clone_font_family toolkit/library/x86_64-unknown-linux-gnu/release/build/style-625572085c0ab7a1/out/gecko_properties.rs:10000 6 libxul.so style::properties::animated_properties::AnimationValue::from_computed_values toolkit/library/x86_64-unknown-linux-gnu/release/build/style-625572085c0ab7a1/out/properties.rs:82861 7 libxul.so style::gecko::wrapper::{{impl}}::needs_transitions_update::{{closure}} servo/components/style/gecko/wrapper.rs:800 8 libxul.so style::gecko::wrapper::{{impl}}::needs_transitions_update src/libcore/iter/iterator.rs:1576 9 libxul.so style::traversal::compute_style<style::gecko::wrapper::GeckoElement> servo/components/style/matching.rs:357 ============================================================= There are 2 crashes (from 2 installations) in nightly 60 with buildid 20180228100110. In analyzing the backtrace, the regression may have been introduced by patch [1]. [1] https://hg.mozilla.org/mozilla-central/rev?node=fd7b1a798fa0

"Default generic must be serif or sans-serif" Two Linux users, hmmm... Maybe they changed the default font settings to specify something other than serif / sans-serif as default font?

Yeah, I have no idea why that assertion is supposed to hold. Manish, care explaining?

It's what the Gecko code does. But it should also be allowed to be _none, which is causing the crash here. There's a SetDefaultFontType function in Gecko with the same assertion.

(In reply to Manish Goregaokar [:manishearth] from comment #3) > It's what the Gecko code does. But it should also be allowed to be _none, > which is causing the crash here. Should we open a separate bug to do that or should we just do that here?

Just do it here. The PR will be servo side anyway.

We have 900 users and just upgraded to Firefox 60 from 59.0.2 and as best as I can tell, this is the right bug affecting us greatly. 59.0.2 was very stable, few calls about browser issues. 60.0 is crashing quite often and we're getting a lot of calls about pages that will not work and crash the current tab. Restoring the tab immediately crashes again. I have uploaded the crash report here: https://crash-stats.mozilla.com/report/index/277fe92d-561a-41ee-9198-929630180514 I can get one particular page to crash every time. 1) I go to https://www.volgistics.com/ 2) I click on the [ Login ] button in the upper right corner 3) The login page renders OK, then the command line spewage below appears and then the tab crashes. We're using Ubuntu 16.04.1 LTS Someone mentioned the default font being changed, our default font globally is "Liberation Sans". I'm going to change that for my account and try the page again. drichard@desktopnxcloud:/u/firefox_2018_05_10/firefox$ ./firefox thread '<unnamed>' panicked at 'Default generic must be serif or sans-serif', /builds/worker/workspace/build/src/obj-firefox/toolkit/library/x86_64-unknown-linux-gnu/release/build/style-0285f03e3935f7e2/out/gecko_properties.rs:9999:22 note: Run with `RUST_BACKTRACE=1` for a backtrace. Redirecting call to abort() to mozalloc_abort [Parent 59861, Gecko_IOThread] WARNING: pipe error (107): Connection reset by peer: file /builds/worker/workspace/build/src/ipc/chromium/src/chrome/common/ipc_channel_posix.cc, line 353 [Parent 59861, Gecko_IOThread] WARNING: pipe error (119): Connection reset by peer: file /builds/worker/workspace/build/src/ipc/chromium/src/chrome/common/ipc_channel_posix.cc, line 353 [Parent 59861, Gecko_IOThread] WARNING: pipe error (116): Connection reset by peer: file /builds/worker/workspace/build/src/ipc/chromium/src/chrome/common/ipc_channel_posix.cc, line 353 ###!!! [Parent][MessageChannel] Error: (msgtype=0x480017,name=PHttpChannel::Msg_DeleteSelf) Channel error: cannot send/recv ###!!! [Parent][MessageChannel] Error: (msgtype=0x480017,name=PHttpChannel::Msg_DeleteSelf) Channel error: cannot send/recv ###!!! [Parent][MessageChannel] Error: (msgtype=0x15007F,name=PBrowser::Msg_Destroy) Channel error: cannot send/recv

I changed the font from "Liberation Sans" to "san-serif" and the crash no longer happens. Good guess in comment #1 I could change this font for the short term globally, but I think this should still be fixed. We have many web pages that would have to be tested with a new font, and the Liberation font was picked after some testing of options years ago.

Sounds like that value can be affected by user setting. We should just remove that check. That also raises the concern that we probably should avoid abusing panic-ish stuff in Rust code. That's effectively upgrading lots of assertion from MOZ_ASSERT to MOZ_RELEASE_ASSERT while they don't really matter that much.

I'll prepare a patch tomorrow for this case if no one beats me tonight :)

I can write it :)

Alright, thanks.

Comment on attachment 8975471 [details] Bug 1442195: Don't crash when finding an unexpected default font type. https://reviewboard.mozilla.org/r/243756/#review249878

Pushed by ecoal95@gmail.com: https://hg.mozilla.org/integration/autoland/rev/339ffc4c6651 Don't crash when finding an unexpected default font type. r=xidorn

Please request Beta approval on this when you're comfortable doing so.

Comment on attachment 8975471 [details] Bug 1442195: Don't crash when finding an unexpected default font type. Approval Request Comment [Feature/Bug causing the regression]: stylo [User impact if declined]: crashes with some pref combinations. [Is this code covered by automated tests?]: no [Has the fix been verified in Nightly?]: yes, just did [Needs manual test from QE? If yes, steps to reproduce]: no [List of other uplifts needed for the feature/fix]: none [Is the change risky?]: no [Why is the change risky/not risky?]: simple crash fix that avoids panicking the thread. [String changes made/needed]: none

Comment on attachment 8975471 [details] Bug 1442195: Don't crash when finding an unexpected default font type. Simple fix for a low-volume crash. Approved for 61.0b6.

Do we want to fix this in esr60 given the low risk?

Comment on attachment 8975471 [details] Bug 1442195: Don't crash when finding an unexpected default font type. [Approval Request Comment] If this is not a sec:{high,crit} bug, please state case for ESR consideration: trivial crash-fix. User impact if declined: see comment 17 Fix Landed on Version: 61 Risk to taking this patch (and alternatives if risky): not really risky. String or UUID changes made by this patch: none See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

(In reply to Julien Cristau [:jcristau] from comment #20) > Do we want to fix this in esr60 given the low risk? Yeah, sounds fine :)

Comment on attachment 8975471 [details] Bug 1442195: Don't crash when finding an unexpected default font type. crash fix, approved for 60.1esr