Title: [247129] trunk
- Revision
- 247129
- Author
- simon.fra...@apple.com
- Date
- 2019-07-03 18:29:55 -0700 (Wed, 03 Jul 2019)
Log Message
RELEASE_ASSERT in WebCore: WebCore::ScrollingStateTree::insertNode()
https://bugs.webkit.org/show_bug.cgi?id=199479
rdar://problem/52392556
Reviewed by Zalan Bujtas.
Source/WebCore:
Certain compositing tree updates could leave a layer with a ScrollingProxy role, but having an
AncestorClippingStack with no overflow scrolling layers - for example, a related scroller could become
scrollable, but we failed to mark the layer with the ancestor clippings stack as needing a geometry update.
When this happened updateScrollingNodeForScrollingProxyRole() would return 0, causing the next child to be
inserted with a parent of 0 (which should only happen for the root), and triggering a release assert in
ScrollingStateTree::insertNode().
Fix by ensuring that updateScrollingNodeForScrollingProxyRole() always returns the existing parentNodeID if we
don't have a new node to insert.
Test: scrollingcoordinator/scrolling-tree/scrolling-proxy-with-no-scrolling-layer.html
* rendering/RenderLayerBacking.cpp:
(WebCore::RenderLayerBacking::updateAncestorClippingStack):
* rendering/RenderLayerCompositor.cpp:
(WebCore::RenderLayerCompositor::updateScrollingNodeForScrollingProxyRole):
LayoutTests:
* scrollingcoordinator/scrolling-tree/scrolling-proxy-with-no-scrolling-layer-expected.txt: Added.
* scrollingcoordinator/scrolling-tree/scrolling-proxy-with-no-scrolling-layer.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (247128 => 247129)
--- trunk/LayoutTests/ChangeLog 2019-07-04 01:18:09 UTC (rev 247128)
+++ trunk/LayoutTests/ChangeLog 2019-07-04 01:29:55 UTC (rev 247129)
@@ -1,3 +1,14 @@
+2019-07-03 Simon Fraser <simon.fra...@apple.com>
+
+ RELEASE_ASSERT in WebCore: WebCore::ScrollingStateTree::insertNode()
+ https://bugs.webkit.org/show_bug.cgi?id=199479
+ rdar://problem/52392556
+
+ Reviewed by Zalan Bujtas.
+
+ * scrollingcoordinator/scrolling-tree/scrolling-proxy-with-no-scrolling-layer-expected.txt: Added.
+ * scrollingcoordinator/scrolling-tree/scrolling-proxy-with-no-scrolling-layer.html: Added.
+
2019-07-02 Myles C. Maxfield <mmaxfi...@apple.com>
[WHLSL] Standard library is too big to directly include in WebCore
Added: trunk/LayoutTests/scrollingcoordinator/scrolling-tree/scrolling-proxy-with-no-scrolling-layer-expected.txt (0 => 247129)
--- trunk/LayoutTests/scrollingcoordinator/scrolling-tree/scrolling-proxy-with-no-scrolling-layer-expected.txt (rev 0)
+++ trunk/LayoutTests/scrollingcoordinator/scrolling-tree/scrolling-proxy-with-no-scrolling-layer-expected.txt 2019-07-04 01:29:55 UTC (rev 247129)
@@ -0,0 +1,3 @@
+This test should not trigger assertions or crash.
+
+
Added: trunk/LayoutTests/scrollingcoordinator/scrolling-tree/scrolling-proxy-with-no-scrolling-layer.html (0 => 247129)
--- trunk/LayoutTests/scrollingcoordinator/scrolling-tree/scrolling-proxy-with-no-scrolling-layer.html (rev 0)
+++ trunk/LayoutTests/scrollingcoordinator/scrolling-tree/scrolling-proxy-with-no-scrolling-layer.html 2019-07-04 01:29:55 UTC (rev 247129)
@@ -0,0 +1,58 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <style>
+ .scroller {
+ overflow-x: hidden;
+ height: 400px;
+ width: 400px;
+ border: 1px solid black;
+ }
+
+ .wrapper {
+ position: relative;
+ }
+
+ .absolute {
+ position: absolute;
+ top: 10px;
+ left: 20px;
+ width: 200px;
+ height: 200px;
+ background-color: green;
+ transform: translateZ(0);
+ }
+
+ .content {
+ height: 300px;
+ }
+
+ .content.changed {
+ height: 700px;
+ }
+ </style>
+ <script>
+ if (window.testRunner) {
+ testRunner.waitUntilDone();
+ testRunner.dumpAsText();
+ }
+ window.addEventListener('load', () => {
+ setTimeout(() => {
+ document.querySelector('.content').classList.add('changed');
+ if (window.testRunner)
+ testRunner.notifyDone();
+ }, 0);
+ }, false);
+ </script>
+</head>
+<body>
+ <p>This test should not trigger assertions or crash.</p>
+ <div class="scroller">
+ <div class="wrapper">
+ <div class="content"></div>
+ <div class="absolute">
+ </div>
+ </div>
+ </div>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (247128 => 247129)
--- trunk/Source/WebCore/ChangeLog 2019-07-04 01:18:09 UTC (rev 247128)
+++ trunk/Source/WebCore/ChangeLog 2019-07-04 01:29:55 UTC (rev 247129)
@@ -1,3 +1,29 @@
+2019-07-03 Simon Fraser <simon.fra...@apple.com>
+
+ RELEASE_ASSERT in WebCore: WebCore::ScrollingStateTree::insertNode()
+ https://bugs.webkit.org/show_bug.cgi?id=199479
+ rdar://problem/52392556
+
+ Reviewed by Zalan Bujtas.
+
+ Certain compositing tree updates could leave a layer with a ScrollingProxy role, but having an
+ AncestorClippingStack with no overflow scrolling layers - for example, a related scroller could become
+ scrollable, but we failed to mark the layer with the ancestor clippings stack as needing a geometry update.
+
+ When this happened updateScrollingNodeForScrollingProxyRole() would return 0, causing the next child to be
+ inserted with a parent of 0 (which should only happen for the root), and triggering a release assert in
+ ScrollingStateTree::insertNode().
+
+ Fix by ensuring that updateScrollingNodeForScrollingProxyRole() always returns the existing parentNodeID if we
+ don't have a new node to insert.
+
+ Test: scrollingcoordinator/scrolling-tree/scrolling-proxy-with-no-scrolling-layer.html
+
+ * rendering/RenderLayerBacking.cpp:
+ (WebCore::RenderLayerBacking::updateAncestorClippingStack):
+ * rendering/RenderLayerCompositor.cpp:
+ (WebCore::RenderLayerCompositor::updateScrollingNodeForScrollingProxyRole):
+
2019-07-03 Konstantin Tokarev <annu...@yandex.ru>
RenderLayerCompositor.cpp should include RenderImage.h
Modified: trunk/Source/WebCore/rendering/RenderLayerBacking.cpp (247128 => 247129)
--- trunk/Source/WebCore/rendering/RenderLayerBacking.cpp 2019-07-04 01:18:09 UTC (rev 247128)
+++ trunk/Source/WebCore/rendering/RenderLayerBacking.cpp 2019-07-04 01:29:55 UTC (rev 247129)
@@ -1575,17 +1575,17 @@
if (!m_ancestorClippingStack) {
m_ancestorClippingStack = std::make_unique<LayerAncestorClippingStack>(WTFMove(clippingData));
- LOG_WITH_STREAM(Compositing, stream << "layer " << &m_owningLayer << " ancestorClippingStack " << *m_ancestorClippingStack);
+ LOG_WITH_STREAM(Compositing, stream << "layer " << &m_owningLayer << " ancestorClippingStack " << *m_ancestorClippingStack);
return true;
}
if (m_ancestorClippingStack->equalToClipData(clippingData)) {
- LOG_WITH_STREAM(Compositing, stream << "layer " << &m_owningLayer << " ancestorClippingStack " << *m_ancestorClippingStack);
+ LOG_WITH_STREAM(Compositing, stream << "layer " << &m_owningLayer << " ancestorClippingStack " << *m_ancestorClippingStack);
return false;
}
m_ancestorClippingStack->updateWithClipData(scrollingCoordinator, WTFMove(clippingData));
- LOG_WITH_STREAM(Compositing, stream << "layer " << &m_owningLayer << " ancestorClippingStack " << *m_ancestorClippingStack);
+ LOG_WITH_STREAM(Compositing, stream << "layer " << &m_owningLayer << " ancestorClippingStack " << *m_ancestorClippingStack);
return true;
}
Modified: trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp (247128 => 247129)
--- trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp 2019-07-04 01:18:09 UTC (rev 247128)
+++ trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp 2019-07-04 01:29:55 UTC (rev 247129)
@@ -4483,8 +4483,10 @@
{
auto* scrollingCoordinator = this->scrollingCoordinator();
auto* clippingStack = layer.backing()->ancestorClippingStack();
- if (!clippingStack)
- return 0;
+ if (!clippingStack) {
+ ASSERT_NOT_REACHED();
+ return treeState.parentNodeID.valueOr(0);
+ }
ScrollingNodeID nodeID = 0;
for (auto& entry : clippingStack->stack()) {
@@ -4508,7 +4510,7 @@
auto overflowScrollNodeID = 0;
if (auto* backing = entry.clipData.clippingLayer->backing())
overflowScrollNodeID = backing->scrollingNodeIDForRole(ScrollCoordinationRole::Scrolling);
-
+
Vector<ScrollingNodeID> scrollingNodeIDs;
if (overflowScrollNodeID)
scrollingNodeIDs.append(overflowScrollNodeID);
@@ -4516,6 +4518,9 @@
}
}
+ if (!nodeID)
+ return treeState.parentNodeID.valueOr(0);
+
return nodeID;
}
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
