- Revision
- 291073
- Author
- [email protected]
- Date
- 2022-03-09 14:16:31 -0800 (Wed, 09 Mar 2022)
Log Message
Cherry-pick r290760. rdar://problem/83745510
Rendering issues with many dynamically-added sticky elements inside overflow scroll
https://bugs.webkit.org/show_bug.cgi?id=237378
<rdar://83745510>
Reviewed by Cameron McCormack.
Source/WebCore:
A bug was filed describing a symptom where position:sticky elements would eventually
fail to render when many of them were dynamically added inside a non-stacking context
overflow scroll.
Debugging showed that CALayers were accumulating, causing us to hit per-process IOSurface
limits, at which point content fails to render (this is being fixed via rdar://89640915).
Further debugging showed that these layers were unparented, and being entrained by
ScrollingStateStickyNodes, which were accumulating in ScrollingStateTree's m_unparentedNodes.
This happened because with this scrolling configuration, each ScrollingStateStickyNode
is parented via a ScrollingStateOverflowScrollProxyNode which is referenced by entries
in RenderLayerBacking's m_ancestorClippingStack. On cleanup, the ScrollingStateOverflowScrollProxyNode
was unparented first, leaving the ScrollingStateStickyNode in m_unparentedNodes.
The fix is to have ScrollingStateTree::unparentChildrenAndDestroyNode() remove nodes
from m_unparentedNodes.
To test, add a m_unparentedNodes count to scrollingStateTreeAsText output, which means
having scrollingStateTreeAsText() as a member function on ScrollingStateTree.
Test: scrollingcoordinator/scrolling-tree/accumulated-unparented-sticky-nodes.html
* page/scrolling/AsyncScrollingCoordinator.cpp:
(WebCore::AsyncScrollingCoordinator::scrollingStateTreeAsText const):
* page/scrolling/ScrollingStateTree.cpp:
(WebCore::ScrollingStateTree::unparentChildrenAndDestroyNode):
(WebCore::ScrollingStateTree::scrollingStateTreeAsText const):
* page/scrolling/ScrollingStateTree.h:
LayoutTests:
* scrollingcoordinator/scrolling-tree/accumulated-unparented-sticky-nodes-expected.txt: Added.
* scrollingcoordinator/scrolling-tree/accumulated-unparented-sticky-nodes.html: Added.
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@290760 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Modified Paths
Added Paths
Diff
Modified: branches/safari-613-branch/LayoutTests/ChangeLog (291072 => 291073)
--- branches/safari-613-branch/LayoutTests/ChangeLog 2022-03-09 22:16:21 UTC (rev 291072)
+++ branches/safari-613-branch/LayoutTests/ChangeLog 2022-03-09 22:16:31 UTC (rev 291073)
@@ -1,5 +1,64 @@
2022-03-09 Russell Epstein <[email protected]>
+ Cherry-pick r290760. rdar://problem/83745510
+
+ Rendering issues with many dynamically-added sticky elements inside overflow scroll
+ https://bugs.webkit.org/show_bug.cgi?id=237378
+ <rdar://83745510>
+
+ Reviewed by Cameron McCormack.
+
+ Source/WebCore:
+
+ A bug was filed describing a symptom where position:sticky elements would eventually
+ fail to render when many of them were dynamically added inside a non-stacking context
+ overflow scroll.
+
+ Debugging showed that CALayers were accumulating, causing us to hit per-process IOSurface
+ limits, at which point content fails to render (this is being fixed via rdar://89640915).
+ Further debugging showed that these layers were unparented, and being entrained by
+ ScrollingStateStickyNodes, which were accumulating in ScrollingStateTree's m_unparentedNodes.
+
+ This happened because with this scrolling configuration, each ScrollingStateStickyNode
+ is parented via a ScrollingStateOverflowScrollProxyNode which is referenced by entries
+ in RenderLayerBacking's m_ancestorClippingStack. On cleanup, the ScrollingStateOverflowScrollProxyNode
+ was unparented first, leaving the ScrollingStateStickyNode in m_unparentedNodes.
+
+ The fix is to have ScrollingStateTree::unparentChildrenAndDestroyNode() remove nodes
+ from m_unparentedNodes.
+
+ To test, add a m_unparentedNodes count to scrollingStateTreeAsText output, which means
+ having scrollingStateTreeAsText() as a member function on ScrollingStateTree.
+
+ Test: scrollingcoordinator/scrolling-tree/accumulated-unparented-sticky-nodes.html
+
+ * page/scrolling/AsyncScrollingCoordinator.cpp:
+ (WebCore::AsyncScrollingCoordinator::scrollingStateTreeAsText const):
+ * page/scrolling/ScrollingStateTree.cpp:
+ (WebCore::ScrollingStateTree::unparentChildrenAndDestroyNode):
+ (WebCore::ScrollingStateTree::scrollingStateTreeAsText const):
+ * page/scrolling/ScrollingStateTree.h:
+
+ LayoutTests:
+
+ * scrollingcoordinator/scrolling-tree/accumulated-unparented-sticky-nodes-expected.txt: Added.
+ * scrollingcoordinator/scrolling-tree/accumulated-unparented-sticky-nodes.html: Added.
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@290760 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2022-03-02 Simon Fraser <[email protected]>
+
+ Rendering issues with many dynamically-added sticky elements inside overflow scroll
+ https://bugs.webkit.org/show_bug.cgi?id=237378
+ <rdar://83745510>
+
+ Reviewed by Cameron McCormack.
+
+ * scrollingcoordinator/scrolling-tree/accumulated-unparented-sticky-nodes-expected.txt: Added.
+ * scrollingcoordinator/scrolling-tree/accumulated-unparented-sticky-nodes.html: Added.
+
+2022-03-09 Russell Epstein <[email protected]>
+
Cherry-pick r290646. rdar://problem/88911184
focus({preventScroll: true}) does not prevent scrolling on iOS
Added: branches/safari-613-branch/LayoutTests/scrollingcoordinator/scrolling-tree/accumulated-unparented-sticky-nodes-expected.txt (0 => 291073)
--- branches/safari-613-branch/LayoutTests/scrollingcoordinator/scrolling-tree/accumulated-unparented-sticky-nodes-expected.txt (rev 0)
+++ branches/safari-613-branch/LayoutTests/scrollingcoordinator/scrolling-tree/accumulated-unparented-sticky-nodes-expected.txt 2022-03-09 22:16:31 UTC (rev 291073)
@@ -0,0 +1,23 @@
+7
+8
+8
+9
+9
+10
+10
+11
+11
+12
+12
+13
+13
+14
+14
+15
+15
+16
+ PASS window.internals?.scrollingStateTreeAsText().indexOf('unparented node count') == -1 is true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: branches/safari-613-branch/LayoutTests/scrollingcoordinator/scrolling-tree/accumulated-unparented-sticky-nodes.html (0 => 291073)
--- branches/safari-613-branch/LayoutTests/scrollingcoordinator/scrolling-tree/accumulated-unparented-sticky-nodes.html (rev 0)
+++ branches/safari-613-branch/LayoutTests/scrollingcoordinator/scrolling-tree/accumulated-unparented-sticky-nodes.html 2022-03-09 22:16:31 UTC (rev 291073)
@@ -0,0 +1,114 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ AsyncOverflowScrollingEnabled=true ] -->
+<html>
+<head>
+<style>
+ .viewport {
+ border: 1px solid #000;
+ height: 200px;
+ width: 300px;
+ overflow: auto;
+ }
+
+ .grid {
+ position: relative;
+ background-color: red;
+ display: inline-grid;
+ grid-template-columns: repeat(2, 120px);
+ grid-template-rows: repeat(25px);
+ grid-gap: 1px;
+ }
+
+ .grid > div {
+ background-color: white;
+ box-sizing: border-box;
+ height: 25px;
+ }
+</style>
+<script src=""
+<script>
+ jsTestIsAsync = true;
+
+ window.addEventListener('load', () => {
+ let viewport = document.querySelector('.viewport');
+ let canvas = document.querySelector('.canvas');
+ let grid = document.querySelector('.grid');
+ const gridGap = 1;
+ const rowHeight = 25;
+ const rowHeightWithGridGap = rowHeight + gridGap;
+
+ let data = "" index) => [
+ index,
+ index + 1
+ ]);
+
+ canvas.style = `
+ height: ${(rowHeight + gridGap) * 500}px;
+ width: ${(120 + gridGap) * 10}px
+ `;
+
+ viewport.addEventListener('scroll', getVisibleRows);
+
+ function getVisibleRows()
+ {
+ let { first, last } = getVisibleRowsRange();
+ let top = first * rowHeightWithGridGap;
+ redrawRows(data.slice(first, last), top);
+ }
+
+ function getVisibleRowsRange()
+ {
+ const { offsetHeight, scrollTop } = viewport;
+ const firstVisibleRow = Math.max(Math.floor((scrollTop + gridGap) / rowHeightWithGridGap), 0);
+ const lastVisibleRow = Math.min(Math.floor((scrollTop + offsetHeight) / rowHeightWithGridGap), data.length - 1);
+ return { first: firstVisibleRow, last: lastVisibleRow + 1 };
+ }
+
+ function redrawRows(rows, top)
+ {
+ clearExistingRows();
+ grid.style.top = `${top}px`;
+ drawRows(rows);
+ }
+
+ function clearExistingRows()
+ {
+ while (grid.firstChild)
+ grid.removeChild(grid.firstChild);
+ }
+
+ function drawRows(rows)
+ {
+ rows.forEach(row => {
+ row.forEach((cell, i) => {
+ let el = document.createElement('div');
+ el.style = i < 1 ? 'position: sticky' : '';
+ el.textContent = cell;
+ grid.appendChild(el);
+ });
+ });
+ }
+
+ getVisibleRows();
+
+ requestAnimationFrame(() => {
+ viewport.scrollTo(0, 200);
+
+ requestAnimationFrame(() => {
+ viewport.scrollTo(0, 400);
+ shouldBeTrue("window.internals?.scrollingStateTreeAsText().indexOf('unparented node count') == -1");
+ finishJSTest();
+ })
+ })
+ }, false);
+</script>
+</head>
+<body>
+ <div class="viewport">
+ <div class="canvas">
+ <div class="grid"></div>
+ </div>
+ </div>
+<div id="console"></div>
+<script src=""
+</body>
+</html>
\ No newline at end of file
Modified: branches/safari-613-branch/Source/WebCore/ChangeLog (291072 => 291073)
--- branches/safari-613-branch/Source/WebCore/ChangeLog 2022-03-09 22:16:21 UTC (rev 291072)
+++ branches/safari-613-branch/Source/WebCore/ChangeLog 2022-03-09 22:16:31 UTC (rev 291073)
@@ -1,5 +1,90 @@
2022-03-09 Russell Epstein <[email protected]>
+ Cherry-pick r290760. rdar://problem/83745510
+
+ Rendering issues with many dynamically-added sticky elements inside overflow scroll
+ https://bugs.webkit.org/show_bug.cgi?id=237378
+ <rdar://83745510>
+
+ Reviewed by Cameron McCormack.
+
+ Source/WebCore:
+
+ A bug was filed describing a symptom where position:sticky elements would eventually
+ fail to render when many of them were dynamically added inside a non-stacking context
+ overflow scroll.
+
+ Debugging showed that CALayers were accumulating, causing us to hit per-process IOSurface
+ limits, at which point content fails to render (this is being fixed via rdar://89640915).
+ Further debugging showed that these layers were unparented, and being entrained by
+ ScrollingStateStickyNodes, which were accumulating in ScrollingStateTree's m_unparentedNodes.
+
+ This happened because with this scrolling configuration, each ScrollingStateStickyNode
+ is parented via a ScrollingStateOverflowScrollProxyNode which is referenced by entries
+ in RenderLayerBacking's m_ancestorClippingStack. On cleanup, the ScrollingStateOverflowScrollProxyNode
+ was unparented first, leaving the ScrollingStateStickyNode in m_unparentedNodes.
+
+ The fix is to have ScrollingStateTree::unparentChildrenAndDestroyNode() remove nodes
+ from m_unparentedNodes.
+
+ To test, add a m_unparentedNodes count to scrollingStateTreeAsText output, which means
+ having scrollingStateTreeAsText() as a member function on ScrollingStateTree.
+
+ Test: scrollingcoordinator/scrolling-tree/accumulated-unparented-sticky-nodes.html
+
+ * page/scrolling/AsyncScrollingCoordinator.cpp:
+ (WebCore::AsyncScrollingCoordinator::scrollingStateTreeAsText const):
+ * page/scrolling/ScrollingStateTree.cpp:
+ (WebCore::ScrollingStateTree::unparentChildrenAndDestroyNode):
+ (WebCore::ScrollingStateTree::scrollingStateTreeAsText const):
+ * page/scrolling/ScrollingStateTree.h:
+
+ LayoutTests:
+
+ * scrollingcoordinator/scrolling-tree/accumulated-unparented-sticky-nodes-expected.txt: Added.
+ * scrollingcoordinator/scrolling-tree/accumulated-unparented-sticky-nodes.html: Added.
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@290760 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2022-03-02 Simon Fraser <[email protected]>
+
+ Rendering issues with many dynamically-added sticky elements inside overflow scroll
+ https://bugs.webkit.org/show_bug.cgi?id=237378
+ <rdar://83745510>
+
+ Reviewed by Cameron McCormack.
+
+ A bug was filed describing a symptom where position:sticky elements would eventually
+ fail to render when many of them were dynamically added inside a non-stacking context
+ overflow scroll.
+
+ Debugging showed that CALayers were accumulating, causing us to hit per-process IOSurface
+ limits, at which point content fails to render (this is being fixed via rdar://89640915).
+ Further debugging showed that these layers were unparented, and being entrained by
+ ScrollingStateStickyNodes, which were accumulating in ScrollingStateTree's m_unparentedNodes.
+
+ This happened because with this scrolling configuration, each ScrollingStateStickyNode
+ is parented via a ScrollingStateOverflowScrollProxyNode which is referenced by entries
+ in RenderLayerBacking's m_ancestorClippingStack. On cleanup, the ScrollingStateOverflowScrollProxyNode
+ was unparented first, leaving the ScrollingStateStickyNode in m_unparentedNodes.
+
+ The fix is to have ScrollingStateTree::unparentChildrenAndDestroyNode() remove nodes
+ from m_unparentedNodes.
+
+ To test, add a m_unparentedNodes count to scrollingStateTreeAsText output, which means
+ having scrollingStateTreeAsText() as a member function on ScrollingStateTree.
+
+ Test: scrollingcoordinator/scrolling-tree/accumulated-unparented-sticky-nodes.html
+
+ * page/scrolling/AsyncScrollingCoordinator.cpp:
+ (WebCore::AsyncScrollingCoordinator::scrollingStateTreeAsText const):
+ * page/scrolling/ScrollingStateTree.cpp:
+ (WebCore::ScrollingStateTree::unparentChildrenAndDestroyNode):
+ (WebCore::ScrollingStateTree::scrollingStateTreeAsText const):
+ * page/scrolling/ScrollingStateTree.h:
+
+2022-03-09 Russell Epstein <[email protected]>
+
Cherry-pick r290646. rdar://problem/88911184
focus({preventScroll: true}) does not prevent scrolling on iOS
Modified: branches/safari-613-branch/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp (291072 => 291073)
--- branches/safari-613-branch/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp 2022-03-09 22:16:21 UTC (rev 291072)
+++ branches/safari-613-branch/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp 2022-03-09 22:16:31 UTC (rev 291073)
@@ -949,7 +949,7 @@
if (m_scrollingStateTree->rootStateNode()) {
if (m_eventTrackingRegionsDirty)
m_scrollingStateTree->rootStateNode()->setEventTrackingRegions(absoluteEventTrackingRegions());
- return m_scrollingStateTree->rootStateNode()->scrollingStateTreeAsText(behavior);
+ return m_scrollingStateTree->scrollingStateTreeAsText(behavior);
}
return emptyString();
Modified: branches/safari-613-branch/Source/WebCore/page/scrolling/ScrollingStateTree.cpp (291072 => 291073)
--- branches/safari-613-branch/Source/WebCore/page/scrolling/ScrollingStateTree.cpp 2022-03-09 22:16:21 UTC (rev 291072)
+++ branches/safari-613-branch/Source/WebCore/page/scrolling/ScrollingStateTree.cpp 2022-03-09 22:16:31 UTC (rev 291073)
@@ -245,6 +245,7 @@
}
protectedNode->removeFromParent();
+ m_unparentedNodes.remove(nodeID);
willRemoveNode(protectedNode.get());
}
@@ -393,6 +394,17 @@
reconcileLayerPositionsRecursive(*scrollingNode, layoutViewport, action);
}
+String ScrollingStateTree::scrollingStateTreeAsText(OptionSet<ScrollingStateTreeAsTextBehavior> behavior) const
+{
+ if (!rootStateNode())
+ return emptyString();
+
+ auto stateTreeAsString = rootStateNode()->scrollingStateTreeAsText(behavior);
+ if (!m_unparentedNodes.isEmpty())
+ stateTreeAsString.append(makeString("\nunparented node count: ", m_unparentedNodes.size()));
+ return stateTreeAsString;
+}
+
} // namespace WebCore
#ifndef NDEBUG
Modified: branches/safari-613-branch/Source/WebCore/page/scrolling/ScrollingStateTree.h (291072 => 291073)
--- branches/safari-613-branch/Source/WebCore/page/scrolling/ScrollingStateTree.h 2022-03-09 22:16:21 UTC (rev 291072)
+++ branches/safari-613-branch/Source/WebCore/page/scrolling/ScrollingStateTree.h 2022-03-09 22:16:31 UTC (rev 291073)
@@ -85,6 +85,7 @@
--m_scrollingNodeCount;
}
+ String scrollingStateTreeAsText(OptionSet<ScrollingStateTreeAsTextBehavior>) const;
private:
void setRootStateNode(Ref<ScrollingStateFrameScrollingNode>&&);
