WebContent jetsams on Sony lens webpage due to spike of IOSurfaces
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 10 Feb 2020 22:27:47 +0000 (22:27 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 10 Feb 2020 22:27:47 +0000 (22:27 +0000)
https://bugs.webkit.org/show_bug.cgi?id=207493
rdar://problem/59020443

Reviewed by Zalan Bujtas.
Source/WebCore:

There were three issues that contributed to massive backing store allocation on
<https://www.sony.com/electronics/lenses/t/camera-lenses>.

The first, fixed in r256095, was that the Web Animations code unioned the untransitioning
bounds with the transitioning bounds, causing the computation of large extent rects.

The second, fixed in r256181, was that GraphicsLayerCA would keep hold of a transform
animation for an extra frame, causing a rendering update where
RenderLayerBacking::updateGeometry() would have cleared the extent, but GraphicsLayerCA
still thought transform was animating, causing GraphicsLayerCA::updateCoverage() to keep
backing store attached.

This patch is the final fix; when animations start and end, we need to ensure that
RenderLayerBacking::updateGeometry() is called so that we compute the animation extent in
the same frame that adds the animation.

Test: compositing/backing/transition-extent.html

* rendering/RenderLayerBacking.cpp:
(WebCore::RenderLayerBacking::startAnimation):
(WebCore::RenderLayerBacking::animationFinished):

LayoutTests:

Test with an out-of-view transitioning element which should not get backing store.

* compositing/backing/transition-extent-expected.txt: Added.
* compositing/backing/transition-extent.html: Added.
* platform/ios-wk2/compositing/backing/transition-extent-expected.txt: Added.

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

LayoutTests/ChangeLog
LayoutTests/compositing/backing/transition-extent-expected.txt [new file with mode: 0644]
LayoutTests/compositing/backing/transition-extent.html [new file with mode: 0644]
LayoutTests/platform/ios-wk2/compositing/backing/transition-extent-expected.txt [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderLayerBacking.cpp

index 05d166f..de1961d 100644 (file)
 
         * platform/win/TestExpectations:
 
+2020-02-10  Simon Fraser  <simon.fraser@apple.com>
+
+        WebContent jetsams on Sony lens webpage due to spike of IOSurfaces
+        https://bugs.webkit.org/show_bug.cgi?id=207493
+        rdar://problem/59020443
+
+        Reviewed by Zalan Bujtas.
+        
+        Test with an out-of-view transitioning element which should not get backing store.
+
+        * compositing/backing/transition-extent-expected.txt: Added.
+        * compositing/backing/transition-extent.html: Added.
+        * platform/ios-wk2/compositing/backing/transition-extent-expected.txt: Added.
+
 2020-02-10  Truitt Savell  <tsavell@apple.com>
 
         REGRESSION: [ Win ] animations/stacking-context-not-fill-forwards.html is failing
diff --git a/LayoutTests/compositing/backing/transition-extent-expected.txt b/LayoutTests/compositing/backing/transition-extent-expected.txt
new file mode 100644 (file)
index 0000000..09c1387
--- /dev/null
@@ -0,0 +1,31 @@
+The second box should not have attached backing store.
+
+(GraphicsLayer
+  (anchor 0.00 0.00)
+  (bounds 785.00 2024.00)
+  (backingStoreAttached 1)
+  (children 1
+    (GraphicsLayer
+      (bounds 785.00 2024.00)
+      (contentsOpaque 1)
+      (backingStoreAttached 1)
+      (children 2
+        (GraphicsLayer
+          (position 20.00 20.00)
+          (bounds 100.00 100.00)
+          (contentsOpaque 1)
+          (drawsContent 1)
+          (backingStoreAttached 1)
+        )
+        (GraphicsLayer
+          (position 20.00 1500.00)
+          (bounds 100.00 100.00)
+          (contentsOpaque 1)
+          (drawsContent 1)
+          (backingStoreAttached 0)
+        )
+      )
+    )
+  )
+)
+visible boxhidden box
diff --git a/LayoutTests/compositing/backing/transition-extent.html b/LayoutTests/compositing/backing/transition-extent.html
new file mode 100644 (file)
index 0000000..5e48f56
--- /dev/null
@@ -0,0 +1,55 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <style>
+        body {
+            height: 2000px;
+        }
+
+        .box {
+            position: absolute;
+            left: 20px;
+            top: 20px;
+            width: 100px;
+            height: 100px;
+            background-color: silver;
+            transition: transform 2s;
+            transform: rotate3d(0, 0, 1, 0deg)
+        }
+
+        body.changed .box {
+            transform: rotate3d(0, 0, 1, -180deg);
+        }
+        
+        .invisible {
+            top: 1500px;
+        }
+    </style>
+    <script>
+        if (window.testRunner) {
+            testRunner.dumpAsText();
+            testRunner.waitUntilDone();
+        }
+        window.addEventListener('load', () => {
+            let box = document.querySelector('.box');
+            box.addEventListener('transitionstart', () => {
+                requestAnimationFrame(() => {
+                    var out = document.getElementById('layers');
+                    out.innerText = internals.layerTreeAsText(document, internals.LAYER_TREE_INCLUDES_BACKING_STORE_ATTACHED);
+                    testRunner.notifyDone();
+                });
+            }, false);
+
+            requestAnimationFrame(() => {
+                document.body.classList.add('changed');
+            });
+        }, false);
+    </script>
+</head>
+<body>
+    <p>The second box should not have attached backing store.</p>
+<pre id="layers"></pre>
+    <div class="box">visible box</div>
+    <div class="invisible box">hidden box</div>
+</body>
+</html>
diff --git a/LayoutTests/platform/ios-wk2/compositing/backing/transition-extent-expected.txt b/LayoutTests/platform/ios-wk2/compositing/backing/transition-extent-expected.txt
new file mode 100644 (file)
index 0000000..23d2de8
--- /dev/null
@@ -0,0 +1,31 @@
+The second box should not have attached backing store.
+
+(GraphicsLayer
+  (anchor 0.00 0.00)
+  (bounds 800.00 2024.00)
+  (backingStoreAttached 1)
+  (children 1
+    (GraphicsLayer
+      (bounds 800.00 2024.00)
+      (contentsOpaque 1)
+      (backingStoreAttached 1)
+      (children 2
+        (GraphicsLayer
+          (position 20.00 20.00)
+          (bounds 100.00 100.00)
+          (contentsOpaque 1)
+          (drawsContent 1)
+          (backingStoreAttached 1)
+        )
+        (GraphicsLayer
+          (position 20.00 1500.00)
+          (bounds 100.00 100.00)
+          (contentsOpaque 1)
+          (drawsContent 1)
+          (backingStoreAttached 0)
+        )
+      )
+    )
+  )
+)
+visible boxhidden box
index c1ab268..b4c27c0 100644 (file)
         * rendering/svg/SVGRootInlineBox.cpp:
         (WebCore::SVGRootInlineBox::paint):
 
+2020-02-10  Simon Fraser  <simon.fraser@apple.com>
+
+        WebContent jetsams on Sony lens webpage due to spike of IOSurfaces
+        https://bugs.webkit.org/show_bug.cgi?id=207493
+        rdar://problem/59020443
+
+        Reviewed by Zalan Bujtas.
+
+        There were three issues that contributed to massive backing store allocation on
+        <https://www.sony.com/electronics/lenses/t/camera-lenses>.
+
+        The first, fixed in r256095, was that the Web Animations code unioned the untransitioning
+        bounds with the transitioning bounds, causing the computation of large extent rects.
+
+        The second, fixed in r256181, was that GraphicsLayerCA would keep hold of a transform
+        animation for an extra frame, causing a rendering update where
+        RenderLayerBacking::updateGeometry() would have cleared the extent, but GraphicsLayerCA
+        still thought transform was animating, causing GraphicsLayerCA::updateCoverage() to keep
+        backing store attached.
+
+        This patch is the final fix; when animations start and end, we need to ensure that
+        RenderLayerBacking::updateGeometry() is called so that we compute the animation extent in
+        the same frame that adds the animation.
+
+        Test: compositing/backing/transition-extent.html
+
+        * rendering/RenderLayerBacking.cpp:
+        (WebCore::RenderLayerBacking::startAnimation):
+        (WebCore::RenderLayerBacking::animationFinished):
+
 2020-02-10  Said Abou-Hallawa  <sabouhallawa@apple.com>
 
         Unreachable code hit in WebCore::Shape::createShape
index 7d0205c..1ef4efa 100644 (file)
@@ -3283,8 +3283,10 @@ bool RenderLayerBacking::startAnimation(double timeOffset, const Animation& anim
         didAnimate = true;
 #endif
 
-    if (didAnimate)
+    if (didAnimate) {
         m_owningLayer.setNeedsPostLayoutCompositingUpdate();
+        m_owningLayer.setNeedsCompositingGeometryUpdate();
+    }
 
     return didAnimate;
 }
@@ -3303,6 +3305,7 @@ void RenderLayerBacking::animationFinished(const String& animationName)
 {
     m_graphicsLayer->removeAnimation(animationName);
     m_owningLayer.setNeedsPostLayoutCompositingUpdate();
+    m_owningLayer.setNeedsCompositingGeometryUpdate();
 }
 
 bool RenderLayerBacking::startTransition(double timeOffset, CSSPropertyID property, const RenderStyle* fromStyle, const RenderStyle* toStyle)