[PageLoadMetrics] Keep track of Ad Sizes on Pages

chrome/browser/page_load_metrics/observers/ads_page_load_metrics_observer.cc

Index: chrome/browser/page_load_metrics/observers/ads_page_load_metrics_observer.cc
diff --git a/chrome/browser/page_load_metrics/observers/ads_page_load_metrics_observer.cc b/chrome/browser/page_load_metrics/observers/ads_page_load_metrics_observer.cc
new file mode 100644
index 0000000000000000000000000000000000000000..4845b301b31dae3ddad67a76f7f4aa19cb5c69bb
--- /dev/null
+++ b/chrome/browser/page_load_metrics/observers/ads_page_load_metrics_observer.cc
@@ -0,0 +1,215 @@
+// 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 "chrome/browser/page_load_metrics/page_load_metrics_util.h"
+#include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/web_contents.h"
+
+namespace {
+
+bool FrameIsAd(content::NavigationHandle* navigation_handle) {
+  int frame_tree_node_id = navigation_handle->GetFrameTreeNodeId();

[Charlie Harrison, 2017/04/10 18:39:41]

#include navigation_handle

[jkarlin, 2017/04/11 16:46:28]

Done.

+  content::RenderFrameHost* current_frame_host =
+      navigation_handle->GetWebContents()->FindFrameByFrameTreeNodeId(
+          frame_tree_node_id);
+  const std::string& name = current_frame_host->GetFrameName();

[Charlie Harrison, 2017/04/10 18:39:41]

#include <string>

[jkarlin, 2017/04/11 16:46:28]

Done.

+  const std::string& url_spec = navigation_handle->GetURL().spec();

[Charlie Harrison, 2017/04/10 18:39:41]

#include gurl

[jkarlin, 2017/04/11 16:46:28]

Done.

+
+  // Because sub-resource filtering isn't always enabled, and doesn't work
+  // well in monitoring mode (no CSS enforcement), it's difficult to identify
+  // ads. Google ads are prevalent and easy to track, so we'll start by
+  // tracking those.
+  // TODO(jkarlin): Track other ad networks that are easy to identify.
+  return base::StartsWith(name, "google_ads_iframe",

[Charlie Harrison, 2017/04/10 18:39:41]

#include base/strings/string_util.h

[Charlie Harrison, 2017/04/10 18:39:41]

Can you document that frame names can be very big, so additional code should be
careful not to do full string scans.

[jkarlin, 2017/04/11 16:46:27]

Done.

[jkarlin, 2017/04/11 16:46:28]

Done.

+                          base::CompareCase::SENSITIVE) ||
+         base::StartsWith(name, "google_ads_frame",
+                          base::CompareCase::SENSITIVE) ||
+         base::StartsWith(url_spec,

[Charlie Harrison, 2017/04/10 18:39:41]

Replace these last two with:
url.host_piece() == "tcp.googlesyndication.com" && url.path_piece() ==
"/safeframe/"

Also include stringpiece. If you are really concerned about non-http/s you can
use url.SchemeIsHttpOrHttps

[jkarlin, 2017/04/11 16:46:28]

Done.

+                          "http://tpc.googlesyndication.com/safeframe/",
+                          base::CompareCase::SENSITIVE) ||
+         base::StartsWith(url_spec,
+                          "https://tpc.googlesyndication.com/safeframe/",
+                          base::CompareCase::SENSITIVE);
+}
+
+}  // namespace
+
+AdsPageLoadMetricsObserver::AdsPageLoadMetricsObserver() = default;
+AdsPageLoadMetricsObserver::~AdsPageLoadMetricsObserver() = default;
+
+page_load_metrics::PageLoadMetricsObserver::ObservePolicy
+AdsPageLoadMetricsObserver::OnCommit(
+    content::NavigationHandle* navigation_handle) {
+  DCHECK(ad_frames_.empty());
+  DCHECK(ad_ancestors_.empty());
+
+  // The main frame is never considered an ad.
+  ad_ancestors_[navigation_handle->GetFrameTreeNodeId()] =
+      kInvalidFrameTreeNodeId;
+  ProcessDelayedResources(navigation_handle->GetFrameTreeNodeId());
+  return CONTINUE_OBSERVING;
+}
+
+page_load_metrics::PageLoadMetricsObserver::ObservePolicy
+AdsPageLoadMetricsObserver::OnCommitSubFrame(
+    content::NavigationHandle* navigation_handle) {
+  DCHECK(
+      !base::ContainsKey(ad_frames_, navigation_handle->GetFrameTreeNodeId()));
+
+  FrameTreeNodeId frame_tree_node_id = navigation_handle->GetFrameTreeNodeId();
+  FrameTreeNodeId ancestor_id = FindAdAncestor(navigation_handle);
+
+  if (ancestor_id != kInvalidFrameTreeNodeId) {

[Charlie Harrison, 2017/04/10 18:39:41]

optional: slightly prefer:

if (ancestor_id != kInvalidFrameTreeNodeId) {
  ad_ancestors_[frame_tree_nod_id] = ancestor_id
} else if (FrameIsAd(navigation_handle) {
  ad_frames_[frame_tree_node_id] = frame_tree_node_id;
  ad_ancestors_[frame_tree_nod_id] = frame_tree_node_id;
} else {
  ad_ancestors_[frame_tree_nod_id] = KInvalidFrameTreeNodeId;
}
ProcessDelayedResources(frame_tree_node_id);
return CONTINUE_OBSERVING;

[jkarlin, 2017/04/11 16:46:27]

Done.

+    ad_ancestors_[frame_tree_node_id] = ancestor_id;
+    ProcessDelayedResources(frame_tree_node_id);
+    return CONTINUE_OBSERVING;
+  }
+
+  if (FrameIsAd(navigation_handle)) {
+    ad_frames_[frame_tree_node_id] = AdFrameData();
+    ad_ancestors_[frame_tree_node_id] = frame_tree_node_id;
+    ProcessDelayedResources(frame_tree_node_id);
+    return CONTINUE_OBSERVING;
+  }
+
+  ad_ancestors_[frame_tree_node_id] = kInvalidFrameTreeNodeId;
+  ProcessDelayedResources(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.
+  if (extra_info.did_commit)
+    RecordHistograms();
+
+  return STOP_OBSERVING;
+}
+
+void AdsPageLoadMetricsObserver::OnLoadedResource(
+    const page_load_metrics::ExtraRequestInfo& extra_request_info) {
+  if (!base::ContainsKey(ad_ancestors_,

[Charlie Harrison, 2017/04/10 18:39:41]

Lots of map lookups per resource request :(

I wonder if we can consolidate and share a map for some of this data? I am
nervous this will perform badly on sites with many frames (~50-100)

[jkarlin, 2017/04/11 16:46:28]

Done.

+                         extra_request_info.frame_tree_node_id)) {
+    // This resouce is for a frame that hasn't yet committed. It must be the
+    // main document for the frame. Hold onto it and once it commits we'll run
+    // it in ProcessDelayedResources.
+    auto it_and_success = delayed_resources_.insert(std::make_pair(
+        extra_request_info.frame_tree_node_id, extra_request_info));
+    DCHECK(it_and_success.second);
+    return;
+  }
+
+  page_bytes_ += extra_request_info.raw_body_bytes;
+  if (!extra_request_info.was_cached)
+    uncached_page_bytes_ += extra_request_info.raw_body_bytes;
+
+  // Determine if the frame (or its ancestor) is an ad, if so attribute the
+  // bytes to the highest ad ancestor.
+  FrameTreeNodeId ad_ancestor_id =
+      ad_ancestors_[extra_request_info.frame_tree_node_id];
+  if (ad_ancestor_id != kInvalidFrameTreeNodeId) {
+    ad_frames_[ad_ancestor_id].frame_bytes += extra_request_info.raw_body_bytes;
+    if (!extra_request_info.was_cached)

[Charlie Harrison, 2017/04/10 18:39:41]

nit: needs {}

[jkarlin, 2017/04/11 16:46:28]

Done.

+      ad_frames_[ad_ancestor_id].frame_bytes_uncached +=
+          extra_request_info.raw_body_bytes;
+  }
+}
+
+void AdsPageLoadMetricsObserver::OnComplete(
+    const page_load_metrics::PageLoadTiming& timing,
+    const page_load_metrics::PageLoadExtraInfo& info) {
+  RecordHistograms();
+}
+
+void AdsPageLoadMetricsObserver::RecordHistograms() {
+  if (page_bytes_ == 0)
+    return;
+
+  size_t total_ad_frame_bytes = 0;
+  size_t uncached_ad_frame_bytes = 0;
+  int ad_frames = 0;
+
+  for (const auto& frame_id_and_size : ad_frames_) {

[Charlie Harrison, 2017/04/10 18:39:41]

Is logging on the order of 100 histograms in a loop costly on the CPU?  I
remember seeing some histogram logging in CPU profiles at some point but not
sure how expensive these particular macros are.

[jkarlin, 2017/04/11 16:46:28]

https://chromium.googlesource.com/chromium/src.git/+/HEAD/tools/metrics/histo...

"In general, the histogram code is highly optimized. Do not be concerned about
the processing cost of emitting to a histogram (unless you're using sparse
histograms)."

+    total_ad_frame_bytes += frame_id_and_size.second.frame_bytes;
+    uncached_ad_frame_bytes += frame_id_and_size.second.frame_bytes_uncached;
+
+    const AdFrameData& data = frame_id_and_size.second;
+    if (data.frame_bytes > 0) {
+      ad_frames += 1;
+      PAGE_BYTES_HISTOGRAM("PageLoad.Clients.Ads.Google.Bytes.AdFrame",
+                           data.frame_bytes);
+      PAGE_BYTES_HISTOGRAM(
+          "PageLoad.Clients.Ads.Google.Bytes.AdFrameFromNetwork",
+          data.frame_bytes_uncached);
+      UMA_HISTOGRAM_PERCENTAGE(
+          "PageLoad.Clients.Ads.Google.BytesPercent.AdFrameFromNetwork",
+          data.frame_bytes_uncached * 100 / data.frame_bytes);
+    }
+  }
+
+  // Don't post UMA for pages that don't have ads or content.
+  if (total_ad_frame_bytes == 0) {
+    UMA_HISTOGRAM_COUNTS("PageLoad.Clients.Ads.Google.PageHasNoAds", 1);
+    return;
+  }
+
+  PAGE_BYTES_HISTOGRAM("PageLoad.Clients.Ads.Google.Bytes.AdFrames",
+                       total_ad_frame_bytes);
+
+  PAGE_BYTES_HISTOGRAM("PageLoad.Clients.Ads.Google.Bytes.PageSansAdFrames",
+                       page_bytes_ - total_ad_frame_bytes);
+
+  PAGE_BYTES_HISTOGRAM("PageLoad.Clients.Ads.Google.Bytes.Page", page_bytes_);
+  PAGE_BYTES_HISTOGRAM("PageLoad.Clients.Ads.Google.Bytes.PageFromNetwork",
+                       uncached_page_bytes_);
+
+  UMA_HISTOGRAM_PERCENTAGE("PageLoad.Clients.Ads.Google.BytesPercent.AdFrames",
+                           total_ad_frame_bytes * 100 / page_bytes_);
+
+  UMA_HISTOGRAM_COUNTS_1000("PageLoad.Clients.Ads.Google.AdFrameCount",
+                            ad_frames);
+
+  PAGE_BYTES_HISTOGRAM("PageLoad.Clients.Ads.Google.Bytes.AdFramesFromNetwork",
+                       uncached_ad_frame_bytes);
+
+  UMA_HISTOGRAM_PERCENTAGE(
+      "PageLoad.Clients.Ads.Google.BytesPercent."
+      "AdFramesFromNetworkOfAdFramesTotal",
+      uncached_ad_frame_bytes * 100 / total_ad_frame_bytes);
+
+  int percent_bytes_from_uncached_ads =
+      uncached_page_bytes_ == 0
+          ? 0
+          : uncached_ad_frame_bytes * 100 / uncached_page_bytes_;
+  UMA_HISTOGRAM_PERCENTAGE(
+      "PageLoad.Clients.Ads.Google.Bytes.Percent."
+      "AdFramesFromNetworkOfPageFromNetwork",
+      percent_bytes_from_uncached_ads);
+}
+
+int AdsPageLoadMetricsObserver::FindAdAncestor(
+    content::NavigationHandle* navigation_handle) {
+  // We haven't seen a load from this frame before. We should have seen its
+  // parent though. Use the ad ancestor of its parent.
+  content::RenderFrameHost* parent_frame_host =
+      navigation_handle->GetRenderFrameHost()->GetParent();
+  DCHECK(parent_frame_host);  // Since this isn't a main frame.
+  DCHECK(base::ContainsKey(ad_ancestors_,
+                           parent_frame_host->GetFrameTreeNodeId()));
+  return ad_ancestors_[parent_frame_host->GetFrameTreeNodeId()];
+}
+
+void AdsPageLoadMetricsObserver::ProcessDelayedResources(
+    FrameTreeNodeId frame_tree_node_id) {
+  const auto& frame_id_and_request =
+      delayed_resources_.find(frame_tree_node_id);
+  if (frame_id_and_request == delayed_resources_.end())
+    return;
+  OnLoadedResource(frame_id_and_request->second);
+}