[webkit-reviews] review denied: [Bug 104366] Make order iterator member stack allocated in RenderFlexibleBox : [Attachment 179926] Updated patch

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has denied Carlos Garcia Campos
<cgarcia at igalia.com>'s request for review:
Bug 104366: Make order iterator member stack allocated in RenderFlexibleBox
https://bugs.webkit.org/show_bug.cgi?id=104366

Attachment 179926: Updated patch
https://bugs.webkit.org/attachment.cgi?id=179926&action=review

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=179926&action=review


> Source/WebCore/ChangeLog:8
> +	   It avoids heap allocations during the layout.

Nit: I would say, "It avoids a heap allocation during layout."

> Source/WebCore/rendering/RenderFlexibleBox.h:92
> +	   void setOrderValues(const OrderHashSet& orderValues)
> +	   {
> +	       reset();

Can we move this method definition into the .cpp file? That would also let you
keep OrderHashTraits in the .cpp file.

> Source/WebCore/rendering/RenderFlexibleBox.h:107
> +	   RenderBox* next()
> +	   {
> +	       do {

Can we move this method definition into the .cpp file?

> Source/WebCore/rendering/RenderFlexibleBox.h:133
> +    private:
> +	   RenderFlexibleBox* m_flexibleBox;

Maybe add WTF_MAKE_NONCOPYABLE(OrderIterator) to avoid accidental copies?