Bug 1529892 - Move the clearing of a pending visual scroll update to the end of the paint. r=kats

The fixes a latent bug with WebRender where we would clear it after reading it
in ComputeScrollMetadata, but WR would sometimes call ComputeScrollMetadata a
second time for the same scroll frame in the same transaction, resulting in
the update sometimes not making it into the transaction.

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

--HG--
extra : moz-landing-system : lando

layout/base/PresShell.cpp

@@ -10648,6 +10648,17 @@ void nsIPresShell::SetPendingVisualScrollUpdate(
   }
 }
 
+void nsIPresShell::ClearPendingVisualScrollUpdate() {
+  if (mPendingVisualScrollUpdate && mPendingVisualScrollUpdate->mAcknowledged) {
+    mPendingVisualScrollUpdate = mozilla::Nothing();
+  }
+}
+
+void nsIPresShell::AcknowledgePendingVisualScrollUpdate() {
+  MOZ_ASSERT(mPendingVisualScrollUpdate);
+  mPendingVisualScrollUpdate->mAcknowledged = true;
+}
+
 nsPoint nsIPresShell::GetVisualViewportOffsetRelativeToLayoutViewport() const {
   return GetVisualViewportOffset() - GetLayoutViewportOffset();
 }
@@ -11051,3 +11062,18 @@ PresShell::EventHandler::HandlingTimeAccumulator::~HandlingTimeAccumulator() {
       return;
   }
 }
+
+static bool EndPaintHelper(Document* aDocument, void* aData) {
+  if (PresShell* presShell = aDocument->GetPresShell()) {
+    presShell->EndPaint();
+  }
+  return true;
+}
+
+void PresShell::EndPaint() {
+  ClearPendingVisualScrollUpdate();
+
+  if (mDocument) {
+    mDocument->EnumerateSubDocuments(EndPaintHelper, nullptr);
+  }
+}

layout/base/PresShell.h

@@ -499,6 +499,11 @@ class PresShell final : public nsIPresShell,
     PresShell::SetCapturingContent(nullptr, CaptureFlags::None);
   }
 
+  // Called at the end of nsLayoutUtils::PaintFrame().
+  // This is used to clear any pending visual scroll updates that have been
+  // acknowledged, to make sure they don't stick around for the next paint.
+  void EndPaint();
+
  private:
   ~PresShell();
 

layout/base/nsIPresShell.h

@@ -1446,6 +1446,7 @@ class nsIPresShell : public nsStubDocumentObserver {
   struct VisualScrollUpdate {
     nsPoint mVisualScrollOffset;
     FrameMetrics::ScrollOffsetUpdateType mUpdateType;
+    bool mAcknowledged = false;
   };
 
   // Ask APZ in the next transaction to scroll to the given visual viewport
@@ -1461,9 +1462,8 @@ class nsIPresShell : public nsStubDocumentObserver {
   void ScrollToVisual(const nsPoint& aVisualViewportOffset,
                       FrameMetrics::ScrollOffsetUpdateType aUpdateType,
                       mozilla::ScrollMode aMode);
-  void ClearPendingVisualScrollUpdate() {
-    mPendingVisualScrollUpdate = mozilla::Nothing();
-  }
+  void AcknowledgePendingVisualScrollUpdate();
+  void ClearPendingVisualScrollUpdate();
   const mozilla::Maybe<VisualScrollUpdate>& GetPendingVisualScrollUpdate()
       const {
     return mPendingVisualScrollUpdate;

layout/base/nsLayoutUtils.cpp

@@ -3987,6 +3987,7 @@ nsresult nsLayoutUtils::PaintFrame(gfxContext* aRenderingContext,
       list.PaintRoot(&builder, aRenderingContext, flags);
   Telemetry::AccumulateTimeDelta(Telemetry::PAINT_RASTERIZE_TIME, paintStart);
 
+  presShell->EndPaint();
   builder.Check();
 
   if (gfxPrefs::GfxLoggingPaintedPixelCountEnabled()) {
@@ -8932,7 +8933,7 @@ ScrollMetadata nsLayoutUtils::ComputeScrollMetadata(
         metrics.SetVisualViewportOffset(
             CSSPoint::FromAppUnits(visualUpdate->mVisualScrollOffset));
         metrics.SetVisualScrollUpdateType(visualUpdate->mUpdateType);
-        presShell->ClearPendingVisualScrollUpdate();
+        presShell->AcknowledgePendingVisualScrollUpdate();
       }
     }