Skip to content

Commit

Permalink
[Cleanup] Removed GridTabSwitcherForTablet flag.
Browse files Browse the repository at this point in the history
* Removed unused empty background classes
* Fixed failing tests

Change-Id: Ifdbede93e49ef5cadb2966c318632fbea2371d81
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4327818
Commit-Queue: Sirisha Kavuluru <skavuluru@google.com>
Reviewed-by: Neil Coronado <nemco@google.com>
Reviewed-by: Theresa Sullivan <twellington@chromium.org>
Code-Coverage: Findit <findit-for-me@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1116536}
  • Loading branch information
Sirisha Kavuluru authored and Chromium LUCI CQ committed Mar 13, 2023
1 parent bea1781 commit eb98b97
Show file tree
Hide file tree
Showing 34 changed files with 41 additions and 879 deletions.
1 change: 0 additions & 1 deletion chrome/android/chrome_java_resources.gni
Expand Up @@ -512,7 +512,6 @@ chrome_java_resources = [
"java/res/layout/divider_line_menu_item.xml",
"java/res/layout/editable_option_editor_icons.xml",
"java/res/layout/empty_accessory_sheet.xml",
"java/res/layout/empty_background_view_tablet.xml",
"java/res/layout/fake_search_box_layout.xml",
"java/res/layout/find_in_page.xml",
"java/res/layout/find_toolbar.xml",
Expand Down
4 changes: 0 additions & 4 deletions chrome/android/chrome_java_sources.gni
Expand Up @@ -1141,10 +1141,6 @@ chrome_java_sources = [
"java/src/org/chromium/chrome/browser/ui/RootUiCoordinator.java",
"java/src/org/chromium/chrome/browser/ui/ViewDrawBlocker.java",
"java/src/org/chromium/chrome/browser/ui/system/StatusBarColorController.java",
"java/src/org/chromium/chrome/browser/ui/tablet/emptybackground/EmptyBackgroundViewTablet.java",
"java/src/org/chromium/chrome/browser/ui/tablet/emptybackground/EmptyBackgroundViewWrapper.java",
"java/src/org/chromium/chrome/browser/ui/tablet/emptybackground/incognitotoggle/IncognitoToggleButton.java",
"java/src/org/chromium/chrome/browser/ui/tablet/emptybackground/incognitotoggle/IncognitoToggleButtonTablet.java",
"java/src/org/chromium/chrome/browser/undo_tab_close_snackbar/UndoBarController.java",
"java/src/org/chromium/chrome/browser/upgrade/PackageReplacedBroadcastReceiver.java",
"java/src/org/chromium/chrome/browser/usage_stats/DigitalWellbeingClient.java",
Expand Down
Expand Up @@ -41,6 +41,7 @@
import org.chromium.chrome.browser.tasks.tab_management.TabListRecyclerView.RecyclerViewPosition;
import org.chromium.chrome.browser.tasks.tab_management.TabProperties.UiType;
import org.chromium.chrome.tab_ui.R;
import org.chromium.ui.base.DeviceFormFactor;
import org.chromium.ui.modelutil.MVCListAdapter;
import org.chromium.ui.modelutil.PropertyKey;
import org.chromium.ui.modelutil.PropertyModel;
Expand Down Expand Up @@ -268,7 +269,7 @@ public class TabListCoordinator

if (mMode == TabListMode.GRID) {
mGlobalLayoutListener = this::updateThumbnailLocation;
if (TabUiFeatureUtilities.isTabletGridTabSwitcherEnabled(mContext)) {
if (DeviceFormFactor.isNonMultiDisplayContextOnTablet(mContext)) {
mListLayoutListener = (view, left, top, right, bottom, oldLeft, oldTop, oldRight,
oldBottom) -> updateGridCardLayout(right - left);
}
Expand Down
Expand Up @@ -58,12 +58,6 @@ public class TabUiFeatureUtilities {
new BooleanCachedFieldTrialParameter(ChromeFeatureList.TAB_GRID_LAYOUT_ANDROID,
SHOW_OPEN_IN_TAB_GROUP_MENU_ITEM_FIRST_PARAM, false);

// Field trial parameter for controlling delay grid tab switcher creation for tablets.
private static final String DELAY_GTS_CREATION_PARAM = "delay_creation";
public static final BooleanCachedFieldTrialParameter DELAY_GTS_CREATION =
new BooleanCachedFieldTrialParameter(ChromeFeatureList.GRID_TAB_SWITCHER_FOR_TABLETS,
DELAY_GTS_CREATION_PARAM, true);

// Field trial parameter for defining tab width for tab strip improvements.
private static final String TAB_STRIP_IMPROVEMENTS_TAB_WIDTH_PARAM = "min_tab_width";
public static final DoubleCachedFieldTrialParameter TAB_STRIP_TAB_WIDTH =
Expand Down Expand Up @@ -92,7 +86,6 @@ public class TabUiFeatureUtilities {
TAB_SELECTION_EDITOR_V2_BOOKMARKS_PARAM, true);

private static Boolean sTabManagementModuleSupportedForTesting;
private static Boolean sGridTabSwitcherDelayCreationEnabledForTesting;

/**
* Set whether the tab management module is supported for testing.
Expand All @@ -118,23 +111,14 @@ private static boolean isTabManagementModuleSupported() {
*/
public static boolean isGridTabSwitcherEnabled(Context context) {
if (DeviceFormFactor.isNonMultiDisplayContextOnTablet(context)) {
return isTabletGridTabSwitcherEnabled(context);
return true;
}

// Having Tab Groups or Start implies Grid Tab Switcher.
return isTabManagementModuleSupported() || isTabGroupsAndroidEnabled(context)
|| ReturnToChromeUtil.isStartSurfaceEnabled(context);
}

/**
* @return Whether the tablet Grid Tab Switcher UI is enabled and available for use.
* @param context The activity context.
*/
public static boolean isTabletGridTabSwitcherEnabled(Context context) {
return DeviceFormFactor.isNonMultiDisplayContextOnTablet(context)
&& ChromeFeatureList.sGridTabSwitcherForTablets.isEnabled();
}

/**
* @return Whether the tab strip improvements are enabled.
* @param context The activity context.
Expand All @@ -150,31 +134,11 @@ public static boolean isTabStripImprovementsEnabled(Context context) {
*/
public static boolean isTabletTabGroupsEnabled(Context context) {
return DeviceFormFactor.isNonMultiDisplayContextOnTablet(context)
&& ChromeFeatureList.sGridTabSwitcherForTablets.isEnabled()
&& ChromeFeatureList.sTabStripImprovements.isEnabled()
&& ChromeFeatureList.sTabGroupsForTablets.isEnabled()
&& !DeviceClassManager.enableAccessibilityLayout(context);
}

/**
* Set whether the tablet grid tab switcher polish is enabled for testing.
*/
public static void setGtsDelayCreationEnabledForTesting(@Nullable Boolean enabled) {
sGridTabSwitcherDelayCreationEnabledForTesting = enabled;
}

/**
* @return Whether the tablet Grid Tab Switcher creation should be delayed to on GTS load
* instead of on startup.
*/
public static boolean isTabletGridTabSwitcherDelayCreationEnabled() {
if (sGridTabSwitcherDelayCreationEnabledForTesting != null) {
return sGridTabSwitcherDelayCreationEnabledForTesting;
}

return DELAY_GTS_CREATION.getValue();
}

/**
* @return Whether the tab group feature is enabled and available for use.
* @param context The activity context.
Expand Down
Expand Up @@ -37,7 +37,6 @@
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.when;

import static org.chromium.chrome.browser.flags.ChromeFeatureList.GRID_TAB_SWITCHER_FOR_TABLETS;
import static org.chromium.chrome.browser.flags.ChromeFeatureList.TAB_GRID_LAYOUT_ANDROID;
import static org.chromium.chrome.browser.flags.ChromeFeatureList.TAB_GROUPS_ANDROID;
import static org.chromium.chrome.browser.flags.ChromeFeatureList.TAB_GROUPS_FOR_TABLETS;
Expand Down Expand Up @@ -85,7 +84,6 @@

import org.hamcrest.Matchers;
import org.junit.After;
import org.junit.AfterClass;
import org.junit.Assert;
import org.junit.Before;
import org.junit.BeforeClass;
Expand Down Expand Up @@ -145,7 +143,7 @@
@CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE})
@Restriction({Restriction.RESTRICTION_TYPE_NON_LOW_END_DEVICE})
@Features.EnableFeatures({TAB_GRID_LAYOUT_ANDROID, TAB_GROUPS_ANDROID,
TAB_GROUPS_FOR_TABLETS, GRID_TAB_SWITCHER_FOR_TABLETS, TAB_STRIP_IMPROVEMENTS})
TAB_GROUPS_FOR_TABLETS, TAB_STRIP_IMPROVEMENTS})
@DoNotBatch(reason = "crbug.com/1380489")
public class TabGridDialogTest {
// clang-format on
Expand Down Expand Up @@ -178,7 +176,6 @@ public class TabGridDialogTest {
@BeforeClass
public static void setUpBeforeActivityLaunched() {
ChromeNightModeTestUtils.setUpNightModeBeforeChromeActivityLaunched();
TabUiFeatureUtilities.setGtsDelayCreationEnabledForTesting(false);
}

@ParameterAnnotations.UseMethodParameterBefore(NightModeTestUtils.NightModeParams.class)
Expand All @@ -193,7 +190,6 @@ public void setUp() {
Intents.init();
TabUiFeatureUtilities.setTabManagementModuleSupportedForTesting(true);
mActivityTestRule.startMainActivityOnBlankPage();
TabUiTestHelper.verifyTabSwitcherLayoutType(mActivityTestRule.getActivity());
CriteriaHelper.pollUiThread(
mActivityTestRule.getActivity().getTabModelSelector()::isTabStateInitialized);
}
Expand All @@ -206,11 +202,6 @@ public void tearDown() {
Intents.release();
}

@AfterClass
public static void tearDownAfterActivityDestroyed() {
TabUiFeatureUtilities.setGtsDelayCreationEnabledForTesting(null);
}

@Test
@MediumTest
public void testBackPressCloseDialog() {
Expand Down
Expand Up @@ -16,7 +16,6 @@
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

import static org.chromium.chrome.browser.flags.ChromeFeatureList.GRID_TAB_SWITCHER_FOR_TABLETS;
import static org.chromium.chrome.browser.flags.ChromeFeatureList.TAB_GROUPS_FOR_TABLETS;
import static org.chromium.chrome.browser.flags.ChromeFeatureList.TAB_SELECTION_EDITOR_V2;
import static org.chromium.chrome.browser.flags.ChromeFeatureList.TAB_STRIP_IMPROVEMENTS;
Expand Down Expand Up @@ -83,8 +82,7 @@
*/
@RunWith(ParameterizedRunner.class)
@ParameterAnnotations.UseRunnerDelegate(ChromeJUnit4RunnerDelegate.class)
@Features.EnableFeatures({GRID_TAB_SWITCHER_FOR_TABLETS, TAB_STRIP_IMPROVEMENTS,
TAB_GROUPS_FOR_TABLETS, TAB_SELECTION_EDITOR_V2})
@Features.EnableFeatures({TAB_STRIP_IMPROVEMENTS, TAB_GROUPS_FOR_TABLETS, TAB_SELECTION_EDITOR_V2})
@Batch(Batch.PER_CLASS)
public class TabSelectionEditorMenuTest extends BlankUiTestActivityTestCase {
private static final int TAB_COUNT = 3;
Expand Down
Expand Up @@ -29,7 +29,6 @@

import static org.chromium.base.test.util.Restriction.RESTRICTION_TYPE_LOW_END_DEVICE;
import static org.chromium.base.test.util.Restriction.RESTRICTION_TYPE_NON_LOW_END_DEVICE;
import static org.chromium.chrome.browser.flags.ChromeFeatureList.GRID_TAB_SWITCHER_FOR_TABLETS;
import static org.chromium.chrome.browser.flags.ChromeFeatureList.TAB_GROUPS_ANDROID;
import static org.chromium.chrome.browser.flags.ChromeFeatureList.TAB_GROUPS_FOR_TABLETS;
import static org.chromium.chrome.browser.flags.ChromeFeatureList.TAB_STRIP_IMPROVEMENTS;
Expand Down Expand Up @@ -114,8 +113,7 @@
@RunWith(ChromeJUnit4ClassRunner.class)
@CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE, "force-fieldtrials=Study/Group",
"force-fieldtrial-params=Study.Group:enable_launch_polish/true"})
@EnableFeatures({TAB_GROUPS_ANDROID, GRID_TAB_SWITCHER_FOR_TABLETS + "<Study",
TAB_STRIP_IMPROVEMENTS, TAB_GROUPS_FOR_TABLETS})
@EnableFeatures({TAB_GROUPS_ANDROID, TAB_STRIP_IMPROVEMENTS, TAB_GROUPS_FOR_TABLETS})
@DisableFeatures(TAB_TO_GTS_ANIMATION)
@Batch(Batch.PER_CLASS)
public class TabSelectionEditorTest {
Expand Down
Expand Up @@ -22,7 +22,6 @@
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;

import android.graphics.Bitmap;
import android.support.test.InstrumentationRegistry;
import android.view.ViewGroup;
import android.view.ViewStub;
Expand All @@ -38,7 +37,6 @@
import org.junit.runner.RunWith;

import org.chromium.base.Callback;
import org.chromium.base.GarbageCollectionTestUtils;
import org.chromium.base.test.util.Batch;
import org.chromium.base.test.util.CallbackHelper;
import org.chromium.base.test.util.CommandLineFlags;
Expand Down Expand Up @@ -75,9 +73,6 @@
import org.chromium.ui.test.util.DisableAnimationsTestRule;
import org.chromium.ui.test.util.UiRestriction;

import java.lang.ref.WeakReference;
import java.util.LinkedList;
import java.util.List;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
Expand All @@ -86,10 +81,9 @@
* Tests for the {@link TabSwitcher} on tablet
*/
@RunWith(ChromeJUnit4ClassRunner.class)
@CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE, "force-fieldtrials=Study/Group",
"force-fieldtrial-params=Study.Group:delay_creation/false"})
@EnableFeatures({ChromeFeatureList.GRID_TAB_SWITCHER_FOR_TABLETS + "<Study",
ChromeFeatureList.TAB_STRIP_REDESIGN})
@CommandLineFlags.
Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE, "force-fieldtrials=Study/Group"})
@EnableFeatures({ChromeFeatureList.TAB_STRIP_REDESIGN})
@DisableFeatures(ChromeFeatureList.TAB_TO_GTS_ANIMATION)
@Restriction(
{Restriction.RESTRICTION_TYPE_NON_LOW_END_DEVICE, UiRestriction.RESTRICTION_TYPE_TABLET})
Expand All @@ -106,9 +100,6 @@ public class TabSwitcherTabletTest {
public final BlankCTATabInitialStateRule mInitialStateRule =
new BlankCTATabInitialStateRule(sActivityTestRule, false);

private List<WeakReference<Bitmap>> mAllBitmaps = new LinkedList<>();
private TabSwitcher.TabListDelegate mTabListDelegate;

private CallbackHelper mLayoutChangedCallbackHelper = new CallbackHelper();
private int mCurrentlyActiveLayout;
private Callback<LayoutManagerImpl> mLayoutManagerCallback;
Expand Down Expand Up @@ -144,11 +135,26 @@ public void cleanup() throws TimeoutException {
@Test
@MediumTest
@RequiresRestart
public void testEnterAndExitTabSwitcherVerifyThumbnails()
throws ExecutionException, TimeoutException {
prepareTabsWithThumbnail(1, 1);
public void testEnterAndExitTabSwitcher() throws ExecutionException, TimeoutException {
Layout layout = sActivityTestRule.getActivity().getLayoutManager().getOverviewLayout();
assertNull("StartSurface layout should not be initialized", layout);
ViewStub tabSwitcherStub = (ViewStub) sActivityTestRule.getActivity().findViewById(
R.id.grid_tab_switcher_view_holder_stub);
assertTrue("TabSwitcher view stub should not be inflated",
tabSwitcherStub.getParent() != null);

prepareTabs(1, 1);
enterGTSWithThumbnailChecking();
exitGTSAndVerifyThumbnailsAreReleased();
layout = sActivityTestRule.getActivity().getLayoutManager().getOverviewLayout();
assertTrue("OverviewLayout should be TabSwitcherAndStartSurfaceLayout layout",
layout instanceof TabSwitcherAndStartSurfaceLayout);
ViewGroup tabSwitcherViewHolder =
sActivityTestRule.getActivity().findViewById(R.id.grid_tab_switcher_view_holder);
assertNotNull("TabSwitcher view should be inflated", tabSwitcherViewHolder);

exitSwitcherWithTabClick(0);
assertFalse(sActivityTestRule.getActivity().getLayoutManager().isLayoutVisible(
LayoutType.TAB_SWITCHER));
}

@Test
Expand Down Expand Up @@ -251,31 +257,8 @@ public void testGridTabSwitcherOnNoNextTab() throws ExecutionException {

@Test
@MediumTest
@CommandLineFlags.Add({"force-fieldtrial-params=Study.Group:delay_creation/true"})
public void testGridTabSwitcherDelayCreate() {
Layout layout = sActivityTestRule.getActivity().getLayoutManager().getOverviewLayout();
assertNull("StartSurface layout should not be initialized", layout);
ViewStub tabSwitcherStub = (ViewStub) sActivityTestRule.getActivity().findViewById(
R.id.grid_tab_switcher_view_holder_stub);
assertTrue("TabSwitcher view stub should not be inflated",
tabSwitcherStub.getParent() != null);

// Click tab switcher button
TabUiTestHelper.enterTabSwitcher(sActivityTestRule.getActivity());

layout = sActivityTestRule.getActivity().getLayoutManager().getOverviewLayout();
assertTrue("OverviewLayout should be TabSwitcherAndStartSurfaceLayout layout",
layout instanceof TabSwitcherAndStartSurfaceLayout);
ViewGroup tabSwitcherViewHolder =
sActivityTestRule.getActivity().findViewById(R.id.grid_tab_switcher_view_holder);
assertNotNull("TabSwitcher view should be inflated", tabSwitcherViewHolder);
}

@Test
@MediumTest
@CommandLineFlags.Add({"force-fieldtrial-params=Study.Group:delay_creation/true"})
@EnableFeatures({ChromeFeatureList.START_SURFACE_REFACTOR})
public void testGridTabSwitcherDelayCreate_RefactorEnabled() throws ExecutionException {
public void testGridTabSwitcher_RefactorEnabled() throws ExecutionException {
prepareTabs(2, 0);
// Verifies that the dialog visibility supplier doesn't crash when closing a Tab without the
// grid tab switcher is inflated.
Expand Down Expand Up @@ -331,42 +314,11 @@ public void onTabModelSelected(TabModel newModel, TabModel oldModel) {
});
}

private void prepareTabsWithThumbnail(int numTabs, int numIncognitoTabs) {
setupForThumbnailCheck();
int oldCount = mTabListDelegate.getBitmapFetchCountForTesting();
TabUiTestHelper.prepareTabsWithThumbnail(
sActivityTestRule, numTabs, numIncognitoTabs, null);
assertEquals(0, mTabListDelegate.getBitmapFetchCountForTesting() - oldCount);
}

private void prepareTabs(int numTabs, int numIncognitoTabs) {
TabUiTestHelper.createTabs(sActivityTestRule.getActivity(), false, numTabs);
TabUiTestHelper.createTabs(sActivityTestRule.getActivity(), true, numIncognitoTabs);
}

private void setupForThumbnailCheck() {
Layout layout = sActivityTestRule.getActivity().getLayoutManager().getOverviewLayout();
assertTrue(layout instanceof TabSwitcherAndStartSurfaceLayout);
TabSwitcherAndStartSurfaceLayout mTabSwitcherAndStartSurfaceLayout =
(TabSwitcherAndStartSurfaceLayout) layout;

mTabListDelegate = mTabSwitcherAndStartSurfaceLayout.getStartSurfaceForTesting()
.getGridTabListDelegate();
Callback<Bitmap> mBitmapListener = (bitmap) -> mAllBitmaps.add(new WeakReference<>(bitmap));
mTabListDelegate.setBitmapCallbackForTesting(mBitmapListener);
}

private void exitGTSAndVerifyThumbnailsAreReleased()
throws TimeoutException, ExecutionException {
assertTrue(sActivityTestRule.getActivity().getLayoutManager().isLayoutVisible(
LayoutType.TAB_SWITCHER));

final int index = sActivityTestRule.getActivity().getCurrentTabModel().index();
exitSwitcherWithTabClick(index);

assertThumbnailsAreReleased();
}

private void exitSwitcherWithTabClick(int index) throws TimeoutException {
TabUiTestHelper.clickNthCardFromTabSwitcher(sActivityTestRule.getActivity(), index);
mLayoutChangedCallbackHelper.waitForCallback(1, 1, CALLBACK_WAIT_TIMEOUT, TimeUnit.SECONDS);
Expand All @@ -388,17 +340,6 @@ private void enterGTSWithThumbnailChecking() {
sActivityTestRule.getActivity().getCurrentTabModel());
}

private void assertThumbnailsAreReleased() {
CriteriaHelper.pollUiThread(() -> {
for (WeakReference<Bitmap> bitmap : mAllBitmaps) {
if (!GarbageCollectionTestUtils.canBeGarbageCollected(bitmap)) {
return false;
}
}
return true;
});
}

private void closeTab(final boolean incognito, final int id) {
ChromeTabUtils.closeTabWithAction(InstrumentationRegistry.getInstrumentation(),
sActivityTestRule.getActivity(), () -> {
Expand Down
Expand Up @@ -33,7 +33,6 @@
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import static org.chromium.chrome.browser.flags.ChromeFeatureList.GRID_TAB_SWITCHER_FOR_TABLETS;
import static org.chromium.chrome.browser.flags.ChromeFeatureList.START_SURFACE_ANDROID;
import static org.chromium.chrome.browser.flags.ChromeFeatureList.TAB_GROUPS_ANDROID;
import static org.chromium.chrome.browser.flags.ChromeFeatureList.TAB_GROUPS_CONTINUATION_ANDROID;
Expand Down Expand Up @@ -2320,7 +2319,7 @@ public void updateSpanCount_MultiWindow() {
}

@Test
@Features.EnableFeatures(GRID_TAB_SWITCHER_FOR_TABLETS)
@Config(qualifiers = "sw600dp")
public void updateSpanCount_onTablet_multipleScreenWidths() {
initAndAssertAllProperties(3);
// Mock tablet
Expand Down

0 comments on commit eb98b97

Please sign in to comment.