[webkit-reviews] review denied: [Bug 55257] Support creating compositing layers for scrollable and overflowing objects : [Attachment 84779] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has denied Daniel Sievers
<sievers at google.com>'s request for review:
Bug 55257: Support creating compositing layers for scrollable and overflowing
objects
https://bugs.webkit.org/show_bug.cgi?id=55257

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

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

> Source/WebCore/rendering/RenderLayer.cpp:3816
> +	   || renderer()->isRenderIFrame()
> +	   || isComposited();

You can't do this; the test becomes circular. See
RenderLayerCompositor::canBeComposited().

Non-self-painting layers are used for overflow, so your changes here will break
rendering in some cases.

> Source/WebCore/rendering/RenderLayerCompositor.cpp:1230
> -    return m_hasAcceleratedCompositing && layer->isSelfPaintingLayer();
> +    if (!m_hasAcceleratedCompositing)
> +	   return false;
> +
> +    bool canBeComposited = layer->isSelfPaintingLayer();
> +
> +    if ((m_compositingTriggers & ChromeClient::ScrollableOverflowTrigger)
> +	   && layer->renderer()->isRenderBlock() &&
!layer->renderer()->isTextControl()) {
> +	   // Only do a loose check now, as this function will be queried
before layout.
> +	   // Whether there is actual overflow will be determined after layout
> +	   // by requiresCompositingForScrollableOverflow().
> +	   canBeComposited |=
toRenderBox(layer->renderer())->scrollsOverflow();
> +    }
> +
> +    return canBeComposited;

This seems wrong.