[NTP Customization] Hide the "Discover feed" option in the main bottom sheet when necessary
This CL adds the logics to hide the "Discover feed" option in the main
bottom sheet if the feed section is completely removed from the new tab
page. In other words, it is removed if the user is in one of the EEA
countries and the searching engine using is not the google one.
The change of the CL is shown in the following documentation:
https://docs.google.com/document/d/1bMRVyyqOd0-cAmyTbL9IhKNU9uSvDdpdCngSzSXIHnk/edit?tab=t.0#bookmark=id.yhcjjq3eiz9t
Bug: 376238770
Change-Id: I8a8d51788952339202334cc7c5764d2e6c423ff9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6397805
Reviewed-by: Jian Li <jianli@chromium.org>
Reviewed-by: Xi Han <hanxi@chromium.org>
Reviewed-by: Calder Kitagawa <ckitagawa@chromium.org>
Commit-Queue: Yanling Hu <yanlinghu@google.com>
Cr-Commit-Position: refs/heads/main@{#1442284}
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ChromeTabbedActivity.java b/chrome/android/java/src/org/chromium/chrome/browser/ChromeTabbedActivity.java
index 2b16357f..779d6b4 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/ChromeTabbedActivity.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/ChromeTabbedActivity.java
@@ -3307,7 +3307,10 @@
mTabModelSelector.selectModel(false);
RecordUserAction.record("MobileMenuSwitchOutOfIncognito");
} else if (id == R.id.ntp_customization_id) {
- new NtpCustomizationCoordinator(this, mRootUiCoordinator.getBottomSheetController())
+ new NtpCustomizationCoordinator(
+ this,
+ mRootUiCoordinator.getBottomSheetController(),
+ getProfileProviderSupplier())
.showBottomSheet();
NtpCustomizationMetricsUtils.recordOpenBottomSheetEntry(
NtpCustomizationCoordinator.EntryPointType.MAIN_MENU);
diff --git a/chrome/browser/ntp_customization/BUILD.gn b/chrome/browser/ntp_customization/BUILD.gn
index 7bb259b..6f0594a9 100644
--- a/chrome/browser/ntp_customization/BUILD.gn
+++ b/chrome/browser/ntp_customization/BUILD.gn
@@ -31,8 +31,10 @@
"//chrome/browser/feed/android:java",
"//chrome/browser/magic_stack/android:java",
"//chrome/browser/profiles/android:java",
+ "//chrome/browser/search_engines/android:java",
"//components/browser_ui/bottomsheet/android:java",
"//components/browser_ui/widget/android:java",
+ "//components/search_engines/android:java",
"//third_party/android_deps:com_android_support_support_annotations_java",
"//third_party/androidx:androidx_annotation_annotation_java",
"//third_party/androidx:androidx_appcompat_appcompat_java",
@@ -92,10 +94,14 @@
"//base:base_java_test_support",
"//base:base_junit_test_support",
"//chrome/android:chrome_java",
+ "//chrome/browser/feed/android:java",
"//chrome/browser/magic_stack/android:java",
+ "//chrome/browser/preferences:java",
+ "//chrome/browser/profiles/android:java",
"//components/browser_ui/bottomsheet/android:java",
"//components/browser_ui/theme/android:java_resources",
"//components/browser_ui/widget/android:java",
+ "//components/prefs/android:java",
"//third_party/androidx:androidx_core_core_java",
"//third_party/androidx:androidx_test_core_java",
"//third_party/androidx:androidx_test_runner_java",
diff --git a/chrome/browser/ntp_customization/java/src/org/chromium/chrome/browser/ntp_customization/NtpCustomizationCoordinator.java b/chrome/browser/ntp_customization/java/src/org/chromium/chrome/browser/ntp_customization/NtpCustomizationCoordinator.java
index 0e25de4..2fb8f96 100644
--- a/chrome/browser/ntp_customization/java/src/org/chromium/chrome/browser/ntp_customization/NtpCustomizationCoordinator.java
+++ b/chrome/browser/ntp_customization/java/src/org/chromium/chrome/browser/ntp_customization/NtpCustomizationCoordinator.java
@@ -18,7 +18,9 @@
import android.view.View;
import android.widget.ViewFlipper;
+import org.chromium.base.supplier.Supplier;
import org.chromium.chrome.browser.ntp_customization.ntp_cards.NtpCardsCoordinator;
+import org.chromium.chrome.browser.profiles.ProfileProvider;
import org.chromium.components.browser_ui.bottomsheet.BottomSheetController;
import org.chromium.ui.modelutil.PropertyKey;
import org.chromium.ui.modelutil.PropertyModel;
@@ -58,7 +60,9 @@
}
public NtpCustomizationCoordinator(
- Context context, BottomSheetController bottomSheetController) {
+ Context context,
+ BottomSheetController bottomSheetController,
+ Supplier<ProfileProvider> profileSupplier) {
mContext = context;
View contentView =
LayoutInflater.from(mContext)
@@ -93,7 +97,8 @@
bottomSheetController,
bottomSheetContent,
viewFlipperPropertyModel,
- containerPropertyModel);
+ containerPropertyModel,
+ profileSupplier);
mMediator.registerBottomSheetLayout(MAIN);
mDelegate = createBottomSheetDelegate();
diff --git a/chrome/browser/ntp_customization/java/src/org/chromium/chrome/browser/ntp_customization/NtpCustomizationMediator.java b/chrome/browser/ntp_customization/java/src/org/chromium/chrome/browser/ntp_customization/NtpCustomizationMediator.java
index 415a38f..fc9fa89f 100644
--- a/chrome/browser/ntp_customization/java/src/org/chromium/chrome/browser/ntp_customization/NtpCustomizationMediator.java
+++ b/chrome/browser/ntp_customization/java/src/org/chromium/chrome/browser/ntp_customization/NtpCustomizationMediator.java
@@ -12,9 +12,14 @@
import android.content.Context;
import android.support.annotation.Nullable;
+import android.support.annotation.VisibleForTesting;
import android.view.View;
import android.widget.ViewFlipper;
+import org.chromium.base.supplier.Supplier;
+import org.chromium.chrome.browser.feed.FeedFeatures;
+import org.chromium.chrome.browser.profiles.Profile;
+import org.chromium.chrome.browser.profiles.ProfileProvider;
import org.chromium.components.browser_ui.bottomsheet.BottomSheetContent;
import org.chromium.components.browser_ui.bottomsheet.BottomSheetController;
import org.chromium.components.browser_ui.bottomsheet.BottomSheetObserver;
@@ -44,17 +49,20 @@
private final PropertyModel mViewFlipperPropertyModel;
private final List<Integer> mListContent;
private final PropertyModel mContainerPropertyModel;
+ private final Supplier<ProfileProvider> mProfileSupplier;
private Integer mCurrentBottomSheet;
public NtpCustomizationMediator(
BottomSheetController bottomSheetController,
NtpCustomizationBottomSheetContent bottomSheetContent,
PropertyModel viewFlipperPropertyModel,
- PropertyModel containerPropertyModel) {
+ PropertyModel containerPropertyModel,
+ Supplier<ProfileProvider> profileSupplier) {
mBottomSheetController = bottomSheetController;
mBottomSheetContent = bottomSheetContent;
mViewFlipperPropertyModel = viewFlipperPropertyModel;
mContainerPropertyModel = containerPropertyModel;
+ mProfileSupplier = profileSupplier;
mViewFlipperMap = new HashMap<>();
mTypeToListenersMap = new HashMap<>();
mListContent = buildListContent();
@@ -178,12 +186,23 @@
mTypeToListenersMap.put(type, listener);
}
- /** Returns the content of the list displayed in the main bottom sheet. */
+ /**
+ * Returns the content of the list displayed in the main bottom sheet and includes the "Discover
+ * Feed" list item only if the feed section exists in the new tab page.
+ */
+ @VisibleForTesting
List<Integer> buildListContent() {
+ if (!mProfileSupplier.hasValue()) {
+ return List.of(NTP_CARDS);
+ }
+
+ Profile profile = mProfileSupplier.get().getOriginalProfile();
List<Integer> content = new ArrayList<>();
content.add(NTP_CARDS);
- // TODO(crbug.com/397439004): Add logics to hide the "feeds" list item.
- content.add(DISCOVER_FEED);
+
+ if (FeedFeatures.isFeedEnabled(profile)) {
+ content.add(DISCOVER_FEED);
+ }
return content;
}
diff --git a/chrome/browser/ntp_customization/junit/src/org/chromium/chrome/browser/ntp_customization/NtpCustomizationCoordinatorUnitTest.java b/chrome/browser/ntp_customization/junit/src/org/chromium/chrome/browser/ntp_customization/NtpCustomizationCoordinatorUnitTest.java
index d51ff6fe..c42de81 100644
--- a/chrome/browser/ntp_customization/junit/src/org/chromium/chrome/browser/ntp_customization/NtpCustomizationCoordinatorUnitTest.java
+++ b/chrome/browser/ntp_customization/junit/src/org/chromium/chrome/browser/ntp_customization/NtpCustomizationCoordinatorUnitTest.java
@@ -27,6 +27,7 @@
import org.mockito.junit.MockitoJUnit;
import org.mockito.junit.MockitoRule;
+import org.chromium.base.supplier.Supplier;
import org.chromium.base.test.BaseRobolectricTestRunner;
import org.chromium.chrome.browser.ntp_customization.ntp_cards.NtpCardsCoordinator;
import org.chromium.components.browser_ui.bottomsheet.BottomSheetController;
@@ -48,7 +49,8 @@
public void setUp() {
mContext = ApplicationProvider.getApplicationContext();
mNtpCustomizationCoordinator =
- new NtpCustomizationCoordinator(mContext, mBottomSheetController);
+ new NtpCustomizationCoordinator(
+ mContext, mBottomSheetController, mock(Supplier.class));
mNtpCustomizationCoordinator.setViewFlipperForTesting(mViewFlipper);
mNtpCustomizationCoordinator.setMediatorForTesting(mMediator);
}
diff --git a/chrome/browser/ntp_customization/junit/src/org/chromium/chrome/browser/ntp_customization/NtpCustomizationMediatorUnitTest.java b/chrome/browser/ntp_customization/junit/src/org/chromium/chrome/browser/ntp_customization/NtpCustomizationMediatorUnitTest.java
index 6796e566..14f45fd 100644
--- a/chrome/browser/ntp_customization/junit/src/org/chromium/chrome/browser/ntp_customization/NtpCustomizationMediatorUnitTest.java
+++ b/chrome/browser/ntp_customization/junit/src/org/chromium/chrome/browser/ntp_customization/NtpCustomizationMediatorUnitTest.java
@@ -5,8 +5,10 @@
package org.chromium.chrome.browser.ntp_customization;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.anyInt;
@@ -15,6 +17,7 @@
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
import static org.chromium.chrome.browser.ntp_customization.NtpCustomizationCoordinator.BottomSheetType.DISCOVER_FEED;
import static org.chromium.chrome.browser.ntp_customization.NtpCustomizationCoordinator.BottomSheetType.MAIN;
@@ -36,12 +39,20 @@
import org.mockito.junit.MockitoJUnit;
import org.mockito.junit.MockitoRule;
+import org.chromium.base.supplier.OneshotSupplierImpl;
import org.chromium.base.test.BaseRobolectricTestRunner;
import org.chromium.base.test.util.HistogramWatcher;
+import org.chromium.chrome.browser.feed.FeedFeatures;
+import org.chromium.chrome.browser.feed.FeedServiceBridge;
+import org.chromium.chrome.browser.feed.FeedServiceBridgeJni;
import org.chromium.chrome.browser.ntp_customization.NtpCustomizationCoordinator.BottomSheetType;
+import org.chromium.chrome.browser.preferences.Pref;
+import org.chromium.chrome.browser.profiles.Profile;
+import org.chromium.chrome.browser.profiles.ProfileProvider;
import org.chromium.components.browser_ui.bottomsheet.BottomSheetContent;
import org.chromium.components.browser_ui.bottomsheet.BottomSheetController;
import org.chromium.components.browser_ui.bottomsheet.BottomSheetObserver;
+import org.chromium.components.prefs.PrefService;
import org.chromium.ui.modelutil.PropertyModel;
import java.util.List;
@@ -56,21 +67,28 @@
@Mock private NtpCustomizationBottomSheetContent mBottomSheetContent;
@Mock private PropertyModel mViewFlipperPropertyModel;
@Mock private PropertyModel mContainerPropertyModel;
+ @Mock private PrefService mPrefService;
+ @Mock private FeedServiceBridge.Natives mFeedServiceBridgeJniMock;
+ @Mock private Profile mProfile;
+ @Mock private ProfileProvider mProfileProvider;
private NtpCustomizationMediator mMediator;
private Map<Integer, Integer> mViewFlipperMap;
private ListContainerViewDelegate mListDelegate;
private Context mContext;
+ private OneshotSupplierImpl<ProfileProvider> mSupplier;
@Before
public void setUp() {
mContext = ApplicationProvider.getApplicationContext();
+ mSupplier = new OneshotSupplierImpl<>();
mMediator =
new NtpCustomizationMediator(
mBottomSheetController,
mBottomSheetContent,
mViewFlipperPropertyModel,
- mContainerPropertyModel);
+ mContainerPropertyModel,
+ mSupplier);
mViewFlipperMap = mMediator.getViewFlipperMapForTesting();
mListDelegate = mMediator.createListDelegate();
}
@@ -264,12 +282,6 @@
@Test
@SmallTest
public void testListContainerViewDelegate() {
- // Verifies that the content of the delegate.getListItems() is consist of MAIN and FEEDS
- // while MAIN comes before FEEDS.
- List<Integer> content = mListDelegate.getListItems();
- assertEquals(NTP_CARDS, (int) content.get(0));
- assertEquals(DISCOVER_FEED, (int) content.get(1));
-
// Verifies the subtitle of the "feeds" list item is "On" and is null for other list item.
assertEquals("On", mListDelegate.getListItemSubtitle(DISCOVER_FEED, mContext));
assertNull(mListDelegate.getListItemSubtitle(MAIN, mContext));
@@ -300,4 +312,35 @@
verify(mContainerPropertyModel)
.set(eq(LIST_CONTAINER_VIEW_DELEGATE), any(ListContainerViewDelegate.class));
}
+
+ @Test
+ @SmallTest
+ public void testBuildListContentWhenProfileIsNotReady() {
+ List<Integer> listContent = mMediator.buildListContent();
+ assertEquals(List.of(NTP_CARDS), listContent);
+ }
+
+ @Test
+ @SmallTest
+ public void testBuildListContent() {
+ // Verifies that "DISCOVER_FEED" misses from the result list if and if only isFeedEnabled()
+ // is false;
+ FeedServiceBridgeJni.setInstanceForTesting(mFeedServiceBridgeJniMock);
+ mSupplier.set(mProfileProvider);
+ when(mProfileProvider.getOriginalProfile()).thenReturn(mProfile);
+ FeedFeatures.setFakePrefsForTest(mPrefService);
+
+ // Mock dependencies to enable FeedFeatures.isFeedEnabled(profile) to return true.
+ when(mPrefService.getBoolean(Pref.ENABLE_SNIPPETS_BY_DSE)).thenReturn(true);
+ when(mFeedServiceBridgeJniMock.isEnabled()).thenReturn(true);
+
+ assertTrue(FeedFeatures.isFeedEnabled(mProfile));
+ assertEquals(List.of(NTP_CARDS, DISCOVER_FEED), mMediator.buildListContent());
+
+ // Mock dependencies to enable FeedFeatures.isFeedEnabled(profile) to return false.
+ when(mPrefService.getBoolean(Pref.ENABLE_SNIPPETS_BY_DSE)).thenReturn(false);
+
+ assertFalse(FeedFeatures.isFeedEnabled(mProfile));
+ assertEquals(List.of(NTP_CARDS), mMediator.buildListContent());
+ }
}
