Bug 1232048 - Fix breakage in scroll snapping when snapping twice in a row to the same thing with non-smooth-scrolls in between. r=botond a=sylvestre
authorKartikaya Gupta <kgupta@mozilla.com>
Wed, 16 Dec 2015 10:42:49 -0500
changeset 310481 4951523aa064fe188edc1b6c100af1033eb769ba
parent 310480 9ee15d08d36292f6235bf3485a25640d7b39ac43
child 310482 20e69b35c57e0254770ee706ae72105545a00157
push id5513
push userraliiev@mozilla.com
push dateMon, 25 Jan 2016 13:55:34 +0000
treeherdermozilla-beta@5ee97dd05b5c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbotond, sylvestre
bugs1232048
milestone45.0a2
Bug 1232048 - Fix breakage in scroll snapping when snapping twice in a row to the same thing with non-smooth-scrolls in between. r=botond a=sylvestre
layout/generic/nsGfxScrollFrame.cpp
layout/generic/nsGfxScrollFrame.h
--- a/layout/generic/nsGfxScrollFrame.cpp
+++ b/layout/generic/nsGfxScrollFrame.cpp
@@ -2087,16 +2087,22 @@ ScrollFrameHelper::ScrollToWithOrigin(ns
                                aScrollPosition);
   }
 
   nsRect scrollRange = GetScrollRangeForClamping();
   mDestination = scrollRange.ClampPoint(aScrollPosition);
 
   nsRect range = aRange ? *aRange : nsRect(aScrollPosition, nsSize(0, 0));
 
+  if (aMode != nsIScrollableFrame::SMOOTH_MSD) {
+    // If we get a non-smooth-scroll, reset the cached APZ scroll destination,
+    // so that we know to process the next smooth-scroll destined for APZ.
+    mApzSmoothScrollDestination = Nothing();
+  }
+
   if (aMode == nsIScrollableFrame::INSTANT) {
     // Asynchronous scrolling is not allowed, so we'll kill any existing
     // async-scrolling process and do an instant scroll.
     CompleteAsyncScroll(range, aOrigin);
     return;
   }
 
   nsPresContext* presContext = mOuter->PresContext();
@@ -2116,40 +2122,41 @@ ScrollFrameHelper::ScrollToWithOrigin(ns
         if (mAsyncScroll) {
           if (mAsyncScroll->mIsSmoothScroll) {
             currentVelocity = mAsyncScroll->VelocityAt(now);
           }
           mAsyncScroll = nullptr;
         }
 
         if (nsLayoutUtils::AsyncPanZoomEnabled(mOuter)) {
-          if (mApzSmoothScrollDestination == mDestination &&
+          if (mApzSmoothScrollDestination == Some(mDestination) &&
               mScrollGeneration == sScrollGenerationCounter) {
             // If we already sent APZ a smooth-scroll request to this
             // destination with this generation (i.e. it was the last request
             // we sent), then don't send another one because it is redundant.
             // This is to avoid a scenario where pages do repeated scrollBy
             // calls, incrementing the generation counter, and blocking APZ from
             // syncing the scroll offset back to the main thread.
             // Note that if we get two smooth-scroll requests to the same
-            // destination with some other scroll in between, mDestination will
-            // get reset and so we shouldn't have the problem where this check
-            // discards a legitimate smooth-scroll.
+            // destination with some other scroll in between,
+            // mApzSmoothScrollDestination will get reset to Nothing() and so
+            // we shouldn't have the problem where this check discards a
+            // legitimate smooth-scroll.
             // Note: if there are two separate scrollframes both getting smooth
             // scrolled at the same time, sScrollGenerationCounter can get
             // incremented and this early-exit won't get taken. Bug 1231177 is
             // on file for this.
             return;
           }
 
           // The animation will be handled in the compositor, pass the
           // information needed to start the animation and skip the main-thread
           // animation for this scroll.
           mLastSmoothScrollOrigin = aOrigin;
-          mApzSmoothScrollDestination = mDestination;
+          mApzSmoothScrollDestination = Some(mDestination);
           mScrollGeneration = ++sScrollGenerationCounter;
 
           if (!nsLayoutUtils::GetDisplayPort(mOuter->GetContent())) {
             // If this frame doesn't have a displayport then there won't be an
             // APZC instance for it and so there won't be anything to process
             // this smooth scroll request. We should set a displayport on this
             // frame to force an APZC which can handle the request.
             nsLayoutUtils::CalculateAndSetDisplayPortMargins(
--- a/layout/generic/nsGfxScrollFrame.h
+++ b/layout/generic/nsGfxScrollFrame.h
@@ -427,17 +427,17 @@ public:
   nsIFrame* mResizerBox;
   nsContainerFrame* mOuter;
   RefPtr<AsyncScroll> mAsyncScroll;
   RefPtr<AsyncSmoothMSDScroll> mAsyncSmoothMSDScroll;
   RefPtr<ScrollbarActivity> mScrollbarActivity;
   nsTArray<nsIScrollPositionListener*> mListeners;
   nsIAtom* mLastScrollOrigin;
   nsIAtom* mLastSmoothScrollOrigin;
-  nsPoint mApzSmoothScrollDestination;
+  Maybe<nsPoint> mApzSmoothScrollDestination;
   uint32_t mScrollGeneration;
   nsRect mScrollPort;
   // Where we're currently scrolling to, if we're scrolling asynchronously.
   // If we're not in the middle of an asynchronous scroll then this is
   // just the current scroll position. ScrollBy will choose its
   // destination based on this value.
   nsPoint mDestination;
   nsPoint mScrollPosAtLastPaint;