Initial UKMPageLoadMetricsObserver

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
new file mode 100644
index 0000000000000000000000000000000000000000..ee069659b2ac9d3ce97fb0bdceaa9bcdb90224f9
--- /dev/null
+++ b/chrome/browser/page_load_metrics/observers/ukm_page_load_metrics_observer.cc
@@ -0,0 +1,60 @@
+// 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/ukm_page_load_metrics_observer.h"
+#include "chrome/browser/browser_process.h"
+#include "components/ukm/ukm_service.h"
+#include "components/ukm/ukm_source.h"
+
+// static
+std::unique_ptr<page_load_metrics::PageLoadMetricsObserver>
+UKMPageLoadMetricsObserver::CreateIfNeeded() {
+  if (!g_browser_process->ukm_service())
+    return nullptr;
+
+  return base::MakeUnique<UKMPageLoadMetricsObserver>();
+}
+
+UKMPageLoadMetricsObserver::UKMPageLoadMetricsObserver() {}
+
+page_load_metrics::PageLoadMetricsObserver::ObservePolicy
+UKMPageLoadMetricsObserver::OnStart(

[Zhen Wang, 2017/01/09 17:34:51]

Probably also need to consider OnRedirect etc to make the picture complete.

[oystein (OOO til 10th of July), 2017/01/24 19:21:41]

Bryan: How do the other pageload metrics handle OnRedirect?

+    content::NavigationHandle* navigation_handle,
+    const GURL& currently_committed_url,
+    bool started_in_foreground) {

[Steven Holte, 2017/01/10 23:53:52]

This seems like a good place to check sync pref:

bool IsHistorySyncEnabled(content::NavigationHandle* navigation_handle) {
  Profile* profile =
Profile::FromBrowserContext(navigation_handle->GetWebContents()->GetBrowserContext());
  browser_sync::ProfileSyncService* sync =
      ProfileSyncServiceFactory::GetInstance()->GetForProfile(profile);
  return sync &&
      sync->GetPreferredDataTypes().Has(syncer::HISTORY_DELETE_DIRECTIVES);
}

[oystein (OOO til 10th of July), 2017/01/24 19:21:41]

we've had a couple of meetings since this; we decided to handle this elsewhere,
right?

+  navigation_start_ = navigation_handle->NavigationStart();
+  return started_in_foreground ? CONTINUE_OBSERVING : STOP_OBSERVING;
+}
+
+page_load_metrics::PageLoadMetricsObserver::ObservePolicy
+UKMPageLoadMetricsObserver::OnHidden(
+    const page_load_metrics::PageLoadTiming& timing,
+    const page_load_metrics::PageLoadExtraInfo& extra_info) {
+  return STOP_OBSERVING;

[Zhen Wang, 2017/01/09 17:34:51]

Does this we ignore the metrics whenever a page is put in the background, even
if the page is brought to foreground later?

[oystein (OOO til 10th of July), 2017/01/24 19:21:41]

Yep. My intuition is that it's reasonable to restrict (for now) our measurements
to only fully-foregrounded loads, to remove as many variables as possible. This
is something we can add events for later to UKM.

+}
+
+void UKMPageLoadMetricsObserver::OnComplete(
+    const page_load_metrics::PageLoadTiming& timing,
+    const page_load_metrics::PageLoadExtraInfo& info) {
+  if (info.committed_url.is_empty())
+    return;
+
+  LOG(ERROR) << "UKMPageLoadMetricsObserver::OnComplete for '"
+             << info.committed_url << "': FMP "
+             << timing.first_meaningful_paint.value();
+
+  ukm::UkmService* ukm_service = g_browser_process->ukm_service();
+  if (!ukm_service) {
+    LOG(ERROR) << "No UKM service :'(";
+    return;
+  }
+
+  std::unique_ptr<ukm::UkmSource> source =
+      base::WrapUnique(new ukm::UkmSource());
+  source->set_committed_url(info.committed_url);
+  source->set_navigation_start(navigation_start_);

[Zhen Wang, 2017/01/09 17:34:51]

The |timing| parameter passed in also has |timing.navigation_start|, why do we
need to use |navigation_start_| to track by ourselves? Is there a difference?

[oystein (OOO til 10th of July), 2017/01/24 19:21:41]

Done.

+  source->set_first_meaningful_paint(timing.first_meaningful_paint.value());
+
+  ukm_service->RecordSource(std::move(source));
+}