[webkit-reviews] review denied: [Bug 130891] [CSS Blending] Blending doesn't work if the parent stacking context is a normal flow only element : [Attachment 230376] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has denied Ion Rosca
<rosca at adobe.com>'s request for review:
Bug 130891: [CSS Blending] Blending doesn't work if the parent stacking context
is a normal flow only element
https://bugs.webkit.org/show_bug.cgi?id=130891

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

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


> Source/WebCore/rendering/RenderLayer.cpp:825
> +    if (m_hasUnisolatedBlendingDescendantsStatusDirty) {

Do an early return instead of having the whole body indented.

> Source/WebCore/rendering/RenderLayerCompositor.cpp:1108
> +#if ENABLE(CSS_COMPOSITING)
> +    layer.updateIsolationForBlendingDescendantsIfNeeded();
> +#endif
>      layer.updateDescendantDependentFlags();

Can layer.updateIsolationForBlendingDescendantsIfNeeded() be rolled into
layer.updateDescendantDependentFlags()? That would avoid adding an extra child
walk.

> Source/WebCore/rendering/RenderLayerModelObject.cpp:157
> +#if ENABLE(CSS_COMPOSITING)
> +	   if (oldStyle->hasBlendMode())
> +	       layer()->parent()->dirtyAncestorChainHasBlendingDescendants();
> +#endif

Can't we do this in RenderLayer::styleChanged()?

> LayoutTests/ChangeLog:19
> +	   *
css3/compositing/blend-mode-isolation-accelerated-overflow-hidden-expected.txt:
Added.
> +	   *
css3/compositing/blend-mode-isolation-accelerated-overflow-hidden.html: Added.
> +	   *
css3/compositing/blend-mode-isolation-overflow-hidden-expected.html: Added.
> +	   * css3/compositing/blend-mode-isolation-overflow-hidden.html: Added.

> +	   *
css3/compositing/blend-mode-isolation-turn-off-self-painting-layer-expected.txt
: Added.
> +	   *
css3/compositing/blend-mode-isolation-turn-off-self-painting-layer.html: Added.

> +	   *
css3/compositing/blend-mode-isolation-turn-off-self-painting-layer1-expected.tx
t: Added.
> +	   *
css3/compositing/blend-mode-isolation-turn-off-self-painting-layer1.html:
Added.
> +	   *
css3/compositing/blend-mode-isolation-turn-off-self-painting-layer2-expected.tx
t: Added.
> +	   *
css3/compositing/blend-mode-isolation-turn-off-self-painting-layer2.html:
Added.
> +	   *
css3/compositing/blend-mode-isolation-turn-on-self-painting-layer-expected.txt:
Added.
> +	   *
css3/compositing/blend-mode-isolation-turn-on-self-painting-layer.html: Added.

Can we make these ref tests please?