Browser-side UseCounter track kPageVisits in PageLoadMetricsObserver::OnCommit

Bug: 760110
Change-Id: I7c8e70af3cb5b58939a762d577b5374c54638aed
Reviewed-on: https://chromium-review.googlesource.com/687574Reviewed-by: Bryan McQuade <bmcquade@chromium.org>
Commit-Queue: Luna Lu <loonybear@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505031}

chrome/browser/page_load_metrics/observers/use_counter_page_load_metrics_observer.cc

@@ -12,9 +12,24 @@ using Features = page_load_metrics::mojom::PageLoadFeatures;
 UseCounterPageLoadMetricsObserver::UseCounterPageLoadMetricsObserver() {}
 UseCounterPageLoadMetricsObserver::~UseCounterPageLoadMetricsObserver() {}
 
+page_load_metrics::PageLoadMetricsObserver::ObservePolicy
+UseCounterPageLoadMetricsObserver::OnCommit(
+    content::NavigationHandle* navigation_handle,
+    ukm::SourceId source_id) {
+  // Verify that no feature usage is observed before commit
+  DCHECK(features_recorded_.count() <= 0);
+  UMA_HISTOGRAM_ENUMERATION(internal::kFeaturesHistogramName,
+                            WebFeature::kPageVisits,
+                            WebFeature::kNumberOfFeatures);
+  features_recorded_.set(static_cast<size_t>(WebFeature::kPageVisits));
+  return CONTINUE_OBSERVING;
+}
+
 void UseCounterPageLoadMetricsObserver::OnFeaturesUsageObserved(
     const Features& features) {
   for (auto feature : features.features) {
+    // Verify that kPageVisits is only observed at most once per observer.
+    DCHECK(feature != WebFeature::kPageVisits);
     // The usage of each feature should be only measured once. With OOPIF,
     // multiple child frames may send the same feature to the browser, skip if
     // feature has already been measured.

chrome/browser/page_load_metrics/observers/use_counter_page_load_metrics_observer.h

@@ -25,6 +25,8 @@ class UseCounterPageLoadMetricsObserver
   ~UseCounterPageLoadMetricsObserver() override;
 
   // page_load_metrics::PageLoadMetricsObserver.
+  ObservePolicy OnCommit(content::NavigationHandle* navigation_handle,
+                         ukm::SourceId source_id) override;
   void OnFeaturesUsageObserved(
       const page_load_metrics::mojom::PageLoadFeatures&) override;
 

chrome/browser/page_load_metrics/observers/use_counter_page_load_metrics_observer_unittest.cc

@@ -32,6 +32,10 @@ class UseCounterPageLoadMetricsObserverTest
           page_load_metrics::mojom::PageLoadFeatures()) {
     NavigateAndCommit(GURL(kTestUrl));
     SimulateFeaturesUpdate(first_features);
+    // Verify that kPageVisits is observed on commit.
+    histogram_tester().ExpectBucketCount(
+        internal::kFeaturesHistogramName,
+        static_cast<base::Histogram::Sample>(WebFeature::kPageVisits), 1);
     for (auto feature : first_features.features) {
       histogram_tester().ExpectBucketCount(
           internal::kFeaturesHistogramName,

chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc

@@ -1155,6 +1155,9 @@ IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest,
   histogram_tester_.ExpectBucketCount(
       internal::kFeaturesHistogramName,
       static_cast<int32_t>(WebFeature::kV8Element_Animate_Method), 1);
+  histogram_tester_.ExpectBucketCount(
+      internal::kFeaturesHistogramName,
+      static_cast<int32_t>(WebFeature::kPageVisits), 1);
 }
 
 // Test UseCounter Features observed in a child frame are recorded, exactly
@@ -1176,6 +1179,9 @@ IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, UseCounterFeaturesInIframe) {
   histogram_tester_.ExpectBucketCount(
       internal::kFeaturesHistogramName,
       static_cast<int32_t>(WebFeature::kV8Element_Animate_Method), 1);
+  histogram_tester_.ExpectBucketCount(
+      internal::kFeaturesHistogramName,
+      static_cast<int32_t>(WebFeature::kPageVisits), 1);
 }
 
 // Test UseCounter Features observed in multiple child frames are recorded,
@@ -1199,6 +1205,9 @@ IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest,
   histogram_tester_.ExpectBucketCount(
       internal::kFeaturesHistogramName,
       static_cast<int32_t>(WebFeature::kV8Element_Animate_Method), 1);
+  histogram_tester_.ExpectBucketCount(
+      internal::kFeaturesHistogramName,
+      static_cast<int32_t>(WebFeature::kPageVisits), 1);
 }
 
 IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, LoadingMetrics) {