Crash in [@ gfxFontGroup::GetUnderlineOffset]
Categories
(Core :: Layout: Text and Fonts, defect)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr68 | --- | unaffected |
firefox-esr78 | --- | unaffected |
firefox77 | --- | unaffected |
firefox78 | --- | unaffected |
firefox79 | - | disabled |
firefox80 | --- | disabled |
firefox81 | --- | fixed |
People
(Reporter: over68, Assigned: jfkthame)
References
(Blocks 1 open bug)
Details
Crash Data
Attachments
(2 files)
Steps to reproduce:
- Set
gfx.e10s.font-list.shared
totrue
. - Restart Firefox.
- Download Font Loader.
- Download and extracts the archive.
- Open this page in seven tabs.
- Open the Font Loader, Click on the Add Fonts button, Select the extracted font files then click Open.
- Click on the Load button.
- Switch between tabs.
See https://youtu.be/RTY2JclTEyA
Actual results:
The tab crashed when clicking on the Load button then switch between tabs.
Crash report: bp-f51f216b-f619-464e-995b-4ddc20200623
Top 10 frames of crashing thread:
0 xul.dll gfxFontGroup::GetUnderlineOffset gfx/thebes/gfxTextRun.cpp:2819
1 xul.dll nsFontMetrics::MaxDescent gfx/src/nsFontMetrics.cpp:238
2 xul.dll nsTextFrame::ReflowText layout/generic/nsTextFrame.cpp:9384
3 xul.dll nsLineLayout::ReflowFrame layout/generic/nsLineLayout.cpp:881
4 xul.dll nsInlineFrame::Reflow layout/generic/nsInlineFrame.cpp:363
5 xul.dll nsLineLayout::ReflowFrame layout/generic/nsLineLayout.cpp:878
6 xul.dll nsBlockFrame::DoReflowInlineFrames layout/generic/nsBlockFrame.cpp:4254
7 xul.dll nsBlockFrame::ReflowDirtyLines layout/generic/nsBlockFrame.cpp:2658
8 xul.dll nsBlockFrame::Reflow layout/generic/nsBlockFrame.cpp:1375
9 xul.dll nsContainerFrame::ReflowChild layout/generic/nsContainerFrame.cpp:1074
Updated•4 years ago
|
Comment 1•4 years ago
|
||
This is restricted to Nightly-only at the moment.
Comment 2•4 years ago
|
||
gfx.e10s.font-list.shared
is not enabled by default yet. S3 because the work around exists.
Assignee | ||
Comment 3•4 years ago
|
||
I can reproduce this pretty consistently following the reporter's steps.
What's happening is that although we flush the various font caches when the font-list is rebuilt (because existing font entries are no longer safe to use, as they have pointers into the shared memory that has been discarded), in this case we end up using a font reference that was found via an existing textrun attached to the frame tree, not from the (flushed) font caches.
To avoid the risk of this, we should forcibly reconstruct frames when the font-list is reinitialized; it's not sufficient or safe to just restyle everything.
blinky, there's a tryserver build at https://treeherder.mozilla.org/#/jobs?repo=try&revision=cbe1bb1b5d3c589c29da64d51708335fbfed63d2 with a proof-of-concept patch to address this; if you could confirm whether this prevents the crash, that would be really helpful -- thanks!
Assignee | ||
Comment 4•4 years ago
|
||
Incidentally, this means the existing behavior (without the shared font list enabled) is already buggy, inasmuch as the reflow that happens after the font-list has been changed can continue using metrics from obsolete font entries that shouldn't be available to layout any more. But the results (e.g. relying on an underline-position metric from a font that's supposed to have been disabled) would be scarcely noticeable in normal cases, whereas with the shared font-list the error suddenly becomes a crash-bug.
Assignee | ||
Comment 5•4 years ago
|
||
The other thing I noticed while auditing code to understand what was happening here is that we have some places where we use the pointer returned by FamilyFace::FontEntry() without checking for null. This is normally fine but it is possible, at least in theory, for this to fail -- that didn't turn out to be the root cause of the crashes here, but nevertheless we should add the missing null-checks.
Assignee | ||
Comment 6•4 years ago
|
||
Updated•4 years ago
|
Assignee | ||
Comment 7•4 years ago
|
||
As a bonus, this also fixes a pre-existing bug that we never cleared the
mPrefChangePendingNeedsReflow flag after using it, so future pref changes
that shouldn't need a reflow would still trigger one.
Depends on D84518
I can not reproduce the crash with the build in comment 3. Thanks.
Comment 10•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/13159d59b7f6
https://hg.mozilla.org/mozilla-central/rev/51da3f1e63df
Comment 11•4 years ago
|
||
Since the status are different for nightly and release, what's the status for beta?
For more information, please visit auto_nag documentation.
Assignee | ||
Updated•4 years ago
|
Description
•