Log First User Interaction in Page Load Metrics

components/page_load_metrics/browser/metrics_web_contents_observer.cc

Index: components/page_load_metrics/browser/metrics_web_contents_observer.cc
diff --git a/components/page_load_metrics/browser/metrics_web_contents_observer.cc b/components/page_load_metrics/browser/metrics_web_contents_observer.cc
index 0f40c4ba17a3c29ea60e852e49da36ff7a357de7..93a41487692bfd842e6abe7dc47b5ee630dadb29 100644
--- a/components/page_load_metrics/browser/metrics_web_contents_observer.cc
+++ b/components/page_load_metrics/browser/metrics_web_contents_observer.cc
@@ -365,6 +365,17 @@ void PageLoadTracker::Redirect(content::NavigationHandle* navigation_handle) {
   }
 }
 
+void PageLoadTracker::UserInteraction(blink::WebInputEvent::Type type) {
+  // Only log the first user interaction in a given page load.
+  if (user_interaction_time_.is_null()) {

[Alexei Svitkine (slow), 2016/05/18 14:51:12]

Nit: No {}'s

+    user_interaction_time_ = base::TimeTicks::Now();
+  }
+
+  for (const auto& observer : observers_) {
+    observer->OnUserInteraction(type);
+  }
+}
+
 bool PageLoadTracker::UpdateTiming(const PageLoadTiming& new_timing,
                                    const PageLoadMetadata& new_metadata) {
   // Throw away IPCs that are not relevant to the current navigation.
@@ -411,12 +422,15 @@ void PageLoadTracker::AddObserver(
 PageLoadExtraInfo PageLoadTracker::GetPageLoadMetricsInfo() {
   base::TimeDelta first_background_time;
   base::TimeDelta first_foreground_time;
+  base::TimeDelta first_user_interaction_time;
   base::TimeDelta time_to_abort;
   base::TimeDelta time_to_commit;
   if (!background_time_.is_null())
     first_background_time = background_time_ - navigation_start_;
   if (!foreground_time_.is_null())
     first_foreground_time = foreground_time_ - navigation_start_;
+  if (!user_interaction_time_.is_null())
+    first_user_interaction_time = user_interaction_time_ - navigation_start_;
   if (abort_type_ != ABORT_NONE) {
     DCHECK_GT(abort_time_, navigation_start_);
     time_to_abort = abort_time_ - navigation_start_;
@@ -431,9 +445,9 @@ PageLoadExtraInfo PageLoadTracker::GetPageLoadMetricsInfo() {
     DCHECK(commit_time_.is_null());
   }
   return PageLoadExtraInfo(
-      first_background_time, first_foreground_time, started_in_foreground_,
-      commit_time_.is_null() ? GURL() : url_, time_to_commit, abort_type_,
-      time_to_abort, metadata_);
+      first_background_time, first_foreground_time, first_user_interaction_time,
+      started_in_foreground_, commit_time_.is_null() ? GURL() : url_,
+      time_to_commit, abort_type_, time_to_abort, metadata_);
 }
 
 void PageLoadTracker::NotifyAbort(UserAbortType abort_type,
@@ -656,6 +670,19 @@ void MetricsWebContentsObserver::NavigationStopped() {
   NotifyAbortAllLoads(ABORT_STOP);
 }
 
+void MetricsWebContentsObserver::DidGetUserInteraction(
+    const blink::WebInputEvent::Type type) {
+  // ignore browser navigation or reload which comes with type Undefined
+  if (type != blink::WebInputEvent::Type::Undefined) {

[Bryan McQuade, 2016/05/18 15:21:50]

based on the comment, do you want to ignore if type == undefined?

let's not keep the empty if followed by else and just handle the one case we are
adding code for

+  } else {

[Alexei Svitkine (slow), 2016/05/18 14:51:12]

Nit: Reverse the cond and turn the if else into just an if?

+    if (!committed_load_) {
+      RecordInternalError(ERR_USER_INTERACTION_WITH_NO_RELEVANT_LOAD);
+      return;
+    }
+    committed_load_->UserInteraction(type);
+  }
+}
+
 void MetricsWebContentsObserver::DidRedirectNavigation(
     content::NavigationHandle* navigation_handle) {
   if (!navigation_handle->IsInMainFrame())