Skip to content

Commit

Permalink
Partially revert TabListMediator initialization
Browse files Browse the repository at this point in the history
This CL reverts the changes in TabListMediator initialization in
http://crrev.com/c/2326858 to help identify the cause of a related
crash.

Bug: 1116985, 1111122
Change-Id: Ib1131418ac0ce6f60a76d3dea23ea80eb5a2ca03
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2375885
Commit-Queue: Yue Zhang <yuezhanggg@chromium.org>
Reviewed-by: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#801552}
  • Loading branch information
Yue Zhang authored and Commit Bot committed Aug 25, 2020
1 parent a326861 commit 8723a16
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 25 deletions.
Expand Up @@ -38,7 +38,6 @@
import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.finishActivity;
import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.getSwipeToDismissAction;
import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.leaveTabSwitcher;
import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.mergeAllNormalTabsToAGroup;
import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.rotateDeviceToOrientation;
import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.switchTabModel;
import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.verifyTabModelTabCount;
Expand Down Expand Up @@ -2022,14 +2021,6 @@ public void testSetup_WithInstantStart() throws Exception {
// closure.
closeFirstTabInTabSwitcher();
verifyTabSwitcherCardCount(cta, 0);

// Verify TabGroupModelFilter is correctly setup by checking if tab switcher changes with
// tab grouping.
createTabs(cta, false, 3);
enterTabSwitcher(cta);
verifyTabSwitcherCardCount(cta, 2);
mergeAllNormalTabsToAGroup(cta);
verifyTabSwitcherCardCount(cta, 1);
}

private void enterTabGroupManualSelection(ChromeTabbedActivity cta) {
Expand Down
Expand Up @@ -567,7 +567,8 @@ public void tabRemoved(Tab tab) {
}
};

if (TabUiFeatureUtilities.isTabGroupsAndroidEnabled()) {
if (mTabModelSelector.getTabModelFilterProvider().getCurrentTabModelFilter()
instanceof TabGroupModelFilter) {
mTabGroupObserver = new EmptyTabGroupModelFilterObserver() {
@Override
public void didMoveWithinGroup(
Expand Down Expand Up @@ -695,6 +696,13 @@ public void didMoveTabGroup(
public void didCreateGroup(
List<Tab> tabs, List<Integer> tabOriginalIndex, boolean isSameGroup) {}
};

((TabGroupModelFilter) mTabModelSelector.getTabModelFilterProvider().getTabModelFilter(
false))
.addTabGroupObserver(mTabGroupObserver);
((TabGroupModelFilter) mTabModelSelector.getTabModelFilterProvider().getTabModelFilter(
true))
.addTabGroupObserver(mTabGroupObserver);
}

// TODO(meiliang): follow up with unit tests to test the close signal is sent correctly with
Expand Down Expand Up @@ -758,17 +766,6 @@ public void initWithNative(Profile profile) {
mTabListFaviconProvider.initWithNative(profile);
mTabModelSelector.getTabModelFilterProvider().addTabModelFilterObserver(mTabModelObserver);

if (mTabGroupObserver != null) {
assert mTabModelSelector.getTabModelFilterProvider().getCurrentTabModelFilter()
instanceof TabGroupModelFilter;
((TabGroupModelFilter) mTabModelSelector.getTabModelFilterProvider().getTabModelFilter(
false))
.addTabGroupObserver(mTabGroupObserver);
((TabGroupModelFilter) mTabModelSelector.getTabModelFilterProvider().getTabModelFilter(
true))
.addTabGroupObserver(mTabGroupObserver);
}

if (TabUiFeatureUtilities.isTabGroupsAndroidContinuationEnabled()) {
mTabGroupTitleEditor = new TabGroupTitleEditor(mTabModelSelector) {
@Override
Expand Down
Expand Up @@ -784,6 +784,7 @@ public void testRenderDialog_5Tabs_InitialScroll(boolean nightModeEnabled) throw

@Test
@MediumTest
@DisabledTest(message = "Re-enable this when crbug.com/1116985 is resolved.")
@Features.EnableFeatures({ChromeFeatureList.INSTANT_START})
public void testSetup_WithInstantStart() {
final ChromeTabbedActivity cta = mActivityTestRule.getActivity();
Expand Down
Expand Up @@ -2342,11 +2342,7 @@ private void setUpForTabGroupOperation(@TabListMediatorType int type) {
mTabContentManager::getTabThumbnailWithCallback, mTitleProvider,
mTabListFaviconProvider, actionOnRelatedTabs, null, null, handler,
getClass().getSimpleName(), uiType);

// TabGroupModelFilterObserver is registered when native is ready.
assertThat(mTabGroupModelFilterObserverCaptor.getAllValues().isEmpty(), equalTo(true));
mMediator.initWithNative(mProfile);
assertThat(mTabGroupModelFilterObserverCaptor.getAllValues().isEmpty(), equalTo(false));

// There are two TabModelObserver and two TabGroupModelFilter.Observer added when
// initializing TabListMediator, one set from TabListMediator and the other from
Expand Down

0 comments on commit 8723a16

Please sign in to comment.