Attachment 357178 Details for Bug 192597 – Patch

[patch] Patch

bug-192597-20181212152953.patch (text/plain), 10.79 KB, created by Simon Fraser (smfr) on 2018-12-12 15:29:53 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Simon Fraser (smfr)

Created: 2018-12-12 15:29:53 PST

Size: 10.79 KB

patch

obsolete

>Subversion Revision: 239042
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 1105081b9ac3b1ba3865c34d41c8f52b5dcebfbe..e624c75efe48f6ee6629c768bcb38e21cc738362 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,27 @@
>+2018-12-12  Simon Fraser  <simon.fraser@apple.com>
>+
>+        REGRESSION (r238357): Pins on Yelp map disappear
>+        https://bugs.webkit.org/show_bug.cgi?id=192597
>+        rdar://problem/46578285
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        RenderLayerCompositor::updateBackingAndHierarchy() had a bug where if a RenderLayer gained
>+        a negative z-order child (triggering creation of a foreground layer), we'd fail to 
>+        call the "setChildren()" with the vector containing that foreground layer.
>+        
>+        When updateBackingAndHierarchy() stops visiting descendants because none are composited,
>+        it may still have to update the child list with the foreground layer, so make sure
>+        the code handles this case.
>+
>+        Tests: compositing/z-order/add-negative-z-child.html
>+               compositing/z-order/rebuild-sibling-of-layer-with-foreground-layer.html
>+
>+        * rendering/RenderLayer.cpp:
>+        (WebCore::outputPaintOrderTreeRecursive):
>+        * rendering/RenderLayerCompositor.cpp:
>+        (WebCore::RenderLayerCompositor::updateBackingAndHierarchy):
>+
> 2018-12-10  Simon Fraser  <simon.fraser@apple.com>
> 
>         Allow control over child order when adding nodes to the scrolling tree
>diff --git a/Source/WebCore/rendering/RenderLayer.cpp b/Source/WebCore/rendering/RenderLayer.cpp
>index 80c1bbe74e3b5b04d6553fa7c944bfd0889961fa..db183544719bde917b5dbd5dfd3d3992fbbc5233 100644
>--- a/Source/WebCore/rendering/RenderLayer.cpp
>+++ b/Source/WebCore/rendering/RenderLayer.cpp
>@@ -6702,7 +6702,10 @@ static void outputPaintOrderTreeRecursive(TextStream& stream, const WebCore::Ren
> 
>     auto layerRect = layer.rect();
> 
>-    stream << &layer << " " << layerRect << " " << layer.name();
>+    stream << &layer << " " << layerRect;
>+    if (layer.isComposited())
>+        stream << " (layerID " << layer.backing()->graphicsLayer()->primaryLayerID() << ")";
>+    stream << " " << layer.name();
>     stream.nextLine();
> 
>     const_cast<WebCore::RenderLayer&>(layer).updateLayerListsIfNeeded();
>diff --git a/Source/WebCore/rendering/RenderLayerCompositor.cpp b/Source/WebCore/rendering/RenderLayerCompositor.cpp
>index 77b0120fbaed32236d03f7e858dcd7819fda505e..4f09486d146a28c25ebae7f2d62dd0c89d8004aa 100644
>--- a/Source/WebCore/rendering/RenderLayerCompositor.cpp
>+++ b/Source/WebCore/rendering/RenderLayerCompositor.cpp
>@@ -1184,6 +1184,8 @@ void RenderLayerCompositor::updateBackingAndHierarchy(RenderLayer& layer, Vector
> 
>     bool requireDescendantTraversal = layer.hasDescendantNeedingUpdateBackingOrHierarchyTraversal()
>         || (layer.hasCompositingDescendant() && (!layerBacking || layer.needsCompositingLayerConnection() || !updateLevel.isEmpty()));
>+    
>+    bool requiresChildRebuild = layerBacking && layer.needsCompositingLayerConnection() && !layer.hasCompositingDescendant();
> 
> #if !ASSERT_DISABLED
>     LayerListMutationDetector mutationChecker(layer);
>@@ -1208,11 +1210,11 @@ void RenderLayerCompositor::updateBackingAndHierarchy(RenderLayer& layer, Vector
>         
>         for (auto* renderLayer : layer.positiveZOrderLayers())
>             updateBackingAndHierarchy(*renderLayer, childList, updateLevel, depth + 1);
>-    } else
>+    } else if (requiresChildRebuild)
>         appendForegroundLayerIfNecessary();
> 
>     if (layerBacking) {
>-        if (requireDescendantTraversal) {
>+        if (requireDescendantTraversal || requiresChildRebuild) {
>             bool parented = false;
>             if (is<RenderWidget>(layer.renderer()))
>                 parented = parentFrameContentLayers(&downcast<RenderWidget>(layer.renderer()));
>@@ -1236,7 +1238,7 @@ void RenderLayerCompositor::updateBackingAndHierarchy(RenderLayer& layer, Vector
> 
>         childLayersOfEnclosingLayer.append(*layerBacking->childForSuperlayers());
> 
>-        layerBacking->updateAfterDescendants(); // FIXME: validate.
>+        layerBacking->updateAfterDescendants();
>     }
>     
>     layer.clearUpdateBackingOrHierarchyTraversalState();
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 73e014f5feb9243020ca9d565f1b95f071244041..ab7f5b27607a432c3f61083c0b09caf32e57dfe3 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,18 @@
>+2018-12-12  Simon Fraser  <simon.fraser@apple.com>
>+
>+        REGRESSION (r238357): Pins on Yelp map disappear
>+        https://bugs.webkit.org/show_bug.cgi?id=192597
>+        rdar://problem/46578285
>+
>+        Reviewed by NOBODY (OOPS!).
>+        
>+        Add tests that toggle negative z-index on a child, with and without sibling compositing layers.
>+
>+        * compositing/z-order/add-negative-z-child-expected.html: Added.
>+        * compositing/z-order/add-negative-z-child.html: Added.
>+        * compositing/z-order/rebuild-sibling-of-layer-with-foreground-layer-expected.html: Added.
>+        * compositing/z-order/rebuild-sibling-of-layer-with-foreground-layer.html: Added.
>+
> 2018-12-10  Rob Buis  <rbuis@igalia.com>
> 
>         XMLHttpRequest removes spaces from content-types before processing
>diff --git a/LayoutTests/compositing/z-order/add-negative-z-child-expected.html b/LayoutTests/compositing/z-order/add-negative-z-child-expected.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..413bb53ba2a4c15448a209436bad68cf03616cac
>--- /dev/null
>+++ b/LayoutTests/compositing/z-order/add-negative-z-child-expected.html
>@@ -0,0 +1,33 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+    <style>
>+        .box {
>+            position: absolute;
>+            width: 200px;
>+            height: 200px;
>+            background-color: red;
>+        }
>+        
>+        .composited {
>+            transform: translateZ(1px);
>+        }
>+        
>+        .negative {
>+            z-index: -1;
>+        }
>+
>+        .positive {
>+            z-index: 1;
>+            background-color: green;
>+        }
>+    </style>
>+</head>
>+<body>
>+    <p>You should see a green box below.</p>
>+    <div class="composited box">
>+        <div class="negative box"></div>
>+        <div class="positive box"></div>
>+    </div>
>+</body>
>+</html>
>diff --git a/LayoutTests/compositing/z-order/add-negative-z-child.html b/LayoutTests/compositing/z-order/add-negative-z-child.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..cc961c749d97037958d01b67950348ec7f8f6925
>--- /dev/null
>+++ b/LayoutTests/compositing/z-order/add-negative-z-child.html
>@@ -0,0 +1,48 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+    <meta charset="utf-8">
>+    <style>
>+        
>+        .box {
>+            position: absolute;
>+            width: 200px;
>+            height: 200px;
>+            background-color: red;
>+        }
>+        
>+        .composited {
>+            transform: translateZ(1px);
>+        }
>+        
>+        .negative {
>+            z-index: -1;
>+        }
>+
>+        .positive {
>+            z-index: 1;
>+            background-color: green;
>+        }
>+    </style>
>+    
>+    <script>
>+        if (window.testRunner)
>+            testRunner.waitUntilDone();
>+
>+        window.addEventListener('load', () => {
>+            requestAnimationFrame(() => {
>+                document.getElementById('target').classList.add('negative');
>+                if (window.testRunner)
>+                    testRunner.notifyDone();
>+            });
>+        }, false);
>+    </script>
>+</head>
>+<body>
>+    <p>You should see a green box below.</p>
>+    <div class="composited box">
>+        <div id="target" class="box"></div>
>+        <div class="positive box"></div>
>+    </div>
>+</body>
>+</html>
>diff --git a/LayoutTests/compositing/z-order/rebuild-sibling-of-layer-with-foreground-layer-expected.html b/LayoutTests/compositing/z-order/rebuild-sibling-of-layer-with-foreground-layer-expected.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..d30dc5a6545da37cf44f5712fc533bb142ff04e8
>--- /dev/null
>+++ b/LayoutTests/compositing/z-order/rebuild-sibling-of-layer-with-foreground-layer-expected.html
>@@ -0,0 +1,43 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+    <style>
>+        .container {
>+            position: relative;
>+            margin: 20px;
>+        }
>+        .box {
>+            position: absolute;
>+            top: 0;
>+            left: 0;
>+            width: 200px;
>+            height: 200px;
>+            background-color: silver;
>+        }
>+        .composited {
>+            transform: translateZ(1px);
>+        }
>+        .sibling {
>+            top: 300px;
>+            background-color: gray;
>+        }
>+        .negative {
>+            z-index: -1;
>+            background-color: red;
>+        }
>+        .positive {
>+            z-index: 1;
>+            background-color: green;
>+        }
>+    </style>
>+</head>
>+<body>
>+    <div class="container">
>+        <div class="composited child box">
>+            <div class="negative box"></div>
>+            <div class="composited positive box"></div>
>+        </div>
>+        <div id="target" class="composited sibling box"></div>
>+    </div>
>+</body>
>+</html>
>diff --git a/LayoutTests/compositing/z-order/rebuild-sibling-of-layer-with-foreground-layer.html b/LayoutTests/compositing/z-order/rebuild-sibling-of-layer-with-foreground-layer.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..9908f6b561dc047e03184b126b44e9f36d2fe81e
>--- /dev/null
>+++ b/LayoutTests/compositing/z-order/rebuild-sibling-of-layer-with-foreground-layer.html
>@@ -0,0 +1,55 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+    <style>
>+        .container {
>+            position: relative;
>+            margin: 20px;
>+        }
>+        .box {
>+            position: absolute;
>+            top: 0;
>+            left: 0;
>+            width: 200px;
>+            height: 200px;
>+            background-color: silver;
>+        }
>+        .composited {
>+            transform: translateZ(1px);
>+        }
>+        .sibling {
>+            top: 300px;
>+            background-color: gray;
>+        }
>+        .negative {
>+            z-index: -1;
>+            background-color: red;
>+        }
>+        .positive {
>+            z-index: 1;
>+            background-color: green;
>+        }
>+    </style>
>+    <script>
>+        if (window.testRunner)
>+            testRunner.waitUntilDone();
>+
>+        window.addEventListener('load', () => {
>+            setTimeout(() => {
>+                document.getElementById('target').classList.add('composited');
>+                if (window.testRunner)
>+                    testRunner.notifyDone();
>+            }, 1000);
>+        }, false);
>+    </script>
>+</head>
>+<body>
>+    <div class="container">
>+        <div class="composited child box">
>+            <div class="negative box"></div>
>+            <div class="composited positive box"></div>
>+        </div>
>+        <div id="target" class="sibling box"></div>
>+    </div>
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 192597: 357178