REGRESSION (r238357): Pins on Yelp map disappear
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 13 Dec 2018 02:35:10 +0000 (02:35 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 13 Dec 2018 02:35:10 +0000 (02:35 +0000)
https://bugs.webkit.org/show_bug.cgi?id=192597
rdar://problem/46578285

Reviewed by Zalan Bujtas.
Source/WebCore:

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):

LayoutTests:

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.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@239146 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/compositing/z-order/add-negative-z-child-expected.html [new file with mode: 0644]
LayoutTests/compositing/z-order/add-negative-z-child.html [new file with mode: 0644]
LayoutTests/compositing/z-order/rebuild-sibling-of-layer-with-foreground-layer-expected.html [new file with mode: 0644]
LayoutTests/compositing/z-order/rebuild-sibling-of-layer-with-foreground-layer.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderLayer.cpp
Source/WebCore/rendering/RenderLayerCompositor.cpp

index b78101f..a09c3a8 100644 (file)
@@ -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 Zalan Bujtas.
+        
+        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-12  YUHAN WU  <yuhan_wu@apple.com>
 
         Implement non-timeslice mode encoding for MediaRecorder
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 (file)
index 0000000..413bb53
--- /dev/null
@@ -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 (file)
index 0000000..cc961c7
--- /dev/null
@@ -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 (file)
index 0000000..d30dc5a
--- /dev/null
@@ -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 (file)
index 0000000..9908f6b
--- /dev/null
@@ -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>
index 04db8eb..0db3bd8 100644 (file)
@@ -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 Zalan Bujtas.
+
+        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-12  YUHAN WU  <yuhan_wu@apple.com>
 
         Implement non-timeslice mode encoding for MediaRecorder
index 80c1bbe..db18354 100644 (file)
@@ -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();
index 77b0120..4f09486 100644 (file)
@@ -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();