NavigationPredictor: Skip navigation prediction on insecure webpages

Change-Id: Ib7a70bf6d4921aafdd3786f7402f79547cee70de
Bug: 903978
TBR: ryansturm@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/c/1330338Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Doug Arnett <dougarnett@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607416}

chrome/browser/navigation_predictor/navigation_predictor.cc

@@ -165,6 +165,10 @@ void NavigationPredictor::ReportAnchorElementMetricsOnClick(
       GetTemplateURLService() &&
       GetTemplateURLService()->IsSearchResultsPageFromDefaultSearchProvider(
           metrics->source_url);
+  if (!metrics->source_url.SchemeIsCryptographic() ||
+      !metrics->target_url.SchemeIsCryptographic()) {
+    return;
+  }
 
   RecordTimingOnClick();
 
@@ -273,6 +277,9 @@ void NavigationPredictor::MergeMetricsSameTargetUrl(
     if (metric->target_url == metric->source_url)
       continue;
 
+    if (!metric->target_url.SchemeIsCryptographic())
+      continue;
+
     // Currently, all predictions are made based on elements that are within the
     // main frame since it is unclear if we can pre* the target of the elements
     // within iframes.
@@ -362,6 +369,9 @@ void NavigationPredictor::ReportAnchorElementMetricsOnLoad(
     }
   }
 
+  if (!metrics[0]->source_url.SchemeIsCryptographic())
+    return;
+
   document_loaded_timing_ = base::TimeTicks::Now();
 
   source_is_default_search_engine_page_ =

chrome/browser/navigation_predictor/navigation_predictor_browsertest.cc

@@ -62,9 +62,18 @@ class NavigationPredictorBrowserTest
   }
 
   void SetUp() override {
-    test_server_.ServeFilesFromSourceDirectory(
+    https_server_.reset(
+        new net::EmbeddedTestServer(net::EmbeddedTestServer::TYPE_HTTPS));
+    https_server_->ServeFilesFromSourceDirectory(
         "chrome/test/data/navigation_predictor");
-    ASSERT_TRUE(test_server_.Start());
+    ASSERT_TRUE(https_server_->Start());
+
+    http_server_.reset(
+        new net::EmbeddedTestServer(net::EmbeddedTestServer::TYPE_HTTP));
+    http_server_->ServeFilesFromSourceDirectory(
+        "chrome/test/data/navigation_predictor");
+    ASSERT_TRUE(http_server_->Start());
+
     subresource_filter::SubresourceFilterBrowserTest::SetUp();
   }
 
@@ -74,11 +83,16 @@ class NavigationPredictorBrowserTest
   }
 
   const GURL GetTestURL(const char* file) const {
-    return test_server_.GetURL(file);
+    return https_server_->GetURL(file);
+  }
+
+  const GURL GetHttpTestURL(const char* file) const {
+    return http_server_->GetURL(file);
   }
 
  private:
-  net::EmbeddedTestServer test_server_;
+  std::unique_ptr<net::EmbeddedTestServer> http_server_;
+  std::unique_ptr<net::EmbeddedTestServer> https_server_;
   base::test::ScopedFeatureList feature_list_;
 
   DISALLOW_COPY_AND_ASSIGN(NavigationPredictorBrowserTest);
@@ -127,6 +141,21 @@ IN_PROC_BROWSER_TEST_P(NavigationPredictorBrowserTest, PipelineOffTheRecord) {
       "AnchorElementMetrics.Visible.NumberOfAnchorElementsAfterMerge", 0);
 }
 
+// Test that the browser does not process anchor element metrics from an http
+// web page on page load.
+IN_PROC_BROWSER_TEST_P(NavigationPredictorBrowserTest, PipelineHttp) {
+  base::HistogramTester histogram_tester;
+
+  const GURL& url = GetHttpTestURL("/simple_page_with_anchors.html");
+  ui_test_utils::NavigateToURL(browser(), url);
+  base::RunLoop().RunUntilIdle();
+
+  histogram_tester.ExpectTotalCount(
+      "AnchorElementMetrics.Visible.NumberOfAnchorElements", 0);
+  histogram_tester.ExpectTotalCount(
+      "AnchorElementMetrics.Visible.NumberOfAnchorElementsAfterMerge", 0);
+}
+
 // Test that anchor elements within an iframe tagged as an ad are discarded when
 // predicting next navigation.
 IN_PROC_BROWSER_TEST_P(NavigationPredictorBrowserTest, PipelineAdsFrameTagged) {

chrome/browser/navigation_predictor/navigation_predictor_unittest.cc

@@ -94,7 +94,7 @@ TEST_F(NavigationPredictorTest, ReportAnchorElementMetricsOnClick) {
   base::HistogramTester histogram_tester;
 
   auto metrics =
-      CreateMetricsPtr("http://example.com", "https://google.com", 0.1);
+      CreateMetricsPtr("https://example.com", "https://google.com", 0.1);
   predictor_service()->ReportAnchorElementMetricsOnClick(std::move(metrics));
   base::RunLoop().RunUntilIdle();
 
@@ -107,7 +107,7 @@ TEST_F(NavigationPredictorTest, ReportAnchorElementMetricsOnLoad) {
   base::HistogramTester histogram_tester;
 
   auto metrics =
-      CreateMetricsPtr("https://example.com", "http://google.com", 0.1);
+      CreateMetricsPtr("https://example.com", "https://google.com", 0.1);
   std::vector<blink::mojom::AnchorElementMetricsPtr> metrics_vector;
   metrics_vector.push_back(std::move(metrics));
   predictor_service()->ReportAnchorElementMetricsOnLoad(
@@ -120,7 +120,8 @@ TEST_F(NavigationPredictorTest, ReportAnchorElementMetricsOnLoad) {
 
 // Test that if source/target url is not http or https, no score will be
 // calculated.
-TEST_F(NavigationPredictorTest, BadUrlReportAnchorElementMetricsOnClick) {
+TEST_F(NavigationPredictorTest,
+       BadUrlReportAnchorElementMetricsOnClick_ftp_src) {
   base::HistogramTester histogram_tester;
 
   auto metrics =
@@ -134,7 +135,8 @@ TEST_F(NavigationPredictorTest, BadUrlReportAnchorElementMetricsOnClick) {
 
 // Test that if source/target url is not http or https, no navigation score will
 // be calculated.
-TEST_F(NavigationPredictorTest, BadUrlReportAnchorElementMetricsOnLoad) {
+TEST_F(NavigationPredictorTest,
+       BadUrlReportAnchorElementMetricsOnLoad_ftp_target) {
   base::HistogramTester histogram_tester;
 
   auto metrics =
@@ -149,38 +151,78 @@ TEST_F(NavigationPredictorTest, BadUrlReportAnchorElementMetricsOnLoad) {
       "AnchorElementMetrics.Visible.HighestNavigationScore", 0);
 }
 
+// Test that if the target url is not https, no navigation score will
+// be calculated.
+TEST_F(NavigationPredictorTest,
+       BadUrlReportAnchorElementMetricsOnLoad_http_target) {
+  base::HistogramTester histogram_tester;
+
+  auto metrics =
+      CreateMetricsPtr("https://example.com", "http://google.com", 0.1);
+  std::vector<blink::mojom::AnchorElementMetricsPtr> metrics_vector;
+  metrics_vector.push_back(std::move(metrics));
+  predictor_service()->ReportAnchorElementMetricsOnLoad(
+      std::move(metrics_vector));
+  base::RunLoop().RunUntilIdle();
+
+  histogram_tester.ExpectTotalCount(
+      "AnchorElementMetrics.Visible.HighestNavigationScore", 0);
+}
+
+// Test that if the source url is not https, no navigation score will
+// be calculated.
+TEST_F(NavigationPredictorTest,
+       BadUrlReportAnchorElementMetricsOnLoad_http_src) {
+  base::HistogramTester histogram_tester;
+
+  auto metrics =
+      CreateMetricsPtr("http://example.com", "https://google.com", 0.1);
+  std::vector<blink::mojom::AnchorElementMetricsPtr> metrics_vector;
+  metrics_vector.push_back(std::move(metrics));
+  predictor_service()->ReportAnchorElementMetricsOnLoad(
+      std::move(metrics_vector));
+  base::RunLoop().RunUntilIdle();
+
+  histogram_tester.ExpectTotalCount(
+      "AnchorElementMetrics.Visible.HighestNavigationScore", 0);
+}
+
 // In this test, multiple anchor element metrics are sent to
 // ReportAnchorElementMetricsOnLoad. Test that CalculateAnchorNavigationScore
 // works, and that highest navigation score can be recorded correctly.
 TEST_F(NavigationPredictorTest, MultipleAnchorElementMetricsOnLoad) {
   base::HistogramTester histogram_tester;
 
-  const std::string source = "http://example.com";
-  const std::string href_xlarge = "http://example.com/xlarge";
-  const std::string href_large = "http://google.com/large";
-  const std::string href_medium = "http://google.com/medium";
-  const std::string href_small = "http://google.com/small";
-  const std::string href_xsmall = "http://google.com/xsmall";
+  const std::string source = "https://example.com";
+  const std::string href_xlarge = "https://example.com/xlarge";
+  const std::string href_large = "https://google.com/large";
+  const std::string href_medium = "https://google.com/medium";
+  const std::string href_small = "https://google.com/small";
+  const std::string href_xsmall = "https://google.com/xsmall";
+  const std::string http_href_xsmall = "http://google.com/xsmall";
 
   std::vector<blink::mojom::AnchorElementMetricsPtr> metrics;
   metrics.push_back(CreateMetricsPtr(source, href_xsmall, 0.01));
+  metrics.push_back(CreateMetricsPtr(source, http_href_xsmall, 0.01));
   metrics.push_back(CreateMetricsPtr(source, href_large, 0.08));
   metrics.push_back(CreateMetricsPtr(source, href_xlarge, 0.1));
   metrics.push_back(CreateMetricsPtr(source, href_small, 0.02));
   metrics.push_back(CreateMetricsPtr(source, href_medium, 0.05));
 
-  int number_of_mertics_sent = metrics.size();
+  int number_of_metrics_sent = metrics.size();
   predictor_service()->ReportAnchorElementMetricsOnLoad(std::move(metrics));
   base::RunLoop().RunUntilIdle();
 
   const std::map<GURL, int>& area_rank_map =
       predictor_service_helper()->GetAreaRankMap();
-  EXPECT_EQ(number_of_mertics_sent, static_cast<int>(area_rank_map.size()));
+  // Exclude the http anchor element from |number_of_metrics_sent|.
+  EXPECT_EQ(number_of_metrics_sent - 1, static_cast<int>(area_rank_map.size()));
   EXPECT_EQ(0, area_rank_map.find(GURL(href_xlarge))->second);
   EXPECT_EQ(1, area_rank_map.find(GURL(href_large))->second);
   EXPECT_EQ(2, area_rank_map.find(GURL(href_medium))->second);
   EXPECT_EQ(3, area_rank_map.find(GURL(href_small))->second);
   EXPECT_EQ(4, area_rank_map.find(GURL(href_xsmall))->second);
+  EXPECT_EQ(area_rank_map.end(), area_rank_map.find(GURL(http_href_xsmall)));
 
   // The highest score is 100 (scale factor) * 0.1 (largest area) = 10.
   // After scaling the navigation score across all anchor elements, the score