Changeset 246766 in webkit

Timestamp:

Jun 24, 2019 3:44:09 PM (5 years ago)

Author:

Simon Fraser

Message:

REGRESSION (r246725 ): Crashes on twitch.tv
​https://bugs.webkit.org/show_bug.cgi?id=199176
Source/WebCore:

rdar://problem/52071249

Reviewed by Zalan Bujtas.

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):

LayoutTests:

Reviewed by Zalan Bujtas.

• scrollingcoordinator/scrolling-tree/scroller-with-negative-z-child-expected.txt: Added.
• scrollingcoordinator/scrolling-tree/scroller-with-negative-z-child.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/scrollingcoordinator/scrolling-tree/scroller-with-negative-z-child-expected.txt (added)
• LayoutTests/scrollingcoordinator/scrolling-tree/scroller-with-negative-z-child.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp (modified) (1 diff)
• Source/WebCore/page/scrolling/cocoa/ScrollingTreeOverflowScrollProxyNode.mm (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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 Zalan Bujtas.
+
+        * scrollingcoordinator/scrolling-tree/scroller-with-negative-z-child-expected.txt: Added.
+        * scrollingcoordinator/scrolling-tree/scroller-with-negative-z-child.html: Added.
+
 2019-06-24  Alexey Shvayka  <shvaikalesh@gmail.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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 Zalan Bujtas.
+        
+        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  Chris Dumez  <cdumez@apple.com>
 

trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp

@@ -716 +716 @@
     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();

trunk/Source/WebCore/page/scrolling/cocoa/ScrollingTreeOverflowScrollProxyNode.mm

@@ -58 +58 @@
         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());