Commit 57b28df5 authored by Ryan Landay's avatar Ryan Landay Committed by Commit Bot

Fix incognito toggle animation with >= 6 tabs in Android horizontal tab switcher

I found a bug with the incognito toggle animation in the new Android horizontal
tab switcher if you hit the toggle button while on the last or second-to-last
tab and have >= 6 tabs open. On the second-to-last tab, toggling back to the
stack with >= 6 tabs is jerky, and on the last tab, the animation appears to not
run at all. The fix is very simple, but the cause of the bug, and why it only
occurs with >= 6 tabs open, is somewhat involved.

The incognito toggle animation works by sliding the (up to) 3 visible tabs off
the screen. When toggling from normal to incognito mode, the tabs are slid to
the left. If you do this on the second-to-last or last tab, the last tab is one
of the tabs that gets slid off. getMinScroll() normally returns the scroll
offset of the last tab (with a sign flip) as the minimum allowed overall scroll
offset, to prevent you from scrolling past the last tab when we're not running
an animation. During the switch away animation, we set
mSuppressScrollClamping = true to suppress this clamping. It turns out that this
is currently getting reset to false in finishAnimation() before we run the
switch to animation. This means that with enough tabs open (turns out to be 6
on my test device), the clamped overall scroll offset tracks the last tab,
causing the tabs to appear stationary.

The reason the bug does not occur with fewer tabs open is that getMinScroll()
pulls the scroll offset from the last tab on-screen, which turns out to be
negative for the <= 5 tab case, which becomes positive after doing the sign
flip. MathUtils.clamp() flips the min/max arguments if min > max, so for the
<= 5 tab case, we actually end up using the value of getMaxScroll() (0) for the
min scroll for part of the animation, which masks the problem.

The fix is to set mSuppressScrollClamping = true in runSwitchToAnimation() (we
already have logic to set it back to false when the animation ends).

Bug: 853362,831359
Change-Id: I655ae2c2a4a5a58877e463c84c7c42aeb47a4790
Reviewed-on: https://chromium-review.googlesource.com/1103299Reviewed-by: default avatarMatthew Jones <mdjones@chromium.org>
Commit-Queue: Ryan Landay <rlanday@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568074}
parent b558ba51
......@@ -409,6 +409,7 @@ public class NonOverlappingStack extends Stack {
}
mSwitchedAway = false;
mSuppressScrollClamping = true;
CompositorAnimationHandler handler = mLayout.getAnimationHandler();
Collection<Animator> animationList = new ArrayList<>();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment