1523181 - Crash in nsFontFaceLoader::Cancel

Status: RESOLVED FIXED

(Core :: Layout, defect, P2)

Description

[Gabriele Svelto [:gsvelto] (PTO)]

This bug is for crash report bp-089f7763-2ef4-439d-9e48-cf5800190126.

Top 10 frames of crashing thread:

0 XUL nsFontFaceLoader::Cancel layout/style/nsFontFaceLoader.cpp:321
1 XUL mozilla::dom::FontFaceSet::UpdateRules layout/style/FontFaceSet.cpp:743
2 XUL mozilla::dom::Document::FlushUserFontSet dom/base/Document.cpp:11629
3 XUL nsFontFaceUtils::MarkDirtyForFontChange dom/base/Document.cpp:11592
4 XUL nsFontFaceLoader::LoadTimerCallback layout/base/nsPresContext.cpp:1898
5 XUL nsTimerEvent::Run xpcom/threads/nsTimerImpl.cpp:559
6 XUL mozilla::SchedulerGroup::Runnable::Run xpcom/threads/SchedulerGroup.cpp:299
7 XUL nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1160
8 XUL NS_ProcessNextEvent xpcom/threads/nsThreadUtils.cpp:468
9 XUL mozilla::ipc::MessagePump::Run ipc/glue/MessagePump.cpp:88

Comment 2

[Xidorn Quan [:xidorn] UTC+11]

It seems this is a diagnostic assertion added for bug 1522417 which I don't have access to. Let's mark it block that bug to inform people there.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Yup, looking at it as we speak. Unfortunately the fix is not as trivial as I had hoped.

Comment 4

[Jonathan Watt [:jwatt]]

Please re-prioritize if appropriate, Emilio.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1523181 - Don't implicitly flush the user font set.

Flushing it at a bad time can cancel loads whose timer / completion
handler is in progress, which makes no sense.

Comment 6

[Julien Wajsberg [:julienw]]

I got the problem on https://eurostar.co.uk at load time, but it's not happening at 100%, here's my report: https://crash-stats.mozilla.org/report/index/f0de6f37-c968-462c-938f-d3d1d0190201

Comment 7

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/6882b9791212

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9039612 [details]
Bug 1523181 - Don't implicitly flush the user font set.

Beta/Release Uplift Approval Request

Feature/Bug causing the regression

Bug 1519918

User impact if declined

Crashes, with bad signature.

Is this code covered by automated tests?

No

Has the fix been verified in Nightly?

No

Needs manual test from QE?

No

If yes, steps to reproduce

List of other uplifts needed

None

Risk to taking this patch

Medium

Why is the change risky/not risky? (and alternatives if risky)

Relatively non-isolated change. Alternative would be to back out bug 1519918 from beta.

String changes made/needed

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9039612 [details]
Bug 1523181 - Don't implicitly flush the user font set.

[Triage Comment]
Fixes a new crash in 66, approved for 66.0b6.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/f8aadf41dcd0