NavigationPredictor: Set a threshold score for the prefetch URL

Prefetch a URL only if it's score is more than the specified threshold.
Additionally, only the highest ranking URL is now eligible for
prefetching.

Bug: 903945
Change-Id: I6c8166e8424da8c06f093998d606e1962184d20d
TBR: ryansturm@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/c/1352034Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611533}

chrome/browser/navigation_predictor/navigation_predictor.cc

@@ -108,9 +108,14 @@ NavigationPredictor::NavigationPredictor(
                   is_same_host_scale_ + contains_image_scale_ +
                   is_url_incremented_scale_ + source_engagement_score_scale_ +
                   target_engagement_score_scale_ + area_rank_scale_),
-      is_low_end_device_(base::SysInfo::IsLowEndDevice()) {
+      is_low_end_device_(base::SysInfo::IsLowEndDevice()),
+      prefetch_url_score_threshold_(base::GetFieldTrialParamByFeatureAsInt(
+          blink::features::kRecordAnchorMetricsVisible,
+          "prefetch_url_score_threshold",
+          0)) {
   DCHECK(browser_context_);
   DETACH_FROM_SEQUENCE(sequence_checker_);
+  DCHECK_LE(0, prefetch_url_score_threshold_);
 
   if (render_frame_host != GetMainFrame(render_frame_host))
     return;
@@ -702,15 +707,22 @@ base::Optional<GURL> NavigationPredictor::GetUrlToPrefetch(
   if (source_is_default_search_engine_page_)
     return base::nullopt;
 
-  // Return the same-origin URL that has the highest navigation score.
-  for (const auto& navigation_score : sorted_navigation_scores) {
-    // Currently, only same origin URLs can be prefetched.
-    if (url::Origin::Create(navigation_score->url) != document_origin)
-      continue;
-    return navigation_score->url;
+  if (sorted_navigation_scores.empty())
+    return base::nullopt;
+
+  // Only the same origin URLs are eligible for prefetching. If the URL with
+  // the highest score is from a different origin, then we skip prefetching
+  // since same origin URLs are not likely to be clicked.
+  if (url::Origin::Create(sorted_navigation_scores[0]->url) !=
+      document_origin) {
+    return base::nullopt;
   }
 
-  return base::nullopt;
+  // If the prediction score of the highest scoring URL is less than the
+  // threshold, then return.
+  if (sorted_navigation_scores[0]->score < prefetch_url_score_threshold_)
+    return base::nullopt;
+  return sorted_navigation_scores[0]->url;
 }
 
 void NavigationPredictor::RecordMetricsOnLoad(

chrome/browser/navigation_predictor/navigation_predictor.h

@@ -82,6 +82,7 @@ class NavigationPredictor : public blink::mojom::AnchorElementMetricsHost,
   };
 
  protected:
+  // URL that we decided to prefetch.
   base::Optional<GURL> prefetch_url_;
 
  private:
@@ -183,6 +184,9 @@ class NavigationPredictor : public blink::mojom::AnchorElementMetricsHost,
   // True if device is a low end device.
   const bool is_low_end_device_;
 
+  // Minimum score that a URL should have for it to be prefetched.
+  const int prefetch_url_score_threshold_;
+
   // Timing of document loaded and last click.
   base::TimeTicks document_loaded_timing_;
   base::TimeTicks last_click_timing_;

chrome/browser/navigation_predictor/navigation_predictor_unittest.cc

@@ -5,8 +5,10 @@
 #include "chrome/browser/navigation_predictor/navigation_predictor.h"
 
 #include <map>
+#include <string>
 
 #include "base/run_loop.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/test/metrics/histogram_tester.h"
 #include "base/test/scoped_feature_list.h"
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
@@ -64,6 +66,7 @@ class NavigationPredictorTest : public ChromeRenderViewHostTestHarness {
   ~NavigationPredictorTest() override = default;
 
   void SetUp() override {
+    SetupFieldTrial(base::nullopt /* prefetch_url_score_threshold */);
     ChromeRenderViewHostTestHarness::SetUp();
     predictor_service_helper_ = std::make_unique<TestNavigationPredictor>(
         mojo::MakeRequest(&predictor_service_), main_rfh());
@@ -93,9 +96,25 @@ class NavigationPredictorTest : public ChromeRenderViewHostTestHarness {
     return predictor_service_helper_->prefetch_url();
   }
 
- private:
+ protected:
+  void SetupFieldTrial(base::Optional<int> prefetch_url_score_threshold) {
+    const std::string kTrialName = "TrialFoo2";
+    const std::string kGroupName = "GroupFoo2";  // Value not used
+
+    std::map<std::string, std::string> params;
+    if (prefetch_url_score_threshold.has_value()) {
+      params["prefetch_url_score_threshold"] =
+          base::IntToString(prefetch_url_score_threshold.value());
+    }
+    scoped_feature_list.InitAndEnableFeatureWithParameters(
+        blink::features::kRecordAnchorMetricsVisible, params);
+  }
+
   blink::mojom::AnchorElementMetricsHostPtr predictor_service_;
   std::unique_ptr<TestNavigationPredictor> predictor_service_helper_;
+
+ private:
+  base::test::ScopedFeatureList scoped_feature_list;
 };
 
 }  // namespace
@@ -265,16 +284,17 @@ TEST_F(NavigationPredictorTest, ActionTaken_NoSameHost_Prefetch) {
   EXPECT_FALSE(prefetch_url().has_value());
 }
 
+// URL with highest prefetch score is from the same origin. Prefetch is done.
 TEST_F(NavigationPredictorTest, ActionTaken_SameOrigin_Prefetch) {
   const std::string source = "https://example.com";
   const std::string same_origin_href_small = "https://example.com/small";
   const std::string same_origin_href_large = "https://example.com/large";
-  const std::string diff_origin_href_xlarge = "https://example2.com/xlarge";
+  const std::string diff_origin_href_xsmall = "https://example2.com/xsmall";
 
   std::vector<blink::mojom::AnchorElementMetricsPtr> metrics;
   metrics.push_back(CreateMetricsPtr(source, same_origin_href_large, 1));
   metrics.push_back(CreateMetricsPtr(source, same_origin_href_small, 0.01));
-  metrics.push_back(CreateMetricsPtr(source, diff_origin_href_xlarge, 1));
+  metrics.push_back(CreateMetricsPtr(source, diff_origin_href_xsmall, 0.01));
 
   base::HistogramTester histogram_tester;
   predictor_service()->ReportAnchorElementMetricsOnLoad(std::move(metrics));
@@ -297,6 +317,38 @@ TEST_F(NavigationPredictorTest, ActionTaken_SameOrigin_Prefetch) {
       NavigationPredictor::ActionAccuracy::kPrefetchActionClickToSameOrigin, 1);
 }
 
+// URL with highest prefetch score is from a different origin. So, prefetch is
+// not done.
+TEST_F(NavigationPredictorTest, ActionTaken_SameOrigin_Prefetch_NotSameOrigin) {
+  const std::string source = "https://example.com";
+  const std::string same_origin_href_small = "https://example.com/small";
+  const std::string same_origin_href_large = "https://example.com/large";
+  const std::string diff_origin_href_xlarge = "https://example2.com/xlarge";
+
+  std::vector<blink::mojom::AnchorElementMetricsPtr> metrics;
+  metrics.push_back(CreateMetricsPtr(source, same_origin_href_large, 1));
+  metrics.push_back(CreateMetricsPtr(source, same_origin_href_small, 0.01));
+  metrics.push_back(CreateMetricsPtr(source, diff_origin_href_xlarge, 10));
+
+  base::HistogramTester histogram_tester;
+  predictor_service()->ReportAnchorElementMetricsOnLoad(std::move(metrics));
+  base::RunLoop().RunUntilIdle();
+
+  histogram_tester.ExpectUniqueSample(
+      "NavigationPredictor.OnNonDSE.ActionTaken",
+      NavigationPredictor::Action::kNone, 1);
+  EXPECT_FALSE(prefetch_url().has_value());
+
+  auto metrics_clicked = CreateMetricsPtr(source, same_origin_href_small, 0.01);
+  predictor_service()->ReportAnchorElementMetricsOnClick(
+      std::move(metrics_clicked));
+  base::RunLoop().RunUntilIdle();
+
+  histogram_tester.ExpectUniqueSample(
+      "NavigationPredictor.OnNonDSE.AccuracyActionTaken",
+      NavigationPredictor::ActionAccuracy::kNoActionTakenClickHappened, 1);
+}
+
 TEST_F(NavigationPredictorTest,
        ActionTaken_SameOrigin_DifferentScheme_Prefetch) {
   const std::string source = "https://example.com";
@@ -316,3 +368,49 @@ TEST_F(NavigationPredictorTest,
       NavigationPredictor::Action::kNone, 1);
   EXPECT_FALSE(prefetch_url().has_value());
 }
+
+// Framework for testing cases where prefetch is effectively
+// disabled by setting |prefetch_url_score_threshold| to too high.
+class NavigationPredictorPrefetchDisabledTest : public NavigationPredictorTest {
+ public:
+  void SetUp() override {
+    SetupFieldTrial(101 /* prefetch_url_score_threshold */);
+
+    ChromeRenderViewHostTestHarness::SetUp();
+    predictor_service_helper_ = std::make_unique<TestNavigationPredictor>(
+        mojo::MakeRequest(&predictor_service_), main_rfh());
+  }
+};
+
+TEST_F(NavigationPredictorPrefetchDisabledTest,
+       ActionTaken_SameOrigin_Prefetch_BelowThreshold) {
+  const std::string source = "https://example.com";
+  const std::string same_origin_href_small = "https://example.com/small";
+  const std::string same_origin_href_large = "https://example.com/large";
+  const std::string diff_origin_href_xsmall = "https://example2.com/xsmall";
+
+  std::vector<blink::mojom::AnchorElementMetricsPtr> metrics;
+  metrics.push_back(CreateMetricsPtr(source, same_origin_href_large, 1));
+  metrics.push_back(CreateMetricsPtr(source, same_origin_href_small, 0.01));
+  metrics.push_back(CreateMetricsPtr(source, diff_origin_href_xsmall, 0.0001));
+
+  base::HistogramTester histogram_tester;
+  predictor_service()->ReportAnchorElementMetricsOnLoad(std::move(metrics));
+  base::RunLoop().RunUntilIdle();
+
+  histogram_tester.ExpectUniqueSample(
+      "NavigationPredictor.OnNonDSE.ActionTaken",
+      NavigationPredictor::Action::kNone, 1);
+  EXPECT_FALSE(prefetch_url().has_value());
+
+  auto metrics_clicked = CreateMetricsPtr(source, same_origin_href_small, 0.01);
+  predictor_service()->ReportAnchorElementMetricsOnClick(
+      std::move(metrics_clicked));
+  base::RunLoop().RunUntilIdle();
+
+  histogram_tester.ExpectTotalCount(
+      "AnchorElementMetrics.Clicked.HrefEngagementScore2", 1);
+  histogram_tester.ExpectUniqueSample(
+      "NavigationPredictor.OnNonDSE.AccuracyActionTaken",
+      NavigationPredictor::ActionAccuracy::kNoActionTakenClickHappened, 1);
+}