[Previews] Set an engagement score threshold for top hosts.

This change requires host to have a minimum engagement score to be
considered a top host returned by the TopHostProvider. This prevents
hosts that the user is unlikely to navigate to from having hints
fetched for them. It also adds an extra layer of protection from
disclosing a user's history after enabling DataSaver and
HintsFetcher for the first time.

Bug: 932707
Change-Id: I38eec3a71c23b5fcbb4aa712aff5e337637f52d9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1673859
Commit-Queue: Michael Crouse <mcrouse@chromium.org>
Auto-Submit: Michael Crouse <mcrouse@chromium.org>
Reviewed-by: Robert Ogden <robertogden@chromium.org>
Reviewed-by: Doug Arnett <dougarnett@chromium.org>
Cr-Commit-Position: refs/heads/master@{#671930}

chrome/browser/previews/previews_top_host_provider_impl.cc

@@ -7,6 +7,7 @@
 #include "base/metrics/histogram_macros.h"
 #include "base/values.h"
 #include "chrome/browser/engagement/site_engagement_details.mojom.h"
+#include "chrome/browser/engagement/site_engagement_score.h"
 #include "chrome/browser/engagement/site_engagement_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "components/prefs/pref_service.h"
@@ -203,6 +204,13 @@ std::vector<std::string> PreviewsTopHostProviderImpl::GetTopHosts(
   for (const auto& detail : engagement_details) {
     if (top_hosts.size() >= max_sites)
       return top_hosts;
+    // Once the engagement score is less than the initial engagement score for a
+    // newly navigated host, return the current set of top hosts. This threshold
+    // prevents hosts that have not been engaged recently from having hints
+    // requested for them. The engagement_details are sorted above in descending
+    // order by engagement score.
+    if (detail.total_score <= GetMinTopHostEngagementThreshold())
+      return top_hosts;
     // TODO(b/968542): Skip origins that are local hosts (e.g., IP addresses,
     // localhost:8080 etc.).
     if (detail.origin.SchemeIs(url::kHttpsScheme) &&
@@ -214,4 +222,13 @@ std::vector<std::string> PreviewsTopHostProviderImpl::GetTopHosts(
   return top_hosts;
 }
 
+size_t PreviewsTopHostProviderImpl::GetMinTopHostEngagementThreshold() const {
+  // The base score for the first navigation of a host when added to the site
+  // engagement service. The threshold corresponds to the minimum score that a
+  // host is considered to be a top host, hosts with a lower score have not
+  // been navigated to recently.
+  return SiteEngagementScore::GetNavigationPoints() +
+         SiteEngagementScore::GetFirstDailyEngagementPoints();
+}
+
 }  // namespace previews

chrome/browser/previews/previews_top_host_provider_impl.h

@@ -50,6 +50,10 @@ class PreviewsTopHostProviderImpl : public PreviewsTopHostProvider {
   optimization_guide::prefs::HintsFetcherTopHostBlacklistState
   GetCurrentBlacklistState() const;
 
+  // The minimum engagement score that a host must have to be considered a top
+  // host by |this|.
+  size_t GetMinTopHostEngagementThreshold() const;
+
   // Transition the current HintsFetcherTopHostBlacklist state to |state| and
   // validate the transition. The updated state is persisted in the
   // |kHintsFetcherTopHostBlacklistState| pref.

chrome/browser/previews/previews_top_host_provider_impl_unittest.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/previews/previews_top_host_provider_impl.h"
 
 #include "base/values.h"
+#include "chrome/browser/engagement/site_engagement_score.h"
 #include "chrome/browser/engagement/site_engagement_service.h"
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
 #include "chrome/test/base/testing_profile.h"
@@ -31,8 +32,10 @@ class PreviewsTopHostProviderImplTest : public ChromeRenderViewHostTestHarness {
 
   void AddEngagedHosts(size_t num_hosts) {
     for (size_t i = 1; i <= num_hosts; i++) {
-      AddEngagedHost(GURL(base::StringPrintf("https://domain%zu.com", i)),
-                     static_cast<int>(i));
+      AddEngagedHost(
+          GURL(base::StringPrintf("https://domain%zu.com", i)),
+          static_cast<int>(
+              i + SiteEngagementScore::GetFirstDailyEngagementPoints()));
     }
   }
 
@@ -326,4 +329,37 @@ TEST_F(PreviewsTopHostProviderImplTest,
   EXPECT_FALSE(IsHostBlacklisted(base::StringPrintf("domain%u.com", 1u)));
 }
 
+TEST_F(PreviewsTopHostProviderImplTest, TopHostsFilteredByEngagementThreshold) {
+  size_t engaged_hosts =
+      previews::params::MaxHintsFetcherTopHostBlacklistSize() + 1;
+  size_t max_top_hosts =
+      previews::params::MaxHintsFetcherTopHostBlacklistSize() + 1;
+
+  AddEngagedHosts(engaged_hosts);
+  // Add two hosts with very low engagement scores that should not be returned
+  // by the top host provider.
+  AddEngagedHost(GURL("https://lowengagement.com"), 1);
+  AddEngagedHost(GURL("https://lowengagement2.com"), 1);
+
+  // Blacklist should be populated on the first request.
+  std::vector<std::string> hosts =
+      top_host_provider()->GetTopHosts(max_top_hosts);
+  EXPECT_EQ(hosts.size(), 0u);
+
+  hosts = top_host_provider()->GetTopHosts(max_top_hosts);
+  EXPECT_EQ(
+      hosts.size(),
+      engaged_hosts - previews::params::MaxHintsFetcherTopHostBlacklistSize());
+  EXPECT_EQ(GetCurrentTopHostBlacklistState(),
+            optimization_guide::prefs::HintsFetcherTopHostBlacklistState::
+                kInitialized);
+
+  // The hosts with engagement scores below the minimum threshold should not be
+  // returned.
+  EXPECT_EQ(std::find(hosts.begin(), hosts.end(), "lowengagement.com"),
+            hosts.end());
+  EXPECT_EQ(std::find(hosts.begin(), hosts.end(), "lowengagement2.com"),
+            hosts.end());
+}
+
 }  // namespace previews