Additional heuristic user interaction attribution for page load metrics

chrome/browser/page_load_metrics/page_load_tracker.cc

Index: chrome/browser/page_load_metrics/page_load_tracker.cc
diff --git a/chrome/browser/page_load_metrics/page_load_tracker.cc b/chrome/browser/page_load_metrics/page_load_tracker.cc
index fa68a29219cbe453f9effabe0f40fb398bd64fad..3965e9897b00097268cb810e2b0e9c6140307ed8 100644
--- a/chrome/browser/page_load_metrics/page_load_tracker.cc
+++ b/chrome/browser/page_load_metrics/page_load_tracker.cc
@@ -287,6 +287,7 @@ PageLoadTracker::PageLoadTracker(
     PageLoadMetricsEmbedderInterface* embedder_interface,
     const GURL& currently_committed_url,
     content::NavigationHandle* navigation_handle,
+    UserInitiatedInfo user_initiated_info,
     int aborted_chain_size,
     int aborted_chain_size_same_url)
     : did_stop_tracking_(false),
@@ -294,12 +295,12 @@ PageLoadTracker::PageLoadTracker(
       navigation_start_(navigation_handle->NavigationStart()),
       start_url_(navigation_handle->GetURL()),
       abort_type_(ABORT_NONE),
-      abort_user_initiated_(false),
+      abort_user_initiated_info_(UserInitiatedInfo::NotUserInitiated()),
       started_in_foreground_(in_foreground),
       page_transition_(navigation_handle->GetPageTransition()),
       num_cache_requests_(0),
       num_network_requests_(0),
-      user_initiated_(IsNavigationUserInitiated(navigation_handle)),
+      user_initiated_info_(user_initiated_info),
       aborted_chain_size_(aborted_chain_size),
       aborted_chain_size_same_url_(aborted_chain_size_same_url),
       embedder_interface_(embedder_interface) {
@@ -414,9 +415,8 @@ void PageLoadTracker::WebContentsHidden() {
     // can't be certain that we were backgrounded due to a user action. For
     // example, on Android, the screen times out after a period of inactivity,
     // resulting in a non-user-initiated backgrounding.
-    const bool abort_is_user_initiated = false;
-    NotifyAbort(ABORT_BACKGROUND, abort_is_user_initiated, background_time_,
-                true);
+    NotifyAbort(ABORT_BACKGROUND, UserInitiatedInfo::NotUserInitiated(),
+                background_time_, true);
   }
   const PageLoadExtraInfo info = ComputePageLoadExtraInfo();
   INVOKE_AND_PRUNE_OBSERVERS(observers_, OnHidden, timing_, info);
@@ -440,7 +440,7 @@ void PageLoadTracker::Commit(content::NavigationHandle* navigation_handle) {
   committed_url_ = navigation_handle->GetURL();
   // Some transitions (like CLIENT_REDIRECT) are only known at commit time.
   page_transition_ = navigation_handle->GetPageTransition();
-  user_initiated_ = IsNavigationUserInitiated(navigation_handle);
+  user_initiated_info_.user_gesture = navigation_handle->HasUserGesture();
 
   INVOKE_AND_PRUNE_OBSERVERS(observers_, OnCommit, navigation_handle);
   LogAbortChainHistograms(navigation_handle);
@@ -459,6 +459,7 @@ void PageLoadTracker::Redirect(content::NavigationHandle* navigation_handle) {
 }
 
 void PageLoadTracker::OnInputEvent(const blink::WebInputEvent& event) {
+  input_tracker_.OnInputEvent(event);
   for (const auto& observer : observers_) {
     observer->OnUserInput(event);
   }
@@ -595,17 +596,20 @@ PageLoadExtraInfo PageLoadTracker::ComputePageLoadExtraInfo() {
     DCHECK(abort_time_.is_null());
   }
 
-  // abort_type_ == ABORT_NONE implies !abort_user_initiated_.
-  DCHECK(abort_type_ != ABORT_NONE || !abort_user_initiated_);
+  // abort_type_ == ABORT_NONE implies !abort_user_initiated_info_

[Charlie Harrison, 2016/12/07 15:45:13]

nit: Is !abort_user_initiated_info_ even defined? I would update the comment to
something like "implies abort_user_initiated_info_ is not user initiated" or
something.

[Bryan McQuade, 2016/12/07 16:32:00]

Nope, does not compile. This was intended to be pseudocode shorthand. Your
suggestion definitely makes this clearer, thanks!

+  DCHECK(abort_type_ != ABORT_NONE ||
+         (!abort_user_initiated_info_.browser_initiated &&
+          !abort_user_initiated_info_.user_gesture &&
+          !abort_user_initiated_info_.user_input_event));
   return PageLoadExtraInfo(
       first_background_time, first_foreground_time, started_in_foreground_,
-      user_initiated_, committed_url_, start_url_, abort_type_,
-      abort_user_initiated_, time_to_abort, num_cache_requests_,
+      user_initiated_info_, committed_url_, start_url_, abort_type_,
+      abort_user_initiated_info_, time_to_abort, num_cache_requests_,
       num_network_requests_, metadata_);
 }
 
 void PageLoadTracker::NotifyAbort(UserAbortType abort_type,
-                                  bool user_initiated,
+                                  UserInitiatedInfo user_initiated_info,
                                   base::TimeTicks timestamp,
                                   bool is_certainly_browser_timestamp) {
   DCHECK_NE(abort_type, ABORT_NONE);
@@ -613,12 +617,12 @@ void PageLoadTracker::NotifyAbort(UserAbortType abort_type,
   if (abort_type_ != ABORT_NONE)
     return;
 
-  UpdateAbortInternal(abort_type, user_initiated, timestamp,
+  UpdateAbortInternal(abort_type, user_initiated_info, timestamp,
                       is_certainly_browser_timestamp);
 }
 
 void PageLoadTracker::UpdateAbort(UserAbortType abort_type,
-                                  bool user_initiated,
+                                  UserInitiatedInfo user_initiated_info,
                                   base::TimeTicks timestamp,
                                   bool is_certainly_browser_timestamp) {
   DCHECK_NE(abort_type, ABORT_NONE);
@@ -628,7 +632,7 @@ void PageLoadTracker::UpdateAbort(UserAbortType abort_type,
   // For some aborts (e.g. navigations), the initiated timestamp can be earlier
   // than the timestamp that aborted the load. Taking the minimum gives the
   // closest user initiated time known.
-  UpdateAbortInternal(abort_type, user_initiated,
+  UpdateAbortInternal(abort_type, user_initiated_info,
                       std::min(abort_time_, timestamp),
                       is_certainly_browser_timestamp);
 }
@@ -649,7 +653,7 @@ bool PageLoadTracker::MatchesOriginalNavigation(
 }
 
 void PageLoadTracker::UpdateAbortInternal(UserAbortType abort_type,
-                                          bool user_initiated,
+                                          UserInitiatedInfo user_initiated_info,
                                           base::TimeTicks timestamp,
                                           bool is_certainly_browser_timestamp) {
   // When a provisional navigation commits, that navigation's start time is
@@ -677,7 +681,8 @@ void PageLoadTracker::UpdateAbortInternal(UserAbortType abort_type,
   // these navs may sometimes be reported as user initiated by Blink. Thus, we
   // explicitly filter these types of aborts out when deciding if the abort was
   // user initiated.
-  abort_user_initiated_ = user_initiated && abort_type != ABORT_CLIENT_REDIRECT;
+  if (abort_type != ABORT_CLIENT_REDIRECT)
+    abort_user_initiated_info_ = user_initiated_info;
 
   if (is_certainly_browser_timestamp) {
     ClampBrowserTimestampIfInterProcessTimeTickSkew(&abort_time_);