Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(51)

Unified Diff: chrome/browser/permissions/permission_uma_util.cc

Issue 1921533004: Add a low-frequency RAPPOR configuration, and use it for Safe Browsing and Permissions metrics. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Addressing nits Created 4 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | chrome/browser/safe_browsing/safe_browsing_blocking_page.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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..19a2efa18e9bd01d97168f32082a90a61bcea7fd 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,55 @@ 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.
+ // Retrieve the name of the RAPPOR metric. Currently, the new metric name is
+ // the deprecated name with "2" on the end, e.g.
+ // ContentSettings.PermissionActions_Geolocation.Granted.Url2. For simplicity,
+ // we retrieve the deprecated name and append the "2" for the new name.
+ // TODO(dominickn): remove the deprecated metric and replace it solely with
+ // the new one in GetRapporMetric - crbug.com/605836.
+ const std::string deprecated_metric = GetRapporMetric(permission, action);
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);
+ if (!deprecated_metric.empty() && rappor_service) {
+ 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));
+ std::string rappor_metric = deprecated_metric + "2";
+ 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),
« no previous file with comments | « no previous file | chrome/browser/safe_browsing/safe_browsing_blocking_page.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698