Scroll top controls in from bottom of page causes viewport to get pushed up |
||
Issue descriptionVersion: 51.0.2697.0 OS: Android What steps will reproduce the problem? (1) Disable threaded scrolling in about:flags (2) On any scrollable page, zoom in and scroll all the way to the bottom of the page, hiding the top controls. (3) Slowly start scrolling up, showing the top controls What is the expected output? The top controls should come in, the page should stay still relative to the top controls. What do you see instead? The page scrolls up relative to the top controls.
,
Apr 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/71cb5b185f274ee0f96b346ce8a515073e9e2975 commit 71cb5b185f274ee0f96b346ce8a515073e9e2975 Author: bokan <bokan@chromium.org> Date: Wed Apr 27 03:45:22 2016 Make WebWidget::resize early out if the size doesn't change. This was already done in most of WebWidget's descendants except for render_widget_fullscreen_pepper and WebWidgetPopupImpl. It makes more sense to check this inside the call rather than relying on the caller (RenderWidget::Resize in this case) and this simplifies some changes I'm making here in another CL: https://codereview.chromium.org/1844013002/ WebWidgetPopupImpl turned out to be caching the window rect itself. I removed this cached variable and let it just query the RenderWidget's rect instead. In doing this, I believe RenderWidget has a bug in how it handles setWindowRect. It caches the given rect and uses that when queried until a Move ACK comes back from the browser host at which point it starts returning the view_screen_rect_ again, but it never sets the view_screen_rect_. I now set the view_screen_rect_ to the pending_window_rect_ once we get the last Move ACK. BUG= 600507 Review URL: https://codereview.chromium.org/1894333002 Cr-Commit-Position: refs/heads/master@{#389984} [modify] https://crrev.com/71cb5b185f274ee0f96b346ce8a515073e9e2975/content/renderer/render_widget.cc [modify] https://crrev.com/71cb5b185f274ee0f96b346ce8a515073e9e2975/content/renderer/render_widget_fullscreen_pepper.cc [modify] https://crrev.com/71cb5b185f274ee0f96b346ce8a515073e9e2975/third_party/WebKit/Source/web/WebPagePopupImpl.cpp [modify] https://crrev.com/71cb5b185f274ee0f96b346ce8a515073e9e2975/third_party/WebKit/Source/web/WebPagePopupImpl.h
,
Apr 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c63441cc9941c223e2f2d311085903c00da85938 commit c63441cc9941c223e2f2d311085903c00da85938 Author: bokan <bokan@chromium.org> Date: Wed Apr 27 15:49:12 2016 Fix main thread top controls scrolling to mirror CC. There's a couple of issues I found where our behavior on the main thread is divergent from the compositor. The major one is that we don't anchor the viewport when top controls change the viewport bounds. At the document extents, the changing bounds can cause the viewport's scroll offset to be clamped causing unwanted scrolling. I've added an anchoring in didUpdateTopControls that should match CC's anchoring down to the sub-pixel level. This also required some rearranging of operations in applyViewportDeltas. Now that we anchor the top control resizes on main thread as well, we can't have a separate calls to change the "shrinks_blink_layout_size" flag and resize the Blink size so I've added an overload of resize() in WebWidget that does both at the same time. In addition, there were other minor issues that I found but weren't causing an obvoius noticable effect. CC's MaxScrollOffset method |ceil|s the top controls adjustment so I made the main thread do this too. CC's anchoring logic still scrolled the outer viewport first. This was changed a while back in viewport.cc so I changed it here too. BUG= 600507 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel Review-Url: https://codereview.chromium.org/1844013002 Cr-Commit-Position: refs/heads/master@{#390085} [modify] https://crrev.com/c63441cc9941c223e2f2d311085903c00da85938/cc/trees/layer_tree_host_impl.cc [modify] https://crrev.com/c63441cc9941c223e2f2d311085903c00da85938/content/renderer/render_view_impl.cc [modify] https://crrev.com/c63441cc9941c223e2f2d311085903c00da85938/content/renderer/render_view_impl.h [modify] https://crrev.com/c63441cc9941c223e2f2d311085903c00da85938/content/renderer/render_widget.cc [modify] https://crrev.com/c63441cc9941c223e2f2d311085903c00da85938/content/renderer/render_widget.h [modify] https://crrev.com/c63441cc9941c223e2f2d311085903c00da85938/third_party/WebKit/Source/core/frame/FrameView.h [modify] https://crrev.com/c63441cc9941c223e2f2d311085903c00da85938/third_party/WebKit/Source/core/frame/VisualViewport.cpp [modify] https://crrev.com/c63441cc9941c223e2f2d311085903c00da85938/third_party/WebKit/Source/web/ResizeViewportAnchor.cpp [modify] https://crrev.com/c63441cc9941c223e2f2d311085903c00da85938/third_party/WebKit/Source/web/WebViewFrameWidget.cpp [modify] https://crrev.com/c63441cc9941c223e2f2d311085903c00da85938/third_party/WebKit/Source/web/WebViewFrameWidget.h [modify] https://crrev.com/c63441cc9941c223e2f2d311085903c00da85938/third_party/WebKit/Source/web/WebViewImpl.cpp [modify] https://crrev.com/c63441cc9941c223e2f2d311085903c00da85938/third_party/WebKit/Source/web/WebViewImpl.h [modify] https://crrev.com/c63441cc9941c223e2f2d311085903c00da85938/third_party/WebKit/Source/web/tests/TopControlsTest.cpp [modify] https://crrev.com/c63441cc9941c223e2f2d311085903c00da85938/third_party/WebKit/Source/web/tests/VisualViewportTest.cpp [modify] https://crrev.com/c63441cc9941c223e2f2d311085903c00da85938/third_party/WebKit/Source/web/tests/WebFrameTest.cpp [modify] https://crrev.com/c63441cc9941c223e2f2d311085903c00da85938/third_party/WebKit/public/web/WebView.h [modify] https://crrev.com/c63441cc9941c223e2f2d311085903c00da85938/third_party/WebKit/public/web/WebWidget.h
,
Apr 27 2016
I'm leaving this open since there's still a small issue that I punted from the above CL: 1) visit bokan.ca/clamp.html 2) pinch zoom in and scroll down (at least 100px) to hide the top controls 3) Without lifting your finger, start scrolling up so that top controls are revealed and then the page reaches the top. When the page is fully scrolled to the top, it will scroll in JS to the end of the document 4) Once scrolled to the end of the document, release your finger What should happen: Nothing What happens: The scroll offset is pushed up a little bit I suspect this is happening because we're now doing the top controls scroll compensation/anchoring in two places: LayerTreeHostImpl::UpdateViewportContainerSizes and WebViewImpl::resize.
,
Jun 9 2016
Closing since the above no longer repros for me reliably. |
||
►
Sign in to add a comment |
||
Comment 1 by bokan@chromium.org
, Apr 4 2016