Move the user interaction policy for FirstMeaningfulPaint UMA into renderer

chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.h"
 
 #include <stddef.h>
 #include <stdint.h>
 #include <memory>
 #include <utility>
@@ skipping 86 matching lines @@
 const char kHistogramFirstImagePaint[] =
     "PageLoad.PaintTiming.NavigationToFirstImagePaint";
 const char kBackgroundHistogramFirstImagePaint[] =
     "PageLoad.PaintTiming.NavigationToFirstImagePaint.Background";
 const char kHistogramFirstContentfulPaint[] =
     "PageLoad.PaintTiming.NavigationToFirstContentfulPaint";
 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[] =
     "PageLoad.PaintTiming.ParseStartToFirstContentfulPaint";
 const char kBackgroundHistogramParseStartToFirstContentfulPaint[] =
     "PageLoad.PaintTiming.ParseStartToFirstContentfulPaint.Background";
 const char kHistogramParseStart[] =
     "PageLoad.ParseTiming.NavigationToParseStart";
 const char kBackgroundHistogramParseStart[] =
     "PageLoad.ParseTiming.NavigationToParseStart.Background";
@@ skipping 77 matching lines @@
     "PageLoad.CoarseTiming.NavigationToFirstContentfulPaint";
 
 const char kRapporMetricsNameFirstMeaningfulPaintNotRecorded[] =
     "PageLoad.Experimental.PaintTiming.FirstMeaningfulPaintNotRecorded";
 
 const char kHistogramFirstContentfulPaintUserInitiated[] =
     "PageLoad.PaintTiming.NavigationToFirstContentfulPaint.UserInitiated";
 
 const char kHistogramFirstMeaningfulPaintStatus[] =
     "PageLoad.Experimental.PaintTiming.FirstMeaningfulPaintStatus";
-const char kHistogramFirstMeaningfulPaintSignalStatus2[] =
-    "PageLoad.Experimental.PaintTiming.FirstMeaningfulPaintSignalStatus2";
 
 const char kHistogramFirstNonScrollInputAfterFirstPaint[] =
     "PageLoad.InputTiming.NavigationToFirstNonScroll.AfterPaint";
 const char kHistogramFirstScrollInputAfterFirstPaint[] =
     "PageLoad.InputTiming.NavigationToFirstScroll.AfterPaint";
 
 const char kHistogramTotalBytes[] = "PageLoad.Experimental.Bytes.Total";
 const char kHistogramNetworkBytes[] = "PageLoad.Experimental.Bytes.Network";
 const char kHistogramCacheBytes[] = "PageLoad.Experimental.Bytes.Cache";
 
@@ skipping 245 matching lines @@
         timing.paint_timing.first_contentful_paint.value() -
             timing.parse_timing.parse_start.value());
   }
 }
 
 void CorePageLoadMetricsObserver::OnFirstMeaningfulPaintInMainFrameDocument(
     const page_load_metrics::PageLoadTiming& timing,
     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);
   }
 }
 
 void CorePageLoadMetricsObserver::OnParseStart(
     const page_load_metrics::PageLoadTiming& timing,
     const page_load_metrics::PageLoadExtraInfo& info) {
   if (WasStartedInForegroundOptionalEventInForeground(
           timing.parse_timing.parse_start, info)) {
     PAGE_LOAD_HISTOGRAM(internal::kHistogramParseStart,
                         timing.parse_timing.parse_start.value());
@@ skipping 125 matching lines @@
   }
   // Provide an empty PageLoadTiming, since we don't have any timing metrics
   // for failed provisional loads.
   RecordForegroundDurationHistograms(page_load_metrics::PageLoadTiming(),
                                      extra_info, base::TimeTicks());
 }
 
 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;
 
   if (!received_non_scroll_input_after_first_paint_) {
     if (event.GetType() == blink::WebInputEvent::kGestureTap ||
         event.GetType() == blink::WebInputEvent::kMouseUp) {
       received_non_scroll_input_after_first_paint_ = true;
       if (now.is_null())
         now = base::TimeTicks::Now();
@@ skipping 35 matching lines @@
   }
 
   if (timing.paint_timing.first_paint &&
       !timing.paint_timing.first_meaningful_paint) {
     RecordFirstMeaningfulPaintStatus(
         timing.paint_timing.first_contentful_paint
             ? internal::FIRST_MEANINGFUL_PAINT_DID_NOT_REACH_NETWORK_STABLE
             : 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(
     const page_load_metrics::PageLoadTiming& timing,
     const page_load_metrics::PageLoadExtraInfo& info,
     base::TimeTicks app_background_time) {
   base::Optional<base::TimeDelta> foreground_duration =
       GetInitialForegroundDuration(info, app_background_time);
   if (!foreground_duration)
     return;
@@ skipping 110 matching lines @@
   }
 
   // Log the eTLD+1 of sites that did not report first meaningful paint.
   if (timing.paint_timing.first_paint &&
       !timing.paint_timing.first_meaningful_paint) {
     rappor::SampleDomainAndRegistryFromGURL(
         rappor_service,
         internal::kRapporMetricsNameFirstMeaningfulPaintNotRecorded, info.url);
   }
 }