Trigger heavy ad intervention when an unloaded heavy ad navigates

Bug: 1099014
Change-Id: I98a2d0e548a48fd93618fbae1c1d458f1dbd58a4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2269740Reviewed-by: John Delaney <johnidel@chromium.org>
Commit-Queue: Andrew Paseltiner <apaseltiner@chromium.org>
Cr-Commit-Position: refs/heads/master@{#791488}

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

@@ -243,9 +243,11 @@ AdsPageLoadMetricsObserver::OnCommit(
       navigation_handle->GetReloadType() != content::ReloadType::NONE;
 
   aggregate_frame_data_->UpdateForNavigation(
-      navigation_handle->GetRenderFrameHost(), true /* frame_navigated */);
+      navigation_handle->GetRenderFrameHost(), true /* frame_navigated */,
+      true /* record_frame_metrics */);
   main_frame_data_->UpdateForNavigation(navigation_handle->GetRenderFrameHost(),
-                                        true /* frame_navigated */);
+                                        true /* frame_navigated */,
+                                        true /* record_frame_metrics */);
 
   // The main frame is never considered an ad.
   ad_frames_data_[navigation_handle->GetFrameTreeNodeId()] =
@@ -388,7 +390,8 @@ void AdsPageLoadMetricsObserver::UpdateAdFrameData(
 
   if (should_create_new_frame_data) {
     if (previous_data) {
-      previous_data->UpdateForNavigation(ad_host, frame_navigated);
+      previous_data->UpdateForNavigation(ad_host, frame_navigated,
+                                         true /* record_frame_metrics */);
       return;
     }
     ad_frames_data_storage_.emplace_back(
@@ -396,7 +399,8 @@ void AdsPageLoadMetricsObserver::UpdateAdFrameData(
         heavy_ad_threshold_noise_provider_->GetNetworkThresholdNoiseForFrame());
     ad_data_iterator = --ad_frames_data_storage_.end();
     ad_data = &*ad_data_iterator;
-    ad_data->UpdateForNavigation(ad_host, frame_navigated);
+    ad_data->UpdateForNavigation(ad_host, frame_navigated,
+                                 true /* record_frame_metrics */);
   }
 
   // Maybe update frame depth based on the new ad frames distance to the ad
@@ -439,6 +443,33 @@ void AdsPageLoadMetricsObserver::OnDidFinishSubFrameNavigation(
   content::RenderFrameHost* frame_host =
       FindFrameMaybeUnsafe(navigation_handle);
 
+  // We want to reset the FrameData for an ad after heavy ads fires, so that we
+  // can trigger on subsequent navigations if the page tries to serve another ad
+  // in the frame.
+  if (navigation_handle->IsErrorPage() &&
+      navigation_handle->GetNetErrorCode() == net::ERR_BLOCKED_BY_CLIENT) {
+    const auto& id_and_data = ad_frames_data_.find(frame_tree_node_id);
+    if (id_and_data != ad_frames_data_.end() &&
+        id_and_data->second != ad_frames_data_storage_.end() &&
+        id_and_data->second->heavy_ad_status_with_noise() !=
+            FrameData::HeavyAdStatus::kNone) {
+      RecordPerFrameHistograms(*id_and_data->second);
+      id_and_data->second->RecordAdFrameLoadUkmEvent(
+          GetDelegate().GetSourceId());
+      ad_frames_data_storage_.erase(id_and_data->second);
+      ad_frames_data_.erase(id_and_data);
+      ad_frames_data_storage_.emplace_back(
+          frame_tree_node_id, heavy_ad_threshold_noise_provider_
+                                  ->GetNetworkThresholdNoiseForFrame());
+      auto ad_data_iterator = --ad_frames_data_storage_.end();
+      FrameData* ad_data = &*ad_data_iterator;
+      ad_frames_data_[frame_tree_node_id] = ad_data_iterator;
+      ad_data->UpdateForNavigation(frame_host, true /*frame_navigated=*/,
+                                   false /*record_frame_metrics=*/);
+      return;
+    }
+  }
+
   bool is_adframe = client->GetThrottleManager()->IsFrameTaggedAsAd(frame_host);
 
   // TODO(https://crbug.com): The following block is a hack to ignore certain

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

@@ -1663,15 +1663,31 @@ IN_PROC_BROWSER_TEST_F(AdsPageLoadMetricsObserverResourceBrowserTest,
   EXPECT_EQ(4u, console_observer.messages().size());
 }
 
+class NoHostThresholdHeavyAdsBrowserTest
+    : public AdsPageLoadMetricsObserverResourceBrowserTest {
+ public:
+  NoHostThresholdHeavyAdsBrowserTest() {
+    scoped_feature_list_.InitAndEnableFeatureWithParameters(
+        features::kHeavyAdPrivacyMitigations, {{"host-threshold", "100"}});
+  }
+
+ private:
+  base::test::ScopedFeatureList scoped_feature_list_;
+};
+
 // Verifies that the frame is navigated to the intervention page when a
 // heavy ad intervention triggers.
-IN_PROC_BROWSER_TEST_F(AdsPageLoadMetricsObserverResourceBrowserTest,
+IN_PROC_BROWSER_TEST_F(NoHostThresholdHeavyAdsBrowserTest,
                        HeavyAdInterventionEnabled_ErrorPageLoaded) {
   base::HistogramTester histogram_tester;
   auto incomplete_resource_response =
       std::make_unique<net::test_server::ControllableHttpResponse>(
           embedded_test_server(), "/ads_observer/incomplete_resource.js",
           true /*relative_url_is_prefix*/);
+  auto incomplete_resource_response2 =
+      std::make_unique<net::test_server::ControllableHttpResponse>(
+          embedded_test_server(), "/ads_observer/incomplete_resource2.js",
+          true /*relative_url_is_prefix*/);
   ASSERT_TRUE(embedded_test_server()->Start());
 
   // Create a navigation observer that will watch for the intervention to
@@ -1703,6 +1719,25 @@ IN_PROC_BROWSER_TEST_F(AdsPageLoadMetricsObserverResourceBrowserTest,
   histogram_tester.ExpectBucketCount(
       "Blink.UseCounter.Features",
       blink::mojom::WebFeature::kHeavyAdIntervention, 1);
+
+  // Test that subsequent navigations to the error page can still trigger heavy
+  // ads (crbug.com/1099014).
+  EXPECT_TRUE(content::ExecJs(web_contents, "advanceSrcdoc();",
+                              content::EXECUTE_SCRIPT_NO_USER_GESTURE));
+
+  // Load a resource large enough to trigger the intervention.
+  LoadLargeResource(incomplete_resource_response2.get(),
+                    kMaxHeavyAdNetworkSize);
+
+  // Wait for the resource update to be received for the large resource.
+  waiter->AddMinimumNetworkBytesExpectation(2 * kMaxHeavyAdNetworkSize);
+  waiter->Wait();
+
+  // We can't check whether the navigation didn't occur because the error page
+  // load is not synchronous. Instead check that we didn't log intervention UMA
+  // that is always recorded when the intervention occurs. Both
+  // interventions should have been logged.
+  histogram_tester.ExpectTotalCount(kHeavyAdInterventionTypeHistogramId, 2);
 }
 
 class AdsPageLoadMetricsObserverResourceBrowserTestWithoutHeavyAdIntervention

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

@@ -78,8 +78,10 @@ FrameData::FrameData(FrameTreeNodeId root_frame_tree_node_id,
 FrameData::~FrameData() = default;
 
 void FrameData::UpdateForNavigation(content::RenderFrameHost* render_frame_host,
-                                    bool frame_navigated) {
+                                    bool frame_navigated,
+                                    bool record_metrics) {
   frame_navigated_ = frame_navigated;
+  record_metrics_ = record_metrics;
   if (!render_frame_host)
     return;
 
@@ -237,7 +239,7 @@ void FrameData::MaybeUpdateFrameDepth(
 }
 
 bool FrameData::ShouldRecordFrameForMetrics() const {
-  return bytes() != 0 || !GetTotalCpuUsage().is_zero();
+  return record_metrics_ && (bytes() != 0 || !GetTotalCpuUsage().is_zero());
 }
 
 void FrameData::RecordAdFrameLoadUkmEvent(ukm::SourceId source_id) const {

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

@@ -135,7 +135,8 @@ class FrameData {
 
   // Update the metadata of this frame if it is being navigated.
   void UpdateForNavigation(content::RenderFrameHost* render_frame_host,
-                           bool frame_navigated);
+                           bool frame_navigated,
+                           bool record_frame_metrics);
 
   // Updates the number of bytes loaded in the frame given a resource load.
   void ProcessResourceLoadInFrame(
@@ -385,6 +386,12 @@ class FrameData {
   // Number of bytes of noise that should be added to the network threshold.
   const int heavy_ad_network_threshold_noise_;
 
+  // |record_metrics| indicates whether metrics should be logged for this frame.
+  // This may be false in cases where we are tracking a frame, but do not want
+  // to log metrics until a subsequent navigation, e.g. if a frame is currently
+  // navigated to the heavy ads error page.
+  bool record_metrics_ = true;
+
   DISALLOW_COPY_AND_ASSIGN(FrameData);
 };
 

chrome/test/data/ads_observer/ad_with_incomplete_resource.html

@@ -6,6 +6,10 @@
   let adIframe = createAdIframe();
   // "incomplete_resource.js" should never be a complete resource load.
   adIframe.srcdoc = "<link rel='preload' href='incomplete_resource.js' as='script'>";
+
+  function advanceSrcdoc() {
+    adIframe.srcdoc =  "<link rel='preload' href='incomplete_resource2.js' as='script'>";
+  }
 </script>
 
 </body>