1744069 - Crash in [@ mozilla::nsDisplayListBuilder::BuildCompositorHitTestInfoIfNeeded]

Status: VERIFIED FIXED

(Core :: Web Painting, defect)

Description

[Sean Feng [:sefeng]]

Maybe Fission related. (DOMFissionEnabled=1)

Crash report: https://crash-stats.mozilla.org/report/index/7f797ae1-7d94-46d5-984e-087cf0211127

MOZ_CRASH Reason: MOZ_DIAGNOSTIC_ASSERT(false) (Duplicate display item!)

Top 10 frames of crashing thread:

0 XUL mozilla::nsDisplayListBuilder::BuildCompositorHitTestInfoIfNeeded layout/painting/nsDisplayList.cpp:1980
1 XUL mozilla::SVGOuterSVGFrame::BuildDisplayList layout/svg/SVGOuterSVGFrame.cpp:775
2 XUL nsIFrame::BuildDisplayListForChild layout/generic/nsIFrame.cpp:4286
3 XUL nsBlockFrame::BuildDisplayList layout/generic/nsBlockFrame.cpp:7096
4 XUL nsIFrame::BuildDisplayListForChild layout/generic/nsIFrame.cpp:4286
5 XUL nsGridContainerFrame::BuildDisplayList layout/generic/nsGridContainerFrame.cpp:9383
6 XUL nsIFrame::BuildDisplayListForChild layout/generic/nsIFrame.cpp:4268
7 XUL nsBlockFrame::BuildDisplayList layout/generic/nsBlockFrame.cpp:7096
8 XUL nsIFrame::BuildDisplayListForChild layout/generic/nsIFrame.cpp:4059
9 XUL nsBlockFrame::BuildDisplayList layout/generic/nsBlockFrame.cpp:7096

Comment 1

[Timothy Nikkel (:tnikkel)]

Potentially it might be possible to reproduce this for someone who has access to the urls of the crashes.

Comment 2

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

16 crashes from 11 installations with Firefox 99.0b1 shortly after it got released today. Is this related to the changes in bug 1714584?

Comment 3

[Miko Mynttinen]

I have been trying to reproduce this bug with the crash report URLs without success. If someone can reproduce this reliably, it would be extremely helpful.

Comment 4

[Ryan VanderMeulen [:RyanVM]]

Are we seeing anything like this in the fuzzers?

Comment 5

[Miko Mynttinen]

I've managed to reproduce crash with this site: https://thejigsawpuzzles.com/Puzzle-of-the-Day/Las-Vegas_2-jigsaw-puzzle

Unfortunately, the page is a bit complicated and also reliably crashes rr when I try to record the crash.

Comment 6

[Jason Kratzer [:jkratzer]]

:RyanVM, unfortunately not. The nearest stack I see was filed in bug 1648628.

Comment 7

[Donal Meehan [:dmeehan]]

:miko how feasible would it be to blackout bug 1714584
Given the crash rate volume and investigation problems

Comment 8

[Arthur K. (he/him)]

(In reply to Donal Meehan [:dmeehan] from comment #7)

:miko how feasible would it be to blackout bug 1714584
Given the crash rate volume and investigation problems

The link to the bug is pointing to an invalid location(=17145840 instead of =1714584).

Comment 9

[Donal Meehan [:dmeehan]]

(In reply to Arthur K. [He/Him] from comment #8)

(In reply to Donal Meehan [:dmeehan] from comment #7)

:miko how feasible would it be to blackout bug 1714584
Given the crash rate volume and investigation problems

The link to the bug is pointing to an invalid location(=17145840 instead of =1714584).

URL fixed, thanks

Comment 10

[Miko Mynttinen]

(In reply to Donal Meehan [:dmeehan] from comment #7)

:miko how feasible would it be to blackout bug 1714584
Given the crash rate volume and investigation problems

I think it is possible. Bug 1757184 should be backed out first in that case.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

I took a look at that and rev a261bb633790 doesn't back out cleanly. Looks like it needs a bit of rebasing. Can you please post a backout patch?

Comment 12

[Miko Mynttinen]

Attachment: Bug 1744069 - Restore the previous behavior of merging RDL list traversal r=emilio

Comment 13

[Miko Mynttinen]

This was a tricky bug to figure out. The crashing website also triggers another assertion1 about "unmodified subtree optimization" of merging retained display lists.

The actual bug is that the patch for bug 1714584 changed the display list preprocessing behavior slightly by always emptying the child lists fully. The merging RDL code expects that display list is kept intact when the above "keep linked" optimization fails. I have restored this behavior.

Comment 14

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1714584

Comment 15

[Arthur K. (he/him)]

Miko,

A bit tough to see the number 1 (right after assertion) from comment 13.

Comment 16

[Worcester12345]

Not sure if this is the same thing:

https://crash-stats.mozilla.org/report/index/3e7ff23a-142b-475c-8705-cdd8d0220310

Comment 17

[Donal Meehan [:dmeehan]]

Reminder: :miko Please submit a beta uplift request when this is ready for an uplift. This is a top crash in 99.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/844b96d8f690
Restore the previous behavior of merging RDL list traversal r=emilio

Comment 19

[Miko Mynttinen]

Comment on attachment 9267532 [details]
Bug 1744069 - Restore the previous behavior of merging RDL list traversal r=emilio

Beta/Release Uplift Approval Request

• User impact if declined: Possible crashes.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: 1. Open the page https://thejigsawpuzzles.com/Puzzle-of-the-Day/Las-Vegas_2-jigsaw-puzzle

2. Wait for the jigsaw puzzle to load
3. If no crash, refresh to repeat. The crash rate should be fairly high (30-50%)

• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The patch restores the previous behavior that was changed in bug 1714584.
• String changes made/needed:

Comment 20

[Miko Mynttinen]

Note that the crash is not reproducible in Nightly unless the pref layout.display-list.retain.sc is changed to false. The default value will change to false when bug 1759514 lands.

Comment 21

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/844b96d8f690

Comment 22

[Donal Meehan [:dmeehan]]

Comment on attachment 9267532 [details]
Bug 1744069 - Restore the previous behavior of merging RDL list traversal r=emilio

Approved for 99.0b4. Thanks.

Comment 23

[Donal Meehan [:dmeehan]]

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

Comment 24

[Worcester12345]

(In reply to Worcester12345 from comment #16)

Not sure if this is the same thing:

https://crash-stats.mozilla.org/report/index/3e7ff23a-142b-475c-8705-cdd8d0220310

Yes? No?

Comment 25

[Simona Badau, Desktop QA]

Reproduced the initial issue on Firefox 99 beta 3 on macOS Big Sur 11.6.

Verified as fixed on the latest Nightly 100.0a1 on macOS Big Sur 11.6, Windows 10 x64, and Ubuntu 20.04 - no crash occurs when following the steps from Comment 19 and the pref layout.display-list.retain.sc is set to false.

Comment 26

[Simona Badau, Desktop QA]

Verified as fixed on Firefox 99 beta 4 on macOS Big Sur 11.6, Windows 10 x64, and Ubuntu 20.04 x64.