RESOLVED FIXED 162833
[MultiCol] Render tree should be all clean by the end of FrameView::layout(). 
https://bugs.webkit.org/show_bug.cgi?id=162833
Summary [MultiCol] Render tree should be all clean by the end of FrameView::layout().
alan
Reported 2016-09-30 19:34:36 PDT
We fail to clean all the renderers while running the following test: LayoutTests/imported/blink/fast/multicol/dynamic/multicol-with-abspos-svg-with-foreignobject-with-multicol-crash.html
Attachments
Patch (9.12 KB, patch)
2016-11-14 20:20 PST, alan 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
alan
Comment 1 2016-10-01 19:13:04 PDT
LayoutTests/fast/inline/quotation-text-changes-dynamically.html
alan
Comment 2 2016-10-04 11:50:33 PDT
LayoutTests/fast/css/content/quote-crash-when-floating.html
alan
Comment 3 2016-11-12 19:12:53 PST
(In reply to comment #0) > We fail to clean all the renderers while running the following test: > LayoutTests/imported/blink/fast/multicol/dynamic/multicol-with-abspos-svg- > with-foreignobject-with-multicol-crash.html Fixed by http://trac.webkit.org/changeset/208661
alan
Comment 4 2016-11-14 20:20:28 PST
Created attachment 294800 [details] Patch
Simon Fraser (smfr)
Comment 5 2016-11-14 20:48:01 PST
Comment on attachment 294800 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=294800&action=review > Source/WebCore/rendering/RenderMultiColumnFlowThread.cpp:172 > + RenderTreeInternalMutationScope reparentingIsOn(view()); Do you really want this scope to last until the end of the function?
Simon Fraser (smfr)
Comment 6 2016-11-14 20:51:03 PST
Comment on attachment 294800 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=294800&action=review > Source/WebCore/rendering/RenderView.h:206 > + bool renderTreeIsBeingMutatedInternally() const { return !!m_renderTreeInternalMutationCounter; } No need for !! (C++ converts non-0 to true for you).
alan
Comment 7 2016-11-14 20:53:58 PST
Comment on attachment 294800 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=294800&action=review >> Source/WebCore/rendering/RenderMultiColumnFlowThread.cpp:172 >> + RenderTreeInternalMutationScope reparentingIsOn(view()); > > Do you really want this scope to last until the end of the function? I do. While spanner removal probably won't impact the RenderQuote use case, it is still part of internal render tree mutation and in the future we might need to bail out in some other functions to avoid (similar) unintended changes.
WebKit Commit Bot
Comment 8 2016-11-15 08:02:48 PST
Comment on attachment 294800 [details] Patch Clearing flags on attachment: 294800 Committed r208731: <http://trac.webkit.org/changeset/208731>
WebKit Commit Bot
Comment 9 2016-11-15 08:02:52 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.