Add field trial config to disable specific heavy ad thresholds

What: Add a feaature param to selectively disable the network or CPU
limits.

Why: We may want to run the intervention in different modes to
determine impact.

How: Add a check after an ad has determined to be heavy that filters
based on the feature param.

This change also cleans up some logic by returning an enum instead
of bool in MaybeTriggerHeavyAdIntervention(), and using the new
HeavyAdAction value to control triggering of interventions.

This also removes a mode where the intervention could be enabled
without reporting, which never needs to be used.

Change-Id: Ie7aefdd77edfee5b2d4463aab3e1d1790a0931b2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2363190
Commit-Queue: John Delaney <johnidel@chromium.org>
Reviewed-by: Eric Robinson <ericrobinson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800396}

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

@@ -11,6 +11,7 @@
 
 #include "base/check_op.h"
 #include "base/feature_list.h"
+#include "base/metrics/field_trial_params.h"
 #include "base/notreached.h"
 #include "base/rand_util.h"
 #include "base/strings/strcat.h"
@@ -1156,7 +1157,9 @@ void AdsPageLoadMetricsObserver::MaybeTriggerHeavyAdIntervention(
     content::RenderFrameHost* render_frame_host,
     FrameData* frame_data) {
   DCHECK(render_frame_host);
-  if (!frame_data->MaybeTriggerHeavyAdIntervention())
+  FrameData::HeavyAdAction action =
+      frame_data->MaybeTriggerHeavyAdIntervention();
+  if (action == FrameData::HeavyAdAction::kNone)
     return;
 
   // Don't trigger the heavy ad intervention on reloads. Gate this behind the
@@ -1164,13 +1167,19 @@ void AdsPageLoadMetricsObserver::MaybeTriggerHeavyAdIntervention(
   // trigger new navigations to the site to test it).
   if (heavy_ad_privacy_mitigations_enabled_) {
     UMA_HISTOGRAM_BOOLEAN(kIgnoredByReloadHistogramName, page_load_is_reload_);
-    if (page_load_is_reload_)
+    // Skip firing the intervention, but mark that an action occurred on the
+    // frame.
+    if (page_load_is_reload_) {
+      frame_data->set_heavy_ad_action(FrameData::HeavyAdAction::kIgnored);
       return;
+    }
   }
 
   // Check to see if we are allowed to activate on this host.
-  if (IsBlocklisted())
+  if (IsBlocklisted()) {
+    frame_data->set_heavy_ad_action(FrameData::HeavyAdAction::kIgnored);
     return;
+  }
 
   // We should always unload the root of the ad subtree. Find the
   // RenderFrameHost of the root ad frame associated with |frame_data|.
@@ -1186,40 +1195,35 @@ void AdsPageLoadMetricsObserver::MaybeTriggerHeavyAdIntervention(
                                   frame_data->root_frame_tree_node_id()) {
     render_frame_host = render_frame_host->GetParent();
   }
-  if (!render_frame_host)
+  if (!render_frame_host) {
+    frame_data->set_heavy_ad_action(FrameData::HeavyAdAction::kIgnored);
     return;
+  }
 
   // Ensure that this RenderFrameHost is a subframe.
   DCHECK(render_frame_host->GetParent());
 
-  // We already have a heavy ad at this point so we can query the field trial
-  // params safely.
-  bool will_report_adframe =
-      base::FeatureList::IsEnabled(features::kHeavyAdInterventionWarning);
-  bool will_unload_adframe =
-      base::FeatureList::IsEnabled(features::kHeavyAdIntervention);
-
-  if (will_report_adframe) {
-    // Add an inspector issue for the root of the ad subtree.
-    render_frame_host->ReportHeavyAdIssue(
-        will_unload_adframe
-            ? blink::mojom::HeavyAdResolutionStatus::kHeavyAdBlocked
-            : blink::mojom::HeavyAdResolutionStatus::kHeavyAdWarning,
-        GetHeavyAdReason(frame_data->heavy_ad_status_with_noise()));
-
-    // Report to all child frames that will be unloaded. Once all reports are
-    // queued, the frame will be unloaded. Because the IPC messages are ordered
-    // wrt to each frames unload, we do not need to wait before loading the
-    // error page. Reports will be added to ReportingObserver queues
-    // synchronously when the IPC message is handled, which guarantees they will
-    // be available in the the unload handler.
-    const char kReportId[] = "HeavyAdIntervention";
-    std::string report_message =
-        GetHeavyAdReportMessage(*frame_data, will_unload_adframe);
-    for (content::RenderFrameHost* reporting_frame :
-         render_frame_host->GetFramesInSubtree()) {
-      reporting_frame->SendInterventionReport(kReportId, report_message);
-    }
+  frame_data->set_heavy_ad_action(action);
+
+  // Add an inspector issue for the root of the ad subtree.
+  render_frame_host->ReportHeavyAdIssue(
+      action == FrameData::HeavyAdAction::kUnload
+          ? blink::mojom::HeavyAdResolutionStatus::kHeavyAdBlocked
+          : blink::mojom::HeavyAdResolutionStatus::kHeavyAdWarning,
+      GetHeavyAdReason(frame_data->heavy_ad_status_with_policy()));
+
+  // Report to all child frames that will be unloaded. Once all reports are
+  // queued, the frame will be unloaded. Because the IPC messages are ordered
+  // wrt to each frames unload, we do not need to wait before loading the
+  // error page. Reports will be added to ReportingObserver queues
+  // synchronously when the IPC message is handled, which guarantees they will
+  // be available in the the unload handler.
+  const char kReportId[] = "HeavyAdIntervention";
+  std::string report_message = GetHeavyAdReportMessage(
+      *frame_data, action == FrameData::HeavyAdAction::kUnload);
+  for (content::RenderFrameHost* reporting_frame :
+       render_frame_host->GetFramesInSubtree()) {
+    reporting_frame->SendInterventionReport(kReportId, report_message);
   }
 
   // Report intervention to the blocklist.
@@ -1237,12 +1241,12 @@ void AdsPageLoadMetricsObserver::MaybeTriggerHeavyAdIntervention(
 
   ADS_HISTOGRAM("HeavyAds.InterventionType2", UMA_HISTOGRAM_ENUMERATION,
                 FrameData::FrameVisibility::kAnyVisibility,
-                frame_data->heavy_ad_status_with_noise());
+                frame_data->heavy_ad_status_with_policy());
   ADS_HISTOGRAM("HeavyAds.InterventionType2", UMA_HISTOGRAM_ENUMERATION,
                 frame_data->visibility(),
-                frame_data->heavy_ad_status_with_noise());
+                frame_data->heavy_ad_status_with_policy());
 
-  if (!will_unload_adframe)
+  if (action != FrameData::HeavyAdAction::kUnload)
     return;
 
   // Record heavy ad network size only when an ad is unloaded as a result of

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

@@ -2389,6 +2389,69 @@ TEST_F(AdsPageLoadMetricsObserverTest,
       FrameData::HeavyAdStatus::kNone, 1);
 }
 
+// Tests that each configurable unload policy allows the intervention to trigger
+// on the correct frames.
+TEST_F(AdsPageLoadMetricsObserverTest, HeavyAdPolicyProvided) {
+  struct {
+    // |policy| maps to a FrameData::HeavyAdUnloadPolicy.
+    std::string policy;
+    bool exceed_network;
+    bool exceed_cpu;
+    bool intervention_expected;
+  } kTestCases[] = {
+      {"0" /* policy */, false /* exceed_network */, false /* exceed_cpu */,
+       false /* intervention_expected */},
+      {"0" /* policy */, true /* exceed_network */, false /* exceed_cpu */,
+       true /* intervention_expected */},
+      {"0" /* policy */, false /* exceed_network */, true /* exceed_cpu */,
+       false /* intervention_expected */},
+      {"0" /* policy */, true /* exceed_network */, true /* exceed_cpu */,
+       true /* intervention_expected */},
+      {"1" /* policy */, false /* exceed_network */, false /* exceed_cpu */,
+       false /* intervention_expected */},
+      {"1" /* policy */, true /* exceed_network */, false /* exceed_cpu */,
+       false /* intervention_expected */},
+      {"1" /* policy */, false /* exceed_network */, true /* exceed_cpu */,
+       true /* intervention_expected */},
+      {"1" /* policy */, true /* exceed_network */, true /* exceed_cpu */,
+       true /* intervention_expected */},
+      {"2" /* policy */, false /* exceed_network */, false /* exceed_cpu */,
+       false /* intervention_expected */},
+      {"2" /* policy */, true /* exceed_network */, false /* exceed_cpu */,
+       true /* intervention_expected */},
+      {"2" /* policy */, false /* exceed_network */, true /* exceed_cpu */,
+       true /* intervention_expected */},
+  };
+
+  for (const auto& test_case : kTestCases) {
+    base::test::ScopedFeatureList feature_list;
+    feature_list.InitAndEnableFeatureWithParameters(
+        features::kHeavyAdIntervention, {{"kUnloadPolicy", test_case.policy}});
+    RenderFrameHost* main_frame = NavigateMainFrame(kNonAdUrl);
+    RenderFrameHost* ad_frame = CreateAndNavigateSubFrame(kAdUrl, main_frame);
+
+    ErrorPageWaiter waiter(web_contents());
+    if (test_case.exceed_network) {
+      ResourceDataUpdate(ad_frame, ResourceCached::kNotCached,
+                         (heavy_ad_thresholds::kMaxNetworkBytes / 1024) + 1);
+    }
+    if (test_case.exceed_cpu) {
+      OnCpuTimingUpdate(ad_frame, base::TimeDelta::FromMilliseconds(
+                                      heavy_ad_thresholds::kMaxCpuTime + 1));
+    }
+
+    // We should either see an error page if the intervention happened, or not
+    // see any reports.
+    if (test_case.intervention_expected) {
+      waiter.WaitForError();
+    } else {
+      EXPECT_FALSE(HasInterventionReportsAfterFlush(ad_frame));
+    }
+
+    blocklist()->ClearBlockList(base::Time::Min(), base::Time::Max());
+  }
+}
+
 TEST_F(AdsPageLoadMetricsObserverTest,
        HeavyAdPageNavigated_FrameMarkedAsNotRemoved) {
   base::test::ScopedFeatureList feature_list;
@@ -2667,25 +2730,6 @@ TEST_F(AdsPageLoadMetricsObserverTest,
             1);
 }
 
-TEST_F(AdsPageLoadMetricsObserverTest, HeavyAdReportingDisabled_NoReportSent) {
-  base::test::ScopedFeatureList feature_list;
-  feature_list.InitWithFeatures({features::kHeavyAdIntervention},
-                                {features::kHeavyAdInterventionWarning});
-
-  RenderFrameHost* main_frame = NavigateMainFrame(kNonAdUrl);
-  RenderFrameHost* ad_frame = CreateAndNavigateSubFrame(kAdUrl, main_frame);
-
-  ErrorPageWaiter waiter(web_contents());
-
-  // Load enough bytes to trigger the intervention.
-  ResourceDataUpdate(ad_frame, ResourceCached::kNotCached,
-                     (heavy_ad_thresholds::kMaxNetworkBytes / 1024) + 1);
-
-  EXPECT_FALSE(HasInterventionReportsAfterFlush(ad_frame));
-
-  waiter.WaitForError();
-}
-
 TEST_F(AdsPageLoadMetricsObserverTest, NoFirstContentfulPaint_NotRecorded) {
   RenderFrameHost* main_frame = NavigateMainFrame(kNonAdUrl);
   RenderFrameHost* ad_frame = CreateAndNavigateSubFrame(kAdUrl, main_frame);

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

@@ -9,6 +9,7 @@
 #include <string>
 
 #include "base/feature_list.h"
+#include "base/metrics/field_trial_params.h"
 #include "chrome/browser/page_load_metrics/observers/ad_metrics/ads_page_load_metrics_observer.h"
 #include "chrome/common/chrome_features.h"
 #include "content/public/browser/render_frame_host.h"
@@ -29,6 +30,11 @@ using OriginStatusWithThrottling = FrameData::OriginStatusWithThrottling;
 // visible.
 const int kMinimumVisibleFrameArea = 25;
 
+// Controls what types of heavy ads will be unloaded by the intervention.
+const base::FeatureParam<int> kHeavyAdUnloadPolicyParam = {
+    &features::kHeavyAdIntervention, "kUnloadPolicy",
+    static_cast<int>(FrameData::HeavyAdUnloadPolicy::kAll)};
+
 }  // namespace
 
 // static
@@ -183,28 +189,57 @@ void FrameData::UpdateCpuUsage(base::TimeTicks update_time,
   }
 }
 
-bool FrameData::MaybeTriggerHeavyAdIntervention() {
+FrameData::HeavyAdAction FrameData::MaybeTriggerHeavyAdIntervention() {
+  // TODO(johnidel): This method currently does a lot of heavy lifting: tracking
+  // noised and unnoised metrics, determining feature action, and branching
+  // based on configuration. Consider splitting this out and letting AdsPLMO do
+  // more of the feature specific logic.
+  //
+  // If the intervention has already performed an action on this frame, do not
+  // perform another. Metrics will have been calculated already.
   if (user_activation_status_ == UserActivationStatus::kReceivedActivation ||
-      heavy_ad_status_with_noise_ != HeavyAdStatus::kNone)
-    return false;
+      heavy_ad_action_ != HeavyAdAction::kNone) {
+    return HeavyAdAction::kNone;
+  }
 
+  // Update heavy ad related metrics. Metrics are reported for all thresholds,
+  // regardless of unload policy.
   if (heavy_ad_status_ == HeavyAdStatus::kNone) {
-    heavy_ad_status_ =
-        ComputeHeavyAdStatus(false /* use_network_threshold_noise */);
+    heavy_ad_status_ = ComputeHeavyAdStatus(
+        false /* use_network_threshold_noise */, HeavyAdUnloadPolicy::kAll);
+  }
+  if (heavy_ad_status_with_noise_ == HeavyAdStatus::kNone) {
+    heavy_ad_status_with_noise_ = ComputeHeavyAdStatus(
+        true /* use_network_threshold_noise */, HeavyAdUnloadPolicy::kAll);
   }
 
-  heavy_ad_status_with_noise_ =
-      ComputeHeavyAdStatus(true /* use_network_threshold_noise */);
+  // Only activate the field trial if there is a heavy ad. Getting the feature
+  // param value activates the trial, so we cannot limit activating the trial
+  // based on the HeavyAdUnloadPolicy. Therefore, we just use a heavy ad of any
+  // type as a gate for activating trial.
   if (heavy_ad_status_with_noise_ == HeavyAdStatus::kNone)
-    return false;
+    return HeavyAdAction::kNone;
+
+  heavy_ad_status_with_policy_ = ComputeHeavyAdStatus(
+      true /* use_network_threshold_noise */,
+      static_cast<HeavyAdUnloadPolicy>(kHeavyAdUnloadPolicyParam.Get()));
+
+  if (heavy_ad_status_with_policy_ == HeavyAdStatus::kNone)
+    return HeavyAdAction::kNone;
 
   // Only check if the feature is enabled once we have a heavy ad. This is done
   // to ensure that any experiment for this feature will only be comparing
   // groups who have seen a heavy ad.
-  return base::FeatureList::IsEnabled(features::kHeavyAdIntervention) ||
-         base::FeatureList::IsEnabled(features::kHeavyAdInterventionWarning);
-}
+  if (!base::FeatureList::IsEnabled(features::kHeavyAdIntervention)) {
+    // If the intervention is not enabled, we return whether reporting is
+    // enabled.
+    return base::FeatureList::IsEnabled(features::kHeavyAdInterventionWarning)
+               ? HeavyAdAction::kReport
+               : HeavyAdAction::kNone;
+  }
 
+  return HeavyAdAction::kUnload;
+}
 
 base::TimeDelta FrameData::GetActivationCpuUsage(
     UserActivationStatus status) const {
@@ -346,23 +381,30 @@ void FrameData::UpdateFrameVisibility() {
 }
 
 FrameData::HeavyAdStatus FrameData::ComputeHeavyAdStatus(
-    bool use_network_threshold_noise) const {
-  // Check if the frame meets the peak CPU usage threshold.
-  if (peak_windowed_cpu_percent_ >=
-      heavy_ad_thresholds::kMaxPeakWindowedPercent) {
-    return HeavyAdStatus::kPeakCpu;
+    bool use_network_threshold_noise,
+    HeavyAdUnloadPolicy policy) const {
+  if (policy == HeavyAdUnloadPolicy::kCpuOnly ||
+      policy == HeavyAdUnloadPolicy::kAll) {
+    // Check if the frame meets the peak CPU usage threshold.
+    if (peak_windowed_cpu_percent_ >=
+        heavy_ad_thresholds::kMaxPeakWindowedPercent) {
+      return HeavyAdStatus::kPeakCpu;
+    }
+
+    // Check if the frame meets the absolute CPU time threshold.
+    if (GetTotalCpuUsage().InMilliseconds() >= heavy_ad_thresholds::kMaxCpuTime)
+      return HeavyAdStatus::kTotalCpu;
   }
 
-  // Check if the frame meets the absolute CPU time threshold.
-  if (GetTotalCpuUsage().InMilliseconds() >= heavy_ad_thresholds::kMaxCpuTime)
-    return HeavyAdStatus::kTotalCpu;
-
-  size_t network_threshold =
-      heavy_ad_thresholds::kMaxNetworkBytes +
-      (use_network_threshold_noise ? heavy_ad_network_threshold_noise_ : 0);
+  if (policy == HeavyAdUnloadPolicy::kNetworkOnly ||
+      policy == HeavyAdUnloadPolicy::kAll) {
+    size_t network_threshold =
+        heavy_ad_thresholds::kMaxNetworkBytes +
+        (use_network_threshold_noise ? heavy_ad_network_threshold_noise_ : 0);
 
-  // Check if the frame meets the network threshold, possible including noise.
-  if (network_bytes_ >= network_threshold)
-    return HeavyAdStatus::kNetwork;
+    // Check if the frame meets the network threshold, possible including noise.
+    if (network_bytes_ >= network_threshold)
+      return HeavyAdStatus::kNetwork;
+  }
   return HeavyAdStatus::kNone;
 }

chrome/browser/page_load_metrics/observers/ad_metrics/frame_data.h

@@ -85,6 +85,27 @@ class FrameData {
     kMaxValue = kPeakCpu,
   };
 
+  // Controls what values of HeavyAdStatus will be cause an unload due to the
+  // intervention.
+  enum class HeavyAdUnloadPolicy {
+    kNetworkOnly = 0,
+    kCpuOnly = 1,
+    kAll = 2,
+  };
+
+  // Represents how a frame should be treated by the heavy ad intervention.
+  enum class HeavyAdAction {
+    // Nothing should be done, i.e. the ad is not heavy or the intervention is
+    // not enabled.
+    kNone = 0,
+    // The ad should be reported as heavy.
+    kReport = 1,
+    // The ad should be reported and unloaded.
+    kUnload = 2,
+    // The frame was ignored, i.e. the blocklist was full or page is a reload.
+    kIgnored = 3,
+  };
+
   // These values are persisted to logs. Entries should not be renumbered and
   // numeric values should never be reused. For any additions, also update the
   // corresponding PageEndReason enum in enums.xml.
@@ -158,11 +179,11 @@ class FrameData {
   // |update_time|.
   void UpdateCpuUsage(base::TimeTicks update_time, base::TimeDelta update);
 
-  // Returns whether the heavy ad intervention was triggered on this frame.
+  // Returns how the frame should be treated by the heavy ad intervention.
   // This intervention is triggered when the frame is considered heavy, has not
   // received user gesture, and the intervention feature is enabled. This
-  // returns true the first time the criteria is met, and false afterwards.
-  bool MaybeTriggerHeavyAdIntervention();
+  // returns an action the first time the criteria is met, and false afterwards.
+  HeavyAdAction MaybeTriggerHeavyAdIntervention();
 
   // Get the cpu usage for the appropriate activation period.
   base::TimeDelta GetActivationCpuUsage(UserActivationStatus status) const;
@@ -263,6 +284,14 @@ class FrameData {
     return heavy_ad_status_with_noise_;
   }
 
+  HeavyAdStatus heavy_ad_status_with_policy() const {
+    return heavy_ad_status_with_policy_;
+  }
+
+  void set_heavy_ad_action(HeavyAdAction heavy_ad_action) {
+    heavy_ad_action_ = heavy_ad_action;
+  }
+
  private:
   // Time updates for the frame with a timestamp indicating when they arrived.
   // Used for windowed cpu load reporting.
@@ -280,8 +309,10 @@ class FrameData {
   // the heavy ad intervention and returns the type of threshold hit if any.
   // If |use_network_threshold_noise| is set,
   // |heavy_ad_network_threshold_noise_| is added to the network threshold when
+  // computing the status. |policy| controls which thresholds are used when
   // computing the status.
-  HeavyAdStatus ComputeHeavyAdStatus(bool use_network_threshold_noise) const;
+  HeavyAdStatus ComputeHeavyAdStatus(bool use_network_threshold_noise,
+                                     HeavyAdUnloadPolicy policy) const;
 
   // The frame tree node id of root frame of the subtree that |this| is
   // tracking information for.
@@ -363,6 +394,14 @@ class FrameData {
   // intervention.
   HeavyAdStatus heavy_ad_status_with_noise_;
 
+  // Same as |heavy_ad_status_with_noise_| but selectively uses thresholds based
+  // on a field trial param. This status is used to control when the
+  // intervention fires.
+  HeavyAdStatus heavy_ad_status_with_policy_ = HeavyAdStatus::kNone;
+
+  // The action taken on this frame by the heavy ad intervention if any.
+  HeavyAdAction heavy_ad_action_ = HeavyAdAction::kNone;
+
   // Number of bytes of noise that should be added to the network threshold.
   const int heavy_ad_network_threshold_noise_;