Modified: trunk/Source/WebCore/ChangeLog (290108 => 290109)
--- trunk/Source/WebCore/ChangeLog 2022-02-18 03:46:49 UTC (rev 290108)
+++ trunk/Source/WebCore/ChangeLog 2022-02-18 03:55:27 UTC (rev 290109)
@@ -1,3 +1,27 @@
+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 Chris Dumez <cdu...@apple.com>
Clean up / optimize call sites constructing vectors
Modified: trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp (290108 => 290109)
--- trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp 2022-02-18 03:46:49 UTC (rev 290108)
+++ trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp 2022-02-18 03:55:27 UTC (rev 290109)
@@ -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);
});
}
