Right now, the presence of RenderMultiColumnBlock is a problem. By having a concrete class for multi-column blocks, I created a situation where table cells and list items (and the RenderView too) cannot be multi-column. The old code worked as mix-in to RenderBlock, and so it works. This bug is tracking the elimination of RenderMultiColumnBlock. I'll be folding its code into RenderBlockFlow. The first step is to eliminate all of RenderMultiColumnBlock's member variables and move them elsewhere.
Created attachment 221778 [details] Patch
Created attachment 221783 [details] Patch
Attachment 221783 [details] did not pass style-queue: ERROR: Source/WebCore/rendering/RenderMultiColumnFlowThread.h:64: One space before end of line comments [whitespace/comments] [5] ERROR: Source/WebCore/rendering/RenderMultiColumnFlowThread.h:66: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 2 in 8 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 221783 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=221783&action=review > Source/WebCore/rendering/RenderMultiColumnBlock.cpp:43 > + RenderMultiColumnFlowThread* flowThread = new RenderMultiColumnFlowThread(document(), RenderStyle::createAnonymousStyleWithDisplay(&style(), BLOCK)); > + flowThread->initializeStyle(); > + RenderBlockFlow::addChild(flowThread); > + setMultiColumnFlowThread(flowThread); Trying to work out the lifetime / ownership of the flowThread. We create it here, and set it as both a child and as a member of the RenderBlockFlow rareData. Does that mean it gets deleted as a result of the RenderBlockFlow's normal child deleting and we assume that corresponds with the rare data being torn down so we don't worry about the dangling pointer?
(In reply to comment #4) > (From update of attachment 221783 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=221783&action=review > > > Source/WebCore/rendering/RenderMultiColumnBlock.cpp:43 > > + RenderMultiColumnFlowThread* flowThread = new RenderMultiColumnFlowThread(document(), RenderStyle::createAnonymousStyleWithDisplay(&style(), BLOCK)); > > + flowThread->initializeStyle(); > > + RenderBlockFlow::addChild(flowThread); > > + setMultiColumnFlowThread(flowThread); > > Trying to work out the lifetime / ownership of the flowThread. We create it here, and set it as both a child and as a member of the RenderBlockFlow rareData. Does that mean it gets deleted as a result of the RenderBlockFlow's normal child deleting and we assume that corresponds with the rare data being torn down so we don't worry about the dangling pointer? Correct. It's torn down by anonymous child cleanup.
Note I didn't change the ownership. I just moved it from one class to another.
Comment on attachment 221783 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=221783&action=review > Source/WebCore/rendering/RenderBlockFlow.cpp:2895 > + RenderBlockFlowRareData& rareData = ensureRareBlockFlowData(); Can flowThread be null and thus not require the creation of rareData? > Source/WebCore/rendering/RenderBlockFlow.h:154 > + RenderMultiColumnFlowThread* m_multiColumnFlowThread; Could be a reference? There are lots of unchecked dereferences. > Source/WebCore/rendering/RenderMultiColumnBlock.cpp:95 > + multiColumnFlowThread()->setInBalancingPass(true); // Prevent re-entering this method (and recursion into layout). This should use TemporaryChange<> or some other early-return-proof class. > Source/WebCore/rendering/RenderMultiColumnFlowThread.h:48 > + bool computeColumnCountAndWidth(); Add a comment to say what the return values means. > Source/WebCore/rendering/RenderMultiColumnFlowThread.h:65 > + LayoutUnit m_columnWidth; // since a multi-column block that is split across variable width pages or regions will have different column counts and widths in each. since a -> A > Source/WebCore/rendering/RenderMultiColumnFlowThread.h:68 > + bool m_inBalancingPass; // Set when relayouting for column balancing. Comment doesn't add anything.
First round landed in r162702.