Closed Bug 1403920 Opened 2 years ago Closed 2 years ago

Bajillions of "bad aListVisibleBounds" assertion errors in linux64-qr mda2 debug runs

Categories

(Core :: Graphics: WebRender, defect, P1)

Other Branch
defect

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox57 --- unaffected
firefox58 --- fixed

People

(Reporter: kats, Assigned: kats)

References

(Blocks 1 open bug)

Details

(Whiteboard: [wr-mvp] [gfx-noted])

Attachments

(1 file)

Take a look at a linux64-qr debug mda2 job log, e.g:

https://treeherder.mozilla.org/logviewer.html#?job_id=133788202&repo=mozilla-central

There's a ton of these things:

[Parent 3183, Main Thread] ###!!! ASSERTION: bad aListVisibleBounds: 'r.GetBounds().IsEqualInterior(aListVisibleBounds)', file /builds/worker/workspace/build/src/layout/painting/nsDisplayList.cpp, line 2040
#01: nsDisplayMask::ComputeVisibility [layout/painting/nsDisplayList.cpp:9094]
#02: mozilla::layers::WebRenderLayerManager::GenerateFallbackData [mfbt/RefPtr.h:287]
#03: mozilla::layers::WebRenderLayerManager::BuildWrMaskImage [mfbt/AlreadyAddRefed.h:121]
#04: nsDisplayMask::CreateWebRenderCommands [layout/painting/nsDisplayList.cpp:9186]
#05: mozilla::layers::WebRenderLayerManager::CreateWebRenderCommandsFromDisplayList [gfx/layers/wr/WebRenderLayerManager.cpp:345]
#06: mozilla::layers::WebRenderLayerManager::EndTransactionInternal [gfx/layers/wr/WebRenderLayerManager.cpp:785]
#07: nsDisplayList::PaintRoot [layout/painting/nsDisplayList.cpp:2177]
#08: nsLayoutUtils::PaintFrame [mfbt/RefPtr.h:130]
#09: mozilla::PresShell::Paint [layout/base/PresShell.cpp:6456]
#10: nsViewManager::ProcessPendingUpdatesPaint [gfx/src/nsRegion.h:75]
#11: nsViewManager::ProcessPendingUpdatesForView [view/nsViewManager.cpp:408]
#12: nsViewManager::ProcessPendingUpdates [view/nsViewManager.cpp:1099]
#13: nsRefreshDriver::Tick [layout/base/nsRefreshDriver.cpp:2081]
#14: mozilla::RefreshDriverTimer::TickRefreshDrivers [layout/base/nsRefreshDriver.cpp:309]
#15: mozilla::RefreshDriverTimer::Tick [layout/base/nsRefreshDriver.cpp:331]
#16: mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver [layout/base/nsRefreshDriver.cpp:686]
#17: mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::ParentProcessVsyncNotifier::Run [layout/base/nsRefreshDriver.cpp:531]
#18: nsThread::ProcessNextEvent [mfbt/Maybe.h:445]
#19: NS_ProcessNextEvent [xpcom/threads/nsThreadUtils.cpp:524]
#20: mozilla::ipc::MessagePump::Run [ipc/glue/MessagePump.cpp:98]
#21: MessageLoop::RunInternal [ipc/chromium/src/base/message_loop.cc:327]
#22: MessageLoop::Run [ipc/chromium/src/base/message_loop.cc:298]
#23: nsBaseAppShell::Run [widget/nsBaseAppShell.cpp:160]
#24: nsAppStartup::Run [toolkit/components/startup/nsAppStartup.cpp:289]
#25: XREMain::XRE_mainRun [toolkit/xre/nsAppRunner.cpp:4702]
#26: XREMain::XRE_main [toolkit/xre/nsAppRunner.cpp:4865]
#27: XRE_main [toolkit/xre/nsAppRunner.cpp:4960]
#28: do_main [browser/app/nsBrowserApp.cpp:236]
#29: main [browser/app/nsBrowserApp.cpp:311]
#30: libc.so.6 + 0x20830


I suspect that the sheer volume of assertions is contributing to intermittent failures (the job sometimes fails with "Task timeout after 5400 seconds. Force killing container." which is lumped into bug 1204281.
Whiteboard: [gfx-noted][wr-mvp][triage] → [wr-reserve] [gfx-noted]
Matt, does this assertion even make sense? It seems like a number of call sites of ComputeVisibilityForSublist (e.g. [1], [2]) pass in a `aListVisibleBounds` value that is the intersection of `aVisibleRegion` and the result of a `GetClippedBoundsWithRespectToASR(..)` call. However the assertion [3] does the intersection against `GetBounds()` which presumably sometimes returns different values than `GetClippedBoundsWithRespectToASR`. So it seems to me like the assertion is wrong, or we have a problem elsewhere that is causing the two functions to return different values when they should always be returning the same value.

[1] http://searchfox.org/mozilla-central/rev/e62604a97286f49993eb29c493672c17c7398da9/layout/painting/nsDisplayList.cpp#9106
[2] http://searchfox.org/mozilla-central/rev/e62604a97286f49993eb29c493672c17c7398da9/layout/painting/nsDisplayList.cpp#9366
[3] http://searchfox.org/mozilla-central/rev/e62604a97286f49993eb29c493672c17c7398da9/layout/painting/nsDisplayList.cpp#2038
Flags: needinfo?(matt.woodrow)
We're not supposed to be calling ComputeVisibility directly, does RecomputeVisibility help?
Flags: needinfo?(matt.woodrow)
It does seem to help, at least locally when I run the dom/media/test/test_mediarecorder_record_changing_video_resolution.html mochitest I was seeing ~76 assertions before and they all went away.

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=271ab03a3294071b2eae2531050948b1adcfe57e
Assignee: nobody → bugmail
Status: NEW → ASSIGNED
Priority: P3 → P1
Whiteboard: [wr-reserve] [gfx-noted] → [wr-mvp] [gfx-noted]
Comment on attachment 8915624 [details]
Bug 1403920 - Use RecomputeVisibility instead of ComputeVisibility to avoid triggering assertion failures.

https://reviewboard.mozilla.org/r/186816/#review192004
Attachment #8915624 - Flags: review?(matt.woodrow) → review+
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0baba928b5d6
Use RecomputeVisibility instead of ComputeVisibility to avoid triggering assertion failures. r=mattwoodrow
https://hg.mozilla.org/mozilla-central/rev/0baba928b5d6
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Blocks: 1411358
No longer blocks: 1411358
You need to log in before you can comment on or make changes to this bug.