[webkit-reviews] review granted: [Bug 67858] Pixel result shows that compositing/iframes/repaint-after-losing-scrollbars.html is failing : [Attachment 106952] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Simon Fraser (smfr)
<simon.fraser at apple.com>'s request for review:
Bug 67858: Pixel result shows that
compositing/iframes/repaint-after-losing-scrollbars.html is failing
https://bugs.webkit.org/show_bug.cgi?id=67858

Attachment 106952: Patch
https://bugs.webkit.org/attachment.cgi?id=106952&action=review

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


> Source/WebCore/ChangeLog:15
> +	   Tested by compositing/iframes/repaint-after-losing-scrollbars.html,
even though the pixel
> +	   result doesn't show the correct result (bug 67878).

Does the expected result of that test need to be updated, or is it already
failing?

> Source/WebCore/platform/ScrollView.cpp:583
> +	   if (!m_horizontalScrollbar && !m_verticalScrollbar &&
!oldScrollCornerRect.isEmpty())
> +	       invalidateScrollCornerRect(oldScrollCornerRect);

I’m not sure I understand which call sites are responsible for checking if the
rectangle is empty. There is a check here, but no check in
RenderLayerCompositor::destroyRootLayer. Is it important to check everywhere?
Or unnecessary to check anywhere? Or helpful to check in some places even if
not in others?

> Source/WebCore/rendering/RenderLayerCompositor.cpp:1746
> +	  
m_renderView->frameView()->invalidateScrollCorner(m_renderView->frameView()->sc
rollCornerRect());

If frameView() is a non-trivial function it might be nice to call it only once.