[ImportantSites] Limiting the # of Bookmark signals

chrome/browser/android/preferences/important_sites_util.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 <algorithm>
 #include <map>
 #include <memory>
 #include <set>
@@ skipping 17 matching lines @@
 #include "net/base/registry_controlled_domains/registry_controlled_domain.h"
 #include "url/gurl.h"
 
 namespace {
 using bookmarks::BookmarkModel;
 using ImportantDomainInfo = ImportantSitesUtil::ImportantDomainInfo;
 
 static const char kNumTimesIgnoredName[] = "NumTimesIgnored";
 static const int kTimesIgnoredForBlacklist = 3;
 
+// These are the maximum # of bookmarks we can use as signals. If the user has
+// <= kMaxBookmarks, then we just use those bookmarks. Otherwise we filter all
+// bookmarks on site engagement > 0, sort, and trim to kMaxBookmarks.
+static const int kMaxBookmarks = 5;
+
 // Do not change the values here, as they are used for UMA histograms.
 enum ImportantReason {
   ENGAGEMENT = 0,
   DURABLE = 1,
   BOOKMARKS = 2,
   HOME_SCREEN = 3,
   NOTIFICATIONS = 4,
   REASON_BOUNDARY
 };
 
@@ skipping 150 matching lines @@
     }
 
     ignoring_domains.insert(origin.host());
   }
   return ignoring_domains;
 }
 
 void PopulateInfoMapWithSiteEngagement(
     Profile* profile,
     SiteEngagementService::EngagementLevel minimum_engagement,
+    std::map<GURL, double>* engagement_map,
     base::hash_map<std::string, ImportantDomainInfo>* output) {
   SiteEngagementService* service = SiteEngagementService::Get(profile);
-  std::map<GURL, double> engagement_map = service->GetScoreMap();
+  *engagement_map = service->GetScoreMap();
   // We can have multiple origins for a single domain, so we record the one
   // with the highest engagement score.
-  for (const auto& url_engagement_pair : engagement_map) {
+  for (const auto& url_engagement_pair : *engagement_map) {
     if (!service->IsEngagementAtLeast(url_engagement_pair.first,
                                       minimum_engagement)) {
       continue;
     }
     std::string registerable_domain =
         GetRegisterableDomainOrIP(url_engagement_pair.first);
     ImportantDomainInfo& info = (*output)[registerable_domain];
     if (url_engagement_pair.second > info.engagement_score) {
       info.registerable_domain = registerable_domain;
       info.engagement_score = url_engagement_pair.second;
@@ skipping 19 matching lines @@
   for (const ContentSettingPatternSource& site : content_settings_list) {
     if (site.setting != CONTENT_SETTING_ALLOW)
       continue;
     MaybePopulateImportantInfoForReason(GURL(site.primary_pattern.ToString()),
                                         &content_origins, reason, output);
   }
 }
 
 void PopulateInfoMapWithBookmarks(
     Profile* profile,
+    const std::map<GURL, double>& engagement_map,
     base::hash_map<std::string, ImportantDomainInfo>* output) {
+  SiteEngagementService* service = SiteEngagementService::Get(profile);
   BookmarkModel* model =
       BookmarkModelFactory::GetForBrowserContextIfExists(profile);
   if (!model)
     return;
-  std::vector<BookmarkModel::URLAndTitle> bookmarks;
-  model->GetBookmarks(&bookmarks);
+  std::vector<BookmarkModel::URLAndTitle> untrimmed_bookmarks;
+  model->GetBookmarks(&untrimmed_bookmarks);
+
+  // Process the bookmarks and optionally trim them if we have too many.
+  std::vector<BookmarkModel::URLAndTitle> result_bookmarks;
+  if (untrimmed_bookmarks.size() > kMaxBookmarks) {
+    std::copy_if(untrimmed_bookmarks.begin(), untrimmed_bookmarks.end(),
+                 std::back_inserter(result_bookmarks),
+                 [service](const BookmarkModel::URLAndTitle& entry) {
+                   return service->IsEngagementAtLeast(
+                       entry.url.GetOrigin(),
+                       SiteEngagementService::ENGAGEMENT_LEVEL_LOW);
+                 });
+    std::sort(result_bookmarks.begin(), result_bookmarks.end(),
+              [&engagement_map](const BookmarkModel::URLAndTitle& a,
+                                const BookmarkModel::URLAndTitle& b) {
+                double a_score = engagement_map.at(a.url.GetOrigin());
+                double b_score = engagement_map.at(b.url.GetOrigin());
+                return a_score > b_score;
+              });
+    if (result_bookmarks.size() > kMaxBookmarks)
+      result_bookmarks.resize(kMaxBookmarks);
+  } else {
+    result_bookmarks = std::move(untrimmed_bookmarks);
+  }
+
   std::set<GURL> content_origins;
-  for (const BookmarkModel::URLAndTitle& bookmark : bookmarks) {
+  for (const BookmarkModel::URLAndTitle& bookmark : result_bookmarks) {
     MaybePopulateImportantInfoForReason(bookmark.url, &content_origins,
                                         ImportantReason::BOOKMARKS, output);
   }
 }
 
 void PopulateInfoMapWithHomeScreen(
     Profile* profile,
     base::hash_map<std::string, ImportantDomainInfo>* output) {
   ContentSettingsForOneType content_settings_list;
   HostContentSettingsMapFactory::GetForProfile(profile)->GetSettingsForOneType(
@@ skipping 11 matching lines @@
                                         ImportantReason::HOME_SCREEN, output);
   }
 }
 
 }  // namespace
 
 std::vector<ImportantDomainInfo>
 ImportantSitesUtil::GetImportantRegisterableDomains(Profile* profile,
                                                     size_t max_results) {
   base::hash_map<std::string, ImportantDomainInfo> important_info;
+  std::map<GURL, double> engagement_map;
 
   PopulateInfoMapWithSiteEngagement(
-      profile, SiteEngagementService::ENGAGEMENT_LEVEL_MEDIUM, &important_info);
+      profile, SiteEngagementService::ENGAGEMENT_LEVEL_MEDIUM, &engagement_map,
+      &important_info);
 
   PopulateInfoMapWithContentTypeAllowed(
       profile, CONTENT_SETTINGS_TYPE_NOTIFICATIONS,
       ImportantReason::NOTIFICATIONS, &important_info);
 
   PopulateInfoMapWithContentTypeAllowed(
       profile, CONTENT_SETTINGS_TYPE_DURABLE_STORAGE, ImportantReason::DURABLE,
       &important_info);
 
-  PopulateInfoMapWithBookmarks(profile, &important_info);
+  PopulateInfoMapWithBookmarks(profile, engagement_map, &important_info);
 
   PopulateInfoMapWithHomeScreen(profile, &important_info);
 
   base::hash_set<std::string> blacklisted_domains =
       GetBlacklistedImportantDomains(profile);
 
   std::vector<std::pair<std::string, ImportantDomainInfo>> items(
       important_info.begin(), important_info.end());
   std::sort(items.begin(), items.end(), &CompareDescendingImportantInfo);
 
@@ skipping 81 matching lines @@
 void ImportantSitesUtil::MarkOriginAsImportantForTesting(Profile* profile,
                                                          const GURL& origin) {
   // First get data from site engagement.
   SiteEngagementService* site_engagement_service =
       SiteEngagementService::Get(profile);
   site_engagement_service->ResetScoreForURL(
       origin, SiteEngagementScore::GetMediumEngagementBoundary());
   DCHECK(site_engagement_service->IsEngagementAtLeast(
       origin, SiteEngagementService::ENGAGEMENT_LEVEL_MEDIUM));
 }