Bug 1519462 - Coalesce all scroll anchor adjustments to be performed after layout when flushing notifcations. r=dholbert

We currently perform anchor adjustment in three spots:
  1. If the target of RestyleManager::RecomputePosition is in a scroll anchor chain
  2. If the reflow root is in a scroll anchor chain
  3. In nsHTMLScrollFrame::DidReflow, for itself

It looks like it's possible for a scroll anchor container to be adjusted by (1)
and (2 or 3) in the same PresShell flush.

This should be okay, except that we consume mSuppressAnchorAdjustment when
performing an adjustment, and this can lead us to miss the second time that
we perform adjustments in a PresShell flush.

This commit reworks how we run anchor adjustments so that we collect all
scroll anchor containers that should be adjusted, and only perform the
adjustments once.

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

--HG--
extra : source : cbdbd08379d99f9d55f756c57d728c28331be5da

layout/base/PresShell.cpp

@@ -1311,7 +1311,8 @@ void PresShell::Destroy() {
   }
 
   mFramesToDirty.Clear();
-  mDirtyScrollAnchorContainers.Clear();
+  mPendingScrollAnchorSelection.Clear();
+  mPendingScrollAnchorAdjustment.Clear();
 
   if (mViewManager) {
     // Clear the view manager's weak pointer back to |this| in case it
@@ -2151,7 +2152,8 @@ void PresShell::NotifyDestroyingFrame(nsIFrame* aFrame) {
 
     nsIScrollableFrame* scrollableFrame = do_QueryFrame(aFrame);
     if (scrollableFrame) {
-      mDirtyScrollAnchorContainers.RemoveEntry(scrollableFrame);
+      mPendingScrollAnchorSelection.RemoveEntry(scrollableFrame);
+      mPendingScrollAnchorAdjustment.RemoveEntry(scrollableFrame);
     }
   }
 }
@@ -2574,17 +2576,32 @@ void PresShell::VerifyHasDirtyRootAncestor(nsIFrame* aFrame) {
 }
 #endif
 
-void PresShell::PostDirtyScrollAnchorContainer(nsIScrollableFrame* aFrame) {
-  mDirtyScrollAnchorContainers.PutEntry(aFrame);
+void PresShell::PostPendingScrollAnchorSelection(
+    mozilla::layout::ScrollAnchorContainer* aContainer) {
+  mPendingScrollAnchorSelection.PutEntry(aContainer->ScrollableFrame());
 }
 
-void PresShell::FlushDirtyScrollAnchorContainers() {
-  for (auto iter = mDirtyScrollAnchorContainers.Iter(); !iter.Done();
+void PresShell::FlushPendingScrollAnchorSelections() {
+  for (auto iter = mPendingScrollAnchorSelection.Iter(); !iter.Done();
        iter.Next()) {
     nsIScrollableFrame* scroll = iter.Get()->GetKey();
     scroll->GetAnchor()->SelectAnchor();
   }
-  mDirtyScrollAnchorContainers.Clear();
+  mPendingScrollAnchorSelection.Clear();
+}
+
+void PresShell::PostPendingScrollAnchorAdjustment(
+    ScrollAnchorContainer* aContainer) {
+  mPendingScrollAnchorAdjustment.PutEntry(aContainer->ScrollableFrame());
+}
+
+void PresShell::FlushPendingScrollAnchorAdjustments() {
+  for (auto iter = mPendingScrollAnchorAdjustment.Iter(); !iter.Done();
+       iter.Next()) {
+    nsIScrollableFrame* scroll = iter.Get()->GetKey();
+    scroll->GetAnchor()->ApplyAdjustments();
+  }
+  mPendingScrollAnchorAdjustment.Clear();
 }
 
 void PresShell::FrameNeedsReflow(nsIFrame* aFrame,
@@ -4174,13 +4191,17 @@ void PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush aFlush) {
       didLayoutFlush = true;
       mFrameConstructor->RecalcQuotesAndCounters();
       viewManager->FlushDelayedResize(true);
-      if (ProcessReflowCommands(flushType < FlushType::Layout) &&
-          mContentToScrollTo) {
-        // We didn't get interrupted.  Go ahead and scroll to our content
-        DoScrollContentIntoView();
+      if (ProcessReflowCommands(flushType < FlushType::Layout)) {
+        // We didn't get interrupted. Go ahead and perform scroll anchor
+        // adjustments and scroll content into view
+        FlushPendingScrollAnchorAdjustments();
+
         if (mContentToScrollTo) {
-          mContentToScrollTo->DeleteProperty(nsGkAtoms::scrolling);
-          mContentToScrollTo = nullptr;
+          DoScrollContentIntoView();
+          if (mContentToScrollTo) {
+            mContentToScrollTo->DeleteProperty(nsGkAtoms::scrolling);
+            mContentToScrollTo = nullptr;
+          }
         }
       }
     }
@@ -8498,7 +8519,7 @@ bool PresShell::DoReflow(nsIFrame* target, bool aInterruptible,
   mReflowCause = nullptr;
 #endif
 
-  FlushDirtyScrollAnchorContainers();
+  FlushPendingScrollAnchorSelections();
 
   if (mReflowContinueTimer) {
     mReflowContinueTimer->Cancel();
@@ -8621,7 +8642,7 @@ bool PresShell::DoReflow(nsIFrame* target, bool aInterruptible,
   target->DidReflow(mPresContext, nullptr);
   if (target->IsInScrollAnchorChain()) {
     ScrollAnchorContainer* container = ScrollAnchorContainer::FindFor(target);
-    container->ApplyAdjustments();
+    PostPendingScrollAnchorAdjustment(container);
   }
   if (isRoot && size.BSize(wm) == NS_UNCONSTRAINEDSIZE) {
     mPresContext->SetVisibleArea(boundsRelativeToTarget);
@@ -10034,7 +10055,8 @@ void PresShell::AddSizeOfIncludingThis(nsWindowSizes& aSizes) const {
   aSizes.mLayoutPresShellSize +=
       mApproximatelyVisibleFrames.ShallowSizeOfExcludingThis(mallocSizeOf) +
       mFramesToDirty.ShallowSizeOfExcludingThis(mallocSizeOf) +
-      mDirtyScrollAnchorContainers.ShallowSizeOfExcludingThis(mallocSizeOf);
+      mPendingScrollAnchorSelection.ShallowSizeOfExcludingThis(mallocSizeOf) +
+      mPendingScrollAnchorAdjustment.ShallowSizeOfExcludingThis(mallocSizeOf);
 
   StyleSet()->AddSizeOfIncludingThis(aSizes);
 

layout/base/PresShell.h

@@ -113,8 +113,12 @@ class PresShell final : public nsIPresShell,
   nsIPageSequenceFrame* GetPageSequenceFrame() const override;
   nsCanvasFrame* GetCanvasFrame() const override;
 
-  void PostDirtyScrollAnchorContainer(nsIScrollableFrame* aFrame) override;
-  void FlushDirtyScrollAnchorContainers() override;
+  void PostPendingScrollAnchorSelection(
+      mozilla::layout::ScrollAnchorContainer* aContainer) override;
+  void FlushPendingScrollAnchorSelections() override;
+  void PostPendingScrollAnchorAdjustment(
+      mozilla::layout::ScrollAnchorContainer* aContainer) override;
+  void FlushPendingScrollAnchorAdjustments();
 
   void FrameNeedsReflow(
       nsIFrame* aFrame, IntrinsicDirty aIntrinsicDirty, nsFrameState aBitToAdd,
@@ -747,7 +751,8 @@ class PresShell final : public nsIPresShell,
   // Set of frames that we should mark with NS_FRAME_HAS_DIRTY_CHILDREN after
   // we finish reflowing mCurrentReflowRoot.
   nsTHashtable<nsPtrHashKey<nsIFrame>> mFramesToDirty;
-  nsTHashtable<nsPtrHashKey<nsIScrollableFrame>> mDirtyScrollAnchorContainers;
+  nsTHashtable<nsPtrHashKey<nsIScrollableFrame>> mPendingScrollAnchorSelection;
+  nsTHashtable<nsPtrHashKey<nsIScrollableFrame>> mPendingScrollAnchorAdjustment;
 
   nsTArray<UniquePtr<DelayedEvent>> mDelayedEvents;
 

layout/base/RestyleManager.cpp

@@ -776,7 +776,7 @@ static bool RecomputePosition(nsIFrame* aFrame) {
 
     if (aFrame->IsInScrollAnchorChain()) {
       ScrollAnchorContainer* container = ScrollAnchorContainer::FindFor(aFrame);
-      container->ApplyAdjustments();
+      aFrame->PresShell()->PostPendingScrollAnchorAdjustment(container);
     }
     return true;
   }
@@ -885,7 +885,7 @@ static bool RecomputePosition(nsIFrame* aFrame) {
 
     if (aFrame->IsInScrollAnchorChain()) {
       ScrollAnchorContainer* container = ScrollAnchorContainer::FindFor(aFrame);
-      container->ApplyAdjustments();
+      aFrame->PresShell()->PostPendingScrollAnchorAdjustment(container);
     }
     return true;
   }
@@ -2972,7 +2972,7 @@ void RestyleManager::DoProcessPendingRestyles(ServoTraversalFlags aFlags) {
   // Select scroll anchors for frames that have been scrolled. Do this
   // before restyling so that anchor nodes are correctly marked for
   // scroll anchor update suppressions.
-  presContext->PresShell()->FlushDirtyScrollAnchorContainers();
+  presContext->PresShell()->FlushPendingScrollAnchorSelections();
 
   // Create a AnimationsWithDestroyedFrame during restyling process to
   // stop animations and transitions on elements that have no frame at the end

layout/base/nsIPresShell.h

@@ -103,6 +103,10 @@ class Selection;
 class ShadowRoot;
 }  // namespace dom
 
+namespace layout {
+class ScrollAnchorContainer;
+}  // namespace layout
+
 namespace layers {
 class LayerManager;
 }  // namespace layers
@@ -456,8 +460,11 @@ class nsIPresShell : public nsStubDocumentObserver {
    */
   virtual nsCanvasFrame* GetCanvasFrame() const = 0;
 
-  virtual void PostDirtyScrollAnchorContainer(nsIScrollableFrame* aFrame) = 0;
-  virtual void FlushDirtyScrollAnchorContainers() = 0;
+  virtual void PostPendingScrollAnchorSelection(
+      mozilla::layout::ScrollAnchorContainer* aContainer) = 0;
+  virtual void FlushPendingScrollAnchorSelections() = 0;
+  virtual void PostPendingScrollAnchorAdjustment(
+      mozilla::layout::ScrollAnchorContainer* aContainer) = 0;
 
   /**
    * Tell the pres shell that a frame needs to be marked dirty and needs

layout/generic/ScrollAnchorContainer.cpp

@@ -227,7 +227,7 @@ void ScrollAnchorContainer::InvalidateAnchor() {
   mAnchorNode = nullptr;
   mAnchorNodeIsDirty = true;
   mLastAnchorPos = nsPoint();
-  Frame()->PresShell()->PostDirtyScrollAnchorContainer(ScrollableFrame());
+  Frame()->PresShell()->PostPendingScrollAnchorSelection(this);
 }
 
 void ScrollAnchorContainer::Destroy() {

layout/generic/nsGfxScrollFrame.cpp

@@ -1136,7 +1136,7 @@ void nsHTMLScrollFrame::Reflow(nsPresContext* aPresContext,
 void nsHTMLScrollFrame::DidReflow(nsPresContext* aPresContext,
                                   const ReflowInput* aReflowInput) {
   nsContainerFrame::DidReflow(aPresContext, aReflowInput);
-  mHelper.mAnchor.ApplyAdjustments();
+  PresShell()->PostPendingScrollAnchorAdjustment(GetAnchor());
 }
 
 ////////////////////////////////////////////////////////////////////////////////

testing/web-platform/meta/css/css-scroll-anchoring/anchoring-with-bounds-clamping-div.html.ini

@@ -0,0 +1,3 @@
+[anchoring-with-bounds-clamping-div.html]
+    [Anchoring combined with scroll bounds clamping in a <div>.]
+        expected: FAIL

testing/web-platform/tests/css/css-scroll-anchoring/heuristic-with-offset-update.html

@@ -0,0 +1,58 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<link rel="help" href="https://drafts.csswg.org/css-scroll-anchoring-1/">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<html>
+<head>
+    <style type="text/css">
+        #scroller {
+            overflow: scroll;
+            height: 500px;
+            height: 500px;
+        }
+        #before {
+            height: 200px;
+        }
+        #anchor {
+            position: relative;
+            width: 200px;
+            height: 200px;
+            margin-bottom: 500px;
+            background-color: blue;
+            /*
+             * To trigger the Gecko bug that's being regression-tested here, we
+             * need 'top' to start out at a non-'auto' value, so that the
+             * dynamic change can trigger Gecko's "RecomputePosition" fast path
+             */
+            top: 0px;
+        }
+    </style>
+</head>
+<body>
+    <div id="scroller">
+        <div id="before">
+        </div>
+        <div id="anchor">
+        </div>
+    </div>
+
+    <script type="text/javascript">
+        test(() => {
+            let scroller = document.querySelector('#scroller');
+            let before = document.querySelector('#before');
+            let anchor = document.querySelector('#anchor');
+
+            // Scroll down to select #anchor as a scroll anchor
+            scroller.scrollTop = 200;
+
+            // Adjust the 'top' of #anchor, which should trigger a suppression
+            anchor.style.top = '10px';
+
+            // Expand #before and make sure we don't apply an adjustment
+            before.style.height = '300px';
+            assert_equals(scroller.scrollTop, 200);
+        }, 'Positioned ancestors with dynamic changes to offsets trigger scroll suppressions.');
+    </script>
+</body>
+</html>