Skip to content

GitLab

T
tangled
Commit d534208c authored by Wei-Yin Chen (陳威尹)'s avatar Wei-Yin Chen (陳威尹) Committed by Commit Bot
Browse files
Options

Fix pre-restoration behavior in PseudoTab 

Before this CL, existence of TabModelFilter was used to determine
whether we should use pre-native PseudoTab or not, but having a
TabModelFilter doesn't necessarily mean it's already restored.

This CL uses TabModelSelector#isTabStateInitialized instead.

Bug: 1080810
Change-Id: Id8aa5e1a61cfa563f4f56b64d6c09ea0fd2b277c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2206223
Auto-Submit: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Commit-Queue: Xi Han <hanxi@chromium.org>
Reviewed-by: default avatarXi Han <hanxi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#771173}
parent 1c431c12
Hide whitespace changes
Inline Side-by-side
Showing with 46 additions and 41 deletions
chrome/android/features/tab_ui/java/src/org/chromium/chrome/browser/tasks/pseudotab/PseudoTab.java
View file @ d534208c
......@@ -20,6 +20,7 @@ import org.chromium.chrome.browser.tab.TabImpl;
import org.chromium.chrome.browser.tabmodel.TabList;
import org.chromium.chrome.browser.tabmodel.TabModelFilter;
import org.chromium.chrome.browser.tabmodel.TabModelFilterProvider;
import org.chromium.chrome.browser.tabmodel.TabModelSelector;
import org.chromium.chrome.browser.tabmodel.TabPersistentStore;
import org.chromium.chrome.browser.tabmodel.TabbedModeTabPersistencePolicy;
import org.chromium.chrome.browser.tasks.tab_management.TabUiFeatureUtilities;
......@@ -235,12 +236,12 @@ public class PseudoTab {
* Get related tabs of a certain {@link PseudoTab}, through {@link TabModelFilter}s if
* available.
* @param member The {@link PseudoTab} related to
* @param provider The {@link TabModelFilterProvider} to query the tab relation
* @param tabModelSelector The {@link TabModelSelector} to query the tab relation
* @return Related {@link PseudoTab}s
*/
public static synchronized @NonNull List<PseudoTab> getRelatedTabs(
PseudoTab member, @NonNull TabModelFilterProvider provider) {
List<Tab> relatedTabs = getRelatedTabList(provider, member.getId());
PseudoTab member, @NonNull TabModelSelector tabModelSelector) {
List<Tab> relatedTabs = getRelatedTabList(tabModelSelector, member.getId());
if (relatedTabs != null) return getListOfPseudoTab(relatedTabs);
List<PseudoTab> related = new ArrayList<>();
......@@ -261,17 +262,17 @@ public class PseudoTab {
}
private static @Nullable List<Tab> getRelatedTabList(
@NonNull TabModelFilterProvider provider, int tabId) {
if (provider.getTabModelFilter(false) != null) {
List<Tab> related = provider.getTabModelFilter(false).getRelatedTabList(tabId);
if (related.size() > 0) return related;
@NonNull TabModelSelector tabModelSelector, int tabId) {
if (!tabModelSelector.isTabStateInitialized()) {
assert CachedFeatureFlags.isEnabled(ChromeFeatureList.INSTANT_START);
return null;
}
if (provider.getTabModelFilter(true) != null) {
List<Tab> related = provider.getTabModelFilter(true).getRelatedTabList(tabId);
assert related.size() > 0;
return related;
}
return null;
TabModelFilterProvider provider = tabModelSelector.getTabModelFilterProvider();
List<Tab> related = provider.getTabModelFilter(false).getRelatedTabList(tabId);
if (related.size() > 0) return related;
related = provider.getTabModelFilter(true).getRelatedTabList(tabId);
assert related.size() > 0;
return related;
}
@VisibleForTesting
......@@ -281,7 +282,6 @@ public class PseudoTab {
@Nullable
public static List<PseudoTab> getAllPseudoTabsFromStateFile() {
assert CachedFeatureFlags.isEnabled(ChromeFeatureList.INSTANT_START);
readAllPseudoTabsFromStateFile();
return sAllTabsFromStateFile;
}
......@@ -293,6 +293,7 @@ public class PseudoTab {
}
private static void readAllPseudoTabsFromStateFile() {
assert CachedFeatureFlags.isEnabled(ChromeFeatureList.INSTANT_START);
if (sReadStateFile) return;
sReadStateFile = true;
......
chrome/android/features/tab_ui/java/src/org/chromium/chrome/browser/tasks/tab_management/MultiThumbnailCardProvider.java
View file @ d534208c
......@@ -88,8 +88,7 @@ public class MultiThumbnailCardProvider implements TabListMediator.ThumbnailProv
mCanvas.drawColor(Color.TRANSPARENT);
// Initialize Tabs.
List<PseudoTab> relatedTabList =
PseudoTab.getRelatedTabs(tab, mTabModelSelector.getTabModelFilterProvider());
List<PseudoTab> relatedTabList = PseudoTab.getRelatedTabs(tab, mTabModelSelector);
if (relatedTabList.size() <= 4) {
mThumbnailsToFetch.set(relatedTabList.size());
......@@ -306,10 +305,7 @@ public class MultiThumbnailCardProvider implements TabListMediator.ThumbnailProv
public void getTabThumbnailWithCallback(
int tabId, Callback<Bitmap> finalCallback, boolean forceUpdate, boolean writeToCache) {
PseudoTab tab = PseudoTab.fromTabId(tabId);
if (tab == null
|| PseudoTab.getRelatedTabs(tab, mTabModelSelector.getTabModelFilterProvider())
.size()
== 1) {
if (tab == null || PseudoTab.getRelatedTabs(tab, mTabModelSelector).size() == 1) {
mTabContentManager.getTabThumbnailWithCallback(
tabId, finalCallback, forceUpdate, writeToCache);
return;
......
chrome/android/features/tab_ui/java/src/org/chromium/chrome/browser/tasks/tab_management/TabSwitcherCoordinator.java
View file @ d534208c
......@@ -151,9 +151,7 @@ public class TabSwitcherCoordinator
new MultiThumbnailCardProvider(context, tabContentManager, tabModelSelector);
PseudoTab.TitleProvider titleProvider = tab -> {
int numRelatedTabs =
PseudoTab.getRelatedTabs(tab, tabModelSelector.getTabModelFilterProvider())
.size();
int numRelatedTabs = PseudoTab.getRelatedTabs(tab, tabModelSelector).size();
if (numRelatedTabs == 1) return tab.getTitle();
return context.getResources().getQuantityString(
R.plurals.bottom_tab_grid_title_placeholder, numRelatedTabs, numRelatedTabs);
......
chrome/android/features/tab_ui/junit/src/org/chromium/chrome/browser/tasks/pseudotab/PseudoTabUnitTest.java
View file @ d534208c
......@@ -4,7 +4,6 @@
package org.chromium.chrome.browser.tasks.pseudotab;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock;
......@@ -26,7 +25,10 @@ import org.chromium.chrome.browser.tab.TabImpl;
import org.chromium.chrome.browser.tabmodel.TabList;
import org.chromium.chrome.browser.tabmodel.TabModelFilter;
import org.chromium.chrome.browser.tabmodel.TabModelFilterProvider;
import org.chromium.chrome.browser.tabmodel.TabModelSelector;
import org.chromium.chrome.test.util.browser.Features;
import org.chromium.chrome.test.util.browser.Features.DisableFeatures;
import org.chromium.chrome.test.util.browser.Features.EnableFeatures;
import java.util.ArrayList;
import java.util.Arrays;
......@@ -54,6 +56,8 @@ public class PseudoTabUnitTest {
@Mock
TabModelFilter mTabModelFilter2;
@Mock
TabModelSelector mTabModelSelector;
@Mock
TabModelFilterProvider mTabModelFilterProvider;
private TabImpl mTab1;
......@@ -67,6 +71,8 @@ public class PseudoTabUnitTest {
mTab1 = prepareTab(TAB1_ID);
mTab2 = prepareTab(TAB2_ID);
mTab3 = prepareTab(TAB3_ID);
doReturn(mTabModelFilterProvider).when(mTabModelSelector).getTabModelFilterProvider();
}
@After
......@@ -307,20 +313,22 @@ public class PseudoTabUnitTest {
}
@Test
@Features.DisableFeatures({ChromeFeatureList.TAB_GROUPS_ANDROID})
@DisableFeatures({ChromeFeatureList.TAB_GROUPS_ANDROID})
@EnableFeatures({ChromeFeatureList.INSTANT_START})
public void getRelatedTabs_noProvider_groupDisabled_single() {
doReturn(null).when(mTabModelFilterProvider).getTabModelFilter(anyBoolean());
doReturn(false).when(mTabModelSelector).isTabStateInitialized();
PseudoTab tab1 = PseudoTab.fromTabId(TAB1_ID);
List<PseudoTab> related = PseudoTab.getRelatedTabs(tab1, mTabModelFilterProvider);
List<PseudoTab> related = PseudoTab.getRelatedTabs(tab1, mTabModelSelector);
Assert.assertEquals(1, related.size());
Assert.assertEquals(TAB1_ID, related.get(0).getId());
}
@Test
@Features.DisableFeatures({ChromeFeatureList.TAB_GROUPS_ANDROID})
@DisableFeatures({ChromeFeatureList.TAB_GROUPS_ANDROID})
@EnableFeatures({ChromeFeatureList.INSTANT_START})
public void getRelatedTabs_noProvider_groupDisabled_group() {
doReturn(null).when(mTabModelFilterProvider).getTabModelFilter(anyBoolean());
doReturn(false).when(mTabModelSelector).isTabStateInitialized();
TabAttributeCache.setRootIdForTesting(TAB1_ID, TAB1_ID);
TabAttributeCache.setRootIdForTesting(TAB2_ID, TAB1_ID);
......@@ -329,42 +337,42 @@ public class PseudoTabUnitTest {
PseudoTab tab2 = PseudoTab.fromTabId(TAB2_ID);
Assert.assertEquals(TAB1_ID, tab2.getRootId());
List<PseudoTab> related = PseudoTab.getRelatedTabs(tab1, mTabModelFilterProvider);
List<PseudoTab> related = PseudoTab.getRelatedTabs(tab1, mTabModelSelector);
Assert.assertEquals(1, related.size());
Assert.assertEquals(TAB1_ID, related.get(0).getId());
}
@Test
@Features.EnableFeatures({ChromeFeatureList.TAB_GROUPS_ANDROID})
@EnableFeatures({ChromeFeatureList.TAB_GROUPS_ANDROID, ChromeFeatureList.INSTANT_START})
public void getRelatedTabs_noProvider_single() {
doReturn(null).when(mTabModelFilterProvider).getTabModelFilter(anyBoolean());
doReturn(false).when(mTabModelSelector).isTabStateInitialized();
PseudoTab tab1 = PseudoTab.fromTabId(TAB1_ID);
List<PseudoTab> related = PseudoTab.getRelatedTabs(tab1, mTabModelFilterProvider);
List<PseudoTab> related = PseudoTab.getRelatedTabs(tab1, mTabModelSelector);
Assert.assertEquals(1, related.size());
Assert.assertEquals(TAB1_ID, related.get(0).getId());
}
@Test
@Features.EnableFeatures({ChromeFeatureList.TAB_GROUPS_ANDROID})
@EnableFeatures({ChromeFeatureList.TAB_GROUPS_ANDROID, ChromeFeatureList.INSTANT_START})
public void getRelatedTabs_noProvider_group() {
doReturn(null).when(mTabModelFilterProvider).getTabModelFilter(anyBoolean());
doReturn(false).when(mTabModelSelector).isTabStateInitialized();
TabAttributeCache.setRootIdForTesting(TAB1_ID, TAB1_ID);
TabAttributeCache.setRootIdForTesting(TAB2_ID, TAB1_ID);
PseudoTab tab1 = PseudoTab.fromTabId(TAB1_ID);
PseudoTab.fromTabId(TAB2_ID);
List<PseudoTab> related = PseudoTab.getRelatedTabs(tab1, mTabModelFilterProvider);
List<PseudoTab> related = PseudoTab.getRelatedTabs(tab1, mTabModelSelector);
Assert.assertEquals(2, related.size());
Assert.assertEquals(TAB1_ID, related.get(0).getId());
Assert.assertEquals(TAB2_ID, related.get(1).getId());
}
@Test
@Features.EnableFeatures({ChromeFeatureList.TAB_GROUPS_ANDROID})
@EnableFeatures({ChromeFeatureList.TAB_GROUPS_ANDROID, ChromeFeatureList.INSTANT_START})
public void getRelatedTabs_noProvider_badGroup() {
doReturn(null).when(mTabModelFilterProvider).getTabModelFilter(anyBoolean());
doReturn(false).when(mTabModelSelector).isTabStateInitialized();
TabAttributeCache.setRootIdForTesting(TAB1_ID, TAB1_ID);
TabAttributeCache.setRootIdForTesting(TAB2_ID, Tab.INVALID_TAB_ID);
......@@ -373,19 +381,20 @@ public class PseudoTabUnitTest {
PseudoTab.fromTabId(TAB2_ID);
PseudoTab.fromTabId(TAB3_ID);
List<PseudoTab> related = PseudoTab.getRelatedTabs(tab1, mTabModelFilterProvider);
List<PseudoTab> related = PseudoTab.getRelatedTabs(tab1, mTabModelSelector);
Assert.assertEquals(1, related.size());
Assert.assertEquals(TAB1_ID, related.get(0).getId());
}
@Test
public void getRelatedTabs_provider_normal() {
doReturn(true).when(mTabModelSelector).isTabStateInitialized();
doReturn(mTabModelFilter).when(mTabModelFilterProvider).getTabModelFilter(eq(false));
List<Tab> tabs = new ArrayList<>(Arrays.asList(mTab1, mTab2, mTab3));
doReturn(tabs).when(mTabModelFilter).getRelatedTabList(TAB1_ID);
PseudoTab tab1 = PseudoTab.fromTabId(TAB1_ID);
List<PseudoTab> related = PseudoTab.getRelatedTabs(tab1, mTabModelFilterProvider);
List<PseudoTab> related = PseudoTab.getRelatedTabs(tab1, mTabModelSelector);
Assert.assertEquals(3, related.size());
Assert.assertEquals(TAB1_ID, related.get(0).getId());
Assert.assertEquals(TAB2_ID, related.get(1).getId());
......@@ -394,6 +403,7 @@ public class PseudoTabUnitTest {
@Test
public void getRelatedTabs_provider_incognito() {
doReturn(true).when(mTabModelSelector).isTabStateInitialized();
doReturn(mTabModelFilter).when(mTabModelFilterProvider).getTabModelFilter(eq(false));
List<Tab> empty = new ArrayList<>();
doReturn(empty).when(mTabModelFilter).getRelatedTabList(TAB1_ID);
......@@ -402,7 +412,7 @@ public class PseudoTabUnitTest {
doReturn(tabs).when(mTabModelFilter2).getRelatedTabList(TAB1_ID);
PseudoTab tab1 = PseudoTab.fromTabId(TAB1_ID);
List<PseudoTab> related = PseudoTab.getRelatedTabs(tab1, mTabModelFilterProvider);
List<PseudoTab> related = PseudoTab.getRelatedTabs(tab1, mTabModelSelector);
Assert.assertEquals(2, related.size());
Assert.assertEquals(TAB1_ID, related.get(0).getId());
Assert.assertEquals(TAB2_ID, related.get(1).getId());
......
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