1331683 - Crash in CoreText when opening dropbox PNG

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect, P3)

Description

[Josh Matthews [:jdm]]

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.

Comment 1

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Looks like only crash on MacOS 10.9

Comment 2

[Jonathan Kew [:jfkthame]]

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...

Comment 3

[Jonathan Kew [:jfkthame]]

[Tracking Requested - why for this release]:
Crashing on older MacOS releases.

Comment 4

[Marcia Knous [:marcia]]

Tracking 53+ so perhaps we can implement an OS version check.

Comment 5

[Jonathan Kew [:jfkthame]]

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!

Comment 6

[Josh Matthews [:jdm]]

I got this crash using that build: https://crash-stats.mozilla.com/report/index/f1dbc1e4-3746-4d2c-8542-0e33b2170120

Comment 7

[Jonathan Kew [:jfkthame]]

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.

Comment 8

[Jonathan Kew [:jfkthame]]

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?

Comment 9

[Josh Matthews [:jdm]]

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.

Comment 10

[Jonathan Kew [:jfkthame]]

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

Comment 11

[Josh Matthews [:jdm]]

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

Comment 12

[Josh Matthews [:jdm]]

The actual try build link is https://archive.mozilla.org/pub/firefox/try-builds/[e-mail]-1a0ef62781cd7546943b3ff26698cf068e4a63a8/try-macosx64/.

Comment 13

[yusuhail]

My crash report on OSX 10.9.5: https://crash-stats.mozilla.com/report/index/d442706c-f932-490b-823b-d9ed92170205

Comment 14

[Jonathan Kew [:jfkthame]]

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.

Comment 15

[Josh Matthews [:jdm]]

It works!

Comment 16

[Jonathan Kew [:jfkthame]]

Attachment: Don't attempt to use any Core Text and Core Graphics variation-font APIs on pre-Sierra systems

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.

Comment 18

[Jeff Muizelaar [:jrmuizel]]

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

Comment 20

[Jonathan Kew [:jfkthame]]

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

Comment 21

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/de99933ae324

Comment 22

[Jonathan Kew [:jfkthame]]

Attachment: imported patch -crashtest

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.

Comment 23

[Jonathan Kew [:jfkthame]]

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

Comment 24

[yusuhail]

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.

Comment 25

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/dd79181d183f821a7303b5665d75ebc9ecc60d8f
Bug 1331683 - Crashtest. r=jrmuizel

Comment 26

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/dd79181d183f

Comment 27

[Gerry Chang [:gchang]]

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+.

Comment 28

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/8f207d31a7ca