WebViewImpl::resetScrollAndScaleState() causes the page to render incorrectly
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 15 Dec 2012 01:21:25 +0000 (01:21 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 15 Dec 2012 01:21:25 +0000 (01:21 +0000)
https://bugs.webkit.org/show_bug.cgi?id=104767

Patch by Dan Alcantara <dfalcantara@chromium.org> on 2012-12-14
Reviewed by Darin Fisher.

Change WebViewImpl::resetScrollAndScaleState() so that it directly
alters values in the HistoryItem instead of indirectly changing the
values.

Adds a method in HistoryController to clear the scroll and scale state of
its current HistoryItem.

* public/WebView.h:
(WebView):
* src/WebViewImpl.cpp:
(WebKit::WebViewImpl::resetScrollAndScaleState):
* tests/WebViewTest.cpp:

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

Source/WebCore/loader/HistoryController.cpp
Source/WebCore/loader/HistoryController.h
Source/WebKit/chromium/ChangeLog
Source/WebKit/chromium/public/WebView.h
Source/WebKit/chromium/src/WebViewImpl.cpp
Source/WebKit/chromium/tests/WebViewTest.cpp

index fb6d0e3..a2f72d0 100644 (file)
@@ -93,6 +93,15 @@ void HistoryController::saveScrollPositionAndViewStateToItem(HistoryItem* item)
     m_frame->loader()->client()->saveViewStateToItem(item);
 }
 
+void HistoryController::clearScrollPositionAndViewState()
+{
+    if (!m_currentItem)
+        return;
+
+    m_currentItem->clearScrollPoint();
+    m_currentItem->setPageScaleFactor(0);
+}
+
 /*
  There is a race condition between the layout and load completion that affects restoring the scroll position.
  We try to restore the scroll position at both the first layout and upon load completion.
index 2dc0b27..a396812 100644 (file)
@@ -52,6 +52,7 @@ public:
     ~HistoryController();
 
     void saveScrollPositionAndViewStateToItem(HistoryItem*);
+    void clearScrollPositionAndViewState();
     void restoreScrollPositionAndViewState();
 
     void updateBackForwardListForFragmentScroll();
index 95e785f..d22a699 100644 (file)
@@ -1,3 +1,23 @@
+2012-12-14  Dan Alcantara  <dfalcantara@chromium.org>
+
+        WebViewImpl::resetScrollAndScaleState() causes the page to render incorrectly
+        https://bugs.webkit.org/show_bug.cgi?id=104767
+
+        Reviewed by Darin Fisher.
+
+        Change WebViewImpl::resetScrollAndScaleState() so that it directly
+        alters values in the HistoryItem instead of indirectly changing the
+        values.
+
+        Adds a method in HistoryController to clear the scroll and scale state of
+        its current HistoryItem.
+
+        * public/WebView.h:
+        (WebView):
+        * src/WebViewImpl.cpp:
+        (WebKit::WebViewImpl::resetScrollAndScaleState):
+        * tests/WebViewTest.cpp:
+
 2012-12-14  Sheriff Bot  <webkit.review.bot@gmail.com>
 
         Unreviewed, rolling out r137765.
index 7a7d3f4..bf9e44a 100644 (file)
@@ -262,8 +262,7 @@ public:
     // state, this function deletes any saved scroll and scale state.
     virtual void restoreScrollAndScaleState() = 0;
 
-    // Reset the scroll and scale state and clobber any previously saved values for
-    // these parameters.
+    // Reset any saved values for the scroll and scale state.
     virtual void resetScrollAndScaleState() = 0;
 
     // Prevent the web page from setting a maximum scale via the viewport meta
index 41ee50a..f177e8c 100644 (file)
@@ -3095,14 +3095,18 @@ void WebViewImpl::resetSavedScrollAndScaleState()
 
 void WebViewImpl::resetScrollAndScaleState()
 {
-    page()->setPageScaleFactor(0, IntPoint());
+    page()->setPageScaleFactor(1, IntPoint());
+
+    // Clear out the values for the current history item. This will prevent the history item from clobbering the
+    // value determined during page scale initialization, which may be less than 1.
+    page()->mainFrame()->loader()->history()->saveDocumentAndScrollState();
+    page()->mainFrame()->loader()->history()->clearScrollPositionAndViewState();
     m_pageScaleFactorIsSet = false;
 
     // Clobber saved scales and scroll offsets.
     if (FrameView* view = page()->mainFrame()->document()->view())
         view->cacheCurrentScrollPosition();
     resetSavedScrollAndScaleState();
-    page()->mainFrame()->loader()->history()->saveDocumentAndScrollState();
 }
 
 WebSize WebViewImpl::fixedLayoutSize() const
index 1929c1b..f202df0 100644 (file)
@@ -482,14 +482,15 @@ TEST_F(WebViewTest, ResetScrollAndScaleState)
     EXPECT_EQ(84, webViewImpl->mainFrame()->scrollOffset().height);
     webViewImpl->page()->mainFrame()->loader()->history()->saveDocumentAndScrollState();
 
-    // Confirm that resetting the page state resets both the scale and scroll position, as well
-    // as overwrites the original parameters that were saved to the HistoryController.
+    // Confirm that resetting the page state resets the saved scroll position.
+    // The HistoryController treats a page scale factor of 0.0f as special and avoids
+    // restoring it to the WebView.
     webViewImpl->resetScrollAndScaleState();
-    EXPECT_EQ(0.0f, webViewImpl->pageScaleFactor());
+    EXPECT_EQ(1.0f, webViewImpl->pageScaleFactor());
     EXPECT_EQ(0, webViewImpl->mainFrame()->scrollOffset().width);
     EXPECT_EQ(0, webViewImpl->mainFrame()->scrollOffset().height);
     webViewImpl->page()->mainFrame()->loader()->history()->restoreScrollPositionAndViewState();
-    EXPECT_EQ(0.0f, webViewImpl->pageScaleFactor());
+    EXPECT_EQ(1.0f, webViewImpl->pageScaleFactor());
     EXPECT_EQ(0, webViewImpl->mainFrame()->scrollOffset().width);
     EXPECT_EQ(0, webViewImpl->mainFrame()->scrollOffset().height);
     webViewImpl->close();