[webkit-reviews] review granted: [Bug 112749] [CSS Grid Layout] Improper repainting when grid item change their position : [Attachment 193937] Proposed fix 1. Added a call to repaintDuringLayoutIfMoved along with extra repaint testing.

[bugzilla-daemon at webkit.org]

Ojan Vafai <ojan at chromium.org> has granted Julien Chaffraix
<jchaffraix at webkit.org>'s request for review:
Bug 112749: [CSS Grid Layout] Improper repainting when grid item change their
position
https://bugs.webkit.org/show_bug.cgi?id=112749

Attachment 193937: Proposed fix 1. Added a call to repaintDuringLayoutIfMoved
along with extra repaint testing.
https://bugs.webkit.org/attachment.cgi?id=193937&action=review

------- Additional Comments from Ojan Vafai <ojan at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=193937&action=review


> Source/WebCore/rendering/RenderGrid.cpp:677
> +	   if (!selfNeedsLayout() && child->checkForRepaintDuringLayout())
> +	       child->repaintDuringLayoutIfMoved(oldChildRect);

It bugs me that we're copy-pasting this pattern all over the place. Would be
nice if we had a function on RenderBox:
void RenderBox::repaintChildDuringLayoutIfMoved(RenderBox* child, LayoutRect
oldChildRect)
{
    // If the child moved, we have to repaint it as well as any
floating/positioned
    // descendants.  An exception is if we need a layout.  In this case, we
know we're going to
    // repaint ourselves (and the child) anyway.
    if (!selfNeedsLayout() && child->checkForRepaintDuringLayout())
	child->repaintDuringLayoutIfMoved(oldChildRect);
}

I copy-pasted that comment from RenderFlexibleBox since I needed that extra
explanation for why the !selfNeedsLayout() check is correct.