[NTP] Use short title on custom link

Generate short title for custom link based on a heuristic discussed on
crbug.com/856653. Notice that user can always edit the title as they
want.

Screencast:
https://screencast.googleplex.com/cast/NTE0MTQ3NjMxNzA2OTMxMnwxMWEyNTZjNy1lYw

Bug: 856653
Change-Id: Ibe1e675598d99620b91dcd610310c861803ff83e
Reviewed-on: https://chromium-review.googlesource.com/1181776Reviewed-by: Marc Treib <treib@chromium.org>
Reviewed-by: Kristi Park <kristipark@chromium.org>
Commit-Queue: Weilun Shi <sweilun@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593839}

components/ntp_tiles/most_visited_sites.cc

@@ -12,6 +12,7 @@
 #include "base/callback.h"
 #include "base/feature_list.h"
 #include "base/metrics/user_metrics.h"
+#include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "components/history/core/browser/top_sites.h"
@@ -88,6 +89,37 @@ bool ShouldShowPopularSites() {
   return base::FeatureList::IsEnabled(kUsePopularSitesSuggestions);
 }
 
+// Generate a short title for Most Visited items before they're converted to
+// custom links.
+base::string16 GenerateShortTitle(const base::string16& title) {
+  // Empty title only happened in the unittests.
+  if (title.empty())
+    return base::string16();
+  std::vector<base::string16> short_title_list =
+      SplitString(title, base::UTF8ToUTF16("-:|;"), base::TRIM_WHITESPACE,
+                  base::SPLIT_WANT_NONEMPTY);
+  // Make sure it doesn't crash when the title only contains spaces.
+  if (short_title_list.empty())
+    return base::string16();
+  base::string16 short_title_front = short_title_list.front();
+  base::string16 short_title_back = short_title_list.back();
+  base::string16 short_title = short_title_front;
+  if (short_title_front != short_title_back) {
+    int words_in_front =
+        SplitString(short_title_front, base::kWhitespaceASCIIAs16,
+                    base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY)
+            .size();
+    int words_in_back =
+        SplitString(short_title_back, base::kWhitespaceASCIIAs16,
+                    base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY)
+            .size();
+    if (words_in_front >= 3 && words_in_back >= 1 && words_in_back <= 3) {
+      short_title = short_title_back;
+    }
+  }
+  return short_title;
+}
+
 }  // namespace
 
 MostVisitedSites::MostVisitedSites(
@@ -376,7 +408,8 @@ void MostVisitedSites::OnMostVisitedURLsAvailable(
       continue;
 
     NTPTile tile;
-    tile.title = visited.title;
+    tile.title = IsCustomLinksEnabled() ? GenerateShortTitle(visited.title)
+                                        : visited.title;
     tile.url = visited.url;
     tile.source = TileSource::TOP_SITES;
     tile.whitelist_icon_path = GetWhitelistLargeIconPath(visited.url);
@@ -441,7 +474,10 @@ void MostVisitedSites::BuildCurrentTilesGivenSuggestionsProfile(
       continue;
 
     NTPTile tile;
-    tile.title = base::UTF8ToUTF16(suggestion_pb.title());
+    tile.title =
+        IsCustomLinksEnabled()
+            ? GenerateShortTitle(base::UTF8ToUTF16(suggestion_pb.title()))
+            : base::UTF8ToUTF16(suggestion_pb.title());
     tile.url = url;
     tile.source = TileSource::SUGGESTIONS_SERVICE;
     // The title is an aggregation of multiple history entries of one site.

components/ntp_tiles/most_visited_sites_unittest.cc

@@ -1366,6 +1366,139 @@ TEST_P(MostVisitedSitesTest, DisableCustomLinksWhenInitialized) {
       sections.at(SectionType::PERSONALIZED),
       ElementsAre(MatchesTile(kTestTitle, kTestUrl, TileSource::CUSTOM_LINKS)));
 }
+
+TEST_P(MostVisitedSitesTest, ShouldGenerateShortTitleForTopSites) {
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitAndEnableFeature(kNtpCustomLinks);
+  std::string kTestUrl1 = "https://www.imdb.com/";
+  std::string kTestTitle1 = "IMDb - Movies, TV and Celebrities - IMDb";
+  std::string kTestUrl2 = "https://drive.google.com/";
+  std::string kTestTitle2 =
+      "Google Drive - Cloud Storage & File Backup for Photos, Docs & More";
+  std::string kTestUrl3 = "https://amazon.com/";
+  std::string kTestTitle3 =
+      "Amazon.com: Online Shopping for Electronics, Apparel, Computers, Books, "
+      "DVDs & more";
+
+  std::map<SectionType, NTPTilesVector> sections;
+  EnableCustomLinks();
+  RecreateMostVisitedSites();
+  DisableRemoteSuggestions();
+
+  // Build tiles from Top Sites. The tiles should have short titles.
+  EXPECT_CALL(*mock_custom_links_, RegisterCallbackForOnChanged(_));
+  EXPECT_CALL(*mock_top_sites_, GetMostVisitedURLs(_, false))
+      .WillRepeatedly(InvokeCallbackArgument<0>(
+          MostVisitedURLList{MakeMostVisitedURL(kTestTitle1, kTestUrl1),
+                             MakeMostVisitedURL(kTestTitle2, kTestUrl2),
+                             MakeMostVisitedURL(kTestTitle3, kTestUrl3)}));
+  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=*/3);
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_THAT(
+      sections.at(SectionType::PERSONALIZED),
+      ElementsAre(
+          MatchesTile(/* The short title generated by the heuristic */ "IMDb",
+                      kTestUrl1, TileSource::TOP_SITES),
+          MatchesTile(
+              /* The short title generated by the heuristic */ "Google Drive",
+              kTestUrl2, TileSource::TOP_SITES),
+          MatchesTile(
+              /* The short title generated by the heuristic */ "Amazon.com",
+              kTestUrl3, TileSource::TOP_SITES)));
+}
+
+TEST_P(MostVisitedSitesTest, ShouldGenerateShortTitleForSuggestions) {
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitAndEnableFeature(kNtpCustomLinks);
+  std::string kTestUrl1 = "https://www.imdb.com/";
+  std::string kTestTitle1 = "IMDb - Movies, TV and Celebrities - IMDb";
+  std::string kTestUrl2 = "https://drive.google.com/";
+  std::string kTestTitle2 =
+      "Google Drive - Cloud Storage & File Backup for Photos, Docs & More";
+  std::string kTestUrl3 = "https://amazon.com/";
+  std::string kTestTitle3 =
+      "Amazon.com: Online Shopping for Electronics, Apparel, Computers, Books, "
+      "DVDs & more";
+
+  std::map<SectionType, NTPTilesVector> sections;
+  EnableCustomLinks();
+  RecreateMostVisitedSites();
+
+  // Build tiles from Suggestions. The tiles should have short titles.
+  EXPECT_CALL(*mock_custom_links_, RegisterCallbackForOnChanged(_));
+  EXPECT_CALL(mock_suggestions_service_, AddCallback(_))
+      .WillOnce(Invoke(&suggestions_service_callbacks_,
+                       &SuggestionsService::ResponseCallbackList::Add));
+  EXPECT_CALL(mock_suggestions_service_, GetSuggestionsDataFromCache())
+      .WillOnce(Return(MakeProfile({MakeSuggestion(kTestTitle1, kTestUrl1),
+                                    MakeSuggestion(kTestTitle2, kTestUrl2),
+                                    MakeSuggestion(kTestTitle3, kTestUrl3)})));
+  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=*/3);
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_THAT(
+      sections.at(SectionType::PERSONALIZED),
+      ElementsAre(
+          MatchesTile(/* The short title generated by the heuristic */ "IMDb",
+                      kTestUrl1, TileSource::SUGGESTIONS_SERVICE),
+          MatchesTile(
+              /* The short title generated by the heuristic */ "Google Drive",
+              kTestUrl2, TileSource::SUGGESTIONS_SERVICE),
+          MatchesTile(
+              /* The short title generated by the heuristic */ "Amazon.com",
+              kTestUrl3, TileSource::SUGGESTIONS_SERVICE)));
+}
+
+TEST_P(MostVisitedSitesTest, ShouldNotCrashIfReceiveAnEmptyTitle) {
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitAndEnableFeature(kNtpCustomLinks);
+  std::string kTestUrl1 = "https://site1/";
+  std::string kTestTitle1 = "";  // Empty title
+  std::string kTestUrl2 = "https://site2/";
+  std::string kTestTitle2 = "       ";  // Title only contains spaces
+
+  std::map<SectionType, NTPTilesVector> sections;
+  EnableCustomLinks();
+  RecreateMostVisitedSites();
+  DisableRemoteSuggestions();
+
+  // Build tiles from Top Sites. The tiles should have short titles.
+  EXPECT_CALL(*mock_custom_links_, RegisterCallbackForOnChanged(_));
+  EXPECT_CALL(*mock_top_sites_, GetMostVisitedURLs(_, false))
+      .WillRepeatedly(InvokeCallbackArgument<0>(
+          MostVisitedURLList{MakeMostVisitedURL(kTestTitle1, kTestUrl1),
+                             MakeMostVisitedURL(kTestTitle2, kTestUrl2)}));
+  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=*/2);
+  base::RunLoop().RunUntilIdle();
+
+  // Both cases should not crash and generate an empty title tile.
+  EXPECT_THAT(sections.at(SectionType::PERSONALIZED),
+              ElementsAre(MatchesTile("", kTestUrl1, TileSource::TOP_SITES),
+                          MatchesTile("", kTestUrl2, TileSource::TOP_SITES)));
+}
 #endif
 
 class MostVisitedSitesWithCacheHitTest : public MostVisitedSitesTest {