Title: [290136] branches/safari-613.1.17.1-branch/Source/WebCore
Revision
290136
Author
alanc...@apple.com
Date
2022-02-18 10:10:35 -0800 (Fri, 18 Feb 2022)

Log Message

Cherry-pick r290109. rdar://problem/89072235

    Crash under ScrollingCoordinatorMac::hasNodeWithAnimatedScrollChanged()
    https://bugs.webkit.org/show_bug.cgi?id=236810
    <rdar://89072235>

    Reviewed by Dean Jackson.

    Crash data suggests that r288933 didn't fix the crash entirely.

    Although the code paths I can find appear to hold the lock, there may be an
    opportunity for m_scrollingCoordinator to get nulled out after we null-check it
    but before the copy for the main thread dispatch.

    So make a RefPtr on the stack, null-check it, and then move it into the callback.

    * page/scrolling/ThreadedScrollingTree.cpp:
    (WebCore::ThreadedScrollingTree::reportSynchronousScrollingReasonsChanged):
    (WebCore::ThreadedScrollingTree::reportExposedUnfilledArea):
    (WebCore::ThreadedScrollingTree::currentSnapPointIndicesDidChange):
    (WebCore::ThreadedScrollingTree::handleWheelEventPhase):
    (WebCore::ThreadedScrollingTree::setActiveScrollSnapIndices):
    (WebCore::ThreadedScrollingTree::hasNodeWithAnimatedScrollChanged):

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

Modified Paths

Diff

Modified: branches/safari-613.1.17.1-branch/Source/WebCore/ChangeLog (290135 => 290136)


--- branches/safari-613.1.17.1-branch/Source/WebCore/ChangeLog	2022-02-18 18:10:31 UTC (rev 290135)
+++ branches/safari-613.1.17.1-branch/Source/WebCore/ChangeLog	2022-02-18 18:10:35 UTC (rev 290136)
@@ -1,3 +1,56 @@
+2022-02-18  Russell Epstein  <repst...@apple.com>
+
+        Cherry-pick r290109. rdar://problem/89072235
+
+    Crash under ScrollingCoordinatorMac::hasNodeWithAnimatedScrollChanged()
+    https://bugs.webkit.org/show_bug.cgi?id=236810
+    <rdar://89072235>
+    
+    Reviewed by Dean Jackson.
+    
+    Crash data suggests that r288933 didn't fix the crash entirely.
+    
+    Although the code paths I can find appear to hold the lock, there may be an
+    opportunity for m_scrollingCoordinator to get nulled out after we null-check it
+    but before the copy for the main thread dispatch.
+    
+    So make a RefPtr on the stack, null-check it, and then move it into the callback.
+    
+    * page/scrolling/ThreadedScrollingTree.cpp:
+    (WebCore::ThreadedScrollingTree::reportSynchronousScrollingReasonsChanged):
+    (WebCore::ThreadedScrollingTree::reportExposedUnfilledArea):
+    (WebCore::ThreadedScrollingTree::currentSnapPointIndicesDidChange):
+    (WebCore::ThreadedScrollingTree::handleWheelEventPhase):
+    (WebCore::ThreadedScrollingTree::setActiveScrollSnapIndices):
+    (WebCore::ThreadedScrollingTree::hasNodeWithAnimatedScrollChanged):
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@290109 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2022-02-17  Simon Fraser  <simon.fra...@apple.com>
+
+            Crash under ScrollingCoordinatorMac::hasNodeWithAnimatedScrollChanged()
+            https://bugs.webkit.org/show_bug.cgi?id=236810
+            <rdar://89072235>
+
+            Reviewed by Dean Jackson.
+
+            Crash data suggests that r288933 didn't fix the crash entirely.
+
+            Although the code paths I can find appear to hold the lock, there may be an
+            opportunity for m_scrollingCoordinator to get nulled out after we null-check it
+            but before the copy for the main thread dispatch.
+
+            So make a RefPtr on the stack, null-check it, and then move it into the callback.
+
+            * page/scrolling/ThreadedScrollingTree.cpp:
+            (WebCore::ThreadedScrollingTree::reportSynchronousScrollingReasonsChanged):
+            (WebCore::ThreadedScrollingTree::reportExposedUnfilledArea):
+            (WebCore::ThreadedScrollingTree::currentSnapPointIndicesDidChange):
+            (WebCore::ThreadedScrollingTree::handleWheelEventPhase):
+            (WebCore::ThreadedScrollingTree::setActiveScrollSnapIndices):
+            (WebCore::ThreadedScrollingTree::hasNodeWithAnimatedScrollChanged):
+
 2022-02-17  Russell Epstein  <repst...@apple.com>
 
         Cherry-pick r289693. rdar://problem/88264857

Modified: branches/safari-613.1.17.1-branch/Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp (290135 => 290136)


--- branches/safari-613.1.17.1-branch/Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp	2022-02-18 18:10:31 UTC (rev 290135)
+++ branches/safari-613.1.17.1-branch/Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp	2022-02-18 18:10:35 UTC (rev 290136)
@@ -283,7 +283,11 @@
 
 void ThreadedScrollingTree::reportSynchronousScrollingReasonsChanged(MonotonicTime timestamp, OptionSet<SynchronousScrollingReason> reasons)
 {
-    RunLoop::main().dispatch([scrollingCoordinator = m_scrollingCoordinator, timestamp, reasons] {
+    auto scrollingCoordinator = m_scrollingCoordinator;
+    if (!scrollingCoordinator)
+        return;
+
+    RunLoop::main().dispatch([scrollingCoordinator = WTFMove(scrollingCoordinator), timestamp, reasons] {
         scrollingCoordinator->reportSynchronousScrollingReasonsChanged(timestamp, reasons);
     });
 }
@@ -290,7 +294,10 @@
 
 void ThreadedScrollingTree::reportExposedUnfilledArea(MonotonicTime timestamp, unsigned unfilledArea)
 {
-    RunLoop::main().dispatch([scrollingCoordinator = m_scrollingCoordinator, timestamp, unfilledArea] {
+    auto scrollingCoordinator = m_scrollingCoordinator;
+    if (!scrollingCoordinator)
+        return;
+    RunLoop::main().dispatch([scrollingCoordinator = WTFMove(scrollingCoordinator), timestamp, unfilledArea] {
         scrollingCoordinator->reportExposedUnfilledArea(timestamp, unfilledArea);
     });
 }
@@ -298,10 +305,11 @@
 #if PLATFORM(COCOA)
 void ThreadedScrollingTree::currentSnapPointIndicesDidChange(ScrollingNodeID nodeID, std::optional<unsigned> horizontal, std::optional<unsigned> vertical)
 {
-    if (!m_scrollingCoordinator)
+    auto scrollingCoordinator = m_scrollingCoordinator;
+    if (!scrollingCoordinator)
         return;
 
-    RunLoop::main().dispatch([scrollingCoordinator = m_scrollingCoordinator, nodeID, horizontal, vertical] {
+    RunLoop::main().dispatch([scrollingCoordinator = WTFMove(scrollingCoordinator), nodeID, horizontal, vertical] {
         scrollingCoordinator->setActiveScrollSnapIndices(nodeID, horizontal, vertical);
     });
 }
@@ -310,10 +318,11 @@
 #if PLATFORM(MAC)
 void ThreadedScrollingTree::handleWheelEventPhase(ScrollingNodeID nodeID, PlatformWheelEventPhase phase)
 {
-    if (!m_scrollingCoordinator)
+    auto scrollingCoordinator = m_scrollingCoordinator;
+    if (!scrollingCoordinator)
         return;
 
-    RunLoop::main().dispatch([scrollingCoordinator = m_scrollingCoordinator, nodeID, phase] {
+    RunLoop::main().dispatch([scrollingCoordinator = WTFMove(scrollingCoordinator), nodeID, phase] {
         scrollingCoordinator->handleWheelEventPhase(nodeID, phase);
     });
 }
@@ -320,10 +329,11 @@
 
 void ThreadedScrollingTree::setActiveScrollSnapIndices(ScrollingNodeID nodeID, std::optional<unsigned> horizontalIndex, std::optional<unsigned> verticalIndex)
 {
-    if (!m_scrollingCoordinator)
+    auto scrollingCoordinator = m_scrollingCoordinator;
+    if (!scrollingCoordinator)
         return;
     
-    RunLoop::main().dispatch([scrollingCoordinator = m_scrollingCoordinator, nodeID, horizontalIndex, verticalIndex] {
+    RunLoop::main().dispatch([scrollingCoordinator = WTFMove(scrollingCoordinator), nodeID, horizontalIndex, verticalIndex] {
         scrollingCoordinator->setActiveScrollSnapIndices(nodeID, horizontalIndex, verticalIndex);
     });
 }
@@ -377,10 +387,11 @@
 
 void ThreadedScrollingTree::hasNodeWithAnimatedScrollChanged(bool hasNodeWithAnimatedScroll)
 {
-    if (!m_scrollingCoordinator)
+    auto scrollingCoordinator = m_scrollingCoordinator;
+    if (!scrollingCoordinator)
         return;
 
-    RunLoop::main().dispatch([scrollingCoordinator = m_scrollingCoordinator, hasNodeWithAnimatedScroll] {
+    RunLoop::main().dispatch([scrollingCoordinator = WTFMove(scrollingCoordinator), hasNodeWithAnimatedScroll] {
         scrollingCoordinator->hasNodeWithAnimatedScrollChanged(hasNodeWithAnimatedScroll);
     });
 }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to