Add a low-frequency RAPPOR configuration, and use it for Safe Browsing and Permissions metrics.

chrome/browser/permissions/permission_uma_util.cc

Index: chrome/browser/permissions/permission_uma_util.cc
diff --git a/chrome/browser/permissions/permission_uma_util.cc b/chrome/browser/permissions/permission_uma_util.cc
index 8d38d0451210d25d95cecac7af49fd236e0d2b1b..5f10ef41dcd1ca3efb5033b967fbd02a78489f49 100644
--- a/chrome/browser/permissions/permission_uma_util.cc
+++ b/chrome/browser/permissions/permission_uma_util.cc
@@ -36,8 +36,6 @@ using content::PermissionType;
 
 namespace {
 
-// Deprecated. This method is used for the single-dimensional RAPPOR metrics
-// that are being replaced by the multi-dimensional ones.
 const std::string GetRapporMetric(PermissionType permission,
                                   PermissionAction action) {
   std::string action_str;
@@ -144,50 +142,52 @@ void RecordPermissionAction(PermissionType permission,
                    << " not accounted for";
   }
 
-  // There are two sets of semi-redundant RAPPOR metrics being reported:
-  // The soon-to-be-deprecated single dimensional ones, and the new
-  // multi-dimensional ones.
   rappor::RapporService* rappor_service = g_browser_process->rappor_service();
-  const std::string rappor_metric = GetRapporMetric(permission, action);
-  if (!rappor_metric.empty())
-    rappor::SampleDomainAndRegistryFromGURL(
-        rappor_service, rappor_metric, requesting_origin);
+  const std::string deprecated_metric = GetRapporMetric(permission, action);
+  if (!deprecated_metric.empty() && rappor_service) {
+    // TODO(dominickn): remove this deprecated metric - crbug.com/605836.
+    rappor::SampleDomainAndRegistryFromGURL(rappor_service, deprecated_metric,
+                                            requesting_origin);
 
-  // Add multi-dimensional RAPPOR reporting for safe-browsing users.
-  std::string permission_str =
-      PermissionUtil::GetPermissionString(permission);
-  if (!rappor_service || permission_str.empty())
-    return;
-
-  std::unique_ptr<rappor::Sample> sample =
-      rappor_service->CreateSample(rappor::SAFEBROWSING_RAPPOR_TYPE);
-  sample->SetStringField("Scheme", requesting_origin.scheme());
-  sample->SetStringField("Host", requesting_origin.host());
-  sample->SetStringField("Port", requesting_origin.port());
-  sample->SetStringField("Domain",
-      rappor::GetDomainAndRegistrySampleFromGURL(requesting_origin));
-  sample->SetFlagsField("Actions", static_cast<uint64_t>(1) << action,
-                        PermissionAction::PERMISSION_ACTION_NUM);
-  rappor_service->RecordSampleObj("Permissions.Action." + permission_str,
-                                  std::move(sample));
+    // The new metric name is the deprecated metric name with "2" on the end,
+    // e.g. ContentSettings.PermissionActions_Geolocation.Granted.Url2.
+    std::string rappor_metric = deprecated_metric + "2";

[felt, 2016/04/28 23:40:44]

I'm a little confused here. Why is recording the new metric dependent on the
deprecated metric (deprecated_metric.empty())?

[dominickn, 2016/04/29 07:14:45]

Only because it's easier to append, rather that chop off "2" from the end of the
metric name. It's a little unnatural now, but when I remove the deprecated
metric, the new name will be returned from GetRapporMetric() and this will read
more cleanly. I've made the comment here a bit beefier to explain.

+    rappor_service->RecordSample(
+        rappor_metric, rappor::LOW_FREQUENCY_ETLD_PLUS_ONE_RAPPOR_TYPE,
+        rappor::GetDomainAndRegistrySampleFromGURL(requesting_origin));
+  }
 }
 
 void RecordPermissionRequest(PermissionType permission,
                              const GURL& requesting_origin,
                              const GURL& embedding_origin,
                              Profile* profile) {
-  bool secure_origin = content::IsOriginSecure(requesting_origin);
-  if (permission == PermissionType::GEOLOCATION) {
+  rappor::RapporService* rappor_service = g_browser_process->rappor_service();
+  if (rappor_service) {
+    if (permission == PermissionType::GEOLOCATION) {
+      // TODO(dominickn): remove this deprecated metric - crbug.com/605836.
       rappor::SampleDomainAndRegistryFromGURL(
-          g_browser_process->rappor_service(),
+          rappor_service,
           "ContentSettings.PermissionRequested.Geolocation.Url",
           requesting_origin);
-  } else if (permission == PermissionType::NOTIFICATIONS) {
+      rappor_service->RecordSample(
+          "ContentSettings.PermissionRequested.Geolocation.Url2",
+          rappor::LOW_FREQUENCY_ETLD_PLUS_ONE_RAPPOR_TYPE,
+          rappor::GetDomainAndRegistrySampleFromGURL(requesting_origin));
+    } else if (permission == PermissionType::NOTIFICATIONS) {
+      // TODO(dominickn): remove this deprecated metric - crbug.com/605836.
       rappor::SampleDomainAndRegistryFromGURL(
-          g_browser_process->rappor_service(),
+          rappor_service,
           "ContentSettings.PermissionRequested.Notifications.Url",
           requesting_origin);
+      rappor_service->RecordSample(
+          "ContentSettings.PermissionRequested.Notifications.Url2",
+          rappor::LOW_FREQUENCY_ETLD_PLUS_ONE_RAPPOR_TYPE,
+          rappor::GetDomainAndRegistrySampleFromGURL(requesting_origin));
+    }
   }
+
+  bool secure_origin = content::IsOriginSecure(requesting_origin);
   UMA_HISTOGRAM_ENUMERATION(
       "ContentSettings.PermissionRequested",
       static_cast<base::HistogramBase::Sample>(permission),