Don't log background navigations in StaleWhileRevalidateExperiment histograms

Early results from the
PageLoad.Clients.StaleWhileRevalidateExperiment.Timing2.* histograms
show that there is a fat tail of noise from background navigations.

Remove navigations that started in the background and those that were
backgrounded before the timing event took place. The method is identical
to the PageLoad.Clients.FromGWS.Timing2.* histograms.

BUG=565458
TEST=manually tested with about:histograms

Review URL: https://codereview.chromium.org/1507813002

Cr-Commit-Position: refs/heads/master@{#364366}

4 files changed, 32 insertions, 21 deletions

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 bccbd59..b5f2620 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
@@ -49,12 +49,6 @@ bool IsFromGoogleSearchResult(const GURL& url, const GURL& referrer) {
   return is_possible_search_referrer && !IsFromGoogle(url);
 }
 
-bool ShouldLogEvent(const base::TimeDelta& event,
-                    const base::TimeDelta& first_background) {
-  return !event.is_zero() &&
-         (first_background.is_zero() || event < first_background);
-}
-
 }  // namespace
 
 FromGWSPageLoadMetricsObserver::FromGWSPageLoadMetricsObserver()
@@ -69,46 +63,45 @@ void FromGWSPageLoadMetricsObserver::OnCommit(
 void FromGWSPageLoadMetricsObserver::OnComplete(
     const page_load_metrics::PageLoadTiming& timing,
     const page_load_metrics::PageLoadExtraInfo& extra_info) {
+  using page_load_metrics::EventOccurredInForeground;
+
   if (!navigation_from_gws_)
     return;
-  // Filter out navigations that started in the background.
-  if (!extra_info.started_in_foreground)
-    return;
 
-  const base::TimeDelta& first_background = extra_info.first_background_time;
-  if (ShouldLogEvent(timing.dom_content_loaded_event_start, first_background)) {
+  if (EventOccurredInForeground(timing.dom_content_loaded_event_start,
+                                extra_info)) {
     PAGE_LOAD_HISTOGRAM(
         "PageLoad.Clients.FromGWS.Timing2."
         "NavigationToDOMContentLoadedEventFired",
         timing.dom_content_loaded_event_start);
   }
-  if (ShouldLogEvent(timing.load_event_start, first_background)) {
+  if (EventOccurredInForeground(timing.load_event_start, extra_info)) {
     PAGE_LOAD_HISTOGRAM(
         "PageLoad.Clients.FromGWS.Timing2.NavigationToLoadEventFired",
         timing.load_event_start);
   }
-  if (ShouldLogEvent(timing.first_layout, first_background)) {
+  if (EventOccurredInForeground(timing.first_layout, extra_info)) {
     PAGE_LOAD_HISTOGRAM(
         "PageLoad.Clients.FromGWS.Timing2.NavigationToFirstLayout",
         timing.first_layout);
   }
-  if (ShouldLogEvent(timing.first_text_paint, first_background)) {
+  if (EventOccurredInForeground(timing.first_text_paint, extra_info)) {
     PAGE_LOAD_HISTOGRAM(
         "PageLoad.Clients.FromGWS.Timing2.NavigationToFirstTextPaint",
         timing.first_text_paint);
   }
-  if (ShouldLogEvent(timing.first_image_paint, first_background)) {
+  if (EventOccurredInForeground(timing.first_image_paint, extra_info)) {
     PAGE_LOAD_HISTOGRAM(
         "PageLoad.Clients.FromGWS.Timing2.NavigationToFirstImagePaint",
         timing.first_image_paint);
   }
-  if (ShouldLogEvent(timing.first_paint, first_background)) {
+  if (EventOccurredInForeground(timing.first_paint, extra_info)) {
     PAGE_LOAD_HISTOGRAM(
         "PageLoad.Clients.FromGWS.Timing2.NavigationToFirstPaint",
         timing.first_paint);
   }
   base::TimeDelta first_contentful_paint = GetFirstContentfulPaint(timing);
-  if (ShouldLogEvent(first_contentful_paint, first_background)) {
+  if (EventOccurredInForeground(first_contentful_paint, extra_info)) {
     PAGE_LOAD_HISTOGRAM(
         "PageLoad.Clients.FromGWS.Timing2.NavigationToFirstContentfulPaint",
         first_contentful_paint);
diff --git a/chrome/browser/page_load_metrics/observers/stale_while_revalidate_metrics_observer.cc b/chrome/browser/page_load_metrics/observers/stale_while_revalidate_metrics_observer.cc
index 58df2d1..f474d2f 100644
--- a/chrome/browser/page_load_metrics/observers/stale_while_revalidate_metrics_observer.cc
+++ b/chrome/browser/page_load_metrics/observers/stale_while_revalidate_metrics_observer.cc
@@ -23,22 +23,24 @@ void StaleWhileRevalidateMetricsObserver::OnCommit(
 void StaleWhileRevalidateMetricsObserver::OnComplete(
     const page_load_metrics::PageLoadTiming& timing,
     const page_load_metrics::PageLoadExtraInfo& extra_info) {
+  using page_load_metrics::EventOccurredInForeground;
+
   if (!is_interesting_domain_)
     return;
 
-  if (!timing.load_event_start.is_zero()) {
+  if (EventOccurredInForeground(timing.load_event_start, extra_info)) {
     PAGE_LOAD_HISTOGRAM(
         "PageLoad.Clients.StaleWhileRevalidateExperiment.Timing2."
         "NavigationToLoadEventFired",
         timing.load_event_start);
   }
-  if (!timing.first_layout.is_zero()) {
+  if (EventOccurredInForeground(timing.first_layout, extra_info)) {
     PAGE_LOAD_HISTOGRAM(
         "PageLoad.Clients.StaleWhileRevalidateExperiment.Timing2."
         "NavigationToFirstLayout",
         timing.first_layout);
   }
-  if (!timing.first_text_paint.is_zero()) {
+  if (EventOccurredInForeground(timing.first_text_paint, extra_info)) {
     PAGE_LOAD_HISTOGRAM(
         "PageLoad.Clients.StaleWhileRevalidateExperiment.Timing2."
         "NavigationToFirstTextPaint",
diff --git a/components/page_load_metrics/browser/page_load_metrics_util.cc b/components/page_load_metrics/browser/page_load_metrics_util.cc
index 9b641e5..19436f3 100644
--- a/components/page_load_metrics/browser/page_load_metrics_util.cc
+++ b/components/page_load_metrics/browser/page_load_metrics_util.cc
@@ -6,6 +6,7 @@
 
 #include <algorithm>
 
+#include "components/page_load_metrics/browser/page_load_metrics_observer.h"
 #include "components/page_load_metrics/common/page_load_timing.h"
 
 namespace page_load_metrics {
@@ -18,5 +19,11 @@ base::TimeDelta GetFirstContentfulPaint(const PageLoadTiming& timing) {
   return std::min(timing.first_text_paint, timing.first_image_paint);
 }
 
-}  // namespace page_load_metrics
+bool EventOccurredInForeground(const base::TimeDelta& event,
+                               const PageLoadExtraInfo& info) {
+  return info.started_in_foreground && !event.is_zero() &&
+         (info.first_background_time.is_zero() ||
+          event < info.first_background_time);
+}
 
+}  // namespace page_load_metrics
diff --git a/components/page_load_metrics/browser/page_load_metrics_util.h b/components/page_load_metrics/browser/page_load_metrics_util.h
index b5a5698..0865bfa 100644
--- a/components/page_load_metrics/browser/page_load_metrics_util.h
+++ b/components/page_load_metrics/browser/page_load_metrics_util.h
@@ -15,6 +15,7 @@
 
 namespace page_load_metrics {
 
+struct PageLoadExtraInfo;
 struct PageLoadTiming;
 
 // Get the time of the first 'contentful' paint. A contentful paint is a paint
@@ -22,6 +23,14 @@ struct PageLoadTiming;
 // Painting of a background color is not considered 'contentful'.
 base::TimeDelta GetFirstContentfulPaint(const PageLoadTiming& timing);
 
+// Returns false for events for which we have no timing information, and events
+// that happened on a page that had been in the background. When a page is
+// backgrounded, some events (e.g. paint) are delayed. Since these data points
+// can skew the mean, they should not be mixed with timing events that occurred
+// in the foreground.
+bool EventOccurredInForeground(const base::TimeDelta& event,
+                               const PageLoadExtraInfo& info);
+
 }  // namespace page_load_metrics
 
 #endif  // COMPONENTS_PAGE_LOAD_METRICS_BROWSER_PAGE_LOAD_METRICS_UTIL_H_