NavigationPredictor: Skip anchor elements that belong to an ad frame

At OnLoad event, the renderer notifies browser process of the
anchor elements in the viewport. The browser uses this information
to predict the navigation probability of each of these anchor
elements.

This CL changes the logic in the render side to skip reporting
of elements that are part of ad frames since we expect these
to be clicked less often.

Bug: 899339
Change-Id: I171cce5d4b9f722039222865753f3b7cbe6d8a79
TBR: holte@chromium.org,ryansturm@chromium.org,kinuko@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/c/1315529Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605013}

chrome/browser/navigation_predictor/navigation_predictor_browsertest.cc

@@ -6,20 +6,47 @@
 #include "base/run_loop.h"
 #include "base/test/metrics/histogram_tester.h"
 #include "base/test/scoped_feature_list.h"
+#include "chrome/browser/metrics/subprocess_metrics_provider.h"
+#include "chrome/browser/subresource_filter/subresource_filter_browser_test_harness.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/public/test/browser_test_utils.h"
+#include "net/dns/mock_host_resolver.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
 #include "third_party/blink/public/common/features.h"
 #include "url/gurl.h"
 
+namespace {
+
+// Retries fetching |histogram_name| until it contains at least |count| samples.
+void RetryForHistogramUntilCountReached(base::HistogramTester* histogram_tester,
+                                        const std::string& histogram_name,
+                                        size_t count) {
+  base::RunLoop().RunUntilIdle();
+  for (size_t attempt = 0; attempt < 3; ++attempt) {
+    const std::vector<base::Bucket> buckets =
+        histogram_tester->GetAllSamples(histogram_name);
+    size_t total_count = 0;
+    for (const auto& bucket : buckets)
+      total_count += bucket.count;
+    if (total_count >= count)
+      return;
+    content::FetchHistogramsFromChildProcesses();
+    SubprocessMetricsProvider::MergeHistogramDeltasForTesting();
+    base::RunLoop().RunUntilIdle();
+  }
+}
+
+}  // namespace
+
 class NavigationPredictorBrowserTest
-    : public InProcessBrowserTest,
+    : public subresource_filter::SubresourceFilterBrowserTest,
       public testing::WithParamInterface<bool> {
  public:
-  NavigationPredictorBrowserTest() {
+  NavigationPredictorBrowserTest()
+      : subresource_filter::SubresourceFilterBrowserTest() {
     const std::vector<base::Feature> features = {
         blink::features::kRecordAnchorMetricsVisible,
         blink::features::kRecordAnchorMetricsClicked};
@@ -34,7 +61,12 @@ class NavigationPredictorBrowserTest
     test_server_.ServeFilesFromSourceDirectory(
         "chrome/test/data/navigation_predictor");
     ASSERT_TRUE(test_server_.Start());
-    InProcessBrowserTest::SetUp();
+    subresource_filter::SubresourceFilterBrowserTest::SetUp();
+  }
+
+  void SetUpOnMainThread() override {
+    subresource_filter::SubresourceFilterBrowserTest::SetUpOnMainThread();
+    host_resolver()->ClearRules();
   }
 
   const GURL GetTestURL(const char* file) const {
@@ -76,6 +108,68 @@ IN_PROC_BROWSER_TEST_P(NavigationPredictorBrowserTest, Pipeline) {
   }
 }
 
+// Test that anchor elements within an iframe tagged as an ad are discarded when
+// predicting next navigation.
+IN_PROC_BROWSER_TEST_P(NavigationPredictorBrowserTest, PipelineAdsFrameTagged) {
+  // iframe_ads_simple_page_with_anchors.html is an iframe referenced by
+  // page_with_ads_iframe.html.
+  ASSERT_NO_FATAL_FAILURE(SetRulesetToDisallowURLsWithPathSuffix(
+      "iframe_ads_simple_page_with_anchors.html"));
+
+  base::HistogramTester histogram_tester;
+
+  GURL url = GetTestURL("/page_with_ads_iframe.html");
+  ui_test_utils::NavigateToURL(browser(), url);
+  base::RunLoop().RunUntilIdle();
+
+  if (base::FeatureList::IsEnabled(
+          blink::features::kRecordAnchorMetricsVisible)) {
+    histogram_tester.ExpectUniqueSample(
+        "AnchorElementMetrics.Visible.NumberOfAnchorElements", 2, 1);
+
+    RetryForHistogramUntilCountReached(
+        &histogram_tester, "AnchorElementMetrics.IsAdFrameElement", 4);
+
+    histogram_tester.ExpectTotalCount("AnchorElementMetrics.IsAdFrameElement",
+                                      4);
+    histogram_tester.ExpectBucketCount("AnchorElementMetrics.IsAdFrameElement",
+                                       0 /* false */, 2);
+    histogram_tester.ExpectBucketCount("AnchorElementMetrics.IsAdFrameElement",
+                                       1 /* true */, 2);
+
+  } else {
+    histogram_tester.ExpectTotalCount(
+        "AnchorElementMetrics.Visible.NumberOfAnchorElements", 0);
+  }
+}
+
+// Test that anchor elements within an iframe not tagged as ad are not discarded
+// when predicting next navigation.
+IN_PROC_BROWSER_TEST_P(NavigationPredictorBrowserTest,
+                       PipelineAdsFrameNotTagged) {
+  base::HistogramTester histogram_tester;
+
+  GURL url = GetTestURL("/page_with_ads_iframe.html");
+  ui_test_utils::NavigateToURL(browser(), url);
+  base::RunLoop().RunUntilIdle();
+
+  if (base::FeatureList::IsEnabled(
+          blink::features::kRecordAnchorMetricsVisible)) {
+    histogram_tester.ExpectUniqueSample(
+        "AnchorElementMetrics.Visible.NumberOfAnchorElements", 4, 1);
+
+    RetryForHistogramUntilCountReached(
+        &histogram_tester, "AnchorElementMetrics.IsAdFrameElement", 4);
+
+    histogram_tester.ExpectUniqueSample("AnchorElementMetrics.IsAdFrameElement",
+                                        0 /* false */, 4);
+
+  } else {
+    histogram_tester.ExpectTotalCount(
+        "AnchorElementMetrics.Visible.NumberOfAnchorElements", 0);
+  }
+}
+
 // Test that navigation score of anchor elements can be calculated on page load.
 IN_PROC_BROWSER_TEST_P(NavigationPredictorBrowserTest, NavigationScore) {
   base::HistogramTester histogram_tester;

chrome/test/data/navigation_predictor/iframe_ads_simple_page_with_anchors.html

+<html>
+  <head>
+  </head>
+  <body>
+    <a id="google" href="https://google2.com">Google</a>
+    <a id="example_ad" href="https://example.com/included_script.js">Example</a>
+  </body>
+</html>
\ No newline at end of file

chrome/test/data/navigation_predictor/page_with_ads_iframe.html

+<html>
+  <head>
+  </head>
+  <body>
+    <a id="google" href="https://google.com">Google</a>
+    <a id="example" href="https://example.com">Example</a>
+    <iframe src="iframe_ads_simple_page_with_anchors.html"></iframe>
+  </body>
+</html>
\ No newline at end of file

third_party/blink/renderer/core/html/anchor_element_metrics_sender.cc

@@ -4,6 +4,7 @@
 
 #include "third_party/blink/renderer/core/html/anchor_element_metrics_sender.h"
 
+#include "base/metrics/histogram_macros.h"
 #include "services/service_manager/public/cpp/interface_provider.h"
 #include "third_party/blink/public/common/features.h"
 #include "third_party/blink/renderer/core/dom/document.h"
@@ -12,6 +13,20 @@
 
 namespace blink {
 
+namespace {
+
+// Returns true if |anchor_element| should be discarded, and not used for
+// navigation prediction.
+bool ShouldDiscardAnchorElement(const HTMLAnchorElement& anchor_element) {
+  Frame* frame = anchor_element.GetDocument().GetFrame();
+  if (!frame || !frame->IsLocalFrame())
+    return true;
+  LocalFrame* local_frame = ToLocalFrame(frame);
+  return local_frame->IsAdSubframe();
+}
+
+}  // namespace
+
 // static
 const char AnchorElementMetricsSender::kSupplementName[] =
     "DocumentAnchorElementMetricsSender";
@@ -64,6 +79,15 @@ void AnchorElementMetricsSender::SendAnchorMetricsVectorToBrowser(
 void AnchorElementMetricsSender::AddAnchorElement(HTMLAnchorElement& element) {
   if (has_onload_report_sent_)
     return;
+
+  bool is_ad_frame_element = ShouldDiscardAnchorElement(element);
+  UMA_HISTOGRAM_BOOLEAN("AnchorElementMetrics.IsAdFrameElement",
+                        is_ad_frame_element);
+
+  // We ignore anchor elements that are in ad frames.
+  if (is_ad_frame_element)
+    return;
+
   anchor_elements_.insert(&element);
 }
 

tools/metrics/histograms/histograms.xml

@@ -983,6 +983,13 @@ uploading your change for review.
   </summary>
 </histogram>
 
+<histogram name="AnchorElementMetrics.IsAdFrameElement" units="Boolean">
+  <owner>tbansal@chromium.org</owner>
+  <summary>
+    True if the anchor element was inside an iframe tagged as an ad iframe.
+  </summary>
+</histogram>
+
 <histogram base="true" name="AnchorElementMetrics.IsInIFrame" enum="Boolean">
   <owner>chelu@chromium.org</owner>
   <owner>tbansal@chromium.org</owner>