Bug 1022956 - When fling velocity is high, don't let tap gestures trigger clicks to content. r=drs, r=Bas, a=2.0+
authorKartikaya Gupta <kgupta@mozilla.com>
Thu, 26 Jun 2014 18:37:44 -0400
changeset 208517 8fb4f7972218d779541e0c81dd6b6aff109fbc58
parent 208516 17bc466733ddb8e212bfed26c6fca45af24d9e5d
child 208518 54208d905243160d23de3a10366191d89f9241b9
push id494
push userraliiev@mozilla.com
push dateMon, 25 Aug 2014 18:42:16 +0000
treeherdermozilla-release@a3cc3e46b571 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdrs, Bas, 2
bugs1022956
milestone32.0a2
Bug 1022956 - When fling velocity is high, don't let tap gestures trigger clicks to content. r=drs, r=Bas, a=2.0+
gfx/2d/BasePoint.h
gfx/layers/apz/src/AsyncPanZoomController.cpp
gfx/layers/apz/src/GestureEventListener.cpp
gfx/layers/apz/src/GestureEventListener.h
gfx/tests/gtest/TestAsyncPanZoomController.cpp
gfx/thebes/gfxPrefs.h
--- a/gfx/2d/BasePoint.h
+++ b/gfx/2d/BasePoint.h
@@ -61,16 +61,20 @@ struct BasePoint {
   Sub operator/(T aScale) const {
     return Sub(x / aScale, y / aScale);
   }
 
   Sub operator-() const {
     return Sub(-x, -y);
   }
 
+  T Length() const {
+    return hypot(x, y);
+  }
+
   // Round() is *not* rounding to nearest integer if the values are negative.
   // They are always rounding as floor(n + 0.5).
   // See https://bugzilla.mozilla.org/show_bug.cgi?id=410748#c14
   Sub& Round() {
     x = static_cast<T>(floor(x + 0.5));
     y = static_cast<T>(floor(y + 0.5));
     return *static_cast<Sub*>(this);
   }
--- a/gfx/layers/apz/src/AsyncPanZoomController.cpp
+++ b/gfx/layers/apz/src/AsyncPanZoomController.cpp
@@ -183,16 +183,24 @@ typedef GeckoContentController::APZState
  * "apz.fling_friction"
  * Amount of friction applied during flings.
  *
  * "apz.fling_repaint_interval"
  * Maximum amount of time flinging before sending a viewport change. This will
  * asynchronously repaint the page.
  * Units: milliseconds
  *
+ * "apz.fling_stop_on_tap_threshold"
+ * When flinging, if the velocity is above this number, then a tap on the
+ * screen will stop the fling without dispatching a tap to content. If the
+ * velocity is below this threshold a tap will also be dispatched.
+ * Note: when modifying this pref be sure to run the APZC gtests as some of
+ * them depend on the value of this pref.
+ * Units: screen pixels per millisecond
+ *
  * "apz.fling_stopped_threshold"
  * When flinging, if the velocity goes below this number, we just stop the
  * animation completely. This is to prevent asymptotically approaching 0
  * velocity and rerendering unnecessarily.
  * Units: screen pixels per millisecond
  *
  * "apz.max_velocity_inches_per_ms"
  * Maximum velocity.  Velocity will be capped at this value if a faster fling
@@ -782,27 +790,29 @@ nsEventStatus AsyncPanZoomController::Ha
 }
 
 nsEventStatus AsyncPanZoomController::OnTouchStart(const MultiTouchInput& aEvent) {
   APZC_LOG("%p got a touch-start in state %d\n", this, mState);
   mPanDirRestricted = false;
   ScreenIntPoint point = GetFirstTouchScreenPoint(aEvent);
 
   switch (mState) {
-    case ANIMATING_ZOOM:
-      // We just interrupted a double-tap animation, so force a redraw in case
-      // this touchstart is just a tap that doesn't end up triggering a redraw.
-      {
-        ReentrantMonitorAutoEnter lock(mMonitor);
-        RequestContentRepaint();
-        ScheduleComposite();
-        UpdateSharedCompositorFrameMetrics();
+    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 FLING:
+    case ANIMATING_ZOOM:
       CancelAnimation();
       // Fall through.
     case NOTHING: {
       mX.StartTouch(point.x, aEvent.mTime);
       mY.StartTouch(point.y, aEvent.mTime);
       APZCTreeManager* treeManagerLocal = mTreeManager;
       nsRefPtr<GeckoContentController> controller = GetGeckoContentController();
       if (treeManagerLocal && controller) {
@@ -1699,16 +1709,18 @@ void AsyncPanZoomController::StartAnimat
 void AsyncPanZoomController::CancelAnimation() {
   ReentrantMonitorAutoEnter lock(mMonitor);
   SetState(NOTHING);
   if (mAnimation) {
     mAnimation->Cancel();
     mAnimation = nullptr;
     // mAnimation->Cancel() may have done something that requires a repaint.
     RequestContentRepaint();
+    ScheduleComposite();
+    UpdateSharedCompositorFrameMetrics();
   }
 }
 
 void AsyncPanZoomController::SetCompositorParent(CompositorParent* aCompositorParent) {
   mCompositorParent = aCompositorParent;
 }
 
 void AsyncPanZoomController::ShareFrameMetricsAcrossProcesses() {
--- a/gfx/layers/apz/src/GestureEventListener.cpp
+++ b/gfx/layers/apz/src/GestureEventListener.cpp
@@ -8,16 +8,19 @@
 #include <math.h>                       // for fabsf
 #include <stddef.h>                     // for size_t
 #include "AsyncPanZoomController.h"     // for AsyncPanZoomController
 #include "base/task.h"                  // for CancelableTask, etc
 #include "gfxPrefs.h"                   // for gfxPrefs
 #include "nsDebug.h"                    // for NS_WARNING
 #include "nsMathUtils.h"                // for NS_hypot
 
+#define GEL_LOG(...)
+// #define GEL_LOG(...) printf_stderr("GEL: " __VA_ARGS__)
+
 namespace mozilla {
 namespace layers {
 
 /**
  * Maximum time for a touch on the screen and corresponding lift of the finger
  * to be considered a tap. This also applies to double taps, except that it is
  * used twice.
  */
@@ -56,16 +59,18 @@ GestureEventListener::GestureEventListen
 }
 
 GestureEventListener::~GestureEventListener()
 {
 }
 
 nsEventStatus GestureEventListener::HandleInputEvent(const MultiTouchInput& aEvent)
 {
+  GEL_LOG("Receiving event type %d with %d touches in state %d\n", aEvent.mType, aEvent.mTouches.Length(), mState);
+
   nsEventStatus rv = nsEventStatus_eIgnore;
 
   // Cache the current event since it may become the single or long tap that we
   // send.
   mLastTouchInput = aEvent;
 
   switch (aEvent.mType) {
   case MultiTouchInput::MULTITOUCH_START:
@@ -101,16 +106,33 @@ nsEventStatus GestureEventListener::Hand
     mTouches.Clear();
     rv = HandleInputTouchCancel();
     break;
   }
 
   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) {
     NS_WARNING("GetLastTouchIdentifier() called when last touch event "
                "did not have one touch");
   }
   return mTouches.IsEmpty() ? -1 : mTouches[0].mIdentifier;
 }
--- a/gfx/layers/apz/src/GestureEventListener.h
+++ b/gfx/layers/apz/src/GestureEventListener.h
@@ -49,16 +49,22 @@ public:
   /**
    * General input handler for a touch event. If the touch event is not a part
    * of a gesture, then we pass it along to AsyncPanZoomController. Otherwise,
    * it gets consumed here and never forwarded along.
    */
   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
    * event contained only one touch.
    */
   int32_t GetLastTouchIdentifier() const;
 
 private:
   // Private destructor, to discourage deletion outside of Release():
--- a/gfx/tests/gtest/TestAsyncPanZoomController.cpp
+++ b/gfx/tests/gtest/TestAsyncPanZoomController.cpp
@@ -92,16 +92,29 @@ public:
   // that object might be leaked. This is also why we don't
   // expose mTaskQueue to any users of MockContentControllerDelayed.
   void RunDelayedTask() {
     mTaskQueue[0]->Run();
     delete mTaskQueue[0];
     mTaskQueue.RemoveElementAt(0);
   }
 
+  // Run all the tasks in the queue, returning the number of tasks
+  // run. Note that if a task queues another task while running, that
+  // new task will not be run. Therefore, there may be still be tasks
+  // in the queue after this function is called. Only when the return
+  // value is 0 is the queue guaranteed to be empty.
+  int RunThroughDelayedTasks() {
+    int numTasks = mTaskQueue.Length();
+    for (int i = 0; i < numTasks; i++) {
+      RunDelayedTask();
+    }
+    return numTasks;
+  }
+
 private:
   nsTArray<Task*> mTaskQueue;
 };
 
 
 class TestAPZCContainerLayer : public ContainerLayer {
   public:
     TestAPZCContainerLayer()
@@ -808,16 +821,79 @@ TEST_F(AsyncPanZoomControllerTester, Fli
     apzc->SampleContentTransformForFrame(testStartTime+TimeDuration::FromMilliseconds(i), &viewTransformOut, pointOut);
     EXPECT_GT(pointOut.y, lastPoint.y);
     lastPoint = pointOut;
   }
 
   apzc->Destroy();
 }
 
+// Start a fling, and then tap while the fling is ongoing. When
+// aSlow is false, the tap will happen while the fling is at a
+// high velocity, and we check that the tap doesn't trigger sending a tap
+// to content. If aSlow is true, the tap will happen while the fling
+// is at a slow velocity, and we check that the tap does trigger sending
+// a tap to content. See bug 1022956.
+void
+DoFlingStopTest(bool aSlow) {
+  TimeStamp testStartTime = TimeStamp::Now();
+  AsyncPanZoomController::SetFrameTime(testStartTime);
+
+  nsRefPtr<MockContentControllerDelayed> mcc = new NiceMock<MockContentControllerDelayed>();
+  nsRefPtr<TestAPZCTreeManager> tm = new TestAPZCTreeManager();
+  nsRefPtr<TestAsyncPanZoomController> apzc = new TestAsyncPanZoomController(0, mcc, tm, AsyncPanZoomController::USE_GESTURE_DETECTOR);
+
+  apzc->SetFrameMetrics(TestFrameMetrics());
+  apzc->NotifyLayersUpdated(TestFrameMetrics(), true);
+
+  int time = 0;
+  int touchStart = 50;
+  int touchEnd = 10;
+
+  // Start the fling down.
+  ApzcPan(apzc, tm, time, touchStart, touchEnd);
+  // The touchstart from the pan will leave some cancelled tasks in the queue, clear them out
+  EXPECT_EQ(2, mcc->RunThroughDelayedTasks());
+
+  // If we want to tap while the fling is fast, let the fling advance for 10ms only. If we want
+  // the fling to slow down more, advance to 2000ms. These numbers may need adjusting if our
+  // friction and threshold values change, but they should be deterministic at least.
+  int timeDelta = aSlow ? 2000 : 10;
+  int tapCallsExpected = aSlow ? 1 : 0;
+  int delayedTasksExpected = aSlow ? 3 : 2;
+
+  // Advance the fling animation by timeDelta milliseconds.
+  ScreenPoint pointOut;
+  ViewTransform viewTransformOut;
+  apzc->SampleContentTransformForFrame(testStartTime + TimeDuration::FromMilliseconds(timeDelta), &viewTransformOut, pointOut);
+
+  // Deliver a tap to abort the fling. Ensure that we get a HandleSingleTap
+  // call out of it if and only if the fling is slow.
+  EXPECT_CALL(*mcc, HandleSingleTap(_, 0, apzc->GetGuid())).Times(tapCallsExpected);
+  ApzcTap(apzc, 10, 10, time, 0, nullptr);
+  EXPECT_EQ(delayedTasksExpected, mcc->RunThroughDelayedTasks());
+
+  // Verify that we didn't advance any further after the fling was aborted, in either case.
+  ScreenPoint finalPointOut;
+  apzc->SampleContentTransformForFrame(testStartTime + TimeDuration::FromMilliseconds(timeDelta + 1000), &viewTransformOut, finalPointOut);
+  EXPECT_EQ(pointOut.x, finalPointOut.x);
+  EXPECT_EQ(pointOut.y, finalPointOut.y);
+
+  apzc->AssertStateIsReset();
+  apzc->Destroy();
+}
+
+TEST_F(AsyncPanZoomControllerTester, FlingStop) {
+  DoFlingStopTest(false);
+}
+
+TEST_F(AsyncPanZoomControllerTester, FlingStopTap) {
+  DoFlingStopTest(true);
+}
+
 TEST_F(AsyncPanZoomControllerTester, OverScrollPanning) {
   TimeStamp testStartTime = TimeStamp::Now();
   AsyncPanZoomController::SetFrameTime(testStartTime);
 
   nsRefPtr<MockContentController> mcc = new NiceMock<MockContentController>();
   nsRefPtr<TestAPZCTreeManager> tm = new TestAPZCTreeManager();
   nsRefPtr<TestAsyncPanZoomController> apzc = new TestAsyncPanZoomController(0, mcc, tm);
 
--- a/gfx/thebes/gfxPrefs.h
+++ b/gfx/thebes/gfxPrefs.h
@@ -111,16 +111,17 @@ private:
   DECL_GFX_PREF(Live, "apz.content_response_timeout",          APZContentResponseTimeout, int32_t, 300);
   DECL_GFX_PREF(Live, "apz.cross_slide.enabled",               APZCrossSlideEnabled, bool, false);
   DECL_GFX_PREF(Live, "apz.enlarge_displayport_when_clipped",  APZEnlargeDisplayPortWhenClipped, bool, false);
   DECL_GFX_PREF(Live, "apz.fling_accel_interval_ms",           APZFlingAccelInterval, int32_t, 500);
   DECL_GFX_PREF(Live, "apz.fling_accel_base_mult",             APZFlingAccelBaseMultiplier, float, 1.0f);
   DECL_GFX_PREF(Live, "apz.fling_accel_supplemental_mult",     APZFlingAccelSupplementalMultiplier, float, 1.0f);
   DECL_GFX_PREF(Once, "apz.fling_friction",                    APZFlingFriction, float, 0.002f);
   DECL_GFX_PREF(Live, "apz.fling_repaint_interval",            APZFlingRepaintInterval, int32_t, 75);
+  DECL_GFX_PREF(Once, "apz.fling_stop_on_tap_threshold",       APZFlingStopOnTapThreshold, float, 0.05f);
   DECL_GFX_PREF(Once, "apz.fling_stopped_threshold",           APZFlingStoppedThreshold, float, 0.01f);
   DECL_GFX_PREF(Once, "apz.max_velocity_inches_per_ms",        APZMaxVelocity, float, -1.0f);
   DECL_GFX_PREF(Once, "apz.max_velocity_queue_size",           APZMaxVelocityQueueSize, uint32_t, 5);
   DECL_GFX_PREF(Live, "apz.min_skate_speed",                   APZMinSkateSpeed, float, 1.0f);
   DECL_GFX_PREF(Live, "apz.num_paint_duration_samples",        APZNumPaintDurationSamples, int32_t, 3);
   DECL_GFX_PREF(Live, "apz.overscroll.enabled",                APZOverscrollEnabled, bool, false);
   DECL_GFX_PREF(Live, "apz.overscroll.fling_friction",         APZOverscrollFlingFriction, float, 0.02f);
   DECL_GFX_PREF(Live, "apz.overscroll.fling_stopped_threshold", APZOverscrollFlingStoppedThreshold, float, 0.4f);