Spinning of the test_relative_update failure when apz.allow_zooming=true. I debugged this a bit with rr. The test is doing a scrollBy call which is supposed to send a relative update to APZ. However, the relative-origin scrollby gets promoted to a non-relative origin here, because mLastScrollOrigin is nsGkAtoms::other, and that in turn was set during construction here.

So this seems to imply that if the very first scroll that a scrollframe gets is relative, it will always get promoted to non-relative. Which can then clobber a simultaneous scroll occuring on the APZ side, as happens in this test.

It looks like with apz.allow_zooming=false, the APZC for the subframe gets a NLU call near the beginning which has nonzero displayport margins, and that triggers this code path which triggers a repaint request from APZ to the main thread. That repaint request clears the mLastScrollOrigin on the main-thread side, via this call.

However with apz.allow_zooming=true, that first NLU call seems to not happen; the first actual NLU call received by the APZC is the one after the test sets zero margins and so APZ never triggers that initial repaint request.

Need to trace backwards more to find out why zooming=false does that initial paint while zooming=true does not.

Actually whether or not that initial paint happens seems not that relevant. The point here is that there is an existing bug in the code, where the mLastScrollOrigin of the scrollframe starts out as nsGkAtoms::other on construction, and so if the very first scroll attempt is a relative one, it gets converted to a non-relative one. This seems wrong, and this bug is just being exposed when turning on apz.allow_zooming.

I have patches locally that solve this problem, try push to verify it doesn't break anything else: https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=fe29bab9a615f030f2900c5f14bdadd3783dc35d

https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=6a0d197d80f6a729e5551b6fce4cb3d30c284484 should be better

Well it sure is good to see all those tests I added catching my current mistakes. Thank you, past self!

https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=38b1919644868a83b45ac5b757d48095afdb023e

"Bug 1643042 - Turn the scroll origin parameter into a strongly-typed enum."

Yes! We needed this!

Also from the patch

"Note also that we should never be passing eNone to functions like ScrollToImpl;"

Can we get assserts to this effect?

Sure. Here's the latest try push which had a small simplification relative to the previous one: https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=00d05bf9635437f46525cf04a45f0d74ef77b4b5

I've added an assert locally. Patches incoming.

This patch is a fairly mechanical conversion. The old nullptr gets converted
to ScrollOrigin::eUnknown, and all the other possible values get corresponding
values in the new ScrollOrigin enum. A few switch statements are introduced to
clean up big if statements, but other than that, additional cleanups will happen
in later patches.

This adds a new scroll origin, eNone, which is used as the initial value for
mLastScrollOrigin. Unlike eOther, this scroll origin can be clobbered by any
other scroll origin, including notably eRelative. This means that on a
brand-new scrollframe, if the first scroll call comes in with an origin of
eRelative, it will be preserved as a relative scroll instead of getting
converted to a non-relative scroll.

This in turn fixes a latent bug in the code that was exposed by the
test_relative_update.html APZ mochitest when run with apz.allow_zooming=true.

Note also that we should never be passing eNone to functions like ScrollToImpl;
for those scenarios we continue using eUnknown if we don't have a more specific
scroll origin to use. In other words, eNone is a sort of sentinel value to be
used for class fields, and is not to be used for actual scrollto-type calls.

Depends on D78438

This uses "None" instead of "NotSpecified" as the value for
mLastSmoothScrollOrigin when there is no smooth scroll in progress.

Depends on D78439

https://hg.mozilla.org/mozilla-central/rev/aa8fe25a8715
https://hg.mozilla.org/mozilla-central/rev/674cc15b2fba
https://hg.mozilla.org/mozilla-central/rev/cc8881b3128c