Add understats metrics for FCP

Add understats metrics for first contentful
paint (FCP) broken down by page scheme.
This CL adds metrics that record if FCP value
was less than certain threshold or not.

For background infomation on understats see
slide 20 of http://shortn/_d9oDk4unEX.

Bug: 883445
Change-Id: I7a657960755c055ab1f2bcdbcf6f9754e78ea8a8
Reviewed-on: https://chromium-review.googlesource.com/c/1328230
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607926}

chrome/browser/page_load_metrics/observers/scheme_page_load_metrics_observer.cc

@@ -4,8 +4,25 @@
 
 #include "chrome/browser/page_load_metrics/observers/scheme_page_load_metrics_observer.h"
 
+#include "base/metrics/histogram_functions.h"
+#include "base/stl_util.h"
 #include "chrome/browser/page_load_metrics/page_load_metrics_util.h"
 
+namespace {
+
+// Must remain synchronized with the enum of the same name in enums.xml.
+enum class PageLoadTimingUnderStat {
+  kTotal = 0,
+  kLessThan1Second = 1,
+  kLessThan2Seconds = 2,
+  kLessThan5Seconds = 3,
+  kLessThan8Seconds = 4,
+  kLessThan10Seconds = 5,
+  kMaxValue = kLessThan10Seconds
+};
+
+}  // namespace
+
 page_load_metrics::PageLoadMetricsObserver::ObservePolicy
 SchemePageLoadMetricsObserver::OnStart(
     content::NavigationHandle* navigation_handle,
@@ -49,16 +66,53 @@ void SchemePageLoadMetricsObserver::OnParseStart(
 void SchemePageLoadMetricsObserver::OnFirstContentfulPaintInPage(
     const page_load_metrics::mojom::PageLoadTiming& timing,
     const page_load_metrics::PageLoadExtraInfo& extra_info) {
+  DCHECK(extra_info.url.scheme() == url::kHttpScheme ||
+         extra_info.url.scheme() == url::kHttpsScheme);
+
+  base::TimeDelta fcp = timing.paint_timing->first_contentful_paint.value();
+
   if (extra_info.url.scheme() == url::kHttpScheme) {
     PAGE_LOAD_HISTOGRAM(
         "PageLoad.Clients.Scheme.HTTP.PaintTiming."
         "NavigationToFirstContentfulPaint",
-        timing.paint_timing->first_contentful_paint.value());
-  } else if (extra_info.url.scheme() == url::kHttpsScheme) {
+        fcp);
+
+  } else {
     PAGE_LOAD_HISTOGRAM(
         "PageLoad.Clients.Scheme.HTTPS.PaintTiming."
         "NavigationToFirstContentfulPaint",
-        timing.paint_timing->first_contentful_paint.value());
+        fcp);
+  }
+
+  static constexpr char kUnderStatHistogramHttp[] =
+      "PageLoad.Clients.Scheme.HTTP.PaintTiming.UnderStat";
+  static constexpr char kUnderStatHistogramHttps[] =
+      "PageLoad.Clients.Scheme.HTTPS.PaintTiming.UnderStat";
+
+  // Record understat metrics for the time to first contentful paint.
+  static constexpr const int kUnderStatRecordingIntervalsSeconds[] = {1, 2, 5,
+                                                                      8, 10};
+  static_assert(base::size(kUnderStatRecordingIntervalsSeconds) ==
+                    static_cast<int>(PageLoadTimingUnderStat::kMaxValue),
+                " mismatch in  array length and enum size");
+
+  // Record the total count bucket first.
+  base::UmaHistogramEnumeration(extra_info.url.scheme() == url::kHttpScheme
+                                    ? kUnderStatHistogramHttp
+                                    : kUnderStatHistogramHttps,
+                                PageLoadTimingUnderStat::kTotal);
+
+  for (size_t index = 0;
+       index < base::size(kUnderStatRecordingIntervalsSeconds); ++index) {
+    base::TimeDelta threshold(base::TimeDelta::FromSeconds(
+        kUnderStatRecordingIntervalsSeconds[index]));
+    if (fcp <= threshold) {
+      base::UmaHistogramEnumeration(
+          extra_info.url.scheme() == url::kHttpScheme
+              ? kUnderStatHistogramHttp
+              : kUnderStatHistogramHttps,
+          static_cast<PageLoadTimingUnderStat>(index + 1));
+    }
   }
 }
 

chrome/browser/page_load_metrics/observers/scheme_page_load_metrics_observer_unittest.cc

@@ -6,6 +6,7 @@
 
 #include <memory>
 
+#include "base/stl_util.h"
 #include "base/strings/string_util.h"
 #include "chrome/browser/page_load_metrics/observers/page_load_metrics_observer_test_harness.h"
 #include "chrome/browser/page_load_metrics/page_load_tracker.h"
@@ -50,31 +51,86 @@ class SchemePageLoadMetricsObserverTest
     NavigateAndCommit(GURL(scheme.append("://example.com")));
   }
 
-  int CountTotalProtocolMetricsRecorded() {
+  // Excludes understat metrics.
+  int CountTotalProtocolMetricsRecorded(const std::string& protocol) {
     int count = 0;
-
     base::HistogramTester::CountsMap counts_map =
         histogram_tester().GetTotalCountsForPrefix("PageLoad.Clients.Scheme.");
     for (const auto& entry : counts_map)
       count += entry.second;
-    return count;
+
+    int understat_count = 0;
+    base::HistogramTester::CountsMap understat_counts_map =
+        histogram_tester().GetTotalCountsForPrefix(
+            "PageLoad.Clients.Scheme." + base::ToUpperASCII(protocol) +
+            ".PaintTiming.UnderStat");
+    for (const auto& entry : understat_counts_map)
+      understat_count += entry.second;
+
+    return count - understat_count;
+  }
+
+  // Returns the value of the sample present in |histogram_name|. Should be
+  // called only if |histogram_name| contains exactly 1 sample.
+  int32_t GetRecordedMetricValue(const std::string& histogram_name) const {
+    histogram_tester().ExpectTotalCount(histogram_name, 1);
+
+    std::vector<base::Bucket> buckets =
+        histogram_tester().GetAllSamples(histogram_name);
+    for (const auto& bucket : buckets) {
+      if (bucket.count == 1) {
+        return bucket.min;
+      }
+    }
+    NOTREACHED();
+    return 0;
   }
 
   void CheckHistograms(int expected_count, const std::string& protocol) {
-    EXPECT_EQ(expected_count, CountTotalProtocolMetricsRecorded());
+    EXPECT_EQ(expected_count, CountTotalProtocolMetricsRecorded(protocol));
     if (expected_count == 0)
       return;
 
     std::string prefix = "PageLoad.Clients.Scheme.";
     prefix += base::ToUpperASCII(protocol);
 
+    std::string fcp_histogram_name(
+        prefix + ".PaintTiming.NavigationToFirstContentfulPaint");
+    std::string fcp_understat_histogram_name(prefix + ".PaintTiming.UnderStat");
+
     histogram_tester().ExpectTotalCount(
         prefix + ".ParseTiming.NavigationToParseStart", 1);
-    histogram_tester().ExpectTotalCount(
-        prefix + ".PaintTiming.NavigationToFirstContentfulPaint", 1);
+    histogram_tester().ExpectTotalCount(fcp_histogram_name, 1);
     histogram_tester().ExpectTotalCount(
         prefix + ".Experimental.PaintTiming.NavigationToFirstMeaningfulPaint",
         1);
+
+    histogram_tester().ExpectBucketCount(fcp_understat_histogram_name, 0, 1);
+
+    // Must remain synchronized with the array of the same name in
+    // scheme_page_load_metrics_observer.cc.
+    static constexpr const int kUnderStatRecordingIntervalsSeconds[] = {1, 2, 5,
+                                                                        8, 10};
+
+    base::TimeDelta recorded_fcp_value = base::TimeDelta::FromMilliseconds(
+        GetRecordedMetricValue(fcp_histogram_name));
+
+    for (size_t index = 0;
+         index < base::size(kUnderStatRecordingIntervalsSeconds); ++index) {
+      base::TimeDelta threshold(base::TimeDelta::FromSeconds(
+          kUnderStatRecordingIntervalsSeconds[index]));
+      if (recorded_fcp_value <= threshold) {
+        histogram_tester().ExpectBucketCount(fcp_understat_histogram_name,
+                                             index + 1, 1);
+      }
+    }
+
+    // Overflow bucket should be empty. This also ensures that
+    // kUnderStatRecordingIntervalsSeconds above is synchronized with the array
+    // of the same name in scheme_page_load_metrics_observer.cc.
+    histogram_tester().ExpectBucketCount(
+        fcp_understat_histogram_name,
+        base::size(kUnderStatRecordingIntervalsSeconds) + 1, 0);
   }
 
   SchemePageLoadMetricsObserver* observer_;

tools/metrics/histograms/enums.xml

@@ -38536,6 +38536,15 @@ Called by update_net_trust_anchors.py.-->
   <int value="23" label="Longest input delay less than first input delay"/>
 </enum>
 
+<enum name="PageLoadTimingUnderStat">
+  <int value="0" label="Total count of metric values"/>
+  <int value="1" label="Metric value was less than 1 seconds"/>
+  <int value="2" label="Metric value was less than 2 seconds"/>
+  <int value="3" label="Metric value was less than 5 seconds"/>
+  <int value="4" label="Metric value was less than 8 seconds"/>
+  <int value="5" label="Metric value was less than 10 seconds"/>
+</enum>
+
 <enum name="PageScaleFactorRange">
   <int value="0" label="&lt;25%"/>
   <int value="1" label="25-49%"/>

tools/metrics/histograms/histograms.xml

@@ -73389,6 +73389,30 @@ uploading your change for review.
   </summary>
 </histogram>
 
+<histogram name="PageLoad.Clients.Scheme.HTTP.PaintTiming.UnderStat"
+    enum="PageLoadTimingUnderStat">
+  <owner>tbansal@chromium.org</owner>
+  <summary>
+    Records if the time from navigation to first contentful paint was less than
+    the specified threshold. Also, records the total number of first contentful
+    paints. For a single first contentful paint event, data may be recorded in
+    multiple buckets. Recorded at the time when first contentful paint happens.
+    Recorded only for page loads that have scheme http://.
+  </summary>
+</histogram>
+
+<histogram name="PageLoad.Clients.Scheme.HTTPS.PaintTiming.UnderStat"
+    enum="PageLoadTimingUnderStat">
+  <owner>tbansal@chromium.org</owner>
+  <summary>
+    Records if the time from navigation to first contentful paint was less than
+    the specified threshold. Also, records the total number of first contentful
+    paints. For a single first contentful paint event, data may be recorded in
+    multiple buckets. Recorded at the time when first contentful paint happens.
+    Recorded only for page loads that have scheme https://.
+  </summary>
+</histogram>
+
 <histogram name="PageLoad.Clients.ServiceWorker.PageTransition"
     enum="CorePageTransition">
   <obsolete>