12.36 KB, patch
|Details | Diff | Splinter Review|
976 bytes, patch
|Details | Diff | Splinter Review|
https://crash-stats.mozilla.com/report/index/b2121d23-af15-4760-90a4-9b4d42170117 https://crash-stats.mozilla.com/report/index/349e5bb3-a2e8-4d5f-a8d3-c14ed2170117 The dropbox URL crashes my nightly Firefox on OS X repeatedly every time I open it.
Looks like only crash on MacOS 10.9
Priority: -- → P3
This looks linked to the support for variation fonts (bug 1321031), which touches the font backend code even though it's preffed off at the CSS level. We should probably put the back-end support here behind an OS version check so that it's completely bypassed on pre-Sierra systems. I have heard that Core Text is buggy in this area...
[Tracking Requested - why for this release]: Crashing on older MacOS releases.
Josh, there's a build in progress at https://treeherder.mozilla.org/#/jobs?repo=try&revision=dea91cfb9653eb3dd75649de64de3e3284d22a78 ... when it's ready, could you please give it a try and confirm whether it avoids the crash for you? Thanks!
I got this crash using that build: https://crash-stats.mozilla.com/report/index/f1dbc1e4-3746-4d2c-8542-0e33b2170120
Huh, interesting. Maybe it's not directly due to the variation-font APIs, then, or maybe I missed a place where I should have added a version check. I'll look again.
Could you please use mozregression to pinpoint the Nightly build that first exhibits a crash on this URL? Note that the exact crash signature will probably have changed with the landing of bug 1321031 (landed on mozilla-central 2017-01-07), assuming it was happening then, but did the URL load OK before that and start crashing afterwards, or did it start some other time?
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=44ab9ab869d54feae16cd084cafe36b5595833c4&tochange=273ce98bb9e831b70d9ce2b34590859ea2ce80b8 Before bug 1321031 it did not crash, and after bug 1321031 it did.
Josh, I've started another try build with an attempt to work around this; when it's ready, could you please give it a spin and see whether the crash still happens? Thanks! https://treeherder.mozilla.org/#/jobs?repo=try&revision=1a0ef62781cd7546943b3ff26698cf068e4a63a8
I crash when loading the URL, but with a JS GC-related stack this time: https://crash-stats.mozilla.com/report/index/c1d4007e-720c-4461-aec0-47b142170201
My crash report on OSX 10.9.5: https://crash-stats.mozilla.com/report/index/d442706c-f932-490b-823b-d9ed92170205
Sigh...trivial typo (lack of a negation) in the patch last time. OK, here's another try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=fbeaa432db814e0e43e8578614d78efe5cfab5b3. I confirmed that this prevents the crash for me on a system running 10.10, at least; I don't have access to a 10.9 machine here at the moment. Josh, when the try build is ready, could you please check it on your 10.9 system? Thanks.
This is ugly, but appears to be necessary; even though with the CSS feature preffed off, we never set any variation values on the fonts we're using, merely calling a function like CGFontCopyVariations on older MacOSX versions can result in a crash deep inside CoreText. :( So this patch checks that we're running on Sierra (or later), and if not, short-circuits those calls altogether. Exposing a version of nsCocoaFeatures::OnSierraOrLater() as a global C function seems particularly hacky here, but we need something like that for the Cairo callsite; I didn't want to entirely re-implement the OS version check there.
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Comment on attachment 8835363 [details] [diff] [review] Don't attempt to use any Core Text and Core Graphics variation-font APIs on pre-Sierra systems Review of attachment 8835363 [details] [diff] [review]: ----------------------------------------------------------------- Looks ok to me. Getting additional review from Lee because of Skia
Attachment #8835363 - Flags: review?(lsalzman) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/de99933ae32454ca3081aa60a25739f79a9e0cca Bug 1331683 - Don't attempt to use any Core Text and Core Graphics variation-font APIs on pre-Sierra systems. r=jrmuizel,lsalzman
Here's a minimal crashtest for this bug: with a build prior to the patch here, it consistently dies with an exception deep inside CoreText on 10.9/10.10, and with a patched build it loads fine.
Attachment #8836317 - Flags: review?(jmuizelaar)
Comment on attachment 8835363 [details] [diff] [review] Don't attempt to use any Core Text and Core Graphics variation-font APIs on pre-Sierra systems Approval Request Comment [Feature/Bug causing the regression]: bug 1321031 [User impact if declined]: crashes on certain pages for users on older MacOSX versions [Is this code covered by automated tests?]: crashtest just posted, will uplift together with the patch [Has the fix been verified in Nightly?]: yes [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?]: just reverts to pre-existing code paths on older OS versions, to avoid touching buggy APIs [String changes made/needed]: none
Attachment #8835363 - Flags: approval-mozilla-aurora?
Attachment #8836317 - Flags: review?(jmuizelaar) → review+
The issue is resolved for Nightly 54.0a1 (2017-02-11) on OSX 10.9.5. The tab/window does not crash on www.dropbox.com anymore.
Crash Signature: [@ libsystem_kernel.dylib@0x15866 ] → [@ libsystem_kernel.dylib@0x15866 ] [@ CrashReporter::TerminateHandler ]
Comment on attachment 8835363 [details] [diff] [review] Don't attempt to use any Core Text and Core Graphics variation-font APIs on pre-Sierra systems Fix a crash. Aurora53+.
Attachment #8835363 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.