Refactor scroll updates during flexbox layout.

third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp

Index: third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp
diff --git a/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp b/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp
index 059ae5c56cd837daaecba9df25ae72258ef1403d..feb71483796287ae38f690219eb05e897390ff4d 100644
--- a/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp
+++ b/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp
@@ -587,54 +587,42 @@ int PaintLayerScrollableArea::pixelSnappedScrollHeight() const
     return snapSizeToPixel(scrollHeight(), box().clientTop() + box().location().y());
 }
 
-void PaintLayerScrollableArea::computeScrollDimensions()
+void PaintLayerScrollableArea::updateScrollOrigin()
+{
+    // This should do nothing prior to first layout; the if-clause will catch that.
+    if (!overflowRect().size().isZero()) {

[cbiesinger, 2016/05/13 19:52:48]

Thanks for adding the comment. I'd still change this to
overflowRect().isEmpty(), and to an early return.

[szager1, 2016/05/13 20:52:05]

Done.

+        LayoutPoint scrollableOverflow = m_overflowRect.location() - LayoutSize(box().borderLeft(), box().borderTop());
+        setScrollOrigin(flooredIntPoint(-scrollableOverflow) + box().originAdjustmentForScrollbars());
+    }
+}
+
+void PaintLayerScrollableArea::updateScrollDimensions()
 {
     m_overflowRect = box().layoutOverflowRect();
     box().flipForWritingMode(m_overflowRect);
-
-    LayoutPoint scrollableOverflow = m_overflowRect.location() - LayoutSize(box().borderLeft(), box().borderTop());
-    setScrollOrigin(flooredIntPoint(-scrollableOverflow) + box().originAdjustmentForScrollbars());
+    updateScrollOrigin();
 }
 
 void PaintLayerScrollableArea::scrollToPosition(const DoublePoint& scrollPosition, ScrollOffsetClamping clamp, ScrollBehavior scrollBehavior, ScrollType scrollType)
 {
-    cancelProgrammaticScrollAnimation();
-
     DoublePoint newScrollPosition = clamp == ScrollOffsetClamped ? clampScrollPosition(scrollPosition) : scrollPosition;
     if (newScrollPosition != scrollPositionDouble())
         ScrollableArea::setScrollPosition(newScrollPosition, scrollType, scrollBehavior);
 }
 
-bool PaintLayerScrollableArea::updateAfterLayout(SubtreeLayoutScope* delayedLayoutScope)
+void PaintLayerScrollableArea::updateAfterLayout()
 {
     ASSERT(box().hasOverflowClip());
 
-    bool didMarkForDelayedLayout = false;
+    bool relayoutIsPrevented = PreventRelayoutScope::relayoutIsPrevented();
+    bool scrollbarsAreFrozen = FreezeScrollbarsScope::scrollbarsAreFrozen();
 
     if (needsScrollbarReconstruction()) {
-        m_scrollbarManager.setCanDetachScrollbars(false);
         setHasHorizontalScrollbar(false);
         setHasVerticalScrollbar(false);
     }
 
-    m_scrollbarManager.setCanDetachScrollbars(true);
-
-    IntPoint originalOrigin = scrollOrigin();
-    computeScrollDimensions();
-
-    // Layout may cause us to be at an invalid scroll position. In this case we need
-    // to pull our scroll offsets back to the max (or push them up to the min).
-    DoublePoint clampedScrollPosition = clampScrollPosition(scrollPositionDouble());
-    if (clampedScrollPosition != scrollPositionDouble()) {
-        scrollToPosition(clampedScrollPosition);
-    } else if (originalOrigin != scrollOrigin()) {
-        // TODO: We should be able to use scrollOriginChanged() here, but we can't because
-        // PaintLayerScrollableArea does not maintain that flag: it gets set, but it never
-        // gets unset.  We should unset the flag after layout.
-        scrollPositionChanged(scrollPositionDouble(), ProgrammaticScroll);
-    }
-
-    m_scrollbarManager.setCanDetachScrollbars(false);
+    updateScrollDimensions();
 
     bool hasHorizontalOverflow = this->hasHorizontalOverflow();
     bool hasVerticalOverflow = this->hasVerticalOverflow();
@@ -652,11 +640,11 @@ bool PaintLayerScrollableArea::updateAfterLayout(SubtreeLayoutScope* delayedLayo
 
     // We need to layout again if scrollbars are added or removed by overflow:auto,
     // or by changing between native and custom.
-    bool horizontalScrollBarChanged = (box().hasAutoHorizontalScrollbar() && (hasHorizontalScrollbar() != hasHorizontalOverflow))
+    bool horizontalScrollbarChanged = (box().hasAutoHorizontalScrollbar() && (hasHorizontalScrollbar() != hasHorizontalOverflow))
         || (box().style()->overflowX() == OverflowScroll && !horizontalScrollbar());
-    bool verticalScrollBarChanged = (box().hasAutoVerticalScrollbar() && (hasVerticalScrollbar() != hasVerticalOverflow))
+    bool verticalScrollbarChanged = (box().hasAutoVerticalScrollbar() && (hasVerticalScrollbar() != hasVerticalOverflow))
         || (box().style()->overflowY() == OverflowScroll && !verticalScrollbar());
-    if (!visualViewportSuppliesScrollbars() && (horizontalScrollBarChanged || verticalScrollBarChanged)) {
+    if (!scrollbarsAreFrozen && !visualViewportSuppliesScrollbars() && (horizontalScrollbarChanged || verticalScrollbarChanged)) {
         if (box().hasAutoHorizontalScrollbar())
             setHasHorizontalScrollbar(hasHorizontalOverflow);
         else if (box().style()->overflowX() == OverflowScroll)
@@ -676,29 +664,34 @@ bool PaintLayerScrollableArea::updateAfterLayout(SubtreeLayoutScope* delayedLayo
             box().document().setAnnotatedRegionsDirty(true);
 
         // Our proprietary overflow: overlay value doesn't trigger a layout.
-        if ((horizontalScrollBarChanged && box().style()->overflowX() != OverflowOverlay) || (verticalScrollBarChanged && box().style()->overflowY() != OverflowOverlay)) {
+        if ((horizontalScrollbarChanged && box().style()->overflowX() != OverflowOverlay) || (verticalScrollbarChanged && box().style()->overflowY() != OverflowOverlay)) {
             if (!m_inOverflowRelayout) {
-                m_inOverflowRelayout = true;
-                if (delayedLayoutScope) {
+                if ((verticalScrollbarChanged && box().isHorizontalWritingMode())
+                    || (horizontalScrollbarChanged && !box().isHorizontalWritingMode())) {
+                    box().setPreferredLogicalWidthsDirty();
+                    box().updateLogicalWidth();
+                }
+                if (relayoutIsPrevented) {
                     if (box().isLayoutBlock())
-                        toLayoutBlock(box()).scrollbarsChanged(horizontalScrollBarChanged, verticalScrollBarChanged);
-                    delayedLayoutScope->setNeedsLayout(&box(), LayoutInvalidationReason::ScrollbarChanged);
-                    didMarkForDelayedLayout = true;
+                        toLayoutBlock(box()).scrollbarsChanged(horizontalScrollbarChanged, verticalScrollbarChanged);
+                    PreventRelayoutScope::setNeedsLayout(box());
                 } else {
+                    m_inOverflowRelayout = true;
                     SubtreeLayoutScope layoutScope(box());
                     layoutScope.setNeedsLayout(&box(), LayoutInvalidationReason::ScrollbarChanged);
                     if (box().isLayoutBlock()) {
                         LayoutBlock& block = toLayoutBlock(box());
-                        block.scrollbarsChanged(horizontalScrollBarChanged, verticalScrollBarChanged);
+                        block.scrollbarsChanged(horizontalScrollbarChanged, verticalScrollbarChanged);
                         block.layoutBlock(true);
                     } else {
                         box().layout();
                     }
+                    m_inOverflowRelayout = false;
+                    m_scrollbarManager.destroyDetachedScrollbars();
                 }
                 LayoutObject* parent = box().parent();
                 if (parent && parent->isFlexibleBox())
                     toLayoutFlexibleBox(parent)->clearCachedMainSizeForChild(box());
-                m_inOverflowRelayout = false;
             }
         }
     }
@@ -718,20 +711,45 @@ bool PaintLayerScrollableArea::updateAfterLayout(SubtreeLayoutScope* delayedLayo
         }
     }
 
-    if (hasOverlayScrollbars()) {
+    if (!scrollbarsAreFrozen && hasOverlayScrollbars()) {
         if (!scrollSize(HorizontalScrollbar))
             setHasHorizontalScrollbar(false);
         if (!scrollSize(VerticalScrollbar))
             setHasVerticalScrollbar(false);
     }
 
-    bool hasOverflow = hasScrollableHorizontalOverflow() || hasScrollableVerticalOverflow();
-    updateScrollableAreaSet(hasOverflow);
+    clampScrollPositionsAfterLayout();
+
+    if (!scrollbarsAreFrozen) {
+        bool hasOverflow = hasScrollableHorizontalOverflow() || hasScrollableVerticalOverflow();
+        updateScrollableAreaSet(hasOverflow);
+    }
 
     DisableCompositingQueryAsserts disabler;
     positionOverflowControls();
+}
 
-    return didMarkForDelayedLayout;
+
+void PaintLayerScrollableArea::clampScrollPositionsAfterLayout()
+{
+    // If a vertical scrollbar was removed, the min/max scroll positions may have changed,
+    // so the scroll positions needs to be clamped.  If the scroll position did not change,
+    // but the scroll origin *did* change, we still need to notify the scrollbars to
+    // update their dimensions.
+
+    if (DelayScrollPositionClampScope::clampingIsDelayed()) {
+        DelayScrollPositionClampScope::setNeedsClamp(this);
+        return;
+    }
+
+    DoublePoint newScrollPosition = clampScrollPosition(scrollPositionDouble());
+    if (newScrollPosition != scrollPositionDouble())
+        ScrollableArea::setScrollPosition(newScrollPosition, ProgrammaticScroll);
+    else if (scrollOriginChanged())
+        scrollPositionChanged(newScrollPosition, ProgrammaticScroll);
+
+    setNeedsScrollPositionClamp(false);
+    resetScrollOriginChanged();
 }
 
 ScrollBehavior PaintLayerScrollableArea::scrollBehaviorStyle() const
@@ -799,7 +817,6 @@ void PaintLayerScrollableArea::updateAfterStyleChange(const ComputedStyle* oldSt
     EOverflow overflowX = box().style()->overflowX();
     EOverflow overflowY = box().style()->overflowY();
 
-    // To avoid doing a relayout in updateScrollbarsAfterLayout, we try to keep any automatic scrollbar that was already present.
     bool needsHorizontalScrollbar = (hasHorizontalScrollbar() && overflowDefinesAutomaticScrollbar(overflowX)) || overflowRequiresScrollbar(overflowX);
     bool needsVerticalScrollbar = (hasVerticalScrollbar() && overflowDefinesAutomaticScrollbar(overflowY)) || overflowRequiresScrollbar(overflowY);
 
@@ -870,7 +887,7 @@ bool PaintLayerScrollableArea::updateAfterCompositingChange()
 
 void PaintLayerScrollableArea::updateAfterOverflowRecalc()
 {
-    computeScrollDimensions();
+    updateScrollDimensions();
     if (Scrollbar* horizontalScrollbar = this->horizontalScrollbar()) {
         int clientWidth = box().pixelSnappedClientWidth();
         horizontalScrollbar->setProportion(clientWidth, overflowRect().width());
@@ -882,9 +899,9 @@ void PaintLayerScrollableArea::updateAfterOverflowRecalc()
 
     bool hasHorizontalOverflow = this->hasHorizontalOverflow();
     bool hasVerticalOverflow = this->hasVerticalOverflow();
-    bool autoHorizontalScrollBarChanged = box().hasAutoHorizontalScrollbar() && (hasHorizontalScrollbar() != hasHorizontalOverflow);
-    bool autoVerticalScrollBarChanged = box().hasAutoVerticalScrollbar() && (hasVerticalScrollbar() != hasVerticalOverflow);
-    if (autoHorizontalScrollBarChanged || autoVerticalScrollBarChanged)
+    bool autoHorizontalScrollbarChanged = box().hasAutoHorizontalScrollbar() && (hasHorizontalScrollbar() != hasHorizontalOverflow);
+    bool autoVerticalScrollbarChanged = box().hasAutoVerticalScrollbar() && (hasVerticalScrollbar() != hasVerticalOverflow);
+    if (autoHorizontalScrollbarChanged || autoVerticalScrollbarChanged)
         box().setNeedsLayoutAndFullPaintInvalidation(LayoutInvalidationReason::Unknown);
 }
 
@@ -1002,12 +1019,17 @@ bool PaintLayerScrollableArea::needsScrollbarReconstruction() const
 
 void PaintLayerScrollableArea::setHasHorizontalScrollbar(bool hasScrollbar)
 {
+    if (FreezeScrollbarsScope::scrollbarsAreFrozen())
+        return;
+
     if (hasScrollbar == hasHorizontalScrollbar())
         return;
 
     setScrollbarNeedsPaintInvalidation(HorizontalScrollbar);
 
-    m_scrollbarManager.setHasHorizontalScrollbar(hasScrollbar);
+    m_scrollbarManager.setHasHorizontalScrollbar(hasScrollbar, !m_inOverflowRelayout);
+
+    updateScrollOrigin();
 
     // Destroying or creating one bar can cause our scrollbar corner to come and go. We need to update the opposite scrollbar's style.
     if (hasHorizontalScrollbar())
@@ -1024,12 +1046,17 @@ void PaintLayerScrollableArea::setHasHorizontalScrollbar(bool hasScrollbar)
 
 void PaintLayerScrollableArea::setHasVerticalScrollbar(bool hasScrollbar)
 {
+    if (FreezeScrollbarsScope::scrollbarsAreFrozen())
+        return;
+
     if (hasScrollbar == hasVerticalScrollbar())
         return;
 
     setScrollbarNeedsPaintInvalidation(VerticalScrollbar);
 
-    m_scrollbarManager.setHasVerticalScrollbar(hasScrollbar);
+    m_scrollbarManager.setHasVerticalScrollbar(hasScrollbar, !m_inOverflowRelayout);
+
+    updateScrollOrigin();
 
     // Destroying or creating one bar can cause our scrollbar corner to come and go. We need to update the opposite scrollbar's style.
     if (hasHorizontalScrollbar())
@@ -1500,7 +1527,6 @@ CompositorAnimationTimeline* PaintLayerScrollableArea::compositorAnimationTimeli
 
 PaintLayerScrollableArea::ScrollbarManager::ScrollbarManager(PaintLayerScrollableArea& scrollableArea)
     : m_scrollableArea(&scrollableArea)
-    , m_canDetachScrollbars(0)
     , m_hBarIsAttached(0)
     , m_vBarIsAttached(0)
 {
@@ -1508,25 +1534,22 @@ PaintLayerScrollableArea::ScrollbarManager::ScrollbarManager(PaintLayerScrollabl
 
 void PaintLayerScrollableArea::ScrollbarManager::dispose()
 {
-    m_canDetachScrollbars = m_hBarIsAttached = m_vBarIsAttached = 0;
+    m_hBarIsAttached = m_vBarIsAttached = 0;
     destroyScrollbar(HorizontalScrollbar);
     destroyScrollbar(VerticalScrollbar);
 }
 
-void PaintLayerScrollableArea::ScrollbarManager::setCanDetachScrollbars(bool detach)
+void PaintLayerScrollableArea::ScrollbarManager::destroyDetachedScrollbars()
 {
     ASSERT(!m_hBarIsAttached || m_hBar);
     ASSERT(!m_vBarIsAttached || m_vBar);
-    m_canDetachScrollbars = detach ? 1 : 0;
-    if (!detach) {
-        if (m_hBar && !m_hBarIsAttached)
-            destroyScrollbar(HorizontalScrollbar);
-        if (m_vBar && !m_vBarIsAttached)
-            destroyScrollbar(VerticalScrollbar);
-    }
+    if (m_hBar && !m_hBarIsAttached)
+        destroyScrollbar(HorizontalScrollbar);
+    if (m_vBar && !m_vBarIsAttached)
+        destroyScrollbar(VerticalScrollbar);
 }
 
-void PaintLayerScrollableArea::ScrollbarManager::setHasHorizontalScrollbar(bool hasScrollbar)
+void PaintLayerScrollableArea::ScrollbarManager::setHasHorizontalScrollbar(bool hasScrollbar, bool canDetach)
 {
     if (hasScrollbar) {
         // This doesn't hit in any tests, but since the equivalent code in setHasVerticalScrollbar
@@ -1540,15 +1563,14 @@ void PaintLayerScrollableArea::ScrollbarManager::setHasHorizontalScrollbar(bool
         } else {
             m_hBarIsAttached = 1;
         }
-
     } else {
         m_hBarIsAttached = 0;
-        if (!m_canDetachScrollbars)
+        if (!canDetach)
             destroyScrollbar(HorizontalScrollbar);
     }
 }
 
-void PaintLayerScrollableArea::ScrollbarManager::setHasVerticalScrollbar(bool hasScrollbar)
+void PaintLayerScrollableArea::ScrollbarManager::setHasVerticalScrollbar(bool hasScrollbar, bool canDetach)
 {
     if (hasScrollbar) {
         DisableCompositingQueryAsserts disabler;
@@ -1560,10 +1582,9 @@ void PaintLayerScrollableArea::ScrollbarManager::setHasVerticalScrollbar(bool ha
         } else {
             m_vBarIsAttached = 1;
         }
-
     } else {
         m_vBarIsAttached = 0;
-        if (!m_canDetachScrollbars)
+        if (!canDetach)
             destroyScrollbar(VerticalScrollbar);
     }
 }
@@ -1614,4 +1635,83 @@ DEFINE_TRACE(PaintLayerScrollableArea::ScrollbarManager)
     visitor->trace(m_vBar);
 }
 
+int PaintLayerScrollableArea::PreventRelayoutScope::s_count = 0;
+SubtreeLayoutScope* PaintLayerScrollableArea::PreventRelayoutScope::s_layoutScope = nullptr;
+bool PaintLayerScrollableArea::PreventRelayoutScope::s_relayoutNeeded = false;
+WTF::Vector<LayoutObject*>* PaintLayerScrollableArea::PreventRelayoutScope::s_needsRelayout = nullptr;
+
+PaintLayerScrollableArea::PreventRelayoutScope::PreventRelayoutScope(SubtreeLayoutScope& layoutScope)
+{
+    if (!s_count) {
+        ASSERT(!s_layoutScope);
+        ASSERT(!s_needsRelayout || s_needsRelayout->isEmpty());
+        s_layoutScope = &layoutScope;
+    }
+    s_count++;
+}
+
+PaintLayerScrollableArea::PreventRelayoutScope::~PreventRelayoutScope()
+{
+    if (--s_count == 0) {
+        if (s_relayoutNeeded) {
+            for (auto layoutObject : *s_needsRelayout)
+                s_layoutScope->setNeedsLayout(layoutObject, LayoutInvalidationReason::ScrollbarChanged);
+            s_needsRelayout->clear();
+        }
+        s_layoutScope = nullptr;
+    }
+}
+
+void PaintLayerScrollableArea::PreventRelayoutScope::setNeedsLayout(LayoutObject& layoutObject)
+{
+    ASSERT(s_count);
+    ASSERT(s_layoutScope);
+    s_relayoutNeeded = true;
+    if (!s_needsRelayout)
+        s_needsRelayout = new WTF::Vector<LayoutObject*>();
+    s_needsRelayout->append(&layoutObject);
+}
+
+void PaintLayerScrollableArea::PreventRelayoutScope::resetRelayoutNeeded()
+{
+    ASSERT(s_count == 0);
+    ASSERT(!s_needsRelayout || s_needsRelayout->isEmpty());
+    s_relayoutNeeded = false;
+}
+
+int PaintLayerScrollableArea::FreezeScrollbarsScope::s_count = 0;
+
+int PaintLayerScrollableArea::DelayScrollPositionClampScope::s_count = 0;
+PersistentHeapVector<Member<PaintLayerScrollableArea>>* PaintLayerScrollableArea::DelayScrollPositionClampScope::s_needsClamp = nullptr;
+
+PaintLayerScrollableArea::DelayScrollPositionClampScope::DelayScrollPositionClampScope()
+{
+    if (!s_needsClamp)
+        s_needsClamp = new PersistentHeapVector<Member<PaintLayerScrollableArea>>();
+    ASSERT(s_count > 0 || s_needsClamp->isEmpty());
+    s_count++;
+}
+
+PaintLayerScrollableArea::DelayScrollPositionClampScope::~DelayScrollPositionClampScope()
+{
+    if (--s_count == 0)
+        DelayScrollPositionClampScope::clampScrollableAreas();
+}
+
+void PaintLayerScrollableArea::DelayScrollPositionClampScope::setNeedsClamp(PaintLayerScrollableArea* scrollableArea)
+{
+    if (!scrollableArea->needsScrollPositionClamp()) {
+        scrollableArea->setNeedsScrollPositionClamp(true);
+        s_needsClamp->append(scrollableArea);
+    }
+}
+
+void PaintLayerScrollableArea::DelayScrollPositionClampScope::clampScrollableAreas()
+{
+    for (auto& scrollableArea : *s_needsClamp)
+        scrollableArea->clampScrollPositionsAfterLayout();
+    delete s_needsClamp;
+    s_needsClamp = nullptr;
+}
+
 } // namespace blink