Skip to content

Commit

Permalink
[M115] Fix tab switcher handler keeps consuming back event on tablet
Browse files Browse the repository at this point in the history
mIsTransitionInProgress was introduced to prevent back event from
being consumed by tab navigation during the transition of tab-to-GTS
animation (expand/shrink tabs).

On tablet, a translation replaces that and a back event will cancel
the animation rather than causing the tab to be navigated.

The bug can be triggered when a back gesture is performed during
closing the tab switcher on tablet. The 2nd back event will force
the previous animation to end and assign mIsTransitionInProgress to
false. Then the 2nd back event triggered hideTabSwitcherView again
to assign it to true. But the previous animation has hidden the
tab switcher, postHiding will not be invoked again, leaving
mIsTransitionInProgress being true forever.

(cherry picked from commit 51b108d)

Bug: 1449027
Change-Id: I33eb191907a3e9d95abef9e6a9906d63eefeb329
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4575333
Commit-Queue: Lijin Shen <lazzzis@google.com>
Code-Coverage: Findit <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: Calder Kitagawa <ckitagawa@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1151434}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4599890
Reviewed-by: Mei Liang <meiliang@chromium.org>
Cr-Commit-Position: refs/branch-heads/5790@{#509}
Cr-Branched-From: 1d71a33-refs/heads/main@{#1148114}
  • Loading branch information
Lijin Shen authored and Chromium LUCI CQ committed Jun 8, 2023
1 parent 3060348 commit 37ad3fe
Showing 1 changed file with 8 additions and 5 deletions.
Expand Up @@ -877,10 +877,11 @@ private boolean onBackPressedInternal() {
return true;
}

if (mIsTransitionInProgress && mMode == TabListCoordinator.TabListMode.GRID) {
// crbug.com/1420410: intentionally do nothing to wait for transition to be finished.
// Note this has to be before following if-branch since during transition, the container
// is still invisible.
if (!mIsTablet && mIsTransitionInProgress && mMode == TabListCoordinator.TabListMode.GRID) {
// crbug.com/1420410: intentionally do nothing to wait for tab-to-GTS transition to be
// finished. Note this has to be before following if-branch since during transition, the
// container is still invisible. On tablet, the translation transition replaces the
// tab-to-GTS (expand/shrink) animation, which does not suffer from the same issue.
return true;
}

Expand Down Expand Up @@ -1184,7 +1185,9 @@ boolean shouldInterceptBackPress() {
if (isDialogVisible()) return true;
if (mCustomViewBackPressRunnable != null) return true;

if (mIsTransitionInProgress && mMode == TabListCoordinator.TabListMode.GRID) return true;
if (!mIsTablet && mIsTransitionInProgress && mMode == TabListCoordinator.TabListMode.GRID) {
return true;
}

if (!mContainerViewModel.get(IS_VISIBLE)) return false;

Expand Down

0 comments on commit 37ad3fe

Please sign in to comment.