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 |
index b1b2c49c5f60fb9edf8e7bbd0a28d2fd7ce9a879..d399ee088a0978d5022b0c84b48a48b3bf191c01 100644 |
--- 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 |
@@ -122,7 +122,16 @@ AdsPageLoadMetricsObserver::OnDidFinishSubFrameNavigation( |
// 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()); |
+ if (parent_id_and_data == ad_frames_data_.end()) { |
+ // We don't know who the parent for this frame is. One possibility is that |
+ // it's a frame from a previous navigation. |
+ UMA_HISTOGRAM_BOOLEAN("PageLoad.Clients.Ads.Google.ParentExistsForSubFrame", |
Bryan McQuade
2017/05/16 16:22:36
histogram reviewers typically ask me to factor thi
jkarlin
2017/05/16 17:16:09
Done.
|
+ false); |
+ return CONTINUE_OBSERVING; |
+ } |
+ UMA_HISTOGRAM_BOOLEAN("PageLoad.Clients.Ads.Google.ParentExistsForSubFrame", |
+ true); |
+ |
AdFrameData* ad_data = parent_id_and_data->second; |
if (!ad_data && FrameIsAd(navigation_handle)) { |
@@ -173,20 +182,32 @@ void AdsPageLoadMetricsObserver::ProcessLoadedResource( |
const auto& id_and_data = |
ad_frames_data_.find(extra_request_info.frame_tree_node_id); |
if (id_and_data == ad_frames_data_.end()) { |
- // This resource is for a frame that hasn't ever finished a navigation. We |
- // expect it to be the frame's main resource but don't have enough |
- // confidence in that to dcheck it. For example, there might be races |
- // between doc.written resources and navigation failure. |
- // TODO(jkarlin): Add UMA to measure how often we see multiple resources |
- // for a frame before navigation finishes. |
- ongoing_navigation_resources_.emplace( |
- std::piecewise_construct, |
- std::forward_as_tuple(extra_request_info.frame_tree_node_id), |
- std::forward_as_tuple( |
- extra_request_info.url, extra_request_info.frame_tree_node_id, |
- extra_request_info.was_cached, extra_request_info.raw_body_bytes, |
- extra_request_info.original_network_content_length, nullptr, |
- extra_request_info.resource_type)); |
+ if (extra_request_info.resource_type == content::RESOURCE_TYPE_MAIN_FRAME || |
+ extra_request_info.resource_type == content::RESOURCE_TYPE_SUB_FRAME) { |
+ // This resource request is the primary resource load for a frame that |
+ // hasn't yet finished navigating. Hang onto the request info and replay |
+ // it once the frame finishes navigating. |
+ ongoing_navigation_resources_.emplace( |
+ std::piecewise_construct, |
+ std::forward_as_tuple(extra_request_info.frame_tree_node_id), |
+ std::forward_as_tuple( |
+ extra_request_info.url, extra_request_info.frame_tree_node_id, |
+ extra_request_info.was_cached, extra_request_info.raw_body_bytes, |
+ extra_request_info.original_network_content_length, nullptr, |
+ extra_request_info.resource_type)); |
+ UMA_HISTOGRAM_ENUMERATION( |
+ "PageLoad.Clients.Ads.Google.ResourceTypeWhenNoFrameFound", |
+ extra_request_info.resource_type, content::RESOURCE_TYPE_LAST_TYPE); |
+ } else { |
+ // This is unexpected, it could be: |
+ // 1. a resource from a previous navigation that started its resource |
+ // load after this page started navigation. |
+ // 2. possibly a resource from a document.written frame whose frame |
+ // failure message has yet to arrive. (uncertain of this) |
+ UMA_HISTOGRAM_ENUMERATION( |
+ "PageLoad.Clients.Ads.Google.ResourceTypeWhenNoFrameFound", |
Bryan McQuade
2017/05/16 16:22:36
can we use a different histogram name here, to dis
jkarlin
2017/05/16 17:16:09
They'll be different, as the case above will have
|
+ extra_request_info.resource_type, content::RESOURCE_TYPE_LAST_TYPE); |
+ } |
return; |
} |