Changeset 172112 in webkit


Ignore:
Timestamp:
Aug 5, 2014, 5:54:04 PM (12 years ago)
Author:
Simon Fraser
Message:

[iOS WK2] Crash going back on a specific tumblr blog (under ScrollingStateTree::removeNodeAndAllDescendants)
​https://bugs.webkit.org/show_bug.cgi?id=135629
<rdar://problem/17802174>

Reviewed by Tim Horton.

Source/WebCore:

In r170198 I added an "orphan scrolling nodes" code path that sets aside subtrees
of scrolling nodes into an m_orphanedSubframeNodes map, which keeps them alive until
they get reparented or destroyed. The nodes in that subtree remain in m_stateNodeMap,
which holds raw pointers to them.

However, ScrollingStateTree::commit() can clear m_orphanedSubframeNodes, which is
sometimes non-empty at this point. When that happened, we would destroy nodes which
were still referenced by m_stateNodeMap, with the result that a later query for the
same nodeID would hand back a pointer to a deleted object.

Fix by calling recursiveNodeWillBeRemoved() on nodes in the m_orphanedSubframeNodes
before clearing it, which removes them and all their descendants from the state node map.

Test: platform/mac-wk2/tiled-drawing/scrolling/frames/orphaned-subtree.html

  • page/scrolling/ScrollingStateTree.cpp:

(WebCore::ScrollingStateTree::clear):
(WebCore::ScrollingStateTree::commit):

LayoutTests:

Testcase with nesting of frames inside fixed inside frames, where a subframe disconnects
part of the scrolling tree.

  • platform/mac-wk2/tiled-drawing/scrolling/frames/orphaned-subtree-expected.txt: Added.
  • platform/mac-wk2/tiled-drawing/scrolling/frames/orphaned-subtree.html: Added.
  • platform/mac-wk2/tiled-drawing/scrolling/frames/resources/leaf-frame.html: Added.
  • platform/mac-wk2/tiled-drawing/scrolling/frames/resources/subframe-inside-fixed.html: Added.
Location:
trunk
Files:
4 added
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r172099 r172112  
     12014-08-05  Simon Fraser  <simon.fraser@apple.com>
     2
     3        [iOS WK2] Crash going back on a specific tumblr blog (under ScrollingStateTree::removeNodeAndAllDescendants)
     4        https://bugs.webkit.org/show_bug.cgi?id=135629
     5        <rdar://problem/17802174>
     6
     7        Reviewed by Tim Horton.
     8       
     9        Testcase with nesting of frames inside fixed inside frames, where a subframe disconnects
     10        part of the scrolling tree.
     11
     12        * platform/mac-wk2/tiled-drawing/scrolling/frames/orphaned-subtree-expected.txt: Added.
     13        * platform/mac-wk2/tiled-drawing/scrolling/frames/orphaned-subtree.html: Added.
     14        * platform/mac-wk2/tiled-drawing/scrolling/frames/resources/leaf-frame.html: Added.
     15        * platform/mac-wk2/tiled-drawing/scrolling/frames/resources/subframe-inside-fixed.html: Added.
     16
    1172014-08-05  Andreas Kling  <akling@apple.com>
    218

  • trunk/Source/WebCore/ChangeLog

    r172110 r172112  
     12014-08-05  Simon Fraser  <simon.fraser@apple.com>
     2
     3        [iOS WK2] Crash going back on a specific tumblr blog (under ScrollingStateTree::removeNodeAndAllDescendants)
     4        https://bugs.webkit.org/show_bug.cgi?id=135629
     5        <rdar://problem/17802174>
     6
     7        Reviewed by Tim Horton.
     8       
     9        In r170198 I added an "orphan scrolling nodes" code path that sets aside subtrees
     10        of scrolling nodes into an m_orphanedSubframeNodes map, which keeps them alive until
     11        they get reparented or destroyed. The nodes in that subtree remain in m_stateNodeMap,
     12        which holds raw pointers to them.
     13       
     14        However, ScrollingStateTree::commit() can clear m_orphanedSubframeNodes, which is
     15        sometimes non-empty at this point. When that happened, we would destroy nodes which
     16        were still referenced by m_stateNodeMap, with the result that a later query for the
     17        same nodeID would hand back a pointer to a deleted object.
     18       
     19        Fix by calling recursiveNodeWillBeRemoved() on nodes in the m_orphanedSubframeNodes
     20        before clearing it, which removes them and all their descendants from the state node map.
     21
     22        Test: platform/mac-wk2/tiled-drawing/scrolling/frames/orphaned-subtree.html
     23
     24        * page/scrolling/ScrollingStateTree.cpp:
     25        (WebCore::ScrollingStateTree::clear):
     26        (WebCore::ScrollingStateTree::commit):
     27
    1282014-08-05  Peyton Randolph  <prandolph@apple.com>
    229

  • trunk/Source/WebCore/page/scrolling/ScrollingStateTree.cpp

    r171311 r172112  
    153153        removeNodeAndAllDescendants(rootStateNode());
    154154
    155     ASSERT(m_stateNodeMap.isEmpty());
    156155    m_stateNodeMap.clear();
    157156    m_orphanedSubframeNodes.clear();
    … …  
    160159PassOwnPtr<ScrollingStateTree> ScrollingStateTree::commit(LayerRepresentation::Type preferredLayerRepresentation)
    161160{
    162     m_orphanedSubframeNodes.clear();
     161    if (!m_orphanedSubframeNodes.isEmpty()) {
     162        // If we still have orphaned subtrees, remove them from m_stateNodeMap since they will be deleted
     163        // when clearing m_orphanedSubframeNodes.
     164        for (auto& orphanNode : m_orphanedSubframeNodes.values())
     165            recursiveNodeWillBeRemoved(orphanNode.get(), SubframeNodeRemoval::Delete);
     166        m_orphanedSubframeNodes.clear();
     167    }
    163168
    164169    // This function clones and resets the current state tree, but leaves the tree structure intact.
Note: See TracChangeset for help on using the changeset viewer.