Commit f9f217ed authored by Xi Han's avatar Xi Han Committed by Commit Bot

[InstantStart] Closes all tabs should hide tab switcher.

This bug is caused by the StartSurfaceMediator registers an observer to
the empty tab model before the TabModel is initialized. In this CL, we
register an observer to the TabModeSelector first and registers an
normal TabModel observer after the TabModel is initialized.

Bug: 1126914, 1138162
Change-Id: I653aeac99c58ed032e9a604b4001a389832d3452
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2517623
Commit-Queue: Xi Han <hanxi@chromium.org>
Reviewed-by: default avatarYue Zhang <yuezhanggg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#825345}
parent 2b0323b7
...@@ -138,7 +138,6 @@ class StartSurfaceMediator ...@@ -138,7 +138,6 @@ class StartSurfaceMediator
private int mStartSurfaceState; private int mStartSurfaceState;
@StartSurfaceState @StartSurfaceState
private int mPreviousStartSurfaceState; private int mPreviousStartSurfaceState;
private boolean mIsOmniboxFocused;
@Nullable @Nullable
private TabModel mNormalTabModel; private TabModel mNormalTabModel;
@Nullable @Nullable
...@@ -151,6 +150,11 @@ class StartSurfaceMediator ...@@ -151,6 +150,11 @@ class StartSurfaceMediator
private boolean mExcludeMVTiles; private boolean mExcludeMVTiles;
private boolean mShowStackTabSwitcher; private boolean mShowStackTabSwitcher;
private OneshotSupplier<StartSurface> mStartSurfaceSupplier; private OneshotSupplier<StartSurface> mStartSurfaceSupplier;
/**
* Whether a pending observer needed be added to the normal TabModel after the TabModel is
* initialized.
*/
private boolean mPendingObserver;
/** /**
* The value of {@link Pref#ARTICLES_LIST_VISIBLE} on Startup. Getting this value for recording * The value of {@link Pref#ARTICLES_LIST_VISIBLE} on Startup. Getting this value for recording
* the consistency of {@link ChromePreferenceKeys#FEED_ARTICLES_LIST_VISIBLE} with {@link * the consistency of {@link ChromePreferenceKeys#FEED_ARTICLES_LIST_VISIBLE} with {@link
...@@ -230,12 +234,11 @@ class StartSurfaceMediator ...@@ -230,12 +234,11 @@ class StartSurfaceMediator
// Hide tab carousel, which does not exist in incognito mode, when closing all // Hide tab carousel, which does not exist in incognito mode, when closing all
// normal tabs. // normal tabs.
mNormalTabModel = mTabModelSelector.getModel(false);
mNormalTabModelObserver = new TabModelObserver() { mNormalTabModelObserver = new TabModelObserver() {
@Override @Override
public void willCloseTab(Tab tab, boolean animate) { public void willCloseTab(Tab tab, boolean animate) {
if (mStartSurfaceState == StartSurfaceState.SHOWN_HOMEPAGE if (mStartSurfaceState == StartSurfaceState.SHOWN_HOMEPAGE
&& mNormalTabModel.getCount() <= 1) { && mTabModelSelector.getModel(false).getCount() <= 1) {
setTabCarouselVisibility(false); setTabCarouselVisibility(false);
} }
} }
...@@ -256,6 +259,30 @@ class StartSurfaceMediator ...@@ -256,6 +259,30 @@ class StartSurfaceMediator
mTabModelSelector.getModel(false).getCount() > 0 && !mIsIncognito); mTabModelSelector.getModel(false).getCount() > 0 && !mIsIncognito);
} }
}; };
if (mTabModelSelector.getModels().isEmpty()) {
TabModelSelectorObserver selectorObserver =
new EmptyTabModelSelectorObserver() {
@Override
public void onChange() {
assert !mTabModelSelector.getModels().isEmpty();
assert mTabModelSelector.getTabModelFilterProvider()
.getTabModelFilter(false)
!= null;
assert mTabModelSelector.getTabModelFilterProvider()
.getTabModelFilter(true)
!= null;
mTabModelSelector.removeObserver(this);
mNormalTabModel = mTabModelSelector.getModel(false);
if (mPendingObserver) {
mPendingObserver = false;
mNormalTabModel.addObserver(mNormalTabModelObserver);
}
}
};
mTabModelSelector.addObserver(selectorObserver);
} else {
mNormalTabModel = mTabModelSelector.getModel(false);
}
} }
mBrowserControlsObserver = new BrowserControlsStateProvider.Observer() { mBrowserControlsObserver = new BrowserControlsStateProvider.Observer() {
...@@ -467,8 +494,11 @@ class StartSurfaceMediator ...@@ -467,8 +494,11 @@ class StartSurfaceMediator
// bottom bar. // bottom bar.
mPropertyModel.set( mPropertyModel.set(
BOTTOM_BAR_HEIGHT, mBrowserControlsStateProvider.getBottomControlsHeight()); BOTTOM_BAR_HEIGHT, mBrowserControlsStateProvider.getBottomControlsHeight());
mNormalTabModel.addObserver(mNormalTabModelObserver); if (mNormalTabModel != null) {
mNormalTabModel.addObserver(mNormalTabModelObserver);
} else {
mPendingObserver = true;
}
} else if (mStartSurfaceState == StartSurfaceState.SHOWN_TABSWITCHER) { } else if (mStartSurfaceState == StartSurfaceState.SHOWN_TABSWITCHER) {
mPropertyModel.set(IS_SHOWING_STACK_TAB_SWITCHER, mShowStackTabSwitcher); mPropertyModel.set(IS_SHOWING_STACK_TAB_SWITCHER, mShowStackTabSwitcher);
...@@ -669,7 +699,11 @@ class StartSurfaceMediator ...@@ -669,7 +699,11 @@ class StartSurfaceMediator
destroyFeedSurfaceCoordinator(); destroyFeedSurfaceCoordinator();
} }
if (mNormalTabModelObserver != null) { if (mNormalTabModelObserver != null) {
mNormalTabModel.removeObserver(mNormalTabModelObserver); if (mNormalTabModel != null) {
mNormalTabModel.removeObserver(mNormalTabModelObserver);
} else if (mPendingObserver) {
mPendingObserver = false;
}
} }
if (mTabModelSelectorObserver != null) { if (mTabModelSelectorObserver != null) {
mTabModelSelector.removeObserver(mTabModelSelectorObserver); mTabModelSelector.removeObserver(mTabModelSelectorObserver);
......
...@@ -44,6 +44,7 @@ import android.content.Intent; ...@@ -44,6 +44,7 @@ import android.content.Intent;
import android.os.Build; import android.os.Build;
import android.support.test.InstrumentationRegistry; import android.support.test.InstrumentationRegistry;
import android.view.KeyEvent; import android.view.KeyEvent;
import android.view.View;
import android.view.ViewGroup; import android.view.ViewGroup;
import androidx.test.espresso.ViewAction; import androidx.test.espresso.ViewAction;
...@@ -1124,6 +1125,63 @@ public class StartSurfaceTest { ...@@ -1124,6 +1125,63 @@ public class StartSurfaceTest {
FEED_VISIBILITY_CONSISTENCY, 1)); FEED_VISIBILITY_CONSISTENCY, 1));
} }
} }
@Test
@MediumTest
@Feature({"StartSurface"})
// clang-format off
@CommandLineFlags.Add({BASE_PARAMS + "/single"})
public void testShow_SingleAsHomepage_CloseAllTabsShouldHideTabSwitcher() {
// clang-format on
if (!mImmediateReturn) {
onView(withId(org.chromium.chrome.tab_ui.R.id.home_button)).perform(click());
}
ChromeTabbedActivity cta = mActivityTestRule.getActivity();
CriteriaHelper.pollUiThread(
() -> cta.getLayoutManager() != null && cta.getLayoutManager().overviewVisible());
waitForTabModel();
TabUiTestHelper.verifyTabModelTabCount(cta, 1, 0);
assertEquals(cta.findViewById(org.chromium.chrome.tab_ui.R.id.tab_switcher_title)
.getVisibility(),
View.VISIBLE);
TestThreadUtils.runOnUiThreadBlocking(
() -> { cta.getTabModelSelector().getModel(false).closeAllTabs(); });
TabUiTestHelper.verifyTabModelTabCount(cta, 0, 0);
assertEquals(cta.findViewById(org.chromium.chrome.tab_ui.R.id.tab_switcher_title)
.getVisibility(),
View.GONE);
}
@Test
@MediumTest
@Feature({"StartSurface"})
// clang-format off
@CommandLineFlags.Add({BASE_PARAMS + "/single/exclude_mv_tiles/true"
+ "/show_last_active_tab_only/true/show_stack_tab_switcher/true"})
public void testShow_SingleAsHomepageV2_CloseAllTabsShouldHideTabSwitcher() {
// clang-format on
if (!mImmediateReturn) {
onView(withId(org.chromium.chrome.tab_ui.R.id.home_button)).perform(click());
}
ChromeTabbedActivity cta = mActivityTestRule.getActivity();
CriteriaHelper.pollUiThread(
() -> cta.getLayoutManager() != null && cta.getLayoutManager().overviewVisible());
waitForTabModel();
TabUiTestHelper.verifyTabModelTabCount(cta, 1, 0);
assertEquals(cta.findViewById(org.chromium.chrome.tab_ui.R.id.tab_switcher_title)
.getVisibility(),
View.VISIBLE);
TestThreadUtils.runOnUiThreadBlocking(
() -> { cta.getTabModelSelector().getModel(false).closeAllTabs(); });
TabUiTestHelper.verifyTabModelTabCount(cta, 0, 0);
assertEquals(cta.findViewById(org.chromium.chrome.tab_ui.R.id.tab_switcher_title)
.getVisibility(),
View.GONE);
}
} }
// TODO(crbug.com/1033909): Add more integration tests. // TODO(crbug.com/1033909): Add more integration tests.
...@@ -71,6 +71,8 @@ import org.chromium.chrome.browser.tab.Tab; ...@@ -71,6 +71,8 @@ import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tab.TabLaunchType; import org.chromium.chrome.browser.tab.TabLaunchType;
import org.chromium.chrome.browser.tabmodel.EmptyTabModelSelectorObserver; import org.chromium.chrome.browser.tabmodel.EmptyTabModelSelectorObserver;
import org.chromium.chrome.browser.tabmodel.TabModel; import org.chromium.chrome.browser.tabmodel.TabModel;
import org.chromium.chrome.browser.tabmodel.TabModelFilter;
import org.chromium.chrome.browser.tabmodel.TabModelFilterProvider;
import org.chromium.chrome.browser.tabmodel.TabModelObserver; import org.chromium.chrome.browser.tabmodel.TabModelObserver;
import org.chromium.chrome.browser.tabmodel.TabModelSelector; import org.chromium.chrome.browser.tabmodel.TabModelSelector;
import org.chromium.chrome.browser.tasks.TasksSurfaceProperties; import org.chromium.chrome.browser.tasks.TasksSurfaceProperties;
...@@ -85,6 +87,7 @@ import org.chromium.ui.modelutil.PropertyModel; ...@@ -85,6 +87,7 @@ import org.chromium.ui.modelutil.PropertyModel;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Arrays; import java.util.Arrays;
import java.util.List;
/** Tests for {@link StartSurfaceMediator}. */ /** Tests for {@link StartSurfaceMediator}. */
@RunWith(BaseRobolectricTestRunner.class) @RunWith(BaseRobolectricTestRunner.class)
...@@ -104,6 +107,12 @@ public class StartSurfaceMediatorUnitTest { ...@@ -104,6 +107,12 @@ public class StartSurfaceMediatorUnitTest {
@Mock @Mock
private TabModel mIncognitoTabModel; private TabModel mIncognitoTabModel;
@Mock @Mock
private List<TabModel> mTabModels;
@Mock
private TabModelFilterProvider mTabModelFilterProvider;
@Mock
private TabModelFilter mTabModelFilter;
@Mock
private FakeboxDelegate mFakeBoxDelegate; private FakeboxDelegate mFakeBoxDelegate;
@Mock @Mock
private ExploreSurfaceCoordinator.FeedSurfaceCreator mFeedSurfaceCreator; private ExploreSurfaceCoordinator.FeedSurfaceCreator mFeedSurfaceCreator;
...@@ -147,6 +156,10 @@ public class StartSurfaceMediatorUnitTest { ...@@ -147,6 +156,10 @@ public class StartSurfaceMediatorUnitTest {
mPropertyModel = new PropertyModel(allProperties); mPropertyModel = new PropertyModel(allProperties);
mSecondaryTasksSurfacePropertyModel = new PropertyModel(allProperties); mSecondaryTasksSurfacePropertyModel = new PropertyModel(allProperties);
mTabModels = new ArrayList<>(2);
mTabModels.add(mNormalTabModel);
mTabModels.add(mIncognitoTabModel);
when(mTabModelSelector.getModels()).thenReturn(mTabModels);
doReturn(mNormalTabModel).when(mTabModelSelector).getModel(false); doReturn(mNormalTabModel).when(mTabModelSelector).getModel(false);
doReturn(mIncognitoTabModel).when(mTabModelSelector).getModel(true); doReturn(mIncognitoTabModel).when(mTabModelSelector).getModel(true);
doReturn(false).when(mNormalTabModel).isIncognito(); doReturn(false).when(mNormalTabModel).isIncognito();
...@@ -682,7 +695,79 @@ public class StartSurfaceMediatorUnitTest { ...@@ -682,7 +695,79 @@ public class StartSurfaceMediatorUnitTest {
} }
@Test @Test
public void addAndRemoveTabModelObserverWithOverview() { public void pendingTabModelObserverWithBothShowOverviewAndHideBeforeTabModelInitialization() {
doReturn(false).when(mTabModelSelector).isIncognitoSelected();
doReturn(mVoiceRecognitionHandler).when(mFakeBoxDelegate).getVoiceRecognitionHandler();
doReturn(true).when(mVoiceRecognitionHandler).isVoiceSearchEnabled();
mTabModels.clear();
StartSurfaceMediator mediator = createStartSurfaceMediator(SurfaceMode.SINGLE_PANE, false);
verify(mTabModelSelector).addObserver(mTabModelSelectorObserverCaptor.capture());
mediator.setOverviewState(StartSurfaceState.SHOWN_HOMEPAGE);
mediator.showOverview(false);
verify(mNormalTabModel, never()).addObserver(mTabModelObserverCaptor.capture());
mediator.startedHiding();
verify(mNormalTabModel, never()).addObserver(mTabModelObserverCaptor.capture());
verify(mNormalTabModel, never()).removeObserver(mTabModelObserverCaptor.capture());
}
@Test
public void pendingTabModelObserverWithShowOverviewBeforeAndHideAfterTabModelInitialization() {
doReturn(false).when(mTabModelSelector).isIncognitoSelected();
doReturn(mVoiceRecognitionHandler).when(mFakeBoxDelegate).getVoiceRecognitionHandler();
doReturn(true).when(mVoiceRecognitionHandler).isVoiceSearchEnabled();
mTabModels.clear();
StartSurfaceMediator mediator = createStartSurfaceMediator(SurfaceMode.SINGLE_PANE, false);
verify(mTabModelSelector).addObserver(mTabModelSelectorObserverCaptor.capture());
mediator.setOverviewState(StartSurfaceState.SHOWN_HOMEPAGE);
mediator.showOverview(false);
verify(mNormalTabModel, never()).addObserver(mTabModelObserverCaptor.capture());
mTabModels.add(mNormalTabModel);
mTabModels.add(mIncognitoTabModel);
when(mTabModelSelector.getTabModelFilterProvider()).thenReturn(mTabModelFilterProvider);
when(mTabModelFilterProvider.getTabModelFilter(false)).thenReturn(mTabModelFilter);
when(mTabModelFilterProvider.getTabModelFilter(true)).thenReturn(mTabModelFilter);
mTabModelSelectorObserverCaptor.getValue().onChange();
verify(mTabModelSelector).removeObserver(mTabModelSelectorObserverCaptor.capture());
verify(mNormalTabModel).addObserver(mTabModelObserverCaptor.capture());
mediator.startedHiding();
verify(mNormalTabModel).removeObserver(mTabModelObserverCaptor.capture());
}
@Test
public void pendingTabModelObserverWithBothShowAndHideOverviewAfterTabModelInitialization() {
doReturn(false).when(mTabModelSelector).isIncognitoSelected();
doReturn(mVoiceRecognitionHandler).when(mFakeBoxDelegate).getVoiceRecognitionHandler();
doReturn(true).when(mVoiceRecognitionHandler).isVoiceSearchEnabled();
mTabModels.clear();
StartSurfaceMediator mediator = createStartSurfaceMediator(SurfaceMode.SINGLE_PANE, false);
verify(mTabModelSelector).addObserver(mTabModelSelectorObserverCaptor.capture());
mTabModels.add(mNormalTabModel);
mTabModels.add(mIncognitoTabModel);
when(mTabModelSelector.getTabModelFilterProvider()).thenReturn(mTabModelFilterProvider);
when(mTabModelFilterProvider.getTabModelFilter(false)).thenReturn(mTabModelFilter);
when(mTabModelFilterProvider.getTabModelFilter(true)).thenReturn(mTabModelFilter);
mTabModelSelectorObserverCaptor.getValue().onChange();
verify(mTabModelSelector).removeObserver(mTabModelSelectorObserverCaptor.capture());
verify(mNormalTabModel, never()).addObserver(mTabModelObserverCaptor.capture());
mediator.setOverviewState(StartSurfaceState.SHOWN_HOMEPAGE);
mediator.showOverview(false);
verify(mNormalTabModel).addObserver(mTabModelObserverCaptor.capture());
mediator.startedHiding();
verify(mNormalTabModel).removeObserver(mTabModelObserverCaptor.capture());
}
@Test
public void addAndRemoveTabModelSelectorObserverWithOverviewAfterTabModelInitialization() {
doReturn(false).when(mTabModelSelector).isIncognitoSelected(); doReturn(false).when(mTabModelSelector).isIncognitoSelected();
doReturn(mVoiceRecognitionHandler).when(mFakeBoxDelegate).getVoiceRecognitionHandler(); doReturn(mVoiceRecognitionHandler).when(mFakeBoxDelegate).getVoiceRecognitionHandler();
doReturn(true).when(mVoiceRecognitionHandler).isVoiceSearchEnabled(); doReturn(true).when(mVoiceRecognitionHandler).isVoiceSearchEnabled();
......
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