[webkit-reviews] review granted: [Bug 103510] Reduce the children repaints when moved multiple times during the layout : [Attachment 177453] Updated patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Carlos Garcia Campos
<cgarcia at igalia.com>'s request for review:
Bug 103510: Reduce the children repaints when moved multiple times during the
layout
https://bugs.webkit.org/show_bug.cgi?id=103510

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

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


> Source/WebCore/rendering/RenderFlexibleBox.cpp:338
> +    ChildrenFrameRects childrenOldRects;
> +    appendChildrenFrameRects(childrenOldRects);
>      layoutFlexItems(*m_orderIterator, lineContexts);

It’s strange that layoutFlexItems receives the iterator as an argument, but
appendChildrenFrameRects and repaintChildrenDuringLayoutIfMoved both get the
iterator directly from the data member. The three functions should do this
consistently.

> Source/WebCore/rendering/RenderFlexibleBox.cpp:396
> +}

I think we should add this here:

    ASSERT(childIndex == childrenOldRects.size());

> Source/WebCore/rendering/RenderFlexibleBox.h:81
> +    typedef Vector<LayoutRect, 8> ChildrenFrameRects;

Needs a comment explaining where the number 8 came from.

I think the name ChildFrameRects would be better. Since rects is plural,
children doesn’t also need to be plural. In English it would be "child frame
rectangles" not "children frame rectangles".