[webkit-reviews] review granted: [Bug 110995] [css exclusions] Move ExclusionShapeInsideInfo into RenderBlockRareData : [Attachment 190895] Incorporating feedback

[bugzilla-daemon at webkit.org]

Julien Chaffraix <jchaffraix at webkit.org> has granted Bear Travis
<betravis at adobe.com>'s request for review:
Bug 110995: [css exclusions] Move ExclusionShapeInsideInfo into
RenderBlockRareData
https://bugs.webkit.org/show_bug.cgi?id=110995

Attachment 190895: Incorporating feedback
https://bugs.webkit.org/attachment.cgi?id=190895&action=review

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


> Source/WebCore/rendering/ExclusionShapeInsideInfo.cpp:43
> +    { }

AFAICT this should be on 2 lines. Though I don't think it's a style violation,
just the more usual way so it's probably a 'nit'.

> Source/WebCore/rendering/ExclusionShapeInsideInfo.h:45
> +struct LineSegmentIterator {

Not a super fan of the new Iterator to break a circular dependency but I
couldn't think of something smarter.

> Source/WebCore/rendering/ExclusionShapeInsideInfo.h:53
> +    { }

Same comment about this being on 2 lines here.

> Source/WebCore/rendering/RenderBlock.cpp:1404
> -	   ExclusionShapeInsideInfo::removeInfo(this);
> +	   setExclusionShapeInsideInfo(PassOwnPtr<ExclusionShapeInsideInfo>());


The way you instantiate the PassOwnPtr is weird.
setExclusionShapeInsideInfo(0); should work (using the PassOwnPtr(T*)
constructor).

> Source/WebCore/rendering/RenderBlock.h:454
> +	   if (!m_rareData)
> +	       m_rareData = adoptPtr(new RenderBlockRareData(this));
> +	   if (!m_rareData->m_shapeInsideInfo)
> +	       m_rareData->m_shapeInsideInfo =
ExclusionShapeInsideInfo::createInfo(this);

if (!m_rareData || !m_rareData->m_shapeInsideInfo)
   setExclusionShapeInsideInfo(ExclusionShapeInsideInfo::createInfo(this));