Revert "Disable AdTagging when the Ads content setting is set to allow"

This reverts commit 19e537d0.

Reason for revert: Suspected cause for CreativeOriginAdsPageLoadMetricsObserverBrowserTest.CreativeOriginStatusWithThrottlingUnknown deterministically failing. on Win7, first failure: https://ci.chromium.org/p/chromium/builders/ci/Win7%20%2832%29%20Tests/61594

Original change's description:
> 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/+/2218371
> Reviewed-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}

TBR=jkarlin@chromium.org,csharrison@chromium.org,johnidel@chromium.org

Change-Id: I3f150050d45b258b664178c501254c8fc42315a4
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2269397Reviewed-by: Meredith Lane <meredithl@chromium.org>
Commit-Queue: Meredith Lane <meredithl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#782817}

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

@@ -423,13 +423,6 @@ 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.
@@ -1246,40 +1239,9 @@ 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,16 +8,12 @@
 #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"
@@ -331,32 +327,6 @@ 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,26 +259,15 @@ void ContentSubresourceFilterThrottleManager::OnPageActivationComputed(
     const mojom::ActivationState& activation_state) {
   DCHECK(navigation_handle->IsInMainFrame());
   DCHECK(!navigation_handle->HasCommitted());
-
-  auto it = ongoing_activation_throttles_.find(navigation_handle);
-  if (it == ongoing_activation_throttles_.end())
+  // Do not notify the throttle if activation is disabled.
+  if (activation_state.activation_level == mojom::ActivationLevel::kDisabled)
     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;
+  auto it = ongoing_activation_throttles_.find(navigation_handle);
+  if (it != ongoing_activation_throttles_.end()) {
+    it->second->NotifyPageActivationWithRuleset(EnsureRulesetHandle(),
+                                                activation_state);
   }
-
-  it->second->NotifyPageActivationWithRuleset(EnsureRulesetHandle(),
-                                              activation_state);
 }
 
 void ContentSubresourceFilterThrottleManager::OnSubframeNavigationEvaluated(