Commit b105fbd2 authored by Mei Liang's avatar Mei Liang Committed by Commit Bot

Provide a recommended next tab when closing tab

Bug: 951436
Change-Id: I88aaa98ddb82103eea0920adc830bdbe6f06e551
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1564351Reviewed-by: default avatarWei-Yin Chen (陳威尹) <wychen@chromium.org>
Reviewed-by: default avatarYusuf Ozuysal <yusufo@chromium.org>
Commit-Queue: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#650643}
parent 7e3808b6
...@@ -304,6 +304,8 @@ class TabListMediator { ...@@ -304,6 +304,8 @@ class TabListMediator {
mTabModelSelector.getTabModelFilterProvider().addTabModelFilterObserver(mTabModelObserver); mTabModelSelector.getTabModelFilterProvider().addTabModelFilterObserver(mTabModelObserver);
// TODO(meiliang): follow up with unit tests to test the close signal is sent correctly with
// the recommendedNextTab.
mTabClosedListener = new TabActionListener() { mTabClosedListener = new TabActionListener() {
@Override @Override
public void run(int tabId) { public void run(int tabId) {
...@@ -318,9 +320,32 @@ class TabListMediator { ...@@ -318,9 +320,32 @@ class TabListMediator {
} }
} }
onTabClosedFrom(tabId, mComponentName); onTabClosedFrom(tabId, mComponentName);
Tab currentTab = mTabModelSelector.getCurrentTab();
Tab closingTab =
TabModelUtils.getTabById(mTabModelSelector.getCurrentModel(), tabId);
Tab nextTab = currentTab == closingTab ? getNextTab(tabId) : null;
mTabModelSelector.getCurrentModel().closeTab( mTabModelSelector.getCurrentModel().closeTab(
TabModelUtils.getTabById(mTabModelSelector.getCurrentModel(), tabId), false, closingTab, nextTab, false, false, true);
false, true); }
private Tab getNextTab(int closingTabId) {
int closingTabIndex = mModel.indexFromId(closingTabId);
if (closingTabIndex == TabModel.INVALID_TAB_INDEX) {
assert false;
return null;
}
int nextTabId = Tab.INVALID_TAB_ID;
if (mModel.size() > 1) {
nextTabId = closingTabIndex == 0
? mModel.get(closingTabIndex + 1).get(TabProperties.TAB_ID)
: mModel.get(closingTabIndex - 1).get(TabProperties.TAB_ID);
}
return TabModelUtils.getTabById(mTabModelSelector.getCurrentModel(), nextTabId);
} }
}; };
} }
......
...@@ -184,7 +184,7 @@ public class TabListMediatorUnitTest { ...@@ -184,7 +184,7 @@ public class TabListMediatorUnitTest {
.get(TabProperties.TAB_CLOSED_LISTENER) .get(TabProperties.TAB_CLOSED_LISTENER)
.run(mModel.get(1).get(TabProperties.TAB_ID)); .run(mModel.get(1).get(TabProperties.TAB_ID));
verify(mTabModel).closeTab(eq(mTab2), eq(false), eq(false), eq(true)); verify(mTabModel).closeTab(eq(mTab2), eq(null), eq(false), eq(false), eq(true));
} }
@Test @Test
......
...@@ -110,6 +110,12 @@ public class EmptyTabModel implements TabModel { ...@@ -110,6 +110,12 @@ public class EmptyTabModel implements TabModel {
return false; return false;
} }
@Override
public boolean closeTab(
Tab tab, Tab recommendedNextTab, boolean animate, boolean uponExit, boolean canUndo) {
return closeTab(tab, animate, uponExit, canUndo);
}
@Override @Override
public TabList getComprehensiveModel() { public TabList getComprehensiveModel() {
return this; return this;
......
...@@ -131,6 +131,15 @@ public class IncognitoTabModel implements TabModel { ...@@ -131,6 +131,15 @@ public class IncognitoTabModel implements TabModel {
return retVal; return retVal;
} }
@Override
public boolean closeTab(
Tab tab, Tab recommendedNextTab, boolean animate, boolean uponExit, boolean canUndo) {
boolean retVal =
mDelegateModel.closeTab(tab, recommendedNextTab, animate, uponExit, canUndo);
destroyIncognitoIfNecessary();
return retVal;
}
@Override @Override
public Tab getNextTabIfClosed(int id) { public Tab getNextTabIfClosed(int id) {
return mDelegateModel.getNextTabIfClosed(id); return mDelegateModel.getNextTabIfClosed(id);
......
...@@ -99,6 +99,12 @@ public class SingleTabModel implements TabModel { ...@@ -99,6 +99,12 @@ public class SingleTabModel implements TabModel {
return false; return false;
} }
@Override
public boolean closeTab(
Tab tab, Tab recommendedNextTab, boolean animate, boolean uponExit, boolean canUndo) {
return closeTab(tab, animate, uponExit, canUndo);
}
/** /**
* In webapps, calls finish on the activity, but keeps it in recents. In Document mode, * In webapps, calls finish on the activity, but keeps it in recents. In Document mode,
* finishes and removes from recents. We use mBlockNewWindows flag to distinguish the user * finishes and removes from recents. We use mBlockNewWindows flag to distinguish the user
......
...@@ -4,6 +4,8 @@ ...@@ -4,6 +4,8 @@
package org.chromium.chrome.browser.tabmodel; package org.chromium.chrome.browser.tabmodel;
import android.support.annotation.Nullable;
import org.chromium.chrome.browser.profiles.Profile; import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.browser.tab.Tab; import org.chromium.chrome.browser.tab.Tab;
...@@ -44,6 +46,28 @@ public interface TabModel extends TabList { ...@@ -44,6 +46,28 @@ public interface TabModel extends TabList {
*/ */
public boolean closeTab(Tab tab, boolean animate, boolean uponExit, boolean canUndo); public boolean closeTab(Tab tab, boolean animate, boolean uponExit, boolean canUndo);
/**
* Unregisters and destroys the specified tab, and then switches to {@code recommendedNextTab}
* if it is not null, otherwise switches to the previous tab.
*
* @param tab The non-null tab to close.
* @param recommendedNextTab The tab to switch to if not null.
* @param animate true iff the closing animation should be displayed.
* @param uponExit true iff the tab is being closed upon application exit (after user presses
* the system back button).
* @param canUndo Whether or not this action can be undone. If this is {@code true} and
* {@link #supportsPendingClosures()} is {@code true}, this {@link Tab}
* will not actually be closed until {@link #commitTabClosure(int)} or
* {@link #commitAllTabClosures()} is called, but it will be effectively removed
* from this list. To get a comprehensive list of all tabs, including ones that
* have been partially closed, use the {@link TabList} from
* {@link #getComprehensiveModel()}.
*
* @return true if the tab was found.
*/
public boolean closeTab(Tab tab, @Nullable Tab recommendedNextTab, boolean animate,
boolean uponExit, boolean canUndo);
/** /**
* Returns which tab would be selected if the specified tab {@code id} were closed. * Returns which tab would be selected if the specified tab {@code id} were closed.
* @param id The ID of tab which would be closed. * @param id The ID of tab which would be closed.
......
...@@ -95,7 +95,7 @@ public class TabModelImpl extends TabModelJniBridge { ...@@ -95,7 +95,7 @@ public class TabModelImpl extends TabModelJniBridge {
@Override @Override
public void removeTab(Tab tab) { public void removeTab(Tab tab) {
removeTabAndSelectNext(tab, TabSelectionType.FROM_CLOSE, false, true); removeTabAndSelectNext(tab, null, TabSelectionType.FROM_CLOSE, false, true);
for (TabModelObserver obs : mObservers) obs.tabRemoved(tab); for (TabModelObserver obs : mObservers) obs.tabRemoved(tab);
} }
...@@ -348,7 +348,13 @@ public class TabModelImpl extends TabModelJniBridge { ...@@ -348,7 +348,13 @@ public class TabModelImpl extends TabModelJniBridge {
@Override @Override
public boolean closeTab(Tab tabToClose, boolean animate, boolean uponExit, boolean canUndo) { public boolean closeTab(Tab tabToClose, boolean animate, boolean uponExit, boolean canUndo) {
return closeTab(tabToClose, animate, uponExit, canUndo, canUndo); return closeTab(tabToClose, null, animate, uponExit, canUndo, canUndo);
}
@Override
public boolean closeTab(
Tab tab, Tab recommendedNextTab, boolean animate, boolean uponExit, boolean canUndo) {
return closeTab(tab, recommendedNextTab, animate, uponExit, canUndo, canUndo);
} }
/** /**
...@@ -359,8 +365,8 @@ public class TabModelImpl extends TabModelJniBridge { ...@@ -359,8 +365,8 @@ public class TabModelImpl extends TabModelJniBridge {
* closure. Observers will still be notified of a committed/cancelled closure * closure. Observers will still be notified of a committed/cancelled closure
* even if they are not notified of a pending closure to start with. * even if they are not notified of a pending closure to start with.
*/ */
private boolean closeTab(Tab tabToClose, boolean animate, boolean uponExit, private boolean closeTab(Tab tabToClose, Tab recommendedNextTab, boolean animate,
boolean canUndo, boolean notify) { boolean uponExit, boolean canUndo, boolean notify) {
if (tabToClose == null) { if (tabToClose == null) {
assert false : "Tab is null!"; assert false : "Tab is null!";
return false; return false;
...@@ -373,7 +379,7 @@ public class TabModelImpl extends TabModelJniBridge { ...@@ -373,7 +379,7 @@ public class TabModelImpl extends TabModelJniBridge {
canUndo &= supportsPendingClosures(); canUndo &= supportsPendingClosures();
startTabClosure(tabToClose, animate, uponExit, canUndo); startTabClosure(tabToClose, recommendedNextTab, animate, uponExit, canUndo);
if (notify && canUndo) { if (notify && canUndo) {
for (TabModelObserver obs : mObservers) obs.tabPendingClosure(tabToClose); for (TabModelObserver obs : mObservers) obs.tabPendingClosure(tabToClose);
} }
...@@ -390,7 +396,7 @@ public class TabModelImpl extends TabModelJniBridge { ...@@ -390,7 +396,7 @@ public class TabModelImpl extends TabModelJniBridge {
continue; continue;
} }
tab.setClosing(true); tab.setClosing(true);
closeTab(tab, false, false, canUndo, false); closeTab(tab, null, false, false, canUndo, false);
} }
if (canUndo && supportsPendingClosures()) { if (canUndo && supportsPendingClosures()) {
for (TabModelObserver obs : mObservers) obs.multipleTabsPendingClosure(tabs, false); for (TabModelObserver obs : mObservers) obs.multipleTabsPendingClosure(tabs, false);
...@@ -452,7 +458,7 @@ public class TabModelImpl extends TabModelJniBridge { ...@@ -452,7 +458,7 @@ public class TabModelImpl extends TabModelJniBridge {
while (getCount() > 0) { while (getCount() > 0) {
Tab tab = getTabAt(0); Tab tab = getTabAt(0);
closedTabs.add(tab); closedTabs.add(tab);
closeTab(tab, animate, uponExit, canUndo, false); closeTab(tab, null, animate, uponExit, canUndo, false);
} }
if (!uponExit && canUndo && supportsPendingClosures()) { if (!uponExit && canUndo && supportsPendingClosures()) {
...@@ -548,7 +554,8 @@ public class TabModelImpl extends TabModelJniBridge { ...@@ -548,7 +554,8 @@ public class TabModelImpl extends TabModelJniBridge {
* {@link #commitTabClosure(int)} or {@link #commitAllTabClosures()} needs to be * {@link #commitTabClosure(int)} or {@link #commitAllTabClosures()} needs to be
* called to actually delete and clean up {@code tab}. * called to actually delete and clean up {@code tab}.
*/ */
private void startTabClosure(Tab tab, boolean animate, boolean uponExit, boolean canUndo) { private void startTabClosure(
Tab tab, Tab recommendedNextTab, boolean animate, boolean uponExit, boolean canUndo) {
tab.setClosing(true); tab.setClosing(true);
for (TabModelObserver obs : mObservers) obs.willCloseTab(tab, animate); for (TabModelObserver obs : mObservers) obs.willCloseTab(tab, animate);
...@@ -557,14 +564,15 @@ public class TabModelImpl extends TabModelJniBridge { ...@@ -557,14 +564,15 @@ public class TabModelImpl extends TabModelJniBridge {
int selectionType = uponExit ? TabSelectionType.FROM_EXIT : TabSelectionType.FROM_CLOSE; int selectionType = uponExit ? TabSelectionType.FROM_EXIT : TabSelectionType.FROM_CLOSE;
boolean pauseMedia = canUndo; boolean pauseMedia = canUndo;
boolean updateRewoundList = !canUndo; boolean updateRewoundList = !canUndo;
removeTabAndSelectNext(tab, selectionType, pauseMedia, updateRewoundList); removeTabAndSelectNext(
tab, recommendedNextTab, selectionType, pauseMedia, updateRewoundList);
} }
/** /**
* Removes the given tab from the tab model and selects a new tab. * Removes the given tab from the tab model and selects a new tab.
*/ */
private void removeTabAndSelectNext(Tab tab, @TabSelectionType int selectionType, private void removeTabAndSelectNext(Tab tab, Tab recommendedNextTab,
boolean pauseMedia, boolean updateRewoundList) { @TabSelectionType int selectionType, boolean pauseMedia, boolean updateRewoundList) {
assert selectionType == TabSelectionType.FROM_CLOSE assert selectionType == TabSelectionType.FROM_CLOSE
|| selectionType == TabSelectionType.FROM_EXIT; || selectionType == TabSelectionType.FROM_EXIT;
...@@ -573,7 +581,8 @@ public class TabModelImpl extends TabModelJniBridge { ...@@ -573,7 +581,8 @@ public class TabModelImpl extends TabModelJniBridge {
Tab currentTabInModel = TabModelUtils.getCurrentTab(this); Tab currentTabInModel = TabModelUtils.getCurrentTab(this);
Tab adjacentTabInModel = getTabAt(closingTabIndex == 0 ? 1 : closingTabIndex - 1); Tab adjacentTabInModel = getTabAt(closingTabIndex == 0 ? 1 : closingTabIndex - 1);
Tab nextTab = getNextTabIfClosed(closingTabId); Tab nextTab =
recommendedNextTab == null ? getNextTabIfClosed(closingTabId) : recommendedNextTab;
// TODO(dtrainor): Update the list of undoable tabs instead of committing it. // TODO(dtrainor): Update the list of undoable tabs instead of committing it.
if (updateRewoundList) commitAllTabClosures(); if (updateRewoundList) commitAllTabClosures();
......
...@@ -265,6 +265,12 @@ public class DocumentTabModelImpl extends TabModelJniBridge implements DocumentT ...@@ -265,6 +265,12 @@ public class DocumentTabModelImpl extends TabModelJniBridge implements DocumentT
return false; return false;
} }
@Override
public boolean closeTab(
Tab tab, Tab recommendedNextTab, boolean animate, boolean uponExit, boolean canUndo) {
return closeTab(tab, animate, uponExit, canUndo);
}
@Override @Override
protected TabDelegate getTabCreator(boolean incognito) { protected TabDelegate getTabCreator(boolean incognito) {
return null; return null;
......
...@@ -43,6 +43,12 @@ public class MockDocumentTabModel implements DocumentTabModel { ...@@ -43,6 +43,12 @@ public class MockDocumentTabModel implements DocumentTabModel {
return false; return false;
} }
@Override
public boolean closeTab(
Tab tab, Tab recommendedNextTab, boolean animate, boolean uponExit, boolean canUndo) {
return closeTab(tab, animate, uponExit, canUndo);
}
@Override @Override
public Tab getNextTabIfClosed(int id) { public Tab getNextTabIfClosed(int id) {
Assert.fail(); Assert.fail();
......
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