Skip to content

Commit

Permalink
[M110] Fix being unable to exit tab switcher if last tab is closed by…
Browse files Browse the repository at this point in the history
… site

This happens when user enters overview mode and at the same time
tabs are closed by the sites itself.

Because the tabs are not closed by user's input, tabPendingClosure is
not triggered such that the back press state is out-of-update.

Override onFinishingTabClosure in order to correctly trigger
notifyBackPressStateChangedInternal in this scenario.

(cherry picked from commit 4ff3da2)

Bug: 1401162
Change-Id: I2deeb41ff55819b46ea42fd34ad82467b57c3605
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4108260
Commit-Queue: Lijin Shen <lazzzis@google.com>
Reviewed-by: Mei Liang <meiliang@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1086021}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4133939
Reviewed-by: Yue Zhang <yuezhanggg@chromium.org>
Cr-Commit-Position: refs/branch-heads/5481@{#125}
Cr-Branched-From: 130f3e4-refs/heads/main@{#1084008}
  • Loading branch information
Lijin Shen authored and Chromium LUCI CQ committed Jan 4, 2023
1 parent fd17cd0 commit b4f1f23
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import org.chromium.base.supplier.ObservableSupplierImpl;
import org.chromium.base.supplier.OneshotSupplier;
import org.chromium.base.task.PostTask;
import org.chromium.chrome.browser.back_press.BackPressManager;
import org.chromium.chrome.browser.browser_controls.BrowserControlsStateProvider;
import org.chromium.chrome.browser.compositor.layouts.LayoutManagerImpl;
import org.chromium.chrome.browser.compositor.layouts.content.TabContentManager;
Expand Down Expand Up @@ -426,6 +427,18 @@ public void tabPendingClosure(Tab tab) {
notifyBackPressStateChangedInternal();
}

@Override
public void onFinishingTabClosure(Tab tab) {
// If tab is closed by the site itself rather than user's input,
// tabPendingClosure & tabClosureCommitted won't be called.
notifyBackPressStateChangedInternal();
}

@Override
public void tabRemoved(Tab tab) {
notifyBackPressStateChangedInternal();
}

@Override
public void multipleTabsPendingClosure(List<Tab> tabs, boolean isAllTabs) {
notifyBackPressStateChangedInternal();
Expand Down Expand Up @@ -880,7 +893,10 @@ public boolean onBackPressed(boolean isOnHomepage) {
return true;
}

if (!mContainerViewModel.get(IS_VISIBLE)) return false;
if (!mContainerViewModel.get(IS_VISIBLE)) {
assert !BackPressManager.isEnabled() : "Invisible container: Backpress must be handled";
return false;
}

if (mTabGridDialogController != null && mTabGridDialogController.handleBackPressed()) {
return true;
Expand All @@ -889,7 +905,10 @@ public boolean onBackPressed(boolean isOnHomepage) {
// When the Start surface is showing, we no longer need to call onTabSelecting().
if (isOnHomepage && mMode == TabListCoordinator.TabListMode.CAROUSEL) return false;

if (mTabModelSelector.getCurrentTab() == null) return false;
if (mTabModelSelector.getCurrentTab() == null) {
assert !BackPressManager.isEnabled() : "No tab: Backpress must be handled";
return false;
}

onTabSelecting(mTabModelSelector.getCurrentTabId(), false);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -996,6 +996,40 @@ public void testBackPressAfterSwitchIncognitoMode() {
Assert.assertEquals(Boolean.FALSE, mMediator.getHandleBackPressChangedSupplier().get());
}

@Test
@EnableFeatures({ChromeFeatureList.BACK_GESTURE_REFACTOR})
public void testBackPressAfterTabClosedBySite() {
initAndAssertAllProperties();
Assert.assertFalse(mMediator.shouldInterceptBackPress());

mModel.set(TabListContainerProperties.IS_VISIBLE, true);
doReturn(false).when(mTabModelFilter).isIncognito();
doReturn(false).when(mTabModelSelector).isIncognitoSelected();
doReturn(true).when(mTabModelSelector).isTabStateInitialized();
doReturn(false).when(mIncognitoReauthController).isIncognitoReauthPending();
doReturn(true).when(mTabModel).isIncognito();
doReturn(mTab1).when(mTabModelSelector).getCurrentTab();

mTabModelSelectorObserverCaptor.getValue().onTabModelSelected(mTabModel, null);
mMediator.showTabSwitcherView(true);
Assert.assertTrue(mMediator.shouldInterceptBackPress());
Assert.assertEquals(Boolean.TRUE, mMediator.getHandleBackPressChangedSupplier().get());

// If tab is closed by the site rather than user's input, only
// onFinishingTabClosure will be triggered.
mModel.set(TabListContainerProperties.IS_VISIBLE, true);
doReturn(false).when(mTabModelFilter).isIncognito();
doReturn(false).when(mTabModelSelector).isIncognitoSelected();
doReturn(true).when(mTabModelSelector).isTabStateInitialized();
doReturn(false).when(mIncognitoReauthController).isIncognitoReauthPending();
doReturn(false).when(mTabModel).isIncognito();
doReturn(null).when(mTabModelSelector).getCurrentTab();

mTabModelObserverCaptor.getValue().onFinishingTabClosure(mTab1);
Assert.assertFalse(mMediator.shouldInterceptBackPress());
Assert.assertEquals(Boolean.FALSE, mMediator.getHandleBackPressChangedSupplier().get());
}

@Test
public void testOnTabModelSelected_NewModelIncognito_ReauthPending_ClearsTabList() {
initAndAssertAllProperties();
Expand Down

0 comments on commit b4f1f23

Please sign in to comment.