Bug 1443231 - Ensure we are in the NOTHING state after a pinch gesture, if both fingers are lifted and no animation is triggered. r=kats
MozReview-Commit-ID: HexeLTlfbuq
--- a/gfx/layers/apz/src/AsyncPanZoomController.cpp
+++ b/gfx/layers/apz/src/AsyncPanZoomController.cpp
@@ -1560,16 +1560,24 @@ nsEventStatus AsyncPanZoomController::On
SetState(TOUCHING);
} else {
// If zooming isn't allowed, StartTouch() was already called
// in OnScaleBegin().
StartPanning(aEvent.mLocalFocusPoint);
}
} else {
// Otherwise, handle the fingers being lifted.
+
+ // Some of the code paths below, like ScrollSnap() or HandleEndOfPan(),
+ // may start an animation, but otherwise we want to end up in the NOTHING
+ // state. To avoid state change notification churn, we use a
+ // notification blocker.
+ StateChangeNotificationBlocker blocker(this);
+ SetState(NOTHING);
+
if (mZoomConstraints.mAllowZoom) {
RecursiveMutexAutoLock lock(mRecursiveMutex);
// We can get into a situation where we are overscrolled at the end of a
// pinch if we go into overscroll with a two-finger pan, and then turn
// that into a pinch by increasing the span sufficiently. In such a case,
// there is no snap-back animation to get us out of overscroll, so we need
// to get out of it somehow.
--- a/gfx/layers/apz/test/gtest/APZTestCommon.h
+++ b/gfx/layers/apz/test/gtest/APZTestCommon.h
@@ -260,16 +260,21 @@ public:
EXPECT_EQ(NOTHING, mState);
}
void AssertStateIsFling() const {
RecursiveMutexAutoLock lock(mRecursiveMutex);
EXPECT_EQ(FLING, mState);
}
+ void AssertStateIsSmoothScroll() const {
+ RecursiveMutexAutoLock lock(mRecursiveMutex);
+ EXPECT_EQ(SMOOTH_SCROLL, mState);
+ }
+
void AssertNotAxisLocked() const {
RecursiveMutexAutoLock lock(mRecursiveMutex);
EXPECT_EQ(PANNING, mState);
}
void AssertAxisLocked(ScrollDirection aDirection) const {
RecursiveMutexAutoLock lock(mRecursiveMutex);
switch (aDirection) {
--- a/gfx/layers/apz/test/gtest/TestPinching.cpp
+++ b/gfx/layers/apz/test/gtest/TestPinching.cpp
@@ -46,16 +46,18 @@ protected:
if (mGestureBehavior == AsyncPanZoomController::USE_GESTURE_DETECTOR) {
PinchWithTouchInputAndCheckStatus(apzc, ScreenIntPoint(250, 300), 1.25,
touchInputId, aShouldTriggerPinch, aAllowedTouchBehaviors);
} else {
PinchWithPinchInputAndCheckStatus(apzc, ScreenIntPoint(250, 300), 1.25,
aShouldTriggerPinch);
}
+ apzc->AssertStateIsReset();
+
FrameMetrics fm = apzc->GetFrameMetrics();
if (aShouldTriggerPinch) {
// the visible area of the document in CSS pixels is now x=305 y=310 w=40 h=80
EXPECT_EQ(2.5f, fm.GetZoom().ToScaleFactor().scale);
EXPECT_EQ(305, fm.GetScrollOffset().x);
EXPECT_EQ(310, fm.GetScrollOffset().y);
} else {
@@ -76,16 +78,18 @@ protected:
if (mGestureBehavior == AsyncPanZoomController::USE_GESTURE_DETECTOR) {
PinchWithTouchInputAndCheckStatus(apzc, ScreenIntPoint(250, 300), 0.5,
touchInputId, aShouldTriggerPinch, aAllowedTouchBehaviors);
} else {
PinchWithPinchInputAndCheckStatus(apzc, ScreenIntPoint(250, 300), 0.5,
aShouldTriggerPinch);
}
+ apzc->AssertStateIsReset();
+
fm = apzc->GetFrameMetrics();
if (aShouldTriggerPinch) {
// the visible area of the document in CSS pixels is now x=880 y=0 w=100 h=200
EXPECT_EQ(1.0f, fm.GetZoom().ToScaleFactor().scale);
EXPECT_EQ(880, fm.GetScrollOffset().x);
EXPECT_EQ(0, fm.GetScrollOffset().y);
} else {
--- a/gfx/layers/apz/test/gtest/TestSnapping.cpp
+++ b/gfx/layers/apz/test/gtest/TestSnapping.cpp
@@ -60,8 +60,41 @@ TEST_F(APZCSnappingTester, Bug1265510)
inner->AdvanceAnimationsUntilEnd();
EXPECT_LT(0.0f, inner->GetCurrentAsyncScrollOffset(AsyncPanZoomController::AsyncTransformConsumer::eForHitTesting).y);
// However, the outer frame should also continue to the snap point, otherwise
// it is demonstrating incorrect behaviour by violating the mandatory snapping.
outer->AdvanceAnimationsUntilEnd();
EXPECT_EQ(100.0f, outer->GetCurrentAsyncScrollOffset(AsyncPanZoomController::AsyncTransformConsumer::eForHitTesting).y);
}
+
+TEST_F(APZCSnappingTester, Snap_After_Pinch)
+{
+ SCOPED_GFX_PREF(WebRenderHitTest, bool, false);
+
+ const char* layerTreeSyntax = "c";
+ nsIntRegion layerVisibleRegion[] = {
+ nsIntRegion(IntRect(0, 0, 100, 100)),
+ };
+ root = CreateLayerTree(layerTreeSyntax, layerVisibleRegion, nullptr, lm, layers);
+ SetScrollableFrameMetrics(root, FrameMetrics::START_SCROLL_ID, CSSRect(0, 0, 100, 200));
+
+ // Set up some basic scroll snapping
+ ScrollSnapInfo snap;
+ snap.mScrollSnapTypeY = NS_STYLE_SCROLL_SNAP_TYPE_MANDATORY;
+ snap.mScrollSnapIntervalY = Some(100 * AppUnitsPerCSSPixel());
+
+ ScrollMetadata metadata = root->GetScrollMetadata(0);
+ metadata.SetSnapInfo(ScrollSnapInfo(snap));
+ root->SetScrollMetadata(metadata);
+
+ UniquePtr<ScopedLayerTreeRegistration> registration = MakeUnique<ScopedLayerTreeRegistration>(manager, 0, root, mcc);
+ manager->UpdateHitTestingTree(0, root, false, 0, 0);
+
+ RefPtr<TestAsyncPanZoomController> apzc = ApzcOf(root);
+
+ // Allow zooming
+ apzc->UpdateZoomConstraints(ZoomConstraints(true, true, CSSToParentLayerScale(0.25f), CSSToParentLayerScale(4.0f)));
+
+ PinchWithPinchInput(apzc, ScreenIntPoint(50, 50), ScreenIntPoint(50, 50), 1.2);
+
+ apzc->AssertStateIsSmoothScroll();
+}
\ No newline at end of file