Changeset 246913 in webkit


Ignore:
Timestamp:
Jun 27, 2019 6:09:45 PM (5 years ago)
Author:
Simon Fraser
Message:

Fix crash in ScrollingStateNode::insertChild()
https://bugs.webkit.org/show_bug.cgi?id=199297
rdar://problem/49415136

Reviewed by Tim Horton.

Crash data suggest that 'parent' can be deleted in ScrollingStateTree::insertNode(). To avoid this,
have ScrollingStateTree::m_stateNodeMap store RefPts, and do the same for ScrollingTree::m_nodeMap.

  • page/scrolling/ScrollingStateNode.cpp:

(WebCore::ScrollingStateNode::ScrollingStateNode): The relaxAdoptionRequirement() is required
to avoid ASSERT(!m_adoptionIsRequired) when the node is added to the tree in its constructor.

  • page/scrolling/ScrollingStateTree.cpp:

(WebCore::ScrollingStateTree::unparentNode):
(WebCore::ScrollingStateTree::unparentChildrenAndDestroyNode):
(WebCore::ScrollingStateTree::detachAndDestroySubtree):
(WebCore::ScrollingStateTree::stateNodeForID const):

  • page/scrolling/ScrollingStateTree.h:
  • page/scrolling/ScrollingTree.cpp:

(WebCore::ScrollingTree::updateTreeFromStateNode):

  • page/scrolling/ScrollingTree.h:
Location:
trunk/Source/WebCore
Files:
6 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r246899 r246913  
     12019-06-27  Simon Fraser  <simon.fraser@apple.com>
     2
     3        Fix crash in ScrollingStateNode::insertChild()
     4        https://bugs.webkit.org/show_bug.cgi?id=199297
     5        rdar://problem/49415136
     6
     7        Reviewed by Tim Horton.
     8
     9        Crash data suggest that 'parent' can be deleted in ScrollingStateTree::insertNode(). To avoid this,
     10        have ScrollingStateTree::m_stateNodeMap store RefPts, and do the same for ScrollingTree::m_nodeMap.
     11
     12        * page/scrolling/ScrollingStateNode.cpp:
     13        (WebCore::ScrollingStateNode::ScrollingStateNode): The relaxAdoptionRequirement() is required
     14        to avoid ASSERT(!m_adoptionIsRequired) when the node is added to the tree in its constructor.
     15        * page/scrolling/ScrollingStateTree.cpp:
     16        (WebCore::ScrollingStateTree::unparentNode):
     17        (WebCore::ScrollingStateTree::unparentChildrenAndDestroyNode):
     18        (WebCore::ScrollingStateTree::detachAndDestroySubtree):
     19        (WebCore::ScrollingStateTree::stateNodeForID const):
     20        * page/scrolling/ScrollingStateTree.h:
     21        * page/scrolling/ScrollingTree.cpp:
     22        (WebCore::ScrollingTree::updateTreeFromStateNode):
     23        * page/scrolling/ScrollingTree.h:
     24
    1252019-06-27  Simon Fraser  <simon.fraser@apple.com>
    226
  • trunk/Source/WebCore/page/scrolling/ScrollingStateNode.cpp

    r246593 r246913  
    5454    if (hasChangedProperty(Layer))
    5555        setLayer(stateNode.layer().toRepresentation(adoptiveTree.preferredLayerRepresentation()));
     56
     57    relaxAdoptionRequirement();
    5658    scrollingStateTree().addNode(*this);
    5759}
  • trunk/Source/WebCore/page/scrolling/ScrollingStateTree.cpp

    r246723 r246913  
    210210
    211211    // The node may not be found if clear() was recently called.
    212     RefPtr<ScrollingStateNode> protectedNode = m_stateNodeMap.get(nodeID);
     212    auto protectedNode = m_stateNodeMap.get(nodeID);
    213213    if (!protectedNode)
    214214        return;
     
    229229
    230230    // The node may not be found if clear() was recently called.
    231     RefPtr<ScrollingStateNode> protectedNode = m_stateNodeMap.take(nodeID);
     231    auto protectedNode = m_stateNodeMap.take(nodeID);
    232232    if (!protectedNode)
    233233        return;
     
    257257
    258258    // The node may not be found if clear() was recently called.
    259     auto* node = m_stateNodeMap.take(nodeID);
     259    auto node = m_stateNodeMap.take(nodeID);
    260260    if (!node)
    261261        return;
     
    263263    // If the node was unparented, remove it from m_unparentedNodes (keeping it alive until this function returns).
    264264    auto unparentedNode = m_unparentedNodes.take(nodeID);
    265     removeNodeAndAllDescendants(node);
     265    removeNodeAndAllDescendants(node.get());
    266266}
    267267
     
    366366
    367367    ASSERT(it->value->scrollingNodeID() == scrollLayerID);
    368     return it->value;
     368    return it->value.get();
    369369}
    370370
  • trunk/Source/WebCore/page/scrolling/ScrollingStateTree.h

    r246033 r246913  
    7070    unsigned nodeCount() const { return m_stateNodeMap.size(); }
    7171
    72     typedef HashMap<ScrollingNodeID, ScrollingStateNode*> StateNodeMap;
     72    typedef HashMap<ScrollingNodeID, RefPtr<ScrollingStateNode>> StateNodeMap;
    7373    const StateNodeMap& nodeMap() const { return m_stateNodeMap; }
    7474
  • trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp

    r246725 r246913  
    219219        ASSERT_WITH_SECURITY_IMPLICATION(parentIt != m_nodeMap.end());
    220220        if (parentIt != m_nodeMap.end()) {
    221             auto* parent = parentIt->value;
     221            auto* parent = parentIt->value.get();
    222222
    223223            auto* oldParent = node->parent();
  • trunk/Source/WebCore/page/scrolling/ScrollingTree.h

    r246725 r246913  
    176176    RefPtr<ScrollingTreeFrameScrollingNode> m_rootNode;
    177177
    178     using ScrollingTreeNodeMap = HashMap<ScrollingNodeID, ScrollingTreeNode*>;
     178    using ScrollingTreeNodeMap = HashMap<ScrollingNodeID, RefPtr<ScrollingTreeNode>>;
    179179    ScrollingTreeNodeMap m_nodeMap;
    180180
Note: See TracChangeset for help on using the changeset viewer.