Commit b439253e authored by Nicolas Dossou-gbete's avatar Nicolas Dossou-gbete Committed by Commit Bot

🔬 Cleanup @ChromeHome usage

Simple cleanup change. Replaces some uses of @CommandLineFlags with
@ChromeHome and @EnableFeatures, and removes unused code from
@ChromeHome.

Bug: 783160
Change-Id: I508ea74b923360fde65eb2e247c727aab825f8a9
Reviewed-on: https://chromium-review.googlesource.com/788214Reviewed-by: default avatarPeter Conn <peconn@chromium.org>
Reviewed-by: default avatarTed Choc <tedchoc@chromium.org>
Commit-Queue: Nicolas Dossou-Gbété <dgn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524386}
parent 6ea24ace
......@@ -20,7 +20,6 @@ import org.junit.runner.RunWith;
import org.chromium.base.CommandLine;
import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.base.test.util.Restriction;
import org.chromium.chrome.browser.util.FeatureUtilities;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.chrome.test.ChromeTabbedActivityTestRule;
......@@ -29,7 +28,6 @@ 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 org.chromium.ui.base.DeviceFormFactor;
import org.chromium.ui.test.util.UiRestriction;
import java.util.Arrays;
import java.util.List;
......@@ -76,8 +74,7 @@ public class FeaturesAnnotationsTest {
*/
@Test
@SmallTest
@Restriction(UiRestriction.RESTRICTION_TYPE_PHONE)
@ChromeHome
@ChromeHome.Enable
@EnableFeatures("One")
public void testFeaturesIncludeValuesSetFromOtherRules() throws InterruptedException {
mActivityRule.startMainActivityOnBlankPage();
......
......@@ -16,7 +16,6 @@ import org.junit.runner.RunWith;
import org.chromium.base.ThreadUtils;
import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.base.test.util.Feature;
import org.chromium.base.test.util.Restriction;
import org.chromium.chrome.browser.ChromeActivity;
import org.chromium.chrome.browser.ChromeSwitches;
import org.chromium.chrome.browser.ntp.cards.AllDismissedItem.ViewHolder;
......@@ -24,7 +23,6 @@ import org.chromium.chrome.test.ChromeActivityTestRule;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.chrome.test.util.RenderTestRule;
import org.chromium.chrome.test.util.browser.ChromeHome;
import org.chromium.ui.test.util.UiRestriction;
import java.io.IOException;
......@@ -63,7 +61,7 @@ public class AllDismissedItemTest {
@Test
@MediumTest
@Feature({"Cards", "RenderTest"})
@ChromeHome(false)
@ChromeHome.Disable
public void testNewTabPageAppearance() throws IOException {
SectionList sectionList = null; // The SectionList is only used if the item is clicked on.
ViewHolder viewHolder = new ViewHolder(mContentView, sectionList);
......@@ -76,8 +74,7 @@ public class AllDismissedItemTest {
@Test
@MediumTest
@Feature({"Cards", "RenderTest"})
@ChromeHome
@Restriction(UiRestriction.RESTRICTION_TYPE_PHONE)
@ChromeHome.Enable
public void testChromeHomeAppearance() throws IOException {
renderAtHour(new ViewHolder(mContentView, null), 0, "modern");
}
......
......@@ -4,6 +4,8 @@
package org.chromium.chrome.browser.suggestions;
import static org.chromium.chrome.test.util.browser.Features.EnableFeatures;
import android.support.annotation.Nullable;
import android.support.test.InstrumentationRegistry;
import android.support.test.filters.MediumTest;
......@@ -15,6 +17,7 @@ import org.junit.Assert;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TestRule;
import org.junit.runner.RunWith;
import org.chromium.base.test.util.CommandLineFlags;
......@@ -31,6 +34,7 @@ import org.chromium.chrome.test.ChromeActivityTestRule;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.chrome.test.util.RenderTestRule;
import org.chromium.chrome.test.util.browser.ChromeHome;
import org.chromium.chrome.test.util.browser.Features;
import org.chromium.chrome.test.util.browser.suggestions.FakeSuggestionsSource;
import org.chromium.chrome.test.util.browser.suggestions.SuggestionsDependenciesRule;
import org.chromium.net.test.EmbeddedTestServerRule;
......@@ -46,11 +50,8 @@ import java.util.concurrent.TimeoutException;
*/
@RunWith(ChromeJUnit4ClassRunner.class)
@CommandLineFlags.Add(ChromeActivityTestRule.DISABLE_NETWORK_PREDICTION_FLAG)
@CommandLineFlags.Remove(ChromeHome.ENABLE_FLAGS)
@Restriction(UiRestriction.RESTRICTION_TYPE_PHONE)
public class ContextualSuggestionsTest {
private static final String ENABLE_CONTEXTUAL_SUGGESTIONS = ChromeHome.ENABLE_FLAGS + ","
+ ChromeFeatureList.CONTEXTUAL_SUGGESTIONS_CAROUSEL;
private static final String TEST_PAGE = "/chrome/test/data/android/test.html";
private static final List<SnippetArticle> FAKE_CONTEXTUAL_SUGGESTIONS = Arrays.asList(
......@@ -84,7 +85,10 @@ public class ContextualSuggestionsTest {
0L, // fetchTimestamp
false, // isVideoSuggestion
null)); // thumbnailDominantColor
@Rule
public TestRule mChromeHomeStateRule = new ChromeHome.Processor();
@Rule
public TestRule mProcessor = new Features.InstrumentationProcessor();
@Rule
public SuggestionsDependenciesRule mSuggestionsDeps = new SuggestionsDependenciesRule();
@Rule
......@@ -113,7 +117,7 @@ public class ContextualSuggestionsTest {
@Test
@SmallTest
@Feature({"ContextualSuggestions"})
@CommandLineFlags.Add(ChromeHome.ENABLE_FLAGS)
@ChromeHome.Enable
public void testCarouselIsNotShownWhenFlagIsDisabled() {
Assert.assertFalse(
ChromeFeatureList.isEnabled(ChromeFeatureList.CONTEXTUAL_SUGGESTIONS_CAROUSEL));
......@@ -133,7 +137,8 @@ public class ContextualSuggestionsTest {
+ "suggestions to show. This test covers the desired behaviour and should be"
+ "enabled when this is handled properly.")
@Feature({"ContextualSuggestions"})
@CommandLineFlags.Add(ENABLE_CONTEXTUAL_SUGGESTIONS)
@ChromeHome.Enable
@EnableFeatures(ChromeFeatureList.CONTEXTUAL_SUGGESTIONS_CAROUSEL)
public void testCarouselIsNotShownWhenFlagsEnabledButNoSuggestions() {
Assert.assertTrue(
ChromeFeatureList.isEnabled(ChromeFeatureList.CONTEXTUAL_SUGGESTIONS_CAROUSEL));
......@@ -148,7 +153,8 @@ public class ContextualSuggestionsTest {
@Test
@SmallTest
@Feature({"ContextualSuggestions"})
@CommandLineFlags.Add(ENABLE_CONTEXTUAL_SUGGESTIONS)
@ChromeHome.Enable
@EnableFeatures(ChromeFeatureList.CONTEXTUAL_SUGGESTIONS_CAROUSEL)
public void testCarouselIsShownWhenItReceivedSuggestions()
throws InterruptedException, TimeoutException {
mSuggestionsSource.setContextualSuggestions(FAKE_CONTEXTUAL_SUGGESTIONS);
......@@ -172,7 +178,8 @@ public class ContextualSuggestionsTest {
@Test
@MediumTest
@Feature({"ContextualSuggestions", "RenderTest"})
@CommandLineFlags.Add(ENABLE_CONTEXTUAL_SUGGESTIONS)
@ChromeHome.Enable
@EnableFeatures(ChromeFeatureList.CONTEXTUAL_SUGGESTIONS_CAROUSEL)
public void testCardAppearance() throws InterruptedException, TimeoutException, IOException {
mSuggestionsSource.setContextualSuggestions(FAKE_CONTEXTUAL_SUGGESTIONS);
......
......@@ -35,7 +35,6 @@ import org.chromium.base.test.util.CallbackHelper;
import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.base.test.util.DisabledTest;
import org.chromium.base.test.util.Feature;
import org.chromium.base.test.util.Restriction;
import org.chromium.base.test.util.RetryOnFailure;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.ChromeActivity;
......@@ -64,7 +63,6 @@ import org.chromium.chrome.test.util.browser.suggestions.SuggestionsDependencies
import org.chromium.content.browser.test.util.Criteria;
import org.chromium.content.browser.test.util.CriteriaHelper;
import org.chromium.net.test.EmbeddedTestServerRule;
import org.chromium.ui.test.util.UiRestriction;
import java.io.IOException;
import java.util.ArrayList;
......@@ -173,8 +171,7 @@ public class TileGridLayoutTest {
//@MediumTest
@DisabledTest(message = "crbug.com/771648")
@Feature({"NewTabPage", "RenderTest"})
@ChromeHome
@Restriction(UiRestriction.RESTRICTION_TYPE_PHONE)
@ChromeHome.Enable
public void testModernTileGridAppearance_Full() throws IOException, InterruptedException {
View tileGridLayout = renderTiles(makeSuggestions(FAKE_MOST_VISITED_URLS.length));
......@@ -196,7 +193,7 @@ public class TileGridLayoutTest {
//@MediumTest
@DisabledTest(message = "crbug.com/771648")
@Feature({"NewTabPage", "RenderTest"})
@ChromeHome(false)
@ChromeHome.Disable
public void testTileGridAppearance_Full() throws IOException, InterruptedException {
View tileGridLayout = renderTiles(makeSuggestions(FAKE_MOST_VISITED_URLS.length));
......@@ -219,8 +216,7 @@ public class TileGridLayoutTest {
@DisabledTest(message = "crbug.com/771648")
@RetryOnFailure
@Feature({"NewTabPage", "RenderTest"})
@ChromeHome
@Restriction(UiRestriction.RESTRICTION_TYPE_PHONE)
@ChromeHome.Enable
public void testModernTileGridAppearance_Two() throws IOException, InterruptedException {
View tileGridLayout = renderTiles(makeSuggestions(2));
......@@ -236,7 +232,7 @@ public class TileGridLayoutTest {
@DisabledTest(message = "crbug.com/771648")
@RetryOnFailure
@Feature({"NewTabPage", "RenderTest"})
@ChromeHome(false)
@ChromeHome.Disable
public void testTileGridAppearance_Two() throws IOException, InterruptedException {
View tileGridLayout = renderTiles(makeSuggestions(2));
......@@ -250,8 +246,7 @@ public class TileGridLayoutTest {
@Test
@MediumTest
@Feature({"NewTabPage", "RenderTest"})
@ChromeHome
@Restriction(UiRestriction.RESTRICTION_TYPE_PHONE)
@ChromeHome.Enable
public void testTileAppearanceModern()
throws IOException, InterruptedException, TimeoutException {
List<SiteSuggestion> suggestions = makeSuggestions(2);
......@@ -267,7 +262,7 @@ public class TileGridLayoutTest {
@Test
@MediumTest
@Feature({"NewTabPage", "RenderTest"})
@ChromeHome(false)
@ChromeHome.Disable
public void testTileAppearanceClassic()
throws IOException, InterruptedException, TimeoutException {
List<SiteSuggestion> suggestions = makeSuggestions(2);
......
......@@ -15,19 +15,20 @@ import org.chromium.base.ThreadUtils;
import org.chromium.base.test.util.CallbackHelper;
import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.ChromeFeatureList;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheet;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheet.BottomSheetContent;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheet.StateChangeReason;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetContentController;
import org.chromium.chrome.browser.widget.bottomsheet.EmptyBottomSheetObserver;
import org.chromium.chrome.test.util.browser.ChromeHome;
import org.chromium.chrome.test.util.browser.Features;
import org.chromium.chrome.test.util.browser.RecyclerViewTestUtils;
/**
* Junit4 rule for tests testing the Chrome Home bottom sheet.
*/
@CommandLineFlags.Add({ChromeHome.ENABLE_FLAGS, DISABLE_FIRST_RUN_EXPERIENCE,
DISABLE_NETWORK_PREDICTION_FLAG})
@CommandLineFlags.Add({DISABLE_FIRST_RUN_EXPERIENCE, DISABLE_NETWORK_PREDICTION_FLAG})
public class BottomSheetTestRule extends ChromeTabbedActivityTestRule {
/** An observer used to record events that occur with respect to the bottom sheet. */
public static class Observer extends EmptyBottomSheetObserver {
......@@ -98,6 +99,7 @@ public class BottomSheetTestRule extends ChromeTabbedActivityTestRule {
private @BottomSheet.SheetState int mStartingBottomSheetState = BottomSheet.SHEET_STATE_FULL;
protected void beforeStartingActivity() {
Features.getInstance().enable(ChromeFeatureList.CHROME_HOME);
mChromeHomeEnabler.setPrefs(true);
}
......
......@@ -16,7 +16,6 @@ import org.chromium.chrome.browser.preferences.ChromePreferenceManager;
import org.chromium.chrome.browser.util.FeatureUtilities;
import org.chromium.ui.test.util.UiRestriction;
import java.lang.annotation.Annotation;
import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
......@@ -29,9 +28,7 @@ import java.lang.annotation.Target;
* @see ChromeHome.Processor
* @see FeatureUtilities#isChromeHomeEnabled()
*/
@Retention(RetentionPolicy.RUNTIME)
@Target({ElementType.METHOD, ElementType.TYPE})
public @interface ChromeHome {
public interface ChromeHome {
@Retention(RetentionPolicy.RUNTIME)
@Target({ElementType.METHOD, ElementType.TYPE})
@Restriction(UiRestriction.RESTRICTION_TYPE_PHONE)
......@@ -43,11 +40,6 @@ public @interface ChromeHome {
@Features.DisableFeatures(ChromeFeatureList.CHROME_HOME)
@interface Disable {}
boolean value() default true;
String FEATURES = ChromeFeatureList.CHROME_HOME;
String ENABLE_FLAGS = "enable-features=" + FEATURES;
/**
* Rule to handle setting and resetting the cached feature state for ChromeHome. Can be used by
* explicitly calling methods ({@link #setPrefs(boolean)} and {@link #clearTestState()}) or by
......@@ -57,23 +49,12 @@ public @interface ChromeHome {
private Boolean mOldState;
public Processor() {
super(ChromeHome.class, ChromeHome.Enable.class, ChromeHome.Disable.class);
super(ChromeHome.Enable.class, ChromeHome.Disable.class);
}
@Override
protected void before() throws Throwable {
boolean featureEnabled;
Annotation annotation = getClosestAnnotation();
if (annotation instanceof ChromeHome) {
featureEnabled = ((ChromeHome) annotation).value();
if (featureEnabled) {
Features.getInstance().enable(ChromeFeatureList.CHROME_HOME);
} else {
Features.getInstance().disable(ChromeFeatureList.CHROME_HOME);
}
} else {
featureEnabled = annotation instanceof ChromeHome.Enable;
}
boolean featureEnabled = getClosestAnnotation() instanceof ChromeHome.Enable;
setPrefs(featureEnabled);
}
......
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