Reset VisualViewport position after same page navigation

This issue can be reproduced using pinch zoom in Android, ChromeOS and Mac. After navigating to the **exact** same page, we create a new FrameView but reuse the VisualViewport, meaning its scroll position remains unchanged even though it was relative to an old FrameView. In this patch, we reset the VisualViewport position in Page::didCommitLoad since prior to here we would overwrite the old history item. BUG=642279 Review-Url: https://codereview.chromium.org/2320303002 Cr-Commit-Position: refs/heads/master@{#419167}

Reset VisualViewport position after same page navigation
This issue can be reproduced using pinch zoom in Android, ChromeOS and Mac. After navigating to the **exact** same page, we create a new FrameView but reuse the VisualViewport, meaning its scroll position remains unchanged even though it was relative to an old FrameView. In this patch, we reset the VisualViewport position in Page::didCommitLoad since prior to here we would overwrite the old history item. BUG=642279 Review-Url: https://codereview.chromium.org/2320303002 Cr-Commit-Position: refs/heads/master@{#419167}
4521a9bb · chaopeng · Commit bot · 8bf823df · 4521a9bb · 4521a9bb
Commit 4521a9bb authored Sep 16, 2016 by chaopeng Committed by Commit bot Sep 16, 2016
5 changed files
--- a/third_party/WebKit/LayoutTests/fast/scrolling/same-page-navigate.html
+++ b/third_party/WebKit/LayoutTests/fast/scrolling/same-page-navigate.html
+<!DOCTYPE html>
+
+<a href="./same-page-navigate.html" id="same-page" style="position: absolute; top: 2000px">link to self</a>
+
+<script src="../../resources/testharness.js"></script>
+<script src="../../resources/testharnessreport.js"></script>
+
+<script>
+var same_page_navigate_test = async_test("Ensure that the scroll position is correct when we navigate to same page after scale(zoom-in/zoom-out)");
+
+same_page_navigate_test.step(function(){
+  // exit in second load, using scale to detect
+  if (internals.pageScaleFactor() == 2) {
+    assert_equals(scrollX, 0);
+    assert_equals(scrollY, 0);
+    same_page_navigate_test.done();
+    return;
+  }
+
+  internals.setPageScaleFactor(2);
+  scrollTo(0, 10000);
+
+  var atag = document.getElementById('same-page');
+  var x = (atag.offsetLeft - scrollX + 1) * 2;
+  var y = (atag.offsetTop - scrollY + 1) * 2;
+
+  // this clicks on the link which causes a reload of this page.
+  eventSender.mouseMoveTo(x, y);
+  eventSender.mouseDown();
+  eventSender.mouseUp();
+});
+</script>
\ No newline at end of file
--- a/third_party/WebKit/Source/core/page/Page.cpp
+++ b/third_party/WebKit/Source/core/page/Page.cpp
@@ -438,6 +438,11 @@ void Page::didCommitLoad(LocalFrame* frame)
        useCounter().didCommitLoad();
        deprecation().clearSuppression();
        frameHost().visualViewport().sendUMAMetrics();
+
+        // Need to reset visual viewport position here since before commit load we would update the previous history item,
+        // Page::didCommitLoad is called after a new history item is created in FrameLoader.
+        // fix for crbug.com/642279
+        frameHost().visualViewport().setScrollPosition(DoublePoint(), ProgrammaticScroll);
        m_hostsUsingFeatures.updateMeasurementsAndClear();
        UserGestureIndicator::clearProcessedUserGestureSinceLoad();
    }

--- a/third_party/WebKit/Source/core/testing/Internals.cpp
+++ b/third_party/WebKit/Source/core/testing/Internals.cpp
@@ -1864,6 +1864,17 @@ String Internals::pageSizeAndMarginsInPixels(int pageNumber, int width, int heig
    return PrintContext::pageSizeAndMarginsInPixels(frame(), pageNumber, width, height, marginTop, marginRight, marginBottom, marginLeft);
 }

+float Internals::pageScaleFactor(ExceptionState& exceptionState)
+{
+    Document* document = contextDocument();
+    if (!document || !document->page()) {
+        exceptionState.throwDOMException(InvalidAccessError, document ? "The document's page cannot be retrieved." : "No context document can be obtained.");
+        return 0;
+    }
+    Page* page = document->page();
+    return page->frameHost().visualViewport().pageScale();
+}
+
 void Internals::setPageScaleFactor(float scaleFactor, ExceptionState& exceptionState)
 {
    Document* document = contextDocument();

--- a/third_party/WebKit/Source/core/testing/Internals.h
+++ b/third_party/WebKit/Source/core/testing/Internals.h
@@ -261,6 +261,7 @@ public:
    String pageProperty(String, int, ExceptionState& = ASSERT_NO_EXCEPTION) const;
    String pageSizeAndMarginsInPixels(int, int, int, int, int, int, int, ExceptionState& = ASSERT_NO_EXCEPTION) const;

+    float pageScaleFactor(ExceptionState&);
    void setPageScaleFactor(float scaleFactor, ExceptionState&);
    void setPageScaleFactorLimits(float minScaleFactor, float maxScaleFactor, ExceptionState&);


--- a/third_party/WebKit/Source/core/testing/Internals.idl
+++ b/third_party/WebKit/Source/core/testing/Internals.idl
@@ -210,6 +210,7 @@
    [RaisesException] DOMString pageProperty(DOMString propertyName, long pageNumber);
    [RaisesException] DOMString pageSizeAndMarginsInPixels(long pageIndex, long width, long height, long marginTop, long marginRight, long marginBottom, long marginLeft);

+    [RaisesException] float pageScaleFactor();
    [RaisesException] void setPageScaleFactor(float scaleFactor);
    [RaisesException] void setPageScaleFactorLimits(float minScaleFactor, float maxScaleFactor);