Make sure we clean up CFTimerRefs when destroying scrolling tree nodes
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 22 May 2020 21:52:37 +0000 (21:52 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 22 May 2020 21:52:37 +0000 (21:52 +0000)
https://bugs.webkit.org/show_bug.cgi?id=212278
<rdar://problem/63548212>

Reviewed by Tim Horton.

When destroying scrolling tree nodes, make sure we explicitly stop the RunLoop::Timers,
and do this for all nodes when clearing the m_nodeMap, not just for orphaned nodes as
was done in r262042.

* page/scrolling/ScrollingTree.cpp:
(WebCore::ScrollingTree::commitTreeState):
(WebCore::ScrollingTree::updateTreeFromStateNodeRecursive):
(WebCore::ScrollingTree::removeAllNodes):
* page/scrolling/ScrollingTree.h:
* page/scrolling/ScrollingTreeNode.h:
(WebCore::ScrollingTreeNode::willBeDestroyed):
(WebCore::ScrollingTreeNode::wasRemovedFromTree): Deleted.
* page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.h:
* page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm:
(WebCore::ScrollingTreeFrameScrollingNodeMac::willBeDestroyed):
(WebCore::ScrollingTreeFrameScrollingNodeMac::wasRemovedFromTree): Deleted.
* page/scrolling/mac/ScrollingTreeOverflowScrollingNodeMac.h:
* page/scrolling/mac/ScrollingTreeOverflowScrollingNodeMac.mm:
(WebCore::ScrollingTreeOverflowScrollingNodeMac::willBeDestroyed):
(WebCore::ScrollingTreeOverflowScrollingNodeMac::wasRemovedFromTree): Deleted.

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

Source/WebCore/ChangeLog
Source/WebCore/page/scrolling/ScrollingTree.cpp
Source/WebCore/page/scrolling/ScrollingTree.h
Source/WebCore/page/scrolling/ScrollingTreeNode.h
Source/WebCore/page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.h
Source/WebCore/page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm
Source/WebCore/page/scrolling/mac/ScrollingTreeOverflowScrollingNodeMac.h
Source/WebCore/page/scrolling/mac/ScrollingTreeOverflowScrollingNodeMac.mm

index 5425a5c..a59c9c9 100644 (file)
@@ -1,3 +1,32 @@
+2020-05-22  Simon Fraser  <simon.fraser@apple.com>
+
+        Make sure we clean up CFTimerRefs when destroying scrolling tree nodes
+        https://bugs.webkit.org/show_bug.cgi?id=212278
+        <rdar://problem/63548212>
+
+        Reviewed by Tim Horton.
+
+        When destroying scrolling tree nodes, make sure we explicitly stop the RunLoop::Timers,
+        and do this for all nodes when clearing the m_nodeMap, not just for orphaned nodes as
+        was done in r262042.
+
+        * page/scrolling/ScrollingTree.cpp:
+        (WebCore::ScrollingTree::commitTreeState):
+        (WebCore::ScrollingTree::updateTreeFromStateNodeRecursive):
+        (WebCore::ScrollingTree::removeAllNodes):
+        * page/scrolling/ScrollingTree.h:
+        * page/scrolling/ScrollingTreeNode.h:
+        (WebCore::ScrollingTreeNode::willBeDestroyed):
+        (WebCore::ScrollingTreeNode::wasRemovedFromTree): Deleted.
+        * page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.h:
+        * page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm:
+        (WebCore::ScrollingTreeFrameScrollingNodeMac::willBeDestroyed):
+        (WebCore::ScrollingTreeFrameScrollingNodeMac::wasRemovedFromTree): Deleted.
+        * page/scrolling/mac/ScrollingTreeOverflowScrollingNodeMac.h:
+        * page/scrolling/mac/ScrollingTreeOverflowScrollingNodeMac.mm:
+        (WebCore::ScrollingTreeOverflowScrollingNodeMac::willBeDestroyed):
+        (WebCore::ScrollingTreeOverflowScrollingNodeMac::wasRemovedFromTree): Deleted.
+
 2020-05-22  Andres Gonzalez  <andresg_22@apple.com>
 
         Updates to the isolated tree must happen before posting notifications to clients.
index b0aaafa..3af0687 100644 (file)
@@ -260,7 +260,7 @@ void ScrollingTree::commitTreeState(std::unique_ptr<ScrollingStateTree>&& scroll
         m_latchingController.nodeWasRemoved(nodeID);
 
         if (auto node = m_nodeMap.take(nodeID))
-            node->wasRemovedFromTree();
+            node->willBeDestroyed();
     }
 
     LOG_WITH_STREAM(ScrollingTree, stream << "committed ScrollingTree" << scrollingTreeAsText(ScrollingStateTreeAsTextBehaviorDebug));
@@ -269,7 +269,7 @@ void ScrollingTree::commitTreeState(std::unique_ptr<ScrollingStateTree>&& scroll
 void ScrollingTree::updateTreeFromStateNodeRecursive(const ScrollingStateNode* stateNode, CommitTreeState& state)
 {
     if (!stateNode) {
-        m_nodeMap.clear();
+        removeAllNodes();
         m_rootNode = nullptr;
         return;
     }
@@ -289,7 +289,7 @@ void ScrollingTree::updateTreeFromStateNodeRecursive(const ScrollingStateNode* s
             // This is the root node. Clear the node map.
             ASSERT(stateNode->isFrameScrollingNode());
             m_rootNode = downcast<ScrollingTreeFrameScrollingNode>(node.get());
-            m_nodeMap.clear();
+            removeAllNodes();
         } 
         m_nodeMap.set(nodeID, node.get());
     }
@@ -338,6 +338,14 @@ void ScrollingTree::updateTreeFromStateNodeRecursive(const ScrollingStateNode* s
 #endif
 }
 
+void ScrollingTree::removeAllNodes()
+{
+    for (auto iter : m_nodeMap)
+        iter.value->willBeDestroyed();
+
+    m_nodeMap.clear();
+}
+
 void ScrollingTree::applyLayerPositionsAfterCommit()
 {
     // Scrolling tree needs to make adjustments only if the UI side positions have changed.
index bcba276..45b7174 100644 (file)
@@ -206,6 +206,8 @@ private:
     WEBCORE_EXPORT virtual RefPtr<ScrollingTreeNode> scrollingNodeForPoint(FloatPoint);
     WEBCORE_EXPORT virtual OptionSet<EventListenerRegionType> eventListenerRegionTypesForPoint(FloatPoint) const;
     virtual void receivedWheelEvent(const PlatformWheelEvent&) { }
+    
+    void removeAllNodes();
 
     RefPtr<ScrollingTreeFrameScrollingNode> m_rootNode;
 
index 4dfa945..c67d50a 100644 (file)
@@ -62,7 +62,7 @@ public:
     virtual void commitStateAfterChildren(const ScrollingStateNode&) { }
     virtual void didCompleteCommitForNode() { }
     
-    virtual void wasRemovedFromTree() { }
+    virtual void willBeDestroyed() { }
 
     ScrollingTreeNode* parent() const { return m_parent; }
     void setParent(ScrollingTreeNode* parent) { m_parent = parent; }
index 1168e00..182aa62 100644 (file)
@@ -63,7 +63,7 @@ protected:
     unsigned exposedUnfilledArea() const;
 
 private:
-    void wasRemovedFromTree() final;
+    void willBeDestroyed() final;
 
     FloatPoint adjustedScrollPosition(const FloatPoint&, ScrollClamping) const override;
 
index 7105f75..ce2b246 100644 (file)
@@ -57,7 +57,7 @@ ScrollingTreeFrameScrollingNodeMac::ScrollingTreeFrameScrollingNodeMac(Scrolling
 
 ScrollingTreeFrameScrollingNodeMac::~ScrollingTreeFrameScrollingNodeMac() = default;
 
-void ScrollingTreeFrameScrollingNodeMac::wasRemovedFromTree()
+void ScrollingTreeFrameScrollingNodeMac::willBeDestroyed()
 {
     m_delegate.nodeWillBeDestroyed();
 }
index 8a763a8..071a095 100644 (file)
@@ -53,7 +53,7 @@ protected:
     ScrollingEventResult handleWheelEvent(const PlatformWheelEvent&) override;
 
 private:
-    void wasRemovedFromTree() final;
+    void willBeDestroyed() final;
 
     ScrollingTreeScrollingNodeDelegateMac m_delegate;
 };
index f9bf2f5..a5405a5 100644 (file)
@@ -49,7 +49,7 @@ ScrollingTreeOverflowScrollingNodeMac::ScrollingTreeOverflowScrollingNodeMac(Scr
 
 ScrollingTreeOverflowScrollingNodeMac::~ScrollingTreeOverflowScrollingNodeMac() = default;
 
-void ScrollingTreeOverflowScrollingNodeMac::wasRemovedFromTree()
+void ScrollingTreeOverflowScrollingNodeMac::willBeDestroyed()
 {
     m_delegate.nodeWillBeDestroyed();
 }