RESOLVED FIXED Bug 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().
zalan
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, zalan
no flags
zalan
Comment 1 2016-10-01 19:13:04 PDT
LayoutTests/fast/inline/quotation-text-changes-dynamically.html
zalan
Comment 2 2016-10-04 11:50:33 PDT
LayoutTests/fast/css/content/quote-crash-when-floating.html
zalan
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
zalan
Comment 4 2016-11-14 20:20:28 PST
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).
zalan
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.