Service Worker: Factor out NewTabPage from page load count metrics

NewTabPage is dominating our data. This change:
1) Replaces the user action ControlledPageLoad with a histogram PageLoad,
with buckets for NTP and Other.
2) Removes NTP page loads from RAPPOR metric.

BUG=508399

Review URL: https://codereview.chromium.org/1284143002

Cr-Commit-Position: refs/heads/master@{#343334}

4 files changed, 37 insertions, 10 deletions

diff --git a/content/browser/service_worker/service_worker_metrics.cc b/content/browser/service_worker/service_worker_metrics.cc
index 3a11e24..65429b0 100644
--- a/content/browser/service_worker/service_worker_metrics.cc
+++ b/content/browser/service_worker/service_worker_metrics.cc
@@ -6,12 +6,10 @@
 
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/sparse_histogram.h"
-#include "base/metrics/user_metrics_action.h"
 #include "base/strings/string_util.h"
 #include "content/common/service_worker/service_worker_types.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/content_browser_client.h"
-#include "content/public/browser/user_metrics.h"
 #include "content/public/common/content_client.h"
 
 namespace content {
@@ -24,16 +22,25 @@ void RecordURLMetricOnUI(const GURL& url) {
       "ServiceWorker.ControlledPageUrl", url);
 }
 
+ServiceWorkerMetrics::Site SiteFromURL(const GURL& gurl) {
+  // UIThreadSearchTermsData::GoogleBaseURLValue() returns the google base
+  // URL, but not available in content layer.
+  static const char google_like_scope_prefix[] = "https://www.google.";
+  static const char ntp_scope_path[] = "/_/chrome/";
+  if (base::StartsWith(gurl.spec(), google_like_scope_prefix,
+                       base::CompareCase::INSENSITIVE_ASCII) &&
+      base::StartsWith(gurl.path(), ntp_scope_path,
+                       base::CompareCase::SENSITIVE)) {
+    return ServiceWorkerMetrics::Site::NEW_TAB_PAGE;
+  }
+
+  return ServiceWorkerMetrics::Site::OTHER;
+}
+
 bool ShouldExcludeForHistogram(const GURL& scope) {
   // Exclude NTP scope from UMA for now as it tends to dominate the stats
   // and makes the results largely skewed.
-  // TOOD(kinuko): This should be temporary, revisit this once we have
-  // better idea about what should be excluded in the UMA.
-  // (UIThreadSearchTermsData::GoogleBaseURLValue() returns the google base
-  // URL, but not available in content layer)
-  const char google_like_scope_prefix[] = "https://www.google.";
-  return base::StartsWith(scope.spec(), google_like_scope_prefix,
-                          base::CompareCase::INSENSITIVE_ASCII);
+  return SiteFromURL(scope) == ServiceWorkerMetrics::Site::NEW_TAB_PAGE;
 }
 
 enum EventHandledRatioType {
@@ -103,7 +110,13 @@ void ServiceWorkerMetrics::RecordDeleteAndStartOverResult(
 }
 
 void ServiceWorkerMetrics::CountControlledPageLoad(const GURL& url) {
-  RecordAction(base::UserMetricsAction("ServiceWorker.ControlledPageLoad"));
+  Site site = SiteFromURL(url);
+  UMA_HISTOGRAM_ENUMERATION("ServiceWorker.PageLoad", static_cast<int>(site),
+                            static_cast<int>(Site::NUM_TYPES));
+
+  // Don't record NTP on RAPPOR since it would dominate the data.
+  if (site == Site::NEW_TAB_PAGE)
+    return;
   BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
                           base::Bind(&RecordURLMetricOnUI, url));
 }
diff --git a/content/browser/service_worker/service_worker_metrics.h b/content/browser/service_worker/service_worker_metrics.h
index 0e0fe8e..0c996fe 100644
--- a/content/browser/service_worker/service_worker_metrics.h
+++ b/content/browser/service_worker/service_worker_metrics.h
@@ -83,6 +83,9 @@ class ServiceWorkerMetrics {
     NUM_EVENT_TYPES
   };
 
+  // Used for UMA. Append only.
+  enum class Site { OTHER, NEW_TAB_PAGE, NUM_TYPES };
+
   // Used for ServiceWorkerDiskCache.
   static void CountInitDiskCacheResult(bool result);
   static void CountReadResponseResult(ReadResponseResult result);
diff --git a/tools/metrics/actions/actions.xml b/tools/metrics/actions/actions.xml
index 3bdb935..98de2e1 100644
--- a/tools/metrics/actions/actions.xml
+++ b/tools/metrics/actions/actions.xml
@@ -11200,6 +11200,7 @@ should be able to be added at any place in this file.
   <description>
     Counts the number of page loads controlled by a Service Worker.
   </description>
+  <obsolete>Replaced by ServiceWorker.PageLoad histogram.</obsolete>
 </action>
 
 <action name="SettingsResetBubble.LearnMore">
diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml
index ac5b3f5..b37025b 100644
--- a/tools/metrics/histograms/histograms.xml
+++ b/tools/metrics/histograms/histograms.xml
@@ -38948,6 +38948,11 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
   </summary>
 </histogram>
 
+<histogram name="ServiceWorker.PageLoad" enum="ServiceWorkerSite">
+  <owner>falken@chromium.org</owner>
+  <summary>Counts page loads controlled by a service worker.</summary>
+</histogram>
+
 <histogram name="ServiceWorker.PushEvent.Time" units="milliseconds">
   <owner>johnme@chromium.org</owner>
   <summary>
@@ -68851,6 +68856,11 @@ To add a new entry, add it with any value and run test to compute valid value.
   <int value="8" label="ErrorResponseTypeOpaqueForClientRequest"/>
 </enum>
 
+<enum name="ServiceWorkerSite" type="int">
+  <int value="0" label="Other"/>
+  <int value="1" label="NewTabPage"/>
+</enum>
+
 <enum name="ServiceWorkerStatusCode" type="int">
   <int value="0" label="SERVICE_WORKER_OK"/>
   <int value="1" label="SERVICE_WORKER_ERROR_FAILED"/>