Add metric with time between background tab being foregrounded and the first paint

chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.cc

Index: chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.cc
diff --git a/chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.cc b/chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.cc
index de5dbfee5ba789709a29d469ddc0a744d3f6172e..b5ee2f1ef47264ae22b772758a63ac414e162719 100644
--- a/chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.cc
+++ b/chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.cc
@@ -88,6 +88,9 @@ const char kHistogramBackgroundBeforeCommit[] =
 const char kHistogramFailedProvisionalLoad[] =
     "PageLoad.Timing2.NavigationToFailedProvisionalLoad";
 
+const char kHistogramSwitchToForegroundToFirstPaint[] =
+    "PageLoad.Timing2.SwitchToForegroundToFirstPaint";
+
 const char kRapporMetricsNameCoarseTiming[] =
     "PageLoad.CoarseTiming.NavigationToFirstContentfulPaint";
 
@@ -125,8 +128,9 @@ void CorePageLoadMetricsObserver::OnFailedProvisionalLoad(
 void CorePageLoadMetricsObserver::RecordTimingHistograms(
     const page_load_metrics::PageLoadTiming& timing,
     const page_load_metrics::PageLoadExtraInfo& info) {
-  if (!info.first_background_time.is_zero() &&
-      !EventOccurredInForeground(timing.first_paint, info)) {
+  if (info.started_in_foreground &&
+      !info.first_background_time.is_zero() &&
+      !StartInForegroundEventInForeground(timing.first_paint, info)) {

[Charlie Harrison, 2016/02/22 16:08:32]

At this point I think this is less clear than
started_in_foreground &&
!first_background_time.is_zero() &&
first_background_time < first_paint

[Bryan McQuade, 2016/02/24 19:34:44]

agree - this function is trying to do too much and is thus quite hard to reason
about

     if (!info.time_to_commit.is_zero()) {
       PAGE_LOAD_HISTOGRAM(internal::kHistogramBackgroundBeforePaint,
                           info.first_background_time);
@@ -138,8 +142,8 @@ void CorePageLoadMetricsObserver::RecordTimingHistograms(
 
   if (failed_provisional_load_info_.error != net::OK) {
     // Ignores a background failed provisional load.
-    if (EventOccurredInForeground(failed_provisional_load_info_.interval,
-                                  info)) {
+    if (StartInForegroundEventInForeground(
+            failed_provisional_load_info_.interval, info)) {
       PAGE_LOAD_HISTOGRAM(internal::kHistogramFailedProvisionalLoad,
                           failed_provisional_load_info_.interval);
     }
@@ -150,15 +154,15 @@ void CorePageLoadMetricsObserver::RecordTimingHistograms(
   if (info.time_to_commit.is_zero() || timing.IsEmpty())
     return;
 
-  if (EventOccurredInForeground(info.time_to_commit, info)) {
+  if (StartInForegroundEventInForeground(info.time_to_commit, info)) {
     PAGE_LOAD_HISTOGRAM(internal::kHistogramCommit, info.time_to_commit);
   } else {
     PAGE_LOAD_HISTOGRAM(internal::kBackgroundHistogramCommit,
                         info.time_to_commit);
   }
   if (!timing.dom_content_loaded_event_start.is_zero()) {
-    if (EventOccurredInForeground(timing.dom_content_loaded_event_start,
-                                  info)) {
+    if (StartInForegroundEventInForeground(
+            timing.dom_content_loaded_event_start, info)) {
       PAGE_LOAD_HISTOGRAM(internal::kHistogramDomContentLoaded,
                           timing.dom_content_loaded_event_start);
     } else {
@@ -167,7 +171,7 @@ void CorePageLoadMetricsObserver::RecordTimingHistograms(
     }
   }
   if (!timing.load_event_start.is_zero()) {
-    if (EventOccurredInForeground(timing.load_event_start, info)) {
+    if (StartInForegroundEventInForeground(timing.load_event_start, info)) {
       PAGE_LOAD_HISTOGRAM(internal::kHistogramLoad, timing.load_event_start);
     } else {
       PAGE_LOAD_HISTOGRAM(internal::kBackgroundHistogramLoad,
@@ -175,7 +179,7 @@ void CorePageLoadMetricsObserver::RecordTimingHistograms(
     }
   }
   if (!timing.first_layout.is_zero()) {
-    if (EventOccurredInForeground(timing.first_layout, info)) {
+    if (StartInForegroundEventInForeground(timing.first_layout, info)) {
       PAGE_LOAD_HISTOGRAM(internal::kHistogramFirstLayout, timing.first_layout);
     } else {
       PAGE_LOAD_HISTOGRAM(internal::kBackgroundHistogramFirstLayout,
@@ -183,15 +187,21 @@ void CorePageLoadMetricsObserver::RecordTimingHistograms(
     }
   }
   if (!timing.first_paint.is_zero()) {
-    if (EventOccurredInForeground(timing.first_paint, info)) {
+    if (StartInForegroundEventInForeground(timing.first_paint, info)) {
       PAGE_LOAD_HISTOGRAM(internal::kHistogramFirstPaint, timing.first_paint);
     } else {
       PAGE_LOAD_HISTOGRAM(internal::kBackgroundHistogramFirstPaint,
                           timing.first_paint);
     }
+
+    if (StartInBackgroundEventInForeground(timing.first_paint, info)) {
+      PAGE_LOAD_HISTOGRAM(
+          internal::kHistogramSwitchToForegroundToFirstPaint,
+          timing.first_paint - info.first_foreground_time);
+    }
   }
   if (!timing.first_text_paint.is_zero()) {
-    if (EventOccurredInForeground(timing.first_text_paint, info)) {
+    if (StartInForegroundEventInForeground(timing.first_text_paint, info)) {
       PAGE_LOAD_HISTOGRAM(internal::kHistogramFirstTextPaint,
                           timing.first_text_paint);
     } else {
@@ -200,7 +210,7 @@ void CorePageLoadMetricsObserver::RecordTimingHistograms(
     }
   }
   if (!timing.first_image_paint.is_zero()) {
-    if (EventOccurredInForeground(timing.first_image_paint, info)) {
+    if (StartInForegroundEventInForeground(timing.first_image_paint, info)) {
       PAGE_LOAD_HISTOGRAM(internal::kHistogramFirstImagePaint,
                           timing.first_image_paint);
     } else {
@@ -209,7 +219,8 @@ void CorePageLoadMetricsObserver::RecordTimingHistograms(
     }
   }
   if (!timing.first_contentful_paint.is_zero()) {
-    if (EventOccurredInForeground(timing.first_contentful_paint, info)) {
+    if (StartInForegroundEventInForeground(timing.first_contentful_paint,
+                                           info)) {
       PAGE_LOAD_HISTOGRAM(internal::kHistogramFirstContentfulPaint,
                           timing.first_contentful_paint);
       // Bucket these histograms into high/low resolution clock systems. This
@@ -229,12 +240,15 @@ void CorePageLoadMetricsObserver::RecordTimingHistograms(
 
   // Log time to first foreground / time to first background. Log counts that we
   // started a relevant page load in the foreground / background.
-  if (!info.first_background_time.is_zero())
-    PAGE_LOAD_HISTOGRAM(internal::kHistogramFirstBackground,
-                        info.first_background_time);
-  else if (!info.first_foreground_time.is_zero())
-    PAGE_LOAD_HISTOGRAM(internal::kHistogramFirstForeground,
-                        info.first_foreground_time);
+  if (info.started_in_foreground) {

[Charlie Harrison, 2016/02/22 16:08:32]

Thanks for adding cleaning this up.

+    if (!info.first_background_time.is_zero())
+      PAGE_LOAD_HISTOGRAM(internal::kHistogramFirstBackground,
+                          info.first_background_time);
+  } else {
+    if (!info.first_foreground_time.is_zero())
+      PAGE_LOAD_HISTOGRAM(internal::kHistogramFirstForeground,
+                          info.first_foreground_time);
+  }
 }
 
 void CorePageLoadMetricsObserver::RecordRappor(
@@ -253,7 +267,8 @@ void CorePageLoadMetricsObserver::RecordRappor(
     return;
   DCHECK(!info.committed_url.is_empty());
   // Log the eTLD+1 of sites that show poor loading performance.
-  if (!EventOccurredInForeground(timing.first_contentful_paint, info)) {
+  if (!StartInForegroundEventInForeground(timing.first_contentful_paint,
+                                          info)) {
     return;
   }
   scoped_ptr<rappor::Sample> sample =