Refactor scroll updates during flexbox layout.

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

Index: third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.h
diff --git a/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.h b/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.h
index 2b7f91e72352016d2b89ffcafe4880e5d4a6cd2c..ca47be4e90f8961265b0ebefddb9cf4f59ab0d70 100644
--- a/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.h
+++ b/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.h
@@ -136,13 +136,6 @@ private:
 
         void dispose();
 
-        // When canDetachScrollbars is true, calls to setHas*Scrollbar(false) will NOT destroy
-        // an existing scrollbar, but instead detach it without destroying it.  If, subsequently,
-        // setHas*Scrollbar(true) is called, the existing scrollbar will be reattached.  When
-        // setCanDetachScrollbars(false) is called, any detached scrollbars will be destructed.
-        bool canDetachScrollbars() const { return m_canDetachScrollbars; }
-        void setCanDetachScrollbars(bool);
-
         Scrollbar* horizontalScrollbar() const { return m_hBarIsAttached ? m_hBar.get(): nullptr; }
         Scrollbar* verticalScrollbar() const { return m_vBarIsAttached ? m_vBar.get() : nullptr; }
         bool hasHorizontalScrollbar() const { return horizontalScrollbar(); }
@@ -150,6 +143,7 @@ private:
 
         void setHasHorizontalScrollbar(bool hasScrollbar);
         void setHasVerticalScrollbar(bool hasScrollbar);
+        void destroyDetachedScrollbars();
 
         DECLARE_TRACE();
 
@@ -164,12 +158,63 @@ private:
         Member<Scrollbar> m_hBar;
         Member<Scrollbar> m_vBar;
 
-        unsigned m_canDetachScrollbars: 1;
         unsigned m_hBarIsAttached: 1;
         unsigned m_vBarIsAttached: 1;
     };
 
 public:
+
+    // If a PreventRelayoutScope object is alive, updateAfterLayout() will not
+    // re-run box layout as a result of adding or removing scrollbars.
+    class PreventRelayoutScope {
+    public:
+        PreventRelayoutScope(SubtreeLayoutScope&);
+        ~PreventRelayoutScope();
+
+        static bool relayoutIsPrevented() { return s_count; }
+        static void setNeedsLayout(LayoutObject&);
+        static bool relayoutNeeded() { return s_count == 0 && s_relayoutNeeded; }
+        static void resetRelayoutNeeded();
+
+    private:
+        static int s_count;
+        static SubtreeLayoutScope* s_layoutScope;
+        static bool s_relayoutNeeded;
+        static WTF::Vector<LayoutObject*>* s_needsRelayout;
+    };
+
+    // If a FreezeScrollbarScope object is alive, updateAfterLayout() will not
+    // recompute the existence of overflow:auto scrollbars.
+    class FreezeScrollbarsScope {
+    public:
+        FreezeScrollbarsScope() { s_count++; }

[eae, 2016/06/02 22:57:25]

Could we add a test or assert or something to guard against pathological growth
as that would indicate a failure in the logic. Perhaps an ASSERT if it's more
than the max DOM depth?

[szager1, 2016/06/03 22:33:11]

I made these classes STACK_ALLOCATED, is that good enough?

+        ~FreezeScrollbarsScope() { s_count--; }
+
+        static bool scrollbarsAreFrozen() { return s_count; }
+
+    private:
+        static int s_count;
+    };
+
+    // If a DelayScrollPositionClampScope object is alive, updateAfterLayout() will not
+    // clamp scroll positions to ensure they are in the valid range.  When
+    // the last DelayScrollPositionClampScope object is destructed, all PaintLayerScrollableArea's
+    // that delayed clamping their positions will immediately clamp them.
+    class DelayScrollPositionClampScope {
+    public:
+        DelayScrollPositionClampScope();
+        ~DelayScrollPositionClampScope();
+
+        static bool clampingIsDelayed() { return s_count; }
+        static void setNeedsClamp(PaintLayerScrollableArea*);
+
+    private:
+        static void clampScrollableAreas();
+
+        static int s_count;
+        static PersistentHeapVector<Member<PaintLayerScrollableArea>>* s_needsClamp;
+    };
+
     // FIXME: We should pass in the LayoutBox but this opens a window
     // for crashers during PaintLayer setup (see crbug.com/368062).
     static PaintLayerScrollableArea* create(PaintLayer& layer)
@@ -268,9 +313,12 @@ public:
         scrollToOffset(toDoubleSize(position), ScrollOffsetClamped, scrollBehavior, scrollType);
     }
 
-    // Returns true if a layout object was marked for layout. In such a case, the layout scope's root
-    // should be laid out again.
-    bool updateAfterLayout(SubtreeLayoutScope* = nullptr);
+    // TODO(szager): Actually run these after all of layout is finished.  Currently, they
+    // run at the end of box()'es layout (or after all flexbox layout has finished) but while
+    // document layout is still happening.
+    void updateAfterLayout();
+    void clampScrollPositionsAfterLayout();
+
     void updateAfterStyleChange(const ComputedStyle*);
     void updateAfterOverflowRecalc();
 
@@ -355,6 +403,9 @@ public:
     bool shouldRebuildVerticalScrollbarLayer() const { return m_rebuildVerticalScrollbarLayer; }
     void resetRebuildScrollbarLayerFlags();
 
+    bool needsScrollPositionClamp() const { return m_needsScrollPositionClamp; }
+    void setNeedsScrollPositionClamp(bool val) { m_needsScrollPositionClamp = val; }
+
     StickyConstraintsMap& stickyConstraintsMap() { return ensureRareData().m_stickyConstraintsMap; }
     void invalidateAllStickyConstraints();
     void invalidateStickyConstraintsFor(PaintLayer*, bool needsCompositingUpdate = true);
@@ -372,7 +423,8 @@ private:
 
     bool needsScrollbarReconstruction() const;
 
-    void computeScrollDimensions();
+    void updateScrollOrigin();
+    void updateScrollDimensions();
 
     void setScrollOffset(const DoublePoint&, ScrollType) override;
 
@@ -430,6 +482,8 @@ private:
     unsigned m_rebuildHorizontalScrollbarLayer : 1;
     unsigned m_rebuildVerticalScrollbarLayer : 1;
 
+    unsigned m_needsScrollPositionClamp : 1;
+
     // The width/height of our scrolled area.
     // This is OverflowModel's layout overflow translated to physical
     // coordinates. See OverflowModel for the different overflow and