Title: [169913] trunk/Source/WebCore
Revision
169913
Author
[email protected]
Date
2014-06-12 15:24:13 -0700 (Thu, 12 Jun 2014)

Log 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):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (169912 => 169913)


--- trunk/Source/WebCore/ChangeLog	2014-06-12 21:21:45 UTC (rev 169912)
+++ trunk/Source/WebCore/ChangeLog	2014-06-12 22:24:13 UTC (rev 169913)
@@ -1,3 +1,26 @@
+2014-06-12  Simon Fraser  <[email protected]>
+
+        [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  <[email protected]>
 
         Add a space after the comma.

Modified: trunk/Source/WebCore/loader/HistoryController.cpp (169912 => 169913)


--- trunk/Source/WebCore/loader/HistoryController.cpp	2014-06-12 21:21:45 UTC (rev 169912)
+++ trunk/Source/WebCore/loader/HistoryController.cpp	2014-06-12 22:24:13 UTC (rev 169913)
@@ -124,33 +124,35 @@
     // 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());
-        }
+    // 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()

Modified: trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp (169912 => 169913)


--- trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp	2014-06-12 21:21:45 UTC (rev 169912)
+++ trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp	2014-06-12 22:24:13 UTC (rev 169913)
@@ -197,6 +197,9 @@
 
 FrameView* AsyncScrollingCoordinator::frameViewForScrollingNode(ScrollingNodeID scrollingNodeID) const
 {
+    if (!m_scrollingStateTree->rootStateNode())
+        return nullptr;
+    
     if (scrollingNodeID == m_scrollingStateTree->rootStateNode()->scrollingNodeID())
         return m_page->mainFrame().view();
 
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to