Generalize abort tracking to page end state tracking

chrome/browser/page_load_metrics/observers/aborts_page_load_metrics_observer.cc

Index: chrome/browser/page_load_metrics/observers/aborts_page_load_metrics_observer.cc
diff --git a/chrome/browser/page_load_metrics/observers/aborts_page_load_metrics_observer.cc b/chrome/browser/page_load_metrics/observers/aborts_page_load_metrics_observer.cc
index bee219e8ec12371564fbbc0c8967d6e7cc1c319f..aaa848f9158aa24df7908a099843a51eabd21473 100644
--- a/chrome/browser/page_load_metrics/observers/aborts_page_load_metrics_observer.cc
+++ b/chrome/browser/page_load_metrics/observers/aborts_page_load_metrics_observer.cc
@@ -6,12 +6,10 @@
 
 #include "chrome/browser/page_load_metrics/page_load_metrics_util.h"
 
-using page_load_metrics::UserAbortType;
+using page_load_metrics::PageAbortReason;
 
 namespace internal {
 
-const char kHistogramAbortClientRedirectBeforeCommit[] =
-    "PageLoad.Experimental.AbortTiming.ClientRedirect.BeforeCommit";
 const char kHistogramAbortForwardBackBeforeCommit[] =
     "PageLoad.Experimental.AbortTiming.ForwardBackNavigation.BeforeCommit";
 const char kHistogramAbortReloadBeforeCommit[] =
@@ -27,8 +25,6 @@ const char kHistogramAbortBackgroundBeforeCommit[] =
 const char kHistogramAbortOtherBeforeCommit[] =
     "PageLoad.Experimental.AbortTiming.Other.BeforeCommit";
 
-const char kHistogramAbortClientRedirectBeforePaint[] =
-    "PageLoad.Experimental.AbortTiming.ClientRedirect.AfterCommit.BeforePaint";
 const char kHistogramAbortForwardBackBeforePaint[] =
     "PageLoad.Experimental.AbortTiming.ForwardBackNavigation.AfterCommit."
     "BeforePaint";
@@ -43,8 +39,6 @@ const char kHistogramAbortCloseBeforePaint[] =
 const char kHistogramAbortBackgroundBeforePaint[] =
     "PageLoad.Experimental.AbortTiming.Background.AfterCommit.BeforePaint";
 
-const char kHistogramAbortClientRedirectDuringParse[] =
-    "PageLoad.Experimental.AbortTiming.ClientRedirect.DuringParse";
 const char kHistogramAbortForwardBackDuringParse[] =
     "PageLoad.Experimental.AbortTiming.ForwardBackNavigation.DuringParse";
 const char kHistogramAbortReloadDuringParse[] =
@@ -63,98 +57,91 @@ const char kHistogramAbortBackgroundDuringParse[] =
 namespace {
 
 void RecordAbortBeforeCommit(
-    UserAbortType abort_type,
-    page_load_metrics::UserInitiatedInfo user_initiated_info,
-    base::TimeDelta time_to_abort) {
-  switch (abort_type) {
-    case UserAbortType::ABORT_RELOAD:
+    const page_load_metrics::PageAbortInfo& abort_info) {
+  switch (abort_info.reason) {
+    case PageAbortReason::ABORT_RELOAD:
       PAGE_LOAD_HISTOGRAM(internal::kHistogramAbortReloadBeforeCommit,
-                          time_to_abort);
-      if (user_initiated_info.user_gesture) {
+                          abort_info.time_to_abort);
+      if (abort_info.user_initiated_info.user_gesture) {
         PAGE_LOAD_HISTOGRAM(
             "PageLoad.Experimental.AbortTiming.Reload.BeforeCommit."
             "UserGesture",
-            time_to_abort);
+            abort_info.time_to_abort);
       }
-      if (user_initiated_info.user_input_event) {
+      if (abort_info.user_initiated_info.user_input_event) {
         PAGE_LOAD_HISTOGRAM(
             "PageLoad.Experimental.AbortTiming.Reload.BeforeCommit."
             "UserInputEvent",
-            time_to_abort);
+            abort_info.time_to_abort);
       }
-      if (user_initiated_info.browser_initiated) {
+      if (abort_info.user_initiated_info.browser_initiated) {
         PAGE_LOAD_HISTOGRAM(
             "PageLoad.Experimental.AbortTiming.Reload.BeforeCommit."
             "BrowserInitiated",
-            time_to_abort);
+            abort_info.time_to_abort);
       }
       return;
-    case UserAbortType::ABORT_FORWARD_BACK:
+    case PageAbortReason::ABORT_FORWARD_BACK:
       PAGE_LOAD_HISTOGRAM(internal::kHistogramAbortForwardBackBeforeCommit,
-                          time_to_abort);
-      if (user_initiated_info.user_gesture) {
+                          abort_info.time_to_abort);
+      if (abort_info.user_initiated_info.user_gesture) {
         PAGE_LOAD_HISTOGRAM(
             "PageLoad.Experimental.AbortTiming.ForwardBackNavigation."
             "BeforeCommit.UserGesture",
-            time_to_abort);
+            abort_info.time_to_abort);
       }
-      if (user_initiated_info.user_input_event) {
+      if (abort_info.user_initiated_info.user_input_event) {
         PAGE_LOAD_HISTOGRAM(
             "PageLoad.Experimental.AbortTiming.ForwardBackNavigation."
             "BeforeCommit.UserInputEvent",
-            time_to_abort);
+            abort_info.time_to_abort);
       }
-      if (user_initiated_info.browser_initiated) {
+      if (abort_info.user_initiated_info.browser_initiated) {
         PAGE_LOAD_HISTOGRAM(
             "PageLoad.Experimental.AbortTiming.ForwardBackNavigation."
             "BeforeCommit.BrowserInitiated",
-            time_to_abort);
+            abort_info.time_to_abort);
       }
       return;
-    case UserAbortType::ABORT_CLIENT_REDIRECT:
-      PAGE_LOAD_HISTOGRAM(internal::kHistogramAbortClientRedirectBeforeCommit,
-                          time_to_abort);
-      return;
-    case UserAbortType::ABORT_NEW_NAVIGATION:
+    case PageAbortReason::ABORT_NEW_NAVIGATION:
       PAGE_LOAD_HISTOGRAM(internal::kHistogramAbortNewNavigationBeforeCommit,
-                          time_to_abort);
-      if (user_initiated_info.user_gesture) {
+                          abort_info.time_to_abort);
+      if (abort_info.user_initiated_info.user_gesture) {
         PAGE_LOAD_HISTOGRAM(
             "PageLoad.Experimental.AbortTiming.NewNavigation.BeforeCommit."
             "UserGesture",
-            time_to_abort);
+            abort_info.time_to_abort);
       }
-      if (user_initiated_info.user_input_event) {
+      if (abort_info.user_initiated_info.user_input_event) {
         PAGE_LOAD_HISTOGRAM(
             "PageLoad.Experimental.AbortTiming.NewNavigation.BeforeCommit."
             "UserInputEvent",
-            time_to_abort);
+            abort_info.time_to_abort);
       }
-      if (user_initiated_info.browser_initiated) {
+      if (abort_info.user_initiated_info.browser_initiated) {
         PAGE_LOAD_HISTOGRAM(
             "PageLoad.Experimental.AbortTiming.NewNavigation.BeforeCommit."
             "BrowserInitiated",
-            time_to_abort);
+            abort_info.time_to_abort);
       }
       return;
-    case UserAbortType::ABORT_STOP:
+    case PageAbortReason::ABORT_STOP:
       PAGE_LOAD_HISTOGRAM(internal::kHistogramAbortStopBeforeCommit,
-                          time_to_abort);
+                          abort_info.time_to_abort);
       return;
-    case UserAbortType::ABORT_CLOSE:
+    case PageAbortReason::ABORT_CLOSE:
       PAGE_LOAD_HISTOGRAM(internal::kHistogramAbortCloseBeforeCommit,
-                          time_to_abort);
+                          abort_info.time_to_abort);
       return;
-    case UserAbortType::ABORT_BACKGROUND:
+    case PageAbortReason::ABORT_BACKGROUND:
       PAGE_LOAD_HISTOGRAM(internal::kHistogramAbortBackgroundBeforeCommit,
-                          time_to_abort);
+                          abort_info.time_to_abort);
       return;
-    case UserAbortType::ABORT_OTHER:
+    case PageAbortReason::ABORT_OTHER:
       PAGE_LOAD_HISTOGRAM(internal::kHistogramAbortOtherBeforeCommit,
-                          time_to_abort);
+                          abort_info.time_to_abort);
       return;
-    case UserAbortType::ABORT_NONE:
-    case UserAbortType::ABORT_LAST_ENTRY:
+    case PageAbortReason::ABORT_NONE:
       NOTREACHED();
       return;
   }
@@ -162,155 +149,142 @@ void RecordAbortBeforeCommit(
 }
 
 void RecordAbortAfterCommitBeforePaint(
-    UserAbortType abort_type,
-    page_load_metrics::UserInitiatedInfo user_initiated_info,
-    base::TimeDelta time_to_abort) {
-  switch (abort_type) {
-    case UserAbortType::ABORT_RELOAD:
+    const page_load_metrics::PageAbortInfo& abort_info) {
+  switch (abort_info.reason) {
+    case PageAbortReason::ABORT_RELOAD:
       PAGE_LOAD_HISTOGRAM(internal::kHistogramAbortReloadBeforePaint,
-                          time_to_abort);
-      if (user_initiated_info.user_gesture) {
+                          abort_info.time_to_abort);
+      if (abort_info.user_initiated_info.user_gesture) {
         PAGE_LOAD_HISTOGRAM(
             "PageLoad.Experimental.AbortTiming.Reload.AfterCommit.BeforePaint."
             "UserGesture",
-            time_to_abort);
+            abort_info.time_to_abort);
       }
-      if (user_initiated_info.user_input_event) {
+      if (abort_info.user_initiated_info.user_input_event) {
         PAGE_LOAD_HISTOGRAM(
             "PageLoad.Experimental.AbortTiming.Reload.AfterCommit.BeforePaint."
             "UserInputEvent",
-            time_to_abort);
+            abort_info.time_to_abort);
       }
-      if (user_initiated_info.browser_initiated) {
+      if (abort_info.user_initiated_info.browser_initiated) {
         PAGE_LOAD_HISTOGRAM(
             "PageLoad.Experimental.AbortTiming.Reload.AfterCommit.BeforePaint."
             "BrowserInitiated",
-            time_to_abort);
+            abort_info.time_to_abort);
       }
       return;
-    case UserAbortType::ABORT_FORWARD_BACK:
+    case PageAbortReason::ABORT_FORWARD_BACK:
       PAGE_LOAD_HISTOGRAM(internal::kHistogramAbortForwardBackBeforePaint,
-                          time_to_abort);
-      if (user_initiated_info.user_gesture) {
+                          abort_info.time_to_abort);
+      if (abort_info.user_initiated_info.user_gesture) {
         PAGE_LOAD_HISTOGRAM(
             "PageLoad.Experimental.AbortTiming.ForwardBackNavigation."
             "AfterCommit.BeforePaint.UserGesture",
-            time_to_abort);
+            abort_info.time_to_abort);
       }
-      if (user_initiated_info.user_input_event) {
+      if (abort_info.user_initiated_info.user_input_event) {
         PAGE_LOAD_HISTOGRAM(
             "PageLoad.Experimental.AbortTiming.ForwardBackNavigation."
             "AfterCommit.BeforePaint.UserInputEvent",
-            time_to_abort);
+            abort_info.time_to_abort);
       }
-      if (user_initiated_info.browser_initiated) {
+      if (abort_info.user_initiated_info.browser_initiated) {
         PAGE_LOAD_HISTOGRAM(
             "PageLoad.Experimental.AbortTiming.ForwardBackNavigation."
             "AfterCommit.BeforePaint.BrowserInitiated",
-            time_to_abort);
+            abort_info.time_to_abort);
       }
       return;
-    case UserAbortType::ABORT_CLIENT_REDIRECT:
-      PAGE_LOAD_HISTOGRAM(internal::kHistogramAbortClientRedirectBeforePaint,
-                          time_to_abort);
-      return;
-    case UserAbortType::ABORT_NEW_NAVIGATION:
+    case PageAbortReason::ABORT_NEW_NAVIGATION:
       PAGE_LOAD_HISTOGRAM(internal::kHistogramAbortNewNavigationBeforePaint,
-                          time_to_abort);
-      if (user_initiated_info.user_gesture) {
+                          abort_info.time_to_abort);
+      if (abort_info.user_initiated_info.user_gesture) {
         PAGE_LOAD_HISTOGRAM(
             "PageLoad.Experimental.AbortTiming.NewNavigation.AfterCommit."
             "BeforePaint.UserGesture",
-            time_to_abort);
+            abort_info.time_to_abort);
       }
-      if (user_initiated_info.user_input_event) {
+      if (abort_info.user_initiated_info.user_input_event) {
         PAGE_LOAD_HISTOGRAM(
             "PageLoad.Experimental.AbortTiming.NewNavigation.AfterCommit."
             "BeforePaint.UserInputEvent",
-            time_to_abort);
+            abort_info.time_to_abort);
       }
-      if (user_initiated_info.browser_initiated) {
+      if (abort_info.user_initiated_info.browser_initiated) {
         PAGE_LOAD_HISTOGRAM(
             "PageLoad.Experimental.AbortTiming.NewNavigation.AfterCommit."
             "BeforePaint.BrowserInitiated",
-            time_to_abort);
+            abort_info.time_to_abort);
       }
       return;
-    case UserAbortType::ABORT_STOP:
+    case PageAbortReason::ABORT_STOP:
       PAGE_LOAD_HISTOGRAM(internal::kHistogramAbortStopBeforePaint,
-                          time_to_abort);
+                          abort_info.time_to_abort);
       return;
-    case UserAbortType::ABORT_CLOSE:
+    case PageAbortReason::ABORT_CLOSE:
       PAGE_LOAD_HISTOGRAM(internal::kHistogramAbortCloseBeforePaint,
-                          time_to_abort);
+                          abort_info.time_to_abort);
       return;
-    case UserAbortType::ABORT_BACKGROUND:
+    case PageAbortReason::ABORT_BACKGROUND:
       PAGE_LOAD_HISTOGRAM(internal::kHistogramAbortBackgroundBeforePaint,
-                          time_to_abort);
+                          abort_info.time_to_abort);
       return;
-    case UserAbortType::ABORT_OTHER:
-      NOTREACHED() << "Received UserAbortType::ABORT_OTHER for committed load.";
+    case PageAbortReason::ABORT_OTHER:
+      NOTREACHED()
+          << "Received PageAbortReason::ABORT_OTHER for committed load.";
       return;
-    case UserAbortType::ABORT_NONE:
-    case UserAbortType::ABORT_LAST_ENTRY:
+    case PageAbortReason::ABORT_NONE:
       NOTREACHED();
       return;
   }
   NOTREACHED();
 }
 
-void RecordAbortDuringParse(UserAbortType abort_type,
-                            base::TimeDelta time_to_abort) {
-  switch (abort_type) {
-    case UserAbortType::ABORT_RELOAD:
+void RecordAbortDuringParse(
+    const page_load_metrics::PageAbortInfo& abort_info) {
+  switch (abort_info.reason) {
+    case PageAbortReason::ABORT_RELOAD:
       PAGE_LOAD_HISTOGRAM(internal::kHistogramAbortReloadDuringParse,
-                          time_to_abort);
+                          abort_info.time_to_abort);
       return;
-    case UserAbortType::ABORT_FORWARD_BACK:
+    case PageAbortReason::ABORT_FORWARD_BACK:
       PAGE_LOAD_HISTOGRAM(internal::kHistogramAbortForwardBackDuringParse,
-                          time_to_abort);
-      return;
-    case UserAbortType::ABORT_CLIENT_REDIRECT:
-      PAGE_LOAD_HISTOGRAM(internal::kHistogramAbortClientRedirectDuringParse,
-                          time_to_abort);
+                          abort_info.time_to_abort);
       return;
-    case UserAbortType::ABORT_NEW_NAVIGATION:
+    case PageAbortReason::ABORT_NEW_NAVIGATION:
       PAGE_LOAD_HISTOGRAM(internal::kHistogramAbortNewNavigationDuringParse,
-                          time_to_abort);
+                          abort_info.time_to_abort);
       return;
-    case UserAbortType::ABORT_STOP:
+    case PageAbortReason::ABORT_STOP:
       PAGE_LOAD_HISTOGRAM(internal::kHistogramAbortStopDuringParse,
-                          time_to_abort);
+                          abort_info.time_to_abort);
       return;
-    case UserAbortType::ABORT_CLOSE:
+    case PageAbortReason::ABORT_CLOSE:
       PAGE_LOAD_HISTOGRAM(internal::kHistogramAbortCloseDuringParse,
-                          time_to_abort);
+                          abort_info.time_to_abort);
       return;
-    case UserAbortType::ABORT_BACKGROUND:
+    case PageAbortReason::ABORT_BACKGROUND:
       PAGE_LOAD_HISTOGRAM(internal::kHistogramAbortBackgroundDuringParse,
-                          time_to_abort);
+                          abort_info.time_to_abort);
       return;
-    case UserAbortType::ABORT_OTHER:
-      NOTREACHED() << "Received UserAbortType::ABORT_OTHER for committed load.";
+    case PageAbortReason::ABORT_OTHER:
+      NOTREACHED()
+          << "Received PageAbortReason::ABORT_OTHER for committed load.";
       return;
-    case UserAbortType::ABORT_NONE:
-    case UserAbortType::ABORT_LAST_ENTRY:
+    case PageAbortReason::ABORT_NONE:
       NOTREACHED();
       return;
   }
   NOTREACHED();
 }
 
-bool ShouldTrackMetrics(
-    const page_load_metrics::PageLoadExtraInfo& extra_info) {
-  UserAbortType abort_type = extra_info.abort_type;
-  if (abort_type == UserAbortType::ABORT_NONE)
+bool ShouldTrackMetrics(const page_load_metrics::PageLoadExtraInfo& extra_info,
+                        const page_load_metrics::PageAbortInfo& abort_info) {
+  if (abort_info.reason == PageAbortReason::ABORT_NONE)
     return false;
 
-  DCHECK(extra_info.time_to_abort);
-

[Charlie Harrison, 2017/02/17 22:36:49]

Why remove the DCHECK?

[Bryan McQuade, 2017/02/18 00:30:15]

Ah, I changed the API a bit so time_to_abort isn't an optional field in
PageAbortInfo - it's considered an invalid field when the reason is ABORT_NONE

   // Don't log abort times if the page was backgrounded before the abort event.
-  if (!WasStartedInForegroundOptionalEventInForeground(extra_info.time_to_abort,
+  if (!WasStartedInForegroundOptionalEventInForeground(abort_info.time_to_abort,
                                                        extra_info))
     return false;
 
@@ -324,7 +298,8 @@ AbortsPageLoadMetricsObserver::AbortsPageLoadMetricsObserver() {}
 void AbortsPageLoadMetricsObserver::OnComplete(
     const page_load_metrics::PageLoadTiming& timing,
     const page_load_metrics::PageLoadExtraInfo& extra_info) {
-  if (!ShouldTrackMetrics(extra_info))
+  page_load_metrics::PageAbortInfo abort_info = GetPageAbortInfo(extra_info);
+  if (!ShouldTrackMetrics(extra_info, abort_info))
     return;
 
   // If we did not receive any timing IPCs from the render process, we can't
@@ -337,25 +312,21 @@ void AbortsPageLoadMetricsObserver::OnComplete(
   if (timing.IsEmpty())
     return;
 
-  const base::TimeDelta& time_to_abort = extra_info.time_to_abort.value();
-  if (timing.parse_start && time_to_abort >= timing.parse_start &&
-      (!timing.parse_stop || timing.parse_stop >= time_to_abort)) {
-    RecordAbortDuringParse(extra_info.abort_type, time_to_abort);
+  if (timing.parse_start && abort_info.time_to_abort >= timing.parse_start &&
+      (!timing.parse_stop || timing.parse_stop >= abort_info.time_to_abort)) {
+    RecordAbortDuringParse(abort_info);
   }
-  if (!timing.first_paint || timing.first_paint >= time_to_abort) {
-    RecordAbortAfterCommitBeforePaint(extra_info.abort_type,
-                                      extra_info.abort_user_initiated_info,
-                                      time_to_abort);
+  if (!timing.first_paint || timing.first_paint >= abort_info.time_to_abort) {
+    RecordAbortAfterCommitBeforePaint(abort_info);
   }
 }
 
 void AbortsPageLoadMetricsObserver::OnFailedProvisionalLoad(
     const page_load_metrics::FailedProvisionalLoadInfo& failed_load_info,
     const page_load_metrics::PageLoadExtraInfo& extra_info) {
-  if (!ShouldTrackMetrics(extra_info))
+  page_load_metrics::PageAbortInfo abort_info = GetPageAbortInfo(extra_info);
+  if (!ShouldTrackMetrics(extra_info, abort_info))
     return;
 
-  RecordAbortBeforeCommit(extra_info.abort_type,
-                          extra_info.abort_user_initiated_info,
-                          extra_info.time_to_abort.value());
+  RecordAbortBeforeCommit(abort_info);
 }