Reland "Trigger heavy ad intervention when an unloaded heavy ad navigates"

This is a reland of 312a57a1

Original change's description:
> 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/+/2269740
> Reviewed-by: John Delaney <johnidel@chromium.org>
> Commit-Queue: Andrew Paseltiner <apaseltiner@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#791488}

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

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

@@ -258,9 +258,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()] =
@@ -397,7 +399,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;
     }
     if (base::FeatureList::IsEnabled(features::kV8PerAdFrameMemoryMonitoring) &&
@@ -418,7 +421,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
@@ -461,6 +465,34 @@ 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 &&
+      navigation_handle->HasCommitted()) {
+    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_policy() !=
+            FrameData::HeavyAdStatus::kNone) {
+      RecordPerFrameHistograms(*id_and_data->second);
+      id_and_data->second->RecordAdFrameLoadUkmEvent(
+          GetDelegate().GetPageUkmSourceId());
+      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

@@ -1672,15 +1672,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
@@ -1690,7 +1706,6 @@ IN_PROC_BROWSER_TEST_F(AdsPageLoadMetricsObserverResourceBrowserTest,
   content::TestNavigationObserver error_observer(web_contents,
                                                  net::ERR_BLOCKED_BY_CLIENT);
 
-  auto waiter = CreateAdsPageLoadMetricsTestWaiter();
   GURL url = embedded_test_server()->GetURL(
       "/ads_observer/ad_with_incomplete_resource.html");
   ui_test_utils::NavigateToURL(browser(), url);
@@ -1712,6 +1727,26 @@ IN_PROC_BROWSER_TEST_F(AdsPageLoadMetricsObserverResourceBrowserTest,
   histogram_tester.ExpectBucketCount(
       "Blink.UseCounter.Features",
       blink::mojom::WebFeature::kHeavyAdIntervention, 1);
+
+  content::TestNavigationObserver error_observer2(web_contents,
+                                                  net::ERR_BLOCKED_BY_CLIENT);
+
+  // 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);
+
+  error_observer2.WaitForNavigationFinished();
+
+  // 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

@@ -84,8 +84,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;
 
@@ -267,7 +269,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

@@ -156,7 +156,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(
@@ -405,6 +406,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>