[iOS][WK2] Restore the visual scroll position instead of the dom scroll position...
authorbenjamin@webkit.org <benjamin@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Jun 2014 23:41:15 +0000 (23:41 +0000)
committerbenjamin@webkit.org <benjamin@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Jun 2014 23:41:15 +0000 (23:41 +0000)
https://bugs.webkit.org/show_bug.cgi?id=133490

Patch by Benjamin Poulain <bpoulain@apple.com> on 2014-06-04
Reviewed by Tim Horton.

Source/WebCore:
Expose the WebKit1 parameter "ScaleIsInitial". It is used for a similar concept in WebKit2 (userHasChangedPageScaleFactor).

Add "exposedContentPosition", which is the scroll position of the exposed rect.

* history/HistoryItem.h:
(WebCore::HistoryItem::exposedContentPosition):
(WebCore::HistoryItem::setExposedContentPosition):
(WebCore::HistoryItem::setScaleIsInitial):
* loader/HistoryController.cpp:
(WebCore::HistoryController::saveScrollPositionAndViewStateToItem):

Source/WebKit2:
Instead of restoring the scroll position, restore the visual position. This makes pages appear at the same position
on screen ignoring changes in the obscured top inset.

* WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
* WebProcess/WebCoreSupport/ios/WebFrameLoaderClientIOS.mm:
(WebKit::WebFrameLoaderClient::saveViewStateToItem):
(WebKit::WebFrameLoaderClient::restoreViewState):
Save and restore userHasChangedPageScaleFactor to handle rescaling correctly.
Limit the scale into valid viewport limits in case the viewport meta tag has changed or the device has rotated.

* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::WebPage):
* WebProcess/WebPage/WebPage.h:
(WebKit::WebPage::userHasChangedPageScaleFactor):
* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::restorePageState):
(WebKit::WebPage::updateVisibleContentRects):
In updateVisibleContentRects, we keep track or the current difference between the exposed rect and the unobscured rect.
When restoring the page position, we use the current top inset and the saved exposed rect to restore the visual
scroll position.

It is not very robust as it does not resolve races between the two processes, but that is not worse than what we is there now.

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

Source/WebCore/ChangeLog
Source/WebCore/history/HistoryItem.h
Source/WebCore/loader/HistoryController.cpp
Source/WebKit2/ChangeLog
Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp
Source/WebKit2/WebProcess/WebCoreSupport/ios/WebFrameLoaderClientIOS.mm
Source/WebKit2/WebProcess/WebPage/WebPage.cpp
Source/WebKit2/WebProcess/WebPage/WebPage.h
Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm

index aefbf1f..778ceae 100644 (file)
@@ -1,3 +1,21 @@
+2014-06-04  Benjamin Poulain  <bpoulain@apple.com>
+
+        [iOS][WK2] Restore the visual scroll position instead of the dom scroll position when restoring states from the history
+        https://bugs.webkit.org/show_bug.cgi?id=133490
+
+        Reviewed by Tim Horton.
+
+        Expose the WebKit1 parameter "ScaleIsInitial". It is used for a similar concept in WebKit2 (userHasChangedPageScaleFactor).
+
+        Add "exposedContentPosition", which is the scroll position of the exposed rect.
+
+        * history/HistoryItem.h:
+        (WebCore::HistoryItem::exposedContentPosition):
+        (WebCore::HistoryItem::setExposedContentPosition):
+        (WebCore::HistoryItem::setScaleIsInitial):
+        * loader/HistoryController.cpp:
+        (WebCore::HistoryController::saveScrollPositionAndViewStateToItem):
+
 2014-06-04  Alex Christensen  <achristensen@webkit.org>
 
         Enable WebGL on Windows.
index f65bd7c..95b0c0e 100644 (file)
@@ -180,8 +180,12 @@ public:
 #endif
 
 #if PLATFORM(IOS)
+    IntPoint exposedContentPosition() const { return m_exposedContentPosition; }
+    void setExposedContentPosition(IntPoint exposedContentPosition) { m_exposedContentPosition = exposedContentPosition; }
+
     float scale() const { return m_scale; }
     bool scaleIsInitial() const { return m_scaleIsInitial; }
+    void setScaleIsInitial(bool scaleIsInitial) { m_scaleIsInitial = scaleIsInitial; }
     void setScale(float newScale, bool isInitial)
     {
         m_scale = newScale;
@@ -255,6 +259,7 @@ private:
     std::unique_ptr<CachedPage> m_cachedPage;
 
 #if PLATFORM(IOS)
+    IntPoint m_exposedContentPosition;
     float m_scale;
     bool m_scaleIsInitial;
     ViewportArguments m_viewportArguments;
index 8462c5f..e82e7d1 100644 (file)
@@ -78,6 +78,9 @@ void HistoryController::saveScrollPositionAndViewStateToItem(HistoryItem* item)
         item->setScrollPoint(m_frame.view()->cachedScrollPosition());
     else
         item->setScrollPoint(m_frame.view()->scrollPosition());
+#if PLATFORM(IOS)
+    item->setExposedContentPosition(m_frame.view()->exposedContentRect().location());
+#endif
 
     Page* page = m_frame.page();
     if (page && m_frame.isMainFrame())
index f46eb3a..7c36f43 100644 (file)
@@ -1,3 +1,33 @@
+2014-06-04  Benjamin Poulain  <bpoulain@apple.com>
+
+        [iOS][WK2] Restore the visual scroll position instead of the dom scroll position when restoring states from the history
+        https://bugs.webkit.org/show_bug.cgi?id=133490
+
+        Reviewed by Tim Horton.
+
+        Instead of restoring the scroll position, restore the visual position. This makes pages appear at the same position
+        on screen ignoring changes in the obscured top inset.
+
+        * WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
+        * WebProcess/WebCoreSupport/ios/WebFrameLoaderClientIOS.mm:
+        (WebKit::WebFrameLoaderClient::saveViewStateToItem):
+        (WebKit::WebFrameLoaderClient::restoreViewState):
+        Save and restore userHasChangedPageScaleFactor to handle rescaling correctly.
+        Limit the scale into valid viewport limits in case the viewport meta tag has changed or the device has rotated.
+
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::WebPage):
+        * WebProcess/WebPage/WebPage.h:
+        (WebKit::WebPage::userHasChangedPageScaleFactor):
+        * WebProcess/WebPage/ios/WebPageIOS.mm:
+        (WebKit::WebPage::restorePageState):
+        (WebKit::WebPage::updateVisibleContentRects):
+        In updateVisibleContentRects, we keep track or the current difference between the exposed rect and the unobscured rect.
+        When restoring the page position, we use the current top inset and the saved exposed rect to restore the visual
+        scroll position.
+
+        It is not very robust as it does not resolve races between the two processes, but that is not worse than what we is there now.
+
 2014-06-03  Timothy Horton  <timothy_horton@apple.com>
 
         WebKit2 View Gestures (Zoom): Can show unpainted regions when zoom-pinching-out
index 3274c20..0c36f95 100644 (file)
@@ -1119,12 +1119,12 @@ void WebFrameLoaderClient::frameLoadCompleted()
     }
 }
 
+#if !PLATFORM(IOS)
 void WebFrameLoaderClient::saveViewStateToItem(HistoryItem*)
 {
     notImplemented();
 }
 
-#if !PLATFORM(IOS)
 void WebFrameLoaderClient::restoreViewState()
 {
     // Inform the UI process of the scale factor.
index 50c2701..ff3d915 100644 (file)
@@ -79,18 +79,21 @@ void WebFrameLoaderClient::didCreateQuickLookHandle(WebCore::QuickLookHandle& ha
 }
 #endif
 
+void WebFrameLoaderClient::saveViewStateToItem(HistoryItem* historyItem)
+{
+    if (m_frame->isMainFrame())
+        historyItem->setScaleIsInitial(m_frame->page()->userHasChangedPageScaleFactor());
+}
+
 void WebFrameLoaderClient::restoreViewState()
 {
     Frame& frame = *m_frame->coreFrame();
     HistoryItem* currentItem = frame.loader().history().currentItem();
     if (FrameView* view = frame.view()) {
-        Page* page = frame.page();
-        if (!view->wasScrolledByUser()) {
-            if (page && m_frame->isMainFrame() && currentItem->pageScaleFactor())
-                m_frame->page()->restorePageState(currentItem->pageScaleFactor(), currentItem->scrollPoint());
-            else
-                view->setScrollPosition(currentItem->scrollPoint());
-        }
+        if (m_frame->isMainFrame() && currentItem->pageScaleFactor())
+            m_frame->page()->restorePageState(currentItem->pageScaleFactor(), currentItem->scaleIsInitial(), currentItem->exposedContentPosition());
+        else if (!view->wasScrolledByUser())
+            view->setScrollPosition(currentItem->scrollPoint());
     }
 }
 
index b190315..bf43091 100644 (file)
@@ -293,6 +293,7 @@ WebPage::WebPage(uint64_t pageID, const WebPageCreationParameters& parameters)
 #endif
 #if PLATFORM(IOS)
     , m_lastVisibleContentRectUpdateID(0)
+    , m_obscuredTopInset(0)
     , m_hasReceivedVisibleContentRectsAfterDidCommitLoad(false)
     , m_scaleWasSetByUIProcess(false)
     , m_userHasChangedPageScaleFactor(false)
index 5b8f8c3..76fb361 100644 (file)
@@ -453,11 +453,12 @@ public:
     WebCore::FloatSize availableScreenSize() const;
     void viewportPropertiesDidChange(const WebCore::ViewportArguments&);
     void didReceiveMobileDocType(bool);
-    void restorePageState(double scale, const WebCore::IntPoint& origin);
+    void restorePageState(double scale, bool userHasChangedPageScaleFactor, const WebCore::IntPoint& exposedOrigin);
 
     double minimumPageScaleFactor() const;
     double maximumPageScaleFactor() const;
     bool allowsUserScaling() const;
+    bool userHasChangedPageScaleFactor() const { return m_userHasChangedPageScaleFactor; }
 
     void handleTap(const WebCore::IntPoint&);
     void potentialTapAtPosition(uint64_t requestID, const WebCore::FloatPoint&);
@@ -1206,6 +1207,7 @@ private:
 
     WebCore::ViewportConfiguration m_viewportConfiguration;
     uint64_t m_lastVisibleContentRectUpdateID;
+    float m_obscuredTopInset;
     bool m_hasReceivedVisibleContentRectsAfterDidCommitLoad;
     bool m_scaleWasSetByUIProcess;
     bool m_userHasChangedPageScaleFactor;
index bc96728..8e37571 100644 (file)
@@ -151,13 +151,17 @@ void WebPage::didReceiveMobileDocType(bool isMobileDoctype)
         resetViewportDefaultConfiguration(m_mainFrame.get());
 }
 
-void WebPage::restorePageState(double scale, const IntPoint& scrollPosition)
+void WebPage::restorePageState(double scale, bool userHasChangedPageScaleFactor, const IntPoint& exposedOrigin)
 {
-    scalePage(scale, scrollPosition);
-    m_page->mainFrame().view()->setScrollPosition(scrollPosition);
+    // FIXME: ideally, we should sync this with the UIProcess. We should not try to change the position if the user is interacting
+    // with the page, and we should send the new scroll position as soon as possible to the UIProcess.
 
-    // FIXME: we should get the value of userHasChangedPageScaleFactor from the history.
-    m_userHasChangedPageScaleFactor = true;
+    float boundedScale = std::min<float>(m_viewportConfiguration.maximumScale(), std::max<float>(m_viewportConfiguration.minimumScale(), scale));
+    float topInsetInPageCoordinates = m_obscuredTopInset / boundedScale;
+    IntPoint scrollPosition(exposedOrigin.x(), exposedOrigin.y() + topInsetInPageCoordinates);
+    scalePage(boundedScale, scrollPosition);
+    m_page->mainFrame().view()->setScrollPosition(scrollPosition);
+    m_userHasChangedPageScaleFactor = userHasChangedPageScaleFactor;
 }
 
 double WebPage::minimumPageScaleFactor() const
@@ -2273,6 +2277,7 @@ void WebPage::updateVisibleContentRects(const VisibleContentRectUpdateInfo& visi
 {
     m_hasReceivedVisibleContentRectsAfterDidCommitLoad = true;
     m_lastVisibleContentRectUpdateID = visibleContentRectUpdateInfo.updateID();
+    m_obscuredTopInset = (visibleContentRectUpdateInfo.unobscuredRect().y() - visibleContentRectUpdateInfo.exposedRect().y()) * visibleContentRectUpdateInfo.scale();
 
     double scaleNoiseThreshold = 0.005;
     double filteredScale = visibleContentRectUpdateInfo.scale();