commit 77d5e074eecb55cdb3d568a284f65913add22ee2
parent ef292b5ecbe5cec8aadeb61688b490871a34b912
Author: Botond Ballo <botond@mozilla.com>
Date: Wed, 8 Oct 2025 21:54:49 +0000
Bug 1988043 - Use GetFrameTime() rather than the event timestamp when updating the destination of a keyboard scroll animation. r=hiro
This makes the handling of keyboard scroll animations consistent
with wheel scroll animations, for which a similar change was made
in bug 1926830.
Differential Revision: https://phabricator.services.mozilla.com/D266726
Diffstat:
3 files changed, 77 insertions(+), 6 deletions(-)
diff --git a/gfx/layers/apz/src/AsyncPanZoomController.cpp b/gfx/layers/apz/src/AsyncPanZoomController.cpp
@@ -2253,7 +2253,7 @@ nsEventStatus AsyncPanZoomController::OnKeyboard(const KeyboardInput& aEvent) {
SmoothScrollAnimation* animation = mAnimation->AsSmoothScrollAnimation();
MOZ_ASSERT(animation);
- animation->UpdateDestination(aEvent.mTimeStamp,
+ animation->UpdateDestination(GetFrameTime().Time(),
CSSPixel::ToAppUnits(destination),
nsSize(velocity.x, velocity.y));
diff --git a/gfx/layers/apz/test/gtest/APZTestCommon.h b/gfx/layers/apz/test/gtest/APZTestCommon.h
@@ -400,6 +400,11 @@ class TestAsyncPanZoomController : public AsyncPanZoomController {
EXPECT_TRUE(InScrollAnimation(ScrollAnimationKind::Wheel));
}
+ void AssertInKeyboardScroll() {
+ RecursiveMutexAutoLock lock(mRecursiveMutex);
+ EXPECT_TRUE(InScrollAnimation(ScrollAnimationKind::Keyboard));
+ }
+
void AssertStateIsAutoscroll() {
RecursiveMutexAutoLock lock(mRecursiveMutex);
EXPECT_EQ(AUTOSCROLL, mState);
diff --git a/gfx/layers/apz/test/gtest/TestBasic.cpp b/gfx/layers/apz/test/gtest/TestBasic.cpp
@@ -9,6 +9,7 @@
#include "InputUtils.h"
#include "mozilla/ScrollPositionUpdate.h"
+#include "mozilla/layers/ScrollableLayerGuid.h"
TEST_F(APZCBasicTester, Overzoom) {
// the visible area of the document in CSS pixels is x=10 y=0 w=100 h=100
@@ -622,7 +623,7 @@ class APZCSmoothScrollTester : public APZCBasicTester {
// Test that receiving a wheel event with a timestamp far in the future does
// not cause scrolling to get stuck.
- void TestEventWithFutureStamp() {
+ void TestWheelEventWithFutureStamp() {
// Set up scroll frame. Starting scroll position is (0, 0).
ScrollMetadata metadata;
FrameMetrics& metrics = metadata.GetMetrics();
@@ -665,6 +666,59 @@ class APZCSmoothScrollTester : public APZCBasicTester {
// Clean up by letting the animation run until completion.
apzc->AdvanceAnimationsUntilEnd();
}
+
+ // Test that receiving a key event with a timestamp far in the future does
+ // not cause scrolling to get stuck.
+ void TestKeyEventWithFutureStamp() {
+ // Set up scroll frame. Starting scroll position is (0, 0).
+ ScrollMetadata metadata;
+ FrameMetrics& metrics = metadata.GetMetrics();
+ metrics.SetScrollableRect(CSSRect(0, 0, 1000, 10000));
+ metrics.SetLayoutViewport(CSSRect(0, 0, 1000, 1000));
+ metrics.SetZoom(CSSToParentLayerScale(1.0));
+ metrics.SetCompositionBounds(ParentLayerRect(0, 0, 1000, 1000));
+ metrics.SetVisualScrollOffset(CSSPoint(0, 0));
+ metrics.SetScrollId(ScrollableLayerGuid::START_SCROLL_ID);
+ metrics.SetIsRootContent(true);
+ // Set the line scroll amount to 100 pixels. The key event we send
+ // will scroll by a multiple of this amount.
+ metadata.SetLineScrollAmount({100, 100});
+ apzc->SetScrollMetadata(metadata);
+
+ // Note that, since we are sending the key event to the APZC instance
+ // directly, we don't need to set up a keyboard map or focus state.
+
+ // Send a key event to trigger smooth scrolling by a few lines (the number
+ // of lines is determined by toolkit.scrollbox.verticalScrollDistance).
+ WidgetKeyboardEvent keyEvent(true, eKeyDown, nullptr);
+ // Give the key event a timestamp "far" (here, 1 minute) into the future.
+ // This simulates a scenario, observed in bug 1926830, where a bug in the
+ // system toolkit or widget layers causes something to introduce a skew into
+ // the timestamps received from widget code.
+ TimeStamp futureTimeStamp = mcc->Time() + TimeDuration::FromSeconds(60);
+ keyEvent.mTimeStamp = futureTimeStamp;
+ KeyboardInput keyInput(keyEvent);
+ // The KeyboardScrollAction needs to be specified on the event explicitly,
+ // since the mapping from eKeyDown to it happens in APZCTreeManager which
+ // we are bypassing here.
+ keyInput.mAction = {KeyboardScrollAction::eScrollLine, /*aForward=*/true};
+ Unused << apzc->ReceiveInputEvent(keyInput);
+ apzc->AssertInKeyboardScroll();
+
+ // Sample the animation 10 frames (a shorter overall duration than the
+ // timestamp skew).
+ for (int i = 0; i < 10; ++i) {
+ SampleAnimationOneFrame();
+ }
+
+ // Assert that we have scrolled. Without a mitigation in place for the
+ // timestamp skew, we may wait for the frame (vsync) time to catch up with
+ // the event's timestamp before doing any scrolling.
+ ASSERT_GT(apzc->GetFrameMetrics().GetVisualScrollOffset().y, 0);
+
+ // Clean up by letting the animation run until completion.
+ apzc->AdvanceAnimationsUntilEnd();
+ }
};
TEST_F(APZCSmoothScrollTester, ContentShiftBezier) {
@@ -703,16 +757,28 @@ TEST_F(APZCSmoothScrollTester, ContentShiftDoesNotCauseOvershootMsd) {
TestContentShiftDoesNotCauseOvershoot();
}
-TEST_F(APZCSmoothScrollTester, FutureTimestampBezier) {
+TEST_F(APZCSmoothScrollTester, FutureWheelTimestampBezier) {
+ SCOPED_GFX_PREF_BOOL("general.smoothScroll", true);
+ SCOPED_GFX_PREF_BOOL("general.smoothScroll.msdPhysics.enabled", false);
+ TestWheelEventWithFutureStamp();
+}
+
+TEST_F(APZCSmoothScrollTester, FutureWheelTimestampMsd) {
+ SCOPED_GFX_PREF_BOOL("general.smoothScroll", true);
+ SCOPED_GFX_PREF_BOOL("general.smoothScroll.msdPhysics.enabled", true);
+ TestWheelEventWithFutureStamp();
+}
+
+TEST_F(APZCSmoothScrollTester, FutureKeyTimestampBezier) {
SCOPED_GFX_PREF_BOOL("general.smoothScroll", true);
SCOPED_GFX_PREF_BOOL("general.smoothScroll.msdPhysics.enabled", false);
- TestEventWithFutureStamp();
+ TestKeyEventWithFutureStamp();
}
-TEST_F(APZCSmoothScrollTester, FutureTimestampMsd) {
+TEST_F(APZCSmoothScrollTester, FutureKeyTimestampMsd) {
SCOPED_GFX_PREF_BOOL("general.smoothScroll", true);
SCOPED_GFX_PREF_BOOL("general.smoothScroll.msdPhysics.enabled", true);
- TestEventWithFutureStamp();
+ TestKeyEventWithFutureStamp();
}
TEST_F(APZCBasicTester, ZoomAndScrollableRectChangeAfterZoomChange) {
