[NTP] Ignore notifications for new Top Sites or Suggestions if custom links is initialized

New notifications for Top Sites or Suggestions would overwrite the
current custom links, regardless if custom links was initialized or not.
This would replace the tiles in the frontend with the new set of Top
Sites/Suggestions and break custom link functionality.

Bug: 856394
Change-Id: Id60459722448f626c1c21e2424cbfbe55df5b6c6
Reviewed-on: https://chromium-review.googlesource.com/1159325
Commit-Queue: Kristi Park <kristipark@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580277}

components/ntp_tiles/most_visited_sites.cc

@@ -346,9 +346,10 @@ base::FilePath MostVisitedSites::GetWhitelistLargeIconPath(const GURL& url) {
 
 void MostVisitedSites::OnMostVisitedURLsAvailable(
     const history::MostVisitedURLList& visited_list) {
-  // Ignore the event if tiles provided by the Suggestions Service, which take
-  // precedence.
-  if (mv_source_ == TileSource::SUGGESTIONS_SERVICE) {
+  // Ignore the event if tiles are provided by the Suggestions Service or custom
+  // links, which take precedence.
+  if ((custom_links_ && custom_links_->IsInitialized()) ||
+      mv_source_ == TileSource::SUGGESTIONS_SERVICE) {
     return;
   }
 
@@ -380,8 +381,11 @@ void MostVisitedSites::OnMostVisitedURLsAvailable(
 
 void MostVisitedSites::OnSuggestionsProfileChanged(
     const SuggestionsProfile& suggestions_profile) {
-  if (suggestions_profile.suggestions_size() == 0 &&
-      mv_source_ != TileSource::SUGGESTIONS_SERVICE) {
+  // Ignore the event if tiles are provided by custom links, which take
+  // precedence.
+  if ((custom_links_ && custom_links_->IsInitialized()) ||
+      (suggestions_profile.suggestions_size() == 0 &&
+       mv_source_ != TileSource::SUGGESTIONS_SERVICE)) {
     return;
   }
 

components/ntp_tiles/most_visited_sites_unittest.cc

@@ -1090,7 +1090,8 @@ TEST_P(MostVisitedSitesTest, ShouldOnlyBuildCustomLinksWhenInitialized) {
       .WillRepeatedly(InvokeCallbackArgument<0>(
           MostVisitedURLList{MakeMostVisitedURL(kTestTitle, kTestUrl)}));
   EXPECT_CALL(*mock_top_sites_, SyncWithHistory());
-  EXPECT_CALL(*mock_custom_links_, IsInitialized()).WillOnce(Return(false));
+  EXPECT_CALL(*mock_custom_links_, IsInitialized())
+      .WillRepeatedly(Return(false));
   EXPECT_CALL(mock_observer_, OnURLsAvailable(_))
       .WillOnce(SaveArg<0>(&sections));
 
@@ -1123,7 +1124,8 @@ TEST_P(MostVisitedSitesTest, ShouldOnlyBuildCustomLinksWhenInitialized) {
       .WillRepeatedly(InvokeCallbackArgument<0>(
           MostVisitedURLList{MakeMostVisitedURL(kTestTitle, kTestUrl)}));
   EXPECT_CALL(*mock_top_sites_, SyncWithHistory());
-  EXPECT_CALL(*mock_custom_links_, IsInitialized()).WillOnce(Return(false));
+  EXPECT_CALL(*mock_custom_links_, IsInitialized())
+      .WillRepeatedly(Return(false));
   EXPECT_CALL(mock_observer_, OnURLsAvailable(_))
       .WillOnce(SaveArg<0>(&sections));
 
@@ -1134,6 +1136,118 @@ TEST_P(MostVisitedSitesTest, ShouldOnlyBuildCustomLinksWhenInitialized) {
       sections.at(SectionType::PERSONALIZED),
       ElementsAre(MatchesTile(kTestTitle, kTestUrl, TileSource::TOP_SITES)));
 }
+
+TEST_P(MostVisitedSitesTest, ShouldFavorCustomLinksOverTopSites) {
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitAndEnableFeature(kNtpCustomLinks);
+  const char kTestUrl[] = "http://site1/";
+  const char kTestTitle[] = "Site 1";
+  std::vector<CustomLinksManager::Link> expected_links(
+      {CustomLinksManager::Link{GURL(kTestUrl),
+                                base::UTF8ToUTF16(kTestTitle)}});
+
+  std::map<SectionType, NTPTilesVector> sections;
+  EnableCustomLinks();
+  RecreateMostVisitedSites();
+  DisableRemoteSuggestions();
+
+  // Build tiles when custom links is not initialized. Tiles should be Top
+  // Sites.
+  EXPECT_CALL(*mock_top_sites_, GetMostVisitedURLs(_, false))
+      .WillRepeatedly(InvokeCallbackArgument<0>(
+          MostVisitedURLList{MakeMostVisitedURL(kTestTitle, kTestUrl)}));
+  EXPECT_CALL(*mock_top_sites_, SyncWithHistory());
+  EXPECT_CALL(*mock_custom_links_, IsInitialized())
+      .WillRepeatedly(Return(false));
+  EXPECT_CALL(mock_observer_, OnURLsAvailable(_))
+      .WillOnce(SaveArg<0>(&sections));
+  most_visited_sites_->SetMostVisitedURLsObserver(&mock_observer_,
+                                                  /*num_sites=*/1);
+  base::RunLoop().RunUntilIdle();
+  ASSERT_THAT(
+      sections.at(SectionType::PERSONALIZED),
+      ElementsAre(MatchesTile(kTestTitle, kTestUrl, TileSource::TOP_SITES)));
+
+  // Initialize custom links and rebuild tiles. Tiles should be custom links.
+  EXPECT_CALL(*mock_custom_links_, Initialize(_)).WillOnce(Return(true));
+  EXPECT_CALL(*mock_custom_links_, IsInitialized()).WillOnce(Return(true));
+  EXPECT_CALL(*mock_custom_links_, GetLinks())
+      .WillOnce(ReturnRef(expected_links));
+  EXPECT_CALL(mock_observer_, OnURLsAvailable(_))
+      .WillOnce(SaveArg<0>(&sections));
+  most_visited_sites_->InitializeCustomLinks();
+  base::RunLoop().RunUntilIdle();
+  ASSERT_THAT(
+      sections.at(SectionType::PERSONALIZED),
+      ElementsAre(MatchesTile(kTestTitle, kTestUrl, TileSource::CUSTOM_LINKS)));
+
+  // Initiate notification for new Top Sites. This should be ignored.
+  EXPECT_CALL(*mock_custom_links_, IsInitialized()).WillOnce(Return(true));
+  // Notify with an empty SuggestionsProfile first to trigger a query for
+  // TopSites.
+  suggestions_service_callbacks_.Notify(SuggestionsProfile());
+  VerifyAndClearExpectations();
+  EXPECT_CALL(mock_observer_, OnURLsAvailable(_)).Times(0);
+  top_sites_callbacks_.ClearAndNotify(
+      {MakeMostVisitedURL("Site 2", "http://site2/")});
+  base::RunLoop().RunUntilIdle();
+}
+
+TEST_P(MostVisitedSitesTest, ShouldFavorCustomLinksOverSuggestions) {
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitAndEnableFeature(kNtpCustomLinks);
+  const char kTestUrl[] = "http://site1/";
+  const char kTestTitle[] = "Site 1";
+  std::vector<CustomLinksManager::Link> expected_links(
+      {CustomLinksManager::Link{GURL(kTestUrl),
+                                base::UTF8ToUTF16(kTestTitle)}});
+
+  std::map<SectionType, NTPTilesVector> sections;
+  EnableCustomLinks();
+  RecreateMostVisitedSites();
+
+  // Build tiles when custom links is not initialized. Tiles should be Top
+  // Sites.
+  EXPECT_CALL(mock_suggestions_service_, AddCallback(_))
+      .WillOnce(Invoke(&suggestions_service_callbacks_,
+                       &SuggestionsService::ResponseCallbackList::Add));
+  EXPECT_CALL(mock_suggestions_service_, GetSuggestionsDataFromCache())
+      .WillOnce(Return(MakeProfile({MakeSuggestion(kTestTitle, kTestUrl)})));
+  EXPECT_CALL(*mock_top_sites_, SyncWithHistory());
+  EXPECT_CALL(mock_suggestions_service_, FetchSuggestionsData())
+      .WillOnce(Return(true));
+  EXPECT_CALL(*mock_custom_links_, IsInitialized())
+      .WillRepeatedly(Return(false));
+  EXPECT_CALL(mock_observer_, OnURLsAvailable(_))
+      .WillOnce(SaveArg<0>(&sections));
+  most_visited_sites_->SetMostVisitedURLsObserver(&mock_observer_,
+                                                  /*num_sites=*/1);
+  base::RunLoop().RunUntilIdle();
+  ASSERT_THAT(sections.at(SectionType::PERSONALIZED),
+              ElementsAre(MatchesTile(kTestTitle, kTestUrl,
+                                      TileSource::SUGGESTIONS_SERVICE)));
+
+  // Initialize custom links and rebuild tiles. Tiles should be custom links.
+  EXPECT_CALL(*mock_custom_links_, Initialize(_)).WillOnce(Return(true));
+  EXPECT_CALL(*mock_custom_links_, IsInitialized()).WillOnce(Return(true));
+  EXPECT_CALL(*mock_custom_links_, GetLinks())
+      .WillOnce(ReturnRef(expected_links));
+  EXPECT_CALL(mock_observer_, OnURLsAvailable(_))
+      .WillOnce(SaveArg<0>(&sections));
+  most_visited_sites_->InitializeCustomLinks();
+  base::RunLoop().RunUntilIdle();
+  ASSERT_THAT(
+      sections.at(SectionType::PERSONALIZED),
+      ElementsAre(MatchesTile(kTestTitle, kTestUrl, TileSource::CUSTOM_LINKS)));
+
+  // Initiate notification for new suggestions. This should be ignored.
+  EXPECT_CALL(*mock_custom_links_, IsInitialized())
+      .WillRepeatedly(Return(true));
+  EXPECT_CALL(mock_observer_, OnURLsAvailable(_)).Times(0);
+  suggestions_service_callbacks_.Notify(
+      MakeProfile({MakeSuggestion("Site 2", "http://site2/")}));
+  base::RunLoop().RunUntilIdle();
+}
 #endif
 
 class MostVisitedSitesWithCacheHitTest : public MostVisitedSitesTest {