Generalize the reload PLMO to support other transition types.

chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.cc

Index: chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.cc
diff --git a/chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.cc b/chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.cc
index 365e64360349be42c935ce1dd3e07832083130ad..354e50651d0189f1aa531936d96a5389cf84731f 100644
--- a/chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.cc
+++ b/chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.cc
@@ -12,6 +12,7 @@
 #include "components/page_load_metrics/browser/page_load_metrics_util.h"
 #include "components/rappor/rappor_service.h"
 #include "components/rappor/rappor_utils.h"
+#include "ui/base/page_transition_types.h"
 
 namespace {
 
@@ -38,6 +39,29 @@ uint64_t RapporHistogramBucketIndex(base::TimeDelta time) {
   return 5;
 }
 
+// TODO(bmcquade): If other observers want to log histograms based on load type,
+// promote this enum to page_load_metrics_observer.h.
+enum PageLoadType {
+  LOAD_TYPE_NONE = 0,
+  LOAD_TYPE_RELOAD,
+  LOAD_TYPE_FORWARD_BACK,
+  LOAD_TYPE_NEW_NAVIGATION
+};
+
+PageLoadType GetPageLoadType(ui::PageTransition transition) {
+  if (ui::PageTransitionCoreTypeIs(transition, ui::PAGE_TRANSITION_RELOAD)) {
+    return LOAD_TYPE_RELOAD;
+  }
+  if (transition & ui::PAGE_TRANSITION_FORWARD_BACK) {

[Bryan McQuade, 2016/06/27 11:46:44]

regarding your comment about incorrectly accounting for a forward back as a
reload, i wonder, if we put this test first, before the reload test, if that
resolves the issue. forward/back is a modifier on the core type, so my guess is
that when you go back to a reloaded page, you get reload&fowrard_back. if we
check for the forward/back modifier first, we should handle that case correctly.

it's also arguable that going back to a page that was reloaded should no longer
be reported as a reload, but that's something we can at least work around if we
check forward/back first.

+    return LOAD_TYPE_FORWARD_BACK;
+  }
+  if (ui::PageTransitionIsNewNavigation(transition)) {
+    return LOAD_TYPE_NEW_NAVIGATION;
+  }
+  NOTREACHED() << "Received PageTransition with no matching PageLoadType.";
+  return LOAD_TYPE_NONE;
+}
+
 }  // namespace
 
 namespace internal {
@@ -153,6 +177,19 @@ const char
         "PageLoad.ParseTiming.ParseBlockedOnScriptLoadFromDocumentWrite."
         "Background";
 
+const char kHistogramLoadTypeFirstContentfulPaintReload[] =
+    "PageLoad.PaintTiming.NavigationToFirstContentfulPaint.LoadType."
+    "Reload";
+const char kHistogramLoadTypeFirstContentfulPaintReloadByGesture[] =
+    "PageLoad.PaintTiming.NavigationToFirstContentfulPaint.LoadType."
+    "Reload.UserGesture";
+const char kHistogramLoadTypeFirstContentfulPaintForwardBack[] =
+    "PageLoad.PaintTiming.NavigationToFirstContentfulPaint.LoadType."
+    "ForwardBackNavigation";
+const char kHistogramLoadTypeFirstContentfulPaintNewNavigation[] =
+    "PageLoad.PaintTiming.NavigationToFirstContentfulPaint.LoadType."
+    "NewNavigation";
+
 const char kHistogramFirstContentfulPaintHigh[] =
     "PageLoad.Timing2.NavigationToFirstContentfulPaint.HighResolutionClock";
 const char kHistogramFirstContentfulPaintLow[] =
@@ -180,10 +217,18 @@ const char kRapporMetricsNameCoarseTiming[] =
 
 }  // namespace internal
 
-CorePageLoadMetricsObserver::CorePageLoadMetricsObserver() {}
+CorePageLoadMetricsObserver::CorePageLoadMetricsObserver()
+    : transition_(ui::PAGE_TRANSITION_LINK),
+      initiated_by_user_gesture_(false) {}
 
 CorePageLoadMetricsObserver::~CorePageLoadMetricsObserver() {}
 
+void CorePageLoadMetricsObserver::OnCommit(
+    content::NavigationHandle* navigation_handle) {
+  transition_ = navigation_handle->GetPageTransition();
+  initiated_by_user_gesture_ = navigation_handle->HasUserGesture();
+}
+
 void CorePageLoadMetricsObserver::OnDomContentLoadedEventStart(
     const page_load_metrics::PageLoadTiming& timing,
     const page_load_metrics::PageLoadExtraInfo& info) {
@@ -267,6 +312,32 @@ void CorePageLoadMetricsObserver::OnFirstContentfulPaint(
     PAGE_LOAD_HISTOGRAM(
         internal::kHistogramParseStartToFirstContentfulPaintImmediate,
         timing.first_contentful_paint - timing.parse_start);
+
+    switch (GetPageLoadType(transition_)) {
+      case LOAD_TYPE_RELOAD:
+        PAGE_LOAD_HISTOGRAM(
+            internal::kHistogramLoadTypeFirstContentfulPaintReload,
+            timing.first_contentful_paint);
+        if (initiated_by_user_gesture_) {
+          PAGE_LOAD_HISTOGRAM(
+              internal::kHistogramLoadTypeFirstContentfulPaintReloadByGesture,
+              timing.first_contentful_paint);
+        }
+        break;
+      case LOAD_TYPE_FORWARD_BACK:
+        PAGE_LOAD_HISTOGRAM(
+            internal::kHistogramLoadTypeFirstContentfulPaintForwardBack,
+            timing.first_contentful_paint);
+        break;
+      case LOAD_TYPE_NEW_NAVIGATION:
+        PAGE_LOAD_HISTOGRAM(
+            internal::kHistogramLoadTypeFirstContentfulPaintNewNavigation,
+            timing.first_contentful_paint);
+        break;
+      case LOAD_TYPE_NONE:
+        NOTREACHED();
+        break;
+    }
   } else {
     PAGE_LOAD_HISTOGRAM(
         internal::kBackgroundHistogramFirstContentfulPaintImmediate,