[PageLoadMetrics] Ignore ad frames with no content

chrome/browser/page_load_metrics/observers/ads_page_load_metrics_observer.cc

 // Copyright 2017 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/page_load_metrics/observers/ads_page_load_metrics_observer.h"
 
 #include <string>
 #include <utility>
 
 #include "base/feature_list.h"
@@ skipping 99 matching lines @@
             "PageLoad.Clients.Ads.Google.Navigations.AdFrameRenavigatedToAd",
             FrameIsAd(navigation_handle));
       }
       return CONTINUE_OBSERVING;
     }
     // This frame was previously not an ad, process it as usual. If it had
     // any child frames that were ads, those will still be recorded.
     UMA_HISTOGRAM_BOOLEAN(
         "PageLoad.Clients.Ads.Google.Navigations.NonAdFrameRenavigatedToAd",
         FrameIsAd(navigation_handle));
-  } else if (navigation_handle->IsParentMainFrame()) {
-    top_level_subframe_count_ += 1;
   }
 
   // Determine who the parent frame's ad ancestor is.
   const auto& parent_id_and_data =
       ad_frames_data_.find(parent_frame_host->GetFrameTreeNodeId());
   DCHECK(parent_id_and_data != ad_frames_data_.end());
   AdFrameData* ad_data = parent_id_and_data->second;
 
   if (!ad_data && FrameIsAd(navigation_handle)) {
     // This frame is not nested within an ad frame but is itself an ad.
     ad_frames_data_storage_.emplace_back(frame_tree_node_id);
     ad_data = &ad_frames_data_storage_.back();
   }
 
   ad_frames_data_[frame_tree_node_id] = ad_data;
 
-  if (navigation_handle->IsParentMainFrame() && ad_data)
-    top_level_ad_frame_count_ += 1;
-
   ProcessOngoingNavigationResource(frame_tree_node_id);
   return CONTINUE_OBSERVING;
 }
 
 page_load_metrics::PageLoadMetricsObserver::ObservePolicy
 AdsPageLoadMetricsObserver::FlushMetricsOnAppEnterBackground(
     const page_load_metrics::PageLoadTiming& timing,
     const page_load_metrics::PageLoadExtraInfo& extra_info) {
   // The browser may come back, but there is no guarantee. To be safe, record
   // what we have now and ignore future changes to this navigation.
@@ skipping 57 matching lines @@
     if (!extra_request_info.was_cached) {
       ancestor_data->frame_bytes_uncached += extra_request_info.raw_body_bytes;
     }
   }
 }
 
 void AdsPageLoadMetricsObserver::RecordHistograms() {
   if (page_bytes_ == 0)
     return;
 
+  int non_zero_ad_frames = 0;
   size_t total_ad_frame_bytes = 0;
   size_t uncached_ad_frame_bytes = 0;
 
-  UMA_HISTOGRAM_COUNTS_1000(
-      "PageLoad.Clients.Ads.Google.FrameCounts.AnyParentFrame.AdFrames",
-      ad_frames_data_storage_.size());
+  for (const AdFrameData& ad_frame_data : ad_frames_data_storage_) {
+    if (ad_frame_data.frame_bytes == 0)
+      continue;
 
-  // Don't post UMA for pages that don't have ads.
-  if (ad_frames_data_storage_.empty())
-    return;
-
-  for (const AdFrameData& ad_frame_data : ad_frames_data_storage_) {
+    non_zero_ad_frames += 1;
     total_ad_frame_bytes += ad_frame_data.frame_bytes;
     uncached_ad_frame_bytes += ad_frame_data.frame_bytes_uncached;
 
     PAGE_BYTES_HISTOGRAM(
         "PageLoad.Clients.Ads.Google.Bytes.AdFrames.PerFrame.Total",
         ad_frame_data.frame_bytes);
     PAGE_BYTES_HISTOGRAM(
         "PageLoad.Clients.Ads.Google.Bytes.AdFrames.PerFrame.Network",
         ad_frame_data.frame_bytes_uncached);
-    if (ad_frame_data.frame_bytes > 0) {
-      UMA_HISTOGRAM_PERCENTAGE(
-          "PageLoad.Clients.Ads.Google.Bytes.AdFrames.PerFrame.PercentNetwork",
-          ad_frame_data.frame_bytes_uncached * 100 / ad_frame_data.frame_bytes);
-    }
+    UMA_HISTOGRAM_PERCENTAGE(
+        "PageLoad.Clients.Ads.Google.Bytes.AdFrames.PerFrame.PercentNetwork",
+        ad_frame_data.frame_bytes_uncached * 100 / ad_frame_data.frame_bytes);
   }
 
   UMA_HISTOGRAM_COUNTS_1000(
-      "PageLoad.Clients.Ads.Google.FrameCounts.MainFrameParent.TotalFrames",
-      top_level_subframe_count_);
-  UMA_HISTOGRAM_COUNTS_1000(
-      "PageLoad.Clients.Ads.Google.FrameCounts.MainFrameParent.AdFrames",
-      top_level_ad_frame_count_);
+      "PageLoad.Clients.Ads.Google.FrameCounts.AnyParentFrame.AdFrames",
+      non_zero_ad_frames);
 
-  DCHECK_LT(0, top_level_subframe_count_);  // Because ad frames isn't empty.
-  UMA_HISTOGRAM_PERCENTAGE(
-      "PageLoad.Clients.Ads.Google.FrameCounts.MainFrameParent.PercentAdFrames",
-      top_level_ad_frame_count_ * 100 / top_level_subframe_count_);
+  // Don't post UMA for pages that don't have ads.
+  if (non_zero_ad_frames == 0)
+    return;
 
   PAGE_BYTES_HISTOGRAM(
       "PageLoad.Clients.Ads.Google.Bytes.NonAdFrames.Aggregate.Total",
       page_bytes_ - total_ad_frame_bytes);
 
   PAGE_BYTES_HISTOGRAM("PageLoad.Clients.Ads.Google.Bytes.FullPage.Total",
                        page_bytes_);
   PAGE_BYTES_HISTOGRAM("PageLoad.Clients.Ads.Google.Bytes.FullPage.Network",
                        uncached_page_bytes_);
   if (page_bytes_) {
@@ skipping 24 matching lines @@
 void AdsPageLoadMetricsObserver::ProcessOngoingNavigationResource(
     FrameTreeNodeId frame_tree_node_id) {
   const auto& frame_id_and_request =
       ongoing_navigation_resources_.find(frame_tree_node_id);
   if (frame_id_and_request == ongoing_navigation_resources_.end())
     return;
 
   ProcessLoadedResource(frame_id_and_request->second);
   ongoing_navigation_resources_.erase(frame_id_and_request);
 }