[webkit-reviews] review granted: [Bug 55440] improve layout performance by reducing the traversal time of the floating objects : [Attachment 84682] patch addressing comment #12

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Yuqiang Xian
<yuqiang.xian at intel.com>'s request for review:
Bug 55440: improve layout performance by reducing the traversal time of the
floating objects
https://bugs.webkit.org/show_bug.cgi?id=55440

Attachment 84682: patch addressing comment #12
https://bugs.webkit.org/attachment.cgi?id=84682&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=84682&action=review

Can we double check that increase/decrease is called in all the right places?

I’m going to say review+ even though I think we may want a little more
refinement.

> Source/WebCore/rendering/RenderBlock.cpp:6243
> +inline void RenderBlock::FloatingObjects::clear()

If these are marked inline, I think they need to be in the file *before* the
first call to the function. Otherwise on at least some compilers I think this
won’t be inlined.

> Source/WebCore/rendering/RenderBlock.cpp:6250
> +inline void
RenderBlock::FloatingObjects::increaseObjectsCount(FloatingObject::Type type)

Ditto.

> Source/WebCore/rendering/RenderBlock.cpp:6258
> +inline void
RenderBlock::FloatingObjects::decreaseObjectsCount(FloatingObject::Type type)

Ditto.

> Source/WebCore/rendering/RenderBlock.h:721
> +	   FloatingObjects()
> +	       : m_leftObjectsCount(0)
> +	       , m_rightObjectsCount(0)
> +	   {
> +	   }

Ideally I’d like to see this function body inside the cpp file too; I think the
class is easier to read without the function body here.