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

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

Issue 2711513005: Remove deprecated and out of date permissions metrics. (Closed)
Patch Set: Created 3 years, 10 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 | tools/metrics/histograms/histograms.xml » ('j') | tools/metrics/histograms/histograms.xml » ('J')
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 70eb7e8958af2a24c982433728e17dd5494c6b05..bc61304d41f064c100d2c693c3ae854156026abd 100644
--- a/chrome/browser/permissions/permission_uma_util.cc
+++ b/chrome/browser/permissions/permission_uma_util.cc
@@ -91,7 +91,7 @@ const std::string GetRapporMetric(ContentSettingsType permission,
std::string permission_str = PermissionUtil::GetPermissionString(permission);
if (permission_str.empty())
return "";
- return base::StringPrintf("ContentSettings.PermissionActions_%s.%s.Url",
+ return base::StringPrintf("ContentSettings.PermissionActions_%s.%s.Url2",
permission_str.c_str(), action_str.c_str());
}
@@ -103,30 +103,17 @@ void RecordPermissionRequest(PermissionType permission,
g_browser_process->rappor_service();
if (rappor_service) {
if (permission == PermissionType::GEOLOCATION) {
- // TODO(dominickn): remove this deprecated metric - crbug.com/605836.
- rappor::SampleDomainAndRegistryFromGURL(
- rappor_service, "ContentSettings.PermissionRequested.Geolocation.Url",
- requesting_origin);
rappor_service->RecordSampleString(
"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(
- rappor_service,
- "ContentSettings.PermissionRequested.Notifications.Url",
- requesting_origin);
rappor_service->RecordSampleString(
"ContentSettings.PermissionRequested.Notifications.Url2",
rappor::LOW_FREQUENCY_ETLD_PLUS_ONE_RAPPOR_TYPE,
rappor::GetDomainAndRegistrySampleFromGURL(requesting_origin));
} else if (permission == PermissionType::MIDI ||
permission == PermissionType::MIDI_SYSEX) {
- // TODO(dominickn): remove this deprecated metric - crbug.com/605836.
- rappor::SampleDomainAndRegistryFromGURL(
- rappor_service, "ContentSettings.PermissionRequested.Midi.Url",
- requesting_origin);
rappor_service->RecordSampleString(
"ContentSettings.PermissionRequested.Midi.Url2",
rappor::LOW_FREQUENCY_ETLD_PLUS_ONE_RAPPOR_TYPE,
@@ -700,10 +687,11 @@ void PermissionUmaUtil::RecordPermissionAction(
switch (permission) {
case CONTENT_SETTINGS_TYPE_GEOLOCATION:
- PERMISSION_ACTION_UMA(secure_origin, "Permissions.Action.Geolocation",
- "Permissions.Action.SecureOrigin.Geolocation",
- "Permissions.Action.InsecureOrigin.Geolocation",
- action);
+ // Geolocation is disabled on insecure origins, so there's no need to
+ // record metrics for secure/insecue.
+ UMA_HISTOGRAM_ENUMERATION("Permissions.Action.Geolocation",
+ static_cast<int>(action),
+ static_cast<int>(PermissionAction::NUM));
break;
case CONTENT_SETTINGS_TYPE_NOTIFICATIONS:
PERMISSION_ACTION_UMA(secure_origin, "Permissions.Action.Notifications",
@@ -718,10 +706,11 @@ void PermissionUmaUtil::RecordPermissionAction(
action);
break;
case CONTENT_SETTINGS_TYPE_PUSH_MESSAGING:
- PERMISSION_ACTION_UMA(secure_origin, "Permissions.Action.PushMessaging",
- "Permissions.Action.SecureOrigin.PushMessaging",
- "Permissions.Action.InsecureOrigin.PushMessaging",
- action);
+ // Push messaging is disabled on insecure origins, so there's no need to
+ // record metrics for secure/insecue.
+ UMA_HISTOGRAM_ENUMERATION("Permissions.Action.PushMessaging",
+ static_cast<int>(action),
+ static_cast<int>(PermissionAction::NUM));
raymes 2017/02/23 05:54:57 Is there actually a benefit to changing these ones
dominickn 2017/02/23 06:11:42 It's mainly following the lead of the mediastream
raymes 2017/02/23 06:30:55 Ok, sounds good to me
break;
case CONTENT_SETTINGS_TYPE_PROTECTED_MEDIA_IDENTIFIER:
PERMISSION_ACTION_UMA(secure_origin, "Permissions.Action.ProtectedMedia",
@@ -760,20 +749,10 @@ void PermissionUmaUtil::RecordPermissionAction(
<< " not accounted for";
}
- // 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);
+ const std::string rappor_metric = GetRapporMetric(permission, action);
rappor::RapporServiceImpl* rappor_service =
g_browser_process->rappor_service();
- if (!deprecated_metric.empty() && rappor_service) {
- rappor::SampleDomainAndRegistryFromGURL(rappor_service, deprecated_metric,
- requesting_origin);
-
- std::string rappor_metric = deprecated_metric + "2";
+ if (!rappor_metric.empty() && rappor_service) {
rappor_service->RecordSampleString(
rappor_metric, rappor::LOW_FREQUENCY_ETLD_PLUS_ONE_RAPPOR_TYPE,
rappor::GetDomainAndRegistrySampleFromGURL(requesting_origin));
« no previous file with comments | « no previous file | tools/metrics/histograms/histograms.xml » ('j') | tools/metrics/histograms/histograms.xml » ('J')

Powered by Google App Engine
This is Rietveld 408576698