Bug 197695 - fast/hidpi/video-controls-in-hidpi.html sometimes asserts in WK1
Summary: fast/hidpi/video-controls-in-hidpi.html sometimes asserts in WK1
Alias: None
Product: WebKit
Classification: Unclassified
Component: Compositing (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Simon Fraser (smfr)
Keywords: InRadar
: 197731 (view as bug list)
Depends on:
Reported: 2019-05-08 10:17 PDT by Simon Fraser (smfr)
Modified: 2019-05-09 11:26 PDT (History)
4 users (show)

See Also:

Patch (2.94 KB, patch)
2019-05-09 07:14 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2019-05-08 10:17:52 PDT
With the patch from bug 197561, fast/hidpi/video-controls-in-hidpi.html sometimes asserts on EWS (I have not been able to reproduce).
Comment 1 Simon Fraser (smfr) 2019-05-08 10:18:25 PDT
The assertion is:
Comment 2 Simon Fraser (smfr) 2019-05-08 21:24:07 PDT
This code runs when we have a m_ancestorClippingLayer, and we make one based on the result of RenderLayerCompositor::clippedByAncestor(), which uses:

layer.backgroundClipRect(RenderLayer::ClipRectsContext(computeClipRoot, TemporaryClipRects)).isInfinite()

However, RenderLayerBacking::computeParentGraphicsLayerRect() is doing:

        ShouldRespectOverflowClip shouldRespectOverflowClip = compositedAncestor->isolatesCompositedBlending() ? RespectOverflowClip : IgnoreOverflowClip;
        RenderLayer::ClipRectsContext clipRectsContext(compositedAncestor, TemporaryClipRects, IgnoreOverlayScrollbarSize, shouldRespectOverflowClip);
        LayoutRect parentClipRect = m_owningLayer.backgroundClipRect(clipRectsContext).rect(); // FIXME: Incorrect for CSS regions.

so 'shouldRespectOverflowClip' differs.
Comment 3 Simon Fraser (smfr) 2019-05-08 21:24:14 PDT
*** Bug 197731 has been marked as a duplicate of this bug. ***
Comment 4 Simon Fraser (smfr) 2019-05-08 21:24:37 PDT
That code came from https://trac.webkit.org/changeset/168314/webkit
Comment 5 Simon Fraser (smfr) 2019-05-08 21:49:53 PDT
accessibility/media-element.html also hits this
Comment 6 Simon Fraser (smfr) 2019-05-08 22:37:34 PDT
One thing to note here is that normally we don't have an m_ancestorLayer if our clipping ancestor is a stacking context, but with a composited mix-blend-mode child, RenderLayerCompositor::clippedByAncestor() follows a different code path, and can give us an m_ancestorClippingLayer. I don't know why.
Comment 7 Simon Fraser (smfr) 2019-05-08 22:48:00 PDT
This bug is about maintenance of the isolatesCompositedBlending() bit on RenderLayer vs. the maintenance of m_ancestorClippingLayer.

If compositedAncestor->isolatesCompositedBlending() was true at the last compositing update, we made a m_ancestorClippingLayer. At some point another composited sibling with blending became non-composited, so compositedAncestor->isolatesCompositedBlending() is no longer true but we didn't remove the m_ancestorClippingLayer.
Comment 8 Simon Fraser (smfr) 2019-05-08 22:50:10 PDT
isolatesCompositedBlending depends on both m_hasNotIsolatedCompositedBlendingDescendants and isCSSStackingContext
Comment 9 Simon Fraser (smfr) 2019-05-08 23:07:29 PDT
The fix is something like:

index 15479365f2edf9776a01e4f4e70c792921824975..8532ac6c3acb2df5eb75a5d6b43981d2cc5c4eaa 100644
--- a/Source/WebCore/rendering/RenderLayerCompositor.cpp
+++ b/Source/WebCore/rendering/RenderLayerCompositor.cpp
@@ -1022,8 +1022,13 @@ void RenderLayerCompositor::computeCompositingRequirements(RenderLayer* ancestor
         addToOverlapMap(overlapMap, layer, layerExtent);
+    bool isolatedCompositedBlending = layer.isolatesCompositedBlending();
     ASSERT(!layer.hasNotIsolatedCompositedBlendingDescendants() || layer.hasNotIsolatedBlendingDescendants());
+    if (layer.isolatesCompositedBlending() != isolatedCompositedBlending) {
+        // isolatedCompositedBlending affects the answer to clippedByAncestor().
+        layer.setChildrenNeedCompositingGeometryUpdate();
+    }
     // Now check for reasons to become composited that depend on the state of descendant layers.
     RenderLayer::IndirectCompositingReason indirectCompositingReason;
Comment 10 Simon Fraser (smfr) 2019-05-09 07:14:29 PDT
Created attachment 369491 [details]
Comment 11 Radar WebKit Bug Importer 2019-05-09 07:38:10 PDT
Comment 12 WebKit Commit Bot 2019-05-09 11:26:30 PDT
Comment on attachment 369491 [details]

Clearing flags on attachment: 369491

Committed r245147: <https://trac.webkit.org/changeset/245147>
Comment 13 WebKit Commit Bot 2019-05-09 11:26:32 PDT
All reviewed patches have been landed.  Closing bug.