1915006 - Assertion failure: overscrollScale > 0.0f && overscrollScale <= 1.0f, at /builds/worker/checkouts/gecko/gfx/layers/apz/src/ScrollThumbUtils.cpp:259

Status: VERIFIED FIXED

(Core :: Panning and Zooming, defect, P3)

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.html

Found while fuzzing m-c 20240729-e8bede86288e (--enable-debug --enable-fuzzing)

To reproduce via Grizzly Replay:

$ pip install fuzzfetch grizzly-framework --upgrade
$ python -m fuzzfetch -d --fuzzing -n firefox
$ python -m grizzly.replay.bugzilla ./firefox/firefox <bugid>

Assertion failure: overscrollScale > 0.0f && overscrollScale <= 1.0f, at /builds/worker/checkouts/gecko/gfx/layers/apz/src/ScrollThumbUtils.cpp:259

#0 0x7460f6d35db8 in mozilla::layers::apz::AsyncScrollThumbTransformer::ApplyTransformForAxis(mozilla::layers::Axis const&) /builds/worker/checkouts/gecko/gfx/layers/apz/src/ScrollThumbUtils.cpp:259:5
#1 0x7460f6d35fea in mozilla::layers::apz::AsyncScrollThumbTransformer::ComputeTransform() /builds/worker/checkouts/gecko/gfx/layers/apz/src/ScrollThumbUtils.cpp:294:5
#2 0x7460f6d36506 in mozilla::layers::apz::ComputeTransformForScrollThumb(mozilla::gfx::Matrix4x4Typed<mozilla::LayerPixel, mozilla::ParentLayerPixel, float> const&, mozilla::gfx::Matrix4x4Typed<mozilla::gfx::UnknownUnits, mozilla::gfx::UnknownUnits, float> const&, mozilla::layers::AsyncPanZoomController*, mozilla::layers::FrameMetrics const&, mozilla::layers::ScrollbarData const&, bool) /builds/worker/checkouts/gecko/gfx/layers/apz/src/ScrollThumbUtils.cpp:336:8
#3 0x7460f6c995f1 in ComputeTransformForScrollThumb /builds/worker/checkouts/gecko/gfx/layers/apz/src/APZCTreeManager.cpp:3799:10
#4 0x7460f6c995f1 in operator() /builds/worker/checkouts/gecko/gfx/layers/apz/src/APZCTreeManager.cpp:889:22
#5 0x7460f6c995f1 in CallWithLastContentPaintMetrics<(lambda at /builds/worker/checkouts/gecko/gfx/layers/apz/src/APZCTreeManager.cpp:888:13)> /builds/worker/checkouts/gecko/gfx/layers/apz/src/AsyncPanZoomController.h:1172:12
#6 0x7460f6c995f1 in mozilla::layers::APZCTreeManager::SampleForWebRender(mozilla::Maybe<mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>> const&, mozilla::wr::TransactionWrapper&, mozilla::layers::SampleTime const&) /builds/worker/checkouts/gecko/gfx/layers/apz/src/APZCTreeManager.cpp:887:27
#7 0x7460f6cc6165 in mozilla::layers::APZSampler::SampleForWebRender(mozilla::Maybe<mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>> const&, mozilla::wr::TransactionWrapper&) /builds/worker/checkouts/gecko/gfx/layers/apz/src/APZSampler.cpp:100:9
#8 0x7460f6cc5f8b in mozilla::layers::APZSampler::SampleForWebRender(mozilla::wr::WrWindowId const&, unsigned long const*, mozilla::wr::Transaction*) /builds/worker/checkouts/gecko/gfx/layers/apz/src/APZSampler.cpp:73:14
#9 0x7460f6cc6b58 in apz_sample_transforms /builds/worker/checkouts/gecko/gfx/layers/apz/src/APZSampler.cpp:209:3
#10 0x7460fdf50d96 in _$LT$webrender_bindings..bindings..SamplerCallback$u20$as$u20$webrender..renderer..init..AsyncPropertySampler$GT$::sample::hc0f9d62d9a22d035 /builds/worker/checkouts/gecko/gfx/webrender_bindings/src/bindings.rs:1103:13
#11 0x7460fe299727 in webrender::render_backend::RenderBackend::update_document::he3e7a7b6c3ba6f40 /builds/worker/checkouts/gecko/gfx/wr/webrender/src/render_backend.rs:1380:39
#12 0x7460fe294b64 in webrender::render_backend::RenderBackend::prepare_transactions::h3de333cb7149effc /builds/worker/checkouts/gecko/gfx/wr/webrender/src/render_backend.rs:1292:28
#13 0x7460fe294b64 in webrender::render_backend::RenderBackend::process_api_msg::h0b0af9b2547f93c1 /builds/worker/checkouts/gecko/gfx/wr/webrender/src/render_backend.rs:1145:17
#14 0x7460fdfc4919 in webrender::render_backend::RenderBackend::run::hf96d2ea02dc0d99c /builds/worker/checkouts/gecko/gfx/wr/webrender/src/render_backend.rs:796:21
#15 0x7460fdfc4919 in webrender::renderer::init::create_webrender_instance::_$u7b$$u7b$closure$u7d$$u7d$::hfd0fdad294c3c9b1 /builds/worker/checkouts/gecko/gfx/wr/webrender/src/renderer/init.rs:704:9
#16 0x7460fdfc4919 in std::sys_common::backtrace::__rust_begin_short_backtrace::hfbece956e0694b7a /rustc/129f3b9964af4d4a709d1383930ade12dfe7c081/library/std/src/sys_common/backtrace.rs:155:18
#17 0x7460fdfd173c in std::thread::Builder::spawn_unchecked_::_$u7b$$u7b$closure$u7d$$u7d$::_$u7b$$u7b$closure$u7d$$u7d$::hfbd623adfc480c35 /rustc/129f3b9964af4d4a709d1383930ade12dfe7c081/library/std/src/thread/mod.rs:542:17
#18 0x7460fdfd173c in _$LT$core..panic..unwind_safe..AssertUnwindSafe$LT$F$GT$$u20$as$u20$core..ops..function..FnOnce$LT$$LP$$RP$$GT$$GT$::call_once::h9c3185071145e535 /rustc/129f3b9964af4d4a709d1383930ade12dfe7c081/library/core/src/panic/unwind_safe.rs:272:9
#19 0x7460fdfd173c in std::panicking::try::do_call::h1f3b33afb668dae9 /rustc/129f3b9964af4d4a709d1383930ade12dfe7c081/library/std/src/panicking.rs:559:40
#20 0x7460fdfd173c in std::panicking::try::h963f7b317f7ce35d /rustc/129f3b9964af4d4a709d1383930ade12dfe7c081/library/std/src/panicking.rs:523:19
#21 0x7460fdfd173c in std::panic::catch_unwind::h29a4e7c6561ee576 /rustc/129f3b9964af4d4a709d1383930ade12dfe7c081/library/std/src/panic.rs:149:14
#22 0x7460fdfd173c in std::thread::Builder::spawn_unchecked_::_$u7b$$u7b$closure$u7d$$u7d$::h015e709691d9d18f /rustc/129f3b9964af4d4a709d1383930ade12dfe7c081/library/std/src/thread/mod.rs:541:30
#23 0x7460fdfd173c in core::ops::function::FnOnce::call_once$u7b$$u7b$vtable.shim$u7d$$u7d$::h64c51ab2863928a9 /rustc/129f3b9964af4d4a709d1383930ade12dfe7c081/library/core/src/ops/function.rs:250:5
#24 0x7460ff7cde2a in _$LT$alloc..boxed..Box$LT$F$C$A$GT$$u20$as$u20$core..ops..function..FnOnce$LT$Args$GT$$GT$::call_once::h09e5a4c541afa800 /rustc/129f3b9964af4d4a709d1383930ade12dfe7c081/library/alloc/src/boxed.rs:2022:9
#25 0x7460ff7cde2a in _$LT$alloc..boxed..Box$LT$F$C$A$GT$$u20$as$u20$core..ops..function..FnOnce$LT$Args$GT$$GT$::call_once::h9c8b03c22f4e7026 /rustc/129f3b9964af4d4a709d1383930ade12dfe7c081/library/alloc/src/boxed.rs:2022:9
#26 0x7460ff7cde2a in std::sys::pal::unix::thread::Thread::new::thread_start::h522bc89a54da820a /rustc/129f3b9964af4d4a709d1383930ade12dfe7c081/library/std/src/sys/pal/unix/thread.rs:108:17
#27 0x746108e94ac2 in start_thread nptl/pthread_create.c:442:8
#28 0x746108f2684f  misc/../sysdeps/unix/sysv/linux/x86_64/clone3.S:81

Comment 1

[Bugmon [:jkratzer for issues]]

Verified bug as reproducible on mozilla-central 20240826212858-74b2859d9582.
The bug appears to have been introduced in the following build range:

Start: c2b76f322973a4e46471d3e3e76349ed8b9a054a (20240604151845)
End: b764cb0c3da42e179b882c3c8d57605c056caa47 (20240604152607)
Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=c2b76f322973a4e46471d3e3e76349ed8b9a054a&tochange=b764cb0c3da42e179b882c3c8d57605c056caa47

Comment 2

[Dianna Smith [:diannaS]]

:botond this seems to be related to enabling overscrolling in nightly in bug 1899918. can you confirm? if not feel free to change the regressor field

Comment 3

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

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

Comment 4

[Pascal Chevrel:pascalc]

We built and shipped our release candidate, marking as fix-optional for 130 in case we have a fix for the planned dot release.

Comment 5

[Botond Ballo [:botond]]

(In reply to Dianna Smith [:diannaS] from comment #2)

:botond this seems to be related to enabling overscrolling in nightly in bug 1899918. can you confirm? if not feel free to change the regressor field

Yes, in the regression window from comment 1, it's definitely bug 1899918 which allowed the fuzzers to start hitting this assertion.

That said, the underlying issue is pre-existing, as the code containing the assertion is cross-platform, and overscroll has been enabled on other platforms since before bug 1899918 enabled it on Linux. I suspect the underlying issue has been there since the code's introduction in bug 1700169.

Comment 6

[Botond Ballo [:botond]]

There are two issues here:

1. The code for squishing scrollbar thumbs during an overscroll animation squishes the thumb by the proportion of the overscroll amount to the scrollport length (e.g. scrollport width), and assumes that this proportion will not be greater than 1. However, it's possible for the overscroll amount to be greater than the scrollport length if an overscroll animation is started with a sufficiently high velocity. We need to either adjust the overscroll animation code to avoid getting into this much overscroll, or handle this much overscroll in the thumb squishing code.

2. Programmatic scrolls (e.g. things like scrollTo(), scrollBy(), scrollIntoView(), with scroll-behavior: smooth) should not be able to trigger overscroll at all, but in this test case we are observing that happening.

Comment 7

[Botond Ballo [:botond]]

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

2. Programmatic scrolls (e.g. things like scrollTo(), scrollBy(), scrollIntoView(), with scroll-behavior: smooth) should not be able to trigger overscroll at all, but in this test case we are observing that happening.

To be honest, I'm not quite sure why HandleSmoothScrollOverscroll exists at all. It dates back to the original introduction of scroll-behavior: smooth in bug 1022825, but it seems to violate our long-standing practice of only allowing user scrolling (not programmatic scrolling) to trigger overscroll. (Moreover, looking at the implementation, it can not only trigger an overscroll animation, but also scroll handoff to an enclosing scroll frame, which seems unlikely to be desirable as well.)

Comment 8

[Botond Ballo [:botond]]

After fixing (2), (1) will become even harder to trigger, but in principle it could still happen if e.g. someone on a platform where we use FlingAnimation (like Linux) has their touchpad configured to produce huge deltas; so for completeness, we need a fix for (1) as well.

Comment 9

[Botond Ballo [:botond]]

I'll use this bug to track issue (1), since that's the part that's a regression from bug 1700169.

I've spun out issue (2) into bug 1916137. (That's technically a regression from bug 1022825.)

Comment 10

[Botond Ballo [:botond]]

The user impact of issue (1) is quite low (the thumb just isn't rendered during the part of the overscroll animation when the overscroll amount is greater than the scrollport extent -- which is likely what our fallback behaviour would be when we explicitly handle this scenario anyways), so I'm assigning this an S4.

Comment 11

[Botond Ballo [:botond]]

Marking 'wontfix' for older branches based on severity assessment in comment 10.

Comment 12

[Botond Ballo [:botond]]

(I have a fix locally which I plan to land on trunk, just can't submit it right now because Phabricator seems to be down.)

Comment 13

[Botond Ballo [:botond]]

Attachment: Bug 1915006 - When squishing a scroll thumb during overscroll, handle the case where the overscroll amount is larger than the scroll port length. r=hiro

Comment 14

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

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

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d5ea88747cbe
When squishing a scroll thumb during overscroll, handle the case where the overscroll amount is larger than the scroll port length. r=hiro

Comment 16

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/d5ea88747cbe

Comment 17

[Bugmon [:jkratzer for issues]]

Verified bug as fixed on rev mozilla-central 20240905042147-2afe9c297824.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.

Comment 18

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

The patch landed in nightly and beta is affected.
:botond, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox131 to wontfix.

For more information, please visit BugBot documentation.