[iOS WK2] Fix crash on back/foward swipe
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 12 Jun 2014 22:24:13 +0000 (22:24 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 12 Jun 2014 22:24:13 +0000 (22:24 +0000)
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):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@169913 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/loader/HistoryController.cpp
Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp

index 31fe9a4..be6f53a 100644 (file)
@@ -1,3 +1,26 @@
+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>
 
         Add a space after the comma.
index e82e7d1..eb380d4 100644 (file)
@@ -124,33 +124,35 @@ void HistoryController::restoreScrollPositionAndViewState()
     // so there *is* no scroll or view state to restore!
     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
     // page cache and similar work that needs to occur when it comes out. This is where we do the work
     // that needs to happen when we exit, and the work that needs to happen when we enter is in
     // 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()) {
             if (ScrollingCoordinator* scrollingCoordinator = page->scrollingCoordinator())
                 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());
-        }
-#endif
+    // 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
 }
 
 void HistoryController::updateBackForwardListForFragmentScroll()
index 31f0db9..5ac104f 100644 (file)
@@ -197,6 +197,9 @@ void AsyncScrollingCoordinator::updateScrollPositionAfterAsyncScrollTimerFired(T
 
 FrameView* AsyncScrollingCoordinator::frameViewForScrollingNode(ScrollingNodeID scrollingNodeID) const
 {
+    if (!m_scrollingStateTree->rootStateNode())
+        return nullptr;
+    
     if (scrollingNodeID == m_scrollingStateTree->rootStateNode()->scrollingNodeID())
         return m_page->mainFrame().view();