Move the user interaction policy for FirstMeaningfulPaint UMA into renderer

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 fe5d019c5a1f82a08ab2284710d36b0653318820..52cf6a721a6a6458157bd0d0398f211834aed7c8 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
@@ -104,12 +104,6 @@ const char kBackgroundHistogramFirstContentfulPaint[] =
     "PageLoad.PaintTiming.NavigationToFirstContentfulPaint.Background";
 const char kHistogramFirstMeaningfulPaint[] =
     "PageLoad.Experimental.PaintTiming.NavigationToFirstMeaningfulPaint";
-const char kHistogramFirstMeaningfulPaintNoUserInput[] =
-    "PageLoad.Experimental.PaintTiming.NavigationToFirstMeaningfulPaint."
-    "NoUserInput";
-const char kHistogramFirstMeaningfulPaintHadUserInput[] =
-    "PageLoad.Experimental.PaintTiming.NavigationToFirstMeaningfulPaint."
-    "HadUserInput";
 const char kHistogramParseStartToFirstMeaningfulPaint[] =
     "PageLoad.Experimental.PaintTiming.ParseStartToFirstMeaningfulPaint";
 const char kHistogramParseStartToFirstContentfulPaint[] =
@@ -207,8 +201,6 @@ const char kHistogramFirstContentfulPaintUserInitiated[] =
 
 const char kHistogramFirstMeaningfulPaintStatus[] =
     "PageLoad.Experimental.PaintTiming.FirstMeaningfulPaintStatus";
-const char kHistogramFirstMeaningfulPaintSignalStatus2[] =
-    "PageLoad.Experimental.PaintTiming.FirstMeaningfulPaintSignalStatus2";
 
 const char kHistogramFirstNonScrollInputAfterFirstPaint[] =
     "PageLoad.InputTiming.NavigationToFirstNonScroll.AfterPaint";
@@ -474,27 +466,19 @@ void CorePageLoadMetricsObserver::OnFirstMeaningfulPaintInMainFrameDocument(
     const page_load_metrics::PageLoadExtraInfo& info) {
   base::TimeTicks paint = info.navigation_start +
                           timing.paint_timing.first_meaningful_paint.value();
-  if (first_user_interaction_after_first_paint_.is_null() ||
-      paint < first_user_interaction_after_first_paint_) {
-    if (WasStartedInForegroundOptionalEventInForeground(
-            timing.paint_timing.first_meaningful_paint, info)) {
-      PAGE_LOAD_HISTOGRAM(internal::kHistogramFirstMeaningfulPaint,
-                          timing.paint_timing.first_meaningful_paint.value());
-      PAGE_LOAD_HISTOGRAM(internal::kHistogramParseStartToFirstMeaningfulPaint,
-                          timing.paint_timing.first_meaningful_paint.value() -
-                              timing.parse_timing.parse_start.value());
-      PAGE_LOAD_HISTOGRAM(
-          internal::kHistogramFirstMeaningfulPaintToNetworkStable,
-          base::TimeTicks::Now() - paint);
-      RecordFirstMeaningfulPaintStatus(
-          internal::FIRST_MEANINGFUL_PAINT_RECORDED);
-    } else {
-      RecordFirstMeaningfulPaintStatus(
-          internal::FIRST_MEANINGFUL_PAINT_BACKGROUNDED);
-    }
+  if (WasStartedInForegroundOptionalEventInForeground(
+          timing.paint_timing.first_meaningful_paint, info)) {
+    PAGE_LOAD_HISTOGRAM(internal::kHistogramFirstMeaningfulPaint,
+                        timing.paint_timing.first_meaningful_paint.value());
+    PAGE_LOAD_HISTOGRAM(internal::kHistogramParseStartToFirstMeaningfulPaint,
+                        timing.paint_timing.first_meaningful_paint.value() -
+                            timing.parse_timing.parse_start.value());
+    PAGE_LOAD_HISTOGRAM(internal::kHistogramFirstMeaningfulPaintToNetworkStable,

[Bryan McQuade, 2017/05/17 12:15:16]

is this an important histogram to keep recording? if it's important we should
consider recording this time delta in the render process. the callbacks that run
in the browser process at these point aren't the exact point when these events
happened - there's buffering in the render process and then delays from IPC
delivery etc. So I'd say let's either add a comment noting that this is
approximate, and a TODO to record this time delta in the render process, or
remove this histogram.

[Kunihiko Sakamoto, 2017/05/19 08:21:31]

This histogram was useful to tune FMPDetector's network-idle logic, but we don't
have plan to tweak it further. Let's remove it.

+                        base::TimeTicks::Now() - paint);
+    RecordFirstMeaningfulPaintStatus(internal::FIRST_MEANINGFUL_PAINT_RECORDED);
   } else {
     RecordFirstMeaningfulPaintStatus(
-          internal::FIRST_MEANINGFUL_PAINT_USER_INTERACTION_BEFORE_FMP);
+        internal::FIRST_MEANINGFUL_PAINT_BACKGROUNDED);
   }
 }
 
@@ -640,13 +624,6 @@ void CorePageLoadMetricsObserver::OnFailedProvisionalLoad(
 void CorePageLoadMetricsObserver::OnUserInput(
     const blink::WebInputEvent& event) {
   base::TimeTicks now;
-  if (!first_paint_.is_null() &&
-      first_user_interaction_after_first_paint_.is_null() &&
-      event.GetType() != blink::WebInputEvent::kMouseMove) {
-    if (now.is_null())
-      now = base::TimeTicks::Now();
-    first_user_interaction_after_first_paint_ = now;
-  }
 
   if (first_paint_.is_null())
     return;
@@ -702,30 +679,6 @@ void CorePageLoadMetricsObserver::RecordTimingHistograms(
             : internal::
                   FIRST_MEANINGFUL_PAINT_DID_NOT_REACH_FIRST_CONTENTFUL_PAINT);
   }
-
-  if (timing.paint_timing.first_paint) {
-    enum FirstMeaningfulPaintSignalStatus {
-      HAD_USER_INPUT = 1 << 0,
-      NETWORK_STABLE = 1 << 1,
-      FIRST_MEANINGFUL_PAINT_SIGNAL_STATUS_LAST_ENTRY = 1 << 2
-    };
-    int signal_status =
-        (first_user_interaction_after_first_paint_.is_null() ? 0
-                                                             : HAD_USER_INPUT) +
-        (timing.paint_timing.first_meaningful_paint ? NETWORK_STABLE : 0);
-    UMA_HISTOGRAM_ENUMERATION(
-        internal::kHistogramFirstMeaningfulPaintSignalStatus2,
-        signal_status, FIRST_MEANINGFUL_PAINT_SIGNAL_STATUS_LAST_ENTRY);
-  }
-  if (timing.paint_timing.first_meaningful_paint) {
-    if (first_user_interaction_after_first_paint_.is_null()) {
-      PAGE_LOAD_HISTOGRAM(internal::kHistogramFirstMeaningfulPaintNoUserInput,
-                          timing.paint_timing.first_meaningful_paint.value());
-    } else {
-      PAGE_LOAD_HISTOGRAM(internal::kHistogramFirstMeaningfulPaintHadUserInput,
-                          timing.paint_timing.first_meaningful_paint.value());
-    }
-  }
 }
 
 void CorePageLoadMetricsObserver::RecordForegroundDurationHistograms(