Commit f519f118 authored by Mei Liang's avatar Mei Liang Committed by Commit Bot

Simplify the enabling logic for grid and group

This CL simplifies the enabling logic for grid and group because low-end
and accessibility is supported for GTS with the TabGroup M5 flag.

Change-Id: I0b49f25526252275dc914e6631360e0b45495ff6
Bug: 1019489
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2092186Reviewed-by: default avatarTheresa  <twellington@chromium.org>
Reviewed-by: default avatarWei-Yin Chen (陳威尹) <wychen@chromium.org>
Reviewed-by: default avatarXi Han <hanxi@chromium.org>
Commit-Queue: Mei Liang <meiliang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#749320}
parent 9e1e13dd
...@@ -8,7 +8,6 @@ import androidx.annotation.Nullable; ...@@ -8,7 +8,6 @@ import androidx.annotation.Nullable;
import androidx.annotation.VisibleForTesting; import androidx.annotation.VisibleForTesting;
import org.chromium.base.ContextUtils; import org.chromium.base.ContextUtils;
import org.chromium.base.SysUtils;
import org.chromium.chrome.browser.device.DeviceClassManager; import org.chromium.chrome.browser.device.DeviceClassManager;
import org.chromium.chrome.browser.flags.CachedFeatureFlags; import org.chromium.chrome.browser.flags.CachedFeatureFlags;
import org.chromium.chrome.browser.flags.ChromeFeatureList; import org.chromium.chrome.browser.flags.ChromeFeatureList;
...@@ -65,7 +64,7 @@ public class TabUiFeatureUtilities { ...@@ -65,7 +64,7 @@ public class TabUiFeatureUtilities {
* @return Tab UI related feature flags that should be cached. * @return Tab UI related feature flags that should be cached.
*/ */
public static List<String> getFeaturesToCache() { public static List<String> getFeaturesToCache() {
if (!isEligibleForTabUiExperiments() || DeviceClassManager.enableAccessibilityLayout()) { if (!isEligibleForTabUiExperiments()) {
return Collections.emptyList(); return Collections.emptyList();
} }
return Arrays.asList(ChromeFeatureList.TAB_GRID_LAYOUT_ANDROID, return Arrays.asList(ChromeFeatureList.TAB_GRID_LAYOUT_ANDROID,
...@@ -75,23 +74,18 @@ public class TabUiFeatureUtilities { ...@@ -75,23 +74,18 @@ public class TabUiFeatureUtilities {
} }
private static boolean isEligibleForTabUiExperiments() { private static boolean isEligibleForTabUiExperiments() {
return (ChromeFeatureList.isEnabled(ChromeFeatureList.TAB_GROUPS_CONTINUATION_ANDROID) return !DeviceFormFactor.isNonMultiDisplayContextOnTablet(
|| !SysUtils.isLowEndDevice()) ContextUtils.getApplicationContext());
&& !DeviceFormFactor.isNonMultiDisplayContextOnTablet(
ContextUtils.getApplicationContext());
} }
/** /**
* @return Whether the Grid Tab Switcher UI is enabled and available for use. * @return Whether the Grid Tab Switcher UI is enabled and available for use.
*/ */
public static boolean isGridTabSwitcherEnabled() { public static boolean isGridTabSwitcherEnabled() {
// TODO(yusufo): AccessibilityLayout check should not be here and the flow should support
// changing that setting while Chrome is alive.
// Having Tab Groups or Start implies Grid Tab Switcher. // Having Tab Groups or Start implies Grid Tab Switcher.
return !(CachedFeatureFlags.isEnabled(ChromeFeatureList.TAB_GROUPS_CONTINUATION_ANDROID) return (!DeviceClassManager.enableAccessibilityLayout()
&& SysUtils.isLowEndDevice()) && CachedFeatureFlags.isEnabled(ChromeFeatureList.TAB_GRID_LAYOUT_ANDROID)
&& CachedFeatureFlags.isEnabled(ChromeFeatureList.TAB_GRID_LAYOUT_ANDROID) && isTabManagementModuleSupported())
&& isTabManagementModuleSupported()
|| isTabGroupsAndroidEnabled() || StartSurfaceConfiguration.isStartSurfaceEnabled(); || isTabGroupsAndroidEnabled() || StartSurfaceConfiguration.isStartSurfaceEnabled();
} }
...@@ -99,7 +93,8 @@ public class TabUiFeatureUtilities { ...@@ -99,7 +93,8 @@ public class TabUiFeatureUtilities {
* @return Whether the tab group feature is enabled and available for use. * @return Whether the tab group feature is enabled and available for use.
*/ */
public static boolean isTabGroupsAndroidEnabled() { public static boolean isTabGroupsAndroidEnabled() {
return CachedFeatureFlags.isEnabled(ChromeFeatureList.TAB_GROUPS_ANDROID) return !DeviceClassManager.enableAccessibilityLayout()
&& CachedFeatureFlags.isEnabled(ChromeFeatureList.TAB_GROUPS_ANDROID)
&& isTabManagementModuleSupported(); && isTabManagementModuleSupported();
} }
......
...@@ -47,6 +47,7 @@ import org.chromium.base.Callback; ...@@ -47,6 +47,7 @@ import org.chromium.base.Callback;
import org.chromium.base.metrics.RecordHistogram; import org.chromium.base.metrics.RecordHistogram;
import org.chromium.base.metrics.RecordUserAction; import org.chromium.base.metrics.RecordUserAction;
import org.chromium.base.supplier.ObservableSupplier; import org.chromium.base.supplier.ObservableSupplier;
import org.chromium.base.test.BaseRobolectricTestRunner;
import org.chromium.chrome.browser.flags.CachedFeatureFlags; import org.chromium.chrome.browser.flags.CachedFeatureFlags;
import org.chromium.chrome.browser.flags.ChromeFeatureList; import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.share.ShareDelegate; import org.chromium.chrome.browser.share.ShareDelegate;
...@@ -65,7 +66,6 @@ import org.chromium.chrome.browser.widget.ScrimView; ...@@ -65,7 +66,6 @@ import org.chromium.chrome.browser.widget.ScrimView;
import org.chromium.chrome.tab_ui.R; import org.chromium.chrome.tab_ui.R;
import org.chromium.chrome.test.util.browser.Features; import org.chromium.chrome.test.util.browser.Features;
import org.chromium.content_public.browser.LoadUrlParams; import org.chromium.content_public.browser.LoadUrlParams;
import org.chromium.testing.local.LocalRobolectricTestRunner;
import org.chromium.ui.KeyboardVisibilityDelegate; import org.chromium.ui.KeyboardVisibilityDelegate;
import org.chromium.ui.modelutil.PropertyModel; import org.chromium.ui.modelutil.PropertyModel;
...@@ -76,7 +76,7 @@ import java.util.List; ...@@ -76,7 +76,7 @@ import java.util.List;
/** /**
* Tests for {@link TabGridDialogMediator}. * Tests for {@link TabGridDialogMediator}.
*/ */
@RunWith(LocalRobolectricTestRunner.class) @RunWith(BaseRobolectricTestRunner.class)
@Config(manifest = Config.NONE) @Config(manifest = Config.NONE)
@Features.DisableFeatures(ChromeFeatureList.TAB_GROUPS_CONTINUATION_ANDROID) @Features.DisableFeatures(ChromeFeatureList.TAB_GROUPS_CONTINUATION_ANDROID)
public class TabGridDialogMediatorUnitTest { public class TabGridDialogMediatorUnitTest {
......
...@@ -41,6 +41,7 @@ import org.robolectric.annotation.Config; ...@@ -41,6 +41,7 @@ import org.robolectric.annotation.Config;
import org.chromium.base.metrics.RecordHistogram; import org.chromium.base.metrics.RecordHistogram;
import org.chromium.base.metrics.RecordUserAction; import org.chromium.base.metrics.RecordUserAction;
import org.chromium.base.test.BaseRobolectricTestRunner;
import org.chromium.chrome.browser.feature_engagement.TrackerFactory; import org.chromium.chrome.browser.feature_engagement.TrackerFactory;
import org.chromium.chrome.browser.flags.CachedFeatureFlags; import org.chromium.chrome.browser.flags.CachedFeatureFlags;
import org.chromium.chrome.browser.flags.ChromeFeatureList; import org.chromium.chrome.browser.flags.ChromeFeatureList;
...@@ -54,7 +55,6 @@ import org.chromium.chrome.browser.tasks.tab_groups.TabGroupModelFilter; ...@@ -54,7 +55,6 @@ import org.chromium.chrome.browser.tasks.tab_groups.TabGroupModelFilter;
import org.chromium.chrome.test.util.browser.Features; import org.chromium.chrome.test.util.browser.Features;
import org.chromium.components.feature_engagement.EventConstants; import org.chromium.components.feature_engagement.EventConstants;
import org.chromium.components.feature_engagement.Tracker; import org.chromium.components.feature_engagement.Tracker;
import org.chromium.testing.local.LocalRobolectricTestRunner;
import org.chromium.ui.modelutil.MVCListAdapter; import org.chromium.ui.modelutil.MVCListAdapter;
import org.chromium.ui.modelutil.PropertyKey; import org.chromium.ui.modelutil.PropertyKey;
import org.chromium.ui.modelutil.PropertyModel; import org.chromium.ui.modelutil.PropertyModel;
...@@ -66,7 +66,7 @@ import java.util.List; ...@@ -66,7 +66,7 @@ import java.util.List;
/** /**
* Tests for {@link TabGridItemTouchHelperCallback}. * Tests for {@link TabGridItemTouchHelperCallback}.
*/ */
@RunWith(LocalRobolectricTestRunner.class) @RunWith(BaseRobolectricTestRunner.class)
@Config(manifest = Config.NONE) @Config(manifest = Config.NONE)
public class TabGridItemTouchHelperCallbackUnitTest { public class TabGridItemTouchHelperCallbackUnitTest {
@Rule @Rule
......
...@@ -4,9 +4,12 @@ ...@@ -4,9 +4,12 @@
package org.chromium.chrome.browser.device; package org.chromium.chrome.browser.device;
import androidx.annotation.VisibleForTesting;
import org.chromium.base.CommandLine; import org.chromium.base.CommandLine;
import org.chromium.base.ContextUtils; import org.chromium.base.ContextUtils;
import org.chromium.base.SysUtils; import org.chromium.base.SysUtils;
import org.chromium.chrome.browser.flags.CachedFeatureFlags;
import org.chromium.chrome.browser.flags.ChromeFeatureList; import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.flags.ChromeSwitches; import org.chromium.chrome.browser.flags.ChromeSwitches;
import org.chromium.chrome.browser.preferences.ChromePreferenceKeys; import org.chromium.chrome.browser.preferences.ChromePreferenceKeys;
...@@ -63,7 +66,6 @@ public class DeviceClassManager { ...@@ -63,7 +66,6 @@ public class DeviceClassManager {
// Flag based configurations. // Flag based configurations.
CommandLine commandLine = CommandLine.getInstance(); CommandLine commandLine = CommandLine.getInstance();
assert commandLine.isNativeImplementation();
mEnableAccessibilityLayout |= commandLine mEnableAccessibilityLayout |= commandLine
.hasSwitch(ChromeSwitches.ENABLE_ACCESSIBILITY_TAB_SWITCHER); .hasSwitch(ChromeSwitches.ENABLE_ACCESSIBILITY_TAB_SWITCHER);
mEnableFullscreen = mEnableFullscreen =
...@@ -87,11 +89,8 @@ public class DeviceClassManager { ...@@ -87,11 +89,8 @@ public class DeviceClassManager {
*/ */
public static boolean enableAccessibilityLayout() { public static boolean enableAccessibilityLayout() {
if (isPhone() if (isPhone()
&& ChromeFeatureList.isEnabled(ChromeFeatureList.TAB_GROUPS_CONTINUATION_ANDROID) && CachedFeatureFlags.isEnabled(ChromeFeatureList.TAB_GROUPS_CONTINUATION_ANDROID)
&& (ChromeFeatureList.isEnabled(ChromeFeatureList.TAB_GROUPS_ANDROID) && CachedFeatureFlags.isEnabled(ChromeFeatureList.TAB_GROUPS_ANDROID)) {
|| (!SysUtils.isLowEndDevice()
&& ChromeFeatureList.isEnabled(
ChromeFeatureList.TAB_GRID_LAYOUT_ANDROID)))) {
return false; return false;
} }
...@@ -136,4 +135,12 @@ public class DeviceClassManager { ...@@ -136,4 +135,12 @@ public class DeviceClassManager {
return !DeviceFormFactor.isNonMultiDisplayContextOnTablet( return !DeviceFormFactor.isNonMultiDisplayContextOnTablet(
ContextUtils.getApplicationContext()); ContextUtils.getApplicationContext());
} }
/**
* Reset the instance for testing.
*/
@VisibleForTesting
public static void resetForTesting() {
sInstance = null;
}
} }
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