Chromium Code Reviews Chromium Code Reviews
Issue
1295933003:
Add overflow:auto scrollbars to child flex basis. (Closed)
DescriptionAdd overflow:auto scrollbars to child flex basis.
LayoutBlocks delay updating their scrolling information until the
uppermost flex box has finished layout. However, if the flex item
has overflow:auto, this means that the scrollbar width is not
included in the flex item's logical height during flexing. This
leads to spurious scrollbars in the flex direction.
The original reason for delaying the update of scrolling information
was to fix a bug where scroll positions would be lost when a flexbox
changed:
https://bugs.webkit.org/show_bug.cgi?id=97706
For better or worse, this also turned out to be a performance
optimization (albeit at the expense of correctness). Updating the
scroll information can cause a block to run layout again, if the
existence of scrollbars has changed for the block. If a block in a
nested hierarchy of flexboxes runs layout N times during flexing, it
would then potentially run one additional layout at the end of
flexing, for a total of N+1. However, if the the optimization is
removed (i.e., scroll info updating for the flex item is not delayed, but
run immediately at the end of layout, potentially triggering a second
layout if scrollbars changed), then the potential maximum number of
layouts during flexing is N*2.
This patch fixes the correctness issue while preserving the
optimization: when a flex item runs layout, and its scrollbars have
changed, it immediately adds the size of the scrollbars to its
logical width/height, but defers the second layout until flexing is
finished. The implementation splits the existing code from
DeprecatedPaintLayerScrollableArea::updateAfterLayout() into the
methods updateScrollDimensions, which is called immediately at the
end of layout; and finalizeScrollDimensions, which is called at the
end of flexing. When layout happens outside the context of flexing,
the two methods are called in immediate sequence, preserving existing
behavior.
BUG=512229
R=ojan@chromium.org,cbiesinger@chromium.org,leviw@chromium.org
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=201824
Patch Set 1 #Patch Set 2 : Add all tests, even failing ones #
Total comments: 10
Patch Set 3 : Nits scratched, fixed test to pass. #Patch Set 4 : rebase #
Total comments: 1
Patch Set 5 : Add entries to SlowTests #Messages
Total messages: 21 (7 generated)
|
