Add support for counting same-document AMP loads.

chrome/browser/page_load_metrics/observers/amp_page_load_metrics_observer.cc

Index: chrome/browser/page_load_metrics/observers/amp_page_load_metrics_observer.cc
diff --git a/chrome/browser/page_load_metrics/observers/amp_page_load_metrics_observer.cc b/chrome/browser/page_load_metrics/observers/amp_page_load_metrics_observer.cc
index c53afe320cc678b7ba851026c76704361029c41b..af26625036e180d6c53c6a27b599b3a8a815a676 100644
--- a/chrome/browser/page_load_metrics/observers/amp_page_load_metrics_observer.cc
+++ b/chrome/browser/page_load_metrics/observers/amp_page_load_metrics_observer.cc
@@ -58,10 +58,22 @@ const char kAmpCacheHostSuffix[] = "cdn.ampproject.org";
         break;                                                                \
       case AMPViewType::NONE:                                                 \
         NOTREACHED();                                                         \
+      case AMPViewType::AMP_VIEW_TYPE_LAST:                                   \
         break;                                                                \
     }                                                                         \
   } while (false)
 
+GURL GetCanonicalizedSameDocumentUrl(const GURL& url) {
+  if (!url.has_ref())
+    return url;
+
+  // We're only interested in same document navigations where the full URL
+  // changes, so we ignore the 'ref' or '#fragment' portion of the URL.
+  GURL::Replacements replacements;
+  replacements.ClearRef();
+  return url.ReplaceComponents(replacements);
+}
+
 }  // namespace
 
 AMPPageLoadMetricsObserver::AMPPageLoadMetricsObserver() {}
@@ -71,14 +83,44 @@ AMPPageLoadMetricsObserver::~AMPPageLoadMetricsObserver() {}
 page_load_metrics::PageLoadMetricsObserver::ObservePolicy
 AMPPageLoadMetricsObserver::OnCommit(
     content::NavigationHandle* navigation_handle) {
-  view_type_ = GetAMPViewType(navigation_handle->GetURL());
-  return (view_type_ != AMPViewType::NONE) ? CONTINUE_OBSERVING
-                                           : STOP_OBSERVING;
+  committed_url_ = navigation_handle->GetURL();
+  view_type_ = GetAMPViewType(committed_url_);
+  return CONTINUE_OBSERVING;
+}
+
+void AMPPageLoadMetricsObserver::OnCommitSameDocumentNavigation(
+    content::NavigationHandle* navigation_handle) {
+  const GURL url = GetCanonicalizedSameDocumentUrl(navigation_handle->GetURL());
+
+  // Ignore same document navigations where the URL doesn't change.
+  if (url == last_same_document_url_)
+    return;
+  last_same_document_url_ = url;
+
+  // Ignore same document navigations that go to the committed URL, as they are
+  // unlikely to be real navigations to new content.
+  if (url == committed_url_)

[RyanSturm, 2017/05/16 22:32:10]

IIUC, this check seems odd. Specifically, if a user navigates to a page that can
show some amp views, switching back and forth between two amp views we will log
uma for each switch, but if you navigate directly to the one of the amp views
initially and switch between the two, it will only count half as many.

[Bryan McQuade, 2017/05/17 12:38:34]

Yeah, you're right - we want to ignore nav updates where the URL doesn't change,
but navigating back to the committed URL should still count as a view if it's an
amp document. Updated the logic to reflect this.

+    return;
+
+  AMPViewType same_document_view_type = GetAMPViewType(url);
+  if (same_document_view_type == AMPViewType::NONE)
+    return;
+
+  // Though we're not currently able to track page load metrics such as FCP for
+  // same-document navigations, we can count how often they happen, to better
+  // understand the relative frequency of same-document vs new-document AMP
+  // navigations.
+  UMA_HISTOGRAM_ENUMERATION(
+      std::string(kHistogramPrefix).append("SameDocumentView"),
+      same_document_view_type, AMPViewType::AMP_VIEW_TYPE_LAST);
 }
 
 void AMPPageLoadMetricsObserver::OnDomContentLoadedEventStart(
     const page_load_metrics::PageLoadTiming& timing,
     const page_load_metrics::PageLoadExtraInfo& info) {
+  if (view_type_ == AMPViewType::NONE)
+    return;
+
   if (!WasStartedInForegroundOptionalEventInForeground(
           timing.document_timing.dom_content_loaded_event_start, info)) {
     return;
@@ -91,6 +133,9 @@ void AMPPageLoadMetricsObserver::OnDomContentLoadedEventStart(
 void AMPPageLoadMetricsObserver::OnLoadEventStart(
     const page_load_metrics::PageLoadTiming& timing,
     const page_load_metrics::PageLoadExtraInfo& info) {
+  if (view_type_ == AMPViewType::NONE)
+    return;
+
   if (!WasStartedInForegroundOptionalEventInForeground(
           timing.document_timing.load_event_start, info)) {
     return;
@@ -102,6 +147,9 @@ void AMPPageLoadMetricsObserver::OnLoadEventStart(
 void AMPPageLoadMetricsObserver::OnFirstLayout(
     const page_load_metrics::PageLoadTiming& timing,
     const page_load_metrics::PageLoadExtraInfo& info) {
+  if (view_type_ == AMPViewType::NONE)
+    return;
+
   if (!WasStartedInForegroundOptionalEventInForeground(
           timing.document_timing.first_layout, info)) {
     return;
@@ -113,6 +161,9 @@ void AMPPageLoadMetricsObserver::OnFirstLayout(
 void AMPPageLoadMetricsObserver::OnFirstContentfulPaintInPage(
     const page_load_metrics::PageLoadTiming& timing,
     const page_load_metrics::PageLoadExtraInfo& info) {
+  if (view_type_ == AMPViewType::NONE)
+    return;
+
   if (!WasStartedInForegroundOptionalEventInForeground(
           timing.paint_timing.first_contentful_paint, info)) {
     return;
@@ -124,6 +175,9 @@ void AMPPageLoadMetricsObserver::OnFirstContentfulPaintInPage(
 void AMPPageLoadMetricsObserver::OnParseStart(
     const page_load_metrics::PageLoadTiming& timing,
     const page_load_metrics::PageLoadExtraInfo& info) {
+  if (view_type_ == AMPViewType::NONE)
+    return;
+
   if (!WasStartedInForegroundOptionalEventInForeground(
           timing.parse_timing.parse_start, info)) {
     return;
@@ -152,7 +206,7 @@ AMPPageLoadMetricsObserver::GetAMPViewType(const GURL& url) {
   }
 
   if (google_hostname_prefix.value() == "news" &&
-      url.path_piece() == "/news/amp") {
+      url.path_piece() == "/news/amp" && !url.query_piece().empty()) {
     return AMPViewType::GOOGLE_NEWS_AMP_VIEWER;
   }
   return AMPViewType::NONE;