Bug 1034376 - Avoid APZC being stuck in overscrolled state when CancelAnimation() is called during panning. r=kats
authorBotond Ballo <botond@mozilla.com>
Mon, 14 Jul 2014 12:54:41 -0400
changeset 215847 40522c4512d0aa6794dd73a4ced5245c0f906cfc
parent 215846 058b21302b632b1ee54b089f871f8a800bd6672a
child 215848 f32dd267971e261704d3cd9a61edb9dd2cfc79b7
push id515
push userraliiev@mozilla.com
push dateMon, 06 Oct 2014 12:51:51 +0000
treeherdermozilla-release@267c7a481bef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskats
bugs1034376
milestone33.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1034376 - Avoid APZC being stuck in overscrolled state when CancelAnimation() is called during panning. r=kats
gfx/layers/apz/src/AsyncPanZoomController.cpp
gfx/layers/apz/src/AsyncPanZoomController.h
gfx/tests/gtest/TestAsyncPanZoomController.cpp
--- a/gfx/layers/apz/src/AsyncPanZoomController.cpp
+++ b/gfx/layers/apz/src/AsyncPanZoomController.cpp
@@ -407,44 +407,23 @@ static uint32_t sAsyncPanZoomControllerC
 static TimeStamp
 GetFrameTime() {
   if (sFrameTime.IsNull()) {
     return TimeStamp::Now();
   }
   return sFrameTime;
 }
 
-// This is a base class for animations that can deal with
-// overscroll states. In particular, it ensures that overscroll
-// states are cleared when the animation is cancelled.
-class OverscrollableAnimation: public AsyncPanZoomAnimation {
-public:
-  OverscrollableAnimation(AsyncPanZoomController& aApzc,
-                          const TimeDuration& aRepaintInterval = TimeDuration::Forever())
-    : AsyncPanZoomAnimation(aRepaintInterval)
-    , mApzc(aApzc)
-  {
-  }
-
-  virtual void Cancel() MOZ_OVERRIDE
-  {
-    mApzc.mX.ClearOverscroll();
-    mApzc.mY.ClearOverscroll();
-  }
-
-protected:
-  AsyncPanZoomController& mApzc;
-};
-
-class FlingAnimation: public OverscrollableAnimation {
+class FlingAnimation: public AsyncPanZoomAnimation {
 public:
   FlingAnimation(AsyncPanZoomController& aApzc,
                  bool aApplyAcceleration,
                  bool aAllowOverscroll)
-    : OverscrollableAnimation(aApzc, TimeDuration::FromMilliseconds(gfxPrefs::APZFlingRepaintInterval()))
+    : AsyncPanZoomAnimation(TimeDuration::FromMilliseconds(gfxPrefs::APZFlingRepaintInterval()))
+    , mApzc(aApzc)
     , mAllowOverscroll(aAllowOverscroll)
   {
     TimeStamp now = GetFrameTime();
     ScreenPoint velocity(mApzc.mX.GetVelocity(), mApzc.mY.GetVelocity());
 
     // If the last fling was very recent and in the same direction as this one,
     // boost the velocity to be the sum of the two. Check separate axes separately
     // because we could have two vertical flings with small horizontal components
@@ -589,16 +568,17 @@ private:
   }
 
   static float Accelerate(float aBase, float aSupplemental)
   {
     return (aBase * gfxPrefs::APZFlingAccelBaseMultiplier())
          + (aSupplemental * gfxPrefs::APZFlingAccelSupplementalMultiplier());
   }
 
+  AsyncPanZoomController& mApzc;
   bool mAllowOverscroll;
 };
 
 class ZoomAnimation: public AsyncPanZoomAnimation {
 public:
   ZoomAnimation(CSSPoint aStartOffset, CSSToScreenScale aStartZoom,
                 CSSPoint aEndOffset, CSSToScreenScale aEndZoom)
     : mTotalDuration(TimeDuration::FromMilliseconds(gfxPrefs::APZZoomAnimationDuration()))
@@ -651,31 +631,33 @@ private:
 
   // Target metrics for a zoom to animation. This is only valid when we are in
   // the "ANIMATED_ZOOM" state. We only use the |mViewportScrollOffset| and
   // |mResolution| fields on this.
   CSSPoint mEndOffset;
   CSSToScreenScale mEndZoom;
 };
 
-class OverscrollSnapBackAnimation: public OverscrollableAnimation {
+class OverscrollSnapBackAnimation: public AsyncPanZoomAnimation {
 public:
   OverscrollSnapBackAnimation(AsyncPanZoomController& aApzc)
-    : OverscrollableAnimation(aApzc)
+    : mApzc(aApzc)
   {
   }
 
   virtual bool Sample(FrameMetrics& aFrameMetrics,
                       const TimeDuration& aDelta) MOZ_OVERRIDE
   {
     // Can't inline these variables due to short-circuit evaluation.
     bool continueX = mApzc.mX.SampleSnapBack(aDelta);
     bool continueY = mApzc.mY.SampleSnapBack(aDelta);
     return continueX || continueY;
   }
+private:
+  AsyncPanZoomController& mApzc;
 };
 
 void
 AsyncPanZoomController::SetFrameTime(const TimeStamp& aTime) {
   sFrameTime = aTime;
 }
 
 void
@@ -1766,20 +1748,23 @@ void AsyncPanZoomController::StartAnimat
   mLastSampleTime = GetFrameTime();
   ScheduleComposite();
 }
 
 void AsyncPanZoomController::CancelAnimation() {
   ReentrantMonitorAutoEnter lock(mMonitor);
   APZC_LOG("%p running CancelAnimation in state %d\n", this, mState);
   SetState(NOTHING);
-  if (mAnimation) {
-    mAnimation->Cancel();
-    mAnimation = nullptr;
-    // mAnimation->Cancel() may have done something that requires a repaint.
+  mAnimation = nullptr;
+  // Setting the state to nothing and cancelling the animation can
+  // preempt normal mechanisms for relieving overscroll, so we need to clear
+  // overscroll here.
+  if (mX.IsOverscrolled() || mY.IsOverscrolled()) {
+    mX.ClearOverscroll();
+    mY.ClearOverscroll();
     RequestContentRepaint();
     ScheduleComposite();
     UpdateSharedCompositorFrameMetrics();
   }
 }
 
 void AsyncPanZoomController::SetCompositorParent(CompositorParent* aCompositorParent) {
   mCompositorParent = aCompositorParent;
--- a/gfx/layers/apz/src/AsyncPanZoomController.h
+++ b/gfx/layers/apz/src/AsyncPanZoomController.h
@@ -795,17 +795,16 @@ public:
    * during overscroll handoff for a fling. If we are not pannable, calls
    * mTreeManager->HandOffFling() to hand the fling off further.
    * Returns true iff. any APZC (whether this one or one further in the handoff
    * chain accepted the fling).
    */
   bool TakeOverFling(ScreenPoint aVelocity);
 
 private:
-  friend class OverscrollableAnimation;
   friend class FlingAnimation;
   friend class OverscrollSnapBackAnimation;
   // The initial velocity of the most recent fling.
   ScreenPoint mLastFlingVelocity;
   // The time at which the most recent fling started.
   TimeStamp mLastFlingTime;
 
   // Deal with overscroll resulting from a fling animation. This is only ever
@@ -1034,19 +1033,16 @@ public:
   AsyncPanZoomAnimation(const TimeDuration& aRepaintInterval =
                         TimeDuration::Forever())
     : mRepaintInterval(aRepaintInterval)
   { }
 
   virtual bool Sample(FrameMetrics& aFrameMetrics,
                       const TimeDuration& aDelta) = 0;
 
-  // Called if the animation is cancelled before it ends.
-  virtual void Cancel() {}
-
   /**
    * Get the deferred tasks in |mDeferredTasks|. See |mDeferredTasks|
    * for more information.
    * Clears |mDeferredTasks|.
    */
   Vector<Task*> TakeDeferredTasks() {
     Vector<Task*> result;
     mDeferredTasks.swap(result);
--- a/gfx/tests/gtest/TestAsyncPanZoomController.cpp
+++ b/gfx/tests/gtest/TestAsyncPanZoomController.cpp
@@ -207,17 +207,18 @@ FrameMetrics TestFrameMetrics() {
 static
 void ApzcPan(AsyncPanZoomController* apzc,
              TestAPZCTreeManager* aTreeManager,
              int& aTime,
              int aTouchStartY,
              int aTouchEndY,
              bool expectIgnoredPan = false,
              bool hasTouchListeners = false,
-             nsTArray<uint32_t>* aAllowedTouchBehaviors = nullptr) {
+             nsTArray<uint32_t>* aAllowedTouchBehaviors = nullptr,
+             bool aKeepFingerDown = false) {
 
   const int TIME_BETWEEN_TOUCH_EVENT = 100;
   const int OVERCOME_TOUCH_TOLERANCE = 100;
   MultiTouchInput mti;
   nsEventStatus status;
 
   // Since we're passing inputs directly to the APZC instead of going through
   // the tree manager, we need to build the overscroll handoff chain explicitly
@@ -267,21 +268,23 @@ void ApzcPan(AsyncPanZoomController* apz
 
   mti =
     MultiTouchInput(MultiTouchInput::MULTITOUCH_MOVE, aTime, TimeStamp(), 0);
   aTime += TIME_BETWEEN_TOUCH_EVENT;
   mti.mTouches.AppendElement(SingleTouchData(0, ScreenIntPoint(10, aTouchEndY), ScreenSize(0, 0), 0, 0));
   status = apzc->ReceiveInputEvent(mti);
   EXPECT_EQ(touchMoveStatus, status);
 
-  mti =
-    MultiTouchInput(MultiTouchInput::MULTITOUCH_END, aTime, TimeStamp(), 0);
-  aTime += TIME_BETWEEN_TOUCH_EVENT;
-  mti.mTouches.AppendElement(SingleTouchData(0, ScreenIntPoint(10, aTouchEndY), ScreenSize(0, 0), 0, 0));
-  status = apzc->ReceiveInputEvent(mti);
+  if (!aKeepFingerDown) {
+    mti =
+      MultiTouchInput(MultiTouchInput::MULTITOUCH_END, aTime, TimeStamp(), 0);
+    aTime += TIME_BETWEEN_TOUCH_EVENT;
+    mti.mTouches.AppendElement(SingleTouchData(0, ScreenIntPoint(10, aTouchEndY), ScreenSize(0, 0), 0, 0));
+    status = apzc->ReceiveInputEvent(mti);
+  }
 
   // Since we've explicitly built the overscroll handoff chain before
   // touch-start, we need to explicitly clear it after touch-end.
   aTreeManager->ClearOverscrollHandoffChain();
 }
 
 static
 void DoPanTest(bool aShouldTriggerScroll, uint32_t aBehavior)
@@ -961,16 +964,46 @@ TEST_F(AsyncPanZoomControllerTester, Ove
   // Check that cancelling the animation clears the overscroll.
   apzc->CancelAnimation();
   EXPECT_FALSE(apzc->IsOverscrolled());
   apzc->AssertStateIsReset();
 
   apzc->Destroy();
 }
 
+TEST_F(AsyncPanZoomControllerTester, OverScrollPanningAbort) {
+  TestScopedBoolPref overscrollEnabledPref("apz.overscroll.enabled", true);
+
+  nsRefPtr<MockContentController> mcc = new NiceMock<MockContentController>();
+  nsRefPtr<TestAPZCTreeManager> tm = new TestAPZCTreeManager();
+  nsRefPtr<TestAsyncPanZoomController> apzc = new TestAsyncPanZoomController(0, mcc, tm);
+
+  apzc->SetFrameMetrics(TestFrameMetrics());
+  apzc->NotifyLayersUpdated(TestFrameMetrics(), true);
+
+  // Pan sufficiently to hit overscroll behaviour. Keep the finger down so
+  // the pan does not end.
+  int time = 0;
+  int touchStart = 500;
+  int touchEnd = 10;
+  ApzcPan(apzc, tm, time, touchStart, touchEnd,
+          false, false, nullptr,   // filling it defaults, wish we had named arguments
+          true);                   // keep finger down
+  EXPECT_TRUE(apzc->IsOverscrolled());
+
+  // Check that calling CancelAnimation() while the user is still panning
+  // (and thus no fling or snap-back animation has had a chance to start)
+  // clears the overscroll.
+  apzc->CancelAnimation();
+  EXPECT_FALSE(apzc->IsOverscrolled());
+  apzc->AssertStateIsReset();
+
+  apzc->Destroy();
+}
+
 TEST_F(AsyncPanZoomControllerTester, ShortPress) {
   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);