Disable AdTagging when the Ads content setting is set to allow

What: Do not run the SF filter in DryRun mode on pages where the
content setting is set to allow.

How: Delete the ongoing activation throttle for the navigation when
we receive an explicit disabled activation state.

Currently we ignore disabled activation states when we receive them,
as normally the SF filter is running in DryRun mode anyway.


Change-Id: Ibc5202ec5f68bd284987cdd4d5dfa0e8de0708a4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2218371Reviewed-by: Josh Karlin <jkarlin@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Commit-Queue: John Delaney <johnidel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#782722}

chrome/browser/page_load_metrics/observers/ad_metrics/ads_page_load_metrics_observer_unittest.cc

@@ -423,6 +423,13 @@ class AdsPageLoadMetricsObserverTest
                 &AdsPageLoadMetricsObserverTest::RegisterObservers,
                 base::Unretained(this)));
     ConfigureAsSubresourceFilterOnlyURL(GURL(kAdUrl));
+
+    // Run all sites in dry run mode, so that AdTagging works as expected. In
+    // browser environments, all sites activate with dry run by default.
+    scoped_configuration().ResetConfiguration(subresource_filter::Configuration(
+        subresource_filter::mojom::ActivationLevel::kDryRun,
+        subresource_filter::ActivationScope::ALL_SITES,
+        subresource_filter::ActivationList::SUBRESOURCE_FILTER));
   }
 
   // Returns the final RenderFrameHost after navigation commits.
@@ -1239,9 +1246,40 @@ TEST_F(AdsPageLoadMetricsObserverTest, NoHistogramWithoutCommit) {
                     .size());
 }
 
+TEST_F(AdsPageLoadMetricsObserverTest,
+       SubresourceFilterDisabled_NoAdsDetected) {
+  // Setup the subresource filter as disabled on all sites.
+  scoped_configuration().ResetConfiguration(subresource_filter::Configuration(
+      subresource_filter::mojom::ActivationLevel::kDisabled,
+      subresource_filter::ActivationScope::ALL_SITES,
+      subresource_filter::ActivationList::SUBRESOURCE_FILTER));
+
+  RenderFrameHost* main_frame = NavigateMainFrame(kNonAdUrl);
+  RenderFrameHost* ad_frame = CreateAndNavigateSubFrame(kAdUrl, main_frame);
+  ResourceDataUpdate(main_frame, ResourceCached::kNotCached, 10);
+  ResourceDataUpdate(ad_frame, ResourceCached::kNotCached, 10);
+
+  // Navigate again to trigger histograms.
+  NavigateFrame(kNonAdUrl, main_frame);
+
+  TestHistograms(histogram_tester(), test_ukm_recorder(),
+                 std::vector<ExpectedFrameBytes>(), 0 /* non_ad_cached_kb */,
+                 20 /* non_ad_uncached_kb */);
+
+  // Verify that other UMA wasn't written.
+  histogram_tester().ExpectTotalCount(
+      "PageLoad.Clients.Ads.Bytes.AdFrames.Aggregate.Total", 0);
+}
+
 // Frames that are disallowed (and filtered) by the subresource filter should
 // not be counted.
 TEST_F(AdsPageLoadMetricsObserverTest, FilterAds_DoNotLogMetrics) {
+  // Setup the subresource filter in non-dryrun mode to trigger on a site.
+  scoped_configuration().ResetConfiguration(subresource_filter::Configuration(
+      subresource_filter::mojom::ActivationLevel::kEnabled,
+      subresource_filter::ActivationScope::ACTIVATION_LIST,
+      subresource_filter::ActivationList::SUBRESOURCE_FILTER));
+
   ConfigureAsSubresourceFilterOnlyURL(GURL(kNonAdUrl));
   NavigateMainFrame(kNonAdUrl);
 

chrome/browser/subresource_filter/ad_tagging_browsertest.cc

@@ -8,12 +8,16 @@
 #include "base/callback.h"
 #include "base/strings/stringprintf.h"
 #include "base/test/metrics/histogram_tester.h"
+#include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/page_load_metrics/observers/ad_metrics/ads_page_load_metrics_observer.h"
 #include "chrome/browser/page_load_metrics/observers/ad_metrics/frame_data.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/ui_test_utils.h"
+#include "components/content_settings/core/browser/host_content_settings_map.h"
+#include "components/content_settings/core/common/content_settings.h"
+#include "components/content_settings/core/common/content_settings_types.h"
 #include "components/embedder_support/switches.h"
 #include "components/metrics/content/subprocess_metrics_provider.h"
 #include "components/page_load_metrics/browser/page_load_metrics_test_waiter.h"
@@ -327,6 +331,32 @@ void AdTaggingBrowserTest::NavigateFrame(
       << navigation_observer.last_net_error_code();
 }
 
+IN_PROC_BROWSER_TEST_F(AdTaggingBrowserTest,
+                       AdContentSettingAllowed_AdTaggingDisabled) {
+  HostContentSettingsMapFactory::GetForProfile(
+      web_contents()->GetBrowserContext())
+      ->SetDefaultContentSetting(ContentSettingsType::ADS,
+                                 CONTENT_SETTING_ALLOW);
+
+  TestSubresourceFilterObserver observer(web_contents());
+  ui_test_utils::NavigateToURL(browser(), GetURL("frame_factory.html"));
+
+  // Create an ad frame.
+  RenderFrameHost* ad_frame =
+      CreateSrcFrame(GetWebContents(), GetURL("frame_factory.html?2&ad=true"));
+
+  // Verify that we are not evaluating subframe navigations.
+  EXPECT_FALSE(observer.GetIsAdSubframe(ad_frame->GetFrameTreeNodeId()));
+
+  // Child frame created by ad script.
+  RenderFrameHost* ad_frame_tagged_by_script = CreateSrcFrameFromAdScript(
+      GetWebContents(), GetURL("frame_factory.html?1"));
+
+  // No frames should be detected by script heuristics.
+  EXPECT_FALSE(observer.GetIsAdSubframe(
+      ad_frame_tagged_by_script->GetFrameTreeNodeId()));
+}
+
 IN_PROC_BROWSER_TEST_F(AdTaggingBrowserTest, FramesByURL) {
   TestSubresourceFilterObserver observer(web_contents());
 

components/subresource_filter/content/browser/content_subresource_filter_throttle_manager.cc

@@ -259,15 +259,26 @@ void ContentSubresourceFilterThrottleManager::OnPageActivationComputed(
     const mojom::ActivationState& activation_state) {
   DCHECK(navigation_handle->IsInMainFrame());
   DCHECK(!navigation_handle->HasCommitted());
-  // Do not notify the throttle if activation is disabled.
-  if (activation_state.activation_level == mojom::ActivationLevel::kDisabled)
-    return;
 
   auto it = ongoing_activation_throttles_.find(navigation_handle);
-  if (it != ongoing_activation_throttles_.end()) {
-    it->second->NotifyPageActivationWithRuleset(EnsureRulesetHandle(),
-                                                activation_state);
+  if (it == ongoing_activation_throttles_.end())
+    return;
+
+  // The subresource filter normally operates in DryRun mode, disabled
+  // activation should only be supplied in cases where DryRun mode is not
+  // otherwise preferable. If the activation level is disabled, we do not want
+  // to run any portion of the subresource filter on this navigation/frame. By
+  // deleting the activation throttle, we prevent an associated
+  // DocumentSubresourceFilter from being created at commit time. This
+  // intentionally disables AdTagging and all dependent features for this
+  // navigation/frame.
+  if (activation_state.activation_level == mojom::ActivationLevel::kDisabled) {
+    ongoing_activation_throttles_.erase(it);
+    return;
   }
+
+  it->second->NotifyPageActivationWithRuleset(EnsureRulesetHandle(),
+                                              activation_state);
 }
 
 void ContentSubresourceFilterThrottleManager::OnSubframeNavigationEvaluated(