diff options
| author | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2017-11-20 10:33:36 +0100 |
|---|---|---|
| committer | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2017-11-22 11:45:12 +0000 |
| commit | be59a35641616a4cf23c4a13fa0632624b021c1b (patch) | |
| tree | 9da183258bdf9cc413f7562079d25ace6955467f /chromium/components/ntp_tiles | |
| parent | d702e4b6a64574e97fc7df8fe3238cde70242080 (diff) | |
| download | qtwebengine-chromium-be59a35641616a4cf23c4a13fa0632624b021c1b.tar.gz | |
BASELINE: Update Chromium to 62.0.3202.101
Change-Id: I2d5eca8117600df6d331f6166ab24d943d9814ac
Reviewed-by: Alexandru Croitor <alexandru.croitor@qt.io>
Diffstat (limited to 'chromium/components/ntp_tiles')
21 files changed, 996 insertions, 318 deletions
diff --git a/chromium/components/ntp_tiles/BUILD.gn b/chromium/components/ntp_tiles/BUILD.gn index d9704187e88..300d4758576 100644 --- a/chromium/components/ntp_tiles/BUILD.gn +++ b/chromium/components/ntp_tiles/BUILD.gn @@ -28,6 +28,7 @@ static_library("ntp_tiles") { "popular_sites_impl.h", "pref_names.cc", "pref_names.h", + "section_type.h", "switches.cc", "switches.h", "tile_source.h", @@ -111,6 +112,7 @@ source_set("unit_tests") { if (is_android) { java_cpp_enum("ntp_tiles_enums_java") { sources = [ + "section_type.h", "tile_source.h", "tile_visual_type.h", ] diff --git a/chromium/components/ntp_tiles/constants.cc b/chromium/components/ntp_tiles/constants.cc index 6af7a1727cd..048cf796baa 100644 --- a/chromium/components/ntp_tiles/constants.cc +++ b/chromium/components/ntp_tiles/constants.cc @@ -18,6 +18,12 @@ extern const base::Feature kPopularSitesBakedInContentFeature{ extern const base::Feature kNtpMostLikelyFaviconsFromServerFeature{ "NTPMostLikelyFaviconsFromServer", base::FEATURE_DISABLED_BY_DEFAULT}; +extern const base::Feature kLowerResolutionFaviconsFeature{ + "NTPTilesLowerResolutionFavicons", base::FEATURE_DISABLED_BY_DEFAULT}; + +extern const base::Feature kSiteExplorationUiFeature{ + "SiteExplorationUi", base::FEATURE_DISABLED_BY_DEFAULT}; + bool AreNtpMostLikelyFaviconsFromServerEnabled() { // Check if the experimental flag is forced on or off. base::CommandLine* command_line = base::CommandLine::ForCurrentProcess(); diff --git a/chromium/components/ntp_tiles/constants.h b/chromium/components/ntp_tiles/constants.h index 4af8d54413b..03a2be48a1e 100644 --- a/chromium/components/ntp_tiles/constants.h +++ b/chromium/components/ntp_tiles/constants.h @@ -23,6 +23,13 @@ extern const base::Feature kPopularSitesBakedInContentFeature; // Likely tiles on the New Tab Page. extern const base::Feature kNtpMostLikelyFaviconsFromServerFeature; +// Feature to allow displaying lower resolution favicons for Tiles on the New +// Tab Page. +extern const base::Feature kLowerResolutionFaviconsFeature; + +// Feature to provide site exploration tiles in addition to personal tiles. +extern const base::Feature kSiteExplorationUiFeature; + // Use this to find out whether the kNtpMostLikelyFaviconsFromServerFeature is // enabled. This helper function abstracts iOS special way to override the // feature (via command-line params). diff --git a/chromium/components/ntp_tiles/icon_cacher_impl.cc b/chromium/components/ntp_tiles/icon_cacher_impl.cc index 7b9e06ff2d4..c1518fd5e8c 100644 --- a/chromium/components/ntp_tiles/icon_cacher_impl.cc +++ b/chromium/components/ntp_tiles/icon_cacher_impl.cc @@ -133,7 +133,7 @@ void IconCacherImpl::OnGetFaviconImageForPageURLFinished( destination: WEBSITE } policy { - cookies_allowed: false + cookies_allowed: NO setting: "This feature cannot be disabled in settings." policy_exception_justification: "Not implemented." })"); @@ -252,7 +252,7 @@ void IconCacherImpl::OnGetLargeIconOrFallbackStyleFinished( destination: GOOGLE_OWNED_SERVICE } policy { - cookies_allowed: false + cookies_allowed: NO setting: "Users can disable this feature via 'History' setting under " "'Advanced sync settings'." diff --git a/chromium/components/ntp_tiles/icon_cacher_impl_unittest.cc b/chromium/components/ntp_tiles/icon_cacher_impl_unittest.cc index 2ce20e89b7d..94ffc498e72 100644 --- a/chromium/components/ntp_tiles/icon_cacher_impl_unittest.cc +++ b/chromium/components/ntp_tiles/icon_cacher_impl_unittest.cc @@ -437,9 +437,7 @@ TEST_F(IconCacherTestPopularSites, LargeNotCachedAndFetchPerformedOnlyOnce) { class IconCacherTestMostLikely : public IconCacherTestBase { protected: IconCacherTestMostLikely() - : large_icon_service_background_task_runner_( - new base::TestSimpleTaskRunner()), - fetcher_for_large_icon_service_( + : fetcher_for_large_icon_service_( base::MakeUnique<::testing::StrictMock<MockImageFetcher>>()), fetcher_for_icon_cacher_( base::MakeUnique<::testing::StrictMock<MockImageFetcher>>()) { @@ -453,8 +451,6 @@ class IconCacherTestMostLikely : public IconCacherTestBase { SetDesiredImageFrameSize(gfx::Size(128, 128))); } - scoped_refptr<base::TestSimpleTaskRunner> - large_icon_service_background_task_runner_; std::unique_ptr<MockImageFetcher> fetcher_for_large_icon_service_; std::unique_ptr<MockImageFetcher> fetcher_for_icon_cacher_; }; @@ -467,8 +463,7 @@ TEST_F(IconCacherTestMostLikely, Cached) { PreloadIcon(page_url, icon_url, favicon_base::TOUCH_ICON, 128, 128); favicon::LargeIconService large_icon_service( - &favicon_service_, large_icon_service_background_task_runner_, - std::move(fetcher_for_large_icon_service_)); + &favicon_service_, std::move(fetcher_for_large_icon_service_)); IconCacherImpl cacher(&favicon_service_, &large_icon_service, std::move(fetcher_for_icon_cacher_)); @@ -502,8 +497,7 @@ TEST_F(IconCacherTestMostLikely, NotCachedAndFetchSucceeded) { } favicon::LargeIconService large_icon_service( - &favicon_service_, large_icon_service_background_task_runner_, - std::move(fetcher_for_large_icon_service_)); + &favicon_service_, std::move(fetcher_for_large_icon_service_)); IconCacherImpl cacher(&favicon_service_, &large_icon_service, std::move(fetcher_for_icon_cacher_)); @@ -511,7 +505,7 @@ TEST_F(IconCacherTestMostLikely, NotCachedAndFetchSucceeded) { // Both these task runners need to be flushed in order to get |done| called by // running the main loop. WaitForHistoryThreadTasksToFinish(); - large_icon_service_background_task_runner_->RunUntilIdle(); + scoped_task_environment_.RunUntilIdle(); loop.Run(); EXPECT_FALSE(IconIsCachedFor(page_url, favicon_base::FAVICON)); @@ -541,8 +535,7 @@ TEST_F(IconCacherTestMostLikely, NotCachedAndFetchFailed) { } favicon::LargeIconService large_icon_service( - &favicon_service_, large_icon_service_background_task_runner_, - std::move(fetcher_for_large_icon_service_)); + &favicon_service_, std::move(fetcher_for_large_icon_service_)); IconCacherImpl cacher(&favicon_service_, &large_icon_service, std::move(fetcher_for_icon_cacher_)); @@ -550,7 +543,7 @@ TEST_F(IconCacherTestMostLikely, NotCachedAndFetchFailed) { // Both these task runners need to be flushed before flushing the main thread // queue in order to finish the work. WaitForHistoryThreadTasksToFinish(); - large_icon_service_background_task_runner_->RunUntilIdle(); + scoped_task_environment_.RunUntilIdle(); WaitForMainThreadTasksToFinish(); EXPECT_FALSE(IconIsCachedFor(page_url, favicon_base::FAVICON)); @@ -573,8 +566,7 @@ TEST_F(IconCacherTestMostLikely, HandlesEmptyCallbacksNicely) { .WillOnce(PassFetch(128, 128)); favicon::LargeIconService large_icon_service( - &favicon_service_, large_icon_service_background_task_runner_, - std::move(fetcher_for_large_icon_service_)); + &favicon_service_, std::move(fetcher_for_large_icon_service_)); IconCacherImpl cacher(&favicon_service_, &large_icon_service, std::move(fetcher_for_icon_cacher_)); @@ -582,7 +574,7 @@ TEST_F(IconCacherTestMostLikely, HandlesEmptyCallbacksNicely) { // Both these task runners need to be flushed before flushing the main thread // queue in order to finish the work. WaitForHistoryThreadTasksToFinish(); - large_icon_service_background_task_runner_->RunUntilIdle(); + scoped_task_environment_.RunUntilIdle(); WaitForMainThreadTasksToFinish(); // Even though the callbacks are not called, the icon gets written out. @@ -609,8 +601,7 @@ TEST_F(IconCacherTestMostLikely, NotCachedAndFetchPerformedOnlyOnce) { } favicon::LargeIconService large_icon_service( - &favicon_service_, large_icon_service_background_task_runner_, - std::move(fetcher_for_large_icon_service_)); + &favicon_service_, std::move(fetcher_for_large_icon_service_)); IconCacherImpl cacher(&favicon_service_, &large_icon_service, std::move(fetcher_for_icon_cacher_)); @@ -619,7 +610,7 @@ TEST_F(IconCacherTestMostLikely, NotCachedAndFetchPerformedOnlyOnce) { // Both these task runners need to be flushed in order to get |done| called by // running the main loop. WaitForHistoryThreadTasksToFinish(); - large_icon_service_background_task_runner_->RunUntilIdle(); + scoped_task_environment_.RunUntilIdle(); loop.Run(); EXPECT_FALSE(IconIsCachedFor(page_url, favicon_base::FAVICON)); diff --git a/chromium/components/ntp_tiles/metrics.cc b/chromium/components/ntp_tiles/metrics.cc index 4064c1594f3..bcd9f4a6a6d 100644 --- a/chromium/components/ntp_tiles/metrics.cc +++ b/chromium/components/ntp_tiles/metrics.cc @@ -23,7 +23,8 @@ const int kMaxNumTiles = 12; // Identifiers for the various tile sources. const char kHistogramClientName[] = "client"; const char kHistogramServerName[] = "server"; -const char kHistogramPopularName[] = "popular"; +const char kHistogramPopularName[] = "popular_fetched"; +const char kHistogramBakedInName[] = "popular_baked_in"; const char kHistogramWhitelistName[] = "whitelist"; const char kHistogramHomepageName[] = "homepage"; @@ -51,6 +52,8 @@ std::string GetSourceHistogramName(TileSource source) { switch (source) { case TileSource::TOP_SITES: return kHistogramClientName; + case TileSource::POPULAR_BAKED_IN: + return kHistogramBakedInName; case TileSource::POPULAR: return kHistogramPopularName; case TileSource::WHITELIST: diff --git a/chromium/components/ntp_tiles/metrics_unittest.cc b/chromium/components/ntp_tiles/metrics_unittest.cc index dbbe4c1e8e3..cba9de50eeb 100644 --- a/chromium/components/ntp_tiles/metrics_unittest.cc +++ b/chromium/components/ntp_tiles/metrics_unittest.cc @@ -71,7 +71,7 @@ TEST(RecordTileImpressionTest, ShouldRecordUmaForIcons) { base::Bucket(/*min=*/3, /*count=*/1), base::Bucket(/*min=*/4, /*count=*/1))); EXPECT_THAT(histogram_tester.GetAllSamples( - "NewTabPage.SuggestionsImpression.popular"), + "NewTabPage.SuggestionsImpression.popular_fetched"), ElementsAre(base::Bucket(/*min=*/7, /*count=*/1))); EXPECT_THAT(histogram_tester.GetAllSamples("NewTabPage.TileType"), ElementsAre(base::Bucket(/*min=*/ICON_REAL, /*count=*/4), @@ -83,9 +83,10 @@ TEST(RecordTileImpressionTest, ShouldRecordUmaForIcons) { EXPECT_THAT(histogram_tester.GetAllSamples("NewTabPage.TileType.client"), ElementsAre(base::Bucket(/*min=*/ICON_REAL, /*count=*/3), base::Bucket(/*min=*/ICON_COLOR, /*count=*/2))); - EXPECT_THAT(histogram_tester.GetAllSamples("NewTabPage.TileType.popular"), - ElementsAre(base::Bucket(/*min=*/ICON_COLOR, - /*count=*/1))); + EXPECT_THAT( + histogram_tester.GetAllSamples("NewTabPage.TileType.popular_fetched"), + ElementsAre(base::Bucket(/*min=*/ICON_COLOR, + /*count=*/1))); EXPECT_THAT(histogram_tester.GetAllSamples( "NewTabPage.SuggestionsImpression.IconsReal"), ElementsAre(base::Bucket(/*min=*/0, /*count=*/1), @@ -124,7 +125,7 @@ TEST(RecordTileImpressionTest, ShouldRecordUmaForThumbnails) { histogram_tester.GetAllSamples("NewTabPage.SuggestionsImpression.client"), ElementsAre(base::Bucket(/*min=*/0, /*count=*/1))); EXPECT_THAT(histogram_tester.GetAllSamples( - "NewTabPage.SuggestionsImpression.popular"), + "NewTabPage.SuggestionsImpression.popular_fetched"), ElementsAre(base::Bucket(/*min=*/2, /*count=*/1))); EXPECT_THAT(histogram_tester.GetAllSamples("NewTabPage.TileType"), ElementsAre(base::Bucket(/*min=*/THUMBNAIL, /*count=*/2), @@ -133,8 +134,9 @@ TEST(RecordTileImpressionTest, ShouldRecordUmaForThumbnails) { ElementsAre(base::Bucket(/*min=*/THUMBNAIL, /*count=*/1))); EXPECT_THAT(histogram_tester.GetAllSamples("NewTabPage.TileType.client"), ElementsAre(base::Bucket(/*min=*/THUMBNAIL_FAILED, /*count=*/1))); - EXPECT_THAT(histogram_tester.GetAllSamples("NewTabPage.TileType.popular"), - ElementsAre(base::Bucket(/*min=*/THUMBNAIL, /*count=*/1))); + EXPECT_THAT( + histogram_tester.GetAllSamples("NewTabPage.TileType.popular_fetched"), + ElementsAre(base::Bucket(/*min=*/THUMBNAIL, /*count=*/1))); EXPECT_THAT(histogram_tester.GetAllSamples( "NewTabPage.SuggestionsImpression.IconsReal"), IsEmpty()); @@ -155,8 +157,9 @@ TEST(RecordTileClickTest, ShouldRecordUmaForIcon) { ElementsAre(base::Bucket(/*min=*/3, /*count=*/1))); EXPECT_THAT(histogram_tester.GetAllSamples("NewTabPage.MostVisited.server"), IsEmpty()); - EXPECT_THAT(histogram_tester.GetAllSamples("NewTabPage.MostVisited.popular"), - IsEmpty()); + EXPECT_THAT( + histogram_tester.GetAllSamples("NewTabPage.MostVisited.popular_fetched"), + IsEmpty()); EXPECT_THAT( histogram_tester.GetAllSamples("NewTabPage.MostVisited.IconsReal"), ElementsAre(base::Bucket(/*min=*/3, /*count=*/1))); @@ -183,8 +186,9 @@ TEST(RecordTileClickTest, ShouldRecordUmaForThumbnail) { ElementsAre(base::Bucket(/*min=*/3, /*count=*/1))); EXPECT_THAT(histogram_tester.GetAllSamples("NewTabPage.MostVisited.server"), IsEmpty()); - EXPECT_THAT(histogram_tester.GetAllSamples("NewTabPage.MostVisited.popular"), - IsEmpty()); + EXPECT_THAT( + histogram_tester.GetAllSamples("NewTabPage.MostVisited.popular_fetched"), + IsEmpty()); EXPECT_THAT( histogram_tester.GetAllSamples("NewTabPage.MostVisited.IconsReal"), IsEmpty()); @@ -212,8 +216,9 @@ TEST(RecordTileClickTest, ShouldNotRecordUnknownTileType) { ElementsAre(base::Bucket(/*min=*/3, /*count=*/1))); EXPECT_THAT(histogram_tester.GetAllSamples("NewTabPage.MostVisited.server"), IsEmpty()); - EXPECT_THAT(histogram_tester.GetAllSamples("NewTabPage.MostVisited.popular"), - IsEmpty()); + EXPECT_THAT( + histogram_tester.GetAllSamples("NewTabPage.MostVisited.popular_fetched"), + IsEmpty()); // But all of the tile type histograms should be empty. EXPECT_THAT( histogram_tester.GetAllSamples("NewTabPage.MostVisited.IconsReal"), diff --git a/chromium/components/ntp_tiles/most_visited_sites.cc b/chromium/components/ntp_tiles/most_visited_sites.cc index 45a4bcddbb8..7327615bab2 100644 --- a/chromium/components/ntp_tiles/most_visited_sites.cc +++ b/chromium/components/ntp_tiles/most_visited_sites.cc @@ -5,12 +5,14 @@ #include "components/ntp_tiles/most_visited_sites.h" #include <algorithm> +#include <iterator> #include <utility> #include "base/bind.h" #include "base/callback.h" #include "base/feature_list.h" #include "base/metrics/user_metrics.h" +#include "base/strings/string_util.h" #include "base/strings/utf_string_conversions.h" #include "components/history/core/browser/top_sites.h" #include "components/ntp_tiles/constants.h" @@ -33,13 +35,27 @@ namespace { const base::Feature kDisplaySuggestionsServiceTiles{ "DisplaySuggestionsServiceTiles", base::FEATURE_ENABLED_BY_DEFAULT}; -// The maximum index of the home page tile. -const size_t kMaxHomeTileIndex = 3; +// URL host prefixes. Hosts with these prefixes often redirect to each other, or +// have the same content. +// Popular sites are excluded if the user has visited a page whose host only +// differs by one of these prefixes. Even if the URL does not point to the exact +// same page, the user will have a personalized suggestion that is more likely +// to be of use for them. +// A cleaner way could be checking the history for redirects but this requires +// the page to be visited on the device. +const char* kKnownGenericPagePrefixes[] = { + "m.", "mobile.", // Common prefixes among popular sites. + "edition.", // Used among news papers (CNN, Independent, ...) + "www.", // Usually no-www domains redirect to www or vice-versa. + // The following entry MUST REMAIN LAST as it is prefix of every string! + ""}; // The no-www domain matches domains on same level . // Determine whether we need any tiles from PopularSites to fill up a grid of -// |num_tiles| tiles. +// |num_tiles| tiles. If exploration sections are used, we need popular sites +// regardless of how many tiles we already have. bool NeedPopularSites(const PrefService* prefs, int num_tiles) { - return prefs->GetInteger(prefs::kNumPersonalTiles) < num_tiles; + return base::FeatureList::IsEnabled(kSiteExplorationUiFeature) || + prefs->GetInteger(prefs::kNumPersonalTiles) < num_tiles; } bool AreURLsEquivalent(const GURL& url1, const GURL& url2) { @@ -56,6 +72,16 @@ bool HasHomeTile(const NTPTilesVector& tiles) { return false; } +std::string StripFirstGenericPrefix(const std::string& host) { + for (const char* prefix : kKnownGenericPagePrefixes) { + if (base::StartsWith(host, prefix, base::CompareCase::INSENSITIVE_ASCII)) { + return std::string( + base::TrimString(host, prefix, base::TrimPositions::TRIM_LEADING)); + } + } + return host; +} + } // namespace MostVisitedSites::MostVisitedSites( @@ -89,12 +115,27 @@ MostVisitedSites::~MostVisitedSites() { supervisor_->SetObserver(nullptr); } +// static +bool MostVisitedSites::IsHostOrMobilePageKnown( + const std::set<std::string>& hosts_to_skip, + const std::string& host) { + std::string no_prefix_host = StripFirstGenericPrefix(host); + for (const char* prefix : kKnownGenericPagePrefixes) { + if (hosts_to_skip.count(prefix + no_prefix_host) || + hosts_to_skip.count(prefix + host)) { + return true; + } + } + return false; +} + bool MostVisitedSites::DoesSourceExist(TileSource source) const { switch (source) { case TileSource::TOP_SITES: return top_sites_ != nullptr; case TileSource::SUGGESTIONS_SERVICE: return suggestions_service_ != nullptr; + case TileSource::POPULAR_BAKED_IN: case TileSource::POPULAR: return popular_sites_ != nullptr; case TileSource::WHITELIST: @@ -128,10 +169,6 @@ void MostVisitedSites::SetMostVisitedURLsObserver(Observer* observer, } if (top_sites_) { - // TopSites updates itself after a delay. To ensure up-to-date results, - // force an update now. - top_sites_->SyncWithHistory(); - // Register as TopSitesObserver so that we can update ourselves when the // TopSites changes. top_sites_observer_.Add(top_sites_.get()); @@ -148,6 +185,14 @@ void MostVisitedSites::SetMostVisitedURLsObserver(Observer* observer, } void MostVisitedSites::Refresh() { + if (top_sites_) { + // TopSites updates itself after a delay. To ensure up-to-date results, + // force an update now. + // TODO(mastiz): Is seems unnecessary to refresh TopSites if we will end up + // using server-side suggestions. + top_sites_->SyncWithHistory(); + } + suggestions_service_->FetchSuggestionsData(); } @@ -345,38 +390,66 @@ NTPTilesVector MostVisitedSites::CreateWhitelistEntryPointTiles( return whitelist_tiles; } -NTPTilesVector MostVisitedSites::CreatePopularSitesTiles( +std::map<SectionType, NTPTilesVector> +MostVisitedSites::CreatePopularSitesSections( const std::set<std::string>& used_hosts, size_t num_actual_tiles) { + std::map<SectionType, NTPTilesVector> sections = { + std::make_pair(SectionType::PERSONALIZED, NTPTilesVector())}; // For child accounts popular sites tiles will not be added. if (supervisor_ && supervisor_->IsChildProfile()) { - return NTPTilesVector(); + return sections; } if (!popular_sites_ || !ShouldShowPopularSites()) { - return NTPTilesVector(); + return sections; + } + + const std::set<std::string> no_hosts; + for (const auto& section_type_and_sites : popular_sites()->sections()) { + SectionType type = section_type_and_sites.first; + const PopularSites::SitesVector& sites = section_type_and_sites.second; + if (type == SectionType::PERSONALIZED) { + size_t num_required_tiles = num_sites_ - num_actual_tiles; + sections[type] = + CreatePopularSitesTiles(/*popular_sites=*/sites, + /*hosts_to_skip=*/used_hosts, + /*num_max_tiles=*/num_required_tiles); + } else { + sections[type] = CreatePopularSitesTiles(/*popular_sites=*/sites, + /*hosts_to_skip=*/no_hosts, + /*num_max_tiles=*/num_sites_); + } } + return sections; +} +NTPTilesVector MostVisitedSites::CreatePopularSitesTiles( + const PopularSites::SitesVector& sites_vector, + const std::set<std::string>& hosts_to_skip, + size_t num_max_tiles) { // Collect non-blacklisted popular suggestions, skipping those already present // in the personal suggestions. NTPTilesVector popular_sites_tiles; - for (const PopularSites::Site& popular_site : popular_sites_->sites()) { - if (popular_sites_tiles.size() + num_actual_tiles >= num_sites_) + for (const PopularSites::Site& popular_site : sites_vector) { + if (popular_sites_tiles.size() >= num_max_tiles) { break; + } // Skip blacklisted sites. if (top_sites_ && top_sites_->IsBlacklisted(popular_site.url)) continue; const std::string& host = popular_site.url.host(); - // Skip tiles already present in personal or whitelists. - if (used_hosts.find(host) != used_hosts.end()) + if (IsHostOrMobilePageKnown(hosts_to_skip, host)) { continue; + } NTPTile tile; tile.title = popular_site.title; tile.url = GURL(popular_site.url); - tile.source = TileSource::POPULAR; + tile.source = popular_site.baked_in ? TileSource::POPULAR_BAKED_IN + : TileSource::POPULAR; popular_sites_tiles.push_back(std::move(tile)); base::Closure icon_available = base::Bind(&MostVisitedSites::OnIconMadeAvailable, @@ -404,38 +477,34 @@ NTPTilesVector MostVisitedSites::InsertHomeTile( const GURL& home_page_url = home_page_client_->GetHomePageUrl(); NTPTilesVector new_tiles; - // Add the home tile to the first four tiles. - NTPTile home_tile; - home_tile.url = home_page_url; - home_tile.title = title; - home_tile.source = TileSource::HOMEPAGE; - bool home_tile_added = false; - size_t index = 0; - while (index < tiles.size() && new_tiles.size() < num_sites_) { - bool hosts_are_equal = tiles[index].url.host() == home_page_url.host(); + for (auto& tile : tiles) { + if (new_tiles.size() >= num_sites_) { + break; + } - // Add the home tile to the first four tiles - // or at the position of a tile that has the same host - // and is ranked higher. // TODO(fhorschig): Introduce a more sophisticated deduplication. - if (!home_tile_added && (index >= kMaxHomeTileIndex || hosts_are_equal)) { - new_tiles.push_back(std::move(home_tile)); + if (tile.url.host() == home_page_url.host()) { + tile.source = TileSource::HOMEPAGE; home_tile_added = true; - continue; // Do not advance the current tile index. - } - - // Add non-home page tiles. - if (!hosts_are_equal) { - new_tiles.push_back(std::move(tiles[index])); } - ++index; + new_tiles.push_back(std::move(tile)); } // Add the home page tile if there are less than 4 tiles // and none of them is the home page (and there is space left). - if (!home_tile_added && new_tiles.size() < num_sites_) { + if (!home_tile_added) { + // Make room for the home page tile. + if (new_tiles.size() >= num_sites_) { + new_tiles.pop_back(); + } + + NTPTile home_tile; + home_tile.url = home_page_url; + home_tile.title = title; + home_tile.source = TileSource::HOMEPAGE; + new_tiles.push_back(std::move(home_tile)); } return new_tiles; @@ -463,13 +532,14 @@ void MostVisitedSites::SaveTilesAndNotify(NTPTilesVector personal_tiles) { CreateWhitelistEntryPointTiles(used_hosts, num_actual_tiles); AddToHostsAndTotalCount(whitelist_tiles, &used_hosts, &num_actual_tiles); - NTPTilesVector popular_sites_tiles = - CreatePopularSitesTiles(used_hosts, num_actual_tiles); - AddToHostsAndTotalCount(popular_sites_tiles, &used_hosts, &num_actual_tiles); + std::map<SectionType, NTPTilesVector> sections = + CreatePopularSitesSections(used_hosts, num_actual_tiles); + AddToHostsAndTotalCount(sections[SectionType::PERSONALIZED], &used_hosts, + &num_actual_tiles); NTPTilesVector new_tiles = MergeTiles(std::move(personal_tiles), std::move(whitelist_tiles), - std::move(popular_sites_tiles)); + std::move(sections[SectionType::PERSONALIZED])); if (current_tiles_.has_value() && (*current_tiles_ == new_tiles)) { return; } @@ -483,10 +553,10 @@ void MostVisitedSites::SaveTilesAndNotify(NTPTilesVector personal_tiles) { num_personal_tiles++; } prefs_->SetInteger(prefs::kNumPersonalTiles, num_personal_tiles); - if (!observer_) return; - observer_->OnMostVisitedURLsAvailable(*current_tiles_); + sections[SectionType::PERSONALIZED] = *current_tiles_; + observer_->OnURLsAvailable(sections); } // static @@ -509,10 +579,12 @@ void MostVisitedSites::OnPopularSitesDownloaded(bool success) { return; } - for (const PopularSites::Site& popular_site : popular_sites_->sites()) { - // Ignore callback; these icons will be seen on the *next* NTP. - icon_cacher_->StartFetchPopularSites(popular_site, base::Closure(), - base::Closure()); + for (const auto& section : popular_sites_->sections()) { + for (const PopularSites::Site& site : section.second) { + // Ignore callback; these icons will be seen on the *next* NTP. + icon_cacher_->StartFetchPopularSites(site, base::Closure(), + base::Closure()); + } } } diff --git a/chromium/components/ntp_tiles/most_visited_sites.h b/chromium/components/ntp_tiles/most_visited_sites.h index 661623d30d4..2331c58f3ca 100644 --- a/chromium/components/ntp_tiles/most_visited_sites.h +++ b/chromium/components/ntp_tiles/most_visited_sites.h @@ -7,6 +7,7 @@ #include <stddef.h> +#include <map> #include <memory> #include <set> #include <string> @@ -15,6 +16,7 @@ #include "base/callback_forward.h" #include "base/compiler_specific.h" #include "base/files/file_path.h" +#include "base/gtest_prod_util.h" #include "base/macros.h" #include "base/memory/weak_ptr.h" #include "base/optional.h" @@ -24,6 +26,7 @@ #include "components/history/core/browser/top_sites_observer.h" #include "components/ntp_tiles/ntp_tile.h" #include "components/ntp_tiles/popular_sites.h" +#include "components/ntp_tiles/section_type.h" #include "components/ntp_tiles/tile_source.h" #include "components/suggestions/proto/suggestions.pb.h" #include "components/suggestions/suggestions_service.h" @@ -85,7 +88,9 @@ class MostVisitedSites : public history::TopSitesObserver, // The observer to be notified when the list of most visited sites changes. class Observer { public: - virtual void OnMostVisitedURLsAvailable(const NTPTilesVector& tiles) = 0; + // |sections| must at least contain the PERSONALIZED section. + virtual void OnURLsAvailable( + const std::map<SectionType, NTPTilesVector>& sections) = 0; virtual void OnIconMadeAvailable(const GURL& site_url) = 0; protected: @@ -165,6 +170,22 @@ class MostVisitedSites : public history::TopSitesObserver, NTPTilesVector popular_tiles); private: + FRIEND_TEST_ALL_PREFIXES(MostVisitedSitesTest, + ShouldDeduplicateDomainWithNoWwwDomain); + FRIEND_TEST_ALL_PREFIXES(MostVisitedSitesTest, + ShouldDeduplicateDomainByRemovingMobilePrefixes); + FRIEND_TEST_ALL_PREFIXES(MostVisitedSitesTest, + ShouldDeduplicateDomainByReplacingMobilePrefixes); + + // This function tries to match the given |host| to a close fit in + // |hosts_to_skip| by removing a prefix that is commonly used to redirect from + // or to mobile pages (m.xyz.com --> xyz.com). + // If this approach fails, the prefix is replaced by another prefix. + // That way, true is returned for m.x.com if www.x.com is in |hosts_to_skip|. + static bool IsHostOrMobilePageKnown( + const std::set<std::string>& hosts_to_skip, + const std::string& host); + // Initialize the query to Top Sites. Called if the SuggestionsService // returned no data. void InitiateTopSitesQuery(); @@ -193,11 +214,19 @@ class MostVisitedSites : public history::TopSitesObserver, const std::set<std::string>& used_hosts, size_t num_actual_tiles); - // Creates popular tiles whose hosts weren't used yet. - NTPTilesVector CreatePopularSitesTiles( + // Creates tiles for all popular site sections. Uses |num_actual_tiles| and + // |used_hosts| to restrict results for the PERSONALIZED section. + std::map<SectionType, NTPTilesVector> CreatePopularSitesSections( const std::set<std::string>& used_hosts, size_t num_actual_tiles); + // Creates tiles for |sites_vector|. The returned vector will neither contain + // more than |num_max_tiles| nor include sites in |hosts_to_skip|. + NTPTilesVector CreatePopularSitesTiles( + const PopularSites::SitesVector& sites_vector, + const std::set<std::string>& hosts_to_skip, + size_t num_max_tiles); + // Initiates a query for the home page tile if needed and calls // |SaveTilesAndNotify| in the end. void InitiateNotificationForNewTiles(NTPTilesVector new_tiles); diff --git a/chromium/components/ntp_tiles/most_visited_sites_unittest.cc b/chromium/components/ntp_tiles/most_visited_sites_unittest.cc index 43bdea7dc97..0eb30120ee7 100644 --- a/chromium/components/ntp_tiles/most_visited_sites_unittest.cc +++ b/chromium/components/ntp_tiles/most_visited_sites_unittest.cc @@ -6,6 +6,7 @@ #include <stddef.h> +#include <map> #include <memory> #include <ostream> #include <string> @@ -29,6 +30,7 @@ #include "components/ntp_tiles/json_unsafe_parser.h" #include "components/ntp_tiles/popular_sites_impl.h" #include "components/ntp_tiles/pref_names.h" +#include "components/ntp_tiles/section_type.h" #include "components/ntp_tiles/switches.h" #include "components/sync_preferences/testing_pref_service_syncable.h" #include "net/url_request/test_url_fetcher_factory.h" @@ -59,11 +61,14 @@ using testing::ByMove; using testing::Contains; using testing::ElementsAre; using testing::Eq; +using testing::Ge; using testing::InSequence; using testing::Invoke; using testing::IsEmpty; +using testing::Key; using testing::Mock; using testing::Not; +using testing::Pair; using testing::Return; using testing::ReturnRef; using testing::SaveArg; @@ -87,13 +92,17 @@ MATCHER_P3(MatchesTile, title, url, source, PrintTile(title, url, source)) { arg.source == source; } -MATCHER_P3(FirstTileIs, +MATCHER_P3(FirstPersonalizedTileIs, title, url, source, std::string("first tile ") + PrintTile(title, url, source)) { - return !arg.empty() && arg[0].title == base::ASCIIToUTF16(title) && - arg[0].url == GURL(url) && arg[0].source == source; + if (arg.count(SectionType::PERSONALIZED) == 0) { + return false; + } + const NTPTilesVector& tiles = arg.at(SectionType::PERSONALIZED); + return !tiles.empty() && tiles[0].title == base::ASCIIToUTF16(title) && + tiles[0].url == GURL(url) && tiles[0].source == source; } // testing::InvokeArgument<N> does not work with base::Callback, fortunately @@ -208,7 +217,8 @@ class MockSuggestionsService : public SuggestionsService { class MockMostVisitedSitesObserver : public MostVisitedSites::Observer { public: - MOCK_METHOD1(OnMostVisitedURLsAvailable, void(const NTPTilesVector& tiles)); + MOCK_METHOD1(OnURLsAvailable, + void(const std::map<SectionType, NTPTilesVector>& sections)); MOCK_METHOD1(OnIconMadeAvailable, void(const GURL& site_url)); }; @@ -273,10 +283,10 @@ class PopularSitesFactoryForTest { PopularSitesImpl::RegisterProfilePrefs(pref_service->registry()); if (enabled) { prefs_->SetString(prefs::kPopularSitesOverrideCountry, "IN"); - prefs_->SetString(prefs::kPopularSitesOverrideVersion, "7"); + prefs_->SetString(prefs::kPopularSitesOverrideVersion, "5"); url_fetcher_factory_.SetFakeResponse( - GURL("https://www.gstatic.com/chrome/ntp/suggested_sites_IN_7.json"), + GURL("https://www.gstatic.com/chrome/ntp/suggested_sites_IN_5.json"), R"([{ "title": "PopularSite1", "url": "http://popularsite1/", @@ -289,6 +299,70 @@ class PopularSitesFactoryForTest { }, ])", net::HTTP_OK, net::URLRequestStatus::SUCCESS); + + url_fetcher_factory_.SetFakeResponse( + GURL("https://www.gstatic.com/chrome/ntp/suggested_sites_US_5.json"), + R"([{ + "title": "ESPN", + "url": "http://www.espn.com", + "favicon_url": "http://www.espn.com/favicon.ico" + }, { + "title": "Mobile", + "url": "http://www.mobile.de", + "favicon_url": "http://www.mobile.de/favicon.ico" + }, { + "title": "Google News", + "url": "http://news.google.com", + "favicon_url": "http://news.google.com/favicon.ico" + }, + ])", + net::HTTP_OK, net::URLRequestStatus::SUCCESS); + + url_fetcher_factory_.SetFakeResponse( + GURL("https://www.gstatic.com/chrome/ntp/suggested_sites_IN_6.json"), + R"([{ + "section": 1, // PERSONALIZED + "sites": [{ + "title": "PopularSite1", + "url": "http://popularsite1/", + "favicon_url": "http://popularsite1/favicon.ico" + }, + { + "title": "PopularSite2", + "url": "http://popularsite2/", + "favicon_url": "http://popularsite2/favicon.ico" + }, + ] + }, + { + "section": 4, // NEWS + "sites": [{ + "large_icon_url": "https://news.google.com/icon.ico", + "title": "Google News", + "url": "https://news.google.com/" + }, + { + "favicon_url": "https://news.google.com/icon.ico", + "title": "Google News Germany", + "url": "https://news.google.de/" + }] + }, + { + "section": 2, // SOCIAL + "sites": [{ + "large_icon_url": "https://ssl.gstatic.com/icon.png", + "title": "Google+", + "url": "https://plus.google.com/" + }] + }, + { + "section": 3, // ENTERTAINMENT + "sites": [ + // Intentionally empty site list. + ] + } + ])", + net::HTTP_OK, net::URLRequestStatus::SUCCESS); } } @@ -330,6 +404,8 @@ class TopSitesCallbackList { std::vector<TopSites::GetMostVisitedURLsCallback> callbacks_; }; +} // namespace + // Param specifies whether Popular Sites is enabled via variations. class MostVisitedSitesTest : public ::testing::TestWithParam<bool> { protected: @@ -350,6 +426,10 @@ class MostVisitedSitesTest : public ::testing::TestWithParam<bool> { feature_list_.InitAndDisableFeature( kNtpMostLikelyFaviconsFromServerFeature); + RecreateMostVisitedSites(); + } + + void RecreateMostVisitedSites() { // We use StrictMock to make sure the object is not used unless Popular // Sites is enabled. auto icon_cacher = base::MakeUnique<StrictMock<MockIconCacher>>(); @@ -444,6 +524,12 @@ TEST_P(MostVisitedSitesTest, ShouldStartNoCallInConstructor) { base::RunLoop().RunUntilIdle(); } +TEST_P(MostVisitedSitesTest, ShouldRefreshBothBackends) { + EXPECT_CALL(*mock_top_sites_, SyncWithHistory()); + EXPECT_CALL(mock_suggestions_service_, FetchSuggestionsData()); + most_visited_sites_->Refresh(); +} + TEST_P(MostVisitedSitesTest, ShouldIncludeTileForHomePage) { FakeHomePageClient* home_page_client = RegisterNewHomePageClient(); home_page_client->SetHomePageEnabled(true); @@ -454,7 +540,7 @@ TEST_P(MostVisitedSitesTest, ShouldIncludeTileForHomePage) { EXPECT_CALL(*mock_top_sites_, IsBlacklisted(Eq(GURL(kHomePageUrl)))) .Times(AnyNumber()) .WillRepeatedly(Return(false)); - EXPECT_CALL(mock_observer_, OnMostVisitedURLsAvailable(FirstTileIs( + EXPECT_CALL(mock_observer_, OnURLsAvailable(FirstPersonalizedTileIs( "", kHomePageUrl, TileSource::HOMEPAGE))); most_visited_sites_->SetMostVisitedURLsObserver(&mock_observer_, /*num_sites=*/3); @@ -467,8 +553,10 @@ TEST_P(MostVisitedSitesTest, ShouldNotIncludeHomePageWithoutClient) { .WillRepeatedly(InvokeCallbackArgument<0>(MostVisitedURLList{})); EXPECT_CALL(*mock_top_sites_, SyncWithHistory()); EXPECT_CALL(mock_observer_, - OnMostVisitedURLsAvailable(Not(Contains( - MatchesTile("", kHomePageUrl, TileSource::HOMEPAGE))))); + OnURLsAvailable(Contains( + Pair(SectionType::PERSONALIZED, + Not(Contains(MatchesTile("", kHomePageUrl, + TileSource::HOMEPAGE))))))); most_visited_sites_->SetMostVisitedURLsObserver(&mock_observer_, /*num_sites=*/3); base::RunLoop().RunUntilIdle(); @@ -491,11 +579,15 @@ TEST_P(MostVisitedSitesTest, ShouldIncludeHomeTileWithUrlBeforeQueryingName) { { testing::Sequence seq; EXPECT_CALL(mock_observer_, - OnMostVisitedURLsAvailable(Not(Contains( - MatchesTile("", kHomePageUrl, TileSource::HOMEPAGE))))); + OnURLsAvailable(Contains( + Pair(SectionType::PERSONALIZED, + Not(Contains(MatchesTile("", kHomePageUrl, + TileSource::HOMEPAGE))))))); EXPECT_CALL(mock_observer_, - OnMostVisitedURLsAvailable(Not(Contains(MatchesTile( - kHomePageTitle, kHomePageUrl, TileSource::HOMEPAGE))))); + OnURLsAvailable(Contains( + Pair(SectionType::PERSONALIZED, + Not(Contains(MatchesTile(kHomePageTitle, kHomePageUrl, + TileSource::HOMEPAGE))))))); } most_visited_sites_->SetMostVisitedURLsObserver(&mock_observer_, /*num_sites=*/3); @@ -514,7 +606,7 @@ TEST_P(MostVisitedSitesTest, ShouldUpdateHomePageTileOnHomePageStateChanged) { EXPECT_CALL(*mock_top_sites_, IsBlacklisted(Eq(GURL(kHomePageUrl)))) .Times(AnyNumber()) .WillRepeatedly(Return(false)); - EXPECT_CALL(mock_observer_, OnMostVisitedURLsAvailable(FirstTileIs( + EXPECT_CALL(mock_observer_, OnURLsAvailable(FirstPersonalizedTileIs( "", kHomePageUrl, TileSource::HOMEPAGE))); most_visited_sites_->SetMostVisitedURLsObserver(&mock_observer_, /*num_sites=*/3); @@ -527,7 +619,7 @@ TEST_P(MostVisitedSitesTest, ShouldUpdateHomePageTileOnHomePageStateChanged) { EXPECT_CALL(*mock_top_sites_, GetMostVisitedURLs(_, false)) .WillRepeatedly(InvokeCallbackArgument<0>(MostVisitedURLList{})); EXPECT_CALL(*mock_top_sites_, SyncWithHistory()).Times(0); - EXPECT_CALL(mock_observer_, OnMostVisitedURLsAvailable(Not(FirstTileIs( + EXPECT_CALL(mock_observer_, OnURLsAvailable(Not(FirstPersonalizedTileIs( "", kHomePageUrl, TileSource::HOMEPAGE)))); most_visited_sites_->OnHomePageStateChanged(); base::RunLoop().RunUntilIdle(); @@ -543,13 +635,15 @@ TEST_P(MostVisitedSitesTest, ShouldNotIncludeHomePageIfNoTileRequested) { EXPECT_CALL(*mock_top_sites_, IsBlacklisted(Eq(GURL(kHomePageUrl)))) .Times(AnyNumber()) .WillRepeatedly(Return(false)); - EXPECT_CALL(mock_observer_, OnMostVisitedURLsAvailable(IsEmpty())); + EXPECT_CALL( + mock_observer_, + OnURLsAvailable(Contains(Pair(SectionType::PERSONALIZED, IsEmpty())))); most_visited_sites_->SetMostVisitedURLsObserver(&mock_observer_, /*num_sites=*/0); base::RunLoop().RunUntilIdle(); } -TEST_P(MostVisitedSitesTest, ShouldReturnMostPopularPageIfOneTileRequested) { +TEST_P(MostVisitedSitesTest, ShouldReturnHomePageIfOneTileRequested) { FakeHomePageClient* home_page_client = RegisterNewHomePageClient(); home_page_client->SetHomePageEnabled(true); DisableRemoteSuggestions(); @@ -560,15 +654,17 @@ TEST_P(MostVisitedSitesTest, ShouldReturnMostPopularPageIfOneTileRequested) { EXPECT_CALL(*mock_top_sites_, IsBlacklisted(Eq(GURL(kHomePageUrl)))) .Times(AnyNumber()) .WillRepeatedly(Return(false)); - EXPECT_CALL(mock_observer_, - OnMostVisitedURLsAvailable(ElementsAre(MatchesTile( - "Site 1", "http://site1/", TileSource::TOP_SITES)))); + EXPECT_CALL( + mock_observer_, + OnURLsAvailable(Contains(Pair( + SectionType::PERSONALIZED, + ElementsAre(MatchesTile("", kHomePageUrl, TileSource::HOMEPAGE)))))); most_visited_sites_->SetMostVisitedURLsObserver(&mock_observer_, /*num_sites=*/1); base::RunLoop().RunUntilIdle(); } -TEST_P(MostVisitedSitesTest, ShouldContainHomePageInFirstFourTiles) { +TEST_P(MostVisitedSitesTest, ShouldReplaceLastTileWithHomePageWhenFull) { FakeHomePageClient* home_page_client = RegisterNewHomePageClient(); home_page_client->SetHomePageEnabled(true); DisableRemoteSuggestions(); @@ -584,16 +680,48 @@ TEST_P(MostVisitedSitesTest, ShouldContainHomePageInFirstFourTiles) { EXPECT_CALL(*mock_top_sites_, IsBlacklisted(Eq(GURL(kHomePageUrl)))) .Times(AnyNumber()) .WillRepeatedly(Return(false)); - std::vector<NTPTile> tiles; - EXPECT_CALL(mock_observer_, OnMostVisitedURLsAvailable(_)) - .WillOnce(SaveArg<0>(&tiles)); + std::map<SectionType, NTPTilesVector> sections; + EXPECT_CALL(mock_observer_, OnURLsAvailable(_)) + .WillOnce(SaveArg<0>(§ions)); most_visited_sites_->SetMostVisitedURLsObserver(&mock_observer_, - /*num_sites=*/8); + /*num_sites=*/4); base::RunLoop().RunUntilIdle(); - // Assert that the home page tile is in the first four tiles. + ASSERT_THAT(sections, Contains(Key(SectionType::PERSONALIZED))); + NTPTilesVector tiles = sections.at(SectionType::PERSONALIZED); + ASSERT_THAT(tiles.size(), Ge(4ul)); + // Assert that the home page is appended as the final tile. EXPECT_THAT(tiles[3], MatchesTile("", kHomePageUrl, TileSource::HOMEPAGE)); } +TEST_P(MostVisitedSitesTest, ShouldAppendHomePageWhenNotFull) { + FakeHomePageClient* home_page_client = RegisterNewHomePageClient(); + home_page_client->SetHomePageEnabled(true); + DisableRemoteSuggestions(); + EXPECT_CALL(*mock_top_sites_, GetMostVisitedURLs(_, false)) + .WillRepeatedly(InvokeCallbackArgument<0>((MostVisitedURLList{ + MakeMostVisitedURL("Site 1", "http://site1/"), + MakeMostVisitedURL("Site 2", "http://site2/"), + MakeMostVisitedURL("Site 3", "http://site3/"), + MakeMostVisitedURL("Site 4", "http://site4/"), + MakeMostVisitedURL("Site 5", "http://site5/"), + }))); + EXPECT_CALL(*mock_top_sites_, SyncWithHistory()); + EXPECT_CALL(*mock_top_sites_, IsBlacklisted(Eq(GURL(kHomePageUrl)))) + .Times(AnyNumber()) + .WillRepeatedly(Return(false)); + std::map<SectionType, NTPTilesVector> sections; + EXPECT_CALL(mock_observer_, OnURLsAvailable(_)) + .WillOnce(SaveArg<0>(§ions)); + most_visited_sites_->SetMostVisitedURLsObserver(&mock_observer_, + /*num_sites=*/8); + base::RunLoop().RunUntilIdle(); + ASSERT_THAT(sections, Contains(Key(SectionType::PERSONALIZED))); + NTPTilesVector tiles = sections.at(SectionType::PERSONALIZED); + ASSERT_THAT(tiles.size(), Ge(6ul)); + // Assert that the home page is appended as the final tile. + EXPECT_THAT(tiles[5], MatchesTile("", kHomePageUrl, TileSource::HOMEPAGE)); +} + TEST_P(MostVisitedSitesTest, ShouldDeduplicateHomePageWithTopSites) { FakeHomePageClient* home_page_client = RegisterNewHomePageClient(); home_page_client->SetHomePageEnabled(true); @@ -606,11 +734,13 @@ TEST_P(MostVisitedSitesTest, ShouldDeduplicateHomePageWithTopSites) { EXPECT_CALL(*mock_top_sites_, IsBlacklisted(Eq(GURL(kHomePageUrl)))) .Times(AnyNumber()) .WillRepeatedly(Return(false)); - EXPECT_CALL(mock_observer_, - OnMostVisitedURLsAvailable(AllOf( - Contains(MatchesTile("", kHomePageUrl, TileSource::HOMEPAGE)), - Not(Contains( - MatchesTile("", kHomePageUrl, TileSource::TOP_SITES)))))); + EXPECT_CALL( + mock_observer_, + OnURLsAvailable(Contains(Pair( + SectionType::PERSONALIZED, + AllOf(Contains(MatchesTile("", kHomePageUrl, TileSource::HOMEPAGE)), + Not(Contains( + MatchesTile("", kHomePageUrl, TileSource::TOP_SITES)))))))); most_visited_sites_->SetMostVisitedURLsObserver(&mock_observer_, /*num_sites=*/3); base::RunLoop().RunUntilIdle(); @@ -628,8 +758,10 @@ TEST_P(MostVisitedSitesTest, ShouldNotIncludeHomePageIfItIsNewTabPage) { .Times(AnyNumber()) .WillRepeatedly(Return(false)); EXPECT_CALL(mock_observer_, - OnMostVisitedURLsAvailable(Not(Contains( - MatchesTile("", kHomePageUrl, TileSource::HOMEPAGE))))); + OnURLsAvailable(Contains( + Pair(SectionType::PERSONALIZED, + Not(Contains(MatchesTile("", kHomePageUrl, + TileSource::HOMEPAGE))))))); most_visited_sites_->SetMostVisitedURLsObserver(&mock_observer_, /*num_sites=*/3); base::RunLoop().RunUntilIdle(); @@ -646,8 +778,10 @@ TEST_P(MostVisitedSitesTest, ShouldNotIncludeHomePageIfThereIsNone) { .Times(AnyNumber()) .WillRepeatedly(Return(false)); EXPECT_CALL(mock_observer_, - OnMostVisitedURLsAvailable(Not(Contains( - MatchesTile("", kHomePageUrl, TileSource::HOMEPAGE))))); + OnURLsAvailable(Contains( + Pair(SectionType::PERSONALIZED, + Not(Contains(MatchesTile("", kHomePageUrl, + TileSource::HOMEPAGE))))))); most_visited_sites_->SetMostVisitedURLsObserver(&mock_observer_, /*num_sites=*/3); base::RunLoop().RunUntilIdle(); @@ -666,8 +800,8 @@ TEST_P(MostVisitedSitesTest, ShouldNotIncludeHomePageIfEmptyUrl) { .Times(AnyNumber()) .WillRepeatedly(Return(false)); EXPECT_CALL(mock_observer_, - OnMostVisitedURLsAvailable(Not( - FirstTileIs("", kEmptyHomePageUrl, TileSource::HOMEPAGE)))); + OnURLsAvailable(Not(FirstPersonalizedTileIs( + "", kEmptyHomePageUrl, TileSource::HOMEPAGE)))); most_visited_sites_->SetMostVisitedURLsObserver(&mock_observer_, /*num_sites=*/3); base::RunLoop().RunUntilIdle(); @@ -689,8 +823,10 @@ TEST_P(MostVisitedSitesTest, ShouldNotIncludeHomePageIfBlacklisted) { .Times(AtLeast(1)) .WillRepeatedly(Return(true)); EXPECT_CALL(mock_observer_, - OnMostVisitedURLsAvailable(Not(Contains( - MatchesTile("", kHomePageUrl, TileSource::HOMEPAGE))))); + OnURLsAvailable(Contains( + Pair(SectionType::PERSONALIZED, + Not(Contains(MatchesTile("", kHomePageUrl, + TileSource::HOMEPAGE))))))); most_visited_sites_->SetMostVisitedURLsObserver(&mock_observer_, /*num_sites=*/3); @@ -710,8 +846,10 @@ TEST_P(MostVisitedSitesTest, ShouldPinHomePageAgainIfBlacklistingUndone) { .Times(AtLeast(1)) .WillRepeatedly(Return(true)); EXPECT_CALL(mock_observer_, - OnMostVisitedURLsAvailable(Not(Contains( - MatchesTile("", kHomePageUrl, TileSource::HOMEPAGE))))); + OnURLsAvailable(Contains( + Pair(SectionType::PERSONALIZED, + Not(Contains(MatchesTile("", kHomePageUrl, + TileSource::HOMEPAGE))))))); most_visited_sites_->SetMostVisitedURLsObserver(&mock_observer_, /*num_sites=*/3); @@ -724,8 +862,11 @@ TEST_P(MostVisitedSitesTest, ShouldPinHomePageAgainIfBlacklistingUndone) { EXPECT_CALL(*mock_top_sites_, IsBlacklisted(Eq(GURL(kHomePageUrl)))) .Times(AtLeast(1)) .WillRepeatedly(Return(false)); - EXPECT_CALL(mock_observer_, OnMostVisitedURLsAvailable(Contains(MatchesTile( - "", kHomePageUrl, TileSource::HOMEPAGE)))); + EXPECT_CALL( + mock_observer_, + OnURLsAvailable(Contains(Pair( + SectionType::PERSONALIZED, + Contains(MatchesTile("", kHomePageUrl, TileSource::HOMEPAGE)))))); most_visited_sites_->OnBlockedSitesChanged(); base::RunLoop().RunUntilIdle(); @@ -747,6 +888,89 @@ TEST_P(MostVisitedSitesTest, ShouldInformSuggestionSourcesWhenBlacklisting) { /*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(_, false)) + .WillRepeatedly(InvokeCallbackArgument<0>( + MostVisitedURLList{MakeMostVisitedURL("Site 1", "http://site1/")})); + EXPECT_CALL(*mock_top_sites_, SyncWithHistory()); + EXPECT_CALL(mock_observer_, OnURLsAvailable(_)) + .WillOnce(SaveArg<0>(§ions)); + + most_visited_sites_->SetMostVisitedURLsObserver(&mock_observer_, + /*num_sites=*/3); + base::RunLoop().RunUntilIdle(); + + if (!IsPopularSitesEnabledViaVariations()) { + 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, + ShouldDeduplicatePopularSitesWithMostVisitedIffHostAndTitleMatches) { + pref_service_.SetString(prefs::kPopularSitesOverrideCountry, "US"); + RecreateMostVisitedSites(); // Refills cache with ESPN and Google News. + DisableRemoteSuggestions(); + EXPECT_CALL(*mock_top_sites_, GetMostVisitedURLs(_, false)) + .WillRepeatedly(InvokeCallbackArgument<0>(MostVisitedURLList{ + MakeMostVisitedURL("ESPN", "http://espn.com/"), + MakeMostVisitedURL("Mobile", "http://m.mobile.de/"), + MakeMostVisitedURL("Google", "http://www.google.com/")})); + EXPECT_CALL(*mock_top_sites_, SyncWithHistory()); + std::map<SectionType, NTPTilesVector> sections; + EXPECT_CALL(mock_observer_, OnURLsAvailable(_)) + .WillOnce(SaveArg<0>(§ions)); + + most_visited_sites_->SetMostVisitedURLsObserver(&mock_observer_, + /*num_sites=*/6); + base::RunLoop().RunUntilIdle(); + ASSERT_THAT(sections, Contains(Key(SectionType::PERSONALIZED))); + EXPECT_THAT(sections.at(SectionType::PERSONALIZED), + Contains(MatchesTile("Google", "http://www.google.com/", + TileSource::TOP_SITES))); + if (IsPopularSitesEnabledViaVariations()) { + EXPECT_THAT(sections.at(SectionType::PERSONALIZED), + Contains(MatchesTile("Google News", "http://news.google.com/", + TileSource::POPULAR))); + } + EXPECT_THAT(sections.at(SectionType::PERSONALIZED), + AllOf(Contains(MatchesTile("ESPN", "http://espn.com/", + TileSource::TOP_SITES)), + Contains(MatchesTile("Mobile", "http://m.mobile.de/", + TileSource::TOP_SITES)), + Not(Contains(MatchesTile("ESPN", "http://www.espn.com/", + TileSource::POPULAR))), + Not(Contains(MatchesTile("Mobile", "http://www.mobile.de/", + TileSource::POPULAR))))); +} + TEST_P(MostVisitedSitesTest, ShouldHandleTopSitesCacheHit) { // If cached, TopSites returns the tiles synchronously, running the callback // even before the function returns. @@ -755,7 +979,6 @@ TEST_P(MostVisitedSitesTest, ShouldHandleTopSitesCacheHit) { MostVisitedURLList{MakeMostVisitedURL("Site 1", "http://site1/")})); InSequence seq; - EXPECT_CALL(*mock_top_sites_, SyncWithHistory()); EXPECT_CALL(mock_suggestions_service_, AddCallback(_)) .WillOnce(Invoke(&suggestions_service_callbacks_, &SuggestionsService::ResponseCallbackList::Add)); @@ -764,17 +987,22 @@ TEST_P(MostVisitedSitesTest, ShouldHandleTopSitesCacheHit) { if (IsPopularSitesEnabledViaVariations()) { EXPECT_CALL( mock_observer_, - OnMostVisitedURLsAvailable(ElementsAre( - MatchesTile("Site 1", "http://site1/", TileSource::TOP_SITES), - MatchesTile("PopularSite1", "http://popularsite1/", - TileSource::POPULAR), - MatchesTile("PopularSite2", "http://popularsite2/", - TileSource::POPULAR)))); + OnURLsAvailable(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)))))); } else { EXPECT_CALL(mock_observer_, - OnMostVisitedURLsAvailable(ElementsAre(MatchesTile( - "Site 1", "http://site1/", TileSource::TOP_SITES)))); + OnURLsAvailable(Contains( + Pair(SectionType::PERSONALIZED, + ElementsAre(MatchesTile("Site 1", "http://site1/", + TileSource::TOP_SITES)))))); } + EXPECT_CALL(*mock_top_sites_, SyncWithHistory()); EXPECT_CALL(mock_suggestions_service_, FetchSuggestionsData()) .WillOnce(Return(true)); @@ -792,7 +1020,7 @@ TEST_P(MostVisitedSitesTest, ShouldHandleTopSitesCacheHit) { EXPECT_CALL(*mock_top_sites_, IsBlacklisted(_)) .WillRepeatedly(Return(false)); } - EXPECT_CALL(mock_observer_, OnMostVisitedURLsAvailable(_)); + EXPECT_CALL(mock_observer_, OnURLsAvailable(_)); mock_top_sites_->NotifyTopSitesChanged( history::TopSitesObserver::ChangeReason::MOST_VISITED); base::RunLoop().RunUntilIdle(); @@ -802,13 +1030,47 @@ INSTANTIATE_TEST_CASE_P(MostVisitedSitesTest, MostVisitedSitesTest, ::testing::Bool()); +TEST(MostVisitedSitesTest, ShouldDeduplicateDomainWithNoWwwDomain) { + EXPECT_TRUE(MostVisitedSites::IsHostOrMobilePageKnown({"www.mobile.de"}, + "mobile.de")); + EXPECT_TRUE(MostVisitedSites::IsHostOrMobilePageKnown({"mobile.de"}, + "www.mobile.de")); + EXPECT_TRUE(MostVisitedSites::IsHostOrMobilePageKnown({"mobile.co.uk"}, + "www.mobile.co.uk")); +} + +TEST(MostVisitedSitesTest, ShouldDeduplicateDomainByRemovingMobilePrefixes) { + EXPECT_TRUE( + MostVisitedSites::IsHostOrMobilePageKnown({"bbc.co.uk"}, "m.bbc.co.uk")); + EXPECT_TRUE( + MostVisitedSites::IsHostOrMobilePageKnown({"m.bbc.co.uk"}, "bbc.co.uk")); + EXPECT_TRUE(MostVisitedSites::IsHostOrMobilePageKnown({"cnn.com"}, + "edition.cnn.com")); + EXPECT_TRUE(MostVisitedSites::IsHostOrMobilePageKnown({"edition.cnn.com"}, + "cnn.com")); + EXPECT_TRUE( + MostVisitedSites::IsHostOrMobilePageKnown({"cnn.com"}, "mobile.cnn.com")); + EXPECT_TRUE( + MostVisitedSites::IsHostOrMobilePageKnown({"mobile.cnn.com"}, "cnn.com")); +} + +TEST(MostVisitedSitesTest, ShouldDeduplicateDomainByReplacingMobilePrefixes) { + EXPECT_TRUE(MostVisitedSites::IsHostOrMobilePageKnown({"www.bbc.co.uk"}, + "m.bbc.co.uk")); + EXPECT_TRUE(MostVisitedSites::IsHostOrMobilePageKnown({"m.mobile.de"}, + "www.mobile.de")); + EXPECT_TRUE(MostVisitedSites::IsHostOrMobilePageKnown({"www.cnn.com"}, + "edition.cnn.com")); + EXPECT_TRUE(MostVisitedSites::IsHostOrMobilePageKnown({"mobile.cnn.com"}, + "www.cnn.com")); +} + class MostVisitedSitesWithCacheHitTest : public MostVisitedSitesTest { public: // Constructor sets the common expectations for the case where suggestions // service has cached results when the observer is registered. MostVisitedSitesWithCacheHitTest() { InSequence seq; - EXPECT_CALL(*mock_top_sites_, SyncWithHistory()); EXPECT_CALL(mock_suggestions_service_, AddCallback(_)) .WillOnce(Invoke(&suggestions_service_callbacks_, &SuggestionsService::ResponseCallbackList::Add)); @@ -818,27 +1080,33 @@ class MostVisitedSitesWithCacheHitTest : public MostVisitedSitesTest { MakeSuggestion("Site 2", "http://site2/"), MakeSuggestion("Site 3", "http://site3/"), }))); + if (IsPopularSitesEnabledViaVariations()) { - EXPECT_CALL(mock_observer_, - OnMostVisitedURLsAvailable(ElementsAre( - MatchesTile("Site 1", "http://site1/", - TileSource::SUGGESTIONS_SERVICE), - MatchesTile("Site 2", "http://site2/", - TileSource::SUGGESTIONS_SERVICE), - MatchesTile("Site 3", "http://site3/", - TileSource::SUGGESTIONS_SERVICE), - MatchesTile("PopularSite1", "http://popularsite1/", - TileSource::POPULAR)))); + EXPECT_CALL( + mock_observer_, + OnURLsAvailable(Contains(Pair( + SectionType::PERSONALIZED, + ElementsAre(MatchesTile("Site 1", "http://site1/", + TileSource::SUGGESTIONS_SERVICE), + MatchesTile("Site 2", "http://site2/", + TileSource::SUGGESTIONS_SERVICE), + MatchesTile("Site 3", "http://site3/", + TileSource::SUGGESTIONS_SERVICE), + MatchesTile("PopularSite1", "http://popularsite1/", + TileSource::POPULAR)))))); } else { - EXPECT_CALL(mock_observer_, - OnMostVisitedURLsAvailable(ElementsAre( - MatchesTile("Site 1", "http://site1/", - TileSource::SUGGESTIONS_SERVICE), - MatchesTile("Site 2", "http://site2/", - TileSource::SUGGESTIONS_SERVICE), - MatchesTile("Site 3", "http://site3/", - TileSource::SUGGESTIONS_SERVICE)))); + EXPECT_CALL( + mock_observer_, + OnURLsAvailable(Contains(Pair( + SectionType::PERSONALIZED, + ElementsAre(MatchesTile("Site 1", "http://site1/", + TileSource::SUGGESTIONS_SERVICE), + MatchesTile("Site 2", "http://site2/", + TileSource::SUGGESTIONS_SERVICE), + MatchesTile("Site 3", "http://site3/", + TileSource::SUGGESTIONS_SERVICE)))))); } + EXPECT_CALL(*mock_top_sites_, SyncWithHistory()); EXPECT_CALL(mock_suggestions_service_, FetchSuggestionsData()) .WillOnce(Return(true)); @@ -858,7 +1126,8 @@ TEST_P(MostVisitedSitesWithCacheHitTest, ShouldFavorSuggestionsServiceCache) { TEST_P(MostVisitedSitesWithCacheHitTest, ShouldPropagateUpdateBySuggestionsService) { EXPECT_CALL(mock_observer_, - OnMostVisitedURLsAvailable( + OnURLsAvailable(Contains(Pair( + SectionType::PERSONALIZED, ElementsAre(MatchesTile("Site 4", "http://site4/", TileSource::SUGGESTIONS_SERVICE), MatchesTile("Site 5", "http://site5/", @@ -866,7 +1135,7 @@ TEST_P(MostVisitedSitesWithCacheHitTest, MatchesTile("Site 6", "http://site6/", TileSource::SUGGESTIONS_SERVICE), MatchesTile("Site 7", "http://site7/", - TileSource::SUGGESTIONS_SERVICE)))); + TileSource::SUGGESTIONS_SERVICE)))))); suggestions_service_callbacks_.Notify( MakeProfile({MakeSuggestion("Site 4", "http://site4/"), MakeSuggestion("Site 5", "http://site5/"), @@ -876,7 +1145,9 @@ TEST_P(MostVisitedSitesWithCacheHitTest, } TEST_P(MostVisitedSitesWithCacheHitTest, ShouldTruncateList) { - EXPECT_CALL(mock_observer_, OnMostVisitedURLsAvailable(SizeIs(4))); + EXPECT_CALL( + mock_observer_, + OnURLsAvailable(Contains(Pair(SectionType::PERSONALIZED, SizeIs(4))))); suggestions_service_callbacks_.Notify( MakeProfile({MakeSuggestion("Site 4", "http://site4/"), MakeSuggestion("Site 5", "http://site5/"), @@ -889,19 +1160,23 @@ TEST_P(MostVisitedSitesWithCacheHitTest, ShouldTruncateList) { TEST_P(MostVisitedSitesWithCacheHitTest, ShouldCompleteWithPopularSitesIffEnabled) { if (IsPopularSitesEnabledViaVariations()) { - EXPECT_CALL(mock_observer_, - OnMostVisitedURLsAvailable(ElementsAre( - MatchesTile("Site 4", "http://site4/", - TileSource::SUGGESTIONS_SERVICE), - MatchesTile("PopularSite1", "http://popularsite1/", - TileSource::POPULAR), - MatchesTile("PopularSite2", "http://popularsite2/", - TileSource::POPULAR)))); + EXPECT_CALL( + mock_observer_, + OnURLsAvailable(Contains( + Pair(SectionType::PERSONALIZED, + ElementsAre(MatchesTile("Site 4", "http://site4/", + TileSource::SUGGESTIONS_SERVICE), + MatchesTile("PopularSite1", "http://popularsite1/", + TileSource::POPULAR), + MatchesTile("PopularSite2", "http://popularsite2/", + TileSource::POPULAR)))))); } else { EXPECT_CALL( mock_observer_, - OnMostVisitedURLsAvailable(ElementsAre(MatchesTile( - "Site 4", "http://site4/", TileSource::SUGGESTIONS_SERVICE)))); + OnURLsAvailable(Contains( + Pair(SectionType::PERSONALIZED, + ElementsAre(MatchesTile("Site 4", "http://site4/", + TileSource::SUGGESTIONS_SERVICE)))))); } suggestions_service_callbacks_.Notify( MakeProfile({MakeSuggestion("Site 4", "http://site4/")})); @@ -917,11 +1192,14 @@ TEST_P(MostVisitedSitesWithCacheHitTest, EXPECT_CALL( mock_observer_, - OnMostVisitedURLsAvailable(ElementsAre( - MatchesTile("Site 4", "http://site4/", TileSource::TOP_SITES), - MatchesTile("Site 5", "http://site5/", TileSource::TOP_SITES), - MatchesTile("Site 6", "http://site6/", TileSource::TOP_SITES), - MatchesTile("Site 7", "http://site7/", TileSource::TOP_SITES)))); + OnURLsAvailable(Contains(Pair( + SectionType::PERSONALIZED, + ElementsAre( + MatchesTile("Site 4", "http://site4/", TileSource::TOP_SITES), + MatchesTile("Site 5", "http://site5/", TileSource::TOP_SITES), + MatchesTile("Site 6", "http://site6/", TileSource::TOP_SITES), + MatchesTile("Site 7", "http://site7/", + TileSource::TOP_SITES)))))); top_sites_callbacks_.ClearAndNotify( {MakeMostVisitedURL("Site 4", "http://site4/"), MakeMostVisitedURL("Site 5", "http://site5/"), @@ -934,7 +1212,7 @@ TEST_P(MostVisitedSitesWithCacheHitTest, ShouldFetchFaviconsIfEnabled) { base::test::ScopedFeatureList feature_list; feature_list.InitAndEnableFeature(kNtpMostLikelyFaviconsFromServerFeature); - EXPECT_CALL(mock_observer_, OnMostVisitedURLsAvailable(_)); + EXPECT_CALL(mock_observer_, OnURLsAvailable(_)); EXPECT_CALL(*icon_cacher_, StartFetchMostLikely(GURL("http://site4/"), _)); suggestions_service_callbacks_.Notify( @@ -952,7 +1230,6 @@ class MostVisitedSitesWithEmptyCacheTest : public MostVisitedSitesTest { // service doesn't have cached results when the observer is registered. MostVisitedSitesWithEmptyCacheTest() { InSequence seq; - EXPECT_CALL(*mock_top_sites_, SyncWithHistory()); EXPECT_CALL(mock_suggestions_service_, AddCallback(_)) .WillOnce(Invoke(&suggestions_service_callbacks_, &SuggestionsService::ResponseCallbackList::Add)); @@ -960,6 +1237,7 @@ class MostVisitedSitesWithEmptyCacheTest : public MostVisitedSitesTest { .WillOnce(Return(SuggestionsProfile())); // Empty cache. EXPECT_CALL(*mock_top_sites_, GetMostVisitedURLs(_, false)) .WillOnce(Invoke(&top_sites_callbacks_, &TopSitesCallbackList::Add)); + EXPECT_CALL(*mock_top_sites_, SyncWithHistory()); EXPECT_CALL(mock_suggestions_service_, FetchSuggestionsData()) .WillOnce(Return(true)); @@ -980,19 +1258,23 @@ TEST_P(MostVisitedSitesWithEmptyCacheTest, TEST_P(MostVisitedSitesWithEmptyCacheTest, ShouldCompleteWithPopularSitesIffEnabled) { if (IsPopularSitesEnabledViaVariations()) { - EXPECT_CALL(mock_observer_, - OnMostVisitedURLsAvailable(ElementsAre( - MatchesTile("Site 4", "http://site4/", - TileSource::SUGGESTIONS_SERVICE), - MatchesTile("PopularSite1", "http://popularsite1/", - TileSource::POPULAR), - MatchesTile("PopularSite2", "http://popularsite2/", - TileSource::POPULAR)))); + EXPECT_CALL( + mock_observer_, + OnURLsAvailable(Contains( + Pair(SectionType::PERSONALIZED, + ElementsAre(MatchesTile("Site 4", "http://site4/", + TileSource::SUGGESTIONS_SERVICE), + MatchesTile("PopularSite1", "http://popularsite1/", + TileSource::POPULAR), + MatchesTile("PopularSite2", "http://popularsite2/", + TileSource::POPULAR)))))); } else { EXPECT_CALL( mock_observer_, - OnMostVisitedURLsAvailable(ElementsAre(MatchesTile( - "Site 4", "http://site4/", TileSource::SUGGESTIONS_SERVICE)))); + OnURLsAvailable(Contains( + Pair(SectionType::PERSONALIZED, + ElementsAre(MatchesTile("Site 4", "http://site4/", + TileSource::SUGGESTIONS_SERVICE)))))); } suggestions_service_callbacks_.Notify( MakeProfile({MakeSuggestion("Site 4", "http://site4/")})); @@ -1003,13 +1285,14 @@ TEST_P(MostVisitedSitesWithEmptyCacheTest, ShouldIgnoreTopSitesIfSuggestionsServiceFaster) { // Reply from suggestions service triggers and update to our observer. EXPECT_CALL(mock_observer_, - OnMostVisitedURLsAvailable( + OnURLsAvailable(Contains(Pair( + SectionType::PERSONALIZED, ElementsAre(MatchesTile("Site 1", "http://site1/", TileSource::SUGGESTIONS_SERVICE), MatchesTile("Site 2", "http://site2/", TileSource::SUGGESTIONS_SERVICE), MatchesTile("Site 3", "http://site3/", - TileSource::SUGGESTIONS_SERVICE)))); + TileSource::SUGGESTIONS_SERVICE)))))); suggestions_service_callbacks_.Notify( MakeProfile({MakeSuggestion("Site 1", "http://site1/"), MakeSuggestion("Site 2", "http://site2/"), @@ -1036,10 +1319,13 @@ TEST_P(MostVisitedSitesWithEmptyCacheTest, // Reply from top sites is propagated to observer. EXPECT_CALL( mock_observer_, - OnMostVisitedURLsAvailable(ElementsAre( - MatchesTile("Site 1", "http://site1/", TileSource::TOP_SITES), - MatchesTile("Site 2", "http://site2/", TileSource::TOP_SITES), - MatchesTile("Site 3", "http://site3/", TileSource::TOP_SITES)))); + OnURLsAvailable(Contains(Pair( + SectionType::PERSONALIZED, + ElementsAre( + MatchesTile("Site 1", "http://site1/", TileSource::TOP_SITES), + MatchesTile("Site 2", "http://site2/", TileSource::TOP_SITES), + MatchesTile("Site 3", "http://site3/", + TileSource::TOP_SITES)))))); top_sites_callbacks_.ClearAndNotify( {MakeMostVisitedURL("Site 1", "http://site1/"), MakeMostVisitedURL("Site 2", "http://site2/"), @@ -1052,10 +1338,13 @@ TEST_P(MostVisitedSitesWithEmptyCacheTest, // Reply from top sites is propagated to observer. EXPECT_CALL( mock_observer_, - OnMostVisitedURLsAvailable(ElementsAre( - MatchesTile("Site 1", "http://site1/", TileSource::TOP_SITES), - MatchesTile("Site 2", "http://site2/", TileSource::TOP_SITES), - MatchesTile("Site 3", "http://site3/", TileSource::TOP_SITES)))); + OnURLsAvailable(Contains(Pair( + SectionType::PERSONALIZED, + ElementsAre( + MatchesTile("Site 1", "http://site1/", TileSource::TOP_SITES), + MatchesTile("Site 2", "http://site2/", TileSource::TOP_SITES), + MatchesTile("Site 3", "http://site3/", + TileSource::TOP_SITES)))))); top_sites_callbacks_.ClearAndNotify( {MakeMostVisitedURL("Site 1", "http://site1/"), MakeMostVisitedURL("Site 2", "http://site2/"), @@ -1065,13 +1354,14 @@ TEST_P(MostVisitedSitesWithEmptyCacheTest, // Reply from suggestions service overrides top sites. InSequence seq; EXPECT_CALL(mock_observer_, - OnMostVisitedURLsAvailable( + OnURLsAvailable(Contains(Pair( + SectionType::PERSONALIZED, ElementsAre(MatchesTile("Site 4", "http://site4/", TileSource::SUGGESTIONS_SERVICE), MatchesTile("Site 5", "http://site5/", TileSource::SUGGESTIONS_SERVICE), MatchesTile("Site 6", "http://site6/", - TileSource::SUGGESTIONS_SERVICE)))); + TileSource::SUGGESTIONS_SERVICE)))))); suggestions_service_callbacks_.Notify( MakeProfile({MakeSuggestion("Site 4", "http://site4/"), MakeSuggestion("Site 5", "http://site5/"), @@ -1084,10 +1374,13 @@ TEST_P(MostVisitedSitesWithEmptyCacheTest, // Reply from top sites is propagated to observer. EXPECT_CALL( mock_observer_, - OnMostVisitedURLsAvailable(ElementsAre( - MatchesTile("Site 1", "http://site1/", TileSource::TOP_SITES), - MatchesTile("Site 2", "http://site2/", TileSource::TOP_SITES), - MatchesTile("Site 3", "http://site3/", TileSource::TOP_SITES)))); + OnURLsAvailable(Contains(Pair( + SectionType::PERSONALIZED, + ElementsAre( + MatchesTile("Site 1", "http://site1/", TileSource::TOP_SITES), + MatchesTile("Site 2", "http://site2/", TileSource::TOP_SITES), + MatchesTile("Site 3", "http://site3/", + TileSource::TOP_SITES)))))); top_sites_callbacks_.ClearAndNotify( {MakeMostVisitedURL("Site 1", "http://site1/"), MakeMostVisitedURL("Site 2", "http://site2/"), @@ -1103,10 +1396,13 @@ TEST_P(MostVisitedSitesWithEmptyCacheTest, ShouldPropagateUpdateByTopSites) { // Reply from top sites is propagated to observer. EXPECT_CALL( mock_observer_, - OnMostVisitedURLsAvailable(ElementsAre( - MatchesTile("Site 1", "http://site1/", TileSource::TOP_SITES), - MatchesTile("Site 2", "http://site2/", TileSource::TOP_SITES), - MatchesTile("Site 3", "http://site3/", TileSource::TOP_SITES)))); + OnURLsAvailable(Contains(Pair( + SectionType::PERSONALIZED, + ElementsAre( + MatchesTile("Site 1", "http://site1/", TileSource::TOP_SITES), + MatchesTile("Site 2", "http://site2/", TileSource::TOP_SITES), + MatchesTile("Site 3", "http://site3/", + TileSource::TOP_SITES)))))); top_sites_callbacks_.ClearAndNotify( {MakeMostVisitedURL("Site 1", "http://site1/"), MakeMostVisitedURL("Site 2", "http://site2/"), @@ -1126,10 +1422,13 @@ TEST_P(MostVisitedSitesWithEmptyCacheTest, ShouldPropagateUpdateByTopSites) { MakeMostVisitedURL("Site 6", "http://site6/")})); EXPECT_CALL( mock_observer_, - OnMostVisitedURLsAvailable(ElementsAre( - MatchesTile("Site 4", "http://site4/", TileSource::TOP_SITES), - MatchesTile("Site 5", "http://site5/", TileSource::TOP_SITES), - MatchesTile("Site 6", "http://site6/", TileSource::TOP_SITES)))); + OnURLsAvailable(Contains(Pair( + SectionType::PERSONALIZED, + ElementsAre( + MatchesTile("Site 4", "http://site4/", TileSource::TOP_SITES), + MatchesTile("Site 5", "http://site5/", TileSource::TOP_SITES), + MatchesTile("Site 6", "http://site6/", + TileSource::TOP_SITES)))))); mock_top_sites_->NotifyTopSitesChanged( history::TopSitesObserver::ChangeReason::MOST_VISITED); base::RunLoop().RunUntilIdle(); @@ -1138,16 +1437,19 @@ TEST_P(MostVisitedSitesWithEmptyCacheTest, ShouldPropagateUpdateByTopSites) { TEST_P(MostVisitedSitesWithEmptyCacheTest, ShouldSendEmptyListIfBothTopSitesAndSuggestionsServiceEmpty) { if (IsPopularSitesEnabledViaVariations()) { - EXPECT_CALL(mock_observer_, - OnMostVisitedURLsAvailable(ElementsAre( - MatchesTile("PopularSite1", "http://popularsite1/", - TileSource::POPULAR), - MatchesTile("PopularSite2", "http://popularsite2/", - TileSource::POPULAR)))); + EXPECT_CALL( + mock_observer_, + OnURLsAvailable(Contains( + Pair(SectionType::PERSONALIZED, + ElementsAre(MatchesTile("PopularSite1", "http://popularsite1/", + TileSource::POPULAR), + MatchesTile("PopularSite2", "http://popularsite2/", + TileSource::POPULAR)))))); } else { // The Android NTP doesn't finish initialization until it gets tiles, so a // 0-tile notification is always needed. - EXPECT_CALL(mock_observer_, OnMostVisitedURLsAvailable(IsEmpty())); + EXPECT_CALL(mock_observer_, OnURLsAvailable(ElementsAre(Pair( + SectionType::PERSONALIZED, IsEmpty())))); } suggestions_service_callbacks_.Notify(SuggestionsProfile()); top_sites_callbacks_.ClearAndNotify(MostVisitedURLList{}); @@ -1159,10 +1461,13 @@ TEST_P(MostVisitedSitesWithEmptyCacheTest, ShouldNotifyOnceIfTopSitesUnchanged) { EXPECT_CALL( mock_observer_, - OnMostVisitedURLsAvailable(ElementsAre( - MatchesTile("Site 1", "http://site1/", TileSource::TOP_SITES), - MatchesTile("Site 2", "http://site2/", TileSource::TOP_SITES), - MatchesTile("Site 3", "http://site3/", TileSource::TOP_SITES)))); + OnURLsAvailable(Contains(Pair( + SectionType::PERSONALIZED, + ElementsAre( + MatchesTile("Site 1", "http://site1/", TileSource::TOP_SITES), + MatchesTile("Site 2", "http://site2/", TileSource::TOP_SITES), + MatchesTile("Site 3", "http://site3/", + TileSource::TOP_SITES)))))); suggestions_service_callbacks_.Notify(SuggestionsProfile()); @@ -1189,13 +1494,14 @@ TEST_P(MostVisitedSitesWithEmptyCacheTest, TEST_P(MostVisitedSitesWithEmptyCacheTest, ShouldNotifyOnceIfSuggestionsUnchanged) { EXPECT_CALL(mock_observer_, - OnMostVisitedURLsAvailable( + OnURLsAvailable(Contains(Pair( + SectionType::PERSONALIZED, ElementsAre(MatchesTile("Site 1", "http://site1/", TileSource::SUGGESTIONS_SERVICE), MatchesTile("Site 2", "http://site2/", TileSource::SUGGESTIONS_SERVICE), MatchesTile("Site 3", "http://site3/", - TileSource::SUGGESTIONS_SERVICE)))); + TileSource::SUGGESTIONS_SERVICE)))))); for (int i = 0; i < 5; ++i) { suggestions_service_callbacks_.Notify( @@ -1282,5 +1588,4 @@ TEST(MostVisitedSitesMergeTest, ShouldMergeTilesFavoringPersonalOverPopular) { TileSource::POPULAR))); } -} // namespace } // namespace ntp_tiles diff --git a/chromium/components/ntp_tiles/popular_sites.h b/chromium/components/ntp_tiles/popular_sites.h index f1e207fcac1..7e6b21db219 100644 --- a/chromium/components/ntp_tiles/popular_sites.h +++ b/chromium/components/ntp_tiles/popular_sites.h @@ -5,12 +5,14 @@ #ifndef COMPONENTS_NTP_TILES_POPULAR_SITES_H_ #define COMPONENTS_NTP_TILES_POPULAR_SITES_H_ +#include <map> #include <string> #include <vector> #include "base/callback.h" #include "base/macros.h" #include "base/strings/string16.h" +#include "components/ntp_tiles/section_type.h" #include "url/gurl.h" namespace base { @@ -37,6 +39,7 @@ class PopularSites { GURL favicon_url; GURL large_icon_url; GURL thumbnail_url; + bool baked_in; int default_icon_resource; // < 0 if there is none. Used for popular sites. }; @@ -60,8 +63,8 @@ class PopularSites { virtual bool MaybeStartFetch(bool force_download, const FinishedCallback& callback) = 0; - // Returns the list of available sites. - virtual const SitesVector& sites() const = 0; + // Returns the cached list of available sections and their sites. + virtual const std::map<SectionType, SitesVector>& sections() const = 0; // Various internals exposed publicly for diagnostic pages only. virtual GURL GetLastURLFetched() const = 0; diff --git a/chromium/components/ntp_tiles/popular_sites_impl.cc b/chromium/components/ntp_tiles/popular_sites_impl.cc index 38fc464509b..c238ab8867a 100644 --- a/chromium/components/ntp_tiles/popular_sites_impl.cc +++ b/chromium/components/ntp_tiles/popular_sites_impl.cc @@ -5,13 +5,16 @@ #include "components/ntp_tiles/popular_sites_impl.h" #include <stddef.h> +#include <map> #include <utility> #include "base/bind.h" #include "base/command_line.h" #include "base/feature_list.h" +#include "base/strings/string_number_conversions.h" #include "base/strings/string_util.h" #include "base/strings/stringprintf.h" +#include "base/strings/utf_string_conversions.h" #include "base/time/time.h" #include "base/values.h" #include "build/build_config.h" @@ -53,12 +56,9 @@ const char kPopularSitesURLFormat[] = const char kPopularSitesDefaultDirectory[] = "chrome/ntp/"; const char kPopularSitesDefaultCountryCode[] = "DEFAULT"; const char kPopularSitesDefaultVersion[] = "5"; +const int kSitesExplorationStartVersion = 6; const int kPopularSitesRedownloadIntervalHours = 24; -const char kPopularSitesLastDownloadPref[] = "popular_sites_last_download"; -const char kPopularSitesURLPref[] = "popular_sites_url"; -const char kPopularSitesJsonPref[] = "suggested_sites_json"; - GURL GetPopularSitesURL(const std::string& directory, const std::string& country, const std::string& version) { @@ -128,10 +128,65 @@ PopularSites::SitesVector ParseSiteList(const base::ListValue& list) { GURL(large_icon_url), GURL(thumbnail_url)); item->GetInteger("default_icon_resource", &sites.back().default_icon_resource); + item->GetBoolean("baked_in", &sites.back().baked_in); } return sites; } +std::map<SectionType, PopularSites::SitesVector> ParseVersion5( + const base::ListValue& list) { + return {{SectionType::PERSONALIZED, ParseSiteList(list)}}; +} + +std::map<SectionType, PopularSites::SitesVector> ParseVersion6OrAbove( + const base::ListValue& list) { + // Valid lists would have contained at least the PERSONALIZED section. + std::map<SectionType, PopularSites::SitesVector> sections = { + std::make_pair(SectionType::PERSONALIZED, PopularSites::SitesVector{})}; + for (size_t i = 0; i < list.GetSize(); i++) { + const base::DictionaryValue* item; + if (!list.GetDictionary(i, &item)) { + LOG(WARNING) << "Parsed SitesExploration list contained an invalid " + << "section at position " << i << "."; + continue; + } + int section; + if (!item->GetInteger("section", §ion) || section < 0 || + section > static_cast<int>(SectionType::LAST)) { + LOG(WARNING) << "Parsed SitesExploration list contained a section with " + << "invalid ID (" << section << ")"; + continue; + } + SectionType section_type = static_cast<SectionType>(section); + if (section_type == SectionType::UNKNOWN) { + LOG(WARNING) << "Dropped an unknown section in SitesExploration list."; + continue; + } + const base::ListValue* sites_list; + if (!item->GetList("sites", &sites_list)) { + continue; + } + 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; +} + +std::map<SectionType, PopularSites::SitesVector> ParseSites( + const base::ListValue& list, + int version) { + if (version >= kSitesExplorationStartVersion) { + return ParseVersion6OrAbove(list); + } else { + return ParseVersion5(list); + } +} + #if defined(GOOGLE_CHROME_BUILD) && (defined(OS_ANDROID) || defined(OS_IOS)) void SetDefaultResourceForSite(int index, int resource_id, @@ -157,6 +212,10 @@ std::unique_ptr<base::ListValue> DefaultPopularSites() { ResourceBundle::GetSharedInstance().GetRawDataResource( IDR_DEFAULT_POPULAR_SITES_JSON))); DCHECK(sites); + for (base::Value& site : *sites) { + base::DictionaryValue& dict = static_cast<base::DictionaryValue&>(site); + dict.SetBoolean("baked_in", true); + } #if defined(GOOGLE_CHROME_BUILD) int index = 0; for (int icon_resource : @@ -183,6 +242,7 @@ PopularSites::Site::Site(const base::string16& title, favicon_url(favicon_url), large_icon_url(large_icon_url), thumbnail_url(thumbnail_url), + baked_in(false), default_icon_resource(-1) {} PopularSites::Site::Site(const Site& other) = default; @@ -201,7 +261,9 @@ PopularSitesImpl::PopularSitesImpl( download_context_(download_context), parse_json_(std::move(parse_json)), is_fallback_(false), - sites_(ParseSiteList(*prefs->GetList(kPopularSitesJsonPref))), + sections_( + ParseSites(*prefs->GetList(prefs::kPopularSitesJsonPref), + prefs_->GetInteger(prefs::kPopularSitesVersionPref))), weak_ptr_factory_(this) {} PopularSitesImpl::~PopularSitesImpl() {} @@ -212,7 +274,7 @@ bool PopularSitesImpl::MaybeStartFetch(bool force_download, callback_ = callback; const base::Time last_download_time = base::Time::FromInternalValue( - prefs_->GetInt64(kPopularSitesLastDownloadPref)); + prefs_->GetInt64(prefs::kPopularSitesLastDownloadPref)); const base::TimeDelta time_since_last_download = base::Time::Now() - last_download_time; const base::TimeDelta redownload_interval = @@ -221,7 +283,7 @@ bool PopularSitesImpl::MaybeStartFetch(bool force_download, pending_url_ = GetURLToFetch(); const bool url_changed = - pending_url_.spec() != prefs_->GetString(kPopularSitesURLPref); + pending_url_.spec() != prefs_->GetString(prefs::kPopularSitesURLPref); // Download forced, or we need to download a new file. if (force_download || download_time_is_future || @@ -232,18 +294,20 @@ bool PopularSitesImpl::MaybeStartFetch(bool force_download, return false; } -const PopularSites::SitesVector& PopularSitesImpl::sites() const { - return sites_; +const std::map<SectionType, PopularSitesImpl::SitesVector>& +PopularSitesImpl::sections() const { + return sections_; } GURL PopularSitesImpl::GetLastURLFetched() const { - return GURL(prefs_->GetString(kPopularSitesURLPref)); + return GURL(prefs_->GetString(prefs::kPopularSitesURLPref)); } GURL PopularSitesImpl::GetURLToFetch() { const std::string directory = GetDirectoryToFetch(); const std::string country = GetCountryToFetch(); const std::string version = GetVersionToFetch(); + base::StringToInt(version, &version_in_pending_url_); const GURL override_url = GURL(prefs_->GetString(ntp_tiles::prefs::kPopularSitesOverrideURL)); @@ -311,7 +375,7 @@ std::string PopularSitesImpl::GetVersionToFetch() { } const base::ListValue* PopularSitesImpl::GetCachedJson() { - return prefs_->GetList(kPopularSitesJsonPref); + return prefs_->GetList(prefs::kPopularSitesJsonPref); } // static @@ -326,9 +390,13 @@ void PopularSitesImpl::RegisterProfilePrefs( user_prefs->RegisterStringPref(ntp_tiles::prefs::kPopularSitesOverrideVersion, std::string()); - user_prefs->RegisterInt64Pref(kPopularSitesLastDownloadPref, 0); - user_prefs->RegisterStringPref(kPopularSitesURLPref, std::string()); - user_prefs->RegisterListPref(kPopularSitesJsonPref, DefaultPopularSites()); + user_prefs->RegisterInt64Pref(prefs::kPopularSitesLastDownloadPref, 0); + user_prefs->RegisterStringPref(prefs::kPopularSitesURLPref, std::string()); + user_prefs->RegisterListPref(prefs::kPopularSitesJsonPref, + DefaultPopularSites()); + int version; + base::StringToInt(kPopularSitesDefaultVersion, &version); + user_prefs->RegisterIntegerPref(prefs::kPopularSitesVersionPref, version); } void PopularSitesImpl::FetchPopularSites() { @@ -347,7 +415,7 @@ void PopularSitesImpl::FetchPopularSites() { destination: GOOGLE_OWNED_SERVICE } policy { - cookies_allowed: false + cookies_allowed: NO setting: "This feature cannot be disabled in settings." policy_exception_justification: "Not implemented, considered not useful." @@ -390,13 +458,13 @@ void PopularSitesImpl::OnJsonParsed(std::unique_ptr<base::Value> json) { OnDownloadFailed(); return; } - - prefs_->Set(kPopularSitesJsonPref, *list); - prefs_->SetInt64(kPopularSitesLastDownloadPref, + prefs_->Set(prefs::kPopularSitesJsonPref, *list); + prefs_->SetInt64(prefs::kPopularSitesLastDownloadPref, base::Time::Now().ToInternalValue()); - prefs_->SetString(kPopularSitesURLPref, pending_url_.spec()); + prefs_->SetInteger(prefs::kPopularSitesVersionPref, version_in_pending_url_); + prefs_->SetString(prefs::kPopularSitesURLPref, pending_url_.spec()); - sites_ = ParseSiteList(*list); + sections_ = ParseSites(*list, version_in_pending_url_); callback_.Run(true); } diff --git a/chromium/components/ntp_tiles/popular_sites_impl.h b/chromium/components/ntp_tiles/popular_sites_impl.h index 116297342df..c58dbf3538d 100644 --- a/chromium/components/ntp_tiles/popular_sites_impl.h +++ b/chromium/components/ntp_tiles/popular_sites_impl.h @@ -5,6 +5,7 @@ #ifndef COMPONENTS_NTP_TILES_POPULAR_SITES_IMPL_H_ #define COMPONENTS_NTP_TILES_POPULAR_SITES_IMPL_H_ +#include <map> #include <memory> #include <string> #include <vector> @@ -59,7 +60,7 @@ class PopularSitesImpl : public PopularSites, public net::URLFetcherDelegate { // PopularSites implementation. bool MaybeStartFetch(bool force_download, const FinishedCallback& callback) override; - const SitesVector& sites() const override; + const std::map<SectionType, SitesVector>& sections() const override; GURL GetLastURLFetched() const override; GURL GetURLToFetch() override; std::string GetDirectoryToFetch() override; @@ -95,8 +96,9 @@ class PopularSitesImpl : public PopularSites, public net::URLFetcherDelegate { std::unique_ptr<net::URLFetcher> fetcher_; bool is_fallback_; - SitesVector sites_; + std::map<SectionType, SitesVector> sections_; GURL pending_url_; + int version_in_pending_url_; base::WeakPtrFactory<PopularSitesImpl> weak_ptr_factory_; diff --git a/chromium/components/ntp_tiles/popular_sites_impl_unittest.cc b/chromium/components/ntp_tiles/popular_sites_impl_unittest.cc index d29217f2045..17fc409c3f3 100644 --- a/chromium/components/ntp_tiles/popular_sites_impl_unittest.cc +++ b/chromium/components/ntp_tiles/popular_sites_impl_unittest.cc @@ -16,6 +16,7 @@ #include "base/message_loop/message_loop.h" #include "base/optional.h" #include "base/run_loop.h" +#include "base/strings/string_number_conversions.h" #include "base/test/scoped_feature_list.h" #include "base/threading/thread_task_runner_handle.h" #include "base/values.h" @@ -24,6 +25,7 @@ #include "components/ntp_tiles/json_unsafe_parser.h" #include "components/ntp_tiles/pref_names.h" #include "components/ntp_tiles/switches.h" +#include "components/ntp_tiles/tile_source.h" #include "components/pref_registry/pref_registry_syncable.h" #include "components/sync_preferences/testing_pref_service_syncable.h" #include "net/http/http_status_code.h" @@ -33,9 +35,15 @@ #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" +using testing::_; +using testing::Contains; +using testing::ElementsAre; using testing::Eq; using testing::Gt; using testing::IsEmpty; +using testing::Not; +using testing::Pair; +using testing::SizeIs; namespace ntp_tiles { namespace { @@ -44,9 +52,13 @@ const char kTitle[] = "title"; const char kUrl[] = "url"; const char kLargeIconUrl[] = "large_icon_url"; const char kFaviconUrl[] = "favicon_url"; +const char kSection[] = "section"; +const char kSites[] = "sites"; using TestPopularSite = std::map<std::string, std::string>; using TestPopularSiteVector = std::vector<TestPopularSite>; +using TestPopularSection = std::pair<SectionType, TestPopularSiteVector>; +using TestPopularSectionVector = std::vector<TestPopularSection>; ::testing::Matcher<const base::string16&> Str16Eq(const std::string& s) { return ::testing::Eq(base::UTF8ToUTF16(s)); @@ -95,18 +107,38 @@ class PopularSitesTest : public ::testing::Test { prefs_->SetString(prefs::kPopularSitesOverrideVersion, version); } - void RespondWithJSON(const std::string& url, - const TestPopularSiteVector& sites) { - base::ListValue sites_value; + std::unique_ptr<base::ListValue> CreateListFromTestSites( + const TestPopularSiteVector& sites) { + auto sites_value = base::MakeUnique<base::ListValue>(); for (const TestPopularSite& site : sites) { auto site_value = base::MakeUnique<base::DictionaryValue>(); for (const std::pair<std::string, std::string>& kv : site) { site_value->SetString(kv.first, kv.second); } - sites_value.Append(std::move(site_value)); + sites_value->Append(std::move(site_value)); } + return sites_value; + } + + void RespondWithV5JSON(const std::string& url, + const TestPopularSiteVector& sites) { std::string sites_string; - base::JSONWriter::Write(sites_value, &sites_string); + base::JSONWriter::Write(*CreateListFromTestSites(sites), &sites_string); + url_fetcher_factory_.SetFakeResponse(GURL(url), sites_string, net::HTTP_OK, + net::URLRequestStatus::SUCCESS); + } + + void RespondWithV6JSON(const std::string& url, + const TestPopularSectionVector& sections) { + base::ListValue sections_value; + for (const TestPopularSection& section : sections) { + auto section_value = base::MakeUnique<base::DictionaryValue>(); + section_value->SetInteger(kSection, static_cast<int>(section.first)); + section_value->SetList(kSites, CreateListFromTestSites(section.second)); + sections_value.Append(std::move(section_value)); + } + std::string sites_string; + base::JSONWriter::Write(sections_value, &sites_string); url_fetcher_factory_.SetFakeResponse(GURL(url), sites_string, net::HTTP_OK, net::URLRequestStatus::SUCCESS); } @@ -130,6 +162,18 @@ class PopularSitesTest : public ::testing::Test { // called at all, and if yes which was the returned bool value. base::Optional<bool> FetchPopularSites(bool force_download, PopularSites::SitesVector* sites) { + std::map<SectionType, PopularSites::SitesVector> sections; + base::Optional<bool> save_success = + FetchAllSections(force_download, §ions); + *sites = sections.at(SectionType::PERSONALIZED); + return save_success; + } + + // Returns an optional bool representing whether the completion callback was + // called at all, and if yes which was the returned bool value. + base::Optional<bool> FetchAllSections( + bool force_download, + std::map<SectionType, PopularSites::SitesVector>* sections) { scoped_refptr<net::TestURLRequestContextGetter> url_request_context( new net::TestURLRequestContextGetter( base::ThreadTaskRunnerHandle::Get())); @@ -148,7 +192,7 @@ class PopularSitesTest : public ::testing::Test { &save_success, &loop))) { loop.Run(); } - *sites = popular_sites->sites(); + *sections = popular_sites->sections(); return save_success; } @@ -176,8 +220,10 @@ TEST_F(PopularSitesTest, ContainsDefaultTilesRightAfterConstruction) { base::ThreadTaskRunnerHandle::Get())); auto popular_sites = CreatePopularSites(url_request_context.get()); - EXPECT_THAT(popular_sites->sites().size(), - Eq(GetNumberOfDefaultPopularSitesForPlatform())); + EXPECT_THAT( + popular_sites->sections(), + ElementsAre(Pair(SectionType::PERSONALIZED, + SizeIs(GetNumberOfDefaultPopularSitesForPlatform())))); } TEST_F(PopularSitesTest, IsEmptyOnConstructionIfDisabledByTrial) { @@ -190,17 +236,18 @@ TEST_F(PopularSitesTest, IsEmptyOnConstructionIfDisabledByTrial) { base::ThreadTaskRunnerHandle::Get())); auto popular_sites = CreatePopularSites(url_request_context.get()); - EXPECT_THAT(popular_sites->sites().size(), Eq(0ul)); + EXPECT_THAT(popular_sites->sections(), + ElementsAre(Pair(SectionType::PERSONALIZED, IsEmpty()))); } TEST_F(PopularSitesTest, ShouldSucceedFetching) { - SetCountryAndVersion("ZZ", "9"); - RespondWithJSON( - "https://www.gstatic.com/chrome/ntp/suggested_sites_ZZ_9.json", + SetCountryAndVersion("ZZ", "5"); + RespondWithV5JSON( + "https://www.gstatic.com/chrome/ntp/suggested_sites_ZZ_5.json", {kWikipedia}); PopularSites::SitesVector sites; - EXPECT_THAT(FetchPopularSites(/*force_download=*/false, &sites), + EXPECT_THAT(FetchPopularSites(/*force_download=*/true, &sites), Eq(base::Optional<bool>(true))); ASSERT_THAT(sites.size(), Eq(1u)); @@ -212,10 +259,10 @@ TEST_F(PopularSitesTest, ShouldSucceedFetching) { } TEST_F(PopularSitesTest, Fallback) { - SetCountryAndVersion("ZZ", "9"); + SetCountryAndVersion("ZZ", "5"); RespondWith404( - "https://www.gstatic.com/chrome/ntp/suggested_sites_ZZ_9.json"); - RespondWithJSON( + "https://www.gstatic.com/chrome/ntp/suggested_sites_ZZ_5.json"); + RespondWithV5JSON( "https://www.gstatic.com/chrome/ntp/suggested_sites_DEFAULT_5.json", {kYouTube, kChromium}); @@ -237,9 +284,9 @@ TEST_F(PopularSitesTest, Fallback) { } TEST_F(PopularSitesTest, PopulatesWithDefaultResoucesOnFailure) { - SetCountryAndVersion("ZZ", "9"); + SetCountryAndVersion("ZZ", "5"); RespondWith404( - "https://www.gstatic.com/chrome/ntp/suggested_sites_ZZ_9.json"); + "https://www.gstatic.com/chrome/ntp/suggested_sites_ZZ_5.json"); RespondWith404( "https://www.gstatic.com/chrome/ntp/suggested_sites_DEFAULT_5.json"); @@ -249,6 +296,7 @@ TEST_F(PopularSitesTest, PopulatesWithDefaultResoucesOnFailure) { EXPECT_THAT(sites.size(), Eq(GetNumberOfDefaultPopularSitesForPlatform())); } +#if defined(OS_ANDROID) || defined(OS_IOS) TEST_F(PopularSitesTest, AddsIconResourcesToDefaultPages) { scoped_refptr<net::TestURLRequestContextGetter> url_request_context( new net::TestURLRequestContextGetter( @@ -256,18 +304,22 @@ TEST_F(PopularSitesTest, AddsIconResourcesToDefaultPages) { std::unique_ptr<PopularSites> popular_sites = CreatePopularSites(url_request_context.get()); -#if defined(GOOGLE_CHROME_BUILD) && (defined(OS_ANDROID) || defined(OS_IOS)) - ASSERT_FALSE(popular_sites->sites().empty()); - for (const auto& site : popular_sites->sites()) { + const PopularSites::SitesVector& sites = + popular_sites->sections().at(SectionType::PERSONALIZED); + ASSERT_FALSE(sites.empty()); + for (const auto& site : sites) { + EXPECT_TRUE(site.baked_in); +#if defined(GOOGLE_CHROME_BUILD) EXPECT_THAT(site.default_icon_resource, Gt(0)); - } #endif + } } +#endif TEST_F(PopularSitesTest, ProvidesDefaultSitesUntilCallbackReturns) { - SetCountryAndVersion("ZZ", "9"); - RespondWithJSON( - "https://www.gstatic.com/chrome/ntp/suggested_sites_ZZ_9.json", + SetCountryAndVersion("ZZ", "5"); + RespondWithV5JSON( + "https://www.gstatic.com/chrome/ntp/suggested_sites_ZZ_5.json", {kWikipedia}); scoped_refptr<net::TestURLRequestContextGetter> url_request_context( new net::TestURLRequestContextGetter( @@ -290,20 +342,21 @@ TEST_F(PopularSitesTest, ProvidesDefaultSitesUntilCallbackReturns) { // Assert that callback was scheduled so we can wait for its completion. ASSERT_TRUE(callback_was_scheduled); // There should be 8 default sites as nothing was fetched yet. - EXPECT_THAT(popular_sites->sites().size(), + EXPECT_THAT(popular_sites->sections().at(SectionType::PERSONALIZED).size(), Eq(GetNumberOfDefaultPopularSitesForPlatform())); loop.Run(); // Wait for the fetch to finish and the callback to return. EXPECT_TRUE(save_success.value()); // The 1 fetched site should replace the default sites. - EXPECT_THAT(popular_sites->sites().size(), Eq(1ul)); + EXPECT_THAT(popular_sites->sections(), + ElementsAre(Pair(SectionType::PERSONALIZED, SizeIs(1ul)))); } TEST_F(PopularSitesTest, UsesCachedJson) { - SetCountryAndVersion("ZZ", "9"); - RespondWithJSON( - "https://www.gstatic.com/chrome/ntp/suggested_sites_ZZ_9.json", + SetCountryAndVersion("ZZ", "5"); + RespondWithV5JSON( + "https://www.gstatic.com/chrome/ntp/suggested_sites_ZZ_5.json", {kWikipedia}); // First request succeeds and gets cached. @@ -313,17 +366,17 @@ TEST_F(PopularSitesTest, UsesCachedJson) { // File disappears from server, but we don't need it because it's cached. RespondWith404( - "https://www.gstatic.com/chrome/ntp/suggested_sites_ZZ_9.json"); + "https://www.gstatic.com/chrome/ntp/suggested_sites_ZZ_5.json"); EXPECT_THAT(FetchPopularSites(/*force_download=*/false, &sites), Eq(base::nullopt)); EXPECT_THAT(sites[0].url, URLEq("https://zz.m.wikipedia.org/")); } TEST_F(PopularSitesTest, CachesEmptyFile) { - SetCountryAndVersion("ZZ", "9"); + SetCountryAndVersion("ZZ", "5"); RespondWithData( - "https://www.gstatic.com/chrome/ntp/suggested_sites_ZZ_9.json", "[]"); - RespondWithJSON( + "https://www.gstatic.com/chrome/ntp/suggested_sites_ZZ_5.json", "[]"); + RespondWithV5JSON( "https://www.gstatic.com/chrome/ntp/suggested_sites_DEFAULT_5.json", {kWikipedia}); @@ -334,8 +387,8 @@ TEST_F(PopularSitesTest, CachesEmptyFile) { EXPECT_THAT(sites, IsEmpty()); // File appears on server, but we continue to use our cached empty file. - RespondWithJSON( - "https://www.gstatic.com/chrome/ntp/suggested_sites_ZZ_9.json", + RespondWithV5JSON( + "https://www.gstatic.com/chrome/ntp/suggested_sites_ZZ_5.json", {kWikipedia}); EXPECT_THAT(FetchPopularSites(/*force_download=*/false, &sites), Eq(base::nullopt)); @@ -343,9 +396,9 @@ TEST_F(PopularSitesTest, CachesEmptyFile) { } TEST_F(PopularSitesTest, DoesntUseCachedFileIfDownloadForced) { - SetCountryAndVersion("ZZ", "9"); - RespondWithJSON( - "https://www.gstatic.com/chrome/ntp/suggested_sites_ZZ_9.json", + SetCountryAndVersion("ZZ", "5"); + RespondWithV5JSON( + "https://www.gstatic.com/chrome/ntp/suggested_sites_ZZ_5.json", {kWikipedia}); // First request succeeds and gets cached. @@ -355,41 +408,65 @@ TEST_F(PopularSitesTest, DoesntUseCachedFileIfDownloadForced) { EXPECT_THAT(sites[0].url, URLEq("https://zz.m.wikipedia.org/")); // File disappears from server. Download is forced, so we get the new file. - RespondWithJSON( - "https://www.gstatic.com/chrome/ntp/suggested_sites_ZZ_9.json", + RespondWithV5JSON( + "https://www.gstatic.com/chrome/ntp/suggested_sites_ZZ_5.json", {kChromium}); EXPECT_THAT(FetchPopularSites(/*force_download=*/true, &sites), Eq(base::Optional<bool>(true))); EXPECT_THAT(sites[0].url, URLEq("https://www.chromium.org/")); } +TEST_F(PopularSitesTest, DoesntUseCacheWithDeprecatedVersion) { + SetCountryAndVersion("ZZ", "5"); + RespondWithV5JSON( + "https://www.gstatic.com/chrome/ntp/suggested_sites_ZZ_5.json", + {kWikipedia}); + + // First request succeeds and gets cached. + PopularSites::SitesVector sites; + EXPECT_THAT(FetchPopularSites(/*force_download=*/false, &sites), + Eq(base::Optional<bool>(true))); + EXPECT_THAT(sites[0].url, URLEq("https://zz.m.wikipedia.org/")); + EXPECT_THAT(prefs_->GetInteger(prefs::kPopularSitesVersionPref), Eq(5)); + + // The client is updated to use V6. Drop old data and refetch. + SetCountryAndVersion("ZZ", "6"); + RespondWithV6JSON( + "https://www.gstatic.com/chrome/ntp/suggested_sites_ZZ_6.json", + {{SectionType::PERSONALIZED, {kChromium}}}); + EXPECT_THAT(FetchPopularSites(/*force_download=*/false, &sites), + Eq(base::Optional<bool>(true))); + EXPECT_THAT(sites[0].url, URLEq("https://www.chromium.org/")); + EXPECT_THAT(prefs_->GetInteger(prefs::kPopularSitesVersionPref), Eq(6)); +} + TEST_F(PopularSitesTest, RefetchesAfterCountryMoved) { - RespondWithJSON( - "https://www.gstatic.com/chrome/ntp/suggested_sites_ZZ_9.json", + RespondWithV5JSON( + "https://www.gstatic.com/chrome/ntp/suggested_sites_ZZ_5.json", {kWikipedia}); - RespondWithJSON( - "https://www.gstatic.com/chrome/ntp/suggested_sites_ZX_9.json", + RespondWithV5JSON( + "https://www.gstatic.com/chrome/ntp/suggested_sites_ZX_5.json", {kChromium}); PopularSites::SitesVector sites; // First request (in ZZ) saves Wikipedia. - SetCountryAndVersion("ZZ", "9"); + SetCountryAndVersion("ZZ", "5"); EXPECT_THAT(FetchPopularSites(/*force_download=*/false, &sites), Eq(base::Optional<bool>(true))); EXPECT_THAT(sites[0].url, URLEq("https://zz.m.wikipedia.org/")); // Second request (now in ZX) saves Chromium. - SetCountryAndVersion("ZX", "9"); + SetCountryAndVersion("ZX", "5"); EXPECT_THAT(FetchPopularSites(/*force_download=*/false, &sites), base::Optional<bool>(true)); EXPECT_THAT(sites[0].url, URLEq("https://www.chromium.org/")); } TEST_F(PopularSitesTest, DoesntCacheInvalidFile) { - SetCountryAndVersion("ZZ", "9"); + SetCountryAndVersion("ZZ", "5"); RespondWithData( - "https://www.gstatic.com/chrome/ntp/suggested_sites_ZZ_9.json", + "https://www.gstatic.com/chrome/ntp/suggested_sites_ZZ_5.json", "ceci n'est pas un json"); RespondWith404( "https://www.gstatic.com/chrome/ntp/suggested_sites_DEFAULT_5.json"); @@ -400,8 +477,8 @@ TEST_F(PopularSitesTest, DoesntCacheInvalidFile) { Eq(base::Optional<bool>(false))); // Second request refetches ZZ_9, which now has data. - RespondWithJSON( - "https://www.gstatic.com/chrome/ntp/suggested_sites_ZZ_9.json", + RespondWithV5JSON( + "https://www.gstatic.com/chrome/ntp/suggested_sites_ZZ_5.json", {kChromium}); EXPECT_THAT(FetchPopularSites(/*force_download=*/false, &sites), Eq(base::Optional<bool>(true))); @@ -410,10 +487,10 @@ TEST_F(PopularSitesTest, DoesntCacheInvalidFile) { } TEST_F(PopularSitesTest, RefetchesAfterFallback) { - SetCountryAndVersion("ZZ", "9"); + SetCountryAndVersion("ZZ", "5"); RespondWith404( - "https://www.gstatic.com/chrome/ntp/suggested_sites_ZZ_9.json"); - RespondWithJSON( + "https://www.gstatic.com/chrome/ntp/suggested_sites_ZZ_5.json"); + RespondWithV5JSON( "https://www.gstatic.com/chrome/ntp/suggested_sites_DEFAULT_5.json", {kWikipedia}); @@ -425,8 +502,8 @@ TEST_F(PopularSitesTest, RefetchesAfterFallback) { EXPECT_THAT(sites[0].url, URLEq("https://zz.m.wikipedia.org/")); // Second request refetches ZZ_9, which now has data. - RespondWithJSON( - "https://www.gstatic.com/chrome/ntp/suggested_sites_ZZ_9.json", + RespondWithV5JSON( + "https://www.gstatic.com/chrome/ntp/suggested_sites_ZZ_5.json", {kChromium}); EXPECT_THAT(FetchPopularSites(/*force_download=*/false, &sites), Eq(base::Optional<bool>(true))); @@ -435,10 +512,10 @@ TEST_F(PopularSitesTest, RefetchesAfterFallback) { } TEST_F(PopularSitesTest, ShouldOverrideDirectory) { - SetCountryAndVersion("ZZ", "9"); + SetCountryAndVersion("ZZ", "5"); prefs_->SetString(prefs::kPopularSitesOverrideDirectory, "foo/bar/"); - RespondWithJSON("https://www.gstatic.com/foo/bar/suggested_sites_ZZ_9.json", - {kWikipedia}); + RespondWithV5JSON("https://www.gstatic.com/foo/bar/suggested_sites_ZZ_5.json", + {kWikipedia}); PopularSites::SitesVector sites; EXPECT_THAT(FetchPopularSites(/*force_download=*/false, &sites), @@ -447,5 +524,64 @@ TEST_F(PopularSitesTest, ShouldOverrideDirectory) { EXPECT_THAT(sites.size(), Eq(1u)); } +TEST_F(PopularSitesTest, DoesNotFetchExplorationSitesWithoutFeature) { + base::test::ScopedFeatureList override_features; + override_features.InitAndDisableFeature(kSiteExplorationUiFeature); + + SetCountryAndVersion("ZZ", "6"); + RespondWithV6JSON( + "https://www.gstatic.com/chrome/ntp/suggested_sites_ZZ_6.json", + {{SectionType::PERSONALIZED, {kChromium}}, + {SectionType::NEWS, {kYouTube}}}); + + std::map<SectionType, PopularSites::SitesVector> sections; + EXPECT_THAT(FetchAllSections(/*force_download=*/false, §ions), + Eq(base::Optional<bool>(true))); + + // The fetched news section should not be propagated without enabled feature. + 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, §ions), + 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, §ions), + 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 ntp_tiles diff --git a/chromium/components/ntp_tiles/pref_names.cc b/chromium/components/ntp_tiles/pref_names.cc index c8839987362..409de49b546 100644 --- a/chromium/components/ntp_tiles/pref_names.cc +++ b/chromium/components/ntp_tiles/pref_names.cc @@ -25,5 +25,11 @@ const char kPopularSitesOverrideCountry[] = "popular_sites.override_country"; // If set, this will override the default file version for popular sites. const char kPopularSitesOverrideVersion[] = "popular_sites.override_version"; +// Prefs used to cache suggested sites and store caching meta data. +const char kPopularSitesLastDownloadPref[] = "popular_sites_last_download"; +const char kPopularSitesURLPref[] = "popular_sites_url"; +const char kPopularSitesJsonPref[] = "suggested_sites_json"; +const char kPopularSitesVersionPref[] = "suggested_sites_version"; + } // namespace prefs } // namespace ntp_tiles diff --git a/chromium/components/ntp_tiles/pref_names.h b/chromium/components/ntp_tiles/pref_names.h index 9c4de063cf9..687f5577b29 100644 --- a/chromium/components/ntp_tiles/pref_names.h +++ b/chromium/components/ntp_tiles/pref_names.h @@ -15,6 +15,11 @@ extern const char kPopularSitesOverrideDirectory[]; extern const char kPopularSitesOverrideCountry[]; extern const char kPopularSitesOverrideVersion[]; +extern const char kPopularSitesLastDownloadPref[]; +extern const char kPopularSitesURLPref[]; +extern const char kPopularSitesJsonPref[]; +extern const char kPopularSitesVersionPref[]; + } // namespace prefs } // namespace ntp_tiles diff --git a/chromium/components/ntp_tiles/section_type.h b/chromium/components/ntp_tiles/section_type.h new file mode 100644 index 00000000000..f1f9f5ca801 --- /dev/null +++ b/chromium/components/ntp_tiles/section_type.h @@ -0,0 +1,30 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef COMPONENTS_NTP_TILES_SECTION_TYPE_H_ +#define COMPONENTS_NTP_TILES_SECTION_TYPE_H_ + +namespace ntp_tiles { + +// The type of a section means all its tiles originate here. Ranked descendingly +// from most important section to least important. +// A Java counterpart will be generated for this enum. +// GENERATED_JAVA_ENUM_PACKAGE: org.chromium.chrome.browser.suggestions +// GENERATED_JAVA_CLASS_NAME_OVERRIDE: TileSectionType +enum class SectionType { + UNKNOWN, + PERSONALIZED, + SOCIAL, + ENTERTAINMENT, + NEWS, + ECOMMERCE, + TOOLS, + TRAVEL, + + LAST = TRAVEL +}; + +} // namespace ntp_tiles + +#endif // COMPONENTS_NTP_TILES_SECTION_TYPE_H_ diff --git a/chromium/components/ntp_tiles/tile_source.h b/chromium/components/ntp_tiles/tile_source.h index 53ca5eab207..551e1f92557 100644 --- a/chromium/components/ntp_tiles/tile_source.h +++ b/chromium/components/ntp_tiles/tile_source.h @@ -17,6 +17,8 @@ enum class TileSource { SUGGESTIONS_SERVICE, // Tile is regionally popular. POPULAR, + // Tile is a popular site baked into the binary. + POPULAR_BAKED_IN, // Tile is on a custodian-managed whitelist. WHITELIST, // Tile containing the user-set home page is replacing the home page button. diff --git a/chromium/components/ntp_tiles/webui/ntp_tiles_internals_message_handler.cc b/chromium/components/ntp_tiles/webui/ntp_tiles_internals_message_handler.cc index 098e7156ae7..64fbb6d2264 100644 --- a/chromium/components/ntp_tiles/webui/ntp_tiles_internals_message_handler.cc +++ b/chromium/components/ntp_tiles/webui/ntp_tiles_internals_message_handler.cc @@ -5,6 +5,8 @@ #include "components/ntp_tiles/webui/ntp_tiles_internals_message_handler.h" #include <array> +#include <utility> +#include <vector> #include "base/bind.h" #include "base/callback.h" @@ -276,10 +278,12 @@ void NTPTilesInternalsMessageHandler::SendTiles( result); } -void NTPTilesInternalsMessageHandler::OnMostVisitedURLsAvailable( - const NTPTilesVector& tiles) { +void NTPTilesInternalsMessageHandler::OnURLsAvailable( + const std::map<SectionType, NTPTilesVector>& sections) { cancelable_task_tracker_.TryCancelAll(); + // TODO(fhorschig): Handle non-personalized tiles - https://crbug.com/753852. + const NTPTilesVector& tiles = sections.at(SectionType::PERSONALIZED); if (tiles.empty()) { SendTiles(tiles, FaviconResultMap()); return; diff --git a/chromium/components/ntp_tiles/webui/ntp_tiles_internals_message_handler.h b/chromium/components/ntp_tiles/webui/ntp_tiles_internals_message_handler.h index 4306f832692..7f42af2b571 100644 --- a/chromium/components/ntp_tiles/webui/ntp_tiles_internals_message_handler.h +++ b/chromium/components/ntp_tiles/webui/ntp_tiles_internals_message_handler.h @@ -60,7 +60,8 @@ class NTPTilesInternalsMessageHandler : public MostVisitedSites::Observer { const FaviconResultMap& result_map); // MostVisitedSites::Observer. - void OnMostVisitedURLsAvailable(const NTPTilesVector& tiles) override; + void OnURLsAvailable( + const std::map<SectionType, NTPTilesVector>& sections) override; void OnIconMadeAvailable(const GURL& site_url) override; void OnFaviconLookupDone(const NTPTilesVector& tiles, diff --git a/chromium/components/ntp_tiles/webui/resources/ntp_tiles_internals.html b/chromium/components/ntp_tiles/webui/resources/ntp_tiles_internals.html index c407b726676..8e922ea74db 100644 --- a/chromium/components/ntp_tiles/webui/resources/ntp_tiles_internals.html +++ b/chromium/components/ntp_tiles/webui/resources/ntp_tiles_internals.html @@ -46,7 +46,7 @@ found in the LICENSE file. <tbody jsselect="suggestionsService"> <tr jsdisplay="$this"> <td class="detail"> - <input id="suggestions-fetch" type="submit" value="Fetch"></input> + <input id="suggestions-fetch" type="submit" value="Fetch"> </td> <td class="value" jscontent="status"></td> </tr> @@ -77,7 +77,7 @@ found in the LICENSE file. </tr> <tr jsdisplay="$this"> <td class="detail"> - <input id="popular-view-json" type="submit", value="View JSON"></input> + <input id="popular-view-json" type="submit", value="View JSON"> </td> <td class="value"><pre id="popular-json-value" jscontent="json"></pre></td> </tr> @@ -96,7 +96,7 @@ found in the LICENSE file. <td class="value" jsdisplay="!$this">no</td> </tr> <tr jsskip="true"> - <th colspan="2"><input id="submit-update" type="submit" value="Update"></input></th> + <th colspan="2"><input id="submit-update" type="submit" value="Update"></th> </tr> </tbody> </table> @@ -115,9 +115,10 @@ found in the LICENSE file. <td class="value" jsdisplay="source == 0">TOP_SITES</td> <td class="value" jsdisplay="source == 1">SUGGESTIONS_SERVICE</td> <td class="value" jsdisplay="source == 2">POPULAR</td> - <td class="value" jsdisplay="source == 3">WHITELIST</td> - <td class="value" jsdisplay="source == 4">HOMEPAGE</td> - <td class="value" jsdisplay="source > 4">???</td> + <td class="value" jsdisplay="source == 3">POPULAR_BAKED_IN</td> + <td class="value" jsdisplay="source == 4">WHITELIST</td> + <td class="value" jsdisplay="source == 5">HOMEPAGE</td> + <td class="value" jsdisplay="source > 5">???</td> </tr> <tr> <td class="detail">URL</td> |