Commit c0efe30a authored by Matt Simmons's avatar Matt Simmons Committed by Commit Bot

Do not display the tab switcher until the tab models are restored.

If tab models are restored while the overview is visible, update the
tab switcher with the updated list.

R=wychen@chromium.org

Bug: 990982, 955436
Change-Id: Ia7c63e35624dd37cd7ff0063727ba64d040fe3ee
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1769298Reviewed-by: default avatarWei-Yin Chen (陳威尹) <wychen@chromium.org>
Reviewed-by: default avatarYusuf Ozuysal <yusufo@chromium.org>
Reviewed-by: default avatarMei Liang <meiliang@chromium.org>
Commit-Queue: Matt Simmons <mattsimmons@chromium.org>
Cr-Commit-Position: refs/heads/master@{#697858}
parent 91bb1f71
......@@ -106,6 +106,12 @@ public class StartSurfaceLayoutPerfTest {
mTabNumCap = 0;
}
assertTrue(FeatureUtilities.isTabToGtsAnimationEnabled());
CriteriaHelper.pollUiThread(Criteria.equals(true,
mActivityTestRule.getActivity()
.getTabModelSelector()
.getTabModelFilterProvider()
.getCurrentTabModelFilter()::isTabModelRestored));
}
@Test
......
......@@ -4,6 +4,10 @@
package org.chromium.chrome.features.start_surface;
import static android.support.test.espresso.Espresso.onView;
import static android.support.test.espresso.action.ViewActions.click;
import static android.support.test.espresso.matcher.ViewMatchers.withId;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertTrue;
......@@ -17,15 +21,16 @@ import android.animation.ValueAnimator;
import android.graphics.Bitmap;
import android.os.Build;
import android.provider.Settings;
import android.support.annotation.Nullable;
import android.support.test.InstrumentationRegistry;
import android.support.test.espresso.Espresso;
import android.support.test.espresso.action.ViewActions;
import android.support.test.espresso.NoMatchingViewException;
import android.support.test.espresso.ViewAssertion;
import android.support.test.espresso.contrib.RecyclerViewActions;
import android.support.test.espresso.matcher.ViewMatchers;
import android.support.test.filters.MediumTest;
import android.support.test.filters.SmallTest;
import android.support.v7.widget.RecyclerView;
import android.text.TextUtils;
import androidx.annotation.Nullable;
import android.view.View;
import org.junit.Assert;
import org.junit.Before;
......@@ -75,7 +80,6 @@ import java.util.List;
"force-fieldtrials=Study/Group"})
@Restriction(UiRestriction.RESTRICTION_TYPE_PHONE)
public class StartSurfaceLayoutTest {
private static final String TAG = "SSLayoutTest";
private static final String BASE_PARAMS = "force-fieldtrial-params="
+ "Study.Group:soft-cleanup-delay/0/cleanup-delay/0/skip-slow-zooming/false"
+ "/zooming-min-sdk-version/19/zooming-min-memory-mb/512";
......@@ -96,6 +100,7 @@ public class StartSurfaceLayoutTest {
@Before
public void setUp() throws InterruptedException {
FeatureUtilities.setGridTabSwitcherEnabledForTesting(true);
EmbeddedTestServer testServer =
EmbeddedTestServer.createAndStartServer(InstrumentationRegistry.getContext());
mActivityTestRule.startMainActivityFromLauncher();
......@@ -113,6 +118,12 @@ public class StartSurfaceLayoutTest {
mActivityTestRule.getActivity().getTabContentManager().setCaptureMinRequestTimeForTesting(
0);
CriteriaHelper.pollUiThread(Criteria.equals(true,
mActivityTestRule.getActivity()
.getTabModelSelector()
.getTabModelFilterProvider()
.getCurrentTabModelFilter()::isTabModelRestored));
}
@Test
......@@ -340,8 +351,9 @@ public class StartSurfaceLayoutTest {
if (mActivityTestRule.getActivity()
.getTabContentManager()
.getPendingReadbacksForTesting()
> 0)
> 0) {
break;
}
// Restart Chrome.
// Although we're destroying the activity, the Application will still live on since its
......@@ -415,9 +427,8 @@ public class StartSurfaceLayoutTest {
waitForCaptureRateControl();
}
int count = getCaptureCount();
Espresso.onView(ViewMatchers.withId(org.chromium.chrome.tab_ui.R.id.tab_list_view))
.perform(RecyclerViewActions.actionOnItemAtPosition(
targetIndex, ViewActions.click()));
onView(withId(org.chromium.chrome.tab_ui.R.id.tab_list_view))
.perform(RecyclerViewActions.actionOnItemAtPosition(targetIndex, click()));
CriteriaHelper.pollUiThread(() -> {
boolean doneHiding =
!mActivityTestRule.getActivity().getLayoutManager().overviewVisible();
......@@ -532,14 +543,57 @@ public class StartSurfaceLayoutTest {
Assert.assertEquals(0, mAllBitmaps.size() - count);
}
@Test
@SmallTest
@CommandLineFlags.Add({BASE_PARAMS})
public void testIncognitoEnterGts() throws Exception {
mActivityTestRule.newIncognitoTabFromMenu();
onView(withId(org.chromium.chrome.R.id.tab_switcher_button)).perform(click());
onView(withId(org.chromium.chrome.tab_ui.R.id.tab_list_view))
.check(TabCountAssertion.havingTabCount(1));
onView(withId(org.chromium.chrome.tab_ui.R.id.tab_list_view))
.perform(RecyclerViewActions.actionOnItemAtPosition(0, click()));
CriteriaHelper.pollInstrumentationThread(
() -> !mActivityTestRule.getActivity().getLayoutManager().overviewVisible());
onView(withId(org.chromium.chrome.R.id.tab_switcher_button)).perform(click());
onView(withId(org.chromium.chrome.tab_ui.R.id.tab_list_view))
.check(TabCountAssertion.havingTabCount(1));
}
private static class TabCountAssertion implements ViewAssertion {
private int mExpectedCount;
public static TabCountAssertion havingTabCount(int tabCount) {
return new TabCountAssertion(tabCount);
}
public TabCountAssertion(int expectedCount) {
mExpectedCount = expectedCount;
}
@Override
public void check(View view, NoMatchingViewException noMatchException) {
if (noMatchException != null) throw noMatchException;
RecyclerView.Adapter adapter = ((RecyclerView) view).getAdapter();
assertEquals(mExpectedCount, adapter.getItemCount());
}
}
private void enterGTS() throws InterruptedException {
Tab currentTab = mActivityTestRule.getActivity().getTabModelSelector().getCurrentTab();
// Native tabs need to be invalidated first to trigger thumbnail taking, so skip them.
boolean checkThumbnail = !currentTab.isNativePage();
if (checkThumbnail)
if (checkThumbnail) {
mActivityTestRule.getActivity().getTabContentManager().removeTabThumbnail(
currentTab.getId());
}
int count = getCaptureCount();
waitForCaptureRateControl();
......
......@@ -101,7 +101,7 @@ public class TabGroupModelFilter extends TabModelFilter {
* group.
*/
private class TabGroup {
private final static int INVALID_GROUP_ID = -1;
private static final int INVALID_GROUP_ID = -1;
private final Set<Integer> mTabIds;
private int mLastShownTabId;
private int mGroupId;
......@@ -174,7 +174,6 @@ public class TabGroupModelFilter extends TabModelFilter {
private int mActualGroupCount;
private Tab mAbsentSelectedTab;
private boolean mShouldRecordUma = true;
private boolean mTabRestoreCompleted;
private boolean mIsResetting;
public TabGroupModelFilter(TabModel tabModel) {
......@@ -188,7 +187,6 @@ public class TabGroupModelFilter extends TabModelFilter {
RecordHistogram.recordCountHistogram("TabGroups.UserGroupCount", mActualGroupCount);
Tab currentTab = TabModelUtils.getCurrentTab(getTabModel());
if (currentTab != null) recordSessionsCount(currentTab);
mTabRestoreCompleted = true;
removeObserver(this);
}
});
......@@ -654,7 +652,7 @@ public class TabGroupModelFilter extends TabModelFilter {
public void didMoveTab(Tab tab, int newIndex, int curIndex) {
// Ignore didMoveTab calls in tab restoring stage. For incognito mode, bypass this check
// since there is no restoring stage.
if (!mTabRestoreCompleted && !isIncognito()) return;
if (!isTabModelRestored()) return;
// Need to cache the flags before resetting the internal data map.
boolean isMergeTabToGroup = isMergeTabToGroup(tab);
boolean isMoveTabOutOfGroup = isMoveTabOutOfGroup(tab);
......
......@@ -739,7 +739,13 @@ class TabListMediator {
* The selected border should re-appear in the final fading-in stage.
*/
void prepareOverview() {
if (!FeatureUtilities.isTabToGtsAnimationEnabled()) return;
if (!FeatureUtilities.isTabToGtsAnimationEnabled()
|| !mTabModelSelector.getTabModelFilterProvider()
.getCurrentTabModelFilter()
.isTabModelRestored()) {
return;
}
assert mVisible;
int count = 0;
for (int i = 0; i < mModel.size(); i++) {
......@@ -816,7 +822,7 @@ class TabListMediator {
}
/**
* @see GridTabSwitcherMediator.ResetHandler#softCleanup
* @see TabSwitcherMediator.ResetHandler#softCleanup
*/
void softCleanup() {
assert !mVisible;
......
......@@ -119,7 +119,7 @@ class TabSwitcherMediator implements TabSwitcher.Controller, TabListRecyclerView
private boolean mShouldIgnoreNextSelect;
private int mModelIndexWhenShown;
private int mTabIdwhenShown;
private int mTabIdWhenShown;
private int mIndexInNewModelWhenSwitched;
private boolean mIsSelectingInTabSwitcher;
......@@ -225,6 +225,15 @@ class TabSwitcherMediator implements TabSwitcher.Controller, TabListRecyclerView
onTabSelecting(tab.getId());
}
}
@Override
public void restoreCompleted() {
if (!mContainerViewModel.get(IS_VISIBLE)) return;
mResetHandler.resetWithTabList(
mTabModelSelector.getTabModelFilterProvider().getCurrentTabModelFilter(),
false, mShowTabsInMruOrder);
}
};
mFullscreenManager.addListener(mFullscreenListener);
......@@ -335,7 +344,7 @@ class TabSwitcherMediator implements TabSwitcher.Controller, TabListRecyclerView
Tab fromTab = TabModelUtils.getTabById(mTabModelSelector.getCurrentModel(), lastId);
assert fromTab != null;
if (mModelIndexWhenShown == mTabModelSelector.getCurrentModelIndex()) {
if (tab.getId() == mTabIdwhenShown) {
if (tab.getId() == mTabIdWhenShown) {
RecordUserAction.record("MobileTabReturnedToCurrentTab");
RecordHistogram.recordSparseHistogram(
"Tabs.TabOffsetOfSwitch." + TabSwitcherCoordinator.COMPONENT_NAME, 0);
......@@ -411,7 +420,10 @@ class TabSwitcherMediator implements TabSwitcher.Controller, TabListRecyclerView
mHandler.removeCallbacks(mSoftClearTabListRunnable);
mHandler.removeCallbacks(mClearTabListRunnable);
boolean quick = false;
if (FeatureUtilities.isTabToGtsAnimationEnabled()) {
if (FeatureUtilities.isTabToGtsAnimationEnabled()
&& mTabModelSelector.getTabModelFilterProvider()
.getCurrentTabModelFilter()
.isTabModelRestored()) {
quick = mResetHandler.resetWithTabList(
mTabModelSelector.getTabModelFilterProvider().getCurrentTabModelFilter(), false,
mShowTabsInMruOrder);
......@@ -429,13 +441,17 @@ class TabSwitcherMediator implements TabSwitcher.Controller, TabListRecyclerView
@Override
public void showOverview(boolean animate) {
mResetHandler.resetWithTabList(
mTabModelSelector.getTabModelFilterProvider().getCurrentTabModelFilter(),
FeatureUtilities.isTabToGtsAnimationEnabled(), mShowTabsInMruOrder);
if (mTabModelSelector.getTabModelFilterProvider()
.getCurrentTabModelFilter()
.isTabModelRestored()) {
mResetHandler.resetWithTabList(
mTabModelSelector.getTabModelFilterProvider().getCurrentTabModelFilter(),
FeatureUtilities.isTabToGtsAnimationEnabled(), mShowTabsInMruOrder);
}
if (!animate) mContainerViewModel.set(ANIMATE_VISIBILITY_CHANGES, false);
setVisibility(true);
mModelIndexWhenShown = mTabModelSelector.getCurrentModelIndex();
mTabIdwhenShown = mTabModelSelector.getCurrentTabId();
mTabIdWhenShown = mTabModelSelector.getCurrentTabId();
mContainerViewModel.set(ANIMATE_VISIBILITY_CHANGES, true);
if (mIphProvider != null) {
mIphProvider.maybeShowIPH(mTabModelSelector.isIncognitoSelected());
......
......@@ -271,6 +271,23 @@ public class TabGroupModelFilterUnitTest {
assertThat(mTabModel.getCount(), equalTo(6));
}
@Test
// TODO(mattsimmons): This is actually testing behavior of the superclass but there's no unit
// tests for the superclass today. If one ever exists, this should move to that test.
public void isTabModelRestored() {
setupTabGroupModelFilter(false, false);
assertThat(mTabGroupModelFilter.isTabModelRestored(), equalTo(false));
setupTabGroupModelFilter(true, false);
assertThat(mTabGroupModelFilter.isTabModelRestored(), equalTo(true));
setupTabGroupModelFilter(false, true);
assertThat(mTabGroupModelFilter.isTabModelRestored(), equalTo(true));
setupTabGroupModelFilter(true, true);
assertThat(mTabGroupModelFilter.isTabModelRestored(), equalTo(true));
}
@Test
public void addTab_ToExistingGroup() {
Tab newTab = prepareTab(NEW_TAB_ID, NEW_TAB_ID, TAB1_ID);
......
......@@ -433,6 +433,60 @@ public class TabSwitcherMediatorUnitTest {
verify(mLayout).onTabSelecting(anyLong(), eq(TAB1_ID));
}
@Test
public void updatesResetHandlerOnRestoreCompleted() {
initAndAssertAllProperties();
mMediator.showOverview(true);
assertThat(mModel.get(TabListContainerProperties.IS_VISIBLE), equalTo(true));
mTabModelObserverCaptor.getValue().restoreCompleted();
// MRU will be false unless the start surface is enabled.
verify(mResetHandler).resetWithTabList(mTabModelFilter, false, false);
}
@Test
public void showOverviewDoesNotUpdateResetHandlerBeforeRestoreCompleted() {
initAndAssertAllProperties();
doReturn(false).when(mTabModelFilter).isTabModelRestored();
mMediator.showOverview(true);
// MRU will be false unless the start surface is enabled.
verify(mResetHandler, never()).resetWithTabList(mTabModelFilter, true, false);
}
@Test
public void prepareOverviewDoesNotUpdateResetHandlerBeforeRestoreCompleted() {
initAndAssertAllProperties();
doReturn(false).when(mTabModelFilter).isTabModelRestored();
mMediator.prepareOverview();
// MRU will be false unless the start surface is enabled.
verify(mResetHandler, never()).resetWithTabList(mTabModelFilter, false, false);
}
@Test
public void showOverviewUpdatesResetHandlerAfterRestoreCompleted() {
initAndAssertAllProperties();
doReturn(true).when(mTabModelFilter).isTabModelRestored();
mMediator.showOverview(true);
// MRU will be false unless the start surface is enabled.
verify(mResetHandler).resetWithTabList(mTabModelFilter, true, false);
}
@Test
public void prepareOverviewUpdatesResetHandlerAfterRestoreCompleted() {
initAndAssertAllProperties();
doReturn(true).when(mTabModelFilter).isTabModelRestored();
mMediator.prepareOverview();
// MRU will be false unless the start surface is enabled.
verify(mResetHandler).resetWithTabList(mTabModelFilter, false, false);
}
@Test
@DisableFeatures(ChromeFeatureList.TAB_GROUPS_UI_IMPROVEMENTS_ANDROID)
public void openDialogButton_FlagDisabled() {
......
......@@ -26,6 +26,7 @@ public abstract class TabModelFilter extends EmptyTabModelObserver implements Ta
Collections.unmodifiableList(new ArrayList<Tab>());
private TabModel mTabModel;
protected ObserverList<TabModelObserver> mFilteredObservers = new ObserverList<>();
private boolean mTabRestoreCompleted;
public TabModelFilter(TabModel tabModel) {
mTabModel = tabModel;
......@@ -88,7 +89,7 @@ public abstract class TabModelFilter extends EmptyTabModelObserver implements Ta
* @return An unmodifiable list of {@link Tab}s that are not related to any tabs
*/
@NonNull
final public List<Tab> getTabsWithNoOtherRelatedTabs() {
public final List<Tab> getTabsWithNoOtherRelatedTabs() {
List<Tab> tabs = new ArrayList<>();
for (int i = 0; i < mTabModel.getCount(); i++) {
Tab tab = mTabModel.getTabAt(i);
......@@ -140,6 +141,13 @@ public abstract class TabModelFilter extends EmptyTabModelObserver implements Ta
*/
protected abstract void resetFilterStateInternal();
/**
* @return Whether the tab model is fully restored.
*/
public boolean isTabModelRestored() {
return mTabRestoreCompleted || isIncognito();
}
/**
* Concrete class requires to define what's the behavior when {@link TabModel} removed a
* {@link Tab}.
......@@ -264,6 +272,8 @@ public abstract class TabModelFilter extends EmptyTabModelObserver implements Ta
@Override
public void restoreCompleted() {
mTabRestoreCompleted = true;
if (getCount() != 0) reorder();
for (TabModelObserver observer : mFilteredObservers) {
......
......@@ -4,11 +4,11 @@
package org.chromium.chrome.browser.tasks;
import static org.junit.Assert.assertEquals;
import static org.chromium.chrome.browser.tasks.ReturnToChromeExperimentsUtil.TAB_SWITCHER_ON_RETURN_MS;
import android.app.Activity;
import android.content.Context;
import android.os.SystemClock;
import android.support.test.InstrumentationRegistry;
import android.support.test.filters.SmallTest;
......@@ -22,13 +22,20 @@ import org.chromium.base.ActivityState;
import org.chromium.base.ApplicationStatus;
import org.chromium.base.test.util.CallbackHelper;
import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.base.test.util.DisabledTest;
import org.chromium.base.test.util.Feature;
import org.chromium.base.test.util.Restriction;
import org.chromium.chrome.browser.ChromeFeatureList;
import org.chromium.chrome.browser.ChromeSwitches;
import org.chromium.chrome.browser.ChromeTabbedActivity;
import org.chromium.chrome.browser.util.FeatureUtilities;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.chrome.test.ChromeTabbedActivityTestRule;
import org.chromium.chrome.test.util.MenuUtils;
import org.chromium.content_public.browser.test.util.Criteria;
import org.chromium.content_public.browser.test.util.CriteriaHelper;
import org.chromium.content_public.browser.test.util.TestThreadUtils;
import org.chromium.net.test.EmbeddedTestServer;
import org.chromium.ui.test.util.UiRestriction;
import java.util.concurrent.TimeoutException;
......@@ -37,6 +44,7 @@ import java.util.concurrent.TimeoutException;
* has passed.
*/
@RunWith(ChromeJUnit4ClassRunner.class)
@Restriction(UiRestriction.RESTRICTION_TYPE_PHONE)
public class ReturnToChromeTest {
@Rule
public ChromeTabbedActivityTestRule mActivityTestRule = new ChromeTabbedActivityTestRule();
......@@ -45,10 +53,12 @@ public class ReturnToChromeTest {
@Before
public void setUp() throws Exception {
Context context = InstrumentationRegistry.getInstrumentation().getTargetContext();
FeatureUtilities.setGridTabSwitcherEnabledForTesting(true);
mActivityTestRule.startMainActivityFromLauncher();
mActivity = mActivityTestRule.getActivity();
setupTabs();
}
/**
......@@ -64,7 +74,7 @@ public class ReturnToChromeTest {
"force-fieldtrial-params=FakeStudyName.Enabled:" + TAB_SWITCHER_ON_RETURN_MS
+ "/100000"})
public void
testObserverModeNotTriggeredWithoutDelay() throws Exception {
testTabSwitcherModeNotTriggeredWithinThreshold() throws Exception {
finishActivityCompletely();
mActivityTestRule.startMainActivityFromLauncher();
......@@ -73,25 +83,51 @@ public class ReturnToChromeTest {
Assert.assertFalse(mActivity.getLayoutManager().overviewVisible());
}
/**
* Test that overview mode is triggered if the delay is shorter than the interval between
* stop and start.
*/
@Test
@SmallTest
@Feature({"ReturnToChrome"})
@DisabledTest(message = "crbug.com/955436")
@CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE,
"enable-features=" + ChromeFeatureList.TAB_SWITCHER_ON_RETURN + "<FakeStudyName",
"force-fieldtrials=FakeStudyName/Enabled",
"force-fieldtrial-params=FakeStudyName.Enabled:" + TAB_SWITCHER_ON_RETURN_MS + "/1"})
"force-fieldtrial-params=FakeStudyName.Enabled:" + TAB_SWITCHER_ON_RETURN_MS + "/0"})
public void
testObserverModeTriggeredWithDelay() throws Exception {
testTabSwitcherModeTriggeredBeyondThreshold() throws Exception {
finishActivityCompletely();
// Sleep past the timeout.
SystemClock.sleep(30);
mActivityTestRule.startMainActivityFromLauncher();
mActivity = mActivityTestRule.getActivity();
Assert.assertTrue(mActivity.getLayoutManager().overviewVisible());
CriteriaHelper.pollUiThread(Criteria.equals(true,
mActivityTestRule.getActivity()
.getTabModelSelector()
.getTabModelFilterProvider()
.getCurrentTabModelFilter()::isTabModelRestored));
assertEquals(2, mActivityTestRule.getActivity().getTabModelSelector().getTotalTabCount());
}
private void setupTabs() throws InterruptedException {
TestThreadUtils.runOnUiThreadBlocking(
() -> mActivityTestRule.getActivity().getTabModelSelector().closeAllTabs());
EmbeddedTestServer testServer =
EmbeddedTestServer.createAndStartServer(InstrumentationRegistry.getContext());
final String url = testServer.getURL("/chrome/test/data/android/navigate/simple.html");
// Add 2 tabs.
MenuUtils.invokeCustomMenuActionSync(InstrumentationRegistry.getInstrumentation(),
mActivity, org.chromium.chrome.R.id.new_tab_menu_id);
mActivityTestRule.loadUrl(url);
MenuUtils.invokeCustomMenuActionSync(InstrumentationRegistry.getInstrumentation(),
mActivity, org.chromium.chrome.R.id.new_tab_menu_id);
mActivityTestRule.loadUrl(url);
assertEquals(2, mActivity.getTabModelSelector().getTotalTabCount());
}
private void finishActivityCompletely() throws InterruptedException, TimeoutException {
......
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