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..46c9ead814bca2d93febdf8a55df39b768d9e716 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) {
@@ -385,6 +434,12 @@ void FromGWSPageLoadMetricsObserver::OnComplete(
   logger_.OnComplete(timing, extra_info);
 }
 
+void FromGWSPageLoadMetricsObserver::OnUserInput(
+    const blink::WebInputEvent& event,
+    base::TimeDelta time_to_input) {
+  logger_.OnUserInput(event, time_to_input);
+}
+
 void FromGWSPageLoadMetricsLogger::OnComplete(
     const page_load_metrics::PageLoadTiming& timing,
     const page_load_metrics::PageLoadExtraInfo& extra_info) {
@@ -405,8 +460,29 @@ 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_time_.is_zero();
+      if (!aborted_before_interaction) {
+        base::TimeDelta skipped_user_interaction_time =
+            first_user_interaction_time_;
+        // 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 +=

[Charlie Harrison, 2016/05/27 18:14:34]

Can you add a rationale for the magic number 1000? Maybe it would be a good idea
to manually test the time it takes to complete a pull-to-refresh / forward-back
action. 

[mushan1, 2016/05/27 18:51:24]

I use 1000 here because it takes definitely less than 1000ms to perform a
pull-to-reload/forward_back. Also 1000ms is a very short time for a user to
consume any content just revealed by his interaction. So it seems to be a good
number as a baseline.

[Charlie Harrison, 2016/05/27 18:58:24]

Fair enough. It might be good to include some of this in the comment. Maybe
append "1000ms is too short a time for a user to to consume any content revealed
by the interaction."

[mushan1, 2016/05/27 19:38:11]

Done.

+              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 +563,7 @@ void FromGWSPageLoadMetricsLogger::OnFirstPaint(
     PAGE_LOAD_HISTOGRAM(internal::kHistogramFromGWSFirstPaint,
                         timing.first_paint);
   }
+  first_paint_triggered_ = true;
 }
 
 void FromGWSPageLoadMetricsLogger::OnFirstTextPaint(
@@ -544,3 +621,10 @@ void FromGWSPageLoadMetricsLogger::OnParseStop(
                         timing.parse_stop - timing.parse_start);
   }
 }
+
+void FromGWSPageLoadMetricsLogger::OnUserInput(
+    const blink::WebInputEvent& event,
+    base::TimeDelta time_to_input) {
+  if (first_paint_triggered_ && first_user_interaction_time_.is_zero())
+    first_user_interaction_time_ = time_to_input;
+}