Skip to content

GitLab

T
tangled
Commit 87ef6569 authored by Brandon Wylie's avatar Brandon Wylie Committed by Commit Bot
Browse files
Options

Add tab reparenting support to the TabModel/Selector classes 

Bug: 1055228
Change-Id: Icac03f005a378c5b2f15b56641f3c754817fdb47
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2123313
Commit-Queue: Brandon Wylie <wylieb@chromium.org>
Reviewed-by: default avatarTed Choc <tedchoc@chromium.org>
Reviewed-by: default avatarYusuf Ozuysal <yusufo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#755145}
parent 71e8ceb9
Hide whitespace changes
Inline Side-by-side
Showing with 108 additions and 245 deletions
chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java
View file @ 87ef6569
......@@ -1340,11 +1340,7 @@ public abstract class ChromeActivity<C extends ChromeActivityComponent>
if (ChromeFeatureList.isEnabled(ChromeFeatureList.ANDROID_NIGHT_MODE_TAB_REPARENTING)) {
mNightModeReparentingController = new NightModeReparentingController(
ReparentingDelegateFactory.createNightModeReparentingControllerDelegate(
getActivityTabProvider(), getTabModelSelector()),
ReparentingDelegateFactory.createReparentingTaskDelegate(
getCompositorViewHolder(), getWindowAndroid(),
getTabDelegateFactory()));
mNightModeReparentingController.onNativeInitialized();
getActivityTabProvider(), getTabModelSelector()));
}
}
......
chrome/android/java/src/org/chromium/chrome/browser/night_mode/NightModeReparentingController.java
View file @ 87ef6569
......@@ -4,16 +4,11 @@
package org.chromium.chrome.browser.night_mode;
import android.util.SparseArray;
import androidx.annotation.NonNull;
import org.chromium.chrome.browser.ActivityTabProvider;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tab.TabCreationState;
import org.chromium.chrome.browser.tab.TabLaunchType;
import org.chromium.chrome.browser.tab_activity_glue.ReparentingTask;
import org.chromium.chrome.browser.tabmodel.AsyncTabParams;
import org.chromium.chrome.browser.tabmodel.AsyncTabParamsManager;
import org.chromium.chrome.browser.tabmodel.TabList;
import org.chromium.chrome.browser.tabmodel.TabModel;
......@@ -21,9 +16,7 @@ import org.chromium.chrome.browser.tabmodel.TabModelSelector;
import org.chromium.chrome.browser.tabmodel.TabReparentingParams;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
/** Controls the reparenting of tabs when the theme is swapped. */
public class NightModeReparentingController implements NightModeStateProvider.Observer {
......@@ -40,119 +33,45 @@ public class NightModeReparentingController implements NightModeStateProvider.Ob
}
private Delegate mDelegate;
private ReparentingTask.Delegate mReparentingDelegate;
/** Constructs a {@link NightModeReparentingController} with the given delegate. */
public NightModeReparentingController(
@NonNull Delegate delegate, @NonNull ReparentingTask.Delegate reparentingDelegate) {
public NightModeReparentingController(@NonNull Delegate delegate) {
mDelegate = delegate;
mReparentingDelegate = reparentingDelegate;
}
public void onNativeInitialized() {
// Iterate through the params stored in AsyncTabParams and find the tabs stored by
// #onNightModeStateChanged. Reparent the background tabs and store the foreground tab
// to be reparented last.
TabReparentingParams foregroundTabParams = null;
SparseArray<AsyncTabParams> paramsArray = AsyncTabParamsManager.getAsyncTabParams().clone();
for (int i = 0; i < paramsArray.size(); i++) {
int tabId = paramsArray.keyAt(i);
AsyncTabParams params = paramsArray.get(tabId);
if (!(params instanceof TabReparentingParams)) continue;
final TabReparentingParams reparentingParams = (TabReparentingParams) params;
if (!reparentingParams.isFromNightModeReparenting()) continue;
if (!reparentingParams.hasTabToReparent()) continue;
final ReparentingTask reparentingTask =
ReparentingTask.get(reparentingParams.getTabToReparent());
if (reparentingTask == null) continue;
final TabModel tabModel = mDelegate.getTabModelSelector().getModel(
reparentingParams.getTabToReparent().isIncognito());
if (tabModel == null) {
AsyncTabParamsManager.remove(tabId);
return;
}
if (reparentingParams.isForegroundTab()) {
foregroundTabParams = reparentingParams;
} else {
reattachTab(reparentingTask, reparentingParams, tabModel);
}
}
if (foregroundTabParams == null) return;
final TabModel tabModel = mDelegate.getTabModelSelector().getModel(
foregroundTabParams.getTabToReparent().isIncognito());
if (tabModel == null) return;
ReparentingTask task = ReparentingTask.get(foregroundTabParams.getTabToReparent());
reattachTab(task, foregroundTabParams, tabModel);
}
/** Reattach the given task/pair to the activity/tab-model. */
private void reattachTab(ReparentingTask task, TabReparentingParams params, TabModel tabModel) {
final Tab tab = params.getTabToReparent();
task.finish(mReparentingDelegate, () -> {
int tabIndex = params.getTabIndex() > tabModel.getCount() ? -1 : params.getTabIndex();
tabModel.addTab(tab, tabIndex, TabLaunchType.FROM_REPARENTING,
TabCreationState.LIVE_IN_FOREGROUND);
AsyncTabParamsManager.remove(tab.getId());
});
}
@Override
public void onNightModeStateChanged() {
Tab foregroundTab = mDelegate.getActivityTabProvider().get();
Map<TabModel, List<Tab>> tabsAndModels = getTabModelAndTabs();
for (final TabModel model : tabsAndModels.keySet()) {
List<Tab> tabs = tabsAndModels.get(model);
for (int i = 0; i < tabs.size(); i++) {
Tab tab = tabs.get(i);
// Intentionally skip new tab pages and allow them to reload and restore scroll
// state themselves.
if (mDelegate.isNTPUrl(tab.getUrlString())) continue;
TabReparentingParams params = new TabReparentingParams(tab, null, null);
params.setFromNightModeReparenting(true);
params.setIsForegroundTab(tab == foregroundTab);
params.setTabIndex(i);
AsyncTabParamsManager.add(tab.getId(), params);
ReparentingTask.from(tab).detach();
}
// TODO(crbug.com/1065201): Make tab models detachable.
TabModelSelector selector = mDelegate.getTabModelSelector();
// Close tabs pending closure before saving params.
selector.getModel(false).commitAllTabClosures();
selector.getModel(true).commitAllTabClosures();
// Aggregate all the tabs.
List<Tab> tabs = new ArrayList<>(selector.getTotalTabCount());
populateComprehensiveTabsFromModel(selector.getModel(false), tabs);
populateComprehensiveTabsFromModel(selector.getModel(true), tabs);
// Save all the tabs in memory to be retrieved after restart.
mDelegate.getTabModelSelector().enterReparentingMode();
for (int i = 0; i < tabs.size(); i++) {
Tab tab = tabs.get(i);
// Intentionally skip new tab pages and allow them to reload and restore scroll
// state themselves.
if (mDelegate.isNTPUrl(tab.getUrlString())) continue;
TabReparentingParams params = new TabReparentingParams(tab, null, null);
AsyncTabParamsManager.add(tab.getId(), params);
ReparentingTask.from(tab).detach();
}
// Save the current state to overwrite tab state that was previously saved to disk. See
// crbug.com/1042565 for more info.
mDelegate.getTabModelSelector().saveState();
}
/** @return Mapping from TabModel to all the Tabs in that model. */
Map<TabModel, List<Tab>> getTabModelAndTabs() {
Map<TabModel, List<Tab>> tabsAndModels = new HashMap<>();
TabModel model = mDelegate.getTabModelSelector().getModel(false);
tabsAndModels.put(model, getTabsForModel(model));
model = mDelegate.getTabModelSelector().getModel(true);
tabsAndModels.put(model, getTabsForModel(model));
return tabsAndModels;
}
/** @return All of a model's Tabs as a List. */
List<Tab> getTabsForModel(TabModel model) {
List<Tab> tabs = new ArrayList<>();
if (model == null) return tabs;
static void populateComprehensiveTabsFromModel(TabModel model, List<Tab> outputTabs) {
TabList tabList = model.getComprehensiveModel();
for (int i = 0; i < tabList.getCount(); i++) {
tabs.add(tabList.getTabAt(i));
outputTabs.add(tabList.getTabAt(i));
}
return tabs;
}
}
chrome/android/java/src/org/chromium/chrome/browser/tabmodel/ChromeTabCreator.java
View file @ 87ef6569
......@@ -351,7 +351,28 @@ public class ChromeTabCreator extends TabCreatorManager.TabCreator {
Tab parent = selector != null ? selector.getTabById(state.parentId) : null;
boolean selectTab = mOrderController.willOpenInForeground(
TabLaunchType.FROM_RESTORE, state.isIncognito());
Tab tab = TabBuilder.createFromFrozenState()
AsyncTabParams asyncParams = AsyncTabParamsManager.remove(id);
Tab tab = null;
@TabLaunchType
int launchType = TabLaunchType.FROM_RESTORE;
@TabCreationState
int creationState = TabCreationState.FROZEN_ON_RESTORE;
if (asyncParams != null && asyncParams.getTabToReparent() != null) {
creationState = TabCreationState.LIVE_IN_BACKGROUND;
TabReparentingParams params = (TabReparentingParams) asyncParams;
tab = params.getTabToReparent();
if (tab.isIncognito() != state.isIncognito()) {
throw new IllegalStateException("Incognito state mismatch");
}
ReparentingTask.from(tab).finish(
ReparentingDelegateFactory.createReparentingTaskDelegate(
mActivity.getCompositorViewHolder(), mActivity.getWindowAndroid(),
createDefaultTabDelegateFactory()),
params.getFinalizeCallback());
}
if (tab == null) {
tab = TabBuilder.createFromFrozenState()
.setId(id)
.setParent(parent)
.setIncognito(state.isIncognito())
......@@ -360,9 +381,9 @@ public class ChromeTabCreator extends TabCreatorManager.TabCreator {
.setInitiallyHidden(!selectTab)
.setTabState(state)
.build();
}
assert state.isIncognito() == mIncognito;
mTabModel.addTab(
tab, index, TabLaunchType.FROM_RESTORE, TabCreationState.FROZEN_ON_RESTORE);
mTabModel.addTab(tab, index, launchType, creationState);
return tab;
}
......
chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabModelDelegate.java
View file @ 87ef6569
......@@ -32,6 +32,11 @@ public interface TabModelDelegate {
*/
boolean isCurrentModel(TabModel model);
/**
* @return Whether reparenting is currently in progress for this TabModel.
*/
boolean isReparentingInProgress();
// TODO(aurimas): clean these methods up.
TabModel getCurrentModel();
TabModel getModel(boolean incognito);
......
chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabModelImpl.java
View file @ 87ef6569
......@@ -106,8 +106,11 @@ public class TabModelImpl extends TabModelJniBridge {
@Override
public void destroy() {
for (Tab tab : mTabs) {
if (tab.isInitialized()) tab.destroy();
// When reparenting tabs, only commit that tab closures and keep the rest of the tabs alive.
if (!mModelDelegate.isReparentingInProgress()) {
for (Tab tab : mTabs) {
if (tab.isInitialized()) tab.destroy();
}
}
mRewoundList.destroy();
......@@ -752,6 +755,8 @@ public class TabModelImpl extends TabModelJniBridge {
* before destroying it.
*/
public void destroy() {
// All tabs pending closure are committed in TabModelImpl#destroy.
if (TabModelImpl.this.mModelDelegate.isReparentingInProgress()) return;
for (Tab tab : mRewoundTabs) {
if (tab.isInitialized()) tab.destroy();
}
......
chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabModelSelector.java
View file @ 87ef6569
......@@ -163,12 +163,6 @@ public interface TabModelSelector {
*/
void commitAllTabClosures();
/**
* Save the current state of the tab model. Usage of this method is discouraged due to it
* writing to disk.
**/
void saveState();
/**
* Sets the delegate to handle the requests to close tabs in a single model.
* @param delegate The delegate to be used.
......@@ -192,6 +186,15 @@ public interface TabModelSelector {
*/
void mergeState();
/**
* Prevents the TabModelSelector from destroying its tabs to allow for reparenting.
*
* This is only safe to be called immediately before destruction. After entering reparenting
* mode, all the tabs are removed and stored in memory and on disk. The app is recreated right
* after, so there should never be a need to "exit" reparenting mode.
*/
void enterReparentingMode();
/**
* Destroy all owned {@link TabModel}s and {@link Tab}s referenced by this selector.
*/
......
chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabModelSelectorBase.java
View file @ 87ef6569
......@@ -36,6 +36,7 @@ public abstract class TabModelSelectorBase implements TabModelSelector {
private final ObserverList<TabModelSelectorObserver> mObservers = new ObserverList<>();
private boolean mTabStateInitialized;
private boolean mStartIncognito;
private boolean mReparentingInProgress;
private final TabCreatorManager mTabCreatorManager;
......@@ -194,9 +195,6 @@ public abstract class TabModelSelectorBase implements TabModelSelector {
}
}
@Override
public void saveState() {}
@Override
public Tab getTabById(int id) {
for (int i = 0; i < getModels().size(); i++) {
......@@ -291,4 +289,14 @@ public abstract class TabModelSelectorBase implements TabModelSelector {
protected TabCreatorManager getTabCreatorManager() {
return mTabCreatorManager;
}
@Override
public void enterReparentingMode() {
mReparentingInProgress = true;
}
/** @see TabModelDelegate#isReparentingInProgress */
protected boolean isReparentingInProgress() {
return mReparentingInProgress;
}
}
chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabModelSelectorImpl.java
View file @ 87ef6569
......@@ -193,7 +193,9 @@ public class TabModelSelectorImpl extends TabModelSelectorBase implements TabMod
@Override
public void onActivityAttachmentChanged(Tab tab, boolean attached) {
if (!attached) getModel(tab.isIncognito()).removeTab(tab);
if (!attached && !isReparentingInProgress()) {
getModel(tab.isIncognito()).removeTab(tab);
}
}
@Override
......@@ -263,7 +265,10 @@ public class TabModelSelectorImpl extends TabModelSelectorBase implements TabMod
return isIncognitoSelected() == model.isIncognito();
}
@Override
/**
* Save the current state of the tab model. Usage of this method is discouraged due to it
* writing to disk.
*/
public void saveState() {
commitAllTabClosures();
mTabSaver.saveState();
......@@ -393,6 +398,11 @@ public class TabModelSelectorImpl extends TabModelSelectorBase implements TabMod
return mSessionRestoreInProgress.get();
}
@Override
public boolean isReparentingInProgress() {
return super.isReparentingInProgress();
}
// TODO(tedchoc): Remove the need for this to be exposed.
@Override
public void notifyChanged() {
......
chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabReparentingParams.java
View file @ 87ef6569
......@@ -23,10 +23,6 @@ public class TabReparentingParams implements AsyncTabParams {
private final Intent mOriginalIntent;
private final Runnable mFinalizeCallback;
private int mTabIndex = TAB_INDEX_NOT_SET;
private boolean mIsFromNightModeReparenting;
private boolean mIsForegroundTab;
/**
* Basic constructor for {@link TabReparentingParams}.
*/
......@@ -82,36 +78,4 @@ public class TabReparentingParams implements AsyncTabParams {
public void destroy() {
if (mTabToReparent != null) mTabToReparent.destroy();
}
// Night mode reparenting implementation.
/** Set the tab index for later retrieval. */
public void setTabIndex(int tabIndex) {
mTabIndex = tabIndex;
}
/** @return Index of the stored tab. */
public int getTabIndex() {
return mTabIndex;
}
/** Set whether these params are from night mode reparenting. */
public void setFromNightModeReparenting(boolean fromNightModeReparenting) {
mIsFromNightModeReparenting = fromNightModeReparenting;
}
/** @return Whether these params are from night mode reparenting. */
public boolean isFromNightModeReparenting() {
return mIsFromNightModeReparenting;
}
/** Set whether this is the foreground tab. */
public void setIsForegroundTab(boolean isForegroundTab) {
mIsForegroundTab = isForegroundTab;
}
/** @return Whether this is a foreground tab. */
public boolean isForegroundTab() {
return mIsForegroundTab;
}
}
chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/TabModelSelectorObserverTestRule.java
View file @ 87ef6569
......@@ -123,6 +123,12 @@ public class TabModelSelectorObserverTestRule extends ChromeBrowserTestRule {
public boolean closeAllTabsRequest(boolean incognito) {
return false;
}
@Override
public boolean isReparentingInProgress() {
return false;
}
};
mNormalTabModel = new TabModelSelectorTestTabModel(
false, orderController, tabContentManager, tabPersistentStore, delegate);
......
chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/TabPersistentStoreTest.java
View file @ 87ef6569
......@@ -235,6 +235,11 @@ public class TabPersistentStoreTest {
public boolean isCurrentModel(TabModel model) {
return false;
}
@Override
public boolean isReparentingInProgress() {
return super.isReparentingInProgress();
}
}
static class MockTabPersistentStoreObserver extends TabPersistentStoreObserver {
......
chrome/android/junit/src/org/chromium/chrome/browser/night_mode/NightModeReparentingControllerTest.java
View file @ 87ef6569
......@@ -4,8 +4,6 @@
package org.chromium.chrome.browser.night_mode;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.anyObject;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.times;
......@@ -84,8 +82,6 @@ public class NightModeReparentingControllerTest {
@Mock
ReparentingTask mTask;
@Mock
ReparentingTask.Delegate mReparentingTaskDelegate;
MockTabModel mTabModel;
MockTabModel mIncognitoTabModel;
......@@ -104,7 +100,7 @@ public class NightModeReparentingControllerTest {
mDelegate = new FakeNightModeReparentingDelegate();
mDelegate.setIsNTPUrl(false);
mController = new NightModeReparentingController(mDelegate, mReparentingTaskDelegate);
mController = new NightModeReparentingController(mDelegate);
}
@After
......@@ -124,18 +120,9 @@ public class NightModeReparentingControllerTest {
Assert.assertTrue(params instanceof TabReparentingParams);
TabReparentingParams trp = (TabReparentingParams) params;
Assert.assertEquals(
"The index of the first tab stored should match it's index in the tab stack.", 0,
trp.getTabIndex());
Assert.assertTrue(trp.isFromNightModeReparenting());
Tab tab = trp.getTabToReparent();
Assert.assertNotNull(tab);
Assert.assertEquals(1, tab.getId());
verify(mTask, times(1)).detach();
mController.onNativeInitialized();
verify(mTask, times(1)).finish(anyObject(), anyObject());
}
@Test
......@@ -147,18 +134,6 @@ public class NightModeReparentingControllerTest {
Assert.assertFalse(AsyncTabParamsManager.hasParamsWithTabToReparent());
}
@Test
public void testReparenting_singleTab_currentModelNullOnStart() {
mForegroundTab = createAndAddMockTab(1, false);
mController.onNightModeStateChanged();
doReturn(null).when(mDelegate.getTabModelSelector()).getModel(anyBoolean());
mController.onNativeInitialized();
AsyncTabParams params = AsyncTabParamsManager.getAsyncTabParams().get(1);
Assert.assertNull(params);
}
@Test
public void testReparenting_multipleTabs() {
mForegroundTab = createAndAddMockTab(1, false);
......@@ -167,29 +142,14 @@ public class NightModeReparentingControllerTest {
TabReparentingParams trp =
(TabReparentingParams) AsyncTabParamsManager.getAsyncTabParams().get(1);
Assert.assertEquals(
"The index of the first tab stored should match its index in the tab stack.", 0,
trp.getTabIndex());
Assert.assertTrue(trp.isFromNightModeReparenting());
Assert.assertTrue(trp.isForegroundTab());
Tab tab = trp.getTabToReparent();
Assert.assertNotNull(tab);
Assert.assertEquals(1, tab.getId());
trp = (TabReparentingParams) AsyncTabParamsManager.getAsyncTabParams().get(2);
Assert.assertEquals(1, trp.getTabIndex());
Assert.assertTrue(trp.isFromNightModeReparenting());
Assert.assertFalse(trp.isForegroundTab());
tab = trp.getTabToReparent();
Assert.assertNotNull(tab);
Assert.assertEquals(2, tab.getId());
verify(mTask, times(2)).detach();
mController.onNativeInitialized();
verify(mTask, times(2)).finish(anyObject(), anyObject());
}
@Test
......@@ -203,18 +163,10 @@ public class NightModeReparentingControllerTest {
Assert.assertTrue(params instanceof TabReparentingParams);
TabReparentingParams trp = (TabReparentingParams) params;
Assert.assertEquals(1, trp.getTabIndex());
Assert.assertTrue(trp.isFromNightModeReparenting());
Assert.assertTrue(trp.isForegroundTab());
Tab tab = trp.getTabToReparent();
Assert.assertNotNull(tab);
Assert.assertEquals(2, tab.getId());
verify(mTask, times(2)).detach();
mController.onNativeInitialized();
verify(mTask, times(2)).finish(anyObject(), anyObject());
}
@Test
......@@ -228,18 +180,11 @@ public class NightModeReparentingControllerTest {
Assert.assertTrue(params instanceof TabReparentingParams);
TabReparentingParams trp = (TabReparentingParams) params;
Assert.assertEquals(0, trp.getTabIndex());
Assert.assertTrue(trp.isFromNightModeReparenting());
Assert.assertTrue(trp.isForegroundTab());
Tab tab = trp.getTabToReparent();
Assert.assertNotNull(tab);
Assert.assertEquals(2, tab.getId());
verify(mTask, times(2)).detach();
mController.onNativeInitialized();
verify(mTask, times(2)).finish(anyObject(), anyObject());
}
@Test
......@@ -252,39 +197,15 @@ public class NightModeReparentingControllerTest {
// Check the foreground tab.
TabReparentingParams trp =
(TabReparentingParams) AsyncTabParamsManager.getAsyncTabParams().get(2);
Assert.assertEquals(1, trp.getTabIndex());
Assert.assertTrue(trp.isFromNightModeReparenting());
Assert.assertTrue(trp.isForegroundTab());
Tab tab = trp.getTabToReparent();
Assert.assertNotNull(tab);
Assert.assertEquals(2, tab.getId());
// Check the background tabs.
trp = (TabReparentingParams) AsyncTabParamsManager.getAsyncTabParams().get(1);
Assert.assertTrue(trp.isFromNightModeReparenting());
Assert.assertFalse(trp.isForegroundTab());
trp = (TabReparentingParams) AsyncTabParamsManager.getAsyncTabParams().get(3);
Assert.assertTrue(trp.isFromNightModeReparenting());
Assert.assertFalse(trp.isForegroundTab());
verify(mTask, times(3)).detach();
mController.onNativeInitialized();
verify(mTask, times(3)).finish(anyObject(), anyObject());
}
@Test
public void testTabGetsStored_noTab() {
try {
mController.onNightModeStateChanged();
mController.onNativeInitialized();
verify(mTask, times(0)).finish(anyObject(), anyObject());
} catch (Exception e) {
Assert.assertTrue(
"NightModeReparentingController shouldn't crash even when there's no tab!",
false);
}
}
/**
......
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