[ImportantSites] Limiting the # of Bookmark signals

chrome/browser/android/preferences/important_sites_util_unittest.cc

 // Copyright 2016 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.
 
 #include "chrome/browser/android/preferences/important_sites_util.h"
 
 #include <memory>
 #include <utility>
 
 #include "base/files/scoped_temp_dir.h"
@@ skipping 167 matching lines @@
   AddBookmark(url1);
   important_sites = ImportantSitesUtil::GetImportantRegisterableDomains(
       profile(), kNumImportantSites);
   expected_sorted_domains = {"youtube.com", "google.com", "foo.bar",
                              "example.com", "chrome.com"};
   expected_sorted_origins = {url6, url3, url7, url5, url4};
   ExpectImportantResultsEq(expected_sorted_domains, expected_sorted_origins,
                            important_sites);
 }
 
+TEST_F(ImportantSitesUtilTest, TooManyBookmarks) {
+  SiteEngagementService* service = SiteEngagementService::Get(profile());
+  ASSERT_TRUE(service);
+
+  GURL url1("http://www.google.com/");
+  GURL url2("https://www.google.com/");
+  GURL url3("https://drive.google.com/");
+  GURL url4("https://www.chrome.com/");
+  GURL url5("https://www.example.com/");
+  GURL url6("https://youtube.com/");
+  GURL url7("https://foo.bar/");
+
+  // Add some as bookmarks.
+  AddBookmark(url1);
+  AddBookmark(url2);
+  AddBookmark(url3);
+  AddBookmark(url4);
+  AddBookmark(url5);
+
+  // We have just below our limit, so all sites are important (the first three
+  // origins collapse, so we end up with 3).
+  std::vector<ImportantDomainInfo> important_sites =
+      ImportantSitesUtil::GetImportantRegisterableDomains(profile(),
+                                                          kNumImportantSites);
+  EXPECT_EQ(3u, important_sites.size());
+
+  // Add the rest, which should put us over the limit.
+  AddBookmark(url6);
+  AddBookmark(url7);
+  // Too many bookmarks! Nothing shows up now.
+  important_sites = ImportantSitesUtil::GetImportantRegisterableDomains(
+      profile(), kNumImportantSites);
+  EXPECT_EQ(0u, important_sites.size());
+
+  // If we add some site engagement, these should show up in order (even though
+  // the engagement is too low for a signal by itself).
+  service->ResetScoreForURL(url1, 2);
+  service->ResetScoreForURL(url4, 3);
+  service->ResetScoreForURL(url7, 0);
+
+  important_sites = ImportantSitesUtil::GetImportantRegisterableDomains(
+      profile(), kNumImportantSites);
+  ASSERT_EQ(2u, important_sites.size());
+  std::vector<std::string> expected_sorted_domains = {"google.com",
+                                                      "chrome.com"};
+  std::vector<GURL> expected_sorted_origins = {url1, url4};
+  ExpectImportantResultsEq(expected_sorted_domains, expected_sorted_origins,
+                           important_sites);
+}
+
 TEST_F(ImportantSitesUtilTest, Blacklisting) {
   SiteEngagementService* service = SiteEngagementService::Get(profile());
   ASSERT_TRUE(service);
 
   GURL url1("http://www.google.com/");
 
   // Set a bunch of positive signals.
   service->ResetScoreForURL(url1, 5);
   AddBookmark(url1);
   AddContentSetting(CONTENT_SETTINGS_TYPE_NOTIFICATIONS, CONTENT_SETTING_ALLOW,
@@ skipping 126 matching lines @@
       histogram_tester.GetAllSamples("Storage.ImportantSites.CBDIgnoredReason"),
       testing::ElementsAre(base::Bucket(BOOKMARKS, 1)));
 
   // Bookmarks are "unknown", as they were added after the crossed reasons.
   EXPECT_THAT(histogram_tester.GetAllSamples(
                   "Storage.BlacklistedImportantSites.Reason"),
               testing::ElementsAre(
                   base::Bucket(CROSSED_NOTIFICATIONS_AND_ENGAGEMENT, 1),
                   base::Bucket(CROSSED_REASON_UNKNOWN, 1)));
 }