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"

[Charlie Harrison, 2016/05/03 01:08:32]

Not sure you need this if you're only using PAGE_LOAD_HISTOGRAM.

[RyanSturm, 2016/05/03 18:14:13]

Done.

+#include "chrome/browser/renderer_host/chrome_navigation_data.h"
+#include "components/data_reduction_proxy/core/browser/data_reduction_proxy_data.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_data.h"
+#include "content/public/browser/navigation_handle.h"
+
+namespace data_reduction_proxy {
+
+namespace internal {
+
+const char kHistogramFirstContentfulPaintDataReductionProxy[] =
+    "PageLoad.Clients.DataReductionProxy.Timing2."
+    "NavigationToFirstContentfulPaint";
+const char kHistogramFirstContentfulPaintDataReductionProxyAutoLofiOn[] =
+    "PageLoad.Clients.DataReductionProxy.AutoLoFiOn.Timing2."
+    "NavigationToFirstContentfulPaint";
+const char kHistogramFirstContentfulPaintDataReductionProxyAutoLofiOff[] =
+    "PageLoad.Clients.DataReductionProxy.AutoLoFiOff.Timing2."
+    "NavigationToFirstContentfulPaint";
+
+}  // namespace internal
+
+DataReductionProxyMetricsObserver::DataReductionProxyMetricsObserver()
+    : lofi_requested_(false), used_data_reduction_proxy_(false) {}
+
+DataReductionProxyMetricsObserver::~DataReductionProxyMetricsObserver() {}
+
+// Check if the NavigationData indicates anything about the DataReductionProxy.
+void DataReductionProxyMetricsObserver::OnCommit(
+    content::NavigationHandle* navigation_handle) {
+  // As documented in content, this NavigationData is a clone of the

[Charlie Harrison, 2016/05/03 01:08:32]

"//content" is clearer than "content". Better yet, the actual path to the
documentation :)

[RyanSturm, 2016/05/03 18:14:13]

Done.

+  // NavigationData instance returned from
+  // ResourceDispatcherHostDelegate::GetNavigationData during commit.
+  // Because ChromeResourceDispatcherHostDelegate always returns a
+  // ChromeNavigationData, it is safe to static_cast here.
+  ChromeNavigationData* chrome_navigation_data =
+      static_cast<ChromeNavigationData*>(
+          navigation_handle->GetNavigationData());
+  if (!chrome_navigation_data)
+    return;
+  data_reduction_proxy::DataReductionProxyData* data =
+      chrome_navigation_data->GetDataReductionProxyData();
+  if (!data)
+    return;
+  used_data_reduction_proxy_ = data->used_data_reduction_proxy();
+  lofi_requested_ = data->lofi_requested();
+}
+
+void DataReductionProxyMetricsObserver::OnComplete(
+    const page_load_metrics::PageLoadTiming& timing,
+    const page_load_metrics::PageLoadExtraInfo& info) {
+  RecordTimingHistograms(timing, info);
+}
+
+// Record first contentful paint UMA for various DataReductionProxy
+// configurations.
+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(

[Charlie Harrison, 2016/05/03 01:08:32]

You might want to filter out loads that occurred in the background. Loads in the
background skew FP metrics heavily, because painting only occurs when the user
foregrounds a tab. Look at some of the other metrics observers to see how.

[RyanSturm, 2016/05/03 18:14:13]

Thanks. That sounds like a good idea.

+      internal::kHistogramFirstContentfulPaintDataReductionProxy,
+      timing.first_contentful_paint);
+  if (!lofi_requested_)
+    return;
+  if (data_reduction_proxy::params::IsIncludedInLoFiEnabledFieldTrial()) {

[Bryan McQuade, 2016/05/03 13:00:40]

are these standard finch field trials? if so, you shouldn't need to log metrics,
as you can slice and analyze them via the finch dashboard (or via dremel queries
to restrict on field trial)

[RyanSturm, 2016/05/03 18:14:13]

Done.

+    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