Commit 6be6515b authored by Mei Liang's avatar Mei Liang Committed by Commit Bot

Fix crashing when closing the very last tab

This CL fixes some closing tab behavior:
 1. Reset strip/grid sheet when closing a group
 2. Update the tab grid sheet title after tab is removed in
    TabGroupModelFilter
 2. Close app when closing the very last tab in Chrome
    when using the TabGroup UI.

Bug: 948527
Change-Id: Ic112475baf6cd718901d0c06019af134573d8cd1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1564883
Commit-Queue: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Reviewed-by: default avatarYusuf Ozuysal <yusufo@chromium.org>
Reviewed-by: default avatarWei-Yin Chen (陳威尹) <wychen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#650644}
parent b105fbd2
...@@ -12,7 +12,6 @@ import org.chromium.base.ApplicationStatus; ...@@ -12,7 +12,6 @@ import org.chromium.base.ApplicationStatus;
import org.chromium.chrome.browser.ChromeTabbedActivity; import org.chromium.chrome.browser.ChromeTabbedActivity;
import org.chromium.chrome.browser.ThemeColorProvider; import org.chromium.chrome.browser.ThemeColorProvider;
import org.chromium.chrome.browser.compositor.layouts.content.TabContentManager; import org.chromium.chrome.browser.compositor.layouts.content.TabContentManager;
import org.chromium.chrome.browser.lifecycle.Destroyable;
import org.chromium.chrome.browser.tab.Tab; import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tabmodel.TabCreatorManager; import org.chromium.chrome.browser.tabmodel.TabCreatorManager;
import org.chromium.chrome.browser.tabmodel.TabModelSelector; import org.chromium.chrome.browser.tabmodel.TabModelSelector;
...@@ -27,7 +26,7 @@ import java.util.List; ...@@ -27,7 +26,7 @@ import java.util.List;
* {@link TabListCoordinator} as well as the life-cycle of shared component * {@link TabListCoordinator} as well as the life-cycle of shared component
* objects. * objects.
*/ */
public class TabGridSheetCoordinator implements Destroyable { public class TabGridSheetCoordinator {
final static String COMPONENT_NAME = "TabGridSheet"; final static String COMPONENT_NAME = "TabGridSheet";
private final Context mContext; private final Context mContext;
private final TabListCoordinator mTabGridCoordinator; private final TabListCoordinator mTabGridCoordinator;
...@@ -56,7 +55,6 @@ public class TabGridSheetCoordinator implements Destroyable { ...@@ -56,7 +55,6 @@ public class TabGridSheetCoordinator implements Destroyable {
/** /**
* Destroy any members that needs clean up. * Destroy any members that needs clean up.
*/ */
@Override
public void destroy() { public void destroy() {
mTabGridCoordinator.destroy(); mTabGridCoordinator.destroy();
mMediator.destroy(); mMediator.destroy();
...@@ -95,6 +93,7 @@ public class TabGridSheetCoordinator implements Destroyable { ...@@ -95,6 +93,7 @@ public class TabGridSheetCoordinator implements Destroyable {
if (mToolbarCoordinator != null) { if (mToolbarCoordinator != null) {
mToolbarCoordinator.destroy(); mToolbarCoordinator.destroy();
mToolbarCoordinator = null;
} }
} }
} }
......
...@@ -12,7 +12,6 @@ import org.chromium.base.metrics.RecordUserAction; ...@@ -12,7 +12,6 @@ import org.chromium.base.metrics.RecordUserAction;
import org.chromium.chrome.R; import org.chromium.chrome.R;
import org.chromium.chrome.browser.ThemeColorProvider; import org.chromium.chrome.browser.ThemeColorProvider;
import org.chromium.chrome.browser.UrlConstants; import org.chromium.chrome.browser.UrlConstants;
import org.chromium.chrome.browser.lifecycle.Destroyable;
import org.chromium.chrome.browser.tab.Tab; import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tabmodel.EmptyTabModelObserver; import org.chromium.chrome.browser.tabmodel.EmptyTabModelObserver;
import org.chromium.chrome.browser.tabmodel.TabCreatorManager; import org.chromium.chrome.browser.tabmodel.TabCreatorManager;
...@@ -35,7 +34,7 @@ import java.util.List; ...@@ -35,7 +34,7 @@ import java.util.List;
* with the components' coordinator as well as managing the state of the bottom * with the components' coordinator as well as managing the state of the bottom
* sheet. * sheet.
*/ */
class TabGridSheetMediator implements Destroyable { class TabGridSheetMediator {
/** /**
* Defines an interface for a {@link TabGridSheetMediator} reset event handler. * Defines an interface for a {@link TabGridSheetMediator} reset event handler.
*/ */
...@@ -58,6 +57,7 @@ class TabGridSheetMediator implements Destroyable { ...@@ -58,6 +57,7 @@ class TabGridSheetMediator implements Destroyable {
private final ThemeColorProvider mThemeColorProvider; private final ThemeColorProvider mThemeColorProvider;
private final ThemeColorProvider.ThemeColorObserver mThemeColorObserver; private final ThemeColorProvider.ThemeColorObserver mThemeColorObserver;
private final ThemeColorProvider.TintObserver mTintObserver; private final ThemeColorProvider.TintObserver mTintObserver;
private final ResetHandler mResetHandler;
TabGridSheetMediator(Context context, BottomSheetController bottomSheetController, TabGridSheetMediator(Context context, BottomSheetController bottomSheetController,
ResetHandler resetHandler, PropertyModel model, TabModelSelector tabModelSelector, ResetHandler resetHandler, PropertyModel model, TabModelSelector tabModelSelector,
...@@ -68,6 +68,7 @@ class TabGridSheetMediator implements Destroyable { ...@@ -68,6 +68,7 @@ class TabGridSheetMediator implements Destroyable {
mTabModelSelector = tabModelSelector; mTabModelSelector = tabModelSelector;
mTabCreatorManager = tabCreatorManager; mTabCreatorManager = tabCreatorManager;
mThemeColorProvider = themeColorProvider; mThemeColorProvider = themeColorProvider;
mResetHandler = resetHandler;
// TODO (ayman): Add instrumentation to observer calls. // TODO (ayman): Add instrumentation to observer calls.
mSheetObserver = new EmptyBottomSheetObserver() { mSheetObserver = new EmptyBottomSheetObserver() {
...@@ -83,19 +84,24 @@ class TabGridSheetMediator implements Destroyable { ...@@ -83,19 +84,24 @@ class TabGridSheetMediator implements Destroyable {
// register for tab model // register for tab model
mTabModelObserver = new EmptyTabModelObserver() { mTabModelObserver = new EmptyTabModelObserver() {
@Override @Override
public void didCloseTab(int tabId, boolean incognito) { public void didAddTab(Tab tab, @TabLaunchType int type) {
updateBottomSheetTitleAndMargin(); updateBottomSheet();
} }
@Override @Override
public void didAddTab(Tab tab, @TabLaunchType int type) { public void tabClosureUndone(Tab tab) {
updateBottomSheetTitleAndMargin(); updateBottomSheet();
} }
@Override @Override
public void didSelectTab(Tab tab, int type, int lastId) { public void didSelectTab(Tab tab, int type, int lastId) {
if (type == TabSelectionType.FROM_USER) resetHandler.resetWithListOfTabs(null); if (type == TabSelectionType.FROM_USER) resetHandler.resetWithListOfTabs(null);
} }
@Override
public void willCloseTab(Tab tab, boolean animate) {
updateBottomSheet();
}
}; };
mThemeColorObserver = mThemeColorObserver =
(color, shouldAnimate) -> mModel.set(TabGridSheetProperties.PRIMARY_COLOR, color); (color, shouldAnimate) -> mModel.set(TabGridSheetProperties.PRIMARY_COLOR, color);
...@@ -127,7 +133,6 @@ class TabGridSheetMediator implements Destroyable { ...@@ -127,7 +133,6 @@ class TabGridSheetMediator implements Destroyable {
/** /**
* Destroy any members that needs clean up. * Destroy any members that needs clean up.
*/ */
@Override
public void destroy() { public void destroy() {
if (mTabModelObserver != null) { if (mTabModelObserver != null) {
mTabModelSelector.getTabModelFilterProvider().removeTabModelFilterObserver( mTabModelSelector.getTabModelFilterProvider().removeTabModelFilterObserver(
...@@ -138,7 +143,7 @@ class TabGridSheetMediator implements Destroyable { ...@@ -138,7 +143,7 @@ class TabGridSheetMediator implements Destroyable {
} }
private void showTabGridSheet(TabGridSheetContent sheetContent) { private void showTabGridSheet(TabGridSheetContent sheetContent) {
updateBottomSheetTitleAndMargin(); updateBottomSheet();
mBottomSheetController.getBottomSheet().addObserver(mSheetObserver); mBottomSheetController.getBottomSheet().addObserver(mSheetObserver);
mBottomSheetController.requestShowContent(sheetContent, true); mBottomSheetController.requestShowContent(sheetContent, true);
mBottomSheetController.expandSheet(); mBottomSheetController.expandSheet();
...@@ -155,13 +160,18 @@ class TabGridSheetMediator implements Destroyable { ...@@ -155,13 +160,18 @@ class TabGridSheetMediator implements Destroyable {
: null; : null;
} }
private void updateBottomSheetTitleAndMargin() { private void updateBottomSheet() {
Tab currentTab = mTabModelSelector.getCurrentTab(); Tab currentTab = mTabModelSelector.getCurrentTab();
if (currentTab == null) return; if (currentTab == null) return;
int tabsCount = mTabModelSelector.getTabModelFilterProvider() int tabsCount = mTabModelSelector.getTabModelFilterProvider()
.getCurrentTabModelFilter() .getCurrentTabModelFilter()
.getRelatedTabList(currentTab.getId()) .getRelatedTabList(currentTab.getId())
.size(); .size();
if (tabsCount == 0) {
mResetHandler.resetWithListOfTabs(null);
return;
}
mModel.set(TabGridSheetProperties.HEADER_TITLE, mModel.set(TabGridSheetProperties.HEADER_TITLE,
mContext.getResources().getQuantityString( mContext.getResources().getQuantityString(
R.plurals.bottom_tab_grid_title_placeholder, tabsCount, tabsCount)); R.plurals.bottom_tab_grid_title_placeholder, tabsCount, tabsCount));
......
...@@ -68,7 +68,8 @@ public class TabGroupUiMediator { ...@@ -68,7 +68,8 @@ public class TabGroupUiMediator {
private final ThemeColorProvider.TintObserver mTintObserver; private final ThemeColorProvider.TintObserver mTintObserver;
private final TabModelSelectorTabObserver mTabModelSelectorTabObserver; private final TabModelSelectorTabObserver mTabModelSelectorTabObserver;
private final TabModelSelectorObserver mTabModelSelectorObserver; private final TabModelSelectorObserver mTabModelSelectorObserver;
private boolean mIsResetWithNonNullList; private boolean mIsTabGroupUiVisible;
private boolean mIsClosingAGroup;
TabGroupUiMediator( TabGroupUiMediator(
BottomControlsCoordinator.BottomControlsVisibilityController visibilityController, BottomControlsCoordinator.BottomControlsVisibilityController visibilityController,
...@@ -87,12 +88,25 @@ public class TabGroupUiMediator { ...@@ -87,12 +88,25 @@ public class TabGroupUiMediator {
mTabModelObserver = new EmptyTabModelObserver() { mTabModelObserver = new EmptyTabModelObserver() {
@Override @Override
public void didSelectTab(Tab tab, @TabSelectionType int type, int lastId) { public void didSelectTab(Tab tab, @TabSelectionType int type, int lastId) {
if (type == TabSelectionType.FROM_CLOSE if (!mIsTabGroupUiVisible) return;
|| getRelatedTabsForId(lastId).contains(tab)) if (type == TabSelectionType.FROM_CLOSE && !mIsClosingAGroup) return;
return; if (getRelatedTabsForId(lastId).contains(tab)) return;
resetTabStripWithRelatedTabsForId(tab.getId()); resetTabStripWithRelatedTabsForId(tab.getId());
} }
@Override
public void willCloseTab(Tab tab, boolean animate) {
if (!mIsTabGroupUiVisible) return;
Tab currentTab = mTabModelSelector.getCurrentTab();
if (currentTab == null) mResetHandler.resetSheetWithListOfTabs(null);
int tabsCount = mTabModelSelector.getTabModelFilterProvider()
.getCurrentTabModelFilter()
.getRelatedTabList(currentTab.getId())
.size();
mIsClosingAGroup = tabsCount == 0;
}
@Override @Override
public void didAddTab(Tab tab, int type) { public void didAddTab(Tab tab, int type) {
if (type == TabLaunchType.FROM_CHROME_UI || type == TabLaunchType.FROM_RESTORE) if (type == TabLaunchType.FROM_CHROME_UI || type == TabLaunchType.FROM_RESTORE)
...@@ -128,7 +142,7 @@ public class TabGroupUiMediator { ...@@ -128,7 +142,7 @@ public class TabGroupUiMediator {
.getRelatedTabList(tab.getId()); .getRelatedTabList(tab.getId());
int numTabs = listOfTabs.size(); int numTabs = listOfTabs.size();
// This is set to zero because the UI is hidden. // This is set to zero because the UI is hidden.
if (!mIsResetWithNonNullList) numTabs = 0; if (!mIsTabGroupUiVisible) numTabs = 0;
RecordHistogram.recordCountHistogram("TabStrip.TabCountOnPageLoad", numTabs); RecordHistogram.recordCountHistogram("TabStrip.TabCountOnPageLoad", numTabs);
} }
}; };
...@@ -190,12 +204,12 @@ public class TabGroupUiMediator { ...@@ -190,12 +204,12 @@ public class TabGroupUiMediator {
.getRelatedTabList(id); .getRelatedTabList(id);
if (listOfTabs.size() < 2) { if (listOfTabs.size() < 2) {
mResetHandler.resetStripWithListOfTabs(null); mResetHandler.resetStripWithListOfTabs(null);
mIsResetWithNonNullList = false; mIsTabGroupUiVisible = false;
} else { } else {
mResetHandler.resetStripWithListOfTabs(listOfTabs); mResetHandler.resetStripWithListOfTabs(listOfTabs);
mIsResetWithNonNullList = true; mIsTabGroupUiVisible = true;
} }
mVisibilityController.setBottomControlsVisible(mIsResetWithNonNullList); mVisibilityController.setBottomControlsVisible(mIsTabGroupUiVisible);
} }
private List<Tab> getRelatedTabsForId(int id) { private List<Tab> getRelatedTabsForId(int id) {
......
...@@ -615,8 +615,15 @@ public class ChromeTabbedActivity ...@@ -615,8 +615,15 @@ public class ChromeTabbedActivity
private void closeIfNoTabsAndHomepageEnabled(boolean isPendingClosure) { private void closeIfNoTabsAndHomepageEnabled(boolean isPendingClosure) {
if (getTabModelSelector().getTotalTabCount() == 0) { if (getTabModelSelector().getTotalTabCount() == 0) {
// If the last tab is closed, and homepage is enabled, then exit Chrome. // If the last tab is closed, and one of the following is true, then exit
if (HomepageManager.shouldCloseAppWithZeroTabs()) { // Chrome:
// 1. If homepage is enabled.
// 2. If TabGroupsAndroid is enabled, and isPendingClosure is true.
// isPendingClosure is used to avoid calling finish() when closing all
// tabs in tab switcher.
if (HomepageManager.shouldCloseAppWithZeroTabs()
|| (FeatureUtilities.isTabGroupsAndroidEnabled()
&& isPendingClosure)) {
finish(); finish();
} else if (isPendingClosure) { } else if (isPendingClosure) {
NewTabPageUma.recordNTPImpression( NewTabPageUma.recordNTPImpression(
......
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