1729586 - Assertion failure: false (item should have finite clip with respect to aASR), at layout/painting/nsDisplayList.cpp:2646

Status: NEW

(Core :: Web Painting, defect, P3)

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.html

Found while fuzzing m-c 20210901-18d7ddf67682 (--enable-debug --enable-fuzzing)

Assertion failure: aChild->GetApzc() != parent, at src/gfx/layers/apz/src/HitTestingTreeNode.cpp:93

#0 0x7f92f6816a6a in mozilla::layers::HitTestingTreeNode::SetLastChild(mozilla::layers::HitTestingTreeNode*) src/gfx/layers/apz/src/HitTestingTreeNode.cpp:93:7
#1 0x7f92f67f7f40 in mozilla::layers::HitTestingTreeNode* mozilla::layers::APZCTreeManager::PrepareNodeForLayer<mozilla::layers::WebRenderScrollDataWrapper>(mozilla::RecursiveMutexAutoLock const&, mozilla::layers::WebRenderScrollDataWrapper const&, mozilla::layers::FrameMetrics const&, mozilla::layers::LayersId, mozilla::Maybe<mozilla::layers::ZoomConstraints> const&, mozilla::layers::AncestorTransform const&, mozilla::layers::HitTestingTreeNode*, mozilla::layers::HitTestingTreeNode*, mozilla::layers::APZCTreeManager::TreeBuildingState&) src/gfx/layers/apz/src/APZCTreeManager.cpp:1383:5
#2 0x7f92f67f667e in void mozilla::layers::APZCTreeManager::UpdateHitTestingTreeImpl<mozilla::layers::WebRenderScrollDataWrapper>(mozilla::layers::WebRenderScrollDataWrapper const&, bool, mozilla::layers::LayersId, unsigned int)::'lambda'(mozilla::layers::WebRenderScrollDataWrapper)::operator()(mozilla::layers::WebRenderScrollDataWrapper) const src/gfx/layers/apz/src/APZCTreeManager.cpp:479:38
#3 0x7f92f67f5e42 in _ZN7mozilla6layersL11ForEachNodeINS0_15ReverseIteratorENS0_26WebRenderScrollDataWrapperEZNS0_15APZCTreeManager24UpdateHitTestingTreeImplIS3_EEvRKT_bNS0_8LayersIdEjEUlS3_E_ZNS5_IS3_EEvS8_bS9_jEUlS3_E0_EENSt9enable_ifIXaasr3stdE9is_same_vIDTclfp0_fp_EEvEsr3stdE9is_same_vIDTclfp1_fp_EEvEEvE4typeET0_RKT1_RKT2_ src/gfx/layers/TreeTraversal.h:139:3
#4 0x7f92f67f5ec7 in _ZN7mozilla6layersL11ForEachNodeINS0_15ReverseIteratorENS0_26WebRenderScrollDataWrapperEZNS0_15APZCTreeManager24UpdateHitTestingTreeImplIS3_EEvRKT_bNS0_8LayersIdEjEUlS3_E_ZNS5_IS3_EEvS8_bS9_jEUlS3_E0_EENSt9enable_ifIXaasr3stdE9is_same_vIDTclfp0_fp_EEvEsr3stdE9is_same_vIDTclfp1_fp_EEvEEvE4typeET0_RKT1_RKT2_ src/gfx/layers/TreeTraversal.h:143:5
#5 0x7f92f67f5ec7 in _ZN7mozilla6layersL11ForEachNodeINS0_15ReverseIteratorENS0_26WebRenderScrollDataWrapperEZNS0_15APZCTreeManager24UpdateHitTestingTreeImplIS3_EEvRKT_bNS0_8LayersIdEjEUlS3_E_ZNS5_IS3_EEvS8_bS9_jEUlS3_E0_EENSt9enable_ifIXaasr3stdE9is_same_vIDTclfp0_fp_EEvEsr3stdE9is_same_vIDTclfp1_fp_EEvEEvE4typeET0_RKT1_RKT2_ src/gfx/layers/TreeTraversal.h:143:5
#6 0x7f92f67f5ec7 in _ZN7mozilla6layersL11ForEachNodeINS0_15ReverseIteratorENS0_26WebRenderScrollDataWrapperEZNS0_15APZCTreeManager24UpdateHitTestingTreeImplIS3_EEvRKT_bNS0_8LayersIdEjEUlS3_E_ZNS5_IS3_EEvS8_bS9_jEUlS3_E0_EENSt9enable_ifIXaasr3stdE9is_same_vIDTclfp0_fp_EEvEsr3stdE9is_same_vIDTclfp1_fp_EEvEEvE4typeET0_RKT1_RKT2_ src/gfx/layers/TreeTraversal.h:143:5
#7 0x7f92f67f5ec7 in _ZN7mozilla6layersL11ForEachNodeINS0_15ReverseIteratorENS0_26WebRenderScrollDataWrapperEZNS0_15APZCTreeManager24UpdateHitTestingTreeImplIS3_EEvRKT_bNS0_8LayersIdEjEUlS3_E_ZNS5_IS3_EEvS8_bS9_jEUlS3_E0_EENSt9enable_ifIXaasr3stdE9is_same_vIDTclfp0_fp_EEvEsr3stdE9is_same_vIDTclfp1_fp_EEvEEvE4typeET0_RKT1_RKT2_ src/gfx/layers/TreeTraversal.h:143:5
#8 0x7f92f67f5ec7 in _ZN7mozilla6layersL11ForEachNodeINS0_15ReverseIteratorENS0_26WebRenderScrollDataWrapperEZNS0_15APZCTreeManager24UpdateHitTestingTreeImplIS3_EEvRKT_bNS0_8LayersIdEjEUlS3_E_ZNS5_IS3_EEvS8_bS9_jEUlS3_E0_EENSt9enable_ifIXaasr3stdE9is_same_vIDTclfp0_fp_EEvEsr3stdE9is_same_vIDTclfp1_fp_EEvEEvE4typeET0_RKT1_RKT2_ src/gfx/layers/TreeTraversal.h:143:5
#9 0x7f92f67f5ec7 in _ZN7mozilla6layersL11ForEachNodeINS0_15ReverseIteratorENS0_26WebRenderScrollDataWrapperEZNS0_15APZCTreeManager24UpdateHitTestingTreeImplIS3_EEvRKT_bNS0_8LayersIdEjEUlS3_E_ZNS5_IS3_EEvS8_bS9_jEUlS3_E0_EENSt9enable_ifIXaasr3stdE9is_same_vIDTclfp0_fp_EEvEsr3stdE9is_same_vIDTclfp1_fp_EEvEEvE4typeET0_RKT1_RKT2_ src/gfx/layers/TreeTraversal.h:143:5
#10 0x7f92f67f5ec7 in _ZN7mozilla6layersL11ForEachNodeINS0_15ReverseIteratorENS0_26WebRenderScrollDataWrapperEZNS0_15APZCTreeManager24UpdateHitTestingTreeImplIS3_EEvRKT_bNS0_8LayersIdEjEUlS3_E_ZNS5_IS3_EEvS8_bS9_jEUlS3_E0_EENSt9enable_ifIXaasr3stdE9is_same_vIDTclfp0_fp_EEvEsr3stdE9is_same_vIDTclfp1_fp_EEvEEvE4typeET0_RKT1_RKT2_ src/gfx/layers/TreeTraversal.h:143:5
#11 0x7f92f678d1bb in void mozilla::layers::APZCTreeManager::UpdateHitTestingTreeImpl<mozilla::layers::WebRenderScrollDataWrapper>(mozilla::layers::WebRenderScrollDataWrapper const&, bool, mozilla::layers::LayersId, unsigned int) src/gfx/layers/apz/src/APZCTreeManager.cpp:438:5
#12 0x7f92f6804bda in UpdateHitTestingTree src/gfx/layers/apz/src/APZCTreeManager.cpp:720:3
#13 0x7f92f6804bda in operator() src/gfx/layers/apz/src/APZUpdater.cpp:204:25
#14 0x7f92f6804bda in mozilla::detail::RunnableFunction<mozilla::layers::APZUpdater::UpdateScrollDataAndTreeState(mozilla::layers::LayersId, mozilla::layers::LayersId, mozilla::wr::Epoch const&, mozilla::layers::WebRenderScrollData&&)::$_31>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:531:5
#15 0x7f92f67b57df in mozilla::layers::APZUpdater::ProcessQueue() src/gfx/layers/apz/src/APZUpdater.cpp:466:23
#16 0x7f92f67b504a in mozilla::layers::APZUpdater::CompleteSceneSwap(mozilla::wr::WrWindowId const&, mozilla::wr::WrPipelineInfo const&) src/gfx/layers/apz/src/APZUpdater.cpp:120:12
#17 0x7f92f67b8519 in apz_post_scene_swap src/gfx/layers/apz/src/APZUpdater.cpp:534:3
#18 0x7f92fd6e770d in _$LT$webrender_bindings..bindings..APZCallbacks$u20$as$u20$webrender..renderer..SceneBuilderHooks$GT$::post_scene_swap::h6285849f0cf19fdc src/gfx/webrender_bindings/src/bindings.rs:976:13
#19 0x7f92fd9d80d9 in webrender::scene_builder_thread::SceneBuilderThread::forward_built_transactions::h6c07854e97bd6a1c src/gfx/wr/webrender/src/scene_builder_thread.rs:699:13
#20 0x7f92fd9d80d9 in webrender::scene_builder_thread::SceneBuilderThread::run::h5813092075594cf2 src/gfx/wr/webrender/src/scene_builder_thread.rs:313:21
#21 0x7f92fd7506e9 in webrender::renderer::Renderer::new::_$u7b$$u7b$closure$u7d$$u7d$::h8489d50038ddbb44 src/gfx/wr/webrender/src/renderer/mod.rs:1230:13
#22 0x7f92fd7506e9 in std::sys_common::backtrace::__rust_begin_short_backtrace::hc976d6d50b6d9ed4 /rustc/a178d0322ce20e33eac124758e837cbd80a6f633/library/std/src/sys_common/backtrace.rs:125:18
#23 0x7f92fd76de0e in std::thread::Builder::spawn_unchecked::_$u7b$$u7b$closure$u7d$$u7d$::_$u7b$$u7b$closure$u7d$$u7d$::h1f8f7716cd7d4482 /rustc/a178d0322ce20e33eac124758e837cbd80a6f633/library/std/src/thread/mod.rs:481:17
#24 0x7f92fd76de0e in _$LT$std..panic..AssertUnwindSafe$LT$F$GT$$u20$as$u20$core..ops..function..FnOnce$LT$$LP$$RP$$GT$$GT$::call_once::h09819f00e606cdc7 /rustc/a178d0322ce20e33eac124758e837cbd80a6f633/library/std/src/panic.rs:347:9
#25 0x7f92fd76de0e in std::panicking::try::do_call::h9b682a12419da40e /rustc/a178d0322ce20e33eac124758e837cbd80a6f633/library/std/src/panicking.rs:401:40
#26 0x7f92fd76de0e in std::panicking::try::h32cd152da2f6a3ae /rustc/a178d0322ce20e33eac124758e837cbd80a6f633/library/std/src/panicking.rs:365:19
#27 0x7f92fd76de0e in std::panic::catch_unwind::h5849d47c1e6cda14 /rustc/a178d0322ce20e33eac124758e837cbd80a6f633/library/std/src/panic.rs:434:14
#28 0x7f92fd76de0e in std::thread::Builder::spawn_unchecked::_$u7b$$u7b$closure$u7d$$u7d$::h68a449b9d6d1d519 /rustc/a178d0322ce20e33eac124758e837cbd80a6f633/library/std/src/thread/mod.rs:480:30
#29 0x7f92fd76de0e in core::ops::function::FnOnce::call_once$u7b$$u7b$vtable.shim$u7d$$u7d$::hc86aa4554ea1c4fa /rustc/a178d0322ce20e33eac124758e837cbd80a6f633/library/core/src/ops/function.rs:227:5
#30 0x7f92fed41736 in _$LT$alloc..boxed..Box$LT$F$C$A$GT$$u20$as$u20$core..ops..function..FnOnce$LT$Args$GT$$GT$::call_once::h7ece6cfefaff1005 /rustc/a178d0322ce20e33eac124758e837cbd80a6f633/library/alloc/src/boxed.rs:1575:9
#31 0x7f92fed41736 in _$LT$alloc..boxed..Box$LT$F$C$A$GT$$u20$as$u20$core..ops..function..FnOnce$LT$Args$GT$$GT$::call_once::hb8b48e55c21f193e /rustc/a178d0322ce20e33eac124758e837cbd80a6f633/library/alloc/src/boxed.rs:1575:9
#32 0x7f92fed41736 in std::sys::unix::thread::Thread::new::thread_start::h8c7c4450dba62914 /rustc/a178d0322ce20e33eac124758e837cbd80a6f633/library/std/src/sys/unix/thread.rs:71:17
#33 0x7f930bac1608 in start_thread /build/glibc-eX1tMB/glibc-2.31/nptl/pthread_create.c:477:8
#34 0x7f930b689292 in clone /build/glibc-eX1tMB/glibc-2.31/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:95

Comment 1

[Tyson Smith [:tsmith]]

A Pernosco session is available here: https://pernos.co/debug/xBwZuHd-8PEOgGVSXKf8vg/index.html

Comment 2

[Bugmon [:jkratzer for issues]]

Bugmon Analysis
Verified bug as reproducible on mozilla-central 20210908032417-a4d2ca53b2a4.
Failed to bisect testcase (Testcase reproduces on start build!):

Start: 6cc2266faca2a2301c81f3442d6d893ad3fc9fdf (20200909093957)
End: 18d7ddf67682e3eb75376f56e4f014c81d2596b3 (20210901214553)
BuildFlags: BuildFlags(asan=False, tsan=False, debug=True, fuzzing=True, coverage=False, valgrind=False, no_opt=False, fuzzilli=False)

Comment 3

[Botond Ballo [:botond] [away until Oct 19]]

I can't reproduce this one locally.

Tyson, is there any chance you could get a Pernosco recording for this with the pref gfx.webrender.dl.dump-content=true set?

Comment 4

[Timothy Nikkel (:tnikkel)]

Another possibility is there might be a pref set during fuzzing. Tyson can probably provide a prefs.js file that was used with this testcase. I've had luck reproducing this way in the past.

Comment 5

[Tyson Smith [:tsmith]]

Attachment: prefs.js

Let me know if it works with this prefs.js file.

Comment 6

[Timothy Nikkel (:tnikkel)]

I could reproduce with that prefs file (after removing the proxy related ones).
user_pref("layout.css.backdrop-filter.enabled", true); seems to be the key pref.

Comment 7

[Botond Ballo [:botond] [away until Oct 19]]

Thanks! I can reproduce the issue locally with layout.css.backdrop-filter.enabled=true.

Comment 8

[Botond Ballo [:botond] [away until Oct 19]]

Hmmm, so the fix for bug 1729581 actually causes this testcase to assert earlier, with:

Assertion failure: false (item should have finite clip with respect to aASR), at layout/painting/nsDisplayList.cpp:2610

Comment 9

[Botond Ballo [:botond] [away until Oct 19]]

On the plus side (?), if allowed to proceed past the finite-clip assertion, the resulting display list does not produce the malformed layer tree that triggers the original assertion.

Comment 10

[Botond Ballo [:botond] [away until Oct 19]]

Attachment: Display list before bug 1729581

Comment 11

[Botond Ballo [:botond] [away until Oct 19]]

Attachment: Display list after bug 1729581

Comment 12

[Botond Ballo [:botond] [away until Oct 19]]

I attached display list dumps before and after bug 1729581. In this comment I'll use addresses from the "after" dump.

Notably, the BackdropRootContainer item changed ASR, from a subframe ASR (0x7f7ed1a52688) to the root scroll frame's ASR (0x7f7ed1a51248).

The assertion then arises when we construct the mask item 0x7f7ec9d7a858, which has the subframe ASR. The nsDisplayWrapList base class constructor calls UpdateBounds(), which calls GetClippedBoundsWithRespectToASR() on the wrapped display list. As I understand it, this basically expects items in the list to have finite bounds with respect to the ASR of the item being constructed (i.e. the subframe ASR).

This criterion then fails to hold for the BackdropRootContainer item, whose clip chain indeed does not contain a clip corresponding to this ASR.

Suggestions for where we might be going wrong here are appreciated.

Comment 13

[Markus Stange [:mstange]]

Hmm, maybe that fix wasn't the right idea after all. Picking the content clip's ASR does seem to be important for the "finite bounds" invariant.

Here's another example:

• Scrolled mask
  • Opacity
    • Fixed element

The scrolled mask creates a scrolled content clip. The opacity doesn't get clipped, it just propagates the clip to its contents. And then the fixed element gets clipped. What should the ASR on the opacity be? Since the opacity doesn't have a clip, and is contained in a scrolled item, it should get the scrolled ASR.

Comment 14

[Botond Ballo [:botond] [away until Oct 19]]

(In reply to Botond Ballo [:botond] from comment #8)

Hmmm, so the fix for bug 1729581 actually causes this testcase to assert earlier, with:

Assertion failure: false (item should have finite clip with respect to aASR), at layout/painting/nsDisplayList.cpp:2610

With bug 1729581 backed out, this testcase no longer triggers the original assertion, likely thanks to our work in bug 1751788. So, I think we can consider the original issue to be resolved.

The remaining issue (for which backing out bug 1729581 may be the appropriate fix, but I don't feel confident in making that call) is more a display list building issue. Updating bug title and component accordingly.

Comment 15

[Bugmon [:jkratzer for issues]]

Bugmon Analysis
Testcase crashes using the initial build (mozilla-central 20210901214553-18d7ddf67682) but not with tip (mozilla-central 20220520153703-1d31a0098979.)
The bug appears to have been fixed in the following build range:

Start: ddbf2ba9acc3bdfe51595b2ee9943c30b25d1338 (20220516133202)
End: 6cc1af98195e7b1f353f5116a2c66dd7921d9af6 (20220515224447)
Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=ddbf2ba9acc3bdfe51595b2ee9943c30b25d1338&tochange=6cc1af98195e7b1f353f5116a2c66dd7921d9af6
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.

Comment 16

[Timothy Nikkel (:tnikkel)]

Since the testcase uses backdrop probably

6cc1af98195e7b1f353f5116a2c66dd7921d9af6 Glenn Watson — Bug 1768986 - Remove nsDisplayBackdropRoot r=gfx-reviewers,lsalzman

Comment 17

[Jason Kratzer [:jkratzer]]

Botond, can you confirm if this bug has actually been fixed based on the regression range in comment 15?

Comment 18

[Botond Ballo [:botond] [away until Oct 19]]

I can confirm that the commit mentioned in comment 16 stopped this assertion failure from happening by removing some code that was required for it to be triggered.

To the extent that the assertion failure was indicative of an underlying issue with ASR assignments during display list building, as hinted in comment 13, that issue may persist, though I don't understand it very well. It probably makes sense to address it if/when it comes up in other bugs (for other instances of this assertion failure).