Convert first contentful paint logging to the new UKM client API

chrome/browser/page_load_metrics/observers/ukm_page_load_metrics_observer.cc

Index: chrome/browser/page_load_metrics/observers/ukm_page_load_metrics_observer.cc
diff --git a/chrome/browser/page_load_metrics/observers/ukm_page_load_metrics_observer.cc b/chrome/browser/page_load_metrics/observers/ukm_page_load_metrics_observer.cc
index ca360c95ed47b607f9839bca11287aee69cd1bbc..53e537ade6bdd072be1cee4287ea7e6f8fbf0f63 100644
--- a/chrome/browser/page_load_metrics/observers/ukm_page_load_metrics_observer.cc
+++ b/chrome/browser/page_load_metrics/observers/ukm_page_load_metrics_observer.cc
@@ -4,8 +4,17 @@
 
 #include "chrome/browser/page_load_metrics/observers/ukm_page_load_metrics_observer.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/page_load_metrics/page_load_metrics_util.h"
+#include "components/ukm/ukm_entry_builder.h"
 #include "components/ukm/ukm_service.h"
-#include "components/ukm/ukm_source.h"
+
+namespace internal {
+
+const char kUkmPageLoadEventName[] = "PageLoad";
+const char kUkmFirstContentfulPaintName[] =
+    "PaintTiming.NavigationToFirstContentfulPaint";
+
+}  // namespace internal
 
 // static
 std::unique_ptr<page_load_metrics::PageLoadMetricsObserver>
@@ -17,48 +26,68 @@ UkmPageLoadMetricsObserver::CreateIfNeeded() {
   return base::MakeUnique<UkmPageLoadMetricsObserver>();
 }
 
-UkmPageLoadMetricsObserver::UkmPageLoadMetricsObserver() {}
+UkmPageLoadMetricsObserver::UkmPageLoadMetricsObserver()
+    : source_id_(ukm::UkmService::NewSourceId()) {}
+
+UkmPageLoadMetricsObserver::~UkmPageLoadMetricsObserver() = default;
 
 UkmPageLoadMetricsObserver::ObservePolicy UkmPageLoadMetricsObserver::OnStart(
     content::NavigationHandle* navigation_handle,
     const GURL& currently_committed_url,
     bool started_in_foreground) {
-  return started_in_foreground ? CONTINUE_OBSERVING : STOP_OBSERVING;
+  if (!started_in_foreground)
+    return STOP_OBSERVING;
+
+  ukm::UkmService* ukm_service = g_browser_process->ukm_service();
+  ukm_service->UpdateSourceURL(source_id_, navigation_handle->GetURL());
+  return CONTINUE_OBSERVING;
 }
 
 UkmPageLoadMetricsObserver::ObservePolicy
 UkmPageLoadMetricsObserver::FlushMetricsOnAppEnterBackground(
     const page_load_metrics::PageLoadTiming& timing,
     const page_load_metrics::PageLoadExtraInfo& info) {
-  SendMetricsToUkm(timing, info);
+  AddTimingMetrics(timing);
+  AddPageLoadExtraInfoMetrics(info, base::TimeTicks::Now());
   return STOP_OBSERVING;
 }
 
 UkmPageLoadMetricsObserver::ObservePolicy UkmPageLoadMetricsObserver::OnHidden(
     const page_load_metrics::PageLoadTiming& timing,
     const page_load_metrics::PageLoadExtraInfo& info) {
-  SendMetricsToUkm(timing, info);
+  AddTimingMetrics(timing);
+  AddPageLoadExtraInfoMetrics(info, base::TimeTicks());

[rkaplow, 2017/03/01 23:54:39]

shouldn't this be Now()? Also i also see the argument seems unused

[Bryan McQuade, 2017/03/02 03:49:50]

ah, this is used for some of the future metrics that aren't part of this change,
so can be removed for now. thanks!

   return STOP_OBSERVING;
 }
 
+void UkmPageLoadMetricsObserver::OnFailedProvisionalLoad(
+    const page_load_metrics::FailedProvisionalLoadInfo& failed_load_info,
+    const page_load_metrics::PageLoadExtraInfo& extra_info) {
+  AddPageLoadExtraInfoMetrics(extra_info, base::TimeTicks());
+}
+
 void UkmPageLoadMetricsObserver::OnComplete(
     const page_load_metrics::PageLoadTiming& timing,
     const page_load_metrics::PageLoadExtraInfo& info) {
-  SendMetricsToUkm(timing, info);
+  AddTimingMetrics(timing);
+  AddPageLoadExtraInfoMetrics(info, base::TimeTicks());
 }
 
-void UkmPageLoadMetricsObserver::SendMetricsToUkm(
-    const page_load_metrics::PageLoadTiming& timing,
-    const page_load_metrics::PageLoadExtraInfo& info) {
-  if (!info.did_commit || !timing.first_contentful_paint)
+void UkmPageLoadMetricsObserver::AddTimingMetrics(
+    const page_load_metrics::PageLoadTiming& timing) {
+  if (!timing.first_contentful_paint)
     return;
 
   ukm::UkmService* ukm_service = g_browser_process->ukm_service();
-  DCHECK(ukm_service);
-
-  std::unique_ptr<ukm::UkmSource> source = base::MakeUnique<ukm::UkmSource>();
-  source->set_committed_url(info.url);
-  source->set_first_contentful_paint(timing.first_contentful_paint.value());
+  std::unique_ptr<ukm::UkmEntryBuilder> builder =
+      ukm_service->GetEntryBuilder(source_id_, internal::kUkmPageLoadEventName);
+  builder->AddMetric(internal::kUkmFirstContentfulPaintName,
+                     timing.first_contentful_paint.value().InMilliseconds());
+}
 
-  ukm_service->RecordSource(std::move(source));
+void UkmPageLoadMetricsObserver::AddPageLoadExtraInfoMetrics(
+    const page_load_metrics::PageLoadExtraInfo& info,
+    base::TimeTicks app_background_time) {

[rkaplow, 2017/03/01 23:54:39]

a_b_t seems to be unused

[Bryan McQuade, 2017/03/02 03:49:50]

removed

+  ukm::UkmService* ukm_service = g_browser_process->ukm_service();
+  ukm_service->UpdateSourceURL(source_id_, info.url);
 }