Chromium Code Reviews Chromium Code Reviews
Issue
1328923002:
Avoid stack overflow triggered by out-of-band layout of flexbox child. (Closed)
DescriptionAvoid stack overflow triggered by out-of-band layout of flexbox child.
Flexbox uses LayoutBlock::finishDelayUpdateScrollInfo(), which may jump to some
arbitrary block in the subtree and lay it out directly. This is bad, at least
for multicol, since it requires that a flow thread be entered have its children
laid out without skipping any parents in the chain.
LayoutMulticolFlowThread::skipColumnSpanner() was called on the wrong flow
thread, which resulted in m_lastSetWorkedOn pointing to a column set that was
part of an inner flow thread, and not the flow thread we called. That made
columnSetAtBlockOffset() return a column set from an inner flow thread, causing
an infinite recursion when walking upwards in the flow thread ancestry chain).
So the spanner was in the inner flow thread, but it was the outer flow thread
that got called. This happened because our LayoutState says that's the
innermost flow thread being laid out at the moment, because
finishDelayUpdateScrollInfo() just teleported and laid out a descendant of the
inner flow thread.
Luckily there's a safer way of obtaining the flow thread associated with a
spanner, so do that instead.
This is not a perfect fix. LayoutState still points to the wrong flow thread
(we just avoid asking it). The root cause remains (out-of-band layout), and
there may be other bugs caused by this. For multicol, we're still not in the
clear. Since we have a skipColumnSpanner() call without laying out the flow
thread, we'll get an assertion failure next time we lay out the flow thread,
because it will assert that there be no m_lastSetWorkedOn (but
skipColumnSpanner() has set that one while not laying out the flow thread).
But at least we're getting rid of the infinite recursion.
Since we now call skipColumnSpanner() on a LayoutMultiColumnFlowThread, instead
of on a LayoutFlowThread, we need to make that method public (private overrides
is just asking for trouble). And since this is the only caller, make it
non-virtual while at it. LayoutFlowThread no longer needs this.
BUG=526664
R=jchaffraix@chromium.org,leviw@chromium.org
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=202519
Patch Set 1 #Patch Set 2 : rebase master #Patch Set 3 : Update test. The previous one started to pass, because of https://codereview.chromium.org/129593300… #
Messages
Total messages: 9 (3 generated)
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||
