New issue
Advanced search Search tips

Issue 627911 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

Animating TopControls in at the bottom of page causes it to scroll up

Project Member Reported by bokan@chromium.org, Jul 13 2016

Issue description

Version: 53.0.2782.9
OS: Android

What steps will reproduce the problem?
(1) Visit a page like wikipedia.org
(2) Scroll all the way to the bottom, hiding the top controls
(3) Show the top controls by just over 50% and let go to animate them in

What is the expected output?
The top controls should animate down but the page content shouldn't move

What do you see instead?
The page scrolls up a bit

 

Comment 1 by bokan@chromium.org, Jul 13 2016

Status: Started (was: Assigned)
Turns out this is an issue with synchronizing the active and pending trees on the impl thread, rather than with viewport anchoring on the main thread. I've got a fix ready but there's some other subtle issues I've discovered so I'll try to fix them all in one go.
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 19 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fe9e63ecc3b6d320006ba0d22b7000eb0af30239

commit fe9e63ecc3b6d320006ba0d22b7000eb0af30239
Author: bokan <bokan@chromium.org>
Date: Tue Jul 19 19:19:08 2016

Update scroll layers' bounds_delta earlier during tree activation.

When a tree activation occurs, we must update the property trees' bounds deltas
so that they get propagated across the trees. This was previously done in
LayerTreeHostImpl::ActivateSyncTree() and needs to happen before
updateViewportContainerSizes() is called, otherwise it will clamp scroll offsets
incorrectly becaues the bounds_deltas haven't been propagated yet.

However, this bug occurs because earlier in ActivateSyncTree(), in
LayerTreeImpl::PushPropertiesTo, we may change the top controls values when
PushTopControls is called and causes a change. This indirectly causes
UpdateViewportContainerSizes to be called which we must do after updating the
bounds deltas.

This CL moves the update to happen inside LayerTreeImpl::PushPropertiesTo near
the beginning. I've rewritten the ViewportBoundsDeltaOnTreeActivation test to
catch this. It was written to catch basically the same issue but with
UpdateViewportContainerSizes being called later than in this bug so checking for
this case should catch both.

BUG= 627911 
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Review-Url: https://codereview.chromium.org/2149033002
Cr-Commit-Position: refs/heads/master@{#406341}

[modify] https://crrev.com/fe9e63ecc3b6d320006ba0d22b7000eb0af30239/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/fe9e63ecc3b6d320006ba0d22b7000eb0af30239/cc/trees/layer_tree_host_impl_unittest.cc
[modify] https://crrev.com/fe9e63ecc3b6d320006ba0d22b7000eb0af30239/cc/trees/layer_tree_impl.cc

Comment 3 by bokan@chromium.org, Jul 20 2016

Status: Fixed (was: Started)

Sign in to add a comment