Changeset 169913 in webkit

Timestamp:

Jun 12, 2014 3:24:13 PM (10 years ago)

Author:

Simon Fraser

Message:

[iOS WK2] Fix crash on back/foward swipe
​https://bugs.webkit.org/show_bug.cgi?id=133826
<rdar://problem/17032752>

Reviewed by Tim Horton.

AsyncScrollingCoordinator::frameViewForScrollingNode() would crash with a null root
state node, because HistoryController::restoreScrollPositionAndViewState() tried
to restore scroll position (via restoreViewState()) before hooking up the scrolling
coordinator.

Fix by doing the scrollingCoordinator->frameViewRootLayerDidChange() before
calling restoreViewState().

Also add a defensive null-check on the root state node in updateScrollPositionAfterAsyncScrollTimerFired().

• loader/HistoryController.cpp:

(WebCore::HistoryController::restoreScrollPositionAndViewState):

• page/scrolling/AsyncScrollingCoordinator.cpp:

(WebCore::AsyncScrollingCoordinator::frameViewForScrollingNode):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• loader/HistoryController.cpp (modified) (3 diffs)
• page/scrolling/AsyncScrollingCoordinator.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2014-06-12  Simon Fraser  <simon.fraser@apple.com>
+
+        [iOS WK2] Fix crash on back/foward swipe
+        https://bugs.webkit.org/show_bug.cgi?id=133826
+        <rdar://problem/17032752>
+
+        Reviewed by Tim Horton.
+
+        AsyncScrollingCoordinator::frameViewForScrollingNode() would crash with a null root
+        state node, because HistoryController::restoreScrollPositionAndViewState() tried
+        to restore scroll position (via restoreViewState()) before hooking up the scrolling
+        coordinator.
+        
+        Fix by doing the scrollingCoordinator->frameViewRootLayerDidChange() before
+        calling restoreViewState().
+        
+        Also add a defensive null-check on the root state node in updateScrollPositionAfterAsyncScrollTimerFired().
+
+        * loader/HistoryController.cpp:
+        (WebCore::HistoryController::restoreScrollPositionAndViewState):
+        * page/scrolling/AsyncScrollingCoordinator.cpp:
+        (WebCore::AsyncScrollingCoordinator::frameViewForScrollingNode):
+
 2014-06-12  Anders Carlsson  <andersca@apple.com>
 

trunk/Source/WebCore/loader/HistoryController.cpp

@@ -125 +125 @@
     if (!m_currentItem)
         return;
-    
-    // FIXME: It would be great to work out a way to put this code in WebCore instead of calling
-    // through to the client. It's currently used only for the PDF view on Mac.
-    m_frame.loader().client().restoreViewState();
+
+    FrameView* view = m_frame.view();
 
     // FIXME: There is some scrolling related work that needs to happen whenever a page goes into the
@@ -135 +133 @@
     // Document::setIsInPageCache(bool). It would be nice if there was more symmetry in these spots.
     // https://bugs.webkit.org/show_bug.cgi?id=98698
-    if (FrameView* view = m_frame.view()) {
+    if (view) {
         Page* page = m_frame.page();
         if (page && m_frame.isMainFrame()) {
@@ -141 +139 @@
                 scrollingCoordinator->frameViewRootLayerDidChange(view);
         }
+    }
+
+    // FIXME: It would be great to work out a way to put this code in WebCore instead of calling
+    // through to the client.
+    m_frame.loader().client().restoreViewState();
 
 #if !PLATFORM(IOS)
-        // Don't restore scroll point on iOS as FrameLoaderClient::restoreViewState() does that.
-        if (!view->wasScrolledByUser()) {
-            if (page && m_frame.isMainFrame() && m_currentItem->pageScaleFactor())
-                page->setPageScaleFactor(m_currentItem->pageScaleFactor(), m_currentItem->scrollPoint());
-            else
-                view->setScrollPosition(m_currentItem->scrollPoint());
-        }
+    // Don't restore scroll point on iOS as FrameLoaderClient::restoreViewState() does that.
+    if (view && !view->wasScrolledByUser()) {
+        if (page && m_frame.isMainFrame() && m_currentItem->pageScaleFactor())
+            page->setPageScaleFactor(m_currentItem->pageScaleFactor(), m_currentItem->scrollPoint());
+        else
+            view->setScrollPosition(m_currentItem->scrollPoint());
+    }
 #endif
-    }
 }
 

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

@@ -198 +198 @@
 FrameView* AsyncScrollingCoordinator::frameViewForScrollingNode(ScrollingNodeID scrollingNodeID) const
 {
+    if (!m_scrollingStateTree->rootStateNode())
+        return nullptr;
+    
     if (scrollingNodeID == m_scrollingStateTree->rootStateNode()->scrollingNodeID())
         return m_page->mainFrame().view();