Changeset 143074 in webkit

Timestamp:

Feb 15, 2013 5:20:38 PM (11 years ago)

Author:

Simon Fraser

Message:

REGRESSION (r142505?): Crashes in WebCore::ScrollingStateNode::appendChild when using back/forward buttons
​https://bugs.webkit.org/show_bug.cgi?id=109826
<rdar://problem/13216100>

Source/WebCore:

Reviewed by Beth Dakin.

Fix a crash when going Back on some pages with fixed position elements.

When a page was being restored from the page cache, and a layout from
FrameLoader::commitProvisionalLoad() caused us to try to register the fixed
position layer before the main scrolling layer, we'd crash trying to dereference
the root node.

Fix by bailing from ScrollingStateTree::attachNode() if we can't find the parent
node.

Test: platform/mac-wk2/tiled-drawing/null-parent-back-crash.html

• page/scrolling/ScrollingStateTree.cpp:

(WebCore::ScrollingStateTree::attachNode):
(WebCore::ScrollingStateTree::stateNodeForID):

• page/scrolling/mac/ScrollingCoordinatorMac.mm:

(WebCore::ScrollingCoordinatorMac::updateViewportConstrainedNode):

LayoutTests:

Reviewed by Beth Dakin.

Test having a page with an iframe that navigates forwards then back.

• platform/mac-wk2/tiled-drawing/null-parent-back-crash.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/platform/mac-wk2/tiled-drawing/null-parent-back-crash.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/page/scrolling/ScrollingStateTree.cpp (modified) (3 diffs)
• Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2013-02-15  Simon Fraser  <simon.fraser@apple.com>
+
+        REGRESSION (r142505?): Crashes in WebCore::ScrollingStateNode::appendChild when using back/forward buttons
+        https://bugs.webkit.org/show_bug.cgi?id=109826
+        <rdar://problem/13216100>
+
+        Reviewed by Beth Dakin.
+        
+        Test having a page with an iframe that navigates forwards then back.
+
+        * platform/mac-wk2/tiled-drawing/null-parent-back-crash.html: Added.
+
 2013-02-15  Simon Fraser  <simon.fraser@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-02-15  Simon Fraser  <simon.fraser@apple.com>
+
+        REGRESSION (r142505?): Crashes in WebCore::ScrollingStateNode::appendChild when using back/forward buttons
+        https://bugs.webkit.org/show_bug.cgi?id=109826
+        <rdar://problem/13216100>
+
+        Reviewed by Beth Dakin.
+
+        Fix a crash when going Back on some pages with fixed position elements.
+        
+        When a page was being restored from the page cache, and a layout from
+        FrameLoader::commitProvisionalLoad() caused us to try to register the fixed
+        position layer before the main scrolling layer, we'd crash trying to dereference
+        the root node.
+        
+        Fix by bailing from ScrollingStateTree::attachNode() if we can't find the parent
+        node.
+
+        Test: platform/mac-wk2/tiled-drawing/null-parent-back-crash.html
+
+        * page/scrolling/ScrollingStateTree.cpp:
+        (WebCore::ScrollingStateTree::attachNode):
+        (WebCore::ScrollingStateTree::stateNodeForID):
+        * page/scrolling/mac/ScrollingCoordinatorMac.mm:
+        (WebCore::ScrollingCoordinatorMac::updateViewportConstrainedNode):
+
 2013-02-15  Simon Fraser  <simon.fraser@apple.com>
 

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

@@ -74 +74 @@
     } else {
         ScrollingStateNode* parent = stateNodeForID(parentID);
+        if (!parent)
+            return 0;
+
         switch (nodeType) {
         case FixedNode: {
@@ -95 +98 @@
             break;
         }
-        default:
-            ASSERT_NOT_REACHED();
         }
     }
@@ -176 +177 @@
         return 0;
 
+    ASSERT(it->value->scrollingNodeID() == scrollLayerID);
     return it->value;
 }

trunk/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm

@@ -349 +349 @@
     ASSERT(supportsFixedPositionLayers());
 
+    ScrollingStateNode* node = m_scrollingStateTree->stateNodeForID(nodeID);
+    if (!node)
+        return;
+
     switch (constraints.constraintType()) {
     case ViewportConstraints::FixedPositionConstaint: {
-        ScrollingStateFixedNode* node = toScrollingStateFixedNode(m_scrollingStateTree->stateNodeForID(nodeID));
-        setScrollLayerForNode(graphicsLayer, node);
-        node->updateConstraints((const FixedPositionViewportConstraints&)constraints);
+        ScrollingStateFixedNode* fixedNode = toScrollingStateFixedNode(node);
+        setScrollLayerForNode(graphicsLayer, fixedNode);
+        fixedNode->updateConstraints((const FixedPositionViewportConstraints&)constraints);
         break;
     }
     case ViewportConstraints::StickyPositionConstraint: {
-        ScrollingStateStickyNode* node = toScrollingStateStickyNode(m_scrollingStateTree->stateNodeForID(nodeID));
-        setScrollLayerForNode(graphicsLayer, node);
-        node->updateConstraints((const StickyPositionViewportConstraints&)constraints);
+        ScrollingStateStickyNode* stickyNode = toScrollingStateStickyNode(node);
+        setScrollLayerForNode(graphicsLayer, stickyNode);
+        stickyNode->updateConstraints((const StickyPositionViewportConstraints&)constraints);
         break;
     }