Bug 1651332. Use purely relative scroll offset updates for many scrollbar initiated scrolls. r=kats

This patch basically attempts to make clicking in the scrollbar track outside of the scrollthumb "work". Clicking in the scrollbar track usually does a page scroll via nsSliderFrame::PageScroll. This eventually ends up in ScrollFrameHelper::ScrollBy where we turn the request from a relative one ("scroll by a page") into an absolute one ("scroll to this position").

This page scroll is typically a smooth scroll and is currently done on the main thread/layout side. Once we start scrolling the visual viewport offset with the scrollbars we can no longer do this purely on the layout side, we at least need the help of the compositor side. I think the simplest way to do this is to hand the scroll request off to the compositor and have it handle the whole thing.

Now we need to consider the following situation: user clicks scrollbar track to page scroll, smooth scroll gets partway complete on the compositor, user clicks again on scrollbar track for a further page scroll. The main thread can't send an absolute scroll offset update request to the compositor at this point because it has outdated information (it needs a 'starting position' to add the page scroll offset amount) so it'll end up scrolling to the wrong place. It has to send a relative scroll offset update.

We already have a mechanism to send relative scroll offset updates. It is implemented by sending a base scroll offset and the desired scroll offset, and then on the compositor send the difference between those two is computed and then added to the scroll offset.

This patch creates a new mechanism (so called "pure relative") that just sends a relative offset update without any absolute scroll positions. The reason I did this is because the existing relative scroll offset update mechanism is not aware of visual viewport offsets, but rather only layout scroll position. For example, here

https://searchfox.org/mozilla-central/rev/8d55e18875b89cdf2a22a7cba60dc40999c18356/layout/generic/nsGfxScrollFrame.h#446

the value we use for the base scroll offset (mApzScrollPos) is set to the layout scroll position. It may be entirely reasonable to make this existing mechanism vv offset aware, but I wanted to implement something to get it working with a smaller chance of regressions to things that already exist and work. Ideally these two mechanims would be merged.

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

gfx/layers/FrameMetrics.h

@@ -147,7 +147,8 @@ struct FrameMetrics {
            mIsRelative == aOther.mIsRelative &&
            mDoSmoothScroll == aOther.mDoSmoothScroll &&
            mIsScrollInfoLayer == aOther.mIsScrollInfoLayer &&
-           mFixedLayerMargins == aOther.mFixedLayerMargins;
+           mFixedLayerMargins == aOther.mFixedLayerMargins &&
+           mPureRelativeOffset == aOther.mPureRelativeOffset;
   }
 
   bool operator!=(const FrameMetrics& aOther) const {
@@ -308,6 +309,16 @@ struct FrameMetrics {
     mDoSmoothScroll = aOther.mDoSmoothScroll;
   }
 
+  void ApplyPureRelativeSmoothScrollUpdateFrom(const FrameMetrics& aOther,
+                                               bool aApplyToSmoothScroll) {
+    MOZ_ASSERT(aOther.IsPureRelative() && aOther.mPureRelativeOffset.isSome());
+    ClampAndSetSmoothScrollOffset(
+        (aApplyToSmoothScroll ? mSmoothScrollOffset : mScrollOffset) +
+        *aOther.mPureRelativeOffset);
+    mScrollGeneration = aOther.mScrollGeneration;
+    mDoSmoothScroll = true;
+  }
+
   void UpdatePendingScrollInfo(const ScrollUpdateInfo& aInfo) {
     mScrollOffset = aInfo.mScrollOffset;
     mBaseScrollOffset = aInfo.mBaseScrollOffset;
@@ -422,6 +433,8 @@ struct FrameMetrics {
 
   bool IsRelative() const { return mIsRelative; }
 
+  bool IsPureRelative() const { return mPureRelativeOffset.isSome(); }
+
   bool GetDoSmoothScroll() const { return mDoSmoothScroll; }
 
   uint32_t GetScrollGeneration() const { return mScrollGeneration; }
@@ -519,6 +532,10 @@ struct FrameMetrics {
     return mFixedLayerMargins;
   }
 
+  void SetPureRelativeOffset(const Maybe<CSSPoint>& aPureRelativeOffset) {
+    mPureRelativeOffset = aPureRelativeOffset;
+  }
+
   // Helper function for RecalculateViewportOffset(). Exposed so that
   // APZC can perform the operation on other copies of the layout
   // and visual viewport rects (e.g. the "effective" ones used to implement
@@ -694,6 +711,10 @@ struct FrameMetrics {
   // root-content scroll frame.
   ScreenMargin mFixedLayerMargins;
 
+  // When this is Some it means a smooth scroll is requested with the
+  // destination being the current scroll offset plus this relative offset.
+  Maybe<CSSPoint> mPureRelativeOffset;
+
   // Whether or not this is the root scroll frame for the root content document.
   bool mIsRootContent : 1;
 

gfx/layers/apz/src/AsyncPanZoomController.cpp

@@ -4551,6 +4551,10 @@ void AsyncPanZoomController::NotifyLayersUpdated(
       aLayerMetrics.GetDoSmoothScroll() &&
       (aLayerMetrics.GetScrollGeneration() != Metrics().GetScrollGeneration());
 
+  bool pureRelativeSmoothScrollRequested =
+      aLayerMetrics.IsPureRelative() &&
+      (aLayerMetrics.GetScrollGeneration() != Metrics().GetScrollGeneration());
+
   // If `isDefault` is true, this APZC is a "new" one (this is the first time
   // it's getting a NotifyLayersUpdated call). In this case we want to apply the
   // visual scroll offset from the main thread to our scroll offset.
@@ -4569,7 +4573,7 @@ void AsyncPanZoomController::NotifyLayersUpdated(
   if ((aLayerMetrics.GetScrollUpdateType() == FrameMetrics::eMainThread &&
        aLayerMetrics.GetVisualScrollUpdateType() !=
            FrameMetrics::eMainThread) ||
-      smoothScrollRequested) {
+      smoothScrollRequested || pureRelativeSmoothScrollRequested) {
     visualScrollOffsetUpdated = false;
   }
 
@@ -4724,6 +4728,7 @@ void AsyncPanZoomController::NotifyLayersUpdated(
                  ToString(aLayerMetrics.GetScrollOffset()).c_str());
         Metrics().ApplyScrollUpdateFrom(aLayerMetrics);
       }
+
       Metrics().RecalculateLayoutViewportOffset();
 
       for (auto& sampledState : mSampledState) {
@@ -4763,7 +4768,7 @@ void AsyncPanZoomController::NotifyLayersUpdated(
     }
   }
 
-  if (smoothScrollRequested) {
+  if (smoothScrollRequested || pureRelativeSmoothScrollRequested) {
     // A smooth scroll has been requested for animation on the compositor
     // thread.  This flag will be reset by the main thread when it receives
     // the scroll update acknowledgement.
@@ -4772,14 +4777,24 @@ void AsyncPanZoomController::NotifyLayersUpdated(
              Stringify(Metrics().GetScrollOffset()).c_str(),
              Stringify(aLayerMetrics.GetSmoothScrollOffset()).c_str(), mState);
 
-    // See comment on the similar code in the |if (scrollOffsetUpdated)| block
-    // above.
-    if (StaticPrefs::apz_relative_update_enabled() &&
-        aLayerMetrics.IsRelative()) {
-      Metrics().ApplyRelativeSmoothScrollUpdateFrom(aLayerMetrics);
-    } else {
-      Metrics().ApplySmoothScrollUpdateFrom(aLayerMetrics);
+    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);
+      } else {
+        Metrics().ApplySmoothScrollUpdateFrom(aLayerMetrics);
+      }
     }
+
+    if (pureRelativeSmoothScrollRequested) {
+      MOZ_ASSERT(aLayerMetrics.IsPureRelative());
+      MOZ_ASSERT(gfxPlatform::UseDesktopZoomingScrollbars());
+      Metrics().ApplyPureRelativeSmoothScrollUpdateFrom(aLayerMetrics,
+                                                        smoothScrollRequested);
+    }
+
     needContentRepaint = true;
     mExpectedGeckoMetrics.UpdateFrom(aLayerMetrics);
 

gfx/layers/apz/util/APZCCallbackHelper.cpp

@@ -836,7 +836,8 @@ void APZCCallbackHelper::NotifyFlushComplete(PresShell* aPresShell) {
 bool APZCCallbackHelper::IsScrollInProgress(nsIScrollableFrame* aFrame) {
   return aFrame->IsProcessingAsyncScroll() ||
          nsLayoutUtils::CanScrollOriginClobberApz(aFrame->LastScrollOrigin()) ||
-         aFrame->LastSmoothScrollOrigin() != ScrollOrigin::None;
+         aFrame->LastSmoothScrollOrigin() != ScrollOrigin::None ||
+         aFrame->GetRelativeOffset().isSome();
 }
 
 /* static */

gfx/layers/ipc/LayersMessageUtils.h

@@ -213,6 +213,7 @@ struct ParamTraits<mozilla::layers::FrameMetrics>
     WriteParam(aMsg, aParam.mVisualViewportOffset);
     WriteParam(aMsg, aParam.mVisualScrollUpdateType);
     WriteParam(aMsg, aParam.mFixedLayerMargins);
+    WriteParam(aMsg, aParam.mPureRelativeOffset);
     WriteParam(aMsg, aParam.mIsRootContent);
     WriteParam(aMsg, aParam.mIsRelative);
     WriteParam(aMsg, aParam.mDoSmoothScroll);
@@ -245,6 +246,7 @@ struct ParamTraits<mozilla::layers::FrameMetrics>
         ReadParam(aMsg, aIter, &aResult->mVisualViewportOffset) &&
         ReadParam(aMsg, aIter, &aResult->mVisualScrollUpdateType) &&
         ReadParam(aMsg, aIter, &aResult->mFixedLayerMargins) &&
+        ReadParam(aMsg, aIter, &aResult->mPureRelativeOffset) &&
         ReadBoolForBitfield(aMsg, aIter, aResult,
                             &paramType::SetIsRootContent) &&
         ReadBoolForBitfield(aMsg, aIter, aResult, &paramType::SetIsRelative) &&

layout/base/nsLayoutUtils.cpp

@@ -9257,6 +9257,13 @@ ScrollMetadata nsLayoutUtils::ComputeScrollMetadata(
       }
     }
 
+    Maybe<nsPoint> relativeOffset = scrollableFrame->GetRelativeOffset();
+    if (relativeOffset.isSome()) {
+      metrics.SetScrollGeneration(scrollableFrame->CurrentScrollGeneration());
+      metrics.SetPureRelativeOffset(
+          Some(CSSPoint::FromAppUnits(*relativeOffset)));
+    }
+
     CSSRect viewport = metrics.GetLayoutViewport();
     viewport.MoveTo(scrollPosition);
     metrics.SetLayoutViewport(viewport);

layout/generic/nsGfxScrollFrame.cpp

@@ -2947,6 +2947,11 @@ void ScrollFrameHelper::ScrollToImpl(nsPoint aPt, const nsRect& aRange,
     mAllowScrollOriginDowngrade = false;
   }
   mLastSmoothScrollOrigin = ScrollOrigin::None;
+  // Check if this scroll clobbers a pure relative scroll.
+  if (aOrigin == ScrollOrigin::Restore || aOrigin == ScrollOrigin::Other ||
+      aOrigin == ScrollOrigin::Scrollbars) {
+    mRelativeOffset.reset();
+  }
   mScrollGeneration = ++sScrollGenerationCounter;
   if (mLastScrollOrigin == ScrollOrigin::Apz) {
     mApzScrollPos = GetScrollPosition();
@@ -4492,6 +4497,43 @@ void ScrollFrameHelper::ScrollBy(nsIntPoint aDelta, ScrollUnit aUnit,
       return;
   }
 
+  // xxx need to handle scroll snapping
+  if (gfxPlatform::UseDesktopZoomingScrollbars() &&
+      nsLayoutUtils::AsyncPanZoomEnabled(mOuter) &&
+      !nsLayoutUtils::ShouldDisableApzForElement(mOuter->GetContent()) &&
+      (WantAsyncScroll() || mZoomableByAPZ)) {
+    if (mRelativeOffset.isNothing()) {
+      mRelativeOffset = Some(nsPoint(0, 0));
+    }
+    mRelativeOffset->x = NSCoordSaturatingAdd(
+        mRelativeOffset->x,
+        NSCoordSaturatingNonnegativeMultiply(aDelta.x, deltaMultiplier.width));
+    mRelativeOffset->y = NSCoordSaturatingAdd(
+        mRelativeOffset->y,
+        NSCoordSaturatingNonnegativeMultiply(aDelta.y, deltaMultiplier.height));
+
+    mScrollGeneration = ++sScrollGenerationCounter;
+
+    if (!nsLayoutUtils::HasDisplayPort(mOuter->GetContent())) {
+      if (MOZ_LOG_TEST(sDisplayportLog, LogLevel::Debug)) {
+        mozilla::layers::ScrollableLayerGuid::ViewID viewID =
+            mozilla::layers::ScrollableLayerGuid::NULL_SCROLL_ID;
+        nsLayoutUtils::FindIDFor(mOuter->GetContent(), &viewID);
+        MOZ_LOG(
+            sDisplayportLog, LogLevel::Debug,
+            ("ScrollBy setting displayport on scrollId=%" PRIu64 "\n", viewID));
+      }
+
+      nsLayoutUtils::CalculateAndSetDisplayPortMargins(
+          mOuter->GetScrollTargetFrame(), nsLayoutUtils::RepaintMode::Repaint);
+      nsIFrame* frame = do_QueryFrame(mOuter->GetScrollTargetFrame());
+      nsLayoutUtils::SetZeroMarginDisplayPortOnAsyncScrollableAncestors(frame);
+    }
+
+    mOuter->SchedulePaint();
+    return;
+  }
+
   nsPoint newPos(NSCoordSaturatingAdd(mDestination.x,
                                       NSCoordSaturatingNonnegativeMultiply(
                                           aDelta.x, deltaMultiplier.width)),
@@ -6794,7 +6836,8 @@ UniquePtr<PresState> ScrollFrameHelper::SaveState() const {
   // Don't store a scroll state if we never have been scrolled or restored
   // a previous scroll state, and we're not in the middle of a smooth scroll.
   bool isInSmoothScroll = IsProcessingAsyncScroll() ||
-                          mLastSmoothScrollOrigin != ScrollOrigin::None;
+                          mLastSmoothScrollOrigin != ScrollOrigin::None ||
+                          mRelativeOffset.isSome();
   if (!mHasBeenScrolled && !mDidHistoryRestore && !isInSmoothScroll) {
     return nullptr;
   }

layout/generic/nsGfxScrollFrame.h

@@ -444,6 +444,7 @@ class ScrollFrameHelper : public nsIReflowCallback {
   void NotifyApzTransaction() {
     mAllowScrollOriginDowngrade = true;
     mApzScrollPos = GetScrollPosition();
+    mRelativeOffset.reset();
   }
   void NotifyApproximateFrameVisibilityUpdate(bool aIgnoreDisplayPort);
   bool GetDisplayPortAtLastApproximateFrameVisibilityUpdate(
@@ -470,6 +471,7 @@ class ScrollFrameHelper : public nsIReflowCallback {
       mLastSmoothScrollOrigin = ScrollOrigin::None;
     }
   }
+  Maybe<nsPoint> GetRelativeOffset() const { return mRelativeOffset; }
   bool WantAsyncScroll() const;
   Maybe<mozilla::layers::ScrollMetadata> ComputeScrollMetadata(
       LayerManager* aLayerManager, const nsIFrame* aContainerReferenceFrame,
@@ -575,6 +577,10 @@ class ScrollFrameHelper : public nsIReflowCallback {
   // destination based on this value.
   nsPoint mDestination;
 
+  // Tracks a relative scroll offset to pass to apz to do a smooth scroll by.
+  // It is used by ScrollBy when this scroll frame can be scrolled by APZ.
+  Maybe<nsPoint> mRelativeOffset;
+
   // A goal position to try to scroll to as content loads. As long as mLastPos
   // matches the current logical scroll position, we try to scroll to
   // mRestorePos after every reflow --- because after each time content is
@@ -1040,6 +1046,9 @@ class nsHTMLScrollFrame : public nsContainerFrame,
   void ResetScrollInfoIfGeneration(uint32_t aGeneration) final {
     mHelper.ResetScrollInfoIfGeneration(aGeneration);
   }
+  Maybe<nsPoint> GetRelativeOffset() const final {
+    return mHelper.GetRelativeOffset();
+  }
   bool WantAsyncScroll() const final { return mHelper.WantAsyncScroll(); }
   mozilla::Maybe<mozilla::layers::ScrollMetadata> ComputeScrollMetadata(
       LayerManager* aLayerManager, const nsIFrame* aContainerReferenceFrame,
@@ -1512,6 +1521,9 @@ class nsXULScrollFrame final : public nsBoxFrame,
   void ResetScrollInfoIfGeneration(uint32_t aGeneration) final {
     mHelper.ResetScrollInfoIfGeneration(aGeneration);
   }
+  Maybe<nsPoint> GetRelativeOffset() const final {
+    return mHelper.GetRelativeOffset();
+  }
   bool WantAsyncScroll() const final { return mHelper.WantAsyncScroll(); }
   mozilla::Maybe<mozilla::layers::ScrollMetadata> ComputeScrollMetadata(
       LayerManager* aLayerManager, const nsIFrame* aContainerReferenceFrame,

layout/generic/nsIScrollableFrame.h

@@ -451,6 +451,10 @@ class nsIScrollableFrame : public nsIScrollbarMediator {
    * counter.
    */
   virtual void ResetScrollInfoIfGeneration(uint32_t aGeneration) = 0;
+  /**
+   * Relative scrolling offset to be requested of apz.
+   */
+  virtual Maybe<nsPoint> GetRelativeOffset() const = 0;
   /**
    * Determine whether it is desirable to be able to asynchronously scroll this
    * scroll frame.