Cherry-pick 1021d66. rdar://121960496

Crash under RenderLayer::calculateClipRects() when going into fullscreen https://bugs.webkit.org/show_bug.cgi?id=268891 rdar://121960496 Reviewed by Alan Baradlay. A combination of top layer and compositing backing sharing can cause a null de-ref when entering fullscreen, or using modal dialogs or popovers. The issue occurs when the renderer going into top layer participates in a backing sharing sequence, in the `RenderLayer::paintsIntoProvidedBacking()` sense. What happens in that case is that after the top layer configuration is changed we do a layout, after which `RenderLayerBacking::updateAfterLayout()` calls `RenderLayerBacking::updateCompositedBounds()` (this seems like an odd thing to do, because we're going to do a compositing update anyway, but a comment explains why we do it). This call requires that we compute clip rects, which calls `RenderLayer::canUseOffsetFromAncestor()`, which gets confused because the ancestor layer is no longer an ancestor. The fix is to clear any relevant backing sharing sequences when going into top layer, where "relevant" means backing sharing sequences in the stacking context of the layer that's going into top layer. We do that by calling into RenderLayerCompositor from `RenderLayer::establishesTopLayerWillChange()`. Normally traversing layers in a stacking context would walk the z-order lists, and this works for popover and dialog, but fullscreen triggers a style update before this code runs, which clears the z-order lists. So this stacking context traversal is written in terms of the RenderLayer tree (like `collectLayers()`). * LayoutTests/compositing/shared-backing/top-layer/backing-sharing-split-by-dialog-expected.txt: Added. * LayoutTests/compositing/shared-backing/top-layer/backing-sharing-split-by-dialog.html: Added. * LayoutTests/compositing/shared-backing/top-layer/backing-sharing-split-by-fullscreen-expected.txt: Added. * LayoutTests/compositing/shared-backing/top-layer/backing-sharing-split-by-fullscreen-variant-expected.txt: Added. * LayoutTests/compositing/shared-backing/top-layer/backing-sharing-split-by-fullscreen-variant.html: Added. * LayoutTests/compositing/shared-backing/top-layer/backing-sharing-split-by-fullscreen.html: Added. * LayoutTests/compositing/shared-backing/top-layer/backing-sharing-split-by-popover-expected.txt: Added. * LayoutTests/compositing/shared-backing/top-layer/backing-sharing-split-by-popover.html: Added. * Source/WebCore/rendering/RenderLayer.cpp: (WebCore::RenderLayer::establishesTopLayerWillChange): (WebCore::RenderLayer::calculateClipRects const): (WebCore::outputPaintOrderTreeLegend): (WebCore::outputPaintOrderTreeRecursive): * Source/WebCore/rendering/RenderLayerCompositor.cpp: (WebCore::RenderLayerCompositor::establishesTopLayerWillChangeForLayer): (WebCore::clearBackingSharingWithinStackingContext): (WebCore::RenderLayerCompositor::clearBackingProviderSequencesInStackingContextOfLayer): * Source/WebCore/rendering/RenderLayerCompositor.h: Canonical link: https://commits.webkit.org/274290@main Identifier: 272448.536@safari-7618.1.15.10-branch
WebKit · Feb 8, 2024 · 73b226b · 73b226b
1 parent 5addef3
commit 73b226b
Show file tree

Hide file tree

Showing 11 changed files with 367 additions and 2 deletions.
diff --git a/...utTests/compositing/shared-backing/top-layer/backing-sharing-split-by-dialog-expected.txt b/...utTests/compositing/shared-backing/top-layer/backing-sharing-split-by-dialog-expected.txt
@@ -0,0 +1,2 @@
+x x
+Test passes if it does not crash.
diff --git a/LayoutTests/compositing/shared-backing/top-layer/backing-sharing-split-by-dialog.html b/LayoutTests/compositing/shared-backing/top-layer/backing-sharing-split-by-dialog.html
@@ -0,0 +1,77 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <style>
+        .negative-z {
+            position: absolute;
+            top: 20px;
+            left: 20px;
+            z-index: -1;
+            width: 20px;
+            height: 20px;
+            border: 1px solid blue;
+        }
+
+        .transformed {
+            transform: translateZ(0);
+        }
+
+        .relpos {
+            position: relative;
+            height: 500px;
+            height: 600px;
+            margin: 20px 40px;
+            border: 2px solid gray;
+        }
+
+        dialog {
+            position: relative !important;
+            display: block;
+            margin: 0px auto;
+            width: 100%;
+            height: 100%;
+            background-color: rgba(0, 0, 0, 0.5);
+            overflow: hidden;
+        }
+
+        .abspos {
+            position: absolute;
+            z-index: 2;
+            width: 500px;
+            height: 200px;
+            background-color: green;
+        }
+    </style>
+    <script>
+        if (window.testRunner) {
+            testRunner.dumpAsText();
+            testRunner.waitUntilDone();
+        }
+
+        function showDialog()
+        {
+            let dialog = document.getElementsByTagName('dialog')[0];
+            dialog.showModal();
+        }
+
+        window.addEventListener('load', () => {
+            setTimeout(() => {
+                showDialog();
+                if (window.testRunner)
+                    testRunner.notifyDone();
+            }, 0);
+        }, false);
+    </script>
+</head>
+<body>
+    <div class="negative-z">
+        x
+        <div class="negative-z transformed">x</div>
+    </div>
+    <div class="relpos">
+        <dialog>
+            <div class="abspos">Test passes if it does not crash.</div>
+        </dialog>
+    </div>
+</body>
+</html>
diff --git a/...sts/compositing/shared-backing/top-layer/backing-sharing-split-by-fullscreen-expected.txt b/...sts/compositing/shared-backing/top-layer/backing-sharing-split-by-fullscreen-expected.txt
@@ -0,0 +1,2 @@
+x x
+Test passes if it does not crash.
diff --git a/...ositing/shared-backing/top-layer/backing-sharing-split-by-fullscreen-variant-expected.txt b/...ositing/shared-backing/top-layer/backing-sharing-split-by-fullscreen-variant-expected.txt
@@ -0,0 +1,2 @@
+x x
+Test passes if it does not crash..
diff --git a/...sts/compositing/shared-backing/top-layer/backing-sharing-split-by-fullscreen-variant.html b/...sts/compositing/shared-backing/top-layer/backing-sharing-split-by-fullscreen-variant.html
@@ -0,0 +1,82 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <style>
+        .negative-z {
+            position: absolute;
+            top: 20px;
+            left: 20px;
+            z-index: -1;
+            width: 20px;
+            height: 20px;
+            border: 1px solid blue;
+        }
+
+        .transformed {
+            transform: translateZ(0);
+        }
+
+        .relpos {
+            position: relative;
+            height: 500px;
+            height: 600px;
+            margin: 20px 40px;
+            border: 2px solid gray;
+        }
+
+        .fullscreen {
+            position: relative;
+            margin: 0px auto;
+            width: 100%;
+            height: 100%;
+            background-color: rgba(0, 0, 0, 0.5);
+            overflow: hidden;
+        }
+
+        .abspos {
+            position: absolute;
+            z-index: 2;
+            width: 500px;
+            height: 165px;
+            background-color: green;
+        }
+    </style>
+    <script>
+        if (window.testRunner) {
+            testRunner.dumpAsText();
+            testRunner.waitUntilDone();
+        }
+
+        function requestFullscreen()
+        {
+            let fullscreen = document.querySelector('.fullscreen');
+
+            fullscreen.addEventListener("fullscreenchange", () => {
+                if (window.testRunner)
+                    testRunner.notifyDone();
+            });
+
+            internals.withUserGesture(() => {
+                fullscreen.requestFullscreen();
+            });
+        }
+
+        window.addEventListener('load', () => {
+            setTimeout(() => {
+                requestFullscreen();
+            }, 0);
+        }, false);
+    </script>
+</head>
+<body>
+    <div class="negative-z">
+        x
+        <div class="negative-z transformed">x</div>
+    </div>
+    <div class="relpos">
+        <div class="fullscreen">
+            <div class="abspos">Test passes if it does not crash..</div>
+        </div>
+    </div>
+</body>
+</html>
diff --git a/LayoutTests/compositing/shared-backing/top-layer/backing-sharing-split-by-fullscreen.html b/LayoutTests/compositing/shared-backing/top-layer/backing-sharing-split-by-fullscreen.html
@@ -0,0 +1,82 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <style>
+        .negative-z {
+            position: absolute;
+            top: 20px;
+            left: 20px;
+            z-index: -1;
+            width: 20px;
+            height: 20px;
+            border: 1px solid blue;
+        }
+
+        .transformed {
+            transform: translateZ(0);
+        }
+
+        .relpos {
+            position: relative;
+            height: 500px;
+            height: 600px;
+            margin: 20px 40px;
+            border: 2px solid gray;
+        }
+
+        .fullscreen {
+            position: relative;
+            margin: 0px auto;
+            width: 100%;
+            height: 100%;
+            background-color: rgba(0, 0, 0, 0.5);
+            overflow: hidden;
+        }
+
+        .abspos {
+            position: absolute;
+            z-index: 2;
+            width: 500px;
+            height: 165px;
+            background-color: green;
+        }
+    </style>
+    <script>
+        if (window.testRunner) {
+            testRunner.dumpAsText();
+            testRunner.waitUntilDone();
+        }
+
+        function requestFullscreen()
+        {
+            let fullscreen = document.querySelector('.fullscreen');
+
+            fullscreen.addEventListener("fullscreenchange", () => {
+                if (window.testRunner)
+                    testRunner.notifyDone();
+            });
+
+            internals.withUserGesture(() => {
+                fullscreen.requestFullscreen();
+            });
+        }
+
+        window.addEventListener('load', () => {
+            setTimeout(() => {
+                requestFullscreen();
+            }, 0);
+        }, false);
+    </script>
+</head>
+<body>
+    <div class="negative-z">
+        x
+        <div class="negative-z transformed">x</div>
+    </div>
+    <div class="relpos">
+        <div class="fullscreen">
+            <div class="abspos">Test passes if it does not crash.</div>
+        </div>
+    </div>
+</body>
+</html>
diff --git a/...tTests/compositing/shared-backing/top-layer/backing-sharing-split-by-popover-expected.txt b/...tTests/compositing/shared-backing/top-layer/backing-sharing-split-by-popover-expected.txt
@@ -0,0 +1,2 @@
+x x
+Test passes if it does not crash.
diff --git a/LayoutTests/compositing/shared-backing/top-layer/backing-sharing-split-by-popover.html b/LayoutTests/compositing/shared-backing/top-layer/backing-sharing-split-by-popover.html
@@ -0,0 +1,79 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <style>
+        .negative-z {
+            position: absolute;
+            top: 20px;
+            left: 20px;
+            z-index: -1;
+            width: 20px;
+            height: 20px;
+            border: 1px solid blue;
+        }
+
+        .transformed {
+            transform: translateZ(0);
+        }
+
+        .relpos {
+            position: relative;
+            height: 650px;
+            height: 770px;
+            margin: 20px 40px;
+            border: 2px solid gray;
+        }
+
+        .popover {
+            position: relative;
+            margin: 0px auto;
+            width: 100%;
+            height: 100%;
+            background-color: rgba(0, 0, 0, 0.5);
+            overflow: hidden;
+            display: block;
+        }
+
+        .abspos {
+            position: absolute;
+            z-index: 2;
+            width: 500px;
+            height: 165px;
+            background-color: green;
+        }
+    </style>
+    <script>
+        if (window.testRunner) {
+            testRunner.dumpAsText();
+            testRunner.waitUntilDone();
+        }
+
+        function showPopover()
+        {
+            let popover = document.querySelector('.popover');
+            popover.showPopover();
+        }
+
+        window.addEventListener('load', () => {
+            setTimeout(() => {
+                showPopover();
+                if (window.testRunner)
+                    testRunner.notifyDone();
+            }, 0);
+        }, false);
+    </script>
+</head>
+<body>
+    <div class="negative-z">
+        x
+        <div class="negative-z transformed">x</div>
+    </div>
+    <div class="relpos">
+        <div popover class="popover" id="mypopover">
+            <div class="abspos">
+                Test passes if it does not crash.
+            </div>
+        </div>
+    </div>
+</body>
+</html>
diff --git a/Source/WebCore/rendering/RenderLayer.cpp b/Source/WebCore/rendering/RenderLayer.cpp
@@ -4070,6 +4070,8 @@ bool RenderLayer::establishesTopLayer() const
 
 void RenderLayer::establishesTopLayerWillChange()
 {
+    compositor().establishesTopLayerWillChangeForLayer(*this);
+
     if (auto* parentLayer = parent())
         parentLayer->removeChild(*this);
 }
@@ -5345,7 +5347,7 @@ void RenderLayer::repaintIncludingDescendants()
 
 bool RenderLayer::canUseOffsetFromAncestor(const RenderLayer& ancestor) const
 {
-    for (auto* layer = this; layer != &ancestor; layer = layer->parent()) {
+    for (auto* layer = this; layer && layer != &ancestor; layer = layer->parent()) {
         if (!layer->canUseOffsetFromAncestor())
             return false;
     }
@@ -6007,7 +6009,7 @@ void showLayerTree(const WebCore::RenderObject* renderer)
 static void outputPaintOrderTreeLegend(TextStream& stream)
 {
     stream.nextLine();
-    stream << "(S)tacking Context/(F)orced SC/O(P)portunistic SC, (N)ormal flow only, (O)verflow clip, (A)lpha (opacity or mask), has (B)lend mode, (I)solates blending, (T)ransform-ish, (F)ilter, Fi(X)ed position, Behaves as fi(x)ed, (C)omposited, (P)rovides backing/uses (p)rovided backing/paints to (a)ncestor, (c)omposited descendant, (s)scrolling ancestor, (t)transformed ancestor\n"
+    stream << "(T)op layer, (S)tacking Context/(F)orced SC/O(P)portunistic SC, (N)ormal flow only, (O)verflow clip, (A)lpha (opacity or mask), has (B)lend mode, (I)solates blending, (T)ransform-ish, (F)ilter, Fi(X)ed position, Behaves as fi(x)ed, (C)omposited, (P)rovides backing/uses (p)rovided backing/paints to (a)ncestor, (c)omposited descendant, (s)scrolling ancestor, (t)transformed ancestor\n"
         "Dirty (z)-lists, Dirty (n)ormal flow lists\n"
         "Traversal needs: requirements (t)raversal on descendants, (b)acking or hierarchy traversal on descendants, (r)equirements traversal on all descendants, requirements traversal on all (s)ubsequent layers, (h)ierarchy traversal on all descendants, update of paint (o)rder children\n"
         "Update needs:    post-(l)ayout requirements, (g)eometry, (k)ids geometry, (c)onfig, layer conne(x)ion, (s)crolling tree\n"
@@ -6024,6 +6026,7 @@ static void outputIdent(TextStream& stream, unsigned depth)
 
 static void outputPaintOrderTreeRecursive(TextStream& stream, const WebCore::RenderLayer& layer, const char* prefix, unsigned depth = 0)
 {
+    stream << (layer.establishesTopLayer() ? "T" : "-");
     stream << (layer.isCSSStackingContext() ? "S" : (layer.isForcedStackingContext() ? "F" : (layer.isOpportunisticStackingContext() ? "P" : "-")));
     stream << (layer.isNormalFlowOnly() ? "N" : "-");
     stream << (layer.renderer().hasNonVisibleOverflow() ? "O" : "-");