diff --git a/chrome/browser/content_settings/one_time_permission_provider.cc b/chrome/browser/content_settings/one_time_permission_provider.cc index 4ff07c2c5cffe6..11027aff927fd0 100644 --- a/chrome/browser/content_settings/one_time_permission_provider.cc +++ b/chrome/browser/content_settings/one_time_permission_provider.cc @@ -15,6 +15,7 @@ #include "chrome/browser/permissions/one_time_permissions_tracker_factory.h" #include "components/content_settings/core/browser/content_settings_rule.h" #include "components/content_settings/core/common/content_settings_utils.h" +#include "components/permissions/permission_uma_util.h" #include "components/permissions/permission_util.h" #include "url/gurl.h" @@ -53,6 +54,11 @@ bool OneTimePermissionProvider::SetWebsiteSetting( if (constraints.session_model != content_settings::SessionModel::OneTime) { value_map_.DeleteValue(primary_pattern, secondary_pattern, content_settings_type); + + permissions::PermissionUmaUtil::RecordOneTimePermissionEvent( + content_settings_type, + permissions::OneTimePermissionEvent::REVOKED_MANUALLY); + return false; } DCHECK_EQ(content_settings::ValueToContentSetting(value), @@ -66,6 +72,10 @@ bool OneTimePermissionProvider::SetWebsiteSetting( .session_model = content_settings::SessionModel::OneTime, }); + permissions::PermissionUmaUtil::RecordOneTimePermissionEvent( + content_settings_type, + permissions::OneTimePermissionEvent::GRANTED_ONE_TIME); + // We need to handle transitions from Allow to Allow Once gracefully. // In that case we add the Allow Once setting in this provider, but also // have to clear the Allow setting in the pref provider. By returning false @@ -112,7 +122,9 @@ void OneTimePermissionProvider::OnLastPageFromOriginClosed( for (auto setting_type : {ContentSettingsType::GEOLOCATION, ContentSettingsType::MEDIASTREAM_CAMERA, ContentSettingsType::MEDIASTREAM_MIC}) { - DeleteValuesMatchingGurl(setting_type, origin.GetURL()); + DeleteValuesMatchingGurl( + setting_type, origin.GetURL(), + permissions::OneTimePermissionEvent::ALL_TABS_CLOSED_OR_DISCARDED); } } @@ -122,7 +134,9 @@ void OneTimePermissionProvider::OnLastPageFromOriginClosed( // origin. void OneTimePermissionProvider::OnAllTabsInBackgroundTimerExpired( const url::Origin& origin) { - DeleteValuesMatchingGurl(ContentSettingsType::GEOLOCATION, origin.GetURL()); + DeleteValuesMatchingGurl( + ContentSettingsType::GEOLOCATION, origin.GetURL(), + permissions::OneTimePermissionEvent::EXPIRED_IN_BACKGROUND); } // All tabs to the origin have not shown a tab indicator for video for a certain @@ -130,8 +144,9 @@ void OneTimePermissionProvider::OnAllTabsInBackgroundTimerExpired( // associated with the origin. void OneTimePermissionProvider::OnCapturingVideoExpired( const url::Origin& origin) { - DeleteValuesMatchingGurl(ContentSettingsType::MEDIASTREAM_CAMERA, - origin.GetURL()); + DeleteValuesMatchingGurl( + ContentSettingsType::MEDIASTREAM_CAMERA, origin.GetURL(), + permissions::OneTimePermissionEvent::EXPIRED_IN_BACKGROUND); } // All tabs to the origin have not shown a tab indicator for microphone access @@ -139,13 +154,15 @@ void OneTimePermissionProvider::OnCapturingVideoExpired( // permission associated with the origin. void OneTimePermissionProvider::OnCapturingAudioExpired( const url::Origin& origin) { - DeleteValuesMatchingGurl(ContentSettingsType::MEDIASTREAM_MIC, - origin.GetURL()); + DeleteValuesMatchingGurl( + ContentSettingsType::MEDIASTREAM_MIC, origin.GetURL(), + permissions::OneTimePermissionEvent::EXPIRED_IN_BACKGROUND); } void OneTimePermissionProvider::DeleteValuesMatchingGurl( ContentSettingsType content_setting_type, - const GURL& origin_gurl) { + const GURL& origin_gurl, + permissions::OneTimePermissionEvent trigger_event) { std::set patterns_to_delete; std::unique_ptr rule_iterator( @@ -156,6 +173,10 @@ void OneTimePermissionProvider::DeleteValuesMatchingGurl( if (rule.primary_pattern.Matches(origin_gurl) && rule.secondary_pattern.Matches(origin_gurl)) { patterns_to_delete.insert({rule.primary_pattern, rule.secondary_pattern}); + if (rule.metadata.expiration >= clock_->Now()) { + permissions::PermissionUmaUtil::RecordOneTimePermissionEvent( + content_setting_type, trigger_event); + } } } rule_iterator.reset(); diff --git a/chrome/browser/content_settings/one_time_permission_provider.h b/chrome/browser/content_settings/one_time_permission_provider.h index 09f01a84c7e6df..bb0c126682e328 100644 --- a/chrome/browser/content_settings/one_time_permission_provider.h +++ b/chrome/browser/content_settings/one_time_permission_provider.h @@ -14,6 +14,7 @@ #include "components/content_settings/core/browser/content_settings_origin_identifier_value_map.h" #include "components/content_settings/core/browser/user_modifiable_provider.h" #include "components/content_settings/core/common/content_settings_pattern.h" +#include "components/permissions/permission_uma_util.h" class OneTimePermissionsTracker; @@ -72,8 +73,11 @@ class OneTimePermissionProvider void OnShutdown() override; private: - void DeleteValuesMatchingGurl(ContentSettingsType content_setting_type, - const GURL& origin_gurl); + // Deletes the matching values and records matching UMA events. + void DeleteValuesMatchingGurl( + ContentSettingsType content_setting_type, + const GURL& origin_gurl, + permissions::OneTimePermissionEvent trigger_event); content_settings::OriginIdentifierValueMap value_map_; raw_ptr one_time_permissions_tracker_ = nullptr; diff --git a/chrome/browser/content_settings/one_time_permission_provider_unittest.cc b/chrome/browser/content_settings/one_time_permission_provider_unittest.cc index 727cfbe2396066..29c8b57a03a0de 100644 --- a/chrome/browser/content_settings/one_time_permission_provider_unittest.cc +++ b/chrome/browser/content_settings/one_time_permission_provider_unittest.cc @@ -4,6 +4,7 @@ #include "chrome/browser/content_settings/one_time_permission_provider.h" #include +#include "base/test/metrics/histogram_tester.h" #include "base/test/scoped_feature_list.h" #include "base/time/time.h" #include "base/values.h" @@ -50,6 +51,23 @@ class OneTimePermissionProviderTest : public testing::Test { .session_model = content_settings::SessionModel::OneTime}; } + // TODO(fjacky): extract to utility class crbug.com/1439219 + // Utility method, because uma mapping functions aren't exposed + std::string GetOneTimePermissionEventHistogram(ContentSettingsType type) { + std::string permission_type; + if (type == ContentSettingsType::GEOLOCATION) { + permission_type = "Geolocation"; + } else if (type == ContentSettingsType::MEDIASTREAM_MIC) { + permission_type = "AudioCapture"; + } else if (type == ContentSettingsType::MEDIASTREAM_CAMERA) { + permission_type = "VideoCapture"; + } else { + NOTREACHED(); + } + + return "Permissions.OneTimePermission." + permission_type + ".Event"; + } + GURL primary_url = GURL("http://example.com/"); ContentSettingsPattern primary_pattern = ContentSettingsPattern::FromURLNoWildcard(primary_url); @@ -69,6 +87,7 @@ class OneTimePermissionProviderTest : public testing::Test { }; TEST_F(OneTimePermissionProviderTest, SetAndGetContentSetting) { + base::HistogramTester histograms; EXPECT_EQ(CONTENT_SETTING_DEFAULT, TestUtils::GetContentSetting( one_time_permission_provider_.get(), primary_url, secondary_url, @@ -83,6 +102,12 @@ TEST_F(OneTimePermissionProviderTest, SetAndGetContentSetting) { TestUtils::GetContentSetting( one_time_permission_provider_.get(), primary_url, secondary_url, ContentSettingsType::GEOLOCATION, false)); + + histograms.ExpectUniqueSample( + GetOneTimePermissionEventHistogram(ContentSettingsType::GEOLOCATION), + static_cast( + permissions::OneTimePermissionEvent::GRANTED_ONE_TIME), + 1); } TEST_F(OneTimePermissionProviderTest, @@ -122,6 +147,7 @@ TEST_F(OneTimePermissionProviderTest, TEST_F(OneTimePermissionProviderTest, AllTabsInBackgroundExpiryRevokesGeolocation) { + base::HistogramTester histograms; EXPECT_EQ(CONTENT_SETTING_DEFAULT, TestUtils::GetContentSetting( one_time_permission_provider_.get(), primary_url, secondary_url, @@ -149,14 +175,24 @@ TEST_F(OneTimePermissionProviderTest, TestUtils::GetContentSetting( one_time_permission_provider_.get(), other_url, secondary_url, ContentSettingsType::GEOLOCATION, false)); + + // We granted to two distinct origins + histograms.ExpectBucketCount( + GetOneTimePermissionEventHistogram(ContentSettingsType::GEOLOCATION), + static_cast( + permissions::OneTimePermissionEvent::GRANTED_ONE_TIME), + 2); + + // Only one origin was in the background and should have been expired + histograms.ExpectBucketCount( + GetOneTimePermissionEventHistogram(ContentSettingsType::GEOLOCATION), + static_cast( + permissions::OneTimePermissionEvent::EXPIRED_IN_BACKGROUND), + 1); } TEST_F(OneTimePermissionProviderTest, CaptureExpiryRevokesPermissions) { - EXPECT_EQ(CONTENT_SETTING_DEFAULT, - TestUtils::GetContentSetting( - one_time_permission_provider_.get(), primary_url, secondary_url, - ContentSettingsType::GEOLOCATION, false)); - + base::HistogramTester histograms; one_time_permission_provider_->SetWebsiteSetting( primary_pattern, ContentSettingsPattern::Wildcard(), ContentSettingsType::MEDIASTREAM_CAMERA, @@ -181,6 +217,35 @@ TEST_F(OneTimePermissionProviderTest, CaptureExpiryRevokesPermissions) { TestUtils::GetContentSetting( one_time_permission_provider_.get(), other_url, secondary_url, ContentSettingsType::MEDIASTREAM_MIC, false)); + + histograms.ExpectTotalCount(GetOneTimePermissionEventHistogram( + ContentSettingsType::MEDIASTREAM_CAMERA), + 2); + histograms.ExpectTotalCount( + GetOneTimePermissionEventHistogram(ContentSettingsType::MEDIASTREAM_MIC), + 2); + histograms.ExpectBucketCount( + GetOneTimePermissionEventHistogram( + ContentSettingsType::MEDIASTREAM_CAMERA), + static_cast( + permissions::OneTimePermissionEvent::GRANTED_ONE_TIME), + 1); + histograms.ExpectBucketCount( + GetOneTimePermissionEventHistogram( + ContentSettingsType::MEDIASTREAM_CAMERA), + static_cast( + permissions::OneTimePermissionEvent::EXPIRED_IN_BACKGROUND), + 1); + histograms.ExpectBucketCount( + GetOneTimePermissionEventHistogram(ContentSettingsType::MEDIASTREAM_MIC), + static_cast( + permissions::OneTimePermissionEvent::GRANTED_ONE_TIME), + 1); + histograms.ExpectBucketCount( + GetOneTimePermissionEventHistogram(ContentSettingsType::MEDIASTREAM_MIC), + static_cast( + permissions::OneTimePermissionEvent::EXPIRED_IN_BACKGROUND), + 1); } TEST_F(OneTimePermissionProviderTest, @@ -219,6 +284,7 @@ TEST_F(OneTimePermissionProviderTest, } TEST_F(OneTimePermissionProviderTest, EnsureOneDayExpiry) { + base::HistogramTester histograms; EXPECT_EQ(CONTENT_SETTING_DEFAULT, TestUtils::GetContentSetting( one_time_permission_provider_.get(), primary_url, secondary_url, @@ -235,5 +301,44 @@ TEST_F(OneTimePermissionProviderTest, EnsureOneDayExpiry) { TestUtils::GetContentSetting( one_time_permission_provider_.get(), primary_url, secondary_url, ContentSettingsType::GEOLOCATION, false)); + + // Only a grant sample should be recorded. 1-day expiry can be computed from + // #grants - #other buckets + histograms.ExpectUniqueSample( + GetOneTimePermissionEventHistogram(ContentSettingsType::GEOLOCATION), + static_cast( + permissions::OneTimePermissionEvent::GRANTED_ONE_TIME), + 1); +} + +TEST_F(OneTimePermissionProviderTest, ManualRevocationUmaTest) { + base::HistogramTester histograms; + EXPECT_EQ(CONTENT_SETTING_DEFAULT, + TestUtils::GetContentSetting( + one_time_permission_provider_.get(), primary_url, secondary_url, + ContentSettingsType::GEOLOCATION, false)); + + one_time_permission_provider_->SetWebsiteSetting( + primary_pattern, ContentSettingsPattern::Wildcard(), + ContentSettingsType::GEOLOCATION, base::Value(CONTENT_SETTING_ALLOW), + one_time_constraints()); + + one_time_permission_provider_->SetWebsiteSetting( + primary_pattern, ContentSettingsPattern::Wildcard(), + ContentSettingsType::GEOLOCATION, base::Value(CONTENT_SETTING_ASK), {}); + + histograms.ExpectTotalCount( + GetOneTimePermissionEventHistogram(ContentSettingsType::GEOLOCATION), 2); + histograms.ExpectBucketCount( + GetOneTimePermissionEventHistogram(ContentSettingsType::GEOLOCATION), + static_cast( + permissions::OneTimePermissionEvent::GRANTED_ONE_TIME), + 1); + + histograms.ExpectBucketCount( + GetOneTimePermissionEventHistogram(ContentSettingsType::GEOLOCATION), + static_cast( + permissions::OneTimePermissionEvent::REVOKED_MANUALLY), + 1); } } // namespace content_settings diff --git a/chrome/browser/ui/views/permissions/one_time_permission_interactive_ui_test.cc b/chrome/browser/ui/views/permissions/one_time_permission_interactive_ui_test.cc index 8caa656ddf855a..d2a426e3f73577 100644 --- a/chrome/browser/ui/views/permissions/one_time_permission_interactive_ui_test.cc +++ b/chrome/browser/ui/views/permissions/one_time_permission_interactive_ui_test.cc @@ -25,6 +25,7 @@ #include "chrome/test/base/test_switches.h" #include "chrome/test/base/ui_test_utils.h" #include "components/content_settings/core/common/content_settings.h" +#include "components/content_settings/core/common/content_settings_types.h" #include "components/performance_manager/public/performance_manager.h" #include "components/permissions/features.h" #include "components/permissions/permission_request_manager.h" @@ -216,11 +217,46 @@ class OneTimePermissionInteractiveUiTest : public WebRtcTestBase { } protected: + void OtpEventExpectUniqueSample(ContentSettingsType content_setting_type, + permissions::OneTimePermissionEvent event, + int occ) { + histograms_.ExpectUniqueSample( + GetOneTimePermissionEventHistogram(content_setting_type), + static_cast(event), occ); + } + + void OtpEventExpectBucketCount(ContentSettingsType content_setting_type, + permissions::OneTimePermissionEvent event, + int occ) { + histograms_.ExpectBucketCount( + GetOneTimePermissionEventHistogram(content_setting_type), + static_cast(event), occ); + } + std::unique_ptr geolocation_overrider_; raw_ptr current_browser_ = nullptr; + base::HistogramTester histograms_; + private: + // TODO(fjacky): extract to utility class crbug.com/1439219 + // Utility method, because uma mapping functions aren't exposed + std::string GetOneTimePermissionEventHistogram(ContentSettingsType type) { + std::string permission_type; + if (type == ContentSettingsType::GEOLOCATION) { + permission_type = "Geolocation"; + } else if (type == ContentSettingsType::MEDIASTREAM_MIC) { + permission_type = "AudioCapture"; + } else if (type == ContentSettingsType::MEDIASTREAM_CAMERA) { + permission_type = "VideoCapture"; + } else { + NOTREACHED(); + } + + return "Permissions.OneTimePermission." + permission_type + ".Event"; + } + // The render frame host where JS calls will be executed. raw_ptr render_frame_host_ = nullptr; @@ -244,6 +280,11 @@ IN_PROC_BROWSER_TEST_F(OneTimePermissionInteractiveUiTest, // Ensure position is accessible in new tab without prompt. WatchPositionAndExpectGrantedPermission( permissions::PermissionRequestManager::ACCEPT_ONCE, false); + + // Ensure grant event is recorded + OtpEventExpectUniqueSample( + ContentSettingsType::GEOLOCATION, + permissions::OneTimePermissionEvent::GRANTED_ONE_TIME, 1); } IN_PROC_BROWSER_TEST_F(OneTimePermissionInteractiveUiTest, @@ -262,6 +303,11 @@ IN_PROC_BROWSER_TEST_F(OneTimePermissionInteractiveUiTest, // Ensure position is accessible in new tab without prompt. WatchPositionAndExpectGrantedPermission( permissions::PermissionRequestManager::ACCEPT_ONCE, false); + + // Ensure grant event is only recorded once + OtpEventExpectUniqueSample( + ContentSettingsType::GEOLOCATION, + permissions::OneTimePermissionEvent::GRANTED_ONE_TIME, 1); } IN_PROC_BROWSER_TEST_F(OneTimePermissionInteractiveUiTest, @@ -279,7 +325,17 @@ IN_PROC_BROWSER_TEST_F(OneTimePermissionInteractiveUiTest, // Ensure that a prompt is triggered. WatchPositionAndExpectGrantedPermission( - permissions::PermissionRequestManager::ACCEPT_ONCE, true); + permissions::PermissionRequestManager::ACCEPT_ALL, true); + + // Since the second request was resolved with a persistent accept, only one + // otp-grant event should be recorded + OtpEventExpectBucketCount( + ContentSettingsType::GEOLOCATION, + permissions::OneTimePermissionEvent::GRANTED_ONE_TIME, 1); + + OtpEventExpectBucketCount( + ContentSettingsType::GEOLOCATION, + permissions::OneTimePermissionEvent::ALL_TABS_CLOSED_OR_DISCARDED, 1); } IN_PROC_BROWSER_TEST_F(OneTimePermissionInteractiveUiTest, @@ -305,7 +361,17 @@ IN_PROC_BROWSER_TEST_F(OneTimePermissionInteractiveUiTest, // Ensure that a prompt is triggered again when requesting geolocation // permission. WatchPositionAndExpectGrantedPermission( - permissions::PermissionRequestManager::ACCEPT_ONCE, true); + permissions::PermissionRequestManager::ACCEPT_ALL, true); + + // Since the second request was resolved with a persistent accept, only one + // otp-grant event should be recorded + OtpEventExpectBucketCount( + ContentSettingsType::GEOLOCATION, + permissions::OneTimePermissionEvent::GRANTED_ONE_TIME, 1); + + OtpEventExpectBucketCount( + ContentSettingsType::GEOLOCATION, + permissions::OneTimePermissionEvent::ALL_TABS_CLOSED_OR_DISCARDED, 1); } IN_PROC_BROWSER_TEST_F( @@ -336,6 +402,11 @@ IN_PROC_BROWSER_TEST_F( // Request geolocation permission and ensure that no prompt is triggered. WatchPositionAndExpectGrantedPermission( permissions::PermissionRequestManager::ACCEPT_ONCE, false); + + // Ensure that only a single GRANTED_ONE_TIME event is recorded + OtpEventExpectUniqueSample( + ContentSettingsType::GEOLOCATION, + permissions::OneTimePermissionEvent::GRANTED_ONE_TIME, 1); } IN_PROC_BROWSER_TEST_F(OneTimePermissionInteractiveUiTest, @@ -364,6 +435,14 @@ IN_PROC_BROWSER_TEST_F(OneTimePermissionInteractiveUiTest, // Ensure that a prompt is triggered again when requesting permission WatchPositionAndExpectGrantedPermission( permissions::PermissionRequestManager::ACCEPT_ONCE, true); + + OtpEventExpectBucketCount( + ContentSettingsType::GEOLOCATION, + permissions::OneTimePermissionEvent::GRANTED_ONE_TIME, 2); + + OtpEventExpectBucketCount( + ContentSettingsType::GEOLOCATION, + permissions::OneTimePermissionEvent::EXPIRED_IN_BACKGROUND, 1); } IN_PROC_BROWSER_TEST_F(OneTimePermissionInteractiveUiTest, @@ -378,14 +457,22 @@ IN_PROC_BROWSER_TEST_F(OneTimePermissionInteractiveUiTest, // web contents). CloseLastLocalStreamAt(0); - // Fire running timers. This means, that all one time permission expiration - // timers that are running at this point in time will fire their callbacks and - // are stopped. + // Fire running timers. This means, that all one time permission + // expiration timers that are running at this point in time will fire + // their callbacks and are stopped. FireRunningExpirationTimers(); // Request cam/mic permission, expect no prompt is triggered. GetUserMediaAndExpectGrantedPermission( permissions::PermissionRequestManager::ACCEPT_ONCE, false, 0); + + // Ensure that only a single GRANTED_ONE_TIME event is recorded per permission + OtpEventExpectUniqueSample( + ContentSettingsType::MEDIASTREAM_MIC, + permissions::OneTimePermissionEvent::GRANTED_ONE_TIME, 1); + OtpEventExpectUniqueSample( + ContentSettingsType::MEDIASTREAM_CAMERA, + permissions::OneTimePermissionEvent::GRANTED_ONE_TIME, 1); } IN_PROC_BROWSER_TEST_F(OneTimePermissionInteractiveUiTest, @@ -403,9 +490,9 @@ IN_PROC_BROWSER_TEST_F(OneTimePermissionInteractiveUiTest, // within specified web contents). CloseLastLocalStreamAt(0); - // Fire running timers. This means, that all one time permission expiration - // timers that are running at this point in time will fire their callbacks and - // are stopped. + // Fire running timers. This means, that all one time permission + // expiration timers that are running at this point in time will fire + // their callbacks and are stopped. FireRunningExpirationTimers(); // Switch back to previous tab @@ -414,6 +501,21 @@ IN_PROC_BROWSER_TEST_F(OneTimePermissionInteractiveUiTest, // Request cam/mic permission, expect a prompt is triggered. GetUserMediaAndExpectGrantedPermission( permissions::PermissionRequestManager::ACCEPT_ONCE, true, 0); + + OtpEventExpectBucketCount( + ContentSettingsType::MEDIASTREAM_MIC, + permissions::OneTimePermissionEvent::GRANTED_ONE_TIME, 2); + + OtpEventExpectBucketCount( + ContentSettingsType::MEDIASTREAM_CAMERA, + permissions::OneTimePermissionEvent::GRANTED_ONE_TIME, 2); + + OtpEventExpectBucketCount( + ContentSettingsType::MEDIASTREAM_MIC, + permissions::OneTimePermissionEvent::EXPIRED_IN_BACKGROUND, 1); + OtpEventExpectBucketCount( + ContentSettingsType::MEDIASTREAM_CAMERA, + permissions::OneTimePermissionEvent::EXPIRED_IN_BACKGROUND, 1); } IN_PROC_BROWSER_TEST_F(OneTimePermissionInteractiveUiTest, @@ -438,6 +540,14 @@ IN_PROC_BROWSER_TEST_F(OneTimePermissionInteractiveUiTest, // Request cam/mic permission, expect no prompt is triggered. GetUserMediaAndExpectGrantedPermission( permissions::PermissionRequestManager::ACCEPT_ONCE, false, 0); + + // Ensure that only a single GRANTED_ONE_TIME event is recorded per permission + OtpEventExpectUniqueSample( + ContentSettingsType::MEDIASTREAM_MIC, + permissions::OneTimePermissionEvent::GRANTED_ONE_TIME, 1); + OtpEventExpectUniqueSample( + ContentSettingsType::MEDIASTREAM_CAMERA, + permissions::OneTimePermissionEvent::GRANTED_ONE_TIME, 1); } IN_PROC_BROWSER_TEST_F(OneTimePermissionInteractiveUiTest, @@ -452,12 +562,20 @@ IN_PROC_BROWSER_TEST_F(OneTimePermissionInteractiveUiTest, // web contents). CloseLastLocalStreamAt(0); - // Fire running timers. This means, that all one time permission expiration - // timers that are running at this point in time will fire their callbacks and - // are stopped. + // Fire running timers. This means, that all one time permission + // expiration timers that are running at this point in time will fire + // their callbacks and are stopped. FireRunningExpirationTimers(); // Request cam/mic permission, expect no prompt is triggered. GetUserMediaAndExpectGrantedPermission( permissions::PermissionRequestManager::ACCEPT_ONCE, false, 0); + + // Ensure that only a single GRANTED_ONE_TIME event is recorded per permission + OtpEventExpectUniqueSample( + ContentSettingsType::MEDIASTREAM_MIC, + permissions::OneTimePermissionEvent::GRANTED_ONE_TIME, 1); + OtpEventExpectUniqueSample( + ContentSettingsType::MEDIASTREAM_CAMERA, + permissions::OneTimePermissionEvent::GRANTED_ONE_TIME, 1); } diff --git a/components/permissions/permission_uma_util.cc b/components/permissions/permission_uma_util.cc index ef002e52ef9a4d..0c79f82bb8017d 100644 --- a/components/permissions/permission_uma_util.cc +++ b/components/permissions/permission_uma_util.cc @@ -1275,6 +1275,19 @@ void PermissionUmaUtil::RecordPageInfoDialogAccessType( "Permissions.ConfirmationChip.PageInfoDialogAccessType", access_type); } +// static +void PermissionUmaUtil::RecordOneTimePermissionEvent( + ContentSettingsType type, + OneTimePermissionEvent event) { + DCHECK(permissions::PermissionUtil::CanPermissionBeAllowedOnce(type)); + + std::string permission_type = GetPermissionRequestString( + GetUmaValueForRequestType(ContentSettingsTypeToRequestType(type))); + + base::UmaHistogramEnumeration( + "Permissions.OneTimePermission." + permission_type + ".Event", event); +} + // static void PermissionUmaUtil::RecordPageInfoPermissionChangeWithin1m( ContentSettingsType type, diff --git a/components/permissions/permission_uma_util.h b/components/permissions/permission_uma_util.h index 601684f354ff94..a415313558798d 100644 --- a/components/permissions/permission_uma_util.h +++ b/components/permissions/permission_uma_util.h @@ -252,6 +252,28 @@ enum class AdaptiveTriggers { THREE_CONSECUTIVE_DENIES = 0x01, }; +// These values are logged to UMA. Entries should not be renumbered and +// numeric values should never be reused. Please keep in sync with +// "OneTimePermissionEvent" in tools/metrics/histograms/enums.xml. +enum class OneTimePermissionEvent { + // Recorded for each one time grant + GRANTED_ONE_TIME = 0, + + // Recorded when the user manually revokes a one time grant + REVOKED_MANUALLY = 1, + + // Recorded when a one time grant expires because all tabs are either closed + // or discarded. + + ALL_TABS_CLOSED_OR_DISCARDED = 2, + + // Recorded when a one time grant expires because the permission was unused in + // the background. + EXPIRED_IN_BACKGROUND = 3, + + kMaxValue = EXPIRED_IN_BACKGROUND +}; + enum class PermissionAutoRevocationHistory { // Permission has not been automatically revoked. NONE = 0, @@ -535,6 +557,9 @@ class PermissionUmaUtil { static void RecordPageInfoDialogAccessType( PageInfoDialogAccessType access_type); + static void RecordOneTimePermissionEvent(ContentSettingsType type, + OneTimePermissionEvent event); + static void RecordPageInfoPermissionChangeWithin1m( ContentSettingsType type, PermissionAction previous_action, diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml index d346428247f819..93710984f45566 100644 --- a/tools/metrics/histograms/enums.xml +++ b/tools/metrics/histograms/enums.xml @@ -76406,6 +76406,14 @@ Called by update_net_trust_anchors.py.--> + + One-time permission events. + + + + + + OnServiceConnected is already called. Everything worked as expected. diff --git a/tools/metrics/histograms/metadata/permissions/histograms.xml b/tools/metrics/histograms/metadata/permissions/histograms.xml index 962170e1f6b552..41d2fdbeee7cb8 100644 --- a/tools/metrics/histograms/metadata/permissions/histograms.xml +++ b/tools/metrics/histograms/metadata/permissions/histograms.xml @@ -674,6 +674,26 @@ chromium-metrics-reviews@google.com. + + fjacky@chromium.org + src/components/permissions/PERMISSIONS_OWNERS + + Logs granting, revocation, and expiry behaviors of one time permissions. It + records grant events, manual revoke events, expiry due to tab closure or + discards, and expiry in the background due to non-usage. Note that the + number of grants that expired due to the 24 hour maximum grant time limit + can be aproximated by calculating #(granted one time) - #(other events), + though this does not take into account one time permissions that are still + currently active. + + + + + + + +