Skip to content

Commit

Permalink
Add one time permission mechanisms metrics
Browse files Browse the repository at this point in the history 
To evaluate the granting and expiry behaviours of one time permissions, record the following events:
- Granted one time
- Revoked one time grant manually
- Expired due to tab closure & discards
- Expired in the background due to non-usage
- Implicitly computable: #(granted one time) - #(other events) = #(grants that expired after 24 hours)

Bug: 1434922
Change-Id: Ifdee7367cceef9a00fd7c361487ccf8dca2d3a10
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4453466
Code-Coverage: Findit <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: Christian Dullweber <dullweber@chromium.org>
Reviewed-by: Andy Paicu <andypaicu@chromium.org>
Commit-Queue: Florian Jacky <fjacky@chromium.org>
Auto-Submit: Florian Jacky <fjacky@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1135167}
  • Loading branch information
@fjacky
fjacky authored and Chromium LUCI CQ committed Apr 25, 2023
1 parent a99bbdf commit 71f3935
Show file tree
Hide file tree
Showing 8 changed files with 339 additions and 25 deletions.
35 changes: 28 additions & 7 deletions chrome/browser/content_settings/one_time_permission_provider.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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),
Expand All @@ -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
Expand Down Expand Up @@ -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);
}
}

Expand All @@ -122,30 +134,35 @@ 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
// time and have been in the background. We remove the camera permission
// 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
// for a certain time and have been in the background. We remove the microphone
// 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<content_settings::OriginIdentifierValueMap::PatternPair>
patterns_to_delete;
std::unique_ptr<content_settings::RuleIterator> rule_iterator(
Expand All @@ -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();
Expand Down
8 changes: 6 additions & 2 deletions chrome/browser/content_settings/one_time_permission_provider.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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<OneTimePermissionsTracker> one_time_permissions_tracker_ = nullptr;
Expand Down
115 changes: 110 additions & 5 deletions chrome/browser/content_settings/one_time_permission_provider_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "chrome/browser/content_settings/one_time_permission_provider.h"
#include <memory>
#include "base/test/metrics/histogram_tester.h"
#include "base/test/scoped_feature_list.h"
#include "base/time/time.h"
#include "base/values.h"
Expand Down Expand Up @@ -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);
Expand All @@ -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,
Expand All @@ -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<base::HistogramBase::Sample>(
permissions::OneTimePermissionEvent::GRANTED_ONE_TIME),
1);
}

TEST_F(OneTimePermissionProviderTest,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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<base::HistogramBase::Sample>(
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<base::HistogramBase::Sample>(
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,
Expand All @@ -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<base::HistogramBase::Sample>(
permissions::OneTimePermissionEvent::GRANTED_ONE_TIME),
1);
histograms.ExpectBucketCount(
GetOneTimePermissionEventHistogram(
ContentSettingsType::MEDIASTREAM_CAMERA),
static_cast<base::HistogramBase::Sample>(
permissions::OneTimePermissionEvent::EXPIRED_IN_BACKGROUND),
1);
histograms.ExpectBucketCount(
GetOneTimePermissionEventHistogram(ContentSettingsType::MEDIASTREAM_MIC),
static_cast<base::HistogramBase::Sample>(
permissions::OneTimePermissionEvent::GRANTED_ONE_TIME),
1);
histograms.ExpectBucketCount(
GetOneTimePermissionEventHistogram(ContentSettingsType::MEDIASTREAM_MIC),
static_cast<base::HistogramBase::Sample>(
permissions::OneTimePermissionEvent::EXPIRED_IN_BACKGROUND),
1);
}

TEST_F(OneTimePermissionProviderTest,
Expand Down Expand Up @@ -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,
Expand All @@ -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<base::HistogramBase::Sample>(
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<base::HistogramBase::Sample>(
permissions::OneTimePermissionEvent::GRANTED_ONE_TIME),
1);

histograms.ExpectBucketCount(
GetOneTimePermissionEventHistogram(ContentSettingsType::GEOLOCATION),
static_cast<base::HistogramBase::Sample>(
permissions::OneTimePermissionEvent::REVOKED_MANUALLY),
1);
}
} // namespace content_settings
Loading

0 comments on commit 71f3935

Please sign in to comment.