Commit 51f0eb20 authored by Natalie Chouinard's avatar Natalie Chouinard Committed by Commit Bot

Remove unused SiteExploration UI feature

This feature is no longer used and the flag to enable it has been
removed.

Bug: 957297
Change-Id: Idbd337e137b9f2a888ca11bf9577db129d5ec438
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1597753Reviewed-by: default avatarKristi Park <kristipark@chromium.org>
Commit-Queue: Natalie Chouinard <chouinard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#657358}
parent 6d5dd575
...@@ -188,7 +188,6 @@ const base::Feature* kFeaturesExposedToJava[] = { ...@@ -188,7 +188,6 @@ const base::Feature* kFeaturesExposedToJava[] = {
&payments::features::kWebPaymentsSingleAppUiSkip, &payments::features::kWebPaymentsSingleAppUiSkip,
&language::kExplicitLanguageAsk, &language::kExplicitLanguageAsk,
&ntp_snippets::kArticleSuggestionsFeature, &ntp_snippets::kArticleSuggestionsFeature,
&ntp_tiles::kSiteExplorationUiFeature,
&offline_pages::kOfflineIndicatorFeature, &offline_pages::kOfflineIndicatorFeature,
&offline_pages::kOfflineIndicatorAlwaysHttpProbeFeature, &offline_pages::kOfflineIndicatorAlwaysHttpProbeFeature,
&offline_pages::kOfflinePagesCTFeature, // See crbug.com/620421. &offline_pages::kOfflinePagesCTFeature, // See crbug.com/620421.
......
...@@ -17,9 +17,6 @@ const base::Feature kPopularSitesBakedInContentFeature{ ...@@ -17,9 +17,6 @@ const base::Feature kPopularSitesBakedInContentFeature{
const base::Feature kNtpMostLikelyFaviconsFromServerFeature{ const base::Feature kNtpMostLikelyFaviconsFromServerFeature{
"NTPMostLikelyFaviconsFromServer", base::FEATURE_ENABLED_BY_DEFAULT}; "NTPMostLikelyFaviconsFromServer", base::FEATURE_ENABLED_BY_DEFAULT};
const base::Feature kSiteExplorationUiFeature{
"SiteExplorationUi", base::FEATURE_DISABLED_BY_DEFAULT};
const base::Feature kUsePopularSitesSuggestions{ const base::Feature kUsePopularSitesSuggestions{
"UsePopularSitesSuggestions", base::FEATURE_ENABLED_BY_DEFAULT}; "UsePopularSitesSuggestions", base::FEATURE_ENABLED_BY_DEFAULT};
......
...@@ -23,10 +23,6 @@ extern const base::Feature kPopularSitesBakedInContentFeature; ...@@ -23,10 +23,6 @@ extern const base::Feature kPopularSitesBakedInContentFeature;
// Likely tiles on the New Tab Page. // Likely tiles on the New Tab Page.
extern const base::Feature kNtpMostLikelyFaviconsFromServerFeature; extern const base::Feature kNtpMostLikelyFaviconsFromServerFeature;
// Feature to provide site exploration tiles in addition to personal tiles.
// TODO(https://crbug.com/957297): Remove this.
extern const base::Feature kSiteExplorationUiFeature;
// If this feature is enabled, we enable popular sites in the suggestions UI. // If this feature is enabled, we enable popular sites in the suggestions UI.
extern const base::Feature kUsePopularSitesSuggestions; extern const base::Feature kUsePopularSitesSuggestions;
......
...@@ -55,11 +55,9 @@ const char* kKnownGenericPagePrefixes[] = { ...@@ -55,11 +55,9 @@ const char* kKnownGenericPagePrefixes[] = {
""}; // The no-www domain matches domains on same level . ""}; // The no-www domain matches domains on same level .
// Determine whether we need any tiles from PopularSites to fill up a grid of // Determine whether we need any tiles from PopularSites to fill up a grid of
// |num_tiles| tiles. If exploration sections are used, we need popular sites // |num_tiles| tiles.
// regardless of how many tiles we already have.
bool NeedPopularSites(const PrefService* prefs, int num_tiles) { bool NeedPopularSites(const PrefService* prefs, int num_tiles) {
return base::FeatureList::IsEnabled(kSiteExplorationUiFeature) || return prefs->GetInteger(prefs::kNumPersonalTiles) < num_tiles;
prefs->GetInteger(prefs::kNumPersonalTiles) < num_tiles;
} }
bool AreURLsEquivalent(const GURL& url1, const GURL& url2) { bool AreURLsEquivalent(const GURL& url1, const GURL& url2) {
......
...@@ -877,51 +877,6 @@ TEST_P(MostVisitedSitesTest, ShouldInformSuggestionSourcesWhenBlacklisting) { ...@@ -877,51 +877,6 @@ TEST_P(MostVisitedSitesTest, ShouldInformSuggestionSourcesWhenBlacklisting) {
/*add_url=*/false); /*add_url=*/false);
} }
TEST_P(MostVisitedSitesTest, ShouldContainSiteExplorationsWhenFeatureEnabled) {
base::test::ScopedFeatureList feature_list;
std::map<SectionType, NTPTilesVector> sections;
feature_list.InitAndEnableFeature(kSiteExplorationUiFeature);
pref_service_.SetString(prefs::kPopularSitesOverrideVersion, "6");
RecreateMostVisitedSites(); // Refills cache with version 6 popular sites.
DisableRemoteSuggestions();
EXPECT_CALL(*mock_top_sites_, GetMostVisitedURLs(_))
.WillRepeatedly(InvokeCallbackArgument<0>(
MostVisitedURLList{MakeMostVisitedURL("Site 1", "http://site1/")}));
EXPECT_CALL(*mock_top_sites_, SyncWithHistory());
EXPECT_CALL(mock_observer_, OnURLsAvailable(_))
.WillOnce(SaveArg<0>(&sections));
most_visited_sites_->SetMostVisitedURLsObserver(&mock_observer_,
/*num_sites=*/3);
base::RunLoop().RunUntilIdle();
if (!IsPopularSitesFeatureEnabled()) {
EXPECT_THAT(
sections,
Contains(Pair(SectionType::PERSONALIZED,
ElementsAre(MatchesTile("Site 1", "http://site1/",
TileSource::TOP_SITES)))));
return;
}
const auto& expected_sections =
most_visited_sites_->popular_sites()->sections();
ASSERT_THAT(expected_sections.size(), Ge(2ul));
EXPECT_THAT(sections.size(), Eq(expected_sections.size()));
EXPECT_THAT(
sections,
AllOf(Contains(Pair(
SectionType::PERSONALIZED,
ElementsAre(MatchesTile("Site 1", "http://site1/",
TileSource::TOP_SITES),
MatchesTile("PopularSite1", "http://popularsite1/",
TileSource::POPULAR),
MatchesTile("PopularSite2", "http://popularsite2/",
TileSource::POPULAR)))),
Contains(Pair(SectionType::NEWS, SizeIs(2ul))),
Contains(Pair(SectionType::SOCIAL, SizeIs(1ul))),
Contains(Pair(_, IsEmpty()))));
}
TEST_P(MostVisitedSitesTest, TEST_P(MostVisitedSitesTest,
ShouldDeduplicatePopularSitesWithMostVisitedIffHostAndTitleMatches) { ShouldDeduplicatePopularSitesWithMostVisitedIffHostAndTitleMatches) {
pref_service_.SetString(prefs::kPopularSitesOverrideCountry, "US"); pref_service_.SetString(prefs::kPopularSitesOverrideCountry, "US");
......
...@@ -172,9 +172,10 @@ std::map<SectionType, PopularSites::SitesVector> ParseVersion6OrAbove( ...@@ -172,9 +172,10 @@ std::map<SectionType, PopularSites::SitesVector> ParseVersion6OrAbove(
<< "invalid ID (" << section << ")"; << "invalid ID (" << section << ")";
continue; continue;
} }
// Non-personalized site exploration tiles are no longer supported, so
// ignore all other section types.
SectionType section_type = static_cast<SectionType>(section); SectionType section_type = static_cast<SectionType>(section);
if (section_type == SectionType::UNKNOWN) { if (section_type != SectionType::PERSONALIZED) {
LOG(WARNING) << "Dropped an unknown section in SitesExploration list.";
continue; continue;
} }
const base::ListValue* sites_list; const base::ListValue* sites_list;
...@@ -183,12 +184,6 @@ std::map<SectionType, PopularSites::SitesVector> ParseVersion6OrAbove( ...@@ -183,12 +184,6 @@ std::map<SectionType, PopularSites::SitesVector> ParseVersion6OrAbove(
} }
sections[section_type] = ParseSiteList(*sites_list); sections[section_type] = ParseSiteList(*sites_list);
} }
if (!base::FeatureList::IsEnabled(kSiteExplorationUiFeature)) {
// New versions of popular sites that should act like old versions will
// mimic having only the personalized list.
return {std::make_pair(SectionType::PERSONALIZED,
std::move(sections[SectionType::PERSONALIZED]))};
}
return sections; return sections;
} }
......
...@@ -531,10 +531,7 @@ TEST_F(PopularSitesTest, ShouldOverrideDirectory) { ...@@ -531,10 +531,7 @@ TEST_F(PopularSitesTest, ShouldOverrideDirectory) {
EXPECT_THAT(sites.size(), Eq(1u)); EXPECT_THAT(sites.size(), Eq(1u));
} }
TEST_F(PopularSitesTest, DoesNotFetchExplorationSitesWithoutFeature) { TEST_F(PopularSitesTest, DoesNotFetchExplorationSites) {
base::test::ScopedFeatureList override_features;
override_features.InitAndDisableFeature(kSiteExplorationUiFeature);
SetCountryAndVersion("ZZ", "6"); SetCountryAndVersion("ZZ", "6");
RespondWithV6JSON( RespondWithV6JSON(
"https://www.gstatic.com/chrome/ntp/suggested_sites_ZZ_6.json", "https://www.gstatic.com/chrome/ntp/suggested_sites_ZZ_6.json",
...@@ -549,46 +546,5 @@ TEST_F(PopularSitesTest, DoesNotFetchExplorationSitesWithoutFeature) { ...@@ -549,46 +546,5 @@ TEST_F(PopularSitesTest, DoesNotFetchExplorationSitesWithoutFeature) {
EXPECT_THAT(sections, Not(Contains(Pair(SectionType::NEWS, _)))); EXPECT_THAT(sections, Not(Contains(Pair(SectionType::NEWS, _))));
} }
TEST_F(PopularSitesTest, FetchesExplorationSitesWithFeature) {
base::test::ScopedFeatureList override_features;
override_features.InitAndEnableFeature(kSiteExplorationUiFeature);
SetCountryAndVersion("ZZ", "6");
RespondWithV6JSON(
"https://www.gstatic.com/chrome/ntp/suggested_sites_ZZ_6.json",
{{SectionType::PERSONALIZED, {kChromium}},
{SectionType::ENTERTAINMENT, {kWikipedia, kYouTube}},
{SectionType::NEWS, {kYouTube}},
{SectionType::TOOLS, TestPopularSiteVector{}}});
std::map<SectionType, PopularSites::SitesVector> sections;
EXPECT_THAT(FetchAllSections(/*force_download=*/false, &sections),
Eq(base::Optional<bool>(true)));
EXPECT_THAT(sections, ElementsAre(Pair(SectionType::PERSONALIZED, SizeIs(1)),
Pair(SectionType::ENTERTAINMENT, SizeIs(2)),
Pair(SectionType::NEWS, SizeIs(1)),
Pair(SectionType::TOOLS, IsEmpty())));
}
TEST_F(PopularSitesTest, FetchesExplorationSitesIgnoreUnknownSections) {
base::test::ScopedFeatureList override_features;
override_features.InitAndEnableFeature(kSiteExplorationUiFeature);
SetCountryAndVersion("ZZ", "6");
RespondWithV6JSON(
"https://www.gstatic.com/chrome/ntp/suggested_sites_ZZ_6.json",
{{SectionType::UNKNOWN, {kChromium}},
{SectionType::NEWS, {kYouTube}},
{SectionType::UNKNOWN, {kWikipedia, kYouTube}}});
std::map<SectionType, PopularSites::SitesVector> sections;
EXPECT_THAT(FetchAllSections(/*force_download=*/false, &sections),
Eq(base::Optional<bool>(true)));
// Expect that there are four sections, none of which is empty.
EXPECT_THAT(sections, ElementsAre(Pair(SectionType::PERSONALIZED, SizeIs(0)),
Pair(SectionType::NEWS, SizeIs(1))));
}
} // namespace } // namespace
} // namespace ntp_tiles } // namespace ntp_tiles
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