- Revision
- 260716
- Author
- [email protected]
- Date
- 2020-04-25 18:24:03 -0700 (Sat, 25 Apr 2020)
Log Message
Commit the scrolling tree from the main thread
https://bugs.webkit.org/show_bug.cgi?id=211026
<rdar://problem/62374855>
Reviewed by Darin Adler.
ScrollingCoordinatorMac::commitTreeStateIfNeeded() passed the new state tree to
the scrolling thread which then did the commit (which updates the scrolling tree
from the state tree). However, applyLayerPositions() immediately waited for that
commit to complete, blocking the main thread anyway.
We might as well just commit the scrolling tree on the main thread. ScrollingTree::commitTreeState()
locks m_treeMutex, so this is still safe. Lock contention with the scrolling or event dispatcher
threads should be rare; those threads are both mostly responsive.
* page/scrolling/AsyncScrollingCoordinator.cpp:
(WebCore::AsyncScrollingCoordinator::scrollingTreeAsText const):
* page/scrolling/ScrollingTree.cpp:
(WebCore::ScrollingTree::commitTreeState):
* page/scrolling/ScrollingTree.h:
(WebCore::ScrollingTree::waitForScrollingTreeCommit): Deleted.
* page/scrolling/ThreadedScrollingTree.cpp:
(WebCore::ThreadedScrollingTree::commitTreeState): Deleted.
(WebCore::ThreadedScrollingTree::incrementPendingCommitCount): Deleted.
(WebCore::ThreadedScrollingTree::decrementPendingCommitCount): Deleted.
(WebCore::ThreadedScrollingTree::waitForPendingCommits): Deleted.
(WebCore::ThreadedScrollingTree::waitForScrollingTreeCommit): Deleted.
(WebCore::ThreadedScrollingTree::applyLayerPositions): Deleted.
* page/scrolling/ThreadedScrollingTree.h:
* page/scrolling/mac/ScrollingCoordinatorMac.mm:
(WebCore::ScrollingCoordinatorMac::commitTreeStateIfNeeded):
* page/scrolling/nicosia/ScrollingCoordinatorNicosia.cpp:
(WebCore::ScrollingCoordinatorNicosia::commitTreeState):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (260715 => 260716)
--- trunk/Source/WebCore/ChangeLog 2020-04-26 01:01:52 UTC (rev 260715)
+++ trunk/Source/WebCore/ChangeLog 2020-04-26 01:24:03 UTC (rev 260716)
@@ -1,3 +1,39 @@
+2020-04-25 Simon Fraser <[email protected]>
+
+ Commit the scrolling tree from the main thread
+ https://bugs.webkit.org/show_bug.cgi?id=211026
+ <rdar://problem/62374855>
+
+ Reviewed by Darin Adler.
+
+ ScrollingCoordinatorMac::commitTreeStateIfNeeded() passed the new state tree to
+ the scrolling thread which then did the commit (which updates the scrolling tree
+ from the state tree). However, applyLayerPositions() immediately waited for that
+ commit to complete, blocking the main thread anyway.
+
+ We might as well just commit the scrolling tree on the main thread. ScrollingTree::commitTreeState()
+ locks m_treeMutex, so this is still safe. Lock contention with the scrolling or event dispatcher
+ threads should be rare; those threads are both mostly responsive.
+
+ * page/scrolling/AsyncScrollingCoordinator.cpp:
+ (WebCore::AsyncScrollingCoordinator::scrollingTreeAsText const):
+ * page/scrolling/ScrollingTree.cpp:
+ (WebCore::ScrollingTree::commitTreeState):
+ * page/scrolling/ScrollingTree.h:
+ (WebCore::ScrollingTree::waitForScrollingTreeCommit): Deleted.
+ * page/scrolling/ThreadedScrollingTree.cpp:
+ (WebCore::ThreadedScrollingTree::commitTreeState): Deleted.
+ (WebCore::ThreadedScrollingTree::incrementPendingCommitCount): Deleted.
+ (WebCore::ThreadedScrollingTree::decrementPendingCommitCount): Deleted.
+ (WebCore::ThreadedScrollingTree::waitForPendingCommits): Deleted.
+ (WebCore::ThreadedScrollingTree::waitForScrollingTreeCommit): Deleted.
+ (WebCore::ThreadedScrollingTree::applyLayerPositions): Deleted.
+ * page/scrolling/ThreadedScrollingTree.h:
+ * page/scrolling/mac/ScrollingCoordinatorMac.mm:
+ (WebCore::ScrollingCoordinatorMac::commitTreeStateIfNeeded):
+ * page/scrolling/nicosia/ScrollingCoordinatorNicosia.cpp:
+ (WebCore::ScrollingCoordinatorNicosia::commitTreeState):
+
2020-04-25 Yusuke Suzuki <[email protected]>
Use static initialized Lock instead of LazyNeverDestroyed<Lock>
Modified: trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp (260715 => 260716)
--- trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp 2020-04-26 01:01:52 UTC (rev 260715)
+++ trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp 2020-04-26 01:24:03 UTC (rev 260716)
@@ -853,7 +853,6 @@
if (!m_scrollingTree)
return emptyString();
- m_scrollingTree->waitForScrollingTreeCommit();
return m_scrollingTree->scrollingTreeAsText(behavior);
}
Modified: trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp (260715 => 260716)
--- trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp 2020-04-26 01:01:52 UTC (rev 260715)
+++ trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp 2020-04-26 01:24:03 UTC (rev 260716)
@@ -191,7 +191,7 @@
m_rootNode->wasScrolledByDelegatedScrolling(scrollPosition, layoutViewport);
}
-void ScrollingTree::commitTreeState(std::unique_ptr<ScrollingStateTree> scrollingStateTree)
+void ScrollingTree::commitTreeState(std::unique_ptr<ScrollingStateTree>&& scrollingStateTree)
{
SetForScope<bool> inCommitTreeState(m_inCommitTreeState, true);
LockHolder locker(m_treeMutex);
Modified: trunk/Source/WebCore/page/scrolling/ScrollingTree.h (260715 => 260716)
--- trunk/Source/WebCore/page/scrolling/ScrollingTree.h 2020-04-26 01:01:52 UTC (rev 260715)
+++ trunk/Source/WebCore/page/scrolling/ScrollingTree.h 2020-04-26 01:24:03 UTC (rev 260716)
@@ -78,7 +78,7 @@
bool isScrollSnapInProgress();
virtual void invalidate() { }
- WEBCORE_EXPORT virtual void commitTreeState(std::unique_ptr<ScrollingStateTree>);
+ WEBCORE_EXPORT virtual void commitTreeState(std::unique_ptr<ScrollingStateTree>&&);
WEBCORE_EXPORT virtual void applyLayerPositions();
WEBCORE_EXPORT void applyLayerPositionsAfterCommit();
@@ -174,8 +174,6 @@
virtual void lockLayersForHitTesting() { }
virtual void unlockLayersForHitTesting() { }
- virtual void waitForScrollingTreeCommit() { }
-
protected:
FloatPoint mainFrameScrollPosition() const;
void setMainFrameScrollPosition(FloatPoint);
Modified: trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp (260715 => 260716)
--- trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp 2020-04-26 01:01:52 UTC (rev 260715)
+++ trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp 2020-04-26 01:24:03 UTC (rev 260716)
@@ -85,14 +85,6 @@
});
}
-void ThreadedScrollingTree::commitTreeState(std::unique_ptr<ScrollingStateTree> scrollingStateTree)
-{
- ASSERT(ScrollingThread::isCurrentThread());
- ScrollingTree::commitTreeState(WTFMove(scrollingStateTree));
-
- decrementPendingCommitCount();
-}
-
void ThreadedScrollingTree::propagateSynchronousScrollingReasons(const HashSet<ScrollingNodeID>& synchronousScrollingNodes)
{
auto propagateStateToAncestors = [&](ScrollingTreeNode& node) {
@@ -160,40 +152,6 @@
});
}
-void ThreadedScrollingTree::incrementPendingCommitCount()
-{
- LockHolder commitLocker(m_pendingCommitCountMutex);
- ++m_pendingCommitCount;
-}
-
-void ThreadedScrollingTree::decrementPendingCommitCount()
-{
- LockHolder commitLocker(m_pendingCommitCountMutex);
- ASSERT(m_pendingCommitCount > 0);
- if (!--m_pendingCommitCount)
- m_commitCondition.notifyOne();
-}
-
-void ThreadedScrollingTree::waitForPendingCommits()
-{
- ASSERT(isMainThread());
-
- LockHolder commitLocker(m_pendingCommitCountMutex);
- while (m_pendingCommitCount)
- m_commitCondition.wait(m_pendingCommitCountMutex);
-}
-
-void ThreadedScrollingTree::waitForScrollingTreeCommit()
-{
- waitForPendingCommits();
-}
-
-void ThreadedScrollingTree::applyLayerPositions()
-{
- waitForPendingCommits();
- ScrollingTree::applyLayerPositions();
-}
-
#if PLATFORM(COCOA)
void ThreadedScrollingTree::currentSnapPointIndicesDidChange(ScrollingNodeID nodeID, unsigned horizontal, unsigned vertical)
{
Modified: trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.h (260715 => 260716)
--- trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.h 2020-04-26 01:01:52 UTC (rev 260715)
+++ trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.h 2020-04-26 01:24:03 UTC (rev 260716)
@@ -44,8 +44,6 @@
public:
virtual ~ThreadedScrollingTree();
- void commitTreeState(std::unique_ptr<ScrollingStateTree>) override;
-
ScrollingEventResult handleWheelEvent(const PlatformWheelEvent&) override;
// Can be called from any thread. Will try to handle the wheel event on the scrolling thread.
@@ -55,9 +53,6 @@
void invalidate() override;
- void incrementPendingCommitCount();
- void decrementPendingCommitCount();
-
protected:
explicit ThreadedScrollingTree(AsyncScrollingCoordinator&);
@@ -77,17 +72,9 @@
private:
bool isThreadedScrollingTree() const override { return true; }
- void applyLayerPositions() override;
- void waitForScrollingTreeCommit() override;
void propagateSynchronousScrollingReasons(const HashSet<ScrollingNodeID>&) override;
RefPtr<AsyncScrollingCoordinator> m_scrollingCoordinator;
-
- void waitForPendingCommits();
-
- Lock m_pendingCommitCountMutex;
- unsigned m_pendingCommitCount { 0 };
- Condition m_commitCondition;
};
} // namespace WebCore
Modified: trunk/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm (260715 => 260716)
--- trunk/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm 2020-04-26 01:01:52 UTC (rev 260715)
+++ trunk/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm 2020-04-26 01:24:03 UTC (rev 260716)
@@ -97,23 +97,16 @@
{
willCommitTree();
- LOG(Scrolling, "ScrollingCoordinatorMac::commitTreeState, has changes %d", scrollingStateTree()->hasChangedProperties());
+ LOG_WITH_STREAM(Scrolling, stream << "ScrollingCoordinatorMac::commitTreeState, has changes " << scrollingStateTree()->hasChangedProperties());
if (!scrollingStateTree()->hasChangedProperties())
return;
- LOG(Scrolling, "%s", scrollingStateTreeAsText(ScrollingStateTreeAsTextBehaviorDebug).utf8().data());
+ LOG_WITH_STREAM(Scrolling, stream << scrollingStateTreeAsText(ScrollingStateTreeAsTextBehaviorDebug));
- RefPtr<ThreadedScrollingTree> threadedScrollingTree = downcast<ThreadedScrollingTree>(scrollingTree());
- ScrollingStateTree* unprotectedTreeState = scrollingStateTree()->commit(LayerRepresentation::PlatformLayerRepresentation).release();
+ auto stateTree = scrollingStateTree()->commit(LayerRepresentation::PlatformLayerRepresentation);
+ scrollingTree()->commitTreeState(WTFMove(stateTree));
- threadedScrollingTree->incrementPendingCommitCount();
-
- ScrollingThread::dispatch([threadedScrollingTree, unprotectedTreeState] {
- std::unique_ptr<ScrollingStateTree> treeState(unprotectedTreeState);
- threadedScrollingTree->commitTreeState(WTFMove(treeState));
- });
-
updateTiledScrollingIndicator();
}
Modified: trunk/Source/WebCore/page/scrolling/nicosia/ScrollingCoordinatorNicosia.cpp (260715 => 260716)
--- trunk/Source/WebCore/page/scrolling/nicosia/ScrollingCoordinatorNicosia.cpp 2020-04-26 01:01:52 UTC (rev 260715)
+++ trunk/Source/WebCore/page/scrolling/nicosia/ScrollingCoordinatorNicosia.cpp 2020-04-26 01:24:03 UTC (rev 260716)
@@ -94,13 +94,8 @@
if (!scrollingStateTree()->hasChangedProperties())
return;
- RefPtr<ThreadedScrollingTree> threadedScrollingTree = downcast<ThreadedScrollingTree>(scrollingTree());
- threadedScrollingTree->incrementPendingCommitCount();
-
- auto treeState = scrollingStateTree()->commit(LayerRepresentation::PlatformLayerRepresentation);
- ScrollingThread::dispatch([threadedScrollingTree, treeState = WTFMove(treeState)]() mutable {
- threadedScrollingTree->commitTreeState(WTFMove(treeState));
- });
+ auto stateTree = scrollingStateTree()->commit(LayerRepresentation::PlatformLayerRepresentation);
+ scrollingTree()->commitTreeState(WTFMove(stateTree));
}
} // namespace WebCore
