When ScrollingStateNodes are destroyed, they should be removed ScrollingCoordinator's HashMap

[Beth Dakin]

There is a FIXME in ScrollingCoordinatorMac::detachFromStateTree() that says:

    // FIXME: removeNode() will destroy children, and those children might still be in the HashMap.
    // This will be a big problem once there are actually children in the tree.

This bugs is to track resolving that FIXME.

[Beth Dakin]

Created attachment 168489 [details]
Patch

[Brady Eidson]

Comment on attachment 168489 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=168489&action=review

> Source/WebCore/page/scrolling/ScrollingStateTree.cpp:58
> +    // Copy and clear the array of node IDs representing nodes that have been removed since the last
> +    // time we committed the tree.
> +    size_t size = m_nodesRemovedSinceLastCommit->size();
> +    for (size_t i = 0; i < size; ++i)
> +        treeState->didRemoveNode(m_nodesRemovedSinceLastCommit->at(i));
> +    m_nodesRemovedSinceLastCommit->clear();

I don't see where the copy is that the comment alludes to.

I might be missing something here:  doesn't didRemoveNode() just call back in to the same m_nodesRemovedSinceLastCommit?

[Beth Dakin]

Created attachment 168513 [details]
Patch

[Beth Dakin]

Thanks Sam and Brady! http://trac.webkit.org/changeset/131238