Log First User Interaction in Page Load Metrics

chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer.cc

Index: chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer.cc
diff --git a/chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer.cc b/chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer.cc
index 612e5cca70428eb8a38deaebfdff20fecf98aa2e..9d91f340ac13501f319be30d07ee9072911a001a 100644
--- a/chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer.cc
+++ b/chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer.cc
@@ -44,32 +44,46 @@ const char kHistogramFromGWSAbortUnknownNavigationBeforeCommit[] =
 const char kHistogramFromGWSAbortNewNavigationBeforePaint[] =
     "PageLoad.Clients.FromGoogleSearch.AbortTiming.NewNavigation.AfterCommit."
     "BeforePaint";
+const char kHistogramFromGWSAbortNewNavigationBeforeInteraction[] =
+    "PageLoad.Clients.FromGoogleSearch.AbortTiming.NewNavigation.AfterPaint."
+    "BeforeInteraction";
 const char kHistogramFromGWSAbortStopBeforeCommit[] =
     "PageLoad.Clients.FromGoogleSearch.AbortTiming.Stop.BeforeCommit";
 const char kHistogramFromGWSAbortStopBeforePaint[] =
     "PageLoad.Clients.FromGoogleSearch.AbortTiming.Stop.AfterCommit."
     "BeforePaint";
+const char kHistogramFromGWSAbortStopBeforeInteraction[] =
+    "PageLoad.Clients.FromGoogleSearch.AbortTiming.Stop.AfterPaint."
+    "BeforeInteraction";
 const char kHistogramFromGWSAbortCloseBeforeCommit[] =
     "PageLoad.Clients.FromGoogleSearch.AbortTiming.Close.BeforeCommit";
 const char kHistogramFromGWSAbortCloseBeforePaint[] =
     "PageLoad.Clients.FromGoogleSearch.AbortTiming.Close.AfterCommit."
     "BeforePaint";
-
+const char kHistogramFromGWSAbortCloseBeforeInteraction[] =
+    "PageLoad.Clients.FromGoogleSearch.AbortTiming.Close.AfterPaint."
+    "BeforeInteraction";
 const char kHistogramFromGWSAbortOtherBeforeCommit[] =
     "PageLoad.Clients.FromGoogleSearch.AbortTiming.Other.BeforeCommit";
 const char kHistogramFromGWSAbortReloadBeforePaint[] =
     "PageLoad.Clients.FromGoogleSearch.AbortTiming.Reload.AfterCommit."
     "BeforePaint";
+const char kHistogramFromGWSAbortReloadBeforeInteraction[] =
+    "PageLoad.Clients.FromGoogleSearch.AbortTiming.Reload.AfterPaint."
+    "BeforeInteraction";
 const char kHistogramFromGWSAbortForwardBackBeforePaint[] =
     "PageLoad.Clients.FromGoogleSearch.AbortTiming.ForwardBackNavigation."
     "AfterCommit.BeforePaint";
+const char kHistogramFromGWSAbortForwardBackBeforeInteraction[] =
+    "PageLoad.Clients.FromGoogleSearch.AbortTiming.ForwardBackNavigation."
+    "AfterPaint.BeforeInteraction";
 
 }  // namespace internal
 
 namespace {
 
 void LogCommittedAbortsBeforePaint(UserAbortType abort_type,
-                        base::TimeDelta time_to_abort) {
+                                   base::TimeDelta time_to_abort) {
   switch (abort_type) {
     case UserAbortType::ABORT_STOP:
       PAGE_LOAD_HISTOGRAM(internal::kHistogramFromGWSAbortStopBeforePaint,
@@ -101,6 +115,41 @@ void LogCommittedAbortsBeforePaint(UserAbortType abort_type,
   }
 }
 
+void LogAbortsAfterPaintBeforeInteraction(UserAbortType abort_type,
+                                          base::TimeDelta time_to_abort) {
+  switch (abort_type) {
+    case UserAbortType::ABORT_STOP:
+      PAGE_LOAD_HISTOGRAM(internal::kHistogramFromGWSAbortStopBeforeInteraction,
+                          time_to_abort);
+      break;
+    case UserAbortType::ABORT_CLOSE:
+      PAGE_LOAD_HISTOGRAM(
+          internal::kHistogramFromGWSAbortCloseBeforeInteraction,
+          time_to_abort);
+      break;
+    case UserAbortType::ABORT_NEW_NAVIGATION:
+      PAGE_LOAD_HISTOGRAM(
+          internal::kHistogramFromGWSAbortNewNavigationBeforeInteraction,
+          time_to_abort);
+      break;
+    case UserAbortType::ABORT_RELOAD:
+      PAGE_LOAD_HISTOGRAM(
+          internal::kHistogramFromGWSAbortReloadBeforeInteraction,
+          time_to_abort);
+      break;
+    case UserAbortType::ABORT_FORWARD_BACK:
+      PAGE_LOAD_HISTOGRAM(
+          internal::kHistogramFromGWSAbortForwardBackBeforeInteraction,
+          time_to_abort);
+      break;
+    default:
+      // These should only be logged for provisional aborts.
+      DCHECK_NE(abort_type, UserAbortType::ABORT_OTHER);
+      DCHECK_NE(abort_type, UserAbortType::ABORT_UNKNOWN_NAVIGATION);
+      break;
+  }
+}
+
 void LogProvisionalAborts(UserAbortType abort_type,
                           base::TimeDelta time_to_abort) {
   switch (abort_type) {
@@ -329,6 +378,8 @@ void FromGWSPageLoadMetricsObserver::OnCommit(
                                    ui::PAGE_TRANSITION_LINK) &&
       ui::PageTransitionIsNewNavigation(
           navigation_handle->GetPageTransition()));
+
+  logger_.SetNavigationStart(navigation_handle->NavigationStart());
 }
 
 void FromGWSPageLoadMetricsObserver::OnDomContentLoadedEventStart(
@@ -385,6 +436,11 @@ void FromGWSPageLoadMetricsObserver::OnComplete(
   logger_.OnComplete(timing, extra_info);
 }
 
+void FromGWSPageLoadMetricsObserver::OnUserInput(
+    const blink::WebInputEvent& event) {
+  logger_.OnUserInput(event);
+}
+
 void FromGWSPageLoadMetricsLogger::OnComplete(
     const page_load_metrics::PageLoadTiming& timing,
     const page_load_metrics::PageLoadExtraInfo& extra_info) {
@@ -405,8 +461,30 @@ void FromGWSPageLoadMetricsLogger::OnComplete(
     bool aborted_before_paint =
         aborted_in_foreground && !timing.IsEmpty() &&
         (timing.first_paint.is_zero() || timing.first_paint >= time_to_abort);
-    if (aborted_before_paint)
+    if (aborted_before_paint) {
       LogCommittedAbortsBeforePaint(abort_type, time_to_abort);
+    } else {
+      bool aborted_before_interaction =
+          first_user_interaction_after_paint_.is_zero();

[Bryan McQuade, 2016/05/27 20:06:35]

like for aborted_before_paint, it think we want to check that either first
interaction after paint is zero, or it is greater than or equal to
time_to_abort.

+      if (!aborted_before_interaction) {
+        base::TimeDelta skipped_user_interaction_time =
+            first_user_interaction_after_paint_;
+        // For the case the abort is a reload or forward_back. Since pull to
+        // reload / forward_back is the most common user case such aborts being
+        // triggered, add a sanitization threshold here: if the first user
+        // interaction are received before a reload / forward_back in a very
+        // short time, treat the interaction as a gesture to perform the abort.
+        if (abort_type == UserAbortType::ABORT_RELOAD ||
+            abort_type == UserAbortType::ABORT_FORWARD_BACK) {
+          skipped_user_interaction_time +=
+              base::TimeDelta::FromMilliseconds(1000);
+        }
+        aborted_before_interaction =
+            skipped_user_interaction_time > time_to_abort;
+      }
+      if (aborted_before_interaction)
+        LogAbortsAfterPaintBeforeInteraction(abort_type, time_to_abort);
+    }
   } else {
     if (aborted_in_foreground)
       LogProvisionalAborts(abort_type, time_to_abort);
@@ -487,6 +565,7 @@ void FromGWSPageLoadMetricsLogger::OnFirstPaint(
     PAGE_LOAD_HISTOGRAM(internal::kHistogramFromGWSFirstPaint,
                         timing.first_paint);
   }
+  first_paint_triggered_ = true;
 }
 
 void FromGWSPageLoadMetricsLogger::OnFirstTextPaint(
@@ -544,3 +623,10 @@ void FromGWSPageLoadMetricsLogger::OnParseStop(
                         timing.parse_stop - timing.parse_start);
   }
 }
+
+void FromGWSPageLoadMetricsLogger::OnUserInput(
+    const blink::WebInputEvent& event) {
+  if (first_paint_triggered_ && first_user_interaction_after_paint_.is_zero())

[Bryan McQuade, 2016/05/27 20:06:35]

just to be thorough let's also add a check for 
&& !navigation_start_.is_null()

or if you think this should not be possible, inside the if you can
DCHECK(!navigation_start_.is_null());

[mushan1, 2016/05/28 03:58:32]

Added the DCHECK.

+    first_user_interaction_after_paint_ =
+        base::TimeTicks::Now() - navigation_start_;
+}