Bugzilla

It turns out my understanding of the situation was incomplete.

Not cancelling ongoing animations when starting a new wheel block does fix bug 1924193, but it does not fix the [testcase](https://bugzilla.mozilla.org/attachment.cgi?id=9445987) I attached in this bug (the scrolling in the testcase remains sluggish), which I discovered when I tried to turn the testcase into a mochitest.

I did some debugging to understand why. It turns out that, even if we don't cancel the ongoing smooth scroll animation from the previous scroll, correctly extending the destination of the existing animation by an additional `scrollBy` delta requires the delta to be communicated to APZ using a **relative** scroll update, and we don't actually have relative updates hooked up for `scrollBy` in the smooth scroll case. (The operation [calls](https://searchfox.org/mozilla-central/rev/d55e89d48a8053ce45a74b0ec92c0ff6a9dcc43d/layout/generic/ScrollContainerFrame.cpp#4973) `ScrollToWithOrigin()` with `ScrollOrigin::Relative`, but in the [smooth scroll case](https://searchfox.org/mozilla-central/rev/d55e89d48a8053ce45a74b0ec92c0ff6a9dcc43d/layout/generic/ScrollContainerFrame.cpp#2495) we use [`ScrollPositionUpdate::NewSmoothScroll`](https://searchfox.org/mozilla-central/rev/d55e89d48a8053ce45a74b0ec92c0ff6a9dcc43d/layout/generic/ScrollContainerFrame.cpp#7935) which hardcodes [`ScrollUpdateType::Absolute`](https://searchfox.org/mozilla-central/rev/d55e89d48a8053ce45a74b0ec92c0ff6a9dcc43d/layout/generic/ScrollPositionUpdate.cpp#70) regardless of origin.)

I then investigated why the vertical tab bar case does not suffer from this issue. The reason is that the arrowscrollbox.js code [does its own tracking of the destination](https://searchfox.org/mozilla-central/rev/d55e89d48a8053ce45a74b0ec92c0ff6a9dcc43d/toolkit/content/widgets/arrowscrollbox.js#705-713) which ensures the deltas are accumulated correctly even in the absence of relative updates.

Finally, I investigated why the arrowscrollbox.js code does suffer from the `CancelAnimation` issue even though it does its own tracking of the destination: the reason is that `CancelAnimation` causes a `scrollend` event to be sent which [resets](https://searchfox.org/mozilla-central/rev/d55e89d48a8053ce45a74b0ec92c0ff6a9dcc43d/toolkit/content/widgets/arrowscrollbox.js#759) the destination tracking.

Summary:

 * The `scrollend` events are indeed part of the cause of the problem in the arrowscrollbox.js case, not just a symptom.
 * The `CancelAnimation` fix is sufficient to get the arrowscrollbox.js code to work properly.
 * Additional fixes would be needed to get the [reduced testcase](https://bugzilla.mozilla.org/attachment.cgi?id=9445987) to work properly (but that's not as high priority since the remaining issues do not affect arrowscrollbox.js).

I will post the `CancelAnimation` fix for review along with a modified testcase which simulates the arrowscrollbox.js code more closely.

It turns out my understanding of the situation was incomplete.

Not cancelling ongoing animations when starting a new wheel block does fix bug 1924193, but it does not fix the [testcase](https://bugzilla.mozilla.org/attachment.cgi?id=9445987) I attached in this bug (the scrolling in the testcase remains sluggish), which I discovered when I tried to turn the testcase into a mochitest.

I did some debugging to understand why. It turns out that, even if we don't cancel the ongoing smooth scroll animation from the previous scroll, correctly extending the destination of the existing animation by an additional `scrollBy` delta requires the delta to be communicated to APZ using a **relative** scroll update, and we don't actually have relative updates hooked up for `scrollBy` in the smooth scroll case. (The operation [calls](https://searchfox.org/mozilla-central/rev/d55e89d48a8053ce45a74b0ec92c0ff6a9dcc43d/layout/generic/ScrollContainerFrame.cpp#4973) `ScrollToWithOrigin()` with `ScrollOrigin::Relative`, but in the [smooth scroll case](https://searchfox.org/mozilla-central/rev/d55e89d48a8053ce45a74b0ec92c0ff6a9dcc43d/layout/generic/ScrollContainerFrame.cpp#2495) we use [`ScrollPositionUpdate::NewSmoothScroll`](https://searchfox.org/mozilla-central/rev/d55e89d48a8053ce45a74b0ec92c0ff6a9dcc43d/layout/generic/ScrollContainerFrame.cpp#7935) which hardcodes [`ScrollUpdateType::Absolute`](https://searchfox.org/mozilla-central/rev/d55e89d48a8053ce45a74b0ec92c0ff6a9dcc43d/layout/generic/ScrollPositionUpdate.cpp#70) regardless of origin.)

I then investigated why the vertical tab bar case does not suffer from this issue. The reason is that the arrowscrollbox.js code [does its own tracking of the destination](https://searchfox.org/mozilla-central/rev/d55e89d48a8053ce45a74b0ec92c0ff6a9dcc43d/toolkit/content/widgets/arrowscrollbox.js#705-713) which ensures the deltas are accumulated correctly even in the absence of relative updates.

Finally, I investigated why the arrowscrollbox.js code does suffer from the `CancelAnimation` issue even though it does its own tracking of the destination: the reason is that `CancelAnimation` causes a `scrollend` event to be sent which [resets](https://searchfox.org/mozilla-central/rev/d55e89d48a8053ce45a74b0ec92c0ff6a9dcc43d/toolkit/content/widgets/arrowscrollbox.js#759) the destination tracking.

Summary:

 * The `scrollend` events are indeed part of the cause of the problem in the arrowscrollbox.js case, not just a symptom.
 * The `CancelAnimation` fix is sufficient to get the arrowscrollbox.js code to work properly.
 * Additional fixes would be needed to get the [reduced testcase](https://bugzilla.mozilla.org/attachment.cgi?id=9445987) (which does not do its own destination tracking) to work properly (but that's not as high priority since the remaining issues do not affect arrowscrollbox.js).

I will post the `CancelAnimation` fix for review along with a modified testcase which simulates the arrowscrollbox.js code more closely.