Bug 1655242. When a relative scroll offset update comes in during a smooth scroll animation make the new destination be based on the existing destination, not the current scroll position. r=kats

Differential Revision: https://phabricator.services.mozilla.com/D84904

gfx/layers/FrameMetrics.h

@@ -297,23 +297,31 @@ struct FrameMetrics {
   }
 
   /**
-   * Applies the relative scroll offset update contained in aOther to the
-   * smooth scroll destination offset contained in this. The scroll delta is
-   * clamped to the scrollable region.
+   * Applies the relative scroll offset update contained in aOther to the smooth
+   * scroll destination offset contained in this, or to the provided existing
+   * destination, if one is provided. The scroll delta is clamped to the
+   * scrollable region.
    */
-  void ApplyRelativeSmoothScrollUpdateFrom(const FrameMetrics& aOther) {
+  void ApplyRelativeSmoothScrollUpdateFrom(
+      const FrameMetrics& aOther, const Maybe<CSSPoint>& aExistingDestination) {
     MOZ_ASSERT(aOther.IsRelative());
     CSSPoint delta = (aOther.mSmoothScrollOffset - aOther.mBaseScrollOffset);
-    ClampAndSetSmoothScrollOffset(mScrollOffset + delta);
+    ClampAndSetSmoothScrollOffset(aExistingDestination.valueOr(mScrollOffset) +
+                                  delta);
     mScrollGeneration = aOther.mScrollGeneration;
     mDoSmoothScroll = aOther.mDoSmoothScroll;
   }
 
-  void ApplyPureRelativeSmoothScrollUpdateFrom(const FrameMetrics& aOther,
-                                               bool aApplyToSmoothScroll) {
+  void ApplyPureRelativeSmoothScrollUpdateFrom(
+      const FrameMetrics& aOther, const Maybe<CSSPoint>& aExistingDestination,
+      bool aApplyToSmoothScroll) {
     MOZ_ASSERT(aOther.IsPureRelative() && aOther.mPureRelativeOffset.isSome());
+    // See AsyncPanZoomController::NotifyLayersUpdated where
+    // pureRelativeSmoothScrollRequested is handled for the explanation for the
+    // logic in this function.
     ClampAndSetSmoothScrollOffset(
-        (aApplyToSmoothScroll ? mSmoothScrollOffset : mScrollOffset) +
+        (aApplyToSmoothScroll ? mSmoothScrollOffset
+                              : aExistingDestination.valueOr(mScrollOffset)) +
         *aOther.mPureRelativeOffset);
     mScrollGeneration = aOther.mScrollGeneration;
     mDoSmoothScroll = true;

gfx/layers/apz/src/AsyncPanZoomController.cpp

@@ -2109,15 +2109,8 @@ CSSPoint AsyncPanZoomController::GetKeyboardDestination(
     pageScrollSize = mScrollMetadata.GetPageScrollAmount() /
                      Metrics().GetDevPixelsPerCSSPixel();
 
-    if (mState == WHEEL_SCROLL) {
-      scrollOffset = mAnimation->AsWheelScrollAnimation()->GetDestination();
-    } else if (mState == SMOOTH_SCROLL) {
-      scrollOffset = mAnimation->AsSmoothScrollAnimation()->GetDestination();
-    } else if (mState == KEYBOARD_SCROLL) {
-      scrollOffset = mAnimation->AsKeyboardScrollAnimation()->GetDestination();
-    } else {
-      scrollOffset = Metrics().GetScrollOffset();
-    }
+    scrollOffset = GetCurrentAnimationDestination(lock).valueOr(
+        Metrics().GetScrollOffset());
 
     scrollRect = Metrics().GetScrollableRect();
   }
@@ -3437,6 +3430,21 @@ float AsyncPanZoomController::ComputePLPPI(ParentLayerPoint aPoint,
   return GetDPI() / screenPerParent;
 }
 
+Maybe<CSSPoint> AsyncPanZoomController::GetCurrentAnimationDestination(
+    const RecursiveMutexAutoLock& aProofOfLock) const {
+  if (mState == WHEEL_SCROLL) {
+    return Some(mAnimation->AsWheelScrollAnimation()->GetDestination());
+  }
+  if (mState == SMOOTH_SCROLL) {
+    return Some(mAnimation->AsSmoothScrollAnimation()->GetDestination());
+  }
+  if (mState == KEYBOARD_SCROLL) {
+    return Some(mAnimation->AsKeyboardScrollAnimation()->GetDestination());
+  }
+
+  return Nothing();
+}
+
 ParentLayerPoint
 AsyncPanZoomController::AdjustHandoffVelocityForOverscrollBehavior(
     ParentLayerPoint& aHandoffVelocity) const {
@@ -4781,12 +4789,17 @@ void AsyncPanZoomController::NotifyLayersUpdated(
              Stringify(Metrics().GetScrollOffset()).c_str(),
              Stringify(aLayerMetrics.GetSmoothScrollOffset()).c_str(), mState);
 
+    // For relative updates we want to add the relative offset to any existing
+    // destination.
+    Maybe<CSSPoint> destination = GetCurrentAnimationDestination(lock);
+
     if (smoothScrollRequested) {
       // See comment on the similar code in the |if (scrollOffsetUpdated)| block
       // above.
       if (StaticPrefs::apz_relative_update_enabled() &&
           aLayerMetrics.IsRelative()) {
-        Metrics().ApplyRelativeSmoothScrollUpdateFrom(aLayerMetrics);
+        Metrics().ApplyRelativeSmoothScrollUpdateFrom(aLayerMetrics,
+                                                      destination);
       } else {
         Metrics().ApplySmoothScrollUpdateFrom(aLayerMetrics);
       }
@@ -4795,8 +4808,14 @@ void AsyncPanZoomController::NotifyLayersUpdated(
     if (pureRelativeSmoothScrollRequested) {
       MOZ_ASSERT(aLayerMetrics.IsPureRelative());
       MOZ_ASSERT(gfxPlatform::UseDesktopZoomingScrollbars());
-      Metrics().ApplyPureRelativeSmoothScrollUpdateFrom(aLayerMetrics,
-                                                        smoothScrollRequested);
+      // If smoothScrollRequested is true then mSmoothScrollOffset of our
+      // metrics has the base we want to add our relative offset to from above;
+      // |destination| has already been included if necessary. If
+      // smoothScrollRequested if false then we want to use destination as our
+      // base offset if it is Some. Otherwise mScrollOffset is our base.
+      // ApplyPureRelativeSmoothScrollUpdateFrom handles this logic for us.
+      Metrics().ApplyPureRelativeSmoothScrollUpdateFrom(
+          aLayerMetrics, destination, smoothScrollRequested);
     }
 
     needContentRepaint = true;

gfx/layers/apz/src/AsyncPanZoomController.h

@@ -1407,6 +1407,9 @@ class AsyncPanZoomController {
   float ComputePLPPI(ParentLayerPoint aPoint,
                      ParentLayerPoint aDirection) const;
 
+  Maybe<CSSPoint> GetCurrentAnimationDestination(
+      const RecursiveMutexAutoLock& aProofOfLock) const;
+
   /* ===================================================================
    * The functions and members in this section are used to make ancestor chains
    * out of APZC instances. These chains can only be walked or manipulated