[PageLoadMetrics] Handle case where navigation aborts

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"
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/string_util.h"
 #include "chrome/browser/page_load_metrics/page_load_metrics_util.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
 #include "url/gurl.h"
 
 namespace {
 
 const base::Feature kAdsFeature{"AdsMetrics",
                                 base::FEATURE_DISABLED_BY_DEFAULT};
 
 bool FrameIsAd(content::NavigationHandle* navigation_handle) {
-  content::RenderFrameHost* current_frame_host =
-      navigation_handle->GetRenderFrameHost();
-  DCHECK(current_frame_host);
-  const std::string& name = current_frame_host->GetFrameName();
-  const GURL& url = navigation_handle->GetURL();
-
   // 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. Note that the frame name can be very large, so be careful
   // to avoid full string searches if possible.
   // TODO(jkarlin): Track other ad networks that are easy to identify.
-  return base::StartsWith(name, "google_ads_iframe",
-                          base::CompareCase::SENSITIVE) ||
-         base::StartsWith(name, "google_ads_frame",
-                          base::CompareCase::SENSITIVE) ||
-         (url.host_piece() == "tpc.googlesyndication.com" &&
-          base::StartsWith(url.path_piece(), "/safeframe",
-                           base::CompareCase::SENSITIVE));
+
+  // In case the navigation aborted, look up the RFH by the Frame Tree Node
+  // ID. If there is no RFH remaining, then the frame is gone. We use the
+  // unsafe method of FindFrameByFrameTreeNodeId because we're not concerned
+  // with which process the frame lives on (we're just measuring bytes and not
+  // granting security priveleges).
+  content::RenderFrameHost* current_frame_host =
+      navigation_handle->GetWebContents()->UnsafeFindFrameByFrameTreeNodeId(

[Bryan McQuade, 2017/05/12 12:43:59]

does this only return frames for committed loads? or will it return frames if a
provisional load is in flight as well? maybe add a comment to explain.

[jkarlin, 2017/05/12 14:31:42]

Good question. It returns the committed RFH for the given frame id. But really
it's silly of me to check the previously committed frame for its frame name. So
instead, only checking frame name on committed navigations. PTAL.

+          navigation_handle->GetFrameTreeNodeId());
+  if (current_frame_host) {
+    const std::string& frame_name = current_frame_host->GetFrameName();
+    if (base::StartsWith(frame_name, "google_ads_iframe",
+                         base::CompareCase::SENSITIVE) ||
+        base::StartsWith(frame_name, "google_ads_frame",
+                         base::CompareCase::SENSITIVE)) {
+      return true;
+    }
+  }
+
+  const GURL& url = navigation_handle->GetURL();
+  return url.host_piece() == "tpc.googlesyndication.com" &&
+         base::StartsWith(url.path_piece(), "/safeframe",
+                          base::CompareCase::SENSITIVE);
 }
 
 }  // namespace
 
 AdsPageLoadMetricsObserver::AdFrameData::AdFrameData(
     FrameTreeNodeId frame_tree_node_id)
     : frame_bytes(0u),
       frame_bytes_uncached(0u),
       frame_tree_node_id(frame_tree_node_id) {}
 
@@ skipping 15 matching lines @@
 
   // The main frame is never considered an ad.
   ad_frames_data_[navigation_handle->GetFrameTreeNodeId()] = nullptr;
   ProcessOngoingNavigationResource(navigation_handle->GetFrameTreeNodeId());
   return CONTINUE_OBSERVING;
 }
 
 page_load_metrics::PageLoadMetricsObserver::ObservePolicy
 AdsPageLoadMetricsObserver::OnDidFinishSubFrameNavigation(
     content::NavigationHandle* navigation_handle) {
+  // Determine if the frame is part of an existing ad, the root of a new ad,
+  // or a non-ad frame. Once a frame is labled as an ad, it is always
+  // considered an ad, even if it navigates to a non-ad page. This function
+  // labels all of a page's frames, even those that fail to commit.
   FrameTreeNodeId frame_tree_node_id = navigation_handle->GetFrameTreeNodeId();
-
-  if (!navigation_handle->HasCommitted() ||
-      navigation_handle->IsSameDocument() || navigation_handle->IsErrorPage()) {
-    // We're not interested in tracking this navigation. In case we've seen a
-    // resource for the navigation before this message, clear it from
-    // ongoing_navigation_resources_.
-    ongoing_navigation_resources_.erase(frame_tree_node_id);
-    return CONTINUE_OBSERVING;
-  }
-
   content::RenderFrameHost* parent_frame_host =
-      navigation_handle->GetRenderFrameHost()->GetParent();
-  DCHECK(parent_frame_host);
-
-  bool top_level_subframe = !parent_frame_host->GetParent();
+      navigation_handle->GetParentFrame();
 
   const auto& id_and_data = ad_frames_data_.find(frame_tree_node_id);
   if (id_and_data != ad_frames_data_.end()) {
     // An existing subframe is navigating again.
     if (id_and_data->second) {
       // The subframe was an ad to begin with, keep tracking it as an ad.
       ProcessOngoingNavigationResource(frame_tree_node_id);
 
       if (frame_tree_node_id == id_and_data->second->frame_tree_node_id) {
         // This is the top-most frame in the ad.
         UMA_HISTOGRAM_BOOLEAN(
             "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 (top_level_subframe) {
+  } 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 (top_level_subframe && 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) {
@@ skipping 22 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 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 ProcessOngoingNavigationResource.
-    // TODO(jkarlin): Plumb the resource type through and DCHECK that the type
-    // is document.
-    auto it_and_success = ongoing_navigation_resources_.emplace(
+    // 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));
-    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.
   AdFrameData* ancestor_data = id_and_data->second;
@@ skipping 86 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);
 }