Closed Bug 1697639 Opened 3 years ago Closed 3 years ago

Fission + WR/SW-WR: Assertion failure: !aParent || result->mParentAGR == aParent, at /builds/worker/checkouts/gecko/layout/painting/nsDisplayList.cpp:802

Categories

(Core :: Web Painting, defect, P3)

defect

Tracking

()

RESOLVED FIXED
91 Branch
Fission Milestone Future
Tracking Status
firefox-esr78 --- unaffected
firefox88 --- wontfix
firefox89 --- wontfix
firefox90 --- wontfix
firefox91 --- fixed

People

(Reporter: cpeterson, Assigned: mikokm)

References

(Blocks 2 open bugs)

Details

(Keywords: assertion, testcase-wanted)

Attachments

(1 file, 1 obsolete file)

In bug 1697195, we tried to auto-enable SW-WR with Fission is enabled. Unfortunately, some tests failed when run with Fission + SW-WR mode:

dt test assertion failure in nsDisplayList.cpp

https://treeherder.mozilla.org/logviewer?job_id=332577782&repo=autoland&lineNumber=8717

INFO - GECKO(2093) | Assertion failure: !aParent || result->mParentAGR == aParent, at /builds/worker/checkouts/gecko/layout/painting/nsDisplayList.cpp:802

From here:

https://searchfox.org/mozilla-central/rev/ca910762568921c0faa34838d6a4efac2471dff1/layout/painting/nsDisplayList.cpp#802

See also bug 1494676 and bug 1645141 for other !aParent || result->mParentAGR == aParent assertion failures found by fuzzers.

No longer blocks: 1697195

Sending this bug to gfx-triage.

Does this dt test assert when is SW-WR enabled without Fission? This seems like a SW-WR bug.

Blocks: gfx-triage

I will attempt to reproduce.

Flags: needinfo?(bwerth)

Tracking for Fission M7a Beta. TBD whether SW-WR will block Fission MVP.

Fission Milestone: ? → M7a

So far I have not been able to reproduce this using SW-WR and Fission. Running the test browser_toolbox_options_disable_buttons.js which was the source of failure in comment 0 didn't hit the assert. I'll keep the needinfo open while I see if I can reason my way through the assertion failure without a reproduction case.

When bug 1697195 tried to land, the test hit the assertion failure 2 out of 2 times on autoland. Maybe there's something unique about the Linux test machines in automation?

https://treeherder.mozilla.org/jobs?repo=autoland&revision=d5ebf7eedb59abee4bc6c5750543917fa4ff8a6d&selectedTaskRun=F-aowUG1RSeisiCNdsqJtw.0

(In reply to Chris Peterson [:cpeterson] from comment #6)

When bug 1697195 tried to land, the test hit the assertion failure 2 out of 2 times on autoland. Maybe there's something unique about the Linux test machines in automation?

https://treeherder.mozilla.org/jobs?repo=autoland&revision=d5ebf7eedb59abee4bc6c5750543917fa4ff8a6d&selectedTaskRun=F-aowUG1RSeisiCNdsqJtw.0

I should have stated that my local testing is on macOS. It's not clear to me from the treeherder job if this test fails there on macOS -- or if it's even run on a fission-enabled macOS build.

I can't reproduce this on Linux, either. Instead of setting prefs, I'll locally apply the patches for Bug 1697195 and try again.

(In reply to Chris Peterson [:cpeterson] from comment #6)

When bug 1697195 tried to land, the test hit the assertion failure 2 out of 2 times on autoland. Maybe there's something unique about the Linux test machines in automation?

https://treeherder.mozilla.org/jobs?repo=autoland&revision=d5ebf7eedb59abee4bc6c5750543917fa4ff8a6d&selectedTaskRun=F-aowUG1RSeisiCNdsqJtw.0

I can't get this failure to reproduce on Linux with the patches for Bug 1697195 applied. I'm running devtools/client/framework/test/browser_toolbox_options_disable_buttons.js which appears to be implicated by the treeherder failure in comment 6. The test does time out, but it doesn't fail with the assert.

I believe this is automation-dependent, but I can't speculate on how or why.

Flags: needinfo?(bwerth)

Matt, this assertion is happening in AnimatedGeometryRoot tree code, but I can't get it to happen in my testing. Do you have any insight into what might be going on here? Or can you reproduce?

Flags: needinfo?(matt.woodrow)

(In reply to Brad Werth [:bradwerth] from comment #10)

Matt, this assertion is happening in AnimatedGeometryRoot tree code, but I can't get it to happen in my testing. Do you have any insight into what might be going on here? Or can you reproduce?

It's hard to tell from the stack alone.

The assert is telling us that we requested an AnimatedGeometryRoot object to be created for a given frame twice, and that we specified/calculated the ancestor AGR differently between the two times.

We'd need to know the stack for when we inserted the current frame into mFrameToAnimatedGeometryRootMap, and try to figure out why the parent was different at that time.

Flags: needinfo?(matt.woodrow)

We don't have any reason to think this is related to swiggle + fission. The assert is in display list code and occurs prior to any interaction with the compositor.

Severity: -- → S3
Component: Graphics: WebRender → Web Painting
Keywords: testcase-wanted
Priority: -- → P3

Andrew is working on turning on tests, he'll follow up.

Since this assertion failure is from a test configuration we're not currently running, this bug doesn't need to block Fission. If this test failure becomes reproducible, we can then track this bug again.

Fission Milestone: M7a → Future

We think this is fixed in our test migration work.

No longer blocks: gfx-triage
See Also: → 1713911

This still happens with Fission jobs, even with full WR, e.g. https://treeherder.mozilla.org/logviewer?job_id=342036416&repo=try&lineNumber=17721

Summary: Fission + sw-wr: Assertion failure: !aParent || result->mParentAGR == aParent, at /builds/worker/checkouts/gecko/layout/painting/nsDisplayList.cpp:802 → Fission + WR/SW-WR: Assertion failure: !aParent || result->mParentAGR == aParent, at /builds/worker/checkouts/gecko/layout/painting/nsDisplayList.cpp:802

I intend to disable this assert and reference this bug in the FIXME comments, unless we come to a resolution soon. Or at least mark it as intermittent in advance so I can possibly get through CI :).

miko, I heard you were investigating this?

Flags: needinfo?(mikokm)

(In reply to Andrew Osmond [:aosmond] from comment #20)

miko, I heard you were investigating this?

Yes.

Assignee: nobody → mikokm
Status: NEW → ASSIGNED
Flags: needinfo?(mikokm)
Attached patch workaround.diff (obsolete) — Splinter Review

This assertion is triggering because the AGR status of a frame is changing due to display port change.

This enforces a stable frame to AGR mapping, which was previously changing when frame AGR status changed with to display port change.
The downside of this is that the intermediate results are not cached, which means more traversals.

Attachment #9227441 - Attachment is obsolete: true
Pushed by mikokm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/eb9c0b2c7516
Only add AGRs to mFrameToAnimatedGeometryRootMap when creating AGRs r=mattwoodrow
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 91 Branch

Miko, now that you've fixed this AGR assertion failure, can we close bug 1645141 and bug 1700257 as fixed?

Flags: needinfo?(mikokm)
See Also: → 1700257

(In reply to Chris Peterson [:cpeterson] from comment #27)

Miko, now that you've fixed this AGR assertion failure, can we close bug 1645141 and bug 1700257 as fixed?

Probably yes.

Flags: needinfo?(mikokm)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: