https://chromium-review.googlesource.com/c/chromium/src/+/1103299

This bug requires manual review: M68 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), kariahda@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

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

commit 57b28df5e6d7e425b7a34972eaec282b8ae51802
Author: Ryan Landay <rlanday@chromium.org>
Date: Mon Jun 18 18:11:20 2018

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/1103299
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Commit-Queue: Ryan Landay <rlanday@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568074}
[modify] https://crrev.com/57b28df5e6d7e425b7a34972eaec282b8ae51802/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/phone/stack/NonOverlappingStack.java

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

commit 9214105d10d9eedcbb3552f99d766aa0e8b7db33
Author: Ryan Landay <rlanday@chromium.org>
Date: Mon Jun 18 23:08:09 2018

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/1103299
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Commit-Queue: Ryan Landay <rlanday@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#568074}(cherry picked from commit 57b28df5e6d7e425b7a34972eaec282b8ae51802)
Reviewed-on: https://chromium-review.googlesource.com/1105361
Reviewed-by: Ryan Landay <rlanday@chromium.org>
Cr-Commit-Position: refs/branch-heads/3440@{#430}
Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733}
[modify] https://crrev.com/9214105d10d9eedcbb3552f99d766aa0e8b7db33/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/phone/stack/NonOverlappingStack.java

Checked this on Sony Xperia M4 Aqua / N - 68.0.3440.40
Toggling incognito mode in horizontal tab switcher on last or second-to-last tab with >=6 tabs is not janky