Bug 1039979 - Restore and improve handling of the stop-fast-fling-on-touchdown behaviour. r=botond

gfx/layers/apz/src/AsyncPanZoomController.cpp

@@ -836,13 +836,26 @@ nsEventStatus AsyncPanZoomController::ReceiveInputEvent(const InputData& aEvent)
   if (aEvent.AsMultiTouchInput().mType == MultiTouchInput::MULTITOUCH_START) {
     block = StartNewTouchBlock(false);
     APZC_LOG("%p started new touch block %p\n", this, block);
+
+    // We want to cancel animations here as soon as possible (i.e. without waiting for
+    // content responses) because a finger has gone down and we don't want to keep moving
+    // the content under the finger. However, to prevent "future" touchstart events from
+    // interfering with "past" animations (i.e. from a previous touch block that is still
+    // being processed) we only do this animation-cancellation if there are no older
+    // touch blocks still in the queue.
+    if (block == CurrentTouchBlock()) {
+      if (GetVelocityVector().Length() > gfxPrefs::APZFlingStopOnTapThreshold()) {
+        // If we're already in a fast fling, then we want the touch event to stop the fling
+        // and to disallow the touch event from being used as part of a fling.
+        block->DisallowSingleTap();
+      }
+      CancelAnimationForHandoffChain();
+    }
+
     if (mFrameMetrics.mMayHaveTouchListeners || mFrameMetrics.mMayHaveTouchCaret) {
       // Content may intercept the touch events and prevent-default them. So we schedule
       // a timeout to give content time to do that.
       ScheduleContentResponseTimeout();
-      // However, we still want to cancel animations here because a finger has gone down
-      // and we don't want to keep moving the content under the finger.
-      CancelAnimationForHandoffChain();
     } else {
       // Content won't prevent-default this, so we can just pretend like we scheduled
       // a timeout and it expired. Note that we will still receive a ContentReceivedTouch
@@ -967,17 +980,6 @@ nsEventStatus AsyncPanZoomController::OnTouchStart(const MultiTouchInput& aEvent
 
   switch (mState) {
     case FLING:
-      if (GetVelocityVector().Length() > gfxPrefs::APZFlingStopOnTapThreshold()) {
-        // This is ugly. Hopefully bug 1009733 can reorganize how events
-        // flow through APZC and change it so that events are handed to the
-        // gesture listener *after* we deal with them here. This should allow
-        // removal of this ugly code.
-        nsRefPtr<GestureEventListener> listener = GetGestureEventListener();
-        if (listener) {
-          listener->CancelSingleTouchDown();
-        }
-      }
-      // Fall through.
     case ANIMATING_ZOOM:
       CancelAnimationForHandoffChain();
       // Fall through.
@@ -1422,7 +1424,9 @@ nsEventStatus AsyncPanZoomController::GenerateSingleTap(const ScreenIntPoint& aP
   if (controller) {
     CSSPoint geckoScreenPoint;
     if (ConvertToGecko(aPoint, &geckoScreenPoint)) {
-      int32_t modifiers = WidgetModifiersToDOMModifiers(aModifiers);
+      if (!CurrentTouchBlock()->SetSingleTapOccurred()) {
+        return nsEventStatus_eIgnore;
+      }
       // Because this may be being running as part of APZCTreeManager::ReceiveInputEvent,
       // calling controller->HandleSingleTap directly might mean that content receives
       // the single tap message before the corresponding touch-up. To avoid that we
@@ -1430,9 +1434,9 @@ nsEventStatus AsyncPanZoomController::GenerateSingleTap(const ScreenIntPoint& aP
       // See bug 965381 for the issue this was causing.
       controller->PostDelayedTask(
         NewRunnableMethod(controller.get(), &GeckoContentController::HandleSingleTap,
-                          geckoScreenPoint, modifiers, GetGuid()),
+                          geckoScreenPoint, WidgetModifiersToDOMModifiers(aModifiers),
+                          GetGuid()),
         0);
-      CurrentTouchBlock()->SetSingleTapOccurred();
       return nsEventStatus_eConsumeNoDefault;
     }
   }
@@ -1795,6 +1799,10 @@ void AsyncPanZoomController::CancelAnimation() {
   APZC_LOG("%p running CancelAnimation in state %d\n", this, mState);
   SetState(NOTHING);
   mAnimation = nullptr;
+  // Since there is no animation in progress now the axes should
+  // have no velocity either.
+  mX.SetVelocity(0);
+  mY.SetVelocity(0);
   // Setting the state to nothing and cancelling the animation can
   // preempt normal mechanisms for relieving overscroll, so we need to clear
   // overscroll here.

gfx/layers/apz/src/GestureEventListener.cpp

@@ -109,23 +109,6 @@ nsEventStatus GestureEventListener::HandleInputEvent(const MultiTouchInput& aEve
   return rv;
 }
 
-void GestureEventListener::CancelSingleTouchDown()
-{
-  GEL_LOG("Cancelling touch-down while in state %d\n", mState);
-
-  switch (mState) {
-  case GESTURE_FIRST_SINGLE_TOUCH_DOWN:
-    CancelLongTapTimeoutTask();
-    CancelMaxTapTimeoutTask();
-    SetState(GESTURE_NONE);
-    break;
-  default:
-    NS_WARNING("IgnoreLastTouchStart() called while in unexpected state");
-    SetState(GESTURE_NONE);
-    break;
-  }
-}
-
 int32_t GestureEventListener::GetLastTouchIdentifier() const
 {
   if (mTouches.Length() != 1) {

gfx/layers/apz/src/GestureEventListener.h

@@ -53,12 +53,6 @@ public:
    */
   nsEventStatus HandleInputEvent(const MultiTouchInput& aEvent);
 
-  /**
-   * Cancels any tap-related timeouts and clears any state that was set because
-   * we recently processed a touch-start.
-   */
-  void CancelSingleTouchDown();
-
   /**
    * Returns the identifier of the touch in the last touch event processed by
    * this GestureEventListener. This should only be called when the last touch

gfx/layers/apz/src/TouchBlockState.cpp

@@ -28,6 +28,7 @@ TouchBlockState::TouchBlockState()
   , mPreventDefault(false)
   , mContentResponded(false)
   , mContentResponseTimerExpired(false)
+  , mSingleTapDisallowed(false)
   , mSingleTapOccurred(false)
 {
   TBS_LOG("Creating %p\n", this);
@@ -104,10 +105,21 @@ TouchBlockState::IsDefaultPrevented() const
 }
 
 void
+TouchBlockState::DisallowSingleTap()
+{
+  TBS_LOG("%p disallowing single-tap\n", this);
+  mSingleTapDisallowed = true;
+}
+
+bool
 TouchBlockState::SetSingleTapOccurred()
 {
-  TBS_LOG("%p setting single-tap occurred\n", this);
+  TBS_LOG("%p attempting to set single-tap occurred; disallowed=%d\n", this, mSingleTapDisallowed);
-  mSingleTapOccurred = true;
+  if (!mSingleTapDisallowed) {
+    mSingleTapOccurred = true;
+    return true;
+  }
+  return false;
 }
 
 bool

gfx/layers/apz/src/TouchBlockState.h

@@ -85,9 +85,14 @@ public:
   bool IsDefaultPrevented() const;
 
   /**
-   * Set a flag that indicates that this touch block triggered a single tap event.
+   * Set a flag that disables setting the single-tap flag on this block.
    */
-  void SetSingleTapOccurred();
+  void DisallowSingleTap();
+  /**
+   * Set a flag that indicates that this touch block triggered a single tap event.
+   * @return true iff DisallowSingleTap was not previously called.
+   */
+  bool SetSingleTapOccurred();
   /**
    * @return true iff SetSingleTapOccurred was previously called on this block.
    */
@@ -135,6 +140,7 @@ private:
   bool mPreventDefault;
   bool mContentResponded;
   bool mContentResponseTimerExpired;
+  bool mSingleTapDisallowed;
   bool mSingleTapOccurred;
   nsTArray<MultiTouchInput> mEvents;
 };

gfx/tests/gtest/TestAsyncPanZoomController.cpp

@@ -379,6 +379,16 @@ ApzcPanAndCheckStatus(AsyncPanZoomController* aApzc,
   EXPECT_EQ(touchMoveStatus, statuses[2]);
 }
 
+static void
+ApzcPanNoFling(AsyncPanZoomController* aApzc,
+               int& aTime,
+               int aTouchStartY,
+               int aTouchEndY)
+{
+  ApzcPan(aApzc, aTime, aTouchStartY, aTouchEndY);
+  aApzc->CancelAnimation();
+}
+
 static void
 ApzcPinchWithPinchInput(AsyncPanZoomController* aApzc,
                         int aFocusX, int aFocusY, float aScale,
@@ -808,6 +818,10 @@ protected:
       EXPECT_EQ(ViewTransform(), viewTransformOut);
     }
 
+    // Clear the fling from the previous pan, or stopping it will
+    // consume the next touchstart
+    apzc->CancelAnimation();
+
     // Pan back
     ApzcPanAndCheckStatus(apzc, time, touchEnd, touchStart, !aShouldTriggerScroll, false, &allowedTouchBehaviors);
     apzc->SampleContentTransformForFrame(testStartTime, &viewTransformOut, pointOut);
@@ -1660,7 +1674,7 @@ TEST_F(APZCTreeManagerTester, HitTesting2) {
   // Since this paint request is in the queue to Gecko, transformToGecko will
   // take it into account.
   manager->BuildOverscrollHandoffChain(apzcroot);
-  ApzcPan(apzcroot, time, 100, 50);
+  ApzcPanNoFling(apzcroot, time, 100, 50);
   manager->ClearOverscrollHandoffChain();
 
   // Hit where layers[3] used to be. It should now hit the root.
@@ -1688,7 +1702,7 @@ TEST_F(APZCTreeManagerTester, HitTesting2) {
   // one yet. Now we have an async transform on top of the pending paint request
   // transform.
   manager->BuildOverscrollHandoffChain(apzcroot);
-  ApzcPan(apzcroot, time, 100, 50);
+  ApzcPanNoFling(apzcroot, time, 100, 50);
   manager->ClearOverscrollHandoffChain();
 
   // Hit where layers[3] used to be. It should now hit the root.