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.

Change-Id: Ia131ac301acda317e7618ac206f4bda60c39e5cf
Bug: 899339
Reviewed-on: https://chromium-review.googlesource.com/c/1300682Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604738}

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,18 @@
 
 namespace blink {
 
+namespace {
+
+// Whether the element is inside an adframe.
+bool IsInAdFrame(const HTMLAnchorElement& anchor_element) {
+  Frame* frame = anchor_element.GetDocument().GetFrame();
+  DCHECK(frame->IsLocalFrame());
+  LocalFrame* local_frame = ToLocalFrame(frame);
+  return local_frame->IsAdSubframe();
+}
+
+}  // namespace
+
 // static
 const char AnchorElementMetricsSender::kSupplementName[] =
     "DocumentAnchorElementMetricsSender";
@@ -64,6 +77,15 @@ void AnchorElementMetricsSender::SendAnchorMetricsVectorToBrowser(
 void AnchorElementMetricsSender::AddAnchorElement(HTMLAnchorElement& element) {
   if (has_onload_report_sent_)
     return;
+
+  bool is_ad_frame_element = IsInAdFrame(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>