[webkit-reviews] review granted: [Bug 81685] [New Multicolumn] Add RenderMultiColumnFlowThread class and refactor the base class : [Attachment 134368] Patch

[bugzilla-daemon at webkit.org]

Julien Chaffraix <jchaffraix at webkit.org> has granted Dave Hyatt
<hyatt at apple.com>'s request for review:
Bug 81685: [New Multicolumn] Add RenderMultiColumnFlowThread class and refactor
the base class
https://bugs.webkit.org/show_bug.cgi?id=81685

Attachment 134368: Patch
https://bugs.webkit.org/attachment.cgi?id=134368&action=review

------- Additional Comments from Julien Chaffraix <jchaffraix at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=134368&action=review


r=me provided it passes the EWS.

> Source/WebCore/rendering/RenderNamedFlowThread.cpp:91
> +    // Do not add anonymous objects.
> +    if (!newChild->node())
> +	   return;

That's weird, normally anonymous objects have node() set to the Document. It's
unrelated to your change though.

> Source/WebCore/rendering/RenderNamedFlowThread.h:47
> +    AtomicString name() const { return m_name; }

I am not a huge fan of name() as it is very generic and would convey something
similar to renderName() in my view. How about flowName() or flowThreadName()?

> LayoutTests/ChangeLog:17
> +	   * fast/repaint/inline-relative-positioned-expected.txt:
> +	   * fast/repaint/overflow-clip-subtree-layout-expected.txt:
> +	   * fast/repaint/subtree-root-clip-2-expected.txt:
> +	   * fast/repaint/subtree-root-clip-3-expected.txt:
> +	   * fast/repaint/subtree-root-clip-expected.txt:

Those baselines are a fallout of lazily allocating layers. They are orthogonal
and preferably landed in a separate patch as they are right!