Commit b2137d11 authored by Theresa's avatar Theresa Committed by Commit Bot

[Multi-instance] Fix merging after process killed in background

An M69 change broke an edge case for multi-instance tab model merging
(see http://crrev.com/c/1153438). This CL updates TabWindowManager to
merge tabs into ChromeTabbedActivity2 when creating a new
TabPersistencePolicy iff there are no other currently assigned tab model
selectors and the activity is not in multi-window mode.

Also adds a test to help prevent future regressions.

BUG=982305

Change-Id: Ib43b1de13aea654d0f2a2e5c14021ee09713ab66
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1693129Reviewed-by: default avatarMatthew Jones <mdjones@chromium.org>
Reviewed-by: default avatarTheresa <twellington@chromium.org>
Commit-Queue: Theresa <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#676078}
parent 6445fc63
......@@ -5,6 +5,7 @@
package org.chromium.chrome.browser.tabmodel;
import android.app.Activity;
import android.os.Build;
import android.util.SparseArray;
import org.chromium.base.ActivityState;
......@@ -12,8 +13,8 @@ import org.chromium.base.ApplicationStatus;
import org.chromium.base.ApplicationStatus.ActivityStateListener;
import org.chromium.base.ThreadUtils;
import org.chromium.base.VisibleForTesting;
import org.chromium.chrome.browser.ChromeTabbedActivity;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.util.FeatureUtilities;
import org.chromium.ui.base.WindowAndroid;
import java.util.ArrayList;
......@@ -207,11 +208,12 @@ public class TabWindowManager implements ActivityStateListener {
@Override
public TabModelSelector buildSelector(Activity activity,
TabCreatorManager tabCreatorManager, int selectorIndex) {
// Merge tabs if this is the TabModelSelector for ChromeTabbedActivity and there are no
// other instances running. This indicates that it is a complete cold start of
// ChromeTabbedActivity. Tabs should only be merged during a cold start of
// ChromeTabbedActivity and not other instances (e.g. ChromeTabbedActivity2).
boolean mergeTabs = activity.getClass().equals(ChromeTabbedActivity.class)
// Merge tabs if this TabModelSelector is for a ChromeTabbedActivity created in
// fullscreen mode and there are no TabModelSelector's currently alive. This indicates
// that it is a cold start or process restart in fullscreen mode.
boolean mergeTabs = Build.VERSION.SDK_INT > Build.VERSION_CODES.M
&& FeatureUtilities.isTabModelMergingEnabled()
&& !activity.isInMultiWindowMode()
&& getInstance().getNumberOfAssignedTabModelSelectors() == 0;
TabPersistencePolicy persistencePolicy = new TabbedModeTabPersistencePolicy(
selectorIndex, mergeTabs);
......
......@@ -24,6 +24,7 @@ import org.chromium.base.ActivityState;
import org.chromium.base.ApplicationStatus;
import org.chromium.base.ApplicationStatus.ActivityStateListener;
import org.chromium.base.ContextUtils;
import org.chromium.base.test.util.CallbackHelper;
import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.base.test.util.Feature;
import org.chromium.base.test.util.MinAndroidSdkLevel;
......@@ -31,6 +32,7 @@ import org.chromium.base.test.util.Restriction;
import org.chromium.base.test.util.UrlUtils;
import org.chromium.chrome.browser.ChromeSwitches;
import org.chromium.chrome.browser.ChromeTabbedActivity;
import org.chromium.chrome.browser.ChromeTabbedActivity2;
import org.chromium.chrome.browser.multiwindow.MultiWindowTestHelper;
import org.chromium.chrome.browser.multiwindow.MultiWindowUtils;
import org.chromium.chrome.browser.tabmodel.TabPersistentStoreTest.MockTabPersistentStoreObserver;
......@@ -44,12 +46,14 @@ import org.chromium.content_public.browser.test.util.CriteriaHelper;
import org.chromium.content_public.browser.test.util.TestThreadUtils;
import org.chromium.ui.test.util.UiRestriction;
import java.util.concurrent.TimeoutException;
/**
* Tests merging tab models for Android N+ multi-instance.
*/
@RunWith(ChromeJUnit4ClassRunner.class)
@CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE})
@TargetApi(Build.VERSION_CODES.LOLLIPOP)
@TargetApi(Build.VERSION_CODES.N)
@MinAndroidSdkLevel(Build.VERSION_CODES.N)
public class TabModelMergingTest {
@Rule
......@@ -71,6 +75,9 @@ public class TabModelMergingTest {
private String[] mMergeIntoActivity1ExpectedTabs;
private String[] mMergeIntoActivity2ExpectedTabs;
CallbackHelper mNewCTA2CallbackHelper = new CallbackHelper();
private ChromeTabbedActivity2 mNewCTA2;
@Before
public void setUp() throws Exception {
mActivityTestRule.startMainActivityOnBlankPage();
......@@ -113,6 +120,10 @@ public class TabModelMergingTest {
mActivity1State = newState;
} else if (activity.equals(mActivity2)) {
mActivity2State = newState;
} else if (activity instanceof ChromeTabbedActivity2
&& newState == ActivityState.CREATED) {
mNewCTA2 = (ChromeTabbedActivity2) activity;
mNewCTA2CallbackHelper.notifyCalled();
}
}
});
......@@ -255,7 +266,7 @@ public class TabModelMergingTest {
@Test
@LargeTest
@Feature({"TabPersistentStore", "MultiWindow"})
public void testMergeIntoChromeTabbedActivity1() throws Exception {
public void testMergeIntoChromeTabbedActivity1() {
mergeTabsAndAssert(mActivity1, mMergeIntoActivity1ExpectedTabs);
mActivity1.finishAndRemoveTask();
}
......@@ -263,7 +274,7 @@ public class TabModelMergingTest {
@Test
@LargeTest
@Feature({"TabPersistentStore", "MultiWindow"})
public void testMergeIntoChromeTabbedActivity2() throws Exception {
public void testMergeIntoChromeTabbedActivity2() {
mergeTabsAndAssert(mActivity2, mMergeIntoActivity2ExpectedTabs);
mActivity2.finishAndRemoveTask();
}
......@@ -271,7 +282,7 @@ public class TabModelMergingTest {
@Test
@LargeTest
@Feature({"TabPersistentStore", "MultiWindow"})
public void testMergeOnColdStart() throws Exception {
public void testMergeOnColdStart() {
String expectedSelectedUrl = mActivity1.getTabModelSelector().getCurrentTab().getUrl();
// Create an intent to launch a new ChromeTabbedActivity.
......@@ -333,7 +344,7 @@ public class TabModelMergingTest {
// Destroy ChromeTabbedActivity2. ChromeTabbedActivity should have been destroyed during the
// merge.
mActivity2.finishAndRemoveTask();
CriteriaHelper.pollUiThread(new Criteria("Both activitie should be destroyed."
CriteriaHelper.pollUiThread(new Criteria("Both activities should be destroyed."
+ "CTA state: " + mActivity1State + " - CTA2State: " + mActivity2State) {
@Override
public boolean isSatisfied() {
......@@ -350,11 +361,66 @@ public class TabModelMergingTest {
newActivity.finishAndRemoveTask();
}
@Test
@LargeTest
@Feature({"TabPersistentStore", "MultiWindow"})
public void testMergeOnColdStartIntoChromeTabbedActivity2()
throws TimeoutException, InterruptedException {
String CTA2ClassName = mActivity2.getClass().getName();
String CTA2PackageName = mActivity2.getPackageName();
TestThreadUtils.runOnUiThreadBlocking(() -> {
mActivity1.saveState();
mActivity2.saveState();
});
// Destroy both activities without removing tasks.
mActivity1.finish();
mActivity2.finish();
CriteriaHelper.pollUiThread(new Criteria("Both activities should be destroyed."
+ "CTA state: " + mActivity1State + " - CTA2State: " + mActivity2State) {
@Override
public boolean isSatisfied() {
return mActivity1State == ActivityState.DESTROYED
&& mActivity2State == ActivityState.DESTROYED;
}
});
// Send a main intent to restart ChromeTabbedActivity2.
Intent CTA2MainIntent = new Intent(Intent.ACTION_MAIN);
CTA2MainIntent.setClassName(CTA2PackageName, CTA2ClassName);
InstrumentationRegistry.getInstrumentation().startActivitySync(CTA2MainIntent);
mNewCTA2CallbackHelper.waitForCallback(0);
CriteriaHelper.pollUiThread(new Criteria("CTA2 tab state failed to initialize.") {
@Override
public boolean isSatisfied() {
return mNewCTA2.areTabModelsInitialized()
&& mNewCTA2.getTabModelSelector().isTabStateInitialized();
}
});
// Check that a merge occurred.
Assert.assertEquals("Wrong number of tabs after restart.",
mMergeIntoActivity2ExpectedTabs.length,
mNewCTA2.getTabModelSelector().getModel(false).getCount());
// TODO(twellington): When manually testing with "Don't keep activities" turned on in
// developer settings, tabs are merged in the right order. In this test, however, the
// order isn't quite as expected. Investigate replacing #finish() with something that
// better simulates the activity being killed in the background due to OOM.
// Clean up.
mNewCTA2.finishAndRemoveTask();
}
@Test
@LargeTest
@Feature({"TabPersistentStore", "MultiWindow"})
@Restriction({UiRestriction.RESTRICTION_TYPE_PHONE, RESTRICTION_TYPE_NON_LOW_END_DEVICE})
public void testMergeWhileInTabSwitcher() throws Exception {
public void testMergeWhileInTabSwitcher() {
OverviewModeBehaviorWatcher overviewModeWatcher = new OverviewModeBehaviorWatcher(
mActivity1.getLayoutManager(), true, false);
TestThreadUtils.runOnUiThreadBlocking(
......
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