Bug 1529892 - Move the clearing of a pending visual scroll update to the end of the paint. r=kats
authorBotond Ballo <botond@mozilla.com>
Fri, 26 Apr 2019 05:14:00 +0000
changeset 530240 094b212a3cbf55d92b85db2b5e1d04f8d46a5dfb
parent 530239 c63d6ed086516d909a65033c62b9343e08f6f839
child 530263 7d47e7fa2489550ffa83aae67715c5497048923f
child 530264 c1b7457b258ac27b7d737160fdb859c2648d9d90
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskats
bugs1529892
milestone68.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 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
layout/base/PresShell.cpp
layout/base/PresShell.h
layout/base/nsIPresShell.h
layout/base/nsLayoutUtils.cpp
--- a/layout/base/PresShell.cpp
+++ b/layout/base/PresShell.cpp
@@ -10643,16 +10643,27 @@ void nsIPresShell::SetPendingVisualScrol
 
   // The pending update is picked up during the next paint.
   // Schedule a paint to make sure one will happen.
   if (nsIFrame* rootFrame = GetRootFrame()) {
     rootFrame->SchedulePaint();
   }
 }
 
+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();
 }
 
 nsPoint nsIPresShell::GetLayoutViewportOffset() const {
   nsPoint result;
   if (nsIScrollableFrame* sf = GetRootScrollFrameAsScrollable()) {
     result = sf->GetScrollPosition();
@@ -11046,8 +11057,23 @@ PresShell::EventHandler::HandlingTimeAcc
             Telemetry::INPUT_EVENT_HANDLED_APZ_TOUCH_MOVE_MS,
             mHandlingStartTime);
       }
       return;
     default:
       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);
+  }
+}
--- a/layout/base/PresShell.h
+++ b/layout/base/PresShell.h
@@ -494,16 +494,21 @@ class PresShell final : public nsIPresSh
   /**
    * Alias for SetCapturingContent(nullptr, CaptureFlags::None) for making
    * callers what they do clearer.
    */
   static void ReleaseCapturingContent() {
     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();
 
   friend class ::AutoPointerEventTargetUpdater;
 
   // ProcessReflowCommands returns whether we processed all our dirty roots
   // without interruptions.
   MOZ_CAN_RUN_SCRIPT bool ProcessReflowCommands(bool aInterruptible);
--- a/layout/base/nsIPresShell.h
+++ b/layout/base/nsIPresShell.h
@@ -1441,34 +1441,34 @@ class nsIPresShell : public nsStubDocume
   nsPoint GetVisualViewportOffsetRelativeToLayoutViewport() const;
 
   // Represents an update to the visual scroll offset that will be sent to APZ.
   // The update type is used to determine priority compared to other scroll
   // updates.
   struct VisualScrollUpdate {
     nsPoint mVisualScrollOffset;
     FrameMetrics::ScrollOffsetUpdateType mUpdateType;
+    bool mAcknowledged = false;
   };
 
   // Ask APZ in the next transaction to scroll to the given visual viewport
   // offset (relative to the document).
   // Use this sparingly, as it will clobber JS-driven scrolling that happens
   // in the same frame. This is mostly intended to be used in special
   // situations like "first paint" or session restore.
   // If scrolling "far away", i.e. not just within the existing layout
   // viewport, it's recommended to use both nsIScrollableFrame.ScrollTo*()
   // (via window.scrollTo if calling from JS) *and* this function; otherwise,
   // temporary checkerboarding may result.
   // Please request APZ review if adding a new call site.
   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;
   }
 
   nsPoint GetLayoutViewportOffset() const;
   nsSize GetLayoutViewportSize() const;
 
--- a/layout/base/nsLayoutUtils.cpp
+++ b/layout/base/nsLayoutUtils.cpp
@@ -3982,16 +3982,17 @@ nsresult nsLayoutUtils::PaintFrame(gfxCo
   }
 #endif
 
   TimeStamp paintStart = TimeStamp::Now();
   RefPtr<LayerManager> layerManager =
       list.PaintRoot(&builder, aRenderingContext, flags);
   Telemetry::AccumulateTimeDelta(Telemetry::PAINT_RASTERIZE_TIME, paintStart);
 
+  presShell->EndPaint();
   builder.Check();
 
   if (gfxPrefs::GfxLoggingPaintedPixelCountEnabled()) {
     TimeStamp now = TimeStamp::Now();
     float rasterizeTime = (now - paintStart).ToMilliseconds();
     uint32_t pixelCount = layerManager->GetAndClearPaintedPixelCount();
     static std::vector<std::pair<TimeStamp, uint32_t>> history;
     if (pixelCount) {
@@ -8927,17 +8928,17 @@ ScrollMetadata nsLayoutUtils::ComputeScr
                                   FrameMetrics::eRestore, ScrollMode::Instant);
       }
 
       if (const Maybe<nsIPresShell::VisualScrollUpdate>& visualUpdate =
               presShell->GetPendingVisualScrollUpdate()) {
         metrics.SetVisualViewportOffset(
             CSSPoint::FromAppUnits(visualUpdate->mVisualScrollOffset));
         metrics.SetVisualScrollUpdateType(visualUpdate->mUpdateType);
-        presShell->ClearPendingVisualScrollUpdate();
+        presShell->AcknowledgePendingVisualScrollUpdate();
       }
     }
 
     CSSRect viewport = metrics.GetLayoutViewport();
     viewport.MoveTo(scrollPosition);
     metrics.SetLayoutViewport(viewport);
 
     nsPoint smoothScrollPosition = scrollableFrame->LastScrollDestination();