Commit 662a21f7 authored by szager's avatar szager Committed by Commit bot

Avoid scrollbar construction/destruction thrashing during flex layout.

BUG=528940
R=skobes@chromium.org,pdr@chromium.org

Before this change:

https://codereview.chromium.org/1295933003

... blocks with overflow:auto would delay updating their scrollbars until
after all flex layout was finished.  After the change, scrollbar info is
updated immediately during the course of layout.  Flex items may run
layout multiple times during flex layout; if a flex item has auto scrollbars,
it may create and destroy its scrollbars multiple times.

Aside from being a performance problem, this can cause WebScrollbarThemePainter
to point to a stale Scrollbar object.  If a flex item has scrollbars prior to
layout; then the flex item destroys and creates scrollbars during layout; and
at the end of layout, it still has a scrollbar; then
CompositedDeprecatedPaintLayerMapping::updateOverflowControlsLayers will not
update the WebScrollbarThemePainter with the final Scrollbar object.

We could fix this in updateOverflowControlsLayers, but that wouldn't address
the performance issue of needlessly creating and destroying scrollbars during
flex layout.  This patch avoids destroying scrollbars that are no longer
deemed necessary, until after all flexing is finished.

Review URL: https://codereview.chromium.org/1357423007

Cr-Commit-Position: refs/heads/master@{#351130}
parent bcfea02f
...@@ -67,6 +67,53 @@ class CORE_EXPORT DeprecatedPaintLayerScrollableArea final : public NoBaseWillBe ...@@ -67,6 +67,53 @@ class CORE_EXPORT DeprecatedPaintLayerScrollableArea final : public NoBaseWillBe
WILL_BE_USING_GARBAGE_COLLECTED_MIXIN(DeprecatedPaintLayerScrollableArea); WILL_BE_USING_GARBAGE_COLLECTED_MIXIN(DeprecatedPaintLayerScrollableArea);
friend class Internals; friend class Internals;
private:
class ScrollbarManager {
// Helper class to manage the life cycle of Scrollbar objects. Some layout containers
// (e.g., flexbox, table) run multi-pass layout on their children, applying different
// constraints. If a child has overflow:auto, it may gain and lose scrollbars multiple
// times during multi-pass layout, causing pointless allocation/deallocation thrashing,
// and potentially leading to other problems (crbug.com/528940).
// ScrollbarManager allows a ScrollableArea to delay the destruction of a scrollbar that
// is no longer needed, until the end of multi-pass layout. If the scrollbar is then
// re-added before multi-pass layout finishes, the previously "deleted" scrollbar will
// be restored, rather than constructing a new one.
public:
ScrollbarManager(DeprecatedPaintLayerScrollableArea&);
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(); }
bool hasVerticalScrollbar() const { return verticalScrollbar(); }
void setHasHorizontalScrollbar(bool hasScrollbar);
void setHasVerticalScrollbar(bool hasScrollbar);
DECLARE_TRACE();
private:
PassRefPtrWillBeRawPtr<Scrollbar> createScrollbar(ScrollbarOrientation);
void destroyScrollbar(ScrollbarOrientation, bool invalidate = false);
private:
DeprecatedPaintLayerScrollableArea& m_scrollableArea;
RefPtrWillBeMember<Scrollbar> m_hBar;
RefPtrWillBeMember<Scrollbar> m_vBar;
unsigned m_canDetachScrollbars: 1;
unsigned m_hBarIsAttached: 1;
unsigned m_vBarIsAttached: 1;
};
public: public:
// FIXME: We should pass in the LayoutBox but this opens a window // FIXME: We should pass in the LayoutBox but this opens a window
// for crashers during DeprecatedPaintLayer setup (see crbug.com/368062). // for crashers during DeprecatedPaintLayer setup (see crbug.com/368062).
...@@ -81,8 +128,8 @@ public: ...@@ -81,8 +128,8 @@ public:
bool hasHorizontalScrollbar() const { return horizontalScrollbar(); } bool hasHorizontalScrollbar() const { return horizontalScrollbar(); }
bool hasVerticalScrollbar() const { return verticalScrollbar(); } bool hasVerticalScrollbar() const { return verticalScrollbar(); }
Scrollbar* horizontalScrollbar() const override { return m_hBar.get(); } Scrollbar* horizontalScrollbar() const override { return m_scrollbarManager.horizontalScrollbar(); }
Scrollbar* verticalScrollbar() const override { return m_vBar.get(); } Scrollbar* verticalScrollbar() const override { return m_scrollbarManager.verticalScrollbar(); }
HostWindow* hostWindow() const override; HostWindow* hostWindow() const override;
...@@ -162,7 +209,7 @@ public: ...@@ -162,7 +209,7 @@ public:
bool updateAfterCompositingChange() override; bool updateAfterCompositingChange() override;
bool hasScrollbar() const { return m_hBar || m_vBar; } bool hasScrollbar() const { return hasHorizontalScrollbar() || hasVerticalScrollbar(); }
LayoutScrollbarPart* scrollCorner() const { return m_scrollCorner; } LayoutScrollbarPart* scrollCorner() const { return m_scrollCorner; }
...@@ -254,9 +301,6 @@ private: ...@@ -254,9 +301,6 @@ private:
LayoutUnit horizontalScrollbarStart(int minX) const; LayoutUnit horizontalScrollbarStart(int minX) const;
IntSize scrollbarOffset(const Scrollbar*) const; IntSize scrollbarOffset(const Scrollbar*) const;
PassRefPtrWillBeRawPtr<Scrollbar> createScrollbar(ScrollbarOrientation);
void destroyScrollbar(ScrollbarOrientation);
void setHasHorizontalScrollbar(bool hasScrollbar); void setHasHorizontalScrollbar(bool hasScrollbar);
void setHasVerticalScrollbar(bool hasScrollbar); void setHasVerticalScrollbar(bool hasScrollbar);
...@@ -289,15 +333,14 @@ private: ...@@ -289,15 +333,14 @@ private:
// The width/height of our scrolled area. // The width/height of our scrolled area.
LayoutRect m_overflowRect; LayoutRect m_overflowRect;
// ScrollbarManager holds the Scrollbar instances.
ScrollbarManager m_scrollbarManager;
// This is the (scroll) offset from scrollOrigin(). // This is the (scroll) offset from scrollOrigin().
DoubleSize m_scrollOffset; DoubleSize m_scrollOffset;
IntPoint m_cachedOverlayScrollbarOffset; IntPoint m_cachedOverlayScrollbarOffset;
// For areas with overflow, we have a pair of scrollbars.
RefPtrWillBeMember<Scrollbar> m_hBar;
RefPtrWillBeMember<Scrollbar> m_vBar;
// LayoutObject to hold our custom scroll corner. // LayoutObject to hold our custom scroll corner.
LayoutScrollbarPart* m_scrollCorner; LayoutScrollbarPart* m_scrollCorner;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment