Commit 38e13ed5 authored by Henrique Nakashima's avatar Henrique Nakashima Committed by Commit Bot

Cache Tab UI flags using cacheNativeFlags().

Refactoring the logic for caching Tab UI flags in the following ways:
- TabManagementModuleProvider.isTabManagementModuleSupported() is not
cached anymore, either to prefs or in memory.
- When DeviceClassManager.enableAccessibilityLayout() is true, the
flags are not cached as false. That is equivalent since the default
return value and the cached value in this case match (both false). Not
having to specify both reduces the risk of inconsistency when the code
is changed.
- Features are referenced by name from ChromeFeatureList, not
prefs key from ChromePreferenceKeys.

A follow up CL will move this logic to TabUiFeatureUtilities and
inline many of the methods into clients. I kept this CL small so it is
easier to review.

Bug: 1012975
Change-Id: I8f9d7ad3d3b935064e568bc8561d6336e44839b5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2063416Reviewed-by: default avatarWei-Yin Chen (陳威尹) <wychen@chromium.org>
Commit-Queue: Henrique Nakashima <hnakashima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#744328}
parent 091440d6
...@@ -4,6 +4,8 @@ ...@@ -4,6 +4,8 @@
package org.chromium.chrome.browser.tasks.tab_management; package org.chromium.chrome.browser.tasks.tab_management;
import androidx.annotation.Nullable;
import org.chromium.chrome.browser.flags.ChromeFeatureList; import org.chromium.chrome.browser.flags.ChromeFeatureList;
/** /**
...@@ -11,11 +13,12 @@ import org.chromium.chrome.browser.flags.ChromeFeatureList; ...@@ -11,11 +13,12 @@ import org.chromium.chrome.browser.flags.ChromeFeatureList;
*/ */
public class TabUiFeatureUtilities { public class TabUiFeatureUtilities {
private static Boolean sSearchTermChipEnabledForTesting; private static Boolean sSearchTermChipEnabledForTesting;
private static Boolean sTabManagementModuleSupportedForTesting;
/** /**
* Set whether the search term chip in Grid tab switcher is enabled for testing. * Set whether the search term chip in Grid tab switcher is enabled for testing.
*/ */
public static void setSearchTermChipEnabledForTesting(Boolean enabled) { public static void setSearchTermChipEnabledForTesting(@Nullable Boolean enabled) {
sSearchTermChipEnabledForTesting = enabled; sSearchTermChipEnabledForTesting = enabled;
} }
...@@ -27,4 +30,22 @@ public class TabUiFeatureUtilities { ...@@ -27,4 +30,22 @@ public class TabUiFeatureUtilities {
return ChromeFeatureList.getFieldTrialParamByFeatureAsBoolean( return ChromeFeatureList.getFieldTrialParamByFeatureAsBoolean(
ChromeFeatureList.TAB_GRID_LAYOUT_ANDROID, "enable_search_term_chip", false); ChromeFeatureList.TAB_GRID_LAYOUT_ANDROID, "enable_search_term_chip", false);
} }
/**
* Set whether the tab management module is supported for testing.
*/
public static void setTabManagementModuleSupportedForTesting(@Nullable Boolean enabled) {
sTabManagementModuleSupportedForTesting = enabled;
}
/**
* @return Whether the tab management module is supported.
*/
public static boolean isTabManagementModuleSupported() {
if (sTabManagementModuleSupportedForTesting != null) {
return sTabManagementModuleSupportedForTesting;
}
return TabManagementModuleProvider.isTabManagementModuleSupported();
}
} }
...@@ -106,6 +106,7 @@ public class TabGridDialogTest { ...@@ -106,6 +106,7 @@ public class TabGridDialogTest {
public void setUp() { public void setUp() {
CachedFeatureFlags.setGridTabSwitcherEnabledForTesting(true); CachedFeatureFlags.setGridTabSwitcherEnabledForTesting(true);
CachedFeatureFlags.setTabGroupsAndroidEnabledForTesting(true); CachedFeatureFlags.setTabGroupsAndroidEnabledForTesting(true);
TabUiFeatureUtilities.setTabManagementModuleSupportedForTesting(true);
mActivityTestRule.startMainActivityFromLauncher(); mActivityTestRule.startMainActivityFromLauncher();
Layout layout = mActivityTestRule.getActivity().getLayoutManager().getOverviewLayout(); Layout layout = mActivityTestRule.getActivity().getLayoutManager().getOverviewLayout();
assertTrue(layout instanceof StartSurfaceLayout); assertTrue(layout instanceof StartSurfaceLayout);
...@@ -119,6 +120,7 @@ public class TabGridDialogTest { ...@@ -119,6 +120,7 @@ public class TabGridDialogTest {
public void tearDown() { public void tearDown() {
CachedFeatureFlags.setGridTabSwitcherEnabledForTesting(null); CachedFeatureFlags.setGridTabSwitcherEnabledForTesting(null);
CachedFeatureFlags.setTabGroupsAndroidEnabledForTesting(null); CachedFeatureFlags.setTabGroupsAndroidEnabledForTesting(null);
TabUiFeatureUtilities.setTabManagementModuleSupportedForTesting(null);
} }
@Test @Test
......
...@@ -16,9 +16,10 @@ import org.chromium.base.library_loader.LibraryLoader; ...@@ -16,9 +16,10 @@ import org.chromium.base.library_loader.LibraryLoader;
import org.chromium.chrome.browser.device.DeviceClassManager; import org.chromium.chrome.browser.device.DeviceClassManager;
import org.chromium.chrome.browser.preferences.ChromePreferenceKeys; import org.chromium.chrome.browser.preferences.ChromePreferenceKeys;
import org.chromium.chrome.browser.preferences.SharedPreferencesManager; import org.chromium.chrome.browser.preferences.SharedPreferencesManager;
import org.chromium.chrome.browser.tasks.tab_management.TabManagementModuleProvider; import org.chromium.chrome.browser.tasks.tab_management.TabUiFeatureUtilities;
import org.chromium.ui.base.DeviceFormFactor; import org.chromium.ui.base.DeviceFormFactor;
import java.util.Arrays;
import java.util.HashMap; import java.util.HashMap;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
...@@ -66,6 +67,9 @@ public class CachedFeatureFlags { ...@@ -66,6 +67,9 @@ public class CachedFeatureFlags {
put(ChromeFeatureList.ANDROID_NIGHT_MODE_CCT, true); put(ChromeFeatureList.ANDROID_NIGHT_MODE_CCT, true);
put(ChromeFeatureList.START_SURFACE_ANDROID, false); put(ChromeFeatureList.START_SURFACE_ANDROID, false);
put(ChromeFeatureList.PAINT_PREVIEW_TEST, false); put(ChromeFeatureList.PAINT_PREVIEW_TEST, false);
put(ChromeFeatureList.TAB_GRID_LAYOUT_ANDROID, false);
put(ChromeFeatureList.TAB_GROUPS_ANDROID, false);
put(ChromeFeatureList.DUET_TABSTRIP_INTEGRATION_ANDROID, false);
} }
}; };
...@@ -106,6 +110,12 @@ public class CachedFeatureFlags { ...@@ -106,6 +110,12 @@ public class CachedFeatureFlags {
ChromePreferenceKeys.FLAGS_CACHED_START_SURFACE_ENABLED); ChromePreferenceKeys.FLAGS_CACHED_START_SURFACE_ENABLED);
put(ChromeFeatureList.PAINT_PREVIEW_TEST, put(ChromeFeatureList.PAINT_PREVIEW_TEST,
ChromePreferenceKeys.FLAGS_CACHED_PAINT_PREVIEW_TEST_ENABLED_KEY); ChromePreferenceKeys.FLAGS_CACHED_PAINT_PREVIEW_TEST_ENABLED_KEY);
put(ChromeFeatureList.TAB_GRID_LAYOUT_ANDROID,
ChromePreferenceKeys.FLAGS_CACHED_GRID_TAB_SWITCHER_ENABLED);
put(ChromeFeatureList.TAB_GROUPS_ANDROID,
ChromePreferenceKeys.FLAGS_CACHED_TAB_GROUPS_ANDROID_ENABLED);
put(ChromeFeatureList.DUET_TABSTRIP_INTEGRATION_ANDROID,
ChromePreferenceKeys.FLAGS_CACHED_DUET_TABSTRIP_INTEGRATION_ANDROID_ENABLED);
} }
}; };
...@@ -318,10 +328,11 @@ public class CachedFeatureFlags { ...@@ -318,10 +328,11 @@ public class CachedFeatureFlags {
@VisibleForTesting @VisibleForTesting
static void cacheNativeTabSwitcherUiFlags() { static void cacheNativeTabSwitcherUiFlags() {
if (isEligibleForTabUiExperiments()) { if (isEligibleForTabUiExperiments() && !DeviceClassManager.enableAccessibilityLayout()) {
cacheGridTabSwitcherEnabled(); List<String> featuresToCache = Arrays.asList(ChromeFeatureList.TAB_GRID_LAYOUT_ANDROID,
cacheTabGroupsAndroidEnabled(); ChromeFeatureList.TAB_GROUPS_ANDROID,
cacheDuetTabStripIntegrationAndroidEnabled(); ChromeFeatureList.DUET_TABSTRIP_INTEGRATION_ANDROID);
cacheNativeFlags(featuresToCache);
} }
} }
...@@ -334,22 +345,6 @@ public class CachedFeatureFlags { ...@@ -334,22 +345,6 @@ public class CachedFeatureFlags {
ChromePreferenceKeys.START_SURFACE_SINGLE_PANE_ENABLED_KEY, false); ChromePreferenceKeys.START_SURFACE_SINGLE_PANE_ENABLED_KEY, false);
} }
@VisibleForTesting
static void cacheGridTabSwitcherEnabled() {
SharedPreferencesManager sharedPreferencesManager = SharedPreferencesManager.getInstance();
String featureKey = ChromePreferenceKeys.FLAGS_CACHED_GRID_TAB_SWITCHER_ENABLED;
boolean shouldQueryFeatureFlag = !DeviceClassManager.enableAccessibilityLayout();
if (!shouldQueryFeatureFlag) {
sharedPreferencesManager.writeBoolean(featureKey, false);
return;
}
boolean queriedFlagValue =
ChromeFeatureList.isEnabled(ChromeFeatureList.TAB_GRID_LAYOUT_ANDROID);
boolean gridTabSwitcherEnabled =
queriedFlagValue && TabManagementModuleProvider.isTabManagementModuleSupported();
sharedPreferencesManager.writeBoolean(featureKey, gridTabSwitcherEnabled);
}
/** /**
* @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.
*/ */
...@@ -358,8 +353,8 @@ public class CachedFeatureFlags { ...@@ -358,8 +353,8 @@ public class CachedFeatureFlags {
// changing that setting while Chrome is alive. // 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 !(isTabGroupsAndroidContinuationChromeFlagEnabled() && SysUtils.isLowEndDevice()) return !(isTabGroupsAndroidContinuationChromeFlagEnabled() && SysUtils.isLowEndDevice())
&& getConsistentBooleanValue( && isEnabled(ChromeFeatureList.TAB_GRID_LAYOUT_ANDROID)
ChromePreferenceKeys.FLAGS_CACHED_GRID_TAB_SWITCHER_ENABLED, false) && TabUiFeatureUtilities.isTabManagementModuleSupported()
|| isTabGroupsAndroidEnabled() || isStartSurfaceEnabled(); || isTabGroupsAndroidEnabled() || isStartSurfaceEnabled();
} }
...@@ -369,48 +364,15 @@ public class CachedFeatureFlags { ...@@ -369,48 +364,15 @@ public class CachedFeatureFlags {
*/ */
@VisibleForTesting @VisibleForTesting
public static void setGridTabSwitcherEnabledForTesting(@Nullable Boolean enabled) { public static void setGridTabSwitcherEnabledForTesting(@Nullable Boolean enabled) {
sBoolValuesReturned.put( setForTesting(ChromeFeatureList.TAB_GRID_LAYOUT_ANDROID, enabled);
ChromePreferenceKeys.FLAGS_CACHED_GRID_TAB_SWITCHER_ENABLED, enabled);
}
@VisibleForTesting
static void cacheTabGroupsAndroidEnabled() {
SharedPreferencesManager sharedPreferencesManager = SharedPreferencesManager.getInstance();
String featureKey = ChromePreferenceKeys.FLAGS_CACHED_TAB_GROUPS_ANDROID_ENABLED;
boolean shouldQueryFeatureFlag = !DeviceClassManager.enableAccessibilityLayout();
if (!shouldQueryFeatureFlag) {
sharedPreferencesManager.writeBoolean(featureKey, false);
return;
}
boolean queriedFlagValue =
ChromeFeatureList.isEnabled(ChromeFeatureList.TAB_GROUPS_ANDROID);
boolean tabGroupsEnabled =
queriedFlagValue && TabManagementModuleProvider.isTabManagementModuleSupported();
sharedPreferencesManager.writeBoolean(featureKey, tabGroupsEnabled);
}
private static void cacheDuetTabStripIntegrationAndroidEnabled() {
SharedPreferencesManager sharedPreferencesManager = SharedPreferencesManager.getInstance();
String featureKey =
ChromePreferenceKeys.FLAGS_CACHED_DUET_TABSTRIP_INTEGRATION_ANDROID_ENABLED;
boolean shouldQueryFeatureFlag = !DeviceClassManager.enableAccessibilityLayout();
if (!shouldQueryFeatureFlag) {
sharedPreferencesManager.writeBoolean(featureKey, false);
return;
}
boolean queriedFlagValue =
ChromeFeatureList.isEnabled(ChromeFeatureList.DUET_TABSTRIP_INTEGRATION_ANDROID);
boolean duetTabStripIntegrationEnabled =
queriedFlagValue && TabManagementModuleProvider.isTabManagementModuleSupported();
sharedPreferencesManager.writeBoolean(featureKey, duetTabStripIntegrationEnabled);
} }
/** /**
* @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 getConsistentBooleanValue( return isEnabled(ChromeFeatureList.TAB_GROUPS_ANDROID)
ChromePreferenceKeys.FLAGS_CACHED_TAB_GROUPS_ANDROID_ENABLED, false); && TabUiFeatureUtilities.isTabManagementModuleSupported();
} }
/** /**
...@@ -419,8 +381,7 @@ public class CachedFeatureFlags { ...@@ -419,8 +381,7 @@ public class CachedFeatureFlags {
*/ */
@VisibleForTesting @VisibleForTesting
public static void setTabGroupsAndroidEnabledForTesting(@Nullable Boolean available) { public static void setTabGroupsAndroidEnabledForTesting(@Nullable Boolean available) {
sBoolValuesReturned.put( setForTesting(ChromeFeatureList.TAB_GROUPS_ANDROID, available);
ChromePreferenceKeys.FLAGS_CACHED_TAB_GROUPS_ANDROID_ENABLED, available);
} }
/** /**
...@@ -440,9 +401,7 @@ public class CachedFeatureFlags { ...@@ -440,9 +401,7 @@ public class CachedFeatureFlags {
@VisibleForTesting @VisibleForTesting
public static void setDuetTabStripIntegrationAndroidEnabledForTesting( public static void setDuetTabStripIntegrationAndroidEnabledForTesting(
@Nullable Boolean isEnabled) { @Nullable Boolean isEnabled) {
sBoolValuesReturned.put( setForTesting(ChromeFeatureList.DUET_TABSTRIP_INTEGRATION_ANDROID, isEnabled);
ChromePreferenceKeys.FLAGS_CACHED_DUET_TABSTRIP_INTEGRATION_ANDROID_ENABLED,
isEnabled);
} }
private static boolean isEligibleForTabUiExperiments() { private static boolean isEligibleForTabUiExperiments() {
...@@ -478,10 +437,9 @@ public class CachedFeatureFlags { ...@@ -478,10 +437,9 @@ public class CachedFeatureFlags {
* @return Whether the tab strip and duet integration feature is enabled and available for use. * @return Whether the tab strip and duet integration feature is enabled and available for use.
*/ */
public static boolean isDuetTabStripIntegrationAndroidEnabled() { public static boolean isDuetTabStripIntegrationAndroidEnabled() {
return isTabGroupsAndroidEnabled() return isEnabled(ChromeFeatureList.TAB_GROUPS_ANDROID)
&& getConsistentBooleanValue( && isEnabled(ChromeFeatureList.DUET_TABSTRIP_INTEGRATION_ANDROID)
ChromePreferenceKeys.FLAGS_CACHED_DUET_TABSTRIP_INTEGRATION_ANDROID_ENABLED, && TabUiFeatureUtilities.isTabManagementModuleSupported();
false);
} }
/** /**
......
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