Add NavigationToFirstContentfulPaint UMA for ServiceWorker controlled pages.

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 148ee64622f2ba6eed00bd1b56a6fa76f5cb1ffd..dba24783ce2906bce5645c3db78bb4a8c1b0fb97 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
@@ -97,6 +97,9 @@ const char
         "PageLoad.Timing2.ParseBlockedOnScriptLoadFromDocumentWrite."
         "ParseComplete.Background";
 
+const char kServiceWorkerHistogramFirstContentfulPaint[] =

[Bryan McQuade, 2016/04/12 20:29:10]

I wonder if this should instead be under PageLoad.Clients.ServiceWorker.*

Take a look at
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/pag...

If we only anticipate ever adding a single service worker specific metric, then
maybe this is ok. But my hunch is we may want to measure various SW metrics, in
which case having a separate observer and client makes more sense.

+    "PageLoad.Timing2.NavigationToFirstContentfulPaint.ServiceWorker";
+
 const char kHistogramFirstContentfulPaintHigh[] =
     "PageLoad.Timing2.NavigationToFirstContentfulPaint.HighResolutionClock";
 const char kHistogramFirstContentfulPaintLow[] =
@@ -282,6 +285,11 @@ void CorePageLoadMetricsObserver::RecordTimingHistograms(
         PAGE_LOAD_HISTOGRAM(internal::kHistogramFirstContentfulPaintLow,
                             timing.first_contentful_paint);
       }
+      if (!timing.worker_start.is_null()) {

[Bryan McQuade, 2016/04/12 20:29:10]

I'm disinclined to use absolute times in our metrics.

It sounds like your goal is to communicate whether or not a worker was used for
a page. For that, you may want to use the new WebLoadingBehaviorFlag to annotate
page load metrics with whether a SW was active for that page on the blink side.
You can see how that works here: https://codereview.chromium.org/1846143003 and
in this pending change for the metrics side:
https://codereview.chromium.org/1857443002

You may want to email Kinuko and Charles to see which they think makes sense. My
hunch is that's better than sending a base::Time which is really only intended
to be used as a flag to indicate that a worker was used for a page load.

+        PAGE_LOAD_HISTOGRAM(
+            internal::kServiceWorkerHistogramFirstContentfulPaint,
+            timing.first_contentful_paint);
+      }
       PAGE_LOAD_HISTOGRAM(internal::kHistogramDomLoadingToFirstContentfulPaint,
                           timing.first_contentful_paint - timing.dom_loading);
     } else {