[Async overflow scrolling] Fix missing or misplaced content inside overflow:scroll
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 27 Jun 2019 04:13:15 +0000 (04:13 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 27 Jun 2019 04:13:15 +0000 (04:13 +0000)
https://bugs.webkit.org/show_bug.cgi?id=199253
Source/WebCore:

rdar://problem/51855156, rdar://problem/51934514

Reviewed by Zalan Bujtas.

This patch fixes a couple of related issues triggered by failing to composite layers inside non-stacking-context
overflow scroll.

First, we relied on overlap testing to composite position:relative layers inside overflow:scroll, but this only
worked when they came later in z-order, so didn't work for layers with negative z-index.
RenderLayerCompositor::requiresCompositingForIndirectReason() was intended to trigger compositing in such cases,
but it only did so for position:absolute inside stacking-context scroller, because
isNonScrolledLayerInsideScrolledCompositedAncestor() tested ancestorMovedByScroller && !layerMovedByScroller.

I fixed this by sharing code between the three places that ask whether compositing crosses a containing-block
boundary to call a single function, RenderLayerCompositor::layerScrollBehahaviorRelativeToCompositedAncestor(),
that returns a ScrollPositioningBehavior. We now do compositing for both "moves" and "stationary" behaviors (but
not "none"), ensuring that position:relative inside non-stacking scroller is always composited.

However, this would trigger compositing on layers that should be using backing sharing; if they were outside the
visible part of the scroller, the overlap code would not trigger, but the
"IndirectCompositingReason::OverflowScrollPositioning" code would. This is undesirable; any layer that can use
backing sharing should, because that's fewer composited layers, so smaller layer trees and less backing store.
To fix this, I moved the backing-sharing check before the overlap check in
RenderLayerCompositor::computeCompositingRequirements().

The "layer.setHasCompositingDescendant(currentState.subtreeIsCompositing)" line was in the wrong place,
triggering assertions on some content; "subtreeIsCompositing" only refers to child layers, so this bit needs to
be set right after we've traversed the z-order lists.

Tests: compositing/scrolling/async-overflow-scrolling/hidden-relative-layer-content-in-scroller.html
       compositing/scrolling/async-overflow-scrolling/layer-for-negative-z-in-scroller.html
       compositing/scrolling/async-overflow-scrolling/negative-z-in-scroller.html

* rendering/RenderLayerCompositor.cpp:
(WebCore::RenderLayerCompositor::computeCompositingRequirements):
(WebCore::RenderLayerCompositor::traverseUnchangedSubtree):
(WebCore::RenderLayerCompositor::requiresCompositingForIndirectReason const):
(WebCore::isScrolledByOverflowScrollLayer):
(WebCore::enclosingCompositedScrollingLayer):
(WebCore::RenderLayerCompositor::layerScrollBehahaviorRelativeToCompositedAncestor):
(WebCore::RenderLayerCompositor::computeCoordinatedPositioningForLayer const):
(WebCore::isNonScrolledLayerInsideScrolledCompositedAncestor): Deleted.
(WebCore::RenderLayerCompositor::layerContainingBlockCrossesCoordinatedScrollingBoundary): Deleted.
* rendering/RenderLayerCompositor.h:

LayoutTests:

Reviewed by Zalan Bujtas.

* compositing/geometry/limit-layer-bounds-clipping-ancestor-expected.txt:
* compositing/layer-creation/clipping-scope/overlap-constrained-inside-scroller-expected.txt:
* compositing/layer-creation/clipping-scope/scroller-with-negative-z-children-expected.txt:
* compositing/rtl/rtl-scrolling-with-transformed-descendants-expected.txt:
* compositing/scrolling/async-overflow-scrolling/hidden-relative-layer-content-in-scroller-expected.html: Added.
* compositing/scrolling/async-overflow-scrolling/hidden-relative-layer-content-in-scroller.html: Added.
* compositing/scrolling/async-overflow-scrolling/layer-for-negative-z-in-scroller-expected.txt: Added.
* compositing/scrolling/async-overflow-scrolling/layer-for-negative-z-in-scroller.html: Added.
* compositing/scrolling/async-overflow-scrolling/negative-z-in-scroller-expected.html: Added.
* compositing/scrolling/async-overflow-scrolling/negative-z-in-scroller.html: Added.
* compositing/shared-backing/overflow-scroll/nested-absolute-with-clipping-in-stacking-overflow-expected.txt:
* platform/ios-wk2/compositing/layer-creation/clipping-scope/overlap-constrained-inside-scroller-expected.txt:
* platform/ios-wk2/compositing/layer-creation/clipping-scope/scroller-with-negative-z-children-expected.txt:
* platform/ios-wk2/compositing/scrolling/async-overflow-scrolling/layer-for-negative-z-in-scroller-expected.txt: Added.
* platform/ios-wk2/compositing/shared-backing/overflow-scroll/nested-absolute-with-clipping-in-stacking-overflow-expected.txt:
* platform/ios/compositing/geometry/limit-layer-bounds-clipping-ancestor-expected.txt:

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

20 files changed:
LayoutTests/ChangeLog
LayoutTests/compositing/geometry/limit-layer-bounds-clipping-ancestor-expected.txt
LayoutTests/compositing/layer-creation/clipping-scope/overlap-constrained-inside-scroller-expected.txt
LayoutTests/compositing/layer-creation/clipping-scope/scroller-with-negative-z-children-expected.txt
LayoutTests/compositing/rtl/rtl-scrolling-with-transformed-descendants-expected.txt
LayoutTests/compositing/scrolling/async-overflow-scrolling/hidden-relative-layer-content-in-scroller-expected.html [new file with mode: 0644]
LayoutTests/compositing/scrolling/async-overflow-scrolling/hidden-relative-layer-content-in-scroller.html [new file with mode: 0644]
LayoutTests/compositing/scrolling/async-overflow-scrolling/layer-for-negative-z-in-scroller-expected.txt [new file with mode: 0644]
LayoutTests/compositing/scrolling/async-overflow-scrolling/layer-for-negative-z-in-scroller.html [new file with mode: 0644]
LayoutTests/compositing/scrolling/async-overflow-scrolling/negative-z-in-scroller-expected.html [new file with mode: 0644]
LayoutTests/compositing/scrolling/async-overflow-scrolling/negative-z-in-scroller.html [new file with mode: 0644]
LayoutTests/compositing/shared-backing/overflow-scroll/nested-absolute-with-clipping-in-stacking-overflow-expected.txt
LayoutTests/platform/ios-wk2/compositing/layer-creation/clipping-scope/overlap-constrained-inside-scroller-expected.txt
LayoutTests/platform/ios-wk2/compositing/layer-creation/clipping-scope/scroller-with-negative-z-children-expected.txt
LayoutTests/platform/ios-wk2/compositing/scrolling/async-overflow-scrolling/layer-for-negative-z-in-scroller-expected.txt [new file with mode: 0644]
LayoutTests/platform/ios-wk2/compositing/shared-backing/overflow-scroll/nested-absolute-with-clipping-in-stacking-overflow-expected.txt
LayoutTests/platform/ios/compositing/geometry/limit-layer-bounds-clipping-ancestor-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderLayerCompositor.cpp
Source/WebCore/rendering/RenderLayerCompositor.h

index b3c74c7..a506334 100644 (file)
@@ -1,3 +1,27 @@
+2019-06-26  Simon Fraser  <simon.fraser@apple.com>
+
+        [Async overflow scrolling] Fix missing or misplaced content inside overflow:scroll
+        https://bugs.webkit.org/show_bug.cgi?id=199253
+
+        Reviewed by Zalan Bujtas.
+
+        * compositing/geometry/limit-layer-bounds-clipping-ancestor-expected.txt:
+        * compositing/layer-creation/clipping-scope/overlap-constrained-inside-scroller-expected.txt:
+        * compositing/layer-creation/clipping-scope/scroller-with-negative-z-children-expected.txt:
+        * compositing/rtl/rtl-scrolling-with-transformed-descendants-expected.txt:
+        * compositing/scrolling/async-overflow-scrolling/hidden-relative-layer-content-in-scroller-expected.html: Added.
+        * compositing/scrolling/async-overflow-scrolling/hidden-relative-layer-content-in-scroller.html: Added.
+        * compositing/scrolling/async-overflow-scrolling/layer-for-negative-z-in-scroller-expected.txt: Added.
+        * compositing/scrolling/async-overflow-scrolling/layer-for-negative-z-in-scroller.html: Added.
+        * compositing/scrolling/async-overflow-scrolling/negative-z-in-scroller-expected.html: Added.
+        * compositing/scrolling/async-overflow-scrolling/negative-z-in-scroller.html: Added.
+        * compositing/shared-backing/overflow-scroll/nested-absolute-with-clipping-in-stacking-overflow-expected.txt:
+        * platform/ios-wk2/compositing/layer-creation/clipping-scope/overlap-constrained-inside-scroller-expected.txt:
+        * platform/ios-wk2/compositing/layer-creation/clipping-scope/scroller-with-negative-z-children-expected.txt:
+        * platform/ios-wk2/compositing/scrolling/async-overflow-scrolling/layer-for-negative-z-in-scroller-expected.txt: Added.
+        * platform/ios-wk2/compositing/shared-backing/overflow-scroll/nested-absolute-with-clipping-in-stacking-overflow-expected.txt:
+        * platform/ios/compositing/geometry/limit-layer-bounds-clipping-ancestor-expected.txt:
+
 2019-06-26  Ryosuke Niwa  <rniwa@webkit.org>
 
         ReplacementFragment should not have script observable side effects
index 65c4c45..42c3656 100644 (file)
@@ -27,7 +27,8 @@ middlebottom
           (children 1
             (GraphicsLayer
               (offsetFromRenderer width=0 height=100)
-              (bounds 149.00 200.00)
+              (bounds 149.00 2086.00)
+              (usingTiledLayer 1)
               (drawsContent 1)
             )
           )
index 49db9d9..4f46c83 100644 (file)
@@ -5,7 +5,7 @@
     (GraphicsLayer
       (bounds 800.00 600.00)
       (contentsOpaque 1)
-      (children 5
+      (children 6
         (GraphicsLayer
           (position 8.00 8.00)
           (bounds 302.00 302.00)
             )
           )
         )
+        (GraphicsLayer
+          (position 9.00 9.00)
+          (bounds 285.00 300.00)
+          (children 1
+            (GraphicsLayer
+              (position 40.00 380.00)
+              (bounds 100.00 100.00)
+              (contentsOpaque 1)
+            )
+          )
+        )
       )
     )
   )
index cd78b43..fd1bdf3 100644 (file)
@@ -5,56 +5,88 @@
     (GraphicsLayer
       (bounds 800.00 600.00)
       (contentsOpaque 1)
-      (children 4
+      (children 1
         (GraphicsLayer
-          (position 8.00 8.00)
-          (bounds 302.00 302.00)
+          (bounds 800.00 600.00)
           (drawsContent 1)
-          (children 1
+          (children 7
             (GraphicsLayer
-              (offsetFromRenderer width=1 height=1)
-              (position 1.00 1.00)
+              (position 9.00 9.00)
               (bounds 285.00 300.00)
               (children 1
                 (GraphicsLayer
+                  (position 40.00 140.00)
+                  (bounds 100.00 100.00)
+                  (contentsOpaque 1)
+                )
+              )
+            )
+            (GraphicsLayer
+              (bounds 800.00 600.00)
+              (drawsContent 1)
+            )
+            (GraphicsLayer
+              (position 8.00 8.00)
+              (bounds 302.00 302.00)
+              (drawsContent 1)
+              (children 1
+                (GraphicsLayer
                   (offsetFromRenderer width=1 height=1)
-                  (anchor 0.00 0.00)
-                  (bounds 285.00 500.00)
+                  (position 1.00 1.00)
+                  (bounds 285.00 300.00)
+                  (children 1
+                    (GraphicsLayer
+                      (offsetFromRenderer width=1 height=1)
+                      (anchor 0.00 0.00)
+                      (bounds 285.00 500.00)
+                    )
+                  )
                 )
               )
             )
-          )
-        )
-        (GraphicsLayer
-          (position 9.00 9.00)
-          (bounds 285.00 300.00)
-          (children 1
             (GraphicsLayer
-              (position 10.00 10.00)
-              (bounds 50.00 200.00)
-              (contentsOpaque 1)
+              (position 9.00 9.00)
+              (bounds 285.00 300.00)
+              (children 1
+                (GraphicsLayer
+                  (position 10.00 10.00)
+                  (bounds 50.00 200.00)
+                  (contentsOpaque 1)
+                )
+              )
             )
-          )
-        )
-        (GraphicsLayer
-          (position 9.00 9.00)
-          (bounds 285.00 300.00)
-          (children 1
             (GraphicsLayer
-              (position 40.00 20.00)
-              (bounds 100.00 100.00)
-              (contentsOpaque 1)
+              (position 9.00 9.00)
+              (bounds 285.00 300.00)
+              (children 1
+                (GraphicsLayer
+                  (position 40.00 20.00)
+                  (bounds 100.00 100.00)
+                  (contentsOpaque 1)
+                )
+              )
             )
-          )
-        )
-        (GraphicsLayer
-          (position 9.00 9.00)
-          (bounds 285.00 300.00)
-          (children 1
             (GraphicsLayer
-              (position 40.00 260.00)
-              (bounds 100.00 100.00)
-              (contentsOpaque 1)
+              (position 9.00 9.00)
+              (bounds 285.00 300.00)
+              (children 1
+                (GraphicsLayer
+                  (position 40.00 260.00)
+                  (bounds 100.00 100.00)
+                  (contentsOpaque 1)
+                )
+              )
+            )
+            (GraphicsLayer
+              (position 9.00 9.00)
+              (bounds 285.00 300.00)
+              (children 1
+                (GraphicsLayer
+                  (position 40.00 380.00)
+                  (bounds 100.00 100.00)
+                  (contentsOpaque 1)
+                )
+              )
             )
           )
         )
index c446161..3e4da39 100644 (file)
@@ -7,7 +7,7 @@
     (GraphicsLayer
       (bounds 800.00 600.00)
       (contentsOpaque 1)
-      (children 3
+      (children 5
         (GraphicsLayer
           (position 8.00 8.00)
           (bounds 404.00 223.00)
             )
           )
         )
+        (GraphicsLayer
+          (position 10.00 10.00)
+          (bounds origin 366.00 0.00)
+          (bounds 400.00 204.00)
+          (clips 1)
+          (children 1
+            (GraphicsLayer
+              (position 154.00 0.00)
+              (bounds 150.00 200.00)
+              (contentsOpaque 1)
+            )
+          )
+        )
+        (GraphicsLayer
+          (position 10.00 10.00)
+          (bounds origin 366.00 0.00)
+          (bounds 400.00 204.00)
+          (clips 1)
+          (children 1
+            (GraphicsLayer
+              (bounds 150.00 200.00)
+              (contentsOpaque 1)
+            )
+          )
+        )
       )
     )
   )
diff --git a/LayoutTests/compositing/scrolling/async-overflow-scrolling/hidden-relative-layer-content-in-scroller-expected.html b/LayoutTests/compositing/scrolling/async-overflow-scrolling/hidden-relative-layer-content-in-scroller-expected.html
new file mode 100644 (file)
index 0000000..873e45c
--- /dev/null
@@ -0,0 +1,71 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <style>
+        .page-scroller {
+            height: 500px;
+            width: 500px;
+            overflow: hidden;
+            border: 1px solid black;
+        }
+
+        .relative {
+            position: relative;
+            height: 500px;
+            width: 100%;
+        }
+                
+        .absolute {
+            position: absolute;
+            z-index: 1;
+            top: 20px;
+            left: 20px;
+            right: 20px;
+            bottom: 20px;
+        }
+
+        .scroller {
+            overflow-y: auto;
+            width: 100%;
+            height: 100%;
+            background: white;
+            border: 1px solid black;
+        }
+
+        .relative-content {
+            position: relative;
+            width: 100%;
+            height: 600px;
+            background-color: green;
+        }
+        .content-sizer {
+            width: 100%;
+            height: 750px;
+            background-color: silver;
+        }
+        
+        .scrollbars-hider {
+            position: absolute;
+            z-index: 2;
+            top: 6px;
+            left: 458px;
+            width: 56px;
+            height: 510px;
+            background-color: gray;
+        }
+    </style>
+</head>
+<body>
+    <div class="page-scroller">
+        <div class="relative">
+            <div class="content-sizer"></div>
+            <div class="absolute">
+                <div class="scroller">
+                    <div class="relative-content"></div>
+                </div>
+            </div>
+        </div>
+    </div>
+    <div class="scrollbars-hider"></div>
+</body>
+</html>
diff --git a/LayoutTests/compositing/scrolling/async-overflow-scrolling/hidden-relative-layer-content-in-scroller.html b/LayoutTests/compositing/scrolling/async-overflow-scrolling/hidden-relative-layer-content-in-scroller.html
new file mode 100644 (file)
index 0000000..8cf03b5
--- /dev/null
@@ -0,0 +1,72 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <style>
+        .page-scroller {
+            height: 500px;
+            width: 500px;
+            overflow-y:auto;
+            overflow-x: hidden;
+            border: 1px solid black;
+        }
+
+        .relative {
+            position: relative;
+            height: 500px;
+            width: 100%;
+        }
+                
+        .absolute {
+            position: absolute;
+            z-index: 1;
+            top: 20px;
+            left: 20px;
+            right: 20px;
+            bottom: 20px;
+        }
+
+        .scroller {
+            overflow-y: auto;
+            width: 100%;
+            height: 100%;
+            background: white;
+            border: 1px solid black;
+        }
+
+        .relative-content {
+            position: relative;
+            width: 100%;
+            height: 600px;
+            background-color: green;
+        }
+        .content-sizer {
+            width: 100%;
+            height: 750px;
+            background-color: silver;
+        }
+        
+        .scrollbars-hider {
+            position: absolute;
+            z-index: 2;
+            top: 6px;
+            left: 458px;
+            width: 56px;
+            height: 510px;
+            background-color: gray;
+        }
+    </style>
+</head>
+<body>
+    <div class="page-scroller">
+        <div class="relative">
+            <div class="content-sizer"></div>
+            <div class="absolute">
+                <div class="scroller">
+                    <div class="relative-content"></div>
+                </div>
+            </div>
+        </div>
+    </div>
+    <div class="scrollbars-hider"></div>
+</body>
+</html>
diff --git a/LayoutTests/compositing/scrolling/async-overflow-scrolling/layer-for-negative-z-in-scroller-expected.txt b/LayoutTests/compositing/scrolling/async-overflow-scrolling/layer-for-negative-z-in-scroller-expected.txt
new file mode 100644 (file)
index 0000000..79e3419
--- /dev/null
@@ -0,0 +1,35 @@
+(GraphicsLayer
+  (anchor 0.00 0.00)
+  (bounds 800.00 600.00)
+  (clips 1)
+  (children 1
+    (GraphicsLayer
+      (bounds 800.00 600.00)
+      (contentsOpaque 1)
+      (children 1
+        (GraphicsLayer
+          (position 8.00 8.00)
+          (bounds 302.00 302.00)
+          (drawsContent 1)
+          (children 1
+            (GraphicsLayer
+              (offsetFromRenderer width=1 height=1)
+              (position 1.00 1.00)
+              (bounds 285.00 300.00)
+              (clips 1)
+              (children 1
+                (GraphicsLayer
+                  (offsetFromRenderer width=1 height=1)
+                  (anchor 0.00 0.00)
+                  (bounds 285.00 1240.00)
+                  (drawsContent 1)
+                )
+              )
+            )
+          )
+        )
+      )
+    )
+  )
+)
+
diff --git a/LayoutTests/compositing/scrolling/async-overflow-scrolling/layer-for-negative-z-in-scroller.html b/LayoutTests/compositing/scrolling/async-overflow-scrolling/layer-for-negative-z-in-scroller.html
new file mode 100644 (file)
index 0000000..8f9e783
--- /dev/null
@@ -0,0 +1,46 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ internal:AsyncOverflowScrollingEnabled=true ] -->
+<html>
+<head>
+    <style>
+               #scroller {
+                       position: relative;
+                       overflow-y: scroll;
+                       width: 300px;
+                       height: 300px;
+                       border: 1px solid black;
+               }
+
+               .negative {
+                       position: relative;
+                       z-index: -1;
+                       margin: 20px;
+                   width: 200px;
+                   height: 200px;
+                   background-color: green;
+               }
+
+        .spacer {
+            height: 1000px;
+            width: 10px;
+            background-color: silver;
+        }
+    </style>
+    <script>
+        if (window.testRunner)
+            testRunner.dumpAsText();
+
+        window.addEventListener('load', () => {
+            if (window.internals)
+                document.getElementById('layers').innerText = window.internals.layerTreeAsText(document, internals.LAYER_TREE_INCLUDES_CLIPPING);
+        }, false);
+    </script>
+</head>
+<body>
+       <div id="scroller">
+        <div class="negative box"></div>
+        <div class="spacer"></div>
+       </div>
+<pre id="layers"></pre>
+</body>
+</html>
+
diff --git a/LayoutTests/compositing/scrolling/async-overflow-scrolling/negative-z-in-scroller-expected.html b/LayoutTests/compositing/scrolling/async-overflow-scrolling/negative-z-in-scroller-expected.html
new file mode 100644 (file)
index 0000000..3f84332
--- /dev/null
@@ -0,0 +1,48 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ internal:AsyncOverflowScrollingEnabled=true ] -->
+<html>
+<head>
+    <style>
+               #scroller {
+                       position: relative;
+                       overflow-y: scroll;
+                       width: 300px;
+                       height: 300px;
+                       border: 1px solid black;
+               }
+
+               .negative {
+                       position: relative;
+                       z-index: 1;
+                       margin: 20px;
+                   width: 200px;
+                   height: 200px;
+                   background-color: green;
+               }
+
+        .spacer {
+            height: 1000px;
+            width: 10px;
+            background-color: silver;
+        }
+    </style>
+    <script>
+        if (window.testRunner)
+            testRunner.waitUntilDone();
+
+        window.addEventListener('load', () => {
+            setTimeout(() => {
+                scroller.scrollTo(0, 300);
+                if (window.testRunner)
+                    testRunner.notifyDone();
+            }, 0)
+        }, false);
+    </script>
+</head>
+<body>
+       <div id="scroller">
+        <div class="negative box"></div>
+        <div class="spacer"></div>
+       </div>
+</body>
+</html>
+
diff --git a/LayoutTests/compositing/scrolling/async-overflow-scrolling/negative-z-in-scroller.html b/LayoutTests/compositing/scrolling/async-overflow-scrolling/negative-z-in-scroller.html
new file mode 100644 (file)
index 0000000..dae9c54
--- /dev/null
@@ -0,0 +1,48 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ internal:AsyncOverflowScrollingEnabled=true ] -->
+<html>
+<head>
+    <style>
+               #scroller {
+                       position: relative;
+                       overflow-y: scroll;
+                       width: 300px;
+                       height: 300px;
+                       border: 1px solid black;
+               }
+
+               .negative {
+                       position: relative;
+                       z-index: -1;
+                       margin: 20px;
+                   width: 200px;
+                   height: 200px;
+                   background-color: green;
+               }
+
+        .spacer {
+            height: 1000px;
+            width: 10px;
+            background-color: silver;
+        }
+    </style>
+    <script>
+        if (window.testRunner)
+            testRunner.waitUntilDone();
+
+        window.addEventListener('load', () => {
+            setTimeout(() => {
+                scroller.scrollTo(0, 300);
+                if (window.testRunner)
+                    testRunner.notifyDone();
+            }, 0)
+        }, false);
+    </script>
+</head>
+<body>
+       <div id="scroller">
+        <div class="negative box"></div>
+        <div class="spacer"></div>
+       </div>
+</body>
+</html>
+
index 5c32192..05a3181 100644 (file)
                       (bounds 140.00 140.00)
                       (contentsOpaque 1)
                       (drawsContent 1)
-                      (children 1
-                        (GraphicsLayer
-                          (bounds 140.00 140.00)
-                        )
-                      )
                     )
                   )
                 )
index 97afdb5..5ef9bda 100644 (file)
@@ -5,7 +5,7 @@
     (GraphicsLayer
       (bounds 800.00 600.00)
       (contentsOpaque 1)
-      (children 5
+      (children 6
         (GraphicsLayer
           (position 8.00 8.00)
           (bounds 302.00 302.00)
             )
           )
         )
+        (GraphicsLayer
+          (position 9.00 9.00)
+          (bounds 300.00 300.00)
+          (children 1
+            (GraphicsLayer
+              (position 40.00 380.00)
+              (bounds 100.00 100.00)
+              (contentsOpaque 1)
+            )
+          )
+        )
       )
     )
   )
index 0d9b8fa..b9656b6 100644 (file)
@@ -5,56 +5,88 @@
     (GraphicsLayer
       (bounds 800.00 600.00)
       (contentsOpaque 1)
-      (children 4
+      (children 1
         (GraphicsLayer
-          (position 8.00 8.00)
-          (bounds 302.00 302.00)
+          (bounds 800.00 600.00)
           (drawsContent 1)
-          (children 1
+          (children 7
             (GraphicsLayer
-              (offsetFromRenderer width=1 height=1)
-              (position 1.00 1.00)
+              (position 9.00 9.00)
               (bounds 300.00 300.00)
               (children 1
                 (GraphicsLayer
+                  (position 40.00 140.00)
+                  (bounds 100.00 100.00)
+                  (contentsOpaque 1)
+                )
+              )
+            )
+            (GraphicsLayer
+              (bounds 800.00 600.00)
+              (drawsContent 1)
+            )
+            (GraphicsLayer
+              (position 8.00 8.00)
+              (bounds 302.00 302.00)
+              (drawsContent 1)
+              (children 1
+                (GraphicsLayer
                   (offsetFromRenderer width=1 height=1)
-                  (anchor 0.00 0.00)
-                  (bounds 300.00 500.00)
+                  (position 1.00 1.00)
+                  (bounds 300.00 300.00)
+                  (children 1
+                    (GraphicsLayer
+                      (offsetFromRenderer width=1 height=1)
+                      (anchor 0.00 0.00)
+                      (bounds 300.00 500.00)
+                    )
+                  )
                 )
               )
             )
-          )
-        )
-        (GraphicsLayer
-          (position 9.00 9.00)
-          (bounds 300.00 300.00)
-          (children 1
             (GraphicsLayer
-              (position 10.00 10.00)
-              (bounds 50.00 200.00)
-              (contentsOpaque 1)
+              (position 9.00 9.00)
+              (bounds 300.00 300.00)
+              (children 1
+                (GraphicsLayer
+                  (position 10.00 10.00)
+                  (bounds 50.00 200.00)
+                  (contentsOpaque 1)
+                )
+              )
             )
-          )
-        )
-        (GraphicsLayer
-          (position 9.00 9.00)
-          (bounds 300.00 300.00)
-          (children 1
             (GraphicsLayer
-              (position 40.00 20.00)
-              (bounds 100.00 100.00)
-              (contentsOpaque 1)
+              (position 9.00 9.00)
+              (bounds 300.00 300.00)
+              (children 1
+                (GraphicsLayer
+                  (position 40.00 20.00)
+                  (bounds 100.00 100.00)
+                  (contentsOpaque 1)
+                )
+              )
             )
-          )
-        )
-        (GraphicsLayer
-          (position 9.00 9.00)
-          (bounds 300.00 300.00)
-          (children 1
             (GraphicsLayer
-              (position 40.00 260.00)
-              (bounds 100.00 100.00)
-              (contentsOpaque 1)
+              (position 9.00 9.00)
+              (bounds 300.00 300.00)
+              (children 1
+                (GraphicsLayer
+                  (position 40.00 260.00)
+                  (bounds 100.00 100.00)
+                  (contentsOpaque 1)
+                )
+              )
+            )
+            (GraphicsLayer
+              (position 9.00 9.00)
+              (bounds 300.00 300.00)
+              (children 1
+                (GraphicsLayer
+                  (position 40.00 380.00)
+                  (bounds 100.00 100.00)
+                  (contentsOpaque 1)
+                )
+              )
             )
           )
         )
diff --git a/LayoutTests/platform/ios-wk2/compositing/scrolling/async-overflow-scrolling/layer-for-negative-z-in-scroller-expected.txt b/LayoutTests/platform/ios-wk2/compositing/scrolling/async-overflow-scrolling/layer-for-negative-z-in-scroller-expected.txt
new file mode 100644 (file)
index 0000000..85ccdaf
--- /dev/null
@@ -0,0 +1,35 @@
+(GraphicsLayer
+  (anchor 0.00 0.00)
+  (bounds 800.00 600.00)
+  (clips 1)
+  (children 1
+    (GraphicsLayer
+      (bounds 800.00 600.00)
+      (contentsOpaque 1)
+      (children 1
+        (GraphicsLayer
+          (position 8.00 8.00)
+          (bounds 302.00 302.00)
+          (drawsContent 1)
+          (children 1
+            (GraphicsLayer
+              (offsetFromRenderer width=1 height=1)
+              (position 1.00 1.00)
+              (bounds 300.00 300.00)
+              (clips 1)
+              (children 1
+                (GraphicsLayer
+                  (offsetFromRenderer width=1 height=1)
+                  (anchor 0.00 0.00)
+                  (bounds 300.00 1240.00)
+                  (drawsContent 1)
+                )
+              )
+            )
+          )
+        )
+      )
+    )
+  )
+)
+
index 7647e2e..8d96467 100644 (file)
                       (bounds 140.00 140.00)
                       (contentsOpaque 1)
                       (drawsContent 1)
-                      (children 1
-                        (GraphicsLayer
-                          (bounds 140.00 140.00)
-                        )
-                      )
                     )
                   )
                 )
index 1648a49..fd6996d 100644 (file)
@@ -27,7 +27,8 @@ middlebottom
           (children 1
             (GraphicsLayer
               (offsetFromRenderer width=0 height=100)
-              (bounds 149.00 200.00)
+              (bounds 149.00 2086.00)
+              (usingTiledLayer 1)
               (drawsContent 1)
             )
           )
index c0cc9de..3f6d99c 100644 (file)
@@ -1,3 +1,52 @@
+2019-06-26  Simon Fraser  <simon.fraser@apple.com>
+
+        [Async overflow scrolling] Fix missing or misplaced content inside overflow:scroll
+        https://bugs.webkit.org/show_bug.cgi?id=199253
+        rdar://problem/51855156, rdar://problem/51934514
+
+        Reviewed by Zalan Bujtas.
+
+        This patch fixes a couple of related issues triggered by failing to composite layers inside non-stacking-context
+        overflow scroll.
+
+        First, we relied on overlap testing to composite position:relative layers inside overflow:scroll, but this only
+        worked when they came later in z-order, so didn't work for layers with negative z-index.
+        RenderLayerCompositor::requiresCompositingForIndirectReason() was intended to trigger compositing in such cases,
+        but it only did so for position:absolute inside stacking-context scroller, because
+        isNonScrolledLayerInsideScrolledCompositedAncestor() tested ancestorMovedByScroller && !layerMovedByScroller.
+
+        I fixed this by sharing code between the three places that ask whether compositing crosses a containing-block
+        boundary to call a single function, RenderLayerCompositor::layerScrollBehahaviorRelativeToCompositedAncestor(),
+        that returns a ScrollPositioningBehavior. We now do compositing for both "moves" and "stationary" behaviors (but
+        not "none"), ensuring that position:relative inside non-stacking scroller is always composited.
+
+        However, this would trigger compositing on layers that should be using backing sharing; if they were outside the
+        visible part of the scroller, the overlap code would not trigger, but the
+        "IndirectCompositingReason::OverflowScrollPositioning" code would. This is undesirable; any layer that can use
+        backing sharing should, because that's fewer composited layers, so smaller layer trees and less backing store.
+        To fix this, I moved the backing-sharing check before the overlap check in
+        RenderLayerCompositor::computeCompositingRequirements().
+
+        The "layer.setHasCompositingDescendant(currentState.subtreeIsCompositing)" line was in the wrong place,
+        triggering assertions on some content; "subtreeIsCompositing" only refers to child layers, so this bit needs to
+        be set right after we've traversed the z-order lists.
+
+        Tests: compositing/scrolling/async-overflow-scrolling/hidden-relative-layer-content-in-scroller.html
+               compositing/scrolling/async-overflow-scrolling/layer-for-negative-z-in-scroller.html
+               compositing/scrolling/async-overflow-scrolling/negative-z-in-scroller.html
+
+        * rendering/RenderLayerCompositor.cpp:
+        (WebCore::RenderLayerCompositor::computeCompositingRequirements):
+        (WebCore::RenderLayerCompositor::traverseUnchangedSubtree):
+        (WebCore::RenderLayerCompositor::requiresCompositingForIndirectReason const):
+        (WebCore::isScrolledByOverflowScrollLayer):
+        (WebCore::enclosingCompositedScrollingLayer):
+        (WebCore::RenderLayerCompositor::layerScrollBehahaviorRelativeToCompositedAncestor):
+        (WebCore::RenderLayerCompositor::computeCoordinatedPositioningForLayer const):
+        (WebCore::isNonScrolledLayerInsideScrolledCompositedAncestor): Deleted.
+        (WebCore::RenderLayerCompositor::layerContainingBlockCrossesCoordinatedScrollingBoundary): Deleted.
+        * rendering/RenderLayerCompositor.h:
+
 2019-06-26  Ryosuke Niwa  <rniwa@webkit.org>
 
         ReplacementFragment should not have script observable side effects
index d6becc3..ffa15da 100644 (file)
@@ -863,12 +863,21 @@ void RenderLayerCompositor::computeCompositingRequirements(RenderLayer* ancestor
     RequiresCompositingData queryData;
     bool willBeComposited = layer.isComposited();
     bool becameCompositedAfterDescendantTraversal = false;
+    IndirectCompositingReason compositingReason = compositingState.subtreeIsCompositing ? IndirectCompositingReason::Stacking : IndirectCompositingReason::None;
 
     if (layer.needsPostLayoutCompositingUpdate() || compositingState.fullPaintOrderTraversalRequired || compositingState.descendantsRequireCompositingUpdate) {
         layer.setIndirectCompositingReason(IndirectCompositingReason::None);
         willBeComposited = needsToBeComposited(layer, queryData);
     }
 
+    bool layerPaintsIntoProvidedBacking = false;
+    if (!willBeComposited && compositingState.subtreeIsCompositing && backingSharingState.backingProviderCandidate() && canBeComposited(layer) && backingProviderLayerCanIncludeLayer(*backingSharingState.backingProviderCandidate(), layer)) {
+        backingSharingState.appendSharingLayer(layer);
+        LOG(Compositing, " layer %p can share with %p", &layer, backingSharingState.backingProviderCandidate());
+        compositingReason = IndirectCompositingReason::None;
+        layerPaintsIntoProvidedBacking = true;
+    }
+
     compositingState.fullPaintOrderTraversalRequired |= layer.subsequentLayersNeedCompositingRequirementsTraversal();
 
     OverlapExtent layerExtent;
@@ -880,22 +889,12 @@ void RenderLayerCompositor::computeCompositingRequirements(RenderLayer* ancestor
     bool respectTransforms = !layerExtent.hasTransformAnimation;
     overlapMap.geometryMap().pushMappingsToAncestor(&layer, ancestorLayer, respectTransforms);
 
-    IndirectCompositingReason compositingReason = compositingState.subtreeIsCompositing ? IndirectCompositingReason::Stacking : IndirectCompositingReason::None;
-    bool layerPaintsIntoProvidedBacking = false;
-    bool didPushOverlapContainer = false;
-
     // If we know for sure the layer is going to be composited, don't bother looking it up in the overlap map
-    if (!willBeComposited && !overlapMap.isEmpty() && compositingState.testingOverlap) {
+    if (!willBeComposited && !layerPaintsIntoProvidedBacking && !overlapMap.isEmpty() && compositingState.testingOverlap) {
         // If we're testing for overlap, we only need to composite if we overlap something that is already composited.
-        if (layerOverlaps(overlapMap, layer, layerExtent)) {
-            if (backingSharingState.backingProviderCandidate() && canBeComposited(layer) && backingProviderLayerCanIncludeLayer(*backingSharingState.backingProviderCandidate(), layer)) {
-                backingSharingState.appendSharingLayer(layer);
-                LOG(Compositing, " layer %p can share with %p", &layer, backingSharingState.backingProviderCandidate());
-                compositingReason = IndirectCompositingReason::None;
-                layerPaintsIntoProvidedBacking = true;
-            } else
-                compositingReason = IndirectCompositingReason::Overlap;
-        } else
+        if (layerOverlaps(overlapMap, layer, layerExtent))
+            compositingReason = IndirectCompositingReason::Overlap;
+        else
             compositingReason = IndirectCompositingReason::None;
     }
 
@@ -922,6 +921,7 @@ void RenderLayerCompositor::computeCompositingRequirements(RenderLayer* ancestor
     // a compositing layer among them, so start by inheriting the compositing
     // ancestor with subtreeIsCompositing set to false.
     CompositingState currentState = compositingState.stateForPaintOrderChildren(layer);
+    bool didPushOverlapContainer = false;
 
     auto layerWillComposite = [&] {
         // This layer is going to be composited, so children can safely ignore the fact that there's an
@@ -993,6 +993,9 @@ void RenderLayerCompositor::computeCompositingRequirements(RenderLayer* ancestor
     for (auto* childLayer : layer.positiveZOrderLayers())
         computeCompositingRequirements(&layer, *childLayer, overlapMap, currentState, backingSharingState, anyDescendantHas3DTransform);
 
+    // Set the flag to say that this layer has compositing children.
+    layer.setHasCompositingDescendant(currentState.subtreeIsCompositing);
+
     // If we just entered compositing mode, the root will have become composited (as long as accelerated compositing is enabled).
     if (layer.isRenderViewLayer()) {
         if (usesCompositing() && m_hasAcceleratedCompositing)
@@ -1022,9 +1025,6 @@ void RenderLayerCompositor::computeCompositingRequirements(RenderLayer* ancestor
         layer.reflectionLayer()->setIndirectCompositingReason(willBeComposited ? IndirectCompositingReason::Stacking : IndirectCompositingReason::None);
     }
 
-    // Set the flag to say that this layer has compositing children.
-    layer.setHasCompositingDescendant(currentState.subtreeIsCompositing);
-
     // setHasCompositingDescendant() may have changed the answer to needsToBeComposited() when clipping, so test that now.
     bool isCompositedClippingLayer = canBeComposited(layer) && clipsCompositingDescendants(layer);
     if (isCompositedClippingLayer & !willBeComposited)
@@ -1150,7 +1150,7 @@ void RenderLayerCompositor::traverseUnchangedSubtree(RenderLayer* ancestorLayer,
         if (currentState.subtreeIsCompositing)
             ASSERT(layerIsComposited);
     }
-    
+
     for (auto* childLayer : layer.normalFlowLayers())
         traverseUnchangedSubtree(&layer, *childLayer, overlapMap, currentState, backingSharingState, anyDescendantHas3DTransform);
 
@@ -2992,43 +2992,6 @@ bool RenderLayerCompositor::requiresCompositingForOverflowScrolling(const Render
     return layer.hasCompositedScrollableOverflow();
 }
 
-static RenderLayer* enclosingCompositedScrollingLayer(const RenderLayer& layer, const RenderLayer& intermediateLayer, bool& sawIntermediateLayer)
-{
-    const auto* ancestorLayer = layer.parent();
-    while (ancestorLayer) {
-        if (ancestorLayer == &intermediateLayer)
-            sawIntermediateLayer = true;
-
-        if (ancestorLayer->hasCompositedScrollableOverflow())
-            return const_cast<RenderLayer*>(ancestorLayer);
-
-        ancestorLayer = ancestorLayer->parent();
-    }
-
-    return nullptr;
-}
-
-static bool isScrolledByOverflowScrollLayer(const RenderLayer& layer, const RenderLayer& overflowScrollLayer)
-{
-    bool scrolledByOverflowScroll = false;
-    traverseAncestorLayers(layer, [&](const RenderLayer& ancestorLayer, bool inContainingBlockChain, bool) {
-        if (&ancestorLayer == &overflowScrollLayer) {
-            scrolledByOverflowScroll = inContainingBlockChain;
-            return AncestorTraversal::Stop;
-        }
-        return AncestorTraversal::Continue;
-    });
-    return scrolledByOverflowScroll;
-}
-
-static bool isNonScrolledLayerInsideScrolledCompositedAncestor(const RenderLayer& layer, const RenderLayer& compositedAncestor, const RenderLayer& scrollingAncestor)
-{
-    bool ancestorMovedByScroller = &compositedAncestor == &scrollingAncestor || isScrolledByOverflowScrollLayer(compositedAncestor, scrollingAncestor);
-    bool layerMovedByScroller = isScrolledByOverflowScrollLayer(layer, scrollingAncestor);
-
-    return ancestorMovedByScroller && !layerMovedByScroller;
-}
-
 // FIXME: why doesn't this handle the clipping cases?
 bool RenderLayerCompositor::requiresCompositingForIndirectReason(const RenderLayer& layer, bool hasCompositedDescendants, bool has3DTransformedDescendants, bool paintsIntoProvidedBacking, IndirectCompositingReason& reason) const
 {
@@ -3057,9 +3020,7 @@ bool RenderLayerCompositor::requiresCompositingForIndirectReason(const RenderLay
     // If this layer scrolls independently from the layer that it would paint into, it needs to get composited.
     if (!paintsIntoProvidedBacking && layer.hasCompositedScrollingAncestor()) {
         auto* paintDestination = layer.paintOrderParent();
-        bool paintDestinationIsScrolling = false;
-        auto* scrollingAncestor = enclosingCompositedScrollingLayer(layer, *paintDestination, paintDestinationIsScrolling);
-        if (isNonScrolledLayerInsideScrolledCompositedAncestor(layer, *paintDestination, *scrollingAncestor)) {
+        if (paintDestination && layerScrollBehahaviorRelativeToCompositedAncestor(layer, *paintDestination) != ScrollPositioningBehavior::None) {
             reason = IndirectCompositingReason::OverflowScrollPositioning;
             return true;
         }
@@ -3164,19 +3125,54 @@ bool RenderLayerCompositor::useCoordinatedScrollingForLayer(const RenderLayer& l
     return false;
 }
 
-bool RenderLayerCompositor::layerContainingBlockCrossesCoordinatedScrollingBoundary(const RenderLayer& layer, const RenderLayer& compositedAncestor)
+static bool isScrolledByOverflowScrollLayer(const RenderLayer& layer, const RenderLayer& overflowScrollLayer)
 {
+    bool scrolledByOverflowScroll = false;
+    traverseAncestorLayers(layer, [&](const RenderLayer& ancestorLayer, bool inContainingBlockChain, bool) {
+        if (&ancestorLayer == &overflowScrollLayer) {
+            scrolledByOverflowScroll = inContainingBlockChain;
+            return AncestorTraversal::Stop;
+        }
+        return AncestorTraversal::Continue;
+    });
+    return scrolledByOverflowScroll;
+}
+
+static RenderLayer* enclosingCompositedScrollingLayer(const RenderLayer& layer, const RenderLayer& intermediateLayer, bool& sawIntermediateLayer)
+{
+    const auto* ancestorLayer = layer.parent();
+    while (ancestorLayer) {
+        if (ancestorLayer == &intermediateLayer)
+            sawIntermediateLayer = true;
+
+        if (ancestorLayer->hasCompositedScrollableOverflow())
+            return const_cast<RenderLayer*>(ancestorLayer);
+
+        ancestorLayer = ancestorLayer->parent();
+    }
+
+    return nullptr;
+}
+
+ScrollPositioningBehavior RenderLayerCompositor::layerScrollBehahaviorRelativeToCompositedAncestor(const RenderLayer& layer, const RenderLayer& compositedAncestor)
+{
+    if (!layer.hasCompositedScrollingAncestor())
+        return ScrollPositioningBehavior::None;
+
     bool compositedAncestorIsInsideScroller = false;
     auto* scrollingAncestor = enclosingCompositedScrollingLayer(layer, compositedAncestor, compositedAncestorIsInsideScroller);
     if (!scrollingAncestor) {
         ASSERT_NOT_REACHED(); // layer.hasCompositedScrollingAncestor() should guarantee we have one.
-        return false;
+        return ScrollPositioningBehavior::None;
     }
     
-    if (!compositedAncestorIsInsideScroller)
-        return false;
+    bool ancestorMovedByScroller = &compositedAncestor == scrollingAncestor || (compositedAncestorIsInsideScroller && isScrolledByOverflowScrollLayer(compositedAncestor, *scrollingAncestor));
+    bool layerMovedByScroller = isScrolledByOverflowScrollLayer(layer, *scrollingAncestor);
 
-    return isNonScrolledLayerInsideScrolledCompositedAncestor(layer, compositedAncestor, *scrollingAncestor);
+    if (ancestorMovedByScroller == layerMovedByScroller)
+        return ScrollPositioningBehavior::None;
+
+    return layerMovedByScroller ? ScrollPositioningBehavior::Moves : ScrollPositioningBehavior::Stationary;
 }
 
 static void collectStationaryLayerRelatedOverflowNodes(const RenderLayer& layer, const RenderLayer&, Vector<ScrollingNodeID>& scrollingNodes)
@@ -3227,27 +3223,7 @@ ScrollPositioningBehavior RenderLayerCompositor::computeCoordinatedPositioningFo
         return ScrollPositioningBehavior::None;
     }
 
-    bool compositedAncestorIsScrolling = false;
-    auto* scrollingAncestor = enclosingCompositedScrollingLayer(layer, *compositedAncestor, compositedAncestorIsScrolling);
-    if (!scrollingAncestor) {
-        ASSERT_NOT_REACHED(); // layer.hasCompositedScrollingAncestor() should guarantee we have one.
-        return ScrollPositioningBehavior::None;
-    }
-
-    // There are two cases we have to deal with here:
-    // 1. There's a composited overflow:scroll in the parent chain between the renderer and its containing block, and the layer's
-    //    composited (z-order) ancestor is inside the scroller or is the scroller. In this case, we have to compensate for scroll position
-    //    changes to make the positioned layer stay in the same place. This only applies to position:absolute or descendants of position:absolute.
-    if (compositedAncestorIsScrolling && isNonScrolledLayerInsideScrolledCompositedAncestor(layer, *compositedAncestor, *scrollingAncestor))
-        return ScrollPositioningBehavior::Stationary;
-
-    // 2. The layer's containing block is the overflow or inside the overflow:scroll, but its z-order ancestor is
-    //    outside the overflow:scroll. In that case, we have to move the layer via the scrolling tree to make
-    //    it move along with the overflow scrolling.
-    if (!compositedAncestorIsScrolling && isScrolledByOverflowScrollLayer(layer, *scrollingAncestor))
-        return ScrollPositioningBehavior::Moves;
-
-    return ScrollPositioningBehavior::None;
+    return layerScrollBehahaviorRelativeToCompositedAncestor(layer, *compositedAncestor);
 }
 
 static Vector<ScrollingNodeID> collectRelatedCoordinatedScrollingNodes(const RenderLayer& layer, ScrollPositioningBehavior positioningBehavior)
index 21c06d9..ec35d7c 100644 (file)
@@ -497,7 +497,7 @@ private:
     bool requiresCompositingForEditableImage(RenderLayerModelObject&) const;
     bool requiresCompositingForIndirectReason(const RenderLayer&, bool hasCompositedDescendants, bool has3DTransformedDescendants, bool paintsIntoProvidedBacking, IndirectCompositingReason&) const;
 
-    static bool layerContainingBlockCrossesCoordinatedScrollingBoundary(const RenderLayer&, const RenderLayer& compositedAncestor);
+    static ScrollPositioningBehavior layerScrollBehahaviorRelativeToCompositedAncestor(const RenderLayer&, const RenderLayer& compositedAncestor);
 
     static bool styleChangeMayAffectIndirectCompositingReasons(const RenderStyle& oldStyle, const RenderStyle& newStyle);