[webkit-reviews] review denied: [Bug 119033] Background doesn't fully repaint on this page : [Attachment 207846] Patch

Fri Aug 2 14:51:09 PDT 2013

Simon Fraser (smfr) <simon.fraser at apple.com> has denied Zalan Bujtas
<zalan at apple.com>'s request for review:
Bug 119033: Background doesn't fully repaint on this page
https://bugs.webkit.org/show_bug.cgi?id=119033

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

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=207846&action=review

> Source/WebCore/page/FrameView.cpp:2023
> +void FrameView::repaintFullContent()
> +{
> +    RenderView* renderView = this->renderView();
> +    if (!renderView)
> +	   return;
> +
> +#if USE(ACCELERATED_COMPOSITING)
> +    RenderLayer* layer = renderView->layer();
> +    if (layer->isComposited()) {
> +	   layer->setBackingNeedsRepaint();
> +	   return;
> +    }
> +#endif
> +    renderView->repaint();
> +}

Since this just gets the RenderView and does stuff to it, I think it should be
on RenderView, which would simplify the call sites. So I think this should just
be moved to RenderView and called something like repaintRootContents() or
something.

> Source/WebCore/rendering/RenderBox.cpp:211
> +	   if (diff >= StyleDifferenceRepaint && (isRoot() || isBody())) {

I'm not sure that isRoot() and node()->hasTagName(htmlTag) are equivalent in
all cases. E.g. what about <html> inside SVG foreign object?