Improve tracking of user initiated page loads.

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 06beda69a78a05cc10b8b946f221efe066c9d09d..fce6cb090ce59254d26a9cd5ccd7250f0aa1c696 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
@@ -65,13 +65,22 @@ const char kHistogramAbortForwardBackUserInitiated[] =
     "PageLoad.AbortTiming.ForwardBackNavigation.BeforeCommit.UserInitiated";
 const char kHistogramAbortReloadUserInitiated[] =
     "PageLoad.AbortTiming.Reload.BeforeCommit.UserInitiated";
+const char kHistogramAbortStopUserInitiated[] =
+    "PageLoad.AbortTiming.Stop.BeforeCommit.UserInitiated";
+const char kHistogramAbortCloseUserInitiated[] =
+    "PageLoad.AbortTiming.Close.BeforeCommit.UserInitiated";
+const char kHistogramAbortBackgroundUserInitiated[] =
+    "PageLoad.AbortTiming.Background.BeforeCommit.UserInitiated";
+const char kHistogramAbortOtherUserInitiated[] =
+    "PageLoad.AbortTiming.Other.BeforeCommit.UserInitiated";
 
 }  // namespace internal
 
 namespace {
 
 bool IsAbortUserInitiated(const page_load_metrics::PageLoadExtraInfo& info) {

[Bryan McQuade, 2016/11/10 15:35:41]

thinking about this more, do we care about whether this nav was user initiated
when deciding if the abort was user initiated? for instance, if a web page
initiates a non-user-initiated navigation to some URL, and I then click the stop
button for that nav, I think I'd consider that a user initiated abort, even
though the nav being aborted wasn't user initiated. What do you think? If this
makes sense to you then I think we should remove the info.user_gesture check
from this method, and look only at the value of info.abort_user_initiated here.

[Charlie Harrison, 2016/11/10 20:33:04]

I'm on the fence about this. I feel like checking that the abortee was user
initiated ensures that this metric is all about the user giving up on what they
were trying to do. If the site is trying to navigate on its own I don't think
the user is necessarily aware they are "giving up" on anything.

[Bryan McQuade, 2016/11/10 20:58:18]

That seems reasonable to me. I could see it being potentially misleading to
consumers of the data, but OTOH once we have high quality user-initited
navigation signals, maybe all of our metrics should only track user initiated
navs by default, and we can break out the non-user-initiated navs into a
separate suffix like we do with .Background. Something to consider as we work on
improving the user gesture signals.

-  return info.abort_user_initiated && info.user_gesture;
+  return info.user_initiated &&
+         info.abort_type != UserAbortType::ABORT_CLIENT_REDIRECT;
 }
 
 void RecordAbortBeforeCommit(UserAbortType abort_type,
@@ -109,18 +118,34 @@ void RecordAbortBeforeCommit(UserAbortType abort_type,
     case UserAbortType::ABORT_STOP:
       PAGE_LOAD_HISTOGRAM(internal::kHistogramAbortStopBeforeCommit,
                           time_to_abort);
+      if (user_initiated) {
+        PAGE_LOAD_HISTOGRAM(internal::kHistogramAbortStopUserInitiated,
+                            time_to_abort);
+      }
       return;
     case UserAbortType::ABORT_CLOSE:
       PAGE_LOAD_HISTOGRAM(internal::kHistogramAbortCloseBeforeCommit,
                           time_to_abort);
+      if (user_initiated) {
+        PAGE_LOAD_HISTOGRAM(internal::kHistogramAbortCloseUserInitiated,
+                            time_to_abort);
+      }
       return;
     case UserAbortType::ABORT_BACKGROUND:
       PAGE_LOAD_HISTOGRAM(internal::kHistogramAbortBackgroundBeforeCommit,
                           time_to_abort);
+      if (user_initiated) {
+        PAGE_LOAD_HISTOGRAM(internal::kHistogramAbortBackgroundUserInitiated,
+                            time_to_abort);
+      }
       return;
     case UserAbortType::ABORT_OTHER:
       PAGE_LOAD_HISTOGRAM(internal::kHistogramAbortOtherBeforeCommit,
                           time_to_abort);
+      if (user_initiated) {
+        PAGE_LOAD_HISTOGRAM(internal::kHistogramAbortOtherUserInitiated,
+                            time_to_abort);
+      }
       return;
     case UserAbortType::ABORT_NONE:
     case UserAbortType::ABORT_LAST_ENTRY: