Skip to content

Commit

Permalink
Remove Modern flag and hardcode #isChromeModernDesignEnabled to true
Browse files Browse the repository at this point in the history
Remove the feature flag that controls whether Modern is enabled,
effectively enabling the feature at 100%. Fix up tests that were
previously running with Modern disabled. Removes some "Chrome classic"
render tests.

This CL effectively enables Modern and "contextual suggestions" (EoC),
which relies on Modern being enabled, for all tests.

BUG=876051

Change-Id: I9938dabbba238f86d42356325d42ace581512d25
Reviewed-on: https://chromium-review.googlesource.com/1183684
Commit-Queue: Theresa <twellington@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Filip Gorski <fgorski@chromium.org>
Reviewed-by: Gayane Petrosyan <gayane@chromium.org>
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585143}
  • Loading branch information
Theresa authored and Commit Bot committed Aug 22, 2018
1 parent 95ddb1c commit 5955e6d
Show file tree
Hide file tree
Showing 89 changed files with 105 additions and 446 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,6 @@ public static boolean getFieldTrialParamByFeatureAsBoolean(
public static final String CHROME_MEMEX = "ChromeMemex";
public static final String CHROME_MODERN_ALTERNATE_CARD_LAYOUT =
"ChromeModernAlternateCardLayout";
public static final String CHROME_MODERN_DESIGN = "ChromeModernDesign";
public static final String CHROME_MODERN_FULL_ROLL = "ChromeModernFullRoll";
public static final String CHROME_SMART_SELECTION = "ChromeSmartSelection";
public static final String CLEAR_OLD_BROWSING_DATA = "ClearOldBrowsingData";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,11 @@ public class ChromePreferenceManager {
*/
public static final String CHROME_DEFAULT_BROWSER = "applink.chrome_default_browser";

/**
* Deprecated in M70. This value may still exist in the shared preferences file. Do not reuse.
* TODO(twellington): Remove preference from the file in a future pref cleanup effort.
*/
@Deprecated
private static final String CHROME_MODERN_DESIGN_ENABLED_KEY = "chrome_modern_design_enabled";

/**
Expand Down Expand Up @@ -401,21 +406,6 @@ public void setNewTabPageSigninPromoSuppressionPeriodStart(long timeMillis) {
writeLong(NTP_SIGNIN_PROMO_SUPPRESSION_PERIOD_START, timeMillis);
}

/**
* @return Whether Chrome modern design is enabled.
*/
public boolean isChromeModernDesignEnabled() {
return mSharedPreferences.getBoolean(CHROME_MODERN_DESIGN_ENABLED_KEY, false);
}

/**
* Set whether or not Chrome modern design is enabled.
* @param isEnabled Whether the feature is enabled.
*/
public void setChromeModernDesignEnabled(boolean isEnabled) {
writeBoolean(CHROME_MODERN_DESIGN_ENABLED_KEY, isEnabled);
}

/**
* Removes the stored timestamp of the suppression period start when signin promos in the New
* Tab Page are no longer suppressed.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import org.chromium.base.metrics.RecordHistogram;
import org.chromium.base.metrics.RecordUserAction;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.ChromeFeatureList;
import org.chromium.chrome.browser.ChromeSwitches;
import org.chromium.chrome.browser.infobar.InfoBarContainer;
import org.chromium.chrome.browser.infobar.InfoBarContainerLayout.Item;
Expand Down Expand Up @@ -322,10 +321,6 @@ boolean isRandomlySelectedForSurvey() {
}

int maxNumber = getMaxNumber();
if (FeatureUtilities.isChromeModernDesignEnabled()) {
maxNumber = ChromeFeatureList.getFieldTrialParamByFeatureAsInt(
ChromeFeatureList.CHROME_MODERN_DESIGN, MAX_NUMBER, maxNumber);
}
if (maxNumber == -1) {
recordSurveyFilteringResult(FilteringResult.MAX_NUMBER_MISSING);
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ public class FeatureUtilities {
private static String sChromeHomeSwipeLogicType;

private static Boolean sIsSoleEnabled;
private static Boolean sIsChromeModernDesignEnabled;
private static Boolean sIsHomePageButtonForceEnabled;
private static Boolean sIsHomepageTileEnabled;
private static Boolean sIsNewTabPageButtonEnabled;
Expand Down Expand Up @@ -164,7 +163,6 @@ public static void cacheNativeFlags() {
cacheSoleEnabled();
cacheCommandLineOnNonRootedEnabled();
FirstRunUtils.cacheFirstRunPrefs();
cacheChromeModernDesignEnabled();
cacheHomePageButtonForceEnabled();
cacheHomepageTileEnabled();
cacheNewTabPageButtonEnabledAndMaybeVariant();
Expand All @@ -188,18 +186,6 @@ public static boolean isTabModelMergingEnabled() {
return Build.VERSION.SDK_INT > Build.VERSION_CODES.M;
}

/**
* Cache whether or not modern design is enabled so on next startup, the value can be made
* available immediately.
*/
public static void cacheChromeModernDesignEnabled() {
boolean isModernEnabled =
ChromeFeatureList.isEnabled(ChromeFeatureList.CHROME_MODERN_DESIGN);

ChromePreferenceManager manager = ChromePreferenceManager.getInstance();
manager.setChromeModernDesignEnabled(isModernEnabled);
}

/**
* Cache whether or not the home page button is force enabled so on next startup, the value can
* be made available immediately.
Expand Down Expand Up @@ -415,26 +401,12 @@ public static boolean isSoleEnabled() {
}

/**
* Resets whether Chrome modern design is enabled for tests. After this is called, the next
* call to #isChromeModernDesignEnabled() will retrieve the value from shared preferences.
*/
public static void resetChromeModernDesignEnabledForTests() {
sIsChromeModernDesignEnabled = null;
}

/**
* Deprecated!
* @return Whether Chrome modern design is enabled. This returns true if Chrome Home is enabled.
*/
@CalledByNative
public static boolean isChromeModernDesignEnabled() {
if (sIsChromeModernDesignEnabled == null) {
ChromePreferenceManager prefManager = ChromePreferenceManager.getInstance();
try (StrictModeContext unused = StrictModeContext.allowDiskReads()) {
sIsChromeModernDesignEnabled = prefManager.isChromeModernDesignEnabled();
}
}

return sIsChromeModernDesignEnabled;
return true;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ public void setTabModelSelector(
}

@VisibleForTesting
public ViewGroup getContainer() {
public AccessibilityTabModelWrapper getContainer() {
return mTabModelWrapper;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import android.widget.ListView;

import org.chromium.base.ApiCompatibilityUtils;
import org.chromium.base.VisibleForTesting;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.ChromeFeatureList;
import org.chromium.chrome.browser.tab.Tab;
Expand Down Expand Up @@ -283,4 +284,14 @@ protected void onDetachedFromWindow() {
mIsAttachedToWindow = false;
super.onDetachedFromWindow();
}

@VisibleForTesting
public TabLayout.Tab getIncognitoTabsButton() {
return mModernIncognitoButton;
}

@VisibleForTesting
public TabLayout.Tab getStandardTabsButton() {
return mModernStandardButton;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@

package org.chromium.chrome.browser;

import static junit.framework.Assert.assertTrue;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.core.IsCollectionContaining.hasItems;
Expand All @@ -18,10 +16,8 @@

import org.chromium.base.CommandLine;
import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.chrome.browser.util.FeatureUtilities;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.chrome.test.ChromeTabbedActivityTestRule;
import org.chromium.chrome.test.util.browser.ChromeModernDesign;
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;
Expand Down Expand Up @@ -58,25 +54,6 @@ public void testFeaturesSetExistingFlags() throws InterruptedException {
assertThat(finalDisabledList.size(), equalTo(1));
}

/**
* Tests the compatibility between {@link EnableFeatures} and other rules.
* {@link @ChromeModernDesign} here explicitly calls {@link Features#enable(String...)}, so
* its feature should also be added to the set of registered flags.
*/
@Test
@SmallTest
@ChromeModernDesign.Enable
@EnableFeatures("One")
public void testFeaturesIncludeValuesSetFromOtherRules() throws InterruptedException {
mActivityRule.startMainActivityOnBlankPage();

List<String> finalEnabledList = getArgsList(true);
assertThat(finalEnabledList, hasItems("One", ChromeFeatureList.CHROME_MODERN_DESIGN));
assertTrue(ChromeFeatureList.isEnabled(ChromeFeatureList.CHROME_MODERN_DESIGN));
assertTrue("ChromeModernDesign should be enabled.",
FeatureUtilities.isChromeModernDesignEnabled());
}

/**
* Tests the compatibility between the legacy {@link CommandLineFlags} annotation usage for
* features and the new dedicated annotations.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,57 +20,25 @@

import org.chromium.base.ApiCompatibilityUtils;
import org.chromium.base.ThreadUtils;
import org.chromium.base.test.params.ParameterAnnotations;
import org.chromium.base.test.params.ParameterProvider;
import org.chromium.base.test.params.ParameterSet;
import org.chromium.base.test.params.ParameterizedRunner;
import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.base.test.util.MinAndroidSdkLevel;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.ntp.NewTabPageTest.ModernParams;
import org.chromium.chrome.test.ChromeJUnit4RunnerDelegate;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.chrome.test.ChromeTabbedActivityTestRule;
import org.chromium.chrome.test.util.ChromeTabUtils;
import org.chromium.chrome.test.util.browser.ChromeModernDesign;

import java.util.Arrays;

/** Tests for the NavigationBarColorController. */
@RunWith(ParameterizedRunner.class)
@ParameterAnnotations.UseRunnerDelegate(ChromeJUnit4RunnerDelegate.class)
@RunWith(ChromeJUnit4ClassRunner.class)
@CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE})
@MinAndroidSdkLevel(Build.VERSION_CODES.O_MR1)
@TargetApi(Build.VERSION_CODES.O_MR1)
public class NavigationBarColorControllerTest {
@Rule
public ChromeTabbedActivityTestRule mActivityTestRule = new ChromeTabbedActivityTestRule();

/** Parameter provider for enabling/disabling Chrome Modern. */
public static class ModernParams implements ParameterProvider {
@Override
public Iterable<ParameterSet> getParameters() {
return Arrays.asList(new ParameterSet().value(false).name("DisableChromeModern"),
new ParameterSet().value(true).name("EnableChromeModern"));
}
}

private ChromeModernDesign.Processor mChromeModernProcessor =
new ChromeModernDesign.Processor();

private Window mWindow;
private int mLightNavigationColor;
private int mDarkNavigationColor;

@ParameterAnnotations.UseMethodParameterBefore(ModernParams.class)
public void setupModernDesign(boolean enabled) {
mChromeModernProcessor.setPrefs(enabled);
}

@ParameterAnnotations.UseMethodParameterAfter(ModernParams.class)
public void teardownModernDesign(boolean enabled) {
mChromeModernProcessor.clearTestState();
}

@Before
public void setUp() throws InterruptedException {
mActivityTestRule.startMainActivityOnBlankPage();
Expand All @@ -82,21 +50,15 @@ public void setUp() throws InterruptedException {

@Test
@SmallTest
@ParameterAnnotations.UseMethodParameter(ModernParams.class)
public void testToggleOverview(boolean modern) {
public void testToggleOverview() {
assertEquals("Navigation bar should be white before entering overview mode.",
mLightNavigationColor, mWindow.getNavigationBarColor());

ThreadUtils.runOnUiThreadBlocking(
() -> mActivityTestRule.getActivity().getLayoutManager().showOverview(false));

if (modern) {
assertEquals("Navigation bar should be white in overview mode.", mLightNavigationColor,
mWindow.getNavigationBarColor());
} else {
assertEquals("Navigation bar should be black in overview mode.", mDarkNavigationColor,
mWindow.getNavigationBarColor());
}
assertEquals("Navigation bar should be white in overview mode.", mLightNavigationColor,
mWindow.getNavigationBarColor());

ThreadUtils.runOnUiThreadBlocking(
() -> mActivityTestRule.getActivity().getLayoutManager().hideOverview(false));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@
import org.chromium.chrome.test.util.ChromeTabUtils;
import org.chromium.chrome.test.util.MenuUtils;
import org.chromium.chrome.test.util.RenderTestRule;
import org.chromium.chrome.test.util.browser.ChromeModernDesign;
import org.chromium.components.bookmarks.BookmarkId;
import org.chromium.components.bookmarks.BookmarkType;
import org.chromium.content.browser.test.util.TouchCommon;
Expand Down Expand Up @@ -382,7 +381,6 @@ public void testSearchBookmarks_Delete() throws Exception {
@Test
@MediumTest
@Feature({"RenderTest"})
@ChromeModernDesign.Enable
public void testBookmarkFolderIcon() throws Exception {
BookmarkPromoHeader.forcePromoStateForTests(BookmarkPromoHeader.PromoState.PROMO_NONE);
addFolder(TEST_FOLDER_TITLE);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@
import org.chromium.chrome.test.util.ChromeTabUtils;
import org.chromium.chrome.test.util.RenderTestRule;
import org.chromium.chrome.test.util.ViewUtils;
import org.chromium.chrome.test.util.browser.ChromeModernDesign;
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;
Expand Down Expand Up @@ -97,7 +96,6 @@
@Restriction(UiRestriction.RESTRICTION_TYPE_PHONE)
@CommandLineFlags.Add(ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE)
@EnableFeatures(ChromeFeatureList.CONTEXTUAL_SUGGESTIONS_BOTTOM_SHEET)
@ChromeModernDesign.Enable
public class ContextualSuggestionsTest {
@Rule
public ChromeTabbedActivityTestRule mActivityTestRule = new ChromeTabbedActivityTestRule();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import org.chromium.chrome.browser.sync.ProfileSyncService;
import org.chromium.chrome.test.ChromeActivityTestRule;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.chrome.test.util.browser.ChromeModernDesign;
import org.chromium.chrome.test.util.browser.Features.EnableFeatures;
import org.chromium.components.signin.ChromeSigninController;
import org.chromium.policy.test.annotations.Policies;
Expand All @@ -37,7 +36,6 @@
@Restriction(UiRestriction.RESTRICTION_TYPE_PHONE)
@Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE})
@EnableFeatures(ChromeFeatureList.CONTEXTUAL_SUGGESTIONS_BOTTOM_SHEET)
@ChromeModernDesign.Enable
public class EnabledStateMonitorTest implements EnabledStateMonitor.Observer {
@Rule
public ChromeActivityTestRule<ChromeActivity> mActivityTestRule =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@
import org.chromium.chrome.test.util.FullscreenTestUtils;
import org.chromium.chrome.test.util.MenuUtils;
import org.chromium.chrome.test.util.OmniboxTestUtils;
import org.chromium.chrome.test.util.browser.Features;
import org.chromium.components.navigation_interception.NavigationParams;
import org.chromium.content.browser.test.util.Criteria;
import org.chromium.content.browser.test.util.CriteriaHelper;
Expand Down Expand Up @@ -2855,10 +2856,14 @@ public void testNetworkDisconnectedDeactivatesSearch()
/**
* Tests that the quick action caption is set correctly when one is available. Also tests that
* the caption gets changed when the panel is expanded and reset when the panel is closed.
* TODO(https://crbug.com/831783): Remove the DisableFeatures block turning off contextual
* suggestions.
*/
@Test
@SmallTest
@Feature({"ContextualSearch"})
@Features.DisableFeatures({ChromeFeatureList.CONTEXTUAL_SUGGESTIONS_BOTTOM_SHEET,
ChromeFeatureList.CONTEXTUAL_SUGGESTIONS_BUTTON})
public void testQuickActionCaptionAndImage() throws InterruptedException, TimeoutException {
mPanel.getAnimationHandler().enableTestingMode();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -897,7 +897,12 @@ public void testToolbarColor() throws InterruptedException {
.getToolbarManager()
.getToolbarModelForTesting()
.shouldEmphasizeHttpsScheme());
if (Build.VERSION.SDK_INT > Build.VERSION_CODES.LOLLIPOP) {
// TODO(https://crbug.com/871805): Use helper class to determine whether dark status icons
// are supported.
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
Assert.assertEquals(expectedColor,
mCustomTabActivityTestRule.getActivity().getWindow().getStatusBarColor());
} else if (Build.VERSION.SDK_INT > Build.VERSION_CODES.LOLLIPOP) {
Assert.assertEquals(ColorUtils.getDarkenedColorForStatusBar(expectedColor),
mCustomTabActivityTestRule.getActivity().getWindow().getStatusBarColor());
}
Expand Down
Loading

0 comments on commit 5955e6d

Please sign in to comment.