Adding DRP specfic UMA for FirstContentfulPaint

chrome/browser/page_load_metrics/observers/data_reduction_proxy_metrics_observer.cc

+// Copyright 2016 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/data_reduction_proxy_metrics_observer.h"
+
+#include "base/metrics/histogram.h"
+#include "components/data_reduction_proxy/core/common/data_reduction_proxy_params.h"
+#include "components/page_load_metrics/browser/page_load_metrics_observer.h"
+#include "components/page_load_metrics/browser/page_load_metrics_util.h"
+#include "components/page_load_metrics/common/page_load_timing.h"
+#include "content/public/browser/navigation_handle.h"
+
+namespace data_reduction_proxy {
+
+namespace internal {

[Charlie Harrison, 2016/03/11 16:06:04]

nit: Add a newline after { and before }.

[RyanSturm, 2016/03/14 21:00:45]

Done.

+const char kHistogramFirstContentfulPaintDataReductionProxy[] =
+    "PageLoad.Timing2.NavigationToFirstContentfulPaint_DataReductionProxy";

[Charlie Harrison, 2016/03/11 16:06:04]

Can you use the more common "PageLoad.Clients.DataReductionProxy*" histogram
names? Checkout histograms.xml for the PageLoadClients suffix that already
exists. I noticed you already have a DataReductionProxy suffix so it may not be
worth it. We're trying to namespace all non core page_load_metrics clients into
PageLoad.Clients though.

[RyanSturm, 2016/03/14 21:00:45]

Done.

+const char kHistogramFirstContentfulPaintDataReductionProxyAutoLofiOn[] =
+    "PageLoad.Timing2.NavigationToFirstContentfulPaint_DataReductionProxy_"
+    "AutoLoFiOn";
+const char kHistogramFirstContentfulPaintDataReductionProxyAutoLofiOff[] =
+    "PageLoad.Timing2.NavigationToFirstContentfulPaint_DataReductionProxy_"
+    "AutoLofiOff";
+}

[Charlie Harrison, 2016/03/11 16:06:04]

  // namespace

[RyanSturm, 2016/03/14 21:00:46]

Done.

+
+DataReductionProxyMetricsObserver::DataReductionProxyMetricsObserver()
+    : is_using_lofi_(false), used_data_reduction_proxy_(false) {}
+
+DataReductionProxyMetricsObserver::~DataReductionProxyMetricsObserver() {}
+
+void DataReductionProxyMetricsObserver::OnCommit(
+    content::NavigationHandle* navigation_handle) {
+  used_data_reduction_proxy_ = navigation_handle->UsedDataReductionProxy();
+  is_using_lofi_ = navigation_handle->IsUsingLofi();
+}
+void DataReductionProxyMetricsObserver::OnComplete(
+    const page_load_metrics::PageLoadTiming& timing,
+    const page_load_metrics::PageLoadExtraInfo& info) {
+  RecordTimingHistograms(timing, info);

[Charlie Harrison, 2016/03/11 16:06:04]

Why the need for a separate method?

[RyanSturm, 2016/03/14 21:00:46]

I prefer longer methods be aptly named, and I expect RecordTimingHistograms to
grow as more of the metrics are recorded. I also expect in the near future,
OnComplete will be used to send some of this data back to the DataReductionProxy
server. If it seems more appropriate to wait for that to happen, I'll leave this
functionality in OnComplete.

+}
+
+void DataReductionProxyMetricsObserver::RecordTimingHistograms(
+    const page_load_metrics::PageLoadTiming& timing,
+    const page_load_metrics::PageLoadExtraInfo& info) const {
+  if (timing.first_contentful_paint.is_zero() || !used_data_reduction_proxy_)
+    return;
+  PAGE_LOAD_HISTOGRAM(
+      internal::kHistogramFirstContentfulPaintDataReductionProxy,
+      timing.first_contentful_paint);
+  if (is_using_lofi_) {
+    if (data_reduction_proxy::params::IsIncludedInLoFiEnabledFieldTrial()) {
+      PAGE_LOAD_HISTOGRAM(
+          internal::kHistogramFirstContentfulPaintDataReductionProxyAutoLofiOn,
+          timing.first_contentful_paint);
+    } else if (data_reduction_proxy::params::
+                   IsIncludedInLoFiControlFieldTrial()) {
+      PAGE_LOAD_HISTOGRAM(
+          internal::kHistogramFirstContentfulPaintDataReductionProxyAutoLofiOff,
+          timing.first_contentful_paint);
+    }
+  }
+}
+
+}  // namespace data_reduction_proxy