[PageLoadMetrics] Relax invariants and log the exceptions

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 38 matching lines @@
       return true;
     }
   }
 
   const GURL& url = navigation_handle->GetURL();
   return url.host_piece() == "tpc.googlesyndication.com" &&
          base::StartsWith(url.path_piece(), "/safeframe",
                           base::CompareCase::SENSITIVE);
 }
 
+void RecordParentExistsForSubFrame(bool parent_exists) {
+  UMA_HISTOGRAM_BOOLEAN("PageLoad.Clients.Ads.Google.ParentExistsForSubFrame",
+                        parent_exists);
+}
+
 }  // namespace
 
 AdsPageLoadMetricsObserver::AdFrameData::AdFrameData(
     FrameTreeNodeId frame_tree_node_id)
     : frame_bytes(0u),
       frame_bytes_uncached(0u),
       frame_tree_node_id(frame_tree_node_id) {}
 
 // static
 std::unique_ptr<AdsPageLoadMetricsObserver>
@@ skipping 46 matching lines @@
     // 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));
   }
 
   // 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.
+    RecordParentExistsForSubFrame(false /* parent_exists */);
+
+    return CONTINUE_OBSERVING;
+  }
+  RecordParentExistsForSubFrame(true /* parent_exists */);
+
   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;
 
@@ skipping 30 matching lines @@
     // Data uris should be accounted for in the generating resource, not
     // here. Blobs for PlzNavigate shouldn't be counted as the http resource
     // was already counted. Blobs for other things like CacheStorage or
     // IndexedDB are also ignored for now, as they're not normal HTTP loads.
     return;
   }
 
   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",

[Bryan McQuade, 2017/05/16 17:28:39]

ah, sorry, didn't realize this was already varying on resource type and thus we
don't need a separate histo name. here like the bool case i think the metrics
team asks that we factor histograms into a single helper function, so we should
try to do that here.

[jkarlin, 2017/05/16 17:33:59]

Done.

+          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",
+          extra_request_info.resource_type, content::RESOURCE_TYPE_LAST_TYPE);
+    }
     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.
   AdFrameData* ancestor_data = id_and_data->second;
@@ skipping 77 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);
 }