Fix scroll anchoring during viewport resize.

third_party/WebKit/Source/web/ResizeViewportAnchor.cpp

Index: third_party/WebKit/Source/web/ResizeViewportAnchor.cpp
diff --git a/third_party/WebKit/Source/web/ResizeViewportAnchor.cpp b/third_party/WebKit/Source/web/ResizeViewportAnchor.cpp
index d262e08301a275144238bcc9960ea802665ebfba..2c333eaa50fcbcd21fd59f4dfc51a40f11ed5cd5 100644
--- a/third_party/WebKit/Source/web/ResizeViewportAnchor.cpp
+++ b/third_party/WebKit/Source/web/ResizeViewportAnchor.cpp
@@ -4,22 +4,41 @@
 
 #include "web/ResizeViewportAnchor.h"
 
+#include "core/frame/FrameHost.h"
 #include "core/frame/FrameView.h"
 #include "core/frame/VisualViewport.h"
+#include "core/page/Page.h"
 #include "platform/geometry/DoubleRect.h"
 #include "platform/geometry/DoubleSize.h"
 #include "platform/geometry/FloatSize.h"
 
 namespace blink {
 
-ResizeViewportAnchor::ResizeViewportAnchor(FrameView& rootFrameView, VisualViewport& visualViewport)
-    : ViewportAnchor(rootFrameView, visualViewport)
-    , m_visualViewportInDocument(rootFrameView.getScrollableArea()->scrollPositionDouble())
+void ResizeViewportAnchor::resizeFrameView(IntSize size)
 {
+    FrameView* frameView = rootFrameView();
+    DCHECK(frameView);
+
+    ScrollableArea* rootViewport = frameView->getScrollableArea();
+    DoublePoint position = rootViewport->scrollPositionDouble();
+
+    frameView->resize(size);

[bokan, 2016/07/28 14:55:55]

I was a little concerned about this at first since the resize call here can
cause both a ScrollAnchor and a clamp, one of which this anchor wants to adjust
for, but after thinking about it some more I think we may be ok.

The reason we wanted to anchor the clamp prior to this was that we set the
topControlsAdjustment before the frame resize which means our scroll position
goes out of bounds and we clamp because of the didUpdateTopControls anchor. But
because the didUpdateTopControls anchor is now a no-op in the
WebViewImpl::resizeWithTopControls case, there should be no clamp between
setting the topControlsAdjustment and  resizing the FrameView. Since the
FrameView can't scroll out of bounds then we should never actually clamp it
here.

Is this your understanding?

[skobes, 2016/07/28 15:45:50]

ScrollAnchor shouldn't actually be doing anything during FrameView::resize,
since there's no layout happening (though this is a little hard to see since we
call restore from updateScrollbars).

We want ResizeViewportAnchor to track drift from the resize, but not from
ScrollAnchor.
I think this is right.  I made nested RVA a nop because otherwise the outer RVA
needs to track drift from the inner RVA, which seemed a bit silly.

But the nice thing about this design is that in principle the clamps can happen
at any time as long as we plumb them through RVA.  For example if we want to
clamp after setting the top controls adjustment we can add
RVA::setTopControlsAdjustment which updates m_drift in the same way as
RVA::resizeFrameView.

In this change I'm being as conservative as possible but I think this gives us a
good foundation to move toward a world where scroll positions are always valid.

+    m_drift += rootViewport->scrollPositionDouble() - position;
 }
 
-ResizeViewportAnchor::~ResizeViewportAnchor()
+void ResizeViewportAnchor::endScope()
 {
+    if (--m_scopeCount > 0)
+        return;
+
+    FrameView* frameView = rootFrameView();
+    if (!frameView)
+        return;
+
+    VisualViewport& visualViewport = m_page->frameHost().visualViewport();
+    DoublePoint visualViewportInDocument =
+        frameView->getScrollableArea()->scrollPositionDouble() - m_drift;
+
     // TODO(bokan): Don't use RootFrameViewport::setScrollPosition since it
     // assumes we can just set a sub-pixel precision offset on the FrameView.
     // While we "can" do this, the offset that will be shipped to CC will be the
@@ -28,22 +47,22 @@ ResizeViewportAnchor::~ResizeViewportAnchor()
     // replace this with RFV::setScrollPosition once Blink is sub-pixel scroll
     // offset aware. crbug.com/414283.
 
-    ScrollableArea* rootViewport = m_rootFrameView->getScrollableArea();
+    ScrollableArea* rootViewport = frameView->getScrollableArea();
     ScrollableArea* layoutViewport =
-        m_rootFrameView->layoutViewportScrollableArea();
+        frameView->layoutViewportScrollableArea();
 
     // Clamp the scroll offset of each viewport now so that we force any invalid
     // offsets to become valid so we can compute the correct deltas.
-    m_visualViewport->clampToBoundaries();
+    visualViewport.clampToBoundaries();
     layoutViewport->setScrollPosition(
         layoutViewport->scrollPositionDouble(), ProgrammaticScroll);
 
-    DoubleSize delta = m_visualViewportInDocument
+    DoubleSize delta = visualViewportInDocument
         - rootViewport->scrollPositionDouble();
 
-    m_visualViewport->move(toFloatSize(delta));
+    visualViewport.move(toFloatSize(delta));
 
-    delta = m_visualViewportInDocument
+    delta = visualViewportInDocument
         - rootViewport->scrollPositionDouble();
 
     // Since the main thread FrameView has integer scroll offsets, scroll it to
@@ -59,9 +78,19 @@ ResizeViewportAnchor::~ResizeViewportAnchor()
         layoutViewport->scrollPosition() + layoutDelta,
         ProgrammaticScroll);
 
-    delta = m_visualViewportInDocument
+    delta = visualViewportInDocument
         - rootViewport->scrollPositionDouble();
-    m_visualViewport->move(toFloatSize(delta));
+    visualViewport.move(toFloatSize(delta));
+    m_drift = DoubleSize();
+}
+
+FrameView* ResizeViewportAnchor::rootFrameView()
+{
+    if (Frame* frame = m_page->mainFrame()) {
+        if (frame->isLocalFrame())
+            return toLocalFrame(frame)->view();
+    }
+    return nullptr;
 }
 
 } // namespace blink