Attachment 372805 Details for Bug 199176 – Patch

[patch] Patch

bug-199176-20190624150851.patch (text/plain), 6.33 KB, created by Simon Fraser (smfr) on 2019-06-24 15:08:52 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Simon Fraser (smfr)

Created: 2019-06-24 15:08:52 PDT

Size: 6.33 KB

patch

obsolete

>Subversion Revision: 246737
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 9d44f8e3995e16229fb3612f743fbe29129765b1..dc020044a4b3d3ae2eae4261085ab337f9bdf5a7 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,23 @@
>+2019-06-24  Simon Fraser  <simon.fraser@apple.com>
>+
>+        REGRESSION (r246725 ): Crashes on twitch.tv
>+        https://bugs.webkit.org/show_bug.cgi?id=199176
>+        rdar://problem/52071249
>+
>+        Reviewed by NOBODY (OOPS!).
>+        
>+        With a composited negative z-index child inside a scroller, we can register the overflow scroll
>+        proxy node before we've traversed the overflow layer, so it that layer hasn't got its OverflowScrollingNode
>+        yet. Thus, AsyncScrollingCoordinator::setRelatedOverflowScrollingNodes() can be called with an empty vector.
>+        Avoid crashing when this happens.
>+
>+        Test: scrollingcoordinator/scrolling-tree/scroller-with-negative-z-child.html
>+
>+        * page/scrolling/AsyncScrollingCoordinator.cpp:
>+        (WebCore::AsyncScrollingCoordinator::setRelatedOverflowScrollingNodes):
>+        * page/scrolling/cocoa/ScrollingTreeOverflowScrollProxyNode.mm:
>+        (WebCore::ScrollingTreeOverflowScrollProxyNode::commitStateBeforeChildren):
>+
> 2019-06-24  Greg Doolittle  <gr3g@apple.com>
> 
>         Web Inspector: AXI: Audit: image label test is throwing spurious errors on elements with existing alt attr, but no value: <img alt>
>diff --git a/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp b/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp
>index 509b62d252c207868eb1ba784c78292e16763da9..ca68f7630765cbcbe598d05e70ffeaf10237dac2 100644
>--- a/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp
>+++ b/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp
>@@ -715,8 +715,10 @@ void AsyncScrollingCoordinator::setRelatedOverflowScrollingNodes(ScrollingNodeID
>         downcast<ScrollingStatePositionedNode>(node)->setRelatedOverflowScrollingNodes(WTFMove(relatedNodes));
>     else if (is<ScrollingStateOverflowScrollProxyNode>(node)) {
>         auto* overflowScrollProxyNode = downcast<ScrollingStateOverflowScrollProxyNode>(node);
>-        ASSERT(relatedNodes.size() == 1);
>-        overflowScrollProxyNode->setOverflowScrollingNode(relatedNodes[0]);
>+        if (!relatedNodes.isEmpty())
>+            overflowScrollProxyNode->setOverflowScrollingNode(relatedNodes[0]);
>+        else
>+            overflowScrollProxyNode->setOverflowScrollingNode(0);
>     } else
>         ASSERT_NOT_REACHED();
> }
>diff --git a/Source/WebCore/page/scrolling/cocoa/ScrollingTreeOverflowScrollProxyNode.mm b/Source/WebCore/page/scrolling/cocoa/ScrollingTreeOverflowScrollProxyNode.mm
>index 94cd6a34d6611aef21f1a4768aaf9502f2b87e73..d5f3d14c9619a2d51ae41b5b6505bbb4ed643a0d 100644
>--- a/Source/WebCore/page/scrolling/cocoa/ScrollingTreeOverflowScrollProxyNode.mm
>+++ b/Source/WebCore/page/scrolling/cocoa/ScrollingTreeOverflowScrollProxyNode.mm
>@@ -57,10 +57,12 @@ void ScrollingTreeOverflowScrollProxyNode::commitStateBeforeChildren(const Scrol
>     if (overflowProxyStateNode.hasChangedProperty(ScrollingStateOverflowScrollProxyNode::OverflowScrollingNode))
>         m_overflowScrollingNodeID = overflowProxyStateNode.overflowScrollingNode();
> 
>-    auto& relatedNodes = scrollingTree().overflowRelatedNodes();
>-    relatedNodes.ensure(m_overflowScrollingNodeID, [] {
>-        return Vector<ScrollingNodeID>();
>-    }).iterator->value.append(scrollingNodeID());
>+    if (m_overflowScrollingNodeID) {
>+        auto& relatedNodes = scrollingTree().overflowRelatedNodes();
>+        relatedNodes.ensure(m_overflowScrollingNodeID, [] {
>+            return Vector<ScrollingNodeID>();
>+        }).iterator->value.append(scrollingNodeID());
>+    }
> 
>     scrollingTree().nodesWithRelatedOverflow().add(scrollingNodeID());
> }
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index c291dd47c02276c7159406c983c2a9d3a0786f83..4f83e29577ca6d96c576fe1b58712ec52a86e17e 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,13 @@
>+2019-06-24  Simon Fraser  <simon.fraser@apple.com>
>+
>+        REGRESSION (r246725 ): Crashes on twitch.tv
>+        https://bugs.webkit.org/show_bug.cgi?id=199176
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * scrollingcoordinator/scrolling-tree/scroller-with-negative-z-child-expected.txt: Added.
>+        * scrollingcoordinator/scrolling-tree/scroller-with-negative-z-child.html: Added.
>+
> 2019-06-24  Greg Doolittle  <gr3g@apple.com>
> 
>         Web Inspector: AXI: Audit: image label test is throwing spurious errors on elements with existing alt attr, but no value: <img alt>
>diff --git a/LayoutTests/scrollingcoordinator/scrolling-tree/scroller-with-negative-z-child-expected.txt b/LayoutTests/scrollingcoordinator/scrolling-tree/scroller-with-negative-z-child-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..5ba3e6203907af4bb454d07e7d47ebd0618ed774
>--- /dev/null
>+++ b/LayoutTests/scrollingcoordinator/scrolling-tree/scroller-with-negative-z-child-expected.txt
>@@ -0,0 +1,3 @@
>+This test should not assert or crash
>+
>+
>diff --git a/LayoutTests/scrollingcoordinator/scrolling-tree/scroller-with-negative-z-child.html b/LayoutTests/scrollingcoordinator/scrolling-tree/scroller-with-negative-z-child.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..01ed6748a74e2a5c0441fe79f0fc4c029393c273
>--- /dev/null
>+++ b/LayoutTests/scrollingcoordinator/scrolling-tree/scroller-with-negative-z-child.html
>@@ -0,0 +1,48 @@
>+<!DOCTYPE html> 
>+<html>
>+<head>
>+    <style>
>+		.scroller {
>+			position: relative;
>+			overflow-y: scroll;
>+			width: 300px;
>+			height: 300px;
>+			border: 1px solid black;
>+		}
>+		
>+		.box {
>+			position: relative;
>+			margin: 20px;
>+		    width: 100px;
>+		    height: 100px;
>+		    background-color: green;
>+		}
>+
>+		.inside {
>+			left: 20px;
>+		}
>+		
>+		.negative {
>+			z-index: -1;
>+		    transform: translateZ(0);
>+		}
>+
>+		.spacer {
>+		    width: 40px;
>+		    height: 500px;
>+		    background-color: silver;
>+		}
>+    </style>
>+	<script>
>+		if (window.testRunner)
>+			testRunner.dumpAsText();
>+	</script>
>+</head>
>+<body>
>+    <p>This test should not assert or crash</p>
>+	<div class="scroller">
>+        <div class="inside negative box"></div>
>+        <div class="spacer"></div>
>+	</div>
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 199176: 372805