Tie scroll anchoring adjustments to frame lifecycle instead of layout.

third_party/WebKit/Source/core/frame/FrameView.cpp

Index: third_party/WebKit/Source/core/frame/FrameView.cpp
diff --git a/third_party/WebKit/Source/core/frame/FrameView.cpp b/third_party/WebKit/Source/core/frame/FrameView.cpp
index a3537839378bfed23940dbbef65496c9484447bf..17b1ce406025d8b00a5ed38518d024692a6b1923 100644
--- a/third_party/WebKit/Source/core/frame/FrameView.cpp
+++ b/third_party/WebKit/Source/core/frame/FrameView.cpp
@@ -223,6 +223,7 @@ DEFINE_TRACE(FrameView) {
   visitor->trace(m_children);
   visitor->trace(m_viewportScrollableArea);
   visitor->trace(m_scrollAnchor);
+  visitor->trace(m_anchoringAdjustmentQueue);
   visitor->trace(m_scrollbarManager);
   Widget::trace(visitor);
   ScrollableArea::trace(visitor);
@@ -909,7 +910,7 @@ void FrameView::performPreLayoutTasks() {
   lifecycle().advanceTo(DocumentLifecycle::StyleClean);
 
   if (shouldPerformScrollAnchoring())
-    m_scrollAnchor.save();
+    m_scrollAnchor.notifyBeforeLayout();
 }
 
 bool FrameView::shouldPerformScrollAnchoring() const {
@@ -2286,11 +2287,6 @@ void FrameView::performPostLayoutTasks() {
     scrollingCoordinator->notifyGeometryChanged();
 
   scrollToFragmentAnchor();
-  // TODO(skobes): Figure out interactions between scroll anchor, fragment
-  // anchor, and history restoration.
-  if (shouldPerformScrollAnchoring())
-    m_scrollAnchor.restore();
-
   sendResizeEventIfNeeded();
 }
 
@@ -2717,6 +2713,10 @@ void FrameView::updateLifecyclePhasesInternal(
     return;
   }
 
+  forAllNonThrottledFrameViews([](FrameView& frameView) {
+    frameView.performScrollAnchoringAdjustments();
+  });
+
   if (targetState == DocumentLifecycle::PaintClean) {
     forAllNonThrottledFrameViews(
         [](FrameView& frameView) { frameView.notifyResizeObservers(); });
@@ -2784,6 +2784,19 @@ void FrameView::updateLifecyclePhasesInternal(
   updateViewportIntersectionsForSubtree(targetState);
 }
 
+void FrameView::enqueueScrollAnchoringAdjustment(
+    ScrollableArea* scrollableArea) {
+  m_anchoringAdjustmentQueue.add(scrollableArea);
+}
+
+void FrameView::performScrollAnchoringAdjustments() {
+  for (WeakMember<ScrollableArea>& scroller : m_anchoringAdjustmentQueue) {
+    if (scroller)
+      scroller->scrollAnchor()->adjust();

[ojan, 2016/10/27 19:02:24]

It looks like it's possible for the scroll anchor to be cleared, but not removed
from the queue. Am I reading that wrong?

I think we could null-check scrollAnchor or remove from the queue when the
anchor is cleared. The former might be cheaper.

[skobes, 2016/10/27 19:44:08]

A cleared ScrollAnchor still exists as a ScrollAnchor object.  All overrides of
ScrollableArea::scrollAnchor guarantee non-nullness.  Added a DCHECK to be safe.

+  }
+  m_anchoringAdjustmentQueue.clear();
+}
+
 void FrameView::updatePaintProperties() {
   TRACE_EVENT0("blink", "FrameView::updatePaintProperties");
 
@@ -3630,7 +3643,7 @@ void FrameView::updateScrollOffset(const ScrollOffset& offset,
       documentLoader->initialScrollState().wasScrolledByUser = true;
   }
 
-  if (scrollType != AnchoringScroll)
+  if (scrollType != AnchoringScroll && scrollType != ClampingScroll)
     clearScrollAnchor();
 }
 
@@ -3858,15 +3871,8 @@ void FrameView::updateScrollbars() {
 
 void FrameView::adjustScrollOffsetFromUpdateScrollbars() {
   ScrollOffset clamped = clampScrollOffset(scrollOffset());
-  // Restore before clamping because clamping clears the scroll anchor.
-  // TODO(ymalik): This same logic exists in PaintLayerScrollableArea.
-  // Remove when root-layer-scrolls is enabled.
-  if (clamped != scrollOffset() && shouldPerformScrollAnchoring()) {
-    m_scrollAnchor.restore();
-    clamped = clampScrollOffset(scrollOffset());
-  }
   if (clamped != scrollOffset() || scrollOriginChanged()) {
-    ScrollableArea::setScrollOffset(clamped, ProgrammaticScroll);
+    ScrollableArea::setScrollOffset(clamped, ClampingScroll);
     resetScrollOriginChanged();
   }
 }