Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(205)

Unified Diff: third_party/WebKit/Source/web/WebViewImpl.cpp

Issue 1844013002: Fix main thread top controls scrolling to mirror CC. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@propertyTreesBoundsDelta
Patch Set: Whitespace Created 4 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: third_party/WebKit/Source/web/WebViewImpl.cpp
diff --git a/third_party/WebKit/Source/web/WebViewImpl.cpp b/third_party/WebKit/Source/web/WebViewImpl.cpp
index 98b04285fcb22d795e62fbf56591a796e7734f21..f66f9eb18954d85f3c84b9e73b8d954c8dfbf2d7 100644
--- a/third_party/WebKit/Source/web/WebViewImpl.cpp
+++ b/third_party/WebKit/Source/web/WebViewImpl.cpp
@@ -1827,11 +1827,30 @@ void WebViewImpl::didUpdateTopControls()
return;
VisualViewport& visualViewport = page()->frameHost().visualViewport();
- float topControlsViewportAdjustment = topControls().layoutHeight() - topControls().contentOffset();
- visualViewport.setTopControlsAdjustment(topControlsViewportAdjustment);
- // Shrink the FrameView by the amount that will maintain the aspect-ratio with the VisualViewport.
- view->setTopControlsViewportAdjustment(topControlsViewportAdjustment / minimumPageScaleFactor());
+ {
+ // This object will save the current visual viewport offset w.r.t. the
+ // document and restore it when the object goes out of scope. It's
+ // needed since the top controls adjustment will change the maximum
+ // scroll offset and we may need to reposition them to keep the user's
+ // apparent position unchanged.
+ ResizeViewportAnchor anchor(*view, visualViewport);
+
+ float topControlsViewportAdjustment =
+ topControls().layoutHeight() - topControls().contentOffset();
+ visualViewport.setTopControlsAdjustment(topControlsViewportAdjustment);
+
+ // Since the FrameView is sized to be the visual viewport at minimum
+ // scale, its adjustment must also be scaled by the minimum scale.
+ view->setTopControlsViewportAdjustment(
+ topControlsViewportAdjustment / minimumPageScaleFactor());
+
+ // Clamp the scroll offset of each viewport now that their bounds have
+ // changed. We'll try to restore hte saved scroll offset when anchor
majidvp 2016/04/01 17:09:36 s/hte/the/
bokan 2016/04/05 17:11:40 Done.
+ // goes out of scope.
+ visualViewport.clampToBoundaries();
+ view->setScrollPosition(view->scrollPositionDouble(), ProgrammaticScroll);
majidvp 2016/04/01 17:09:36 Shouldn't this be unnecessary? The anchor sets the
bokan 2016/04/05 17:11:40 But the anchor needs the viewports to be at their
+ }
}
TopControls& WebViewImpl::topControls()
@@ -4330,25 +4349,32 @@ void WebViewImpl::applyViewportDeltas(
if (!frameView)
return;
+ ScrollableArea* layoutViewport = frameView->layoutViewportScrollableArea();
+ VisualViewport& visualViewport = page()->frameHost().visualViewport();
+
+ // Store the desired offsets for visual and layout viewports before setting
+ // the top controls ratio since doing so will change the bounds and move the
+ // viewports to keep the offsets valid. The compositor may have already done
+ // that so we don't want to double apply the deltas here.
+ FloatPoint visualViewportOffset = visualViewport.visibleRect().location();
+ visualViewportOffset.move(
+ visualViewportDelta.width,
+ visualViewportDelta.height);
+ DoublePoint layoutViewportPosition = layoutViewport->scrollPositionDouble()
+ + DoubleSize(layoutViewportDelta.width, layoutViewportDelta.height);
+
topControls().setShownRatio(topControls().shownRatio() + topControlsShownRatioDelta);
- FloatPoint visualViewportOffset = page()->frameHost().visualViewport().visibleRect().location();
- visualViewportOffset.move(visualViewportDelta.width, visualViewportDelta.height);
setPageScaleFactorAndLocation(pageScaleFactor() * pageScaleDelta, visualViewportOffset);
if (pageScaleDelta != 1) {
m_doubleTapZoomPending = false;
- page()->frameHost().visualViewport().userDidChangeScale();
+ visualViewport.userDidChangeScale();
}
m_elasticOverscroll += elasticOverscrollDelta;
frameView->didUpdateElasticOverscroll();
- ScrollableArea* layoutViewport = frameView->layoutViewportScrollableArea();
-
- DoublePoint layoutViewportPosition = layoutViewport->scrollPositionDouble()
- + DoubleSize(layoutViewportDelta.width, layoutViewportDelta.height);
-
if (layoutViewport->scrollPositionDouble() != layoutViewportPosition) {
layoutViewport->setScrollPosition(layoutViewportPosition, CompositorScroll);
if (DocumentLoader* documentLoader = mainFrameImpl()->frame()->loader().documentLoader())

Powered by Google App Engine
This is Rietveld 408576698