Correctly record if clicked anchor element contains image or not

Currently, when an anchor element is clicked, we record if it
contains image or not based on only the properties of the clicked element.

This CL changes the logic to also look at other anchor elements
in the page that point to the same HREF. If any of
the other anchor element in the page point to the same
href as the clicked href, and contains image,
then we record that the clicked anchor element also contains image.

Bug: 871521
Change-Id: Ia21133e023596128e62621551931ad054fda98e3
Reviewed-on: https://chromium-review.googlesource.com/c/1278468Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599836}

chrome/browser/navigation_predictor/navigation_predictor.cc

@@ -18,8 +18,14 @@
 #include "url/gurl.h"
 
 struct NavigationPredictor::NavigationScore {
-  NavigationScore(const GURL& url, size_t area_rank, double score)
-      : url(url), area_rank(area_rank), score(score) {}
+  NavigationScore(const GURL& url,
+                  size_t area_rank,
+                  double score,
+                  bool contains_image)
+      : url(url),
+        area_rank(area_rank),
+        score(score),
+        contains_image(contains_image) {}
   // URL of the target link.
   const GURL url;
 
@@ -30,6 +36,11 @@ struct NavigationPredictor::NavigationScore {
   // Calculated navigation score, based on |area_rank| and other metrics.
   const double score;
 
+  // Multiple anchor elements may point to the same |url|. |contains_image| is
+  // true if at least one of the anchor elements pointing to |url| contains an
+  // image.
+  const bool contains_image;
+
   // Rank of the |score| in this document. It starts at 0, a lower rank implies
   // a higher |score|.
   base::Optional<size_t> score_rank;
@@ -184,7 +195,9 @@ void NavigationPredictor::ReportAnchorElementMetricsOnClick(
         (number_of_anchors_same_host_ * 100) / number_of_anchors);
   }
 
-  if (metrics->contains_image) {
+  // Check if the clicked anchor element contains image or if any other anchor
+  // element pointing to the same url contains an image.
+  if (metrics->contains_image || iter->second->contains_image) {
     UMA_HISTOGRAM_PERCENTAGE(
         "AnchorElementMetrics.Clicked.RatioContainsImage_ContainsImage",
         (number_of_anchors_contains_image_ * 100) / number_of_anchors);
@@ -365,7 +378,7 @@ void NavigationPredictor::ReportAnchorElementMetricsOnLoad(
         metrics.size());
 
     navigation_scores.push_back(std::make_unique<NavigationScore>(
-        metric->target_url, area_rank, score));
+        metric->target_url, area_rank, score, metric->contains_image));
   }
 
   // Sort scores by the calculated navigation score in descending order. This

chrome/browser/navigation_predictor/navigation_predictor_browsertest.cc

@@ -146,7 +146,7 @@ IN_PROC_BROWSER_TEST_P(NavigationPredictorBrowserTest, AreaRank) {
 // Test that MergeMetricsSameTargetUrl merges anchor elements having the same
 // href. The html file contains two anchor elements having the same href.
 IN_PROC_BROWSER_TEST_P(NavigationPredictorBrowserTest,
-                       MergeMetricsSameTargetUrl) {
+                       MergeMetricsSameTargetUrl_ClickHrefWithNoMergedImage) {
   base::HistogramTester histogram_tester;
 
   const GURL& url = GetTestURL("/anchors_same_href.html");
@@ -161,4 +161,52 @@ IN_PROC_BROWSER_TEST_P(NavigationPredictorBrowserTest,
     histogram_tester.ExpectTotalCount("AnchorElementMetrics.Visible.RatioArea",
                                       0);
   }
+
+  EXPECT_TRUE(content::ExecuteScript(
+      browser()->tab_strip_model()->GetActiveWebContents(),
+      "document.getElementById('diffHref').click();"));
+  base::RunLoop().RunUntilIdle();
+
+  // Anchor element with id 'diffHref' points to an href. No image in the
+  // webpage also points to an image. So, clicking on this non-image anchor
+  // element, should not be recorded as "ContainsImage".
+  if (base::FeatureList::IsEnabled(
+          blink::features::kRecordAnchorMetricsVisible)) {
+    histogram_tester.ExpectTotalCount(
+        "AnchorElementMetrics.Clicked.RatioContainsImage_ContainsImage", 0);
+  }
+}
+
+// Test that MergeMetricsSameTargetUrl merges anchor elements having the same
+// href. The html file contains two anchor elements having the same href.
+IN_PROC_BROWSER_TEST_P(NavigationPredictorBrowserTest,
+                       MergeMetricsSameTargetUrl_ClickHrefWithMergedImage) {
+  base::HistogramTester histogram_tester;
+
+  const GURL& url = GetTestURL("/anchors_same_href.html");
+  ui_test_utils::NavigateToURL(browser(), url);
+  base::RunLoop().RunUntilIdle();
+
+  if (base::FeatureList::IsEnabled(
+          blink::features::kRecordAnchorMetricsVisible)) {
+    histogram_tester.ExpectTotalCount("AnchorElementMetrics.Visible.RatioArea",
+                                      1);
+  } else {
+    histogram_tester.ExpectTotalCount("AnchorElementMetrics.Visible.RatioArea",
+                                      0);
+  }
+
+  EXPECT_TRUE(content::ExecuteScript(
+      browser()->tab_strip_model()->GetActiveWebContents(),
+      "document.getElementById('google').click();"));
+  base::RunLoop().RunUntilIdle();
+
+  // Anchor element with id 'google' points to an href. Another image in the
+  // webpage also points to an image. So, even though we clicked on a non-image
+  // anchor element, it should be recorded as "ContainsImage".
+  if (base::FeatureList::IsEnabled(
+          blink::features::kRecordAnchorMetricsVisible)) {
+    histogram_tester.ExpectTotalCount(
+        "AnchorElementMetrics.Clicked.RatioContainsImage_ContainsImage", 1);
+  }
 }

chrome/test/data/navigation_predictor/anchors_same_href.html

@@ -4,5 +4,7 @@
   <body>
     <a id="google" href="https://google.com">Google</a>
     <a id="google2" href="https://google.com">Google2</a>
+    <a id="imageSameHref" href="https://google.com"><img height="1" width="1"></a>
+    <a id="diffHref" href="https://example.com"></a>
   </body>
 </html>
\ No newline at end of file